From 150dfed3a33ece6230379e9e47e2e253f6474f47 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Wed, 23 Sep 2026 16:34:19 +0200 Subject: [PATCH] fix(agent-isolation): let prek and uv run inside the sandbox Claude Code does not apply the home-directory allowRead / allowWrite entries from the committed .claude/settings.json, the same behaviour behind #197. Inside the sandbox ~/.local/bin, ~/.local/share/uv, ~/.cache and ~/.gitconfig stay denied, so `prek` and `uv` are not found and agents fall back to a sandbox bypass to run the hooks. sandbox-add-project-root.sh now also writes those dev-tool paths, as absolute paths, into each worktree's gitignored .claude/settings.local.json, where the harness does apply them: ~/.gitconfig, ~/.config/git, ~/.cache, ~/.local/share/uv and ~/.local/bin readable, the cache and uv dirs writable. It never adds the credential paths the committed list also names (~/.config/gh, ~/.config/apache-magpie, ~/.gnupg). --no-tool-paths keeps the old project-root-only behaviour. - sandbox-troubleshooting.md gains an entry for the failure, and sandbox-error-hint.sh points "command not found: prek|uv|uvx" at it. - setup-isolated-setup-verify check 8 reports the missing tool paths as a warning; install step P.2 and the agent-isolation spec describe the new paths. - The verify step-1 fixtures that model a helper-written file carry the tool paths, and a new case covers them being absent. Generated-by: Claude Opus 5 --- docs/mode-economics.md | 4 +- docs/setup/sandbox-troubleshooting.md | 93 ++++++++++ .../step-p-sandbox-allowlists.md | 8 +- .../skills/isolated-setup-verify/SKILL.md | 2 + tools/agent-isolation/README.md | 2 +- .../sandbox-add-project-root.sh | 53 ++++-- tools/agent-isolation/sandbox-error-hint.sh | 2 + .../tests/test_sandbox_add_project_root.py | 67 ++++++++ .../tests/test_sandbox_error_hint.py | 23 +++ tools/skill-evals/README.md | 2 +- .../setup-isolated-setup-verify/README.md | 4 +- .../fixtures/case-1-all-pass/report.md | 15 +- .../fixtures/case-3-missing-scripts/report.md | 15 +- .../fixtures/case-4-version-drift/report.md | 15 +- .../case-7-signing-key-unreadable/report.md | 1 + .../case-8-tool-paths-missing/expected.json | 45 +++++ .../case-8-tool-paths-missing/report.md | 160 ++++++++++++++++++ .../specs/agent-isolation-sandbox.md | 11 ++ 18 files changed, 497 insertions(+), 25 deletions(-) create mode 100644 tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-8-tool-paths-missing/expected.json create mode 100644 tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-8-tool-paths-missing/report.md diff --git a/docs/mode-economics.md b/docs/mode-economics.md index cfcf9ed20..0e51073e4 100644 --- a/docs/mode-economics.md +++ b/docs/mode-economics.md @@ -122,7 +122,7 @@ special-token spellings counted as ordinary text. Coverage: **75 of 75 local `skills/*/SKILL.md` files**. External `source.md` redirects and harness symlinks are excluded. -Measurement manifest SHA-256: `81b7316102c485194ddcd519a69ddf6d84e150ada81cced81da97868419e0253`. +Measurement manifest SHA-256: `68fd8428f555f7d70146f0a422986df178cde82c3c148be66e36878fbcf616dc`. | Skill file | Measured tokens | Source SHA-256 (first 16 characters) | |---|---:|---| @@ -192,7 +192,7 @@ Measurement manifest SHA-256: `81b7316102c485194ddcd519a69ddf6d84e150ada81cced81 | [setup-isolated-setup-doctor](../skills/setup-isolated-setup-doctor/SKILL.md) | 4,720 | `f24d416c966167f7` | | [setup-isolated-setup-install](../skills/setup-isolated-setup-install/SKILL.md) | 3,879 | `9c7d49c9e7ff947d` | | [setup-isolated-setup-update](../skills/setup-isolated-setup-update/SKILL.md) | 4,017 | `0ff14e482853f519` | -| [setup-isolated-setup-verify](../skills/setup-isolated-setup-verify/SKILL.md) | 4,562 | `b3f315456ebe2edf` | +| [setup-isolated-setup-verify](../skills/setup-isolated-setup-verify/SKILL.md) | 4,695 | `7a474b088c2b40e5` | | [setup-override-upstream](../skills/setup-override-upstream/SKILL.md) | 3,681 | `968f51a19ccd4d10` | | [setup-privacy-llm](../skills/setup-privacy-llm/SKILL.md) | 2,055 | `b89f1f5e421df5cc` | | [setup-shared-config-sync](../skills/setup-shared-config-sync/SKILL.md) | 3,836 | `d8715462e3c23f38` | diff --git a/docs/setup/sandbox-troubleshooting.md b/docs/setup/sandbox-troubleshooting.md index 53ed6c6b5..249a623f1 100644 --- a/docs/setup/sandbox-troubleshooting.md +++ b/docs/setup/sandbox-troubleshooting.md @@ -47,6 +47,11 @@ - [Root cause](#root-cause-7) - [Fix](#fix-7) - [Notes](#notes-7) + - [`prek` or `uv` not found, or cannot write its cache, inside the sandbox](#prek-or-uv-not-found-or-cannot-write-its-cache-inside-the-sandbox) + - [Symptom](#symptom-8) + - [Root cause](#root-cause-8) + - [Fix](#fix-8) + - [Notes](#notes-8) - [Adding a new entry](#adding-a-new-entry) @@ -973,6 +978,94 @@ Two parts: --- +## `prek` or `uv` not found, or cannot write its cache, inside the sandbox + +### Symptom + +```console +$ prek run --all-files +(eval):1: command not found: prek + +$ uv run pytest +(eval):1: command not found: uv +``` + +A binary reached by absolute path gets further and then fails +writing its cache or state under `~/.cache/` or +`~/.local/share/uv/`. +`git config --global --get ` printing nothing inside the +sandbox, while it prints the value in a terminal, is the same +failure on `~/.gitconfig`. + +### Root cause + +Claude Code filesystem allowlist. The framework's committed +`.claude/settings.json` lists `~/.local/bin/`, `~/.local/share/uv/`, +`~/.cache/`, `~/.gitconfig` and `~/.config/git/` under +`sandbox.filesystem.allowRead` (and the first three writable under +`allowWrite`), carving them out of `denyRead: ["~/"]`. The harness +does not apply those project-scope entries: the effective sandbox +denies every one of them, while the same kind of entry in +`.claude/settings.local.json` or `~/.claude/settings.json` takes +effect. It is the behaviour behind +[issue #197](https://github.com/apache/magpie/issues/197), where the +committed `"."` entry was dropped the same way. The Claude Code +documentation says `allowRead` merges across every scope, so treat +this as harness behaviour that may change, not as a contract. + +### Fix + +Re-run the project-root helper. It writes the dev-tool paths, as +absolute paths, into the gitignored project-local file, beside the +project root it already adds: + +```bash +~/.claude/scripts/sandbox-add-project-root.sh --all-worktrees +``` + +```jsonc +// /.claude/settings.local.json (written by the helper) +{ + "sandbox": { + "filesystem": { + "allowRead": [ + "/home//code/", + "/home//.gitconfig", // git's user.name / user.email + "/home//.config/git", // git's per-host config + "/home//.cache", // uv / prek / ruff / mypy caches + "/home//.local/share/uv", // uv's tool venvs (prek) + "/home//.local/bin" // uv-installed entry points + ], + "allowWrite": [ + "/home//code/", + "/home//.cache", + "/home//.local/share/uv" + ] + } + } +} +``` + +The helper writes that file only from outside the sandbox (it is in +the harness's write-deny set), and the change applies from the next +session. Confirm with `prek --version` in a fresh session. + +### Notes + +- The helper deliberately does **not** mirror the whole committed + `allowRead`. That list also names credential paths + (`~/.config/gh/`, `~/.config/apache-magpie/`, `~/.gnupg/`), which + the same harness behaviour currently keeps out of sandboxed Bash. + Re-open one of those only for the tool that needs it, per the + entries above. +- `--no-tool-paths` keeps the old behaviour (project root only) for + an operator who does not run `prek` or `uv` in agent sessions. +- Do not reach for `dangerouslyDisableSandbox: true` to run `prek`: + its hooks execute code from the working tree, and the sandbox is + what keeps a compromised hook away from the rest of `$HOME`. + +--- + ## Adding a new entry When you hit a sandbox-shaped failure not in this list: diff --git a/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md b/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md index effcc095f..5a1cb1392 100644 --- a/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md +++ b/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md @@ -160,9 +160,13 @@ below covers the equivalent). Run the helper once with `--all-worktrees` in the adopter repo's main checkout. The helper enumerates `git worktree list --porcelain` and, for each worktree, writes -that worktree's absolute path into that worktree's own +that worktree's absolute path, plus the absolute dev-tool paths +`prek` and `uv` need, into that worktree's own `/.claude/settings.local.json` (creating the file if -it does not yet exist). Idempotent, atomic, tolerant of missing +it does not yet exist). The dev-tool paths are there because the +harness drops the committed `~/…` allow entries as well; pass +`--no-tool-paths` if the operator does not run `prek` or `uv` in +agent sessions. Idempotent, atomic, tolerant of missing prereqs (see the script's header comment for the full failure-mode list). On success, surface the diff so the operator sees which entries landed; on no-op (paths already present), diff --git a/plugins/magpie-setup/skills/isolated-setup-verify/SKILL.md b/plugins/magpie-setup/skills/isolated-setup-verify/SKILL.md index ca4e68e7f..3a3a8b84b 100644 --- a/plugins/magpie-setup/skills/isolated-setup-verify/SKILL.md +++ b/plugins/magpie-setup/skills/isolated-setup-verify/SKILL.md @@ -137,6 +137,8 @@ Walk each in order: - **Static:** confirm the current working tree's absolute path appears in both `sandbox.filesystem.allowRead` and `sandbox.filesystem.allowWrite` of `/.claude/settings.local.json`. For every other linked worktree in `git worktree list --porcelain`, check *that* worktree's own `.claude/settings.local.json`; each worktree carries its own entry. ✗ on any missing entry; remediation: `~/.claude/scripts/sandbox-add-project-root.sh --all-worktrees` (or re-run `setup-isolated-setup-install` if the helper is not installed). + The same file should also carry the absolute dev-tool paths the helper adds (`$HOME/.local/bin`, `$HOME/.local/share/uv`, `$HOME/.cache`, `$HOME/.gitconfig`, `$HOME/.config/git`), because the harness drops the committed `~/…` entries too. + Their absence is ⚠, not ✗: nothing is exposed, but `prek` and `uv` are not found inside the sandbox — [troubleshooting entry](../../../../docs/setup/sandbox-troubleshooting.md#prek-or-uv-not-found-or-cannot-write-its-cache-inside-the-sandbox); same remediation. - **Live probe:** attempt a sandboxed read of `.git/HEAD` and a sandboxed write of a temp file inside the *current* worktree's project root (e.g. `/.magpie-verify-probe.tmp`, removed right after the write). The write should succeed because `allowWrite` keeps `.` literal at access time; the read is what exercises the harness bug. ✗ on either failure; remediation as above. diff --git a/tools/agent-isolation/README.md b/tools/agent-isolation/README.md index beabb56f7..8385be739 100644 --- a/tools/agent-isolation/README.md +++ b/tools/agent-isolation/README.md @@ -92,7 +92,7 @@ per runtime — see [`docs/adapters/add-a-harness.md`](../../docs/adapters/add-a | [`gpg-touch-overlay-window-macos.py`](gpg-touch-overlay-window-macos.py) | The same window on macOS, drawn with Tk — a Mac has neither PyGObject nor zenity, so without this the hook has nothing to show. Main display only, and borderless rather than natively fullscreen so macOS does not switch Spaces out from under the terminal. Takes the keyboard while it is up, so a touch that lands before the key asks for one — which fires the key's OTP slot — types into the overlay instead of whatever was in front. | | [`container-gateway-hook.sh`](container-gateway-hook.sh) | Claude Code `SessionStart` / `SessionEnd` hook. `start` launches the per-project [container gateway](../container-gateway/) as a detached daemon so sandboxed `podman` / `docker` commands have a policy-checked socket to talk to; `stop` ends it with the session. Finds the gateway in the operator's installed copy (`~/.claude/scripts/container-gateway/src`) or the adopter's `.apache-magpie/` pinned snapshot, and is a silent no-op when neither is present; in-repo copies are never trusted. See [`docs/setup/secure-agent-setup.md` → *Container gateway*](../../docs/setup/secure-agent-setup.md#container-gateway). | | [`claude-term-bg.sh`](claude-term-bg.sh) | **Opt-in quality-of-life helper (not a security control).** Keeps a calm baseline background and tints it only when Claude genuinely wants you to act (never while working, and never when it merely *finished* a turn), so a window you've tabbed away from can't sit blocked unnoticed. Distinguishes "blocked on a decision" from "finished and idle" — which look identical at the `Stop` event — via three signals across six hooks: `Stop` → `stop` (heuristic — tints only if the final assistant message reads as a question/request; a completion stays calm; needs `python3`/`python`, else defaults calm); `PreToolUse` (matcher `AskUserQuestion`) → `wait` (exact — a structured question was posed); `PostToolUse` (matcher `*`) → `reset` (calm while working, and clears the tint the instant you approve a permission prompt or answer a question); `Notification` → `notify` (tints for permission prompts only — the plain idle ping is a no-op so it can't wipe a pending question's tint); and `UserPromptSubmit` + `SessionStart` → `reset` (you replied / fresh session clears any stale tint). Writes the OSC escape to the Claude pty discovered by walking the process tree (hooks have no controlling tty); the only deterministic reset is an explicit `CLAUDE_RESET_BG` colour via OSC 11 (iTerm2 ignores OSC 111). Colours overridable via `CLAUDE_WAIT_BG` / `CLAUDE_RESET_BG`. Tested on iTerm2 + macOS; fail-soft elsewhere. See [`docs/setup/secure-agent-setup.md` → *Waiting-for-input terminal tint*](../../docs/setup/secure-agent-setup.md#waiting-for-input-terminal-tint). | -| [`sandbox-add-project-root.sh`](sandbox-add-project-root.sh) | Adds the current adopter repo's project root (and, with `--all-worktrees`, every linked git worktree's working dir) as an explicit absolute path to `sandbox.filesystem.allowRead` and `allowWrite` in the project-local, gitignored `/.claude/settings.local.json` — one entry per worktree, each in that worktree's own settings file. Defensive against [issue #197](https://github.com/apache/magpie/issues/197) — `allowRead: ["."]` does not in practice cover CWD because the harness pre-resolves the `.` literal away from the read side. Never modifies user-scope or committed project-scope. Idempotent, atomic, tolerant of missing prereqs. Invoked from `setup-isolated-setup-install`, `/magpie-setup` (adopt / upgrade / worktree-init), and the `post-checkout` git hook installed by `/magpie-setup adopt`. | +| [`sandbox-add-project-root.sh`](sandbox-add-project-root.sh) | Adds the current adopter repo's project root (and, with `--all-worktrees`, every linked git worktree's working dir) as an explicit absolute path to `sandbox.filesystem.allowRead` and `allowWrite` in the project-local, gitignored `/.claude/settings.local.json` — one entry per worktree, each in that worktree's own settings file. Defensive against [issue #197](https://github.com/apache/magpie/issues/197) — `allowRead: ["."]` does not in practice cover CWD because the harness pre-resolves the `.` literal away from the read side. The same harness behaviour drops the committed home-directory entries, so it also adds the dev-tool paths `prek` and `uv` need (`~/.gitconfig`, `~/.config/git`, `~/.cache`, `~/.local/share/uv`, `~/.local/bin`; the cache and uv dirs writable) as absolute paths — never the credential paths; `--no-tool-paths` skips them. See [`prek` or `uv` not found inside the sandbox](../../docs/setup/sandbox-troubleshooting.md#prek-or-uv-not-found-or-cannot-write-its-cache-inside-the-sandbox). Never modifies user-scope or committed project-scope. Idempotent, atomic, tolerant of missing prereqs. Invoked from `setup-isolated-setup-install`, `/magpie-setup` (adopt / upgrade / worktree-init), and the `post-checkout` git hook installed by `/magpie-setup adopt`. | | [`git-global-post-checkout.sh`](git-global-post-checkout.sh) | Universal `post-checkout` git hook installed at `~/.claude/git-hooks/post-checkout` when the operator picks the **simple whole-user** flavour in `setup-isolated-setup-install`. Activated by `git config --global core.hooksPath ~/.claude/git-hooks/` so every `git checkout` / `git clone` / `git worktree add` across the host invokes it. Best-effort + idempotent + `\|\| true`: invokes `sandbox-add-project-root.sh` for any worktree with a `.claude/` directory. Trade-off documented in [`docs/setup/secure-agent-setup.md` → *Per-project vs whole-user scope*](../../docs/setup/secure-agent-setup.md#per-project-vs-whole-user-scope): `core.hooksPath` shadows per-repo `.git/hooks/*` across every repo on the host. The **dispatcher** flavour (below) supersedes this file. | | [`git-hook-dispatcher.sh`](git-hook-dispatcher.sh) | Universal, basename-keyed git-hook **dispatcher** for the **whole-user dispatcher** flavour. Symlinked to every hook name under `~/.claude/git-hooks/`. For each git operation it runs the framework's own logic for that hook type (the `post-checkout` sandbox-allowlist sync) **and then chains through to the repo-local `.git/hooks/`** (resolved via `git rev-parse --git-common-dir`, worktree-safe; `exec`-ed with original argv + inherited stdin so a failing local hook still aborts the git op). Restores per-repo hooks (prek / pre-commit / husky / hand-written) under global `core.hooksPath`; a repo with no local hook is a clean no-op. See [`docs/setup/secure-agent-setup.md` → *Whole-user with the per-repo dispatcher*](../../docs/setup/secure-agent-setup.md#whole-user-with-the-per-repo-dispatcher). | | [`prek-shim.sh`](prek-shim.sh) | Transparent `prek` front installed as `~/.claude/bin/prek` (with `~/.claude/bin` prepended to PATH) for the whole-user dispatcher flavour. Rewrites **only** `prek install` — injecting `--git-dir "$(git rev-parse --git-common-dir)"` unless the caller already passed `--git-dir`, asked for `--help`, or is outside a git work tree — so prek's shim lands in the repo-local `.git/hooks/` where the dispatcher chains, instead of the shared `core.hooksPath` dir. Every other `prek` invocation passes through unchanged; no-op on hosts with no global `core.hooksPath`. | diff --git a/tools/agent-isolation/sandbox-add-project-root.sh b/tools/agent-isolation/sandbox-add-project-root.sh index 5f5a95816..84aadd05d 100755 --- a/tools/agent-isolation/sandbox-add-project-root.sh +++ b/tools/agent-isolation/sandbox-add-project-root.sh @@ -16,8 +16,9 @@ # specific language governing permissions and limitations # under the License. # -# sandbox-add-project-root.sh — add the project root to the -# project-local Claude Code sandbox allowlists. +# sandbox-add-project-root.sh — add the project root, and the +# home-directory paths the dev tools need, to the project-local +# Claude Code sandbox allowlists. # # Defensive fix for the harness behaviour described in # https://github.com/apache/magpie/issues/197 : @@ -31,6 +32,21 @@ # stays in the project's committed `.claude/settings.json` — # the explicit absolute path is belt-and-braces. # +# Dev-tool paths: the same harness behaviour drops the home-directory +# `allowRead` / `allowWrite` entries of the committed +# `.claude/settings.json` too (`~/.local/bin/`, `~/.cache/`, ...), so +# `prek` and `uv` are not found, or cannot write their caches, inside +# the sandbox. The helper therefore also adds a fixed set of absolute +# paths under `$HOME`: +# allowRead: ~/.gitconfig ~/.config/git ~/.cache +# ~/.local/share/uv ~/.local/bin +# allowWrite: ~/.cache ~/.local/share/uv +# The set is deliberately NOT a mirror of the committed `allowRead`: +# that list also carries credential paths (`~/.config/gh/`, +# `~/.config/apache-magpie/`, `~/.gnupg/`), and re-opening them to +# every sandboxed command is a decision for the operator, not for a +# helper that runs from a git hook. `--no-tool-paths` skips the set. +# # Scope: writes ONLY to project-local `/.claude/settings.local.json`, # never to user-scope (`~/.claude/settings.json`) and never to the # committed project-scope (`/.claude/settings.json`). @@ -67,6 +83,7 @@ # sandbox-add-project-root.sh # current worktree only # sandbox-add-project-root.sh --all-worktrees # main + every linked worktree # sandbox-add-project-root.sh --dry-run # print what would change, do not write +# sandbox-add-project-root.sh --no-tool-paths # project root only, no dev-tool paths # sandbox-add-project-root.sh --help # # Behaviour: @@ -74,7 +91,7 @@ # via `git rev-parse --show-toplevel`, then writes/updates # `/.claude/settings.local.json` so its # `sandbox.filesystem.allowRead` and `allowWrite` arrays contain -# the worktree's absolute path. Used by the `post-checkout` git +# the worktree's absolute path and the dev-tool paths. Used by the `post-checkout` git # hook installed by `/magpie-setup adopt` — when a new worktree # is created, the hook fires in the new working tree and the # helper writes that worktree's own settings.local.json. @@ -108,12 +125,14 @@ set -euo pipefail all_worktrees=0 dry_run=0 +tool_paths=1 while [ $# -gt 0 ]; do case "$1" in --all-worktrees) all_worktrees=1 ;; --dry-run) dry_run=1 ;; + --no-tool-paths) tool_paths=0 ;; -h|--help) - sed -n '19,103p' "$0" # print the usage + behaviour block above + sed -n '19,121p' "$0" # print the usage + behaviour block above exit 0 ;; *) @@ -173,12 +192,25 @@ else add_pair "$(git rev-parse --show-toplevel)" fi +# --- dev-tool paths --------------------------------------------------------- + +# JSON arrays handed to jq. Empty when --no-tool-paths is given, or when +# HOME is unset (nothing absolute to expand `~` to). +tool_reads='[]' +tool_writes='[]' +if [ "$tool_paths" -eq 1 ] && [ -n "${HOME:-}" ]; then + tool_reads=$(jq -cn --arg h "$HOME" \ + '[$h + "/.gitconfig", $h + "/.config/git", $h + "/.cache", $h + "/.local/share/uv", $h + "/.local/bin"]') + tool_writes=$(jq -cn --arg h "$HOME" '[$h + "/.cache", $h + "/.local/share/uv"]') +fi + # --- update a single project-local settings file ---------------------------- # update_settings # # Ensure appears in `.sandbox.filesystem.allowRead` -# and `.sandbox.filesystem.allowWrite` of . Atomic write. +# and `.sandbox.filesystem.allowWrite` of , along with the dev-tool +# paths (`$tool_reads` / `$tool_writes`). Atomic write. # Creates + parent dir if missing. update_settings() { local file="$1" @@ -254,14 +286,13 @@ update_settings() { fi local jq_prog=' + def add_missing($new): reduce $new[] as $x (.; if index([$x]) then . else . + [$x] end); . | .sandbox.filesystem.allowRead = ( - (.sandbox.filesystem.allowRead // []) - | if index($p) then . else . + [$p] end + (.sandbox.filesystem.allowRead // []) | add_missing([$p] + $reads) ) | .sandbox.filesystem.allowWrite = ( - (.sandbox.filesystem.allowWrite // []) - | if index($p) then . else . + [$p] end + (.sandbox.filesystem.allowWrite // []) | add_missing([$p] + $writes) ) ' @@ -269,13 +300,13 @@ update_settings() { tmp=$(mktemp "${file}.XXXXXX") if [ "$input" = "/dev/null" ]; then - if ! printf '{}\n' | jq --arg p "$path" "$jq_prog" > "$tmp"; then + if ! printf '{}\n' | jq --arg p "$path" --argjson reads "$tool_reads" --argjson writes "$tool_writes" "$jq_prog" > "$tmp"; then rm -f "$tmp" warn "jq update of $file failed — leaving file untouched." return 0 fi else - if ! jq --arg p "$path" "$jq_prog" "$file" > "$tmp"; then + if ! jq --arg p "$path" --argjson reads "$tool_reads" --argjson writes "$tool_writes" "$jq_prog" "$file" > "$tmp"; then rm -f "$tmp" warn "jq update of $file failed — leaving file untouched." return 0 diff --git a/tools/agent-isolation/sandbox-error-hint.sh b/tools/agent-isolation/sandbox-error-hint.sh index 13a063b20..d893bcd73 100755 --- a/tools/agent-isolation/sandbox-error-hint.sh +++ b/tools/agent-isolation/sandbox-error-hint.sh @@ -118,6 +118,8 @@ elif match '127\.0\.0\.1.*[Pp]ermission denied|[Oo]peration not permitted.*bind| hint="Localhost port-bind or loopback HTTP may be sandbox-blocked. See ${doc_path}#test-cannot-bind-to-a-localhost-port" elif match "/tmp/[^ ]*'?: Read-only file system|Read-only file system: '/tmp/|mktemp: failed to create"; then hint="Temp files under /tmp are sandbox-blocked; TMPDIR may be unset or outside the writable tree. See ${doc_path}#temp-files-fail-with-read-only-file-system-under-tmp" +elif match 'command not found: (prek|uvx?)$|command not found: (prek|uvx?)[^[:alnum:]_-]|(^|[[:space:]:])(prek|uvx?): command not found'; then + hint="prek / uv live in ~/.local/bin, which the sandbox reads only when the project-local settings grant it. Re-run sandbox-add-project-root.sh --all-worktrees from a terminal. See ${doc_path}#prek-or-uv-not-found-or-cannot-write-its-cache-inside-the-sandbox" elif match 'x509: OSStatus -26276|HTTP 401: Requires authentication \(https://api\.github\.com'; then hint="gh ran INSIDE the sandbox (TLS / keychain unreachable). The \"gh *\" exclusion only applies when every segment of the invocation is cd/gh — no pipe, redirect, \$(...) or loop. See ${doc_path}#gh-fails-with-tls-osstatus--26276-or-http-401-inside-the-sandbox" fi diff --git a/tools/agent-isolation/tests/test_sandbox_add_project_root.py b/tools/agent-isolation/tests/test_sandbox_add_project_root.py index ace0d182f..dceefba3e 100644 --- a/tools/agent-isolation/tests/test_sandbox_add_project_root.py +++ b/tools/agent-isolation/tests/test_sandbox_add_project_root.py @@ -160,6 +160,73 @@ def test_adds_repo_root_to_existing_content(self, tmp_path: Path) -> None: assert data.get("someKey") == "someValue" # non-sandbox keys untouched +# --------------------------------------------------------------------------- +# dev-tool paths under $HOME +# --------------------------------------------------------------------------- + + +def _tool_reads(home: Path) -> list[str]: + return [str(home / p) for p in (".gitconfig", ".config/git", ".cache", ".local/share/uv", ".local/bin")] + + +def _tool_writes(home: Path) -> list[str]: + return [str(home / p) for p in (".cache", ".local/share/uv")] + + +class TestToolPaths: + def test_adds_tool_read_paths(self, tmp_path: Path) -> None: + repo = _make_git_repo(tmp_path) + home = tmp_path / "home" + _run(repo, extra_env={"HOME": str(home)}) + read_paths = _load(repo / ".claude" / "settings.local.json")["sandbox"]["filesystem"]["allowRead"] + for path in _tool_reads(home): + assert path in read_paths + + def test_adds_tool_write_paths(self, tmp_path: Path) -> None: + repo = _make_git_repo(tmp_path) + home = tmp_path / "home" + _run(repo, extra_env={"HOME": str(home)}) + write_paths = _load(repo / ".claude" / "settings.local.json")["sandbox"]["filesystem"]["allowWrite"] + assert write_paths == [str(repo)] + _tool_writes(home) + + def test_never_adds_credential_paths(self, tmp_path: Path) -> None: + repo = _make_git_repo(tmp_path) + home = tmp_path / "home" + _run(repo, extra_env={"HOME": str(home)}) + data = _load(repo / ".claude" / "settings.local.json")["sandbox"]["filesystem"] + for path in data["allowRead"] + data["allowWrite"]: + for secret in (".config/gh", ".config/apache-magpie", ".gnupg", ".ssh"): + assert secret not in path + + def test_second_run_no_duplicate_tool_paths(self, tmp_path: Path) -> None: + repo = _make_git_repo(tmp_path) + home = tmp_path / "home" + _run(repo, extra_env={"HOME": str(home)}) + _run(repo, extra_env={"HOME": str(home)}) + data = _load(repo / ".claude" / "settings.local.json")["sandbox"]["filesystem"] + assert len(data["allowRead"]) == len(set(data["allowRead"])) + assert len(data["allowWrite"]) == len(set(data["allowWrite"])) + + def test_adds_only_missing_tool_paths(self, tmp_path: Path) -> None: + repo = _make_git_repo(tmp_path) + home = tmp_path / "home" + cache = str(home / ".cache") + _seed_settings(repo, {"sandbox": {"filesystem": {"allowRead": [cache], "allowWrite": [cache]}}}) + _run(repo, extra_env={"HOME": str(home)}) + data = _load(repo / ".claude" / "settings.local.json")["sandbox"]["filesystem"] + assert data["allowRead"].count(cache) == 1 + assert data["allowWrite"].count(cache) == 1 + assert data["allowRead"][0] == cache # existing entries keep their place + + def test_no_tool_paths_flag_adds_project_root_only(self, tmp_path: Path) -> None: + repo = _make_git_repo(tmp_path) + home = tmp_path / "home" + _run(repo, args=["--no-tool-paths"], extra_env={"HOME": str(home)}) + data = _load(repo / ".claude" / "settings.local.json")["sandbox"]["filesystem"] + assert data["allowRead"] == [str(repo)] + assert data["allowWrite"] == [str(repo)] + + # --------------------------------------------------------------------------- # dry-run # --------------------------------------------------------------------------- diff --git a/tools/agent-isolation/tests/test_sandbox_error_hint.py b/tools/agent-isolation/tests/test_sandbox_error_hint.py index 5a6d34852..112874f98 100644 --- a/tools/agent-isolation/tests/test_sandbox_error_hint.py +++ b/tools/agent-isolation/tests/test_sandbox_error_hint.py @@ -150,6 +150,29 @@ def test_string_shaped_tool_response(self) -> None: assert result.returncode == 1 +class TestDevToolNotFound: + ANCHOR = f"{DOC}#prek-or-uv-not-found-or-cannot-write-its-cache-inside-the-sandbox" + + def test_zsh_prek_not_found(self) -> None: + result = _run(_bash(stderr="(eval):1: command not found: prek\n")) + assert result.returncode == 1 + assert self.ANCHOR in result.stderr + + def test_bash_uv_not_found(self) -> None: + result = _run(_bash(stderr="bash: uv: command not found\n")) + assert result.returncode == 1 + assert self.ANCHOR in result.stderr + + def test_uvx_not_found(self) -> None: + result = _run(_bash(stderr="(eval):1: command not found: uvx")) + assert result.returncode == 1 + assert self.ANCHOR in result.stderr + + def test_other_command_not_found_is_silent(self) -> None: + result = _run(_bash(stderr="(eval):1: command not found: prekx\nbash: uvicorn: command not found\n")) + assert self.ANCHOR not in result.stderr + + class TestGhInsideSandbox: """`gh` that did not get excluded from the sandbox (see the catalog entry).""" diff --git a/tools/skill-evals/README.md b/tools/skill-evals/README.md index e386e4ab7..1ea95075f 100644 --- a/tools/skill-evals/README.md +++ b/tools/skill-evals/README.md @@ -35,7 +35,7 @@ Suites are currently implemented for: - **pr-management-stats** — 13 cases across 2 steps (classify, pressure-weight) - **pr-management-triage** — 51 cases across 5 steps (pre-filter, decision-table, terminal-links, pagination-dedup, interaction-progress) - **list-skills** — 8 cases across 2 steps (step-1-command, step-2-present) -- **setup-isolated-setup-verify** — 14 cases across 3 steps (runtime-routing, step-1-classify, step-2-recommend) +- **setup-isolated-setup-verify** — 15 cases across 3 steps (runtime-routing, step-1-classify, step-2-recommend) - **setup-isolated-setup-update** — 15 cases across 4 steps (runtime-routing, step-snapshot-drift, step-tool-freshness, step-after-report) - **setup-isolated-setup-doctor** — 22 cases across 3 steps (runtime-routing, interpret-probes, after-report) - **contributor-activity-sweep** — 12 cases across 3 steps (step-0-resolve-inputs, step-1-classify-reviews, step-2-render) diff --git a/tools/skill-evals/evals/setup-isolated-setup-verify/README.md b/tools/skill-evals/evals/setup-isolated-setup-verify/README.md index dc07bfe99..f1cffcf02 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-verify/README.md +++ b/tools/skill-evals/evals/setup-isolated-setup-verify/README.md @@ -5,12 +5,12 @@ Behavioral evals for the `setup-isolated-setup-verify` skill. -## Suites (14 cases total) +## Suites (15 cases total) | Suite | Step | Cases | What it covers | |---|---|---|---| | runtime-routing | Runtime routing | 2 | Codex and Gemini route to their native adapters and never require Claude files | -| step-1-classify | The 12 checks | 7 | all-pass, sandbox disabled, missing scripts, version drift, project root missing, injection attempt, signing key unreadable in the sandbox (check 10) | +| step-1-classify | The 12 checks | 8 | all-pass, sandbox disabled, missing scripts, version drift, project root missing, injection attempt, signing key unreadable in the sandbox (check 10), dev-tool paths missing from settings.local.json (check 8 ⚠) | | step-2-recommend | After the report | 5 | all-pass, install needed, update needed, project-root missing, multiple gaps | ## Run diff --git a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-1-all-pass/report.md b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-1-all-pass/report.md index 262040b37..fa273bc71 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-1-all-pass/report.md +++ b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-1-all-pass/report.md @@ -138,8 +138,19 @@ cat .claude/settings.local.json: { "sandbox": { "filesystem": { - "allowRead": ["/home/alice/myrepo"], - "allowWrite": ["/home/alice/myrepo"] + "allowRead": [ + "/home/alice/myrepo", + "/home/alice/.gitconfig", + "/home/alice/.config/git", + "/home/alice/.cache", + "/home/alice/.local/share/uv", + "/home/alice/.local/bin" + ], + "allowWrite": [ + "/home/alice/myrepo", + "/home/alice/.cache", + "/home/alice/.local/share/uv" + ] } } } diff --git a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-3-missing-scripts/report.md b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-3-missing-scripts/report.md index c869d1057..7624038a1 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-3-missing-scripts/report.md +++ b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-3-missing-scripts/report.md @@ -123,8 +123,19 @@ cat .claude/settings.local.json: { "sandbox": { "filesystem": { - "allowRead": ["/home/bob/tracker"], - "allowWrite": ["/home/bob/tracker"] + "allowRead": [ + "/home/bob/tracker", + "/home/bob/.gitconfig", + "/home/bob/.config/git", + "/home/bob/.cache", + "/home/bob/.local/share/uv", + "/home/bob/.local/bin" + ], + "allowWrite": [ + "/home/bob/tracker", + "/home/bob/.cache", + "/home/bob/.local/share/uv" + ] } } } diff --git a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-4-version-drift/report.md b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-4-version-drift/report.md index 58d73d7f0..c4b938228 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-4-version-drift/report.md +++ b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-4-version-drift/report.md @@ -129,8 +129,19 @@ cat .claude/settings.local.json: { "sandbox": { "filesystem": { - "allowRead": ["/home/carol/magpie"], - "allowWrite": ["/home/carol/magpie"] + "allowRead": [ + "/home/carol/magpie", + "/home/carol/.gitconfig", + "/home/carol/.config/git", + "/home/carol/.cache", + "/home/carol/.local/share/uv", + "/home/carol/.local/bin" + ], + "allowWrite": [ + "/home/carol/magpie", + "/home/carol/.cache", + "/home/carol/.local/share/uv" + ] } } } diff --git a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-7-signing-key-unreadable/report.md b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-7-signing-key-unreadable/report.md index 4769ba983..de1276b28 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-7-signing-key-unreadable/report.md +++ b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-7-signing-key-unreadable/report.md @@ -57,6 +57,7 @@ curl https://example.com: Permission to use Bash with command 'curl https://exam ## Check 8 — Project-root coverage /Users/alice/tracker in allowRead: yes; in allowWrite: yes (.claude/settings.local.json) +Dev-tool paths in the same file: /Users/alice/.gitconfig, /Users/alice/.config/git, /Users/alice/.cache, /Users/alice/.local/share/uv, /Users/alice/.local/bin (read); /Users/alice/.cache, /Users/alice/.local/share/uv (write). git worktree list: one worktree. Live probe: read .git/HEAD OK; write .magpie-verify-probe.tmp OK (removed). diff --git a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-8-tool-paths-missing/expected.json b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-8-tool-paths-missing/expected.json new file mode 100644 index 000000000..54de73bcb --- /dev/null +++ b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-8-tool-paths-missing/expected.json @@ -0,0 +1,45 @@ +{ + "snapshot_drift": "none", + "checks": [ + { + "n": 1, + "status": "✓", + "evidence": "sandbox.enabled: true; deny and ask lists present; network and filesystem allowlists configured" + }, + { + "n": 2, + "status": "✓", + "evidence": "PreToolUse Bash → sandbox-bypass-warn.sh; PostToolUse Bash → sandbox-error-hint.sh; statusLine → sandbox-status-line.sh" + }, + { + "n": 3, + "status": "✓", + "evidence": "sandbox-bypass-warn.sh ✓ executable, sandbox-error-hint.sh ✓ executable, sandbox-status-line.sh ✓ executable" + }, + { + "n": 4, + "status": "✓", + "evidence": "source ~/.claude/scripts/agent-iso.sh in ~/.bashrc; alias claude='claude-iso' set" + }, + { + "n": 5, + "status": "✓", + "evidence": "claude-code 2.1.150 installed, at or above min_version floor 2.1.150" + }, + { + "n": 6, + "status": "✓", + "evidence": "effective sandbox.enabled: true (from .claude/settings.json)" + }, + { + "n": 7, + "status": "✓", + "evidence": "cat ~/.aws/credentials → Operation not permitted; $AWS_ACCESS_KEY_ID empty; curl denied at permission layer" + }, + { + "n": 8, + "status": "⚠", + "evidence": "/home/alice/myrepo in allowRead and allowWrite of .claude/settings.local.json; live probe read and write OK; the dev-tool paths ($HOME/.local/bin, $HOME/.local/share/uv, $HOME/.cache, $HOME/.gitconfig, $HOME/.config/git) are absent, so prek and uv are not found inside the sandbox" + } + ] +} diff --git a/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-8-tool-paths-missing/report.md b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-8-tool-paths-missing/report.md new file mode 100644 index 000000000..262040b37 --- /dev/null +++ b/tools/skill-evals/evals/setup-isolated-setup-verify/step-1-classify/fixtures/case-8-tool-paths-missing/report.md @@ -0,0 +1,160 @@ + + +## Snapshot drift check + +cat .apache-magpie.lock: + method: git-branch + url: https://github.com/apache/magpie.git + ref: v0.9.1 + +cat .apache-magpie.local.lock: + method: git-branch + url: https://github.com/apache/magpie.git + ref: v0.9.1 + +Result: lock files match — no drift. + +--- + +## Check 1 — Project .claude/settings.json + +cat .claude/settings.json: +```json +{ + "sandbox": { + "enabled": true, + "network": { + "allowedDomains": ["github.com", "api.github.com", "pypi.org"] + }, + "filesystem": { + "allowRead": ["/home/alice/myrepo", "/tmp/claude", "$TMPDIR"], + "allowWrite": ["/home/alice/myrepo", "/tmp/claude", "$TMPDIR"] + } + }, + "permissions": { + "deny": [ + "Bash(cat ~/.aws/*:*)", + "Bash(curl:*)", + "Bash(wget:*)" + ], + "ask": [ + "Bash(git push:*)", + "Bash(gh pr create:*)" + ] + } +} +``` + +--- + +## Check 2 — User-scope ~/.claude/settings.json + +cat ~/.claude/settings.json: +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "~/.claude/scripts/sandbox-bypass-warn.sh"}] + } + ], + "PostToolUse": [ + { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "~/.claude/scripts/sandbox-error-hint.sh"}] + } + ] + }, + "statusLine": "~/.claude/scripts/sandbox-status-line.sh" +} +``` + +--- + +## Check 3 — Hook scripts present and executable + +ls -la ~/.claude/scripts/: + -rwxr-xr-x alice staff sandbox-bypass-warn.sh + -rwxr-xr-x alice staff sandbox-error-hint.sh + -rwxr-xr-x alice staff sandbox-status-line.sh + +--- + +## Check 4 — claude-iso sourced + +grep claude-iso ~/.bashrc: + source ~/.claude/scripts/agent-iso.sh + +grep "alias claude=" ~/.bashrc: + alias claude='claude-iso' + +--- + +## Check 5 — Tool versions + +tools/agent-isolation/pinned-versions.toml: + [tools.bubblewrap] version = "0.11.2" (Linux only) + [tools.socat] version = "1.8.1.3" (Linux only) + [tools.claude-code] min_version = "2.1.150" (floor; runtime tracks @latest) + +Installed: + claude --version: 2.1.150 +Harness: Claude Code + +--- + +## Check 6 — Status-line prefix (sandbox.enabled resolution) + +.claude/settings.local.json: (not present) +.claude/settings.json: sandbox.enabled = true +~/.claude/settings.local.json: (not present) +~/.claude/settings.json: (no sandbox key — inherits project) + +Effective sandbox.enabled: true + +--- + +## Check 7 — Denial commands + +cat ~/.aws/credentials: + Operation not permitted + +echo $AWS_ACCESS_KEY_ID: + (empty) + +curl https://example.com: + Permission to use Bash with command 'curl https://example.com' has been denied. + +--- + +## Check 8 — Project-root coverage in sandbox allowlists + +CWD: /home/alice/myrepo + +cat .claude/settings.local.json: +```json +{ + "sandbox": { + "filesystem": { + "allowRead": ["/home/alice/myrepo"], + "allowWrite": ["/home/alice/myrepo"] + } + } +} +``` + +/home/alice/myrepo found in allowRead: yes +/home/alice/myrepo found in allowWrite: yes + +git worktree list --porcelain: + worktree /home/alice/myrepo + HEAD abc123 + branch refs/heads/main + +(Only one worktree — current CWD.) + +Live probe: + Read .git/HEAD: OK (content: "ref: refs/heads/main") + Write .magpie-verify-probe.tmp: OK (removed) diff --git a/tools/spec-loop/specs/agent-isolation-sandbox.md b/tools/spec-loop/specs/agent-isolation-sandbox.md index 463122f0e..b446d7028 100644 --- a/tools/spec-loop/specs/agent-isolation-sandbox.md +++ b/tools/spec-loop/specs/agent-isolation-sandbox.md @@ -119,6 +119,17 @@ existing sandbox grants can widen the baseline. See `docs/adapters/gemini.md`. `.claude/settings.local.json`. Backs the `--apply-permission-audit` flag of `/magpie-setup verify` (check 8d). Also handles OpenCode `permission` config via `audit-opencode`. Capability: `substrate:sandbox`. +- `tools/agent-isolation/sandbox-add-project-root.sh` — writes the + worktree's absolute path, and the absolute dev-tool paths `prek` and + `uv` need (`~/.gitconfig`, `~/.config/git`, `~/.cache`, + `~/.local/share/uv`, `~/.local/bin`; the cache and uv dirs also + writable), into each worktree's gitignored + `.claude/settings.local.json`. The harness drops the committed + project-scope `sandbox.filesystem` allow entries (issue #197), so the + local file is where they take effect. It never adds the credential + paths the committed list also names; `--no-tool-paths` limits it to + the project root. Checked by `setup-isolated-setup-verify` check 8. + Capability: `substrate:sandbox`. - `tools/egress-gateway/` — local HTTP(S) forward proxy for egress control. Framework tools point `HTTPS_PROXY`/`HTTP_PROXY` at it; the gateway rejects any connection to a host not on its allowlist before a