From 9e209bb024280f1a4c31d602d8341438019a83f8 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 22 Sep 2026 14:17:30 -0700 Subject: [PATCH 1/2] fix(init): confine .bmad-loop, policy.toml and .gitignore writes (#771) install_into guarded only the hook-config and skill-tree writes. The .bmad-loop mkdir, the fallback policy.toml write and the .gitignore append went through whatever link sat at the name. A symlink or junction out of the project, or a dangling policy.toml link that fails is_file(), made init write outside the tree and still report success. All three destinations are now checked with _confined_to before init's first write. An escape prints FAIL naming the path and returns 1, with no hook config, skills, policy or gitignore change left behind. Strictly-below is the right predicate for each path: - .bmad-loop: a directory that resolves to the root would drop policy.toml at top level. The policy check alone misses that, because root/policy.toml is still below the root. - policy.toml and .gitignore: these name files, and the root is a directory no write should aim at. An in-project link still passes and is written through. The .gitignore append stays an append, now opened by its resolved name, so it keeps the file mode and leaves an in-project link a link. The new policy keeps write_text rather than atomic_write_text. That branch runs only when no regular file exists, so a torn write loses nothing the operator owns (#379's concern). atomic_write_text would also mint the file 0600 instead of the umask default. --- CHANGELOG.md | 3 + src/bmad_loop/install.py | 28 ++++++++- tests/test_install.py | 128 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c47b5a4..80ed778cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,9 @@ breaking changes may land in a minor release. orchestrator resolved (`BMAD_LOOP_LEDGER`) rather than re-deriving it from the YAML (#154, #769). +- Refuse `init` when `.bmad-loop`, its `policy.toml` or `.gitignore` resolves outside the + project, before any setup write (#771). + - Replace stale installed relay hooks when a project moves between Windows and POSIX. - Report stale or unverifiable Codex hook trust in `validate` and `probe-adapter` diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index bf6c575f7..6f9e93624 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -2948,6 +2948,22 @@ def install_into( return 1 bmad_loop_dir = project / ".bmad-loop" + policy_path = bmad_loop_dir / "policy.toml" + gitignore = project / ".gitignore" + # 0. confinement, before the FIRST write (#771). `_register_hooks` and + # `_copy_skills` guard their own destinations, but these three were written + # through whatever link sat at the name — a `.bmad-loop` or `.gitignore` + # symlink (a junction on Windows) out of the tree, or a dangling `policy.toml` + # link that fails `is_file()` below and is then written through. Checked up + # front, not at each write, so a refusal leaves no hook config or skills behind + # either. Strictly-below is right for all three: none may BE the project root — + # a `.bmad-loop` resolving to the root would drop policy.toml at top level, and + # the other two are files, which the root never is. An in-project link still + # passes and is written through, as before. + for target in (bmad_loop_dir, policy_path, gitignore): + if not _confined_to(target, project): + print(f"FAIL: init target escapes the project: {target}") + return 1 bmad_loop_dir.mkdir(parents=True, exist_ok=True) # 1. per-CLI hook registration @@ -2967,10 +2983,15 @@ def install_into( return 1 # 4. policy template - policy_path = bmad_loop_dir / "policy.toml" if policy_path.is_file(): print(" policy exists, leaving untouched") else: + # write_text, not atomic_write_text: #379 is about a truncating REWRITE of + # contents someone owns, and this branch only runs when no regular file is + # there — a short write loses nothing but our own template, and the torn + # TOML fails loudly at the next policy load. atomic_write_text would also + # mint the new file mkstemp's 0600 instead of the umask default, a mode + # change nothing asked for. policy_path.write_text(POLICY_TEMPLATE, encoding="utf-8") print(f" policy written: {policy_path}") @@ -2979,7 +3000,6 @@ def install_into( # Library (.bmad-loop/cache/), and the policy file itself — policy.toml is # per-machine-per-repo (it carries this machine's [mux] backend choice, and # the TUI settings editor rewrites it), so it must never travel to teammates. - gitignore = project / ".gitignore" existing = gitignore.read_text(encoding="utf-8") if gitignore.is_file() else "" have = set(existing.splitlines()) to_add = [ @@ -2993,7 +3013,9 @@ def install_into( if line not in have ] if to_add: - with gitignore.open("a", encoding="utf-8") as f: + # An append, never a replace: it keeps the operator's file mode and an + # in-project link a link. Opened by its resolved name, the one step 0 confined. + with gitignore.resolve().open("a", encoding="utf-8") as f: if existing and not existing.endswith("\n"): f.write("\n") f.write("\n".join(to_add) + "\n") diff --git a/tests/test_install.py b/tests/test_install.py index e9406849c..0f602c71a 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1344,6 +1344,134 @@ def test_register_hooks_refuses_a_config_path_symlinked_out_of_the_project(tmp_p assert "escapes the project" in capsys.readouterr().out +def _assert_init_left_no_state(project: Path, capsys) -> None: + # the #771 refusal runs before init's first write: no hook config, no skills, + # no policy, no gitignore lines — and no `init complete` over a failed setup + out = capsys.readouterr().out + assert "FAIL: init target escapes the project" in out + assert "init complete" not in out + claude = get_profile("claude") + assert not (project / claude.hooks.config_path).exists() + assert not (project / claude.skill_tree).exists() + assert not (project / "policy.toml").exists() + gitignore = project / ".gitignore" + assert not gitignore.exists() or ".bmad-loop/runs/" not in gitignore.read_text() + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") +def test_init_refuses_a_bmad_loop_dir_symlinked_out_of_the_project(tmp_path, capsys): + project, outside = tmp_path / "proj", tmp_path / "outside" + project.mkdir() + outside.mkdir() + (project / ".bmad-loop").symlink_to(outside, target_is_directory=True) + + assert install_into(project) == 1 + + assert list(outside.iterdir()) == [] # no policy.toml through the link + _assert_init_left_no_state(project, capsys) + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") +def test_init_refuses_a_bmad_loop_dir_that_resolves_to_the_project_root(tmp_path, capsys): + # The row that grades the `.bmad-loop` check ALONE: an out-of-project link is + # also caught by the policy.toml check (it resolves through the same link), but + # a link back to the root leaves policy.toml strictly below the project — only + # the strictly-below test on the directory itself refuses the top-level write. + project = tmp_path / "proj" + project.mkdir() + (project / ".bmad-loop").symlink_to(project, target_is_directory=True) + + assert install_into(project) == 1 + + _assert_init_left_no_state(project, capsys) + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows junctions") +def test_init_refuses_a_bmad_loop_dir_junctioned_out_of_the_project(tmp_path, capsys): + # the redirect an unprivileged Windows session can plant without elevation + import _winapi + + project, outside = tmp_path / "proj", tmp_path / "outside" + project.mkdir() + outside.mkdir() + _winapi.CreateJunction(str(outside), str(project / ".bmad-loop")) + + assert install_into(project) == 1 + + assert list(outside.iterdir()) == [] + _assert_init_left_no_state(project, capsys) + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") +def test_init_refuses_a_dangling_policy_symlink_out_of_the_project(tmp_path, capsys): + # dangling: `is_file()` is False, so the unguarded path wrote THROUGH it + project, outside = tmp_path / "proj", tmp_path / "outside" + (project / ".bmad-loop").mkdir(parents=True) + outside.mkdir() + (project / ".bmad-loop" / "policy.toml").symlink_to(outside / "policy.toml") + + assert install_into(project) == 1 + + assert not (outside / "policy.toml").exists() + _assert_init_left_no_state(project, capsys) + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") +def test_init_refuses_a_gitignore_symlinked_out_of_the_project(tmp_path, capsys): + project, outside = tmp_path / "proj", tmp_path / "outside.gitignore" + project.mkdir() + outside.write_text("node_modules/", encoding="utf-8") + before = outside.read_bytes() + (project / ".gitignore").symlink_to(outside) + + assert install_into(project) == 1 + + assert outside.read_bytes() == before + _assert_init_left_no_state(project, capsys) + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") +def test_init_appends_through_a_gitignore_symlinked_inside_the_project(tmp_path): + # an in-repo indirection the operator arranged: appended through, still a link, + # the target's content and mode intact — the ablation partner of the refusal + project = tmp_path / "proj" + real = project / "config" / "ignore" + real.parent.mkdir(parents=True) + real.write_text("node_modules/\n", encoding="utf-8") + real.chmod(0o640) + link = project / ".gitignore" + link.symlink_to(real) + + assert install_into(project, skills=False) == 0 + + assert link.is_symlink() + text = real.read_text(encoding="utf-8") + assert text.startswith("node_modules/\n") + assert ".bmad-loop/runs/" in text and ".bmad-loop/policy.toml" in text + assert stat.S_IMODE(real.stat().st_mode) == 0o640 + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX file modes") +def test_init_gitignore_append_preserves_content_mode_and_is_idempotent(tmp_path): + # characterization of the ordinary-file path the #771 guard must not cost + gitignore = tmp_path / ".gitignore" + gitignore.write_bytes(b"node_modules/\n*.log") # no trailing newline + gitignore.chmod(0o640) + + assert install_into(tmp_path, skills=False) == 0 + + assert gitignore.read_bytes() == ( + b"node_modules/\n*.log\n" + b".bmad-loop/runs/\n.bmad-loop/cache/\n.bmad-loop/policy.toml\n" + + f"{RENDER_DIR_REL}/\n".encode() + ) + assert stat.S_IMODE(gitignore.stat().st_mode) == 0o640 + after_first = gitignore.read_bytes() + + assert install_into(tmp_path, skills=False) == 0 + assert gitignore.read_bytes() == after_first + + # ------------------------------------------------ atomic hook-config rewrite (#379) # # `_register_hooks` and `provision_worktree` both PARSE the operator's hook config, From 8f4da43b0b269cf6d7ed6f750544cea6988d6e44 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 22 Sep 2026 19:52:30 -0700 Subject: [PATCH 2/2] test(install): run the #771 init confinement tests in the project sandbox The refusal rows now snapshot the sandbox's git status (ignored paths included) after planting the link and require it unchanged after the refused init, instead of probing a hand-rolled tmp_path tree. --- tests/test_install.py | 108 +++++++++++++++++++++++------------------- 1 file changed, 58 insertions(+), 50 deletions(-) diff --git a/tests/test_install.py b/tests/test_install.py index 0f602c71a..147639392 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1344,105 +1344,113 @@ def test_register_hooks_refuses_a_config_path_symlinked_out_of_the_project(tmp_p assert "escapes the project" in capsys.readouterr().out -def _assert_init_left_no_state(project: Path, capsys) -> None: - # the #771 refusal runs before init's first write: no hook config, no skills, - # no policy, no gitignore lines — and no `init complete` over a failed setup +def _worktree_snapshot(root: Path) -> str: + # every path git can see, ignored ones included: an init write anywhere in the + # sandbox — hook config, skills, a top-level policy.toml — changes this listing + return git(root, "status", "--porcelain", "--ignored", "--untracked-files=all") + + +def _assert_init_left_no_state(root: Path, before: str, capsys) -> None: + # the #771 refusal runs before init's first write: the sandbox reads exactly as + # it did once the link was planted — and no `init complete` over a failed setup out = capsys.readouterr().out assert "FAIL: init target escapes the project" in out assert "init complete" not in out + assert _worktree_snapshot(root) == before claude = get_profile("claude") - assert not (project / claude.hooks.config_path).exists() - assert not (project / claude.skill_tree).exists() - assert not (project / "policy.toml").exists() - gitignore = project / ".gitignore" - assert not gitignore.exists() or ".bmad-loop/runs/" not in gitignore.read_text() + assert not (root / claude.hooks.config_path).exists() + assert not (root / claude.skill_tree).exists() @pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") -def test_init_refuses_a_bmad_loop_dir_symlinked_out_of_the_project(tmp_path, capsys): - project, outside = tmp_path / "proj", tmp_path / "outside" - project.mkdir() +def test_init_refuses_a_bmad_loop_dir_symlinked_out_of_the_project(project, tmp_path, capsys): + root, outside = project.project, tmp_path / "outside" outside.mkdir() - (project / ".bmad-loop").symlink_to(outside, target_is_directory=True) + (root / ".bmad-loop").symlink_to(outside, target_is_directory=True) + before = _worktree_snapshot(root) - assert install_into(project) == 1 + assert install_into(root) == 1 assert list(outside.iterdir()) == [] # no policy.toml through the link - _assert_init_left_no_state(project, capsys) + _assert_init_left_no_state(root, before, capsys) @pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") -def test_init_refuses_a_bmad_loop_dir_that_resolves_to_the_project_root(tmp_path, capsys): +def test_init_refuses_a_bmad_loop_dir_that_resolves_to_the_project_root(project, capsys): # The row that grades the `.bmad-loop` check ALONE: an out-of-project link is # also caught by the policy.toml check (it resolves through the same link), but # a link back to the root leaves policy.toml strictly below the project — only # the strictly-below test on the directory itself refuses the top-level write. - project = tmp_path / "proj" - project.mkdir() - (project / ".bmad-loop").symlink_to(project, target_is_directory=True) + root = project.project + (root / ".bmad-loop").symlink_to(root, target_is_directory=True) + before = _worktree_snapshot(root) - assert install_into(project) == 1 + assert install_into(root) == 1 - _assert_init_left_no_state(project, capsys) + assert not (root / "policy.toml").exists() + _assert_init_left_no_state(root, before, capsys) @pytest.mark.skipif(sys.platform != "win32", reason="Windows junctions") -def test_init_refuses_a_bmad_loop_dir_junctioned_out_of_the_project(tmp_path, capsys): +def test_init_refuses_a_bmad_loop_dir_junctioned_out_of_the_project(project, tmp_path, capsys): # the redirect an unprivileged Windows session can plant without elevation import _winapi - project, outside = tmp_path / "proj", tmp_path / "outside" - project.mkdir() + root, outside = project.project, tmp_path / "outside" outside.mkdir() - _winapi.CreateJunction(str(outside), str(project / ".bmad-loop")) + _winapi.CreateJunction(str(outside), str(root / ".bmad-loop")) + before = _worktree_snapshot(root) - assert install_into(project) == 1 + assert install_into(root) == 1 assert list(outside.iterdir()) == [] - _assert_init_left_no_state(project, capsys) + _assert_init_left_no_state(root, before, capsys) @pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") -def test_init_refuses_a_dangling_policy_symlink_out_of_the_project(tmp_path, capsys): +def test_init_refuses_a_dangling_policy_symlink_out_of_the_project(project, tmp_path, capsys): # dangling: `is_file()` is False, so the unguarded path wrote THROUGH it - project, outside = tmp_path / "proj", tmp_path / "outside" - (project / ".bmad-loop").mkdir(parents=True) + root, outside = project.project, tmp_path / "outside" + (root / ".bmad-loop").mkdir() outside.mkdir() - (project / ".bmad-loop" / "policy.toml").symlink_to(outside / "policy.toml") + (root / ".bmad-loop" / "policy.toml").symlink_to(outside / "policy.toml") + before = _worktree_snapshot(root) - assert install_into(project) == 1 + assert install_into(root) == 1 assert not (outside / "policy.toml").exists() - _assert_init_left_no_state(project, capsys) + _assert_init_left_no_state(root, before, capsys) @pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") -def test_init_refuses_a_gitignore_symlinked_out_of_the_project(tmp_path, capsys): - project, outside = tmp_path / "proj", tmp_path / "outside.gitignore" - project.mkdir() +def test_init_refuses_a_gitignore_symlinked_out_of_the_project(project, tmp_path, capsys): + root, outside = project.project, tmp_path / "outside.gitignore" outside.write_text("node_modules/", encoding="utf-8") - before = outside.read_bytes() - (project / ".gitignore").symlink_to(outside) + before_bytes = outside.read_bytes() + (root / ".gitignore").unlink() # the sandbox's tracked one, swapped for the link + (root / ".gitignore").symlink_to(outside) + before = _worktree_snapshot(root) - assert install_into(project) == 1 + assert install_into(root) == 1 - assert outside.read_bytes() == before - _assert_init_left_no_state(project, capsys) + assert outside.read_bytes() == before_bytes + _assert_init_left_no_state(root, before, capsys) @pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") -def test_init_appends_through_a_gitignore_symlinked_inside_the_project(tmp_path): +def test_init_appends_through_a_gitignore_symlinked_inside_the_project(project): # an in-repo indirection the operator arranged: appended through, still a link, # the target's content and mode intact — the ablation partner of the refusal - project = tmp_path / "proj" - real = project / "config" / "ignore" - real.parent.mkdir(parents=True) + root = project.project + real = root / "config" / "ignore" + real.parent.mkdir() real.write_text("node_modules/\n", encoding="utf-8") real.chmod(0o640) - link = project / ".gitignore" + link = root / ".gitignore" + link.unlink() link.symlink_to(real) - assert install_into(project, skills=False) == 0 + assert install_into(root, skills=False) == 0 assert link.is_symlink() text = real.read_text(encoding="utf-8") @@ -1452,13 +1460,13 @@ def test_init_appends_through_a_gitignore_symlinked_inside_the_project(tmp_path) @pytest.mark.skipif(os.name == "nt", reason="POSIX file modes") -def test_init_gitignore_append_preserves_content_mode_and_is_idempotent(tmp_path): +def test_init_gitignore_append_preserves_content_mode_and_is_idempotent(project): # characterization of the ordinary-file path the #771 guard must not cost - gitignore = tmp_path / ".gitignore" + gitignore = project.project / ".gitignore" gitignore.write_bytes(b"node_modules/\n*.log") # no trailing newline gitignore.chmod(0o640) - assert install_into(tmp_path, skills=False) == 0 + assert install_into(project.project, skills=False) == 0 assert gitignore.read_bytes() == ( b"node_modules/\n*.log\n" @@ -1468,7 +1476,7 @@ def test_init_gitignore_append_preserves_content_mode_and_is_idempotent(tmp_path assert stat.S_IMODE(gitignore.stat().st_mode) == 0o640 after_first = gitignore.read_bytes() - assert install_into(tmp_path, skills=False) == 0 + assert install_into(project.project, skills=False) == 0 assert gitignore.read_bytes() == after_first