diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d132985..52e4a89c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ 作られる symlink・ディレクトリと終了コードは変わりません。そうした名前は名前を指定した操作 (`devbase up _foo` など)ができず、そのディレクトリの中で名前なしに打てば動きます。 +### Changed +- **スナップショットの名前が、末尾に改行を持つ値(`abc\n`)を受け付けなくなりました(PLAN66 / #203)。** + スナップショットの名前の検証を、位置引数のプロジェクト名と同じ規則(`utils/names`)へ寄せました。 + それ以外に受け付ける名前と、エラーの文言は変わりません。 + ## [3.6.0] - 2026-09-19 ### Added diff --git a/docs/specifications/cli-argument-resolution.md b/docs/specifications/cli-argument-resolution.md index 1859a40b..1bbf02d5 100644 --- a/docs/specifications/cli-argument-resolution.md +++ b/docs/specifications/cli-argument-resolution.md @@ -309,11 +309,15 @@ Python 側の `_resolve_project_name` は同じ結果になるよう、`chdir` 作らないことと保存先を知らせる。判定は `utils/names.is_single_segment_name`、名前の形の 説明文は `utils/names.NAME_FORM_HINT` の 1 か所にあり、`.` 始まりの名前は今と同じく 同期の対象にならず知らせも出ない -- 名前の検証はリポジトリの中で 1 つに寄せていない。`env/bundle.py` の `is_valid_project_name` - (先頭の `_` を許す。`env` の export / import の書庫の中の名前)、`env/secret_store.py` の - `_validate_project_name`(機密の保存先のファイル名)、`snapshot/manager.py` の `_VALID_NAME_RE` - (スナップショットの名前)はそれぞれ別の用途と互換性を持つ。寄せると受け付ける名前が変わる - 範囲が広がるため、位置引数の解決はこの仕様の規則だけを使う +- 名前の検証はリポジトリの中で 1 つに寄せきっていない。寄せていないのは `env/bundle.py` の + `is_valid_project_name`(先頭の `_` を許す。`env` の export / import の書庫の中の名前)と + `env/secret_store.py` の `_validate_project_name`(機密の保存先のファイル名)の **2 つ**で、 + それぞれ別の用途と互換性を持つ。寄せると受け付ける名前が変わる範囲が広がるため、位置引数の + 解決はこの仕様の規則だけを使う。`snapshot/manager.py` のスナップショットの名前 + (`SnapshotManager._validate_name`)は `utils/names.is_single_segment_name` を共有する + (PLAN66)。文字集合が同じで、寄せても受け付ける名前は広がらない(狭まるのは末尾の改行を + 持つ名前だけ)。例外の型と文言は変えていない。述語を将来広げるとスナップショットの名前も + 広がるため、受理と拒否を `tests/snapshot/test_manager_name.py` で固定している - shell 側は macOS 既定の bash 3.2 で動くこと。`[[ =~ ]]` の右辺は変数で渡す(引用した右辺は 文字列として比べられる)。連想配列・`${var,,}`・`mapfile` を使わない - `cli.py` でサブコマンドを足し引きしたら、`bin/devbase` の `_PROJECT_NAME_SUBCOMMANDS` / diff --git a/issues/PLAN66_project-name-validation-impl2.md b/issues/PLAN66_project-name-validation-impl2.md new file mode 100644 index 00000000..03d16596 --- /dev/null +++ b/issues/PLAN66_project-name-validation-impl2.md @@ -0,0 +1,90 @@ +# PLAN66 実装 2 本目: スナップショットの名前の検証を `utils/names` の述語へ寄せる + +## 関連リンク + +- 課題: devbasex/devbase#203(この Pull Request で閉じる) +- 要求と受け入れ条件: `issues/PLAN66_project-name-validation.md` +- 設計: `issues/PLAN66_project-name-validation-design.md`(設計 PR #230。決定 5・6) +- 実装 1 本目: #233(知らせ。F1・F2。`release/v3.7.0` へマージ済み) +- release PR: #212(base は `release/v3.7.0`) +- この計画が扱うのは設計の「実装の分け方」の **2 本目(スナップショットの寄せ。F3)だけ**である + +## モード + +`standard`(要求の文書の判定のまま。受け付ける名前が狭まるのは末尾の改行 1 ケースだけで、利用者が承認済み)。 + +## 目的と非目的 + +達成したい状態: + +- スナップショット名の文字の規則を `utils/names.is_single_segment_name` の 1 か所に置き、`_VALID_NAME_RE` の二重持ちをやめる +- 共有した述語が将来広がったとき、スナップショット側のテストで気づける + +やらないこと: + +- `SnapshotError` の型と文言(「英数字・ハイフン・アンダースコア・ドットのみ使用可能、先頭は英数字」)の変更。 + `NAME_FORM_HINT` へ寄せる案は採らない(決定 5 の採らなかった案) +- 逆向きの寄せ(`utils/names` が `snapshot` の定数を使う) +- `_safe_snap_dir` の封じ込め(`resolve()` + `startswith`)の変更 +- 末尾の改行以外で受け付ける名前を狭める変更 +- 確定仕様「運用」の 1 つ目の箇条書きと、CHANGELOG の Added の行(1 本目の範囲。書き換えない) + +## 受け入れ条件 + +要求の文書の番号をそのまま使う。この Pull Request が満たすのは 10〜12・13 の 2 つ目と 3 つ目・14 の Changed・17 である。 + +- [ ] 10: `_validate_name("abc\n")` が `SnapshotError`(`tests/snapshot/test_manager_name.py`) +- [ ] 11: 受理 4 件(`ok-name`・`a.b`・`A_b`・`0abc`)が例外にならず、拒否 8 件(`_foo`・`.x`・`-x`・空・`..`・`café`・`a/b`・`abc\n`)が + `SnapshotError` で文言 `無効なスナップショット名` を含む(同上) +- [ ] 12: `grep -n "_VALID_NAME_RE" lib/devbase/snapshot/manager.py` が 0 件 +- [ ] 13 の 2 つ目・3 つ目: `docs/specifications/cli-argument-resolution.md` の「運用」の 2 つ目の箇条書きが、寄せていない規則は + `env/bundle.py` と `env/secret_store.py` の 2 つであること、スナップショットの名前が `utils/names` の述語を共有すること + (共有してよい理由と、広がらないことを固定するテストの在り処)を書く +- [ ] 14 の Changed: `CHANGELOG.md` の `[Unreleased]` に Changed(スナップショットの名前が末尾の改行を受け付けなくなる)を足す +- [ ] 17: `uv run --locked pytest tests/ -q` が exit=0 + +## 修正対象 + +- `lib/devbase/snapshot/manager.py` +- `tests/snapshot/test_manager_name.py`(新設) +- `docs/specifications/cli-argument-resolution.md`(「運用」の 2 つ目の箇条書きだけ) +- `CHANGELOG.md`(`[Unreleased]` に Changed を足すだけ) + +## タスク分解 + +### Task 1: 受理と拒否を固定し、述語へ寄せる(F3) + +- **対象ファイル:** `tests/snapshot/test_manager_name.py`・`lib/devbase/snapshot/manager.py` +- **変更内容:** `pytest.mark.parametrize` で受理 4 件・拒否 8 件を固定するテストを先に書く(`abc\n` だけが今の実装で落ちる)。 + `_VALID_NAME_RE` を消し、`_validate_name` の判定を `not is_single_segment_name(name)` にする(空は述語が弾くため `not name` のガードは消す)。 + 例外の型・文言・`_safe_snap_dir` は変えない。`re` は他の正規表現が使うため import を残す +- **満たす受け入れ条件:** 10・11・12 +- **進め方:** 失敗するテスト → 通す最小実装 → 整理 + +### Task 2: 確定仕様と CHANGELOG + +- **対象ファイル:** `docs/specifications/cli-argument-resolution.md`・`CHANGELOG.md` +- **変更内容:** 「運用」の 2 つ目を設計の「確定仕様の書き換え」の表の 2 行目どおりに書き換える。CHANGELOG の `[Unreleased]` に `### Changed` を足す +- **満たす受け入れ条件:** 13 の 2 つ目・3 つ目・14 の Changed +- **進め方:** 文書のためテスト駆動を適用しない + +## 影響範囲 + +- `SnapshotManager` の名前を受ける入口(`create`・`restore`・`rename`・`delete` など `_safe_snap_dir` を通る経路)。 + 末尾に改行を持つ名前だけが新たに弾かれる + +## リスクと対処 + +| リスク | 対処 | +| --- | --- | +| 受け付ける名前が末尾の改行以外でも変わる | 受理 4 件・拒否 8 件のテストで固定し、文字集合が同じ(`[A-Za-z0-9][A-Za-z0-9._-]*`)ことを差分で確かめる | +| 触る範囲は 1 関数で、テストを新設する | 実装の後の構造改善で足りる | + +## 切り戻し手順 + +データ・スキーマを持たない。この Pull Request の revert で完全に戻る。 + +## 完了の定義 + +- [ ] 受け入れ条件 10〜12 がテストと grep で確かめられ、17 が exit=0 +- [ ] Draft の Pull Request の本文に Test plan と実行結果を載せる diff --git a/lib/devbase/snapshot/manager.py b/lib/devbase/snapshot/manager.py index b3cf5742..ada680e6 100644 --- a/lib/devbase/snapshot/manager.py +++ b/lib/devbase/snapshot/manager.py @@ -12,6 +12,7 @@ from devbase.errors import DevbaseError, SnapshotCommandError, SnapshotError from devbase.log import get_logger +from devbase.utils.names import is_single_segment_name from devbase.volume.manager import ( HOME_UBUNTU_VOLUME, SHARED_VOLUME_PREFIX, @@ -35,7 +36,6 @@ DEFAULT_MAX_GENERATIONS = 3 DEFAULT_MAX_INCREMENTALS = 10 METADATA_FILE = 'snapshot.yml' -_VALID_NAME_RE = re.compile(r'^[a-zA-Z0-9][a-zA-Z0-9._-]*$') # GNU tar の incremental はディレクトリを (dev, ino) で追跡して rename を検出する。 # ディレクトリが削除され作り直されると **inode 番号が再利用される**ため、tar は無関係な @@ -146,8 +146,12 @@ def volumes(self) -> dict: @staticmethod def _validate_name(name: str) -> None: - """スナップショット名のバリデーション(パストラバーサル防止)""" - if not name or not _VALID_NAME_RE.match(name): + """スナップショット名のバリデーション(パストラバーサル防止) + + 文字の規則は位置引数のプロジェクト名と同じ ``is_single_segment_name`` を共有する + (PLAN66 決定 5)。受理と拒否は ``tests/snapshot/test_manager_name.py`` で固定している。 + """ + if not is_single_segment_name(name): raise SnapshotError( f"無効なスナップショット名: '{name}' " "(英数字・ハイフン・アンダースコア・ドットのみ使用可能、先頭は英数字)" diff --git a/tests/snapshot/test_manager_name.py b/tests/snapshot/test_manager_name.py new file mode 100644 index 00000000..4f0dce9e --- /dev/null +++ b/tests/snapshot/test_manager_name.py @@ -0,0 +1,57 @@ +"""スナップショット名の受理と拒否を固定する (PLAN66 決定 6)。 + +``SnapshotManager._validate_name`` は ``utils/names.is_single_segment_name`` を共有する +(決定 5)。述語を将来広げるとスナップショット名も黙って広がるため、ここで受理と拒否を +固定し、広げるときにスナップショット名をどうするかを改めて決められるようにする。 +""" + +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from devbase.errors import SnapshotError +from devbase.snapshot.manager import SnapshotManager + + +@pytest.mark.parametrize('name', ['ok-name', 'a.b', 'A_b', '0abc']) +def test_validate_name_accepts(tmp_path, name): + SnapshotManager(tmp_path)._validate_name(name) + + +@pytest.mark.parametrize( + 'name', + ['_foo', '.x', '-x', '', '..', 'café', 'a/b', 'abc\n'], +) +def test_validate_name_rejects(tmp_path, name): + with pytest.raises(SnapshotError, match='無効なスナップショット名'): + SnapshotManager(tmp_path)._validate_name(name) + + +def test_create_rejects_invalid_name_before_side_effects(tmp_path): + """公開入口で不正名を拒否し、副作用へ進まない現状を固定する。""" + manager = SnapshotManager(tmp_path) + + with patch('devbase.snapshot.manager.subprocess.run') as run: + with pytest.raises(SnapshotError, match='無効なスナップショット名'): + manager.create(name='../evil') + + run.assert_not_called() + + assert list((tmp_path / 'backups').iterdir()) == [] + assert not (tmp_path / 'evil').exists() + + +def test_delete_rejects_invalid_name_before_side_effects(tmp_path): + """公開入口で不正名を拒否し、副作用へ進まない現状を固定する。""" + manager = SnapshotManager(tmp_path) + + with patch('devbase.snapshot.manager.shutil.rmtree') as rmtree: + with pytest.raises(SnapshotError, match='無効なスナップショット名'): + manager.delete(name='_foo') + + rmtree.assert_not_called() + + assert list((tmp_path / 'backups').iterdir()) == [] +