fix(agent-isolation): make the gitignore safety check hook-safe

tools/agent-isolation/sandbox-add-project-root.sh

@@ -192,15 +192,38 @@ update_settings() {
   # .gitignore — if we land here without that entry in place, the
   # adopter setup is incomplete and the user should fix the
   # .gitignore first.
-  if ( cd "$(dirname "$file")" 2>/dev/null \
-       && git check-ignore -q "$file" 2>/dev/null ); then
-    : # ignored — safe to write
-  elif [ -d "$(dirname "$file")/.." ] \
-       && git -C "$(dirname "$file")" rev-parse --show-toplevel >/dev/null 2>&1; then
-    # Inside a git repo and check-ignore returned non-zero (path
-    # is not ignored). Refuse to write.
-    warn "$file is not gitignored — refusing to write. Add /.claude/settings.local.json to the adopter's .gitignore and re-run."
-    return 0
+  #
+  # Both git calls below run the repo discovery themselves, from the
+  # repo root, with the hook environment stripped. That matters: git
+  # hooks export GIT_DIR, and with GIT_DIR set but no GIT_WORK_TREE
+  # git treats the *current directory* as the work-tree root. Any
+  # check run from a subdirectory would then evaluate a root-anchored
+  # pattern like `/.claude/settings.local.json` against `.claude/` as
+  # the root, where it cannot match — so the check reported "not
+  # ignored" for a correctly-ignored file on every invocation from a
+  # hook (post-checkout, post-merge, …), and the helper refused to
+  # write. See the regression tests in
+  # tests/test_sandbox_add_project_root.py.
+  local git_env root probe
+  git_env="env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE"
+
+  # `.claude/` may not exist yet, so discover from the nearest
+  # existing ancestor rather than from the file's own parent.
+  probe=$(dirname "$file")
+  while [ ! -d "$probe" ] && [ "$probe" != "/" ] && [ "$probe" != "." ]; do
+    probe=$(dirname "$probe")
+  done
+
+  root=$($git_env git -C "$probe" rev-parse --show-toplevel 2>/dev/null) || root=""
+
+  if [ -n "$root" ]; then
+    if $git_env git -C "$root" check-ignore -q "$file" 2>/dev/null; then
+      : # ignored — safe to write
+    else
+      # Inside a git repo and the path is genuinely not ignored.
+      warn "$file is not gitignored — refusing to write. Add /.claude/settings.local.json to the adopter's .gitignore and re-run."
+      return 0
+    fi
   fi
   # If the parent dir is not in any git repo, we are running under a
   # caller that already verified that. Allow the write to proceed.

tools/agent-isolation/tests/test_sandbox_add_project_root.py

@@ -221,6 +221,129 @@ def test_not_in_git_repo_creates_no_file(self, tmp_path: Path) -> None:
         assert not (non_git / ".claude" / "settings.local.json").exists()
 
 
+# ---------------------------------------------------------------------------
+# gitignore safety check
+# ---------------------------------------------------------------------------
+
+
+def _git_dir_of(repo: Path) -> str:
+    """Absolute .git dir, as a git hook would export it in GIT_DIR."""
+    return subprocess.run(
+        ["git", "-C", str(repo), "rev-parse", "--absolute-git-dir"],
+        check=True,
+        capture_output=True,
+        text=True,
+    ).stdout.strip()
+
+
+def _no_global_git_config(tmp_path: Path) -> dict:
+    """Env that makes only the repo's own .gitignore count.
+
+    A developer's global excludes commonly already ignore
+    `**/.claude/settings.local.json`, which would mask the "refuses"
+    cases and make these tests pass for the wrong reason on some
+    machines and not others.
+
+    Pointing ``GIT_CONFIG_GLOBAL`` at an empty file is not enough: when
+    ``core.excludesFile`` is unset git still falls back to its default
+    ``$XDG_CONFIG_HOME/git/ignore`` (``~/.config/git/ignore``). So the
+    stand-in global config has to set ``core.excludesFile`` explicitly.
+    """
+    cfg = tmp_path / "gitconfig-isolated"
+    cfg.write_text(f"[core]\n\texcludesFile = {os.devnull}\n")
+    return {"GIT_CONFIG_GLOBAL": str(cfg), "GIT_CONFIG_SYSTEM": os.devnull}
+
+
+def _bare_git_repo(tmp_path: Path) -> Path:
+    """An initialised repo with **no** .gitignore, so nothing is ignored."""
+    repo = tmp_path / "repo"
+    repo.mkdir()
+    subprocess.run(["git", "init", str(repo)], check=True, capture_output=True)
+    return repo
+
+
+class TestGitignoreSafetyCheck:
+    def test_refuses_when_not_gitignored(self, tmp_path: Path) -> None:
+        """A genuinely un-ignored settings file must not be written to.
+
+        Also covers the case where `.claude/` does not exist yet: the
+        check has to resolve the repo from the nearest existing ancestor
+        rather than from the file's own (missing) parent directory.
+        """
+        repo = _bare_git_repo(tmp_path)
+        result = _run(repo, extra_env=_no_global_git_config(tmp_path))
+        assert result.returncode == 0
+        assert "is not gitignored" in result.stderr
+        assert not (repo / ".claude" / "settings.local.json").exists()
+
+    def test_writes_when_git_dir_is_exported(self, tmp_path: Path) -> None:
+        """Regression: the check must survive the git-hook environment.
+
+        Git hooks export GIT_DIR. With GIT_DIR set and no GIT_WORK_TREE,
+        git treats the current directory as the work-tree root, so a
+        check run from a subdirectory evaluated the root-anchored
+        `/.claude/settings.local.json` pattern against `.claude/` as the
+        root and wrongly concluded the file was not ignored.
+        """
+        repo = _make_git_repo(tmp_path)
+        # `.claude/` must already exist, as it does in a real adopter
+        # repo. With it missing the pre-fix code failed its `cd` and
+        # fell through to writing anyway, which would mask the bug.
+        (repo / ".claude").mkdir()
+        result = _run(
+            repo, extra_env={**_no_global_git_config(tmp_path), "GIT_DIR": _git_dir_of(repo)}
+        )
+
+        assert "is not gitignored" not in result.stderr
+        assert result.returncode == 0
+        assert (repo / ".claude" / "settings.local.json").exists()
+
+    def test_git_dir_exported_still_records_repo_root(self, tmp_path: Path) -> None:
+        """The allowlist entry must be the repo root, not the .claude subdir."""
+        repo = _make_git_repo(tmp_path)
+        (repo / ".claude").mkdir()
+        _run(repo, extra_env={**_no_global_git_config(tmp_path), "GIT_DIR": _git_dir_of(repo)})
+        data = _load(repo / ".claude" / "settings.local.json")
+        assert str(repo) in data["sandbox"]["filesystem"]["allowRead"]
+
+    def test_git_dir_exported_still_refuses_when_not_gitignored(self, tmp_path: Path) -> None:
+        """The fix must not turn the safety check into a no-op."""
+        repo = _bare_git_repo(tmp_path)
+        result = _run(
+            repo, extra_env={**_no_global_git_config(tmp_path), "GIT_DIR": _git_dir_of(repo)}
+        )
+
+        assert "is not gitignored" in result.stderr
+        assert not (repo / ".claude" / "settings.local.json").exists()
+
+    def test_works_in_linked_worktree(self, tmp_path: Path) -> None:
+        """The real-world trigger: a hook firing inside a git worktree."""
+        repo = _make_git_repo(tmp_path)
+        subprocess.run(
+            ["git", "-C", str(repo), "add", ".gitignore"], check=True, capture_output=True
+        )
+        subprocess.run(
+            ["git", "-C", str(repo), "-c", "user.email=t@e.st", "-c", "user.name=t",
+             "-c", "commit.gpgsign=false", "commit", "-m", "init"],
+            check=True,
+            capture_output=True,
+        )
+        wt = tmp_path / "wt"
+        subprocess.run(
+            ["git", "-C", str(repo), "worktree", "add", "-q", str(wt), "-b", "wt"],
+            check=True,
+            capture_output=True,
+        )
+
+        (wt / ".claude").mkdir()
+        result = _run(wt, extra_env={**_no_global_git_config(tmp_path), "GIT_DIR": _git_dir_of(wt)})
+
+        assert "is not gitignored" not in result.stderr
+        assert (wt / ".claude" / "settings.local.json").exists()
+        data = _load(wt / ".claude" / "settings.local.json")
+        assert str(wt) in data["sandbox"]["filesystem"]["allowRead"]
+
+
 # ---------------------------------------------------------------------------
 # missing jq
 # ---------------------------------------------------------------------------