diff --git a/docs/setup/sandbox-troubleshooting.md b/docs/setup/sandbox-troubleshooting.md index 6fbf445f..53ed6c6b 100644 --- a/docs/setup/sandbox-troubleshooting.md +++ b/docs/setup/sandbox-troubleshooting.md @@ -22,26 +22,31 @@ - [Root cause](#root-cause-2) - [Fix](#fix-2) - [Notes](#notes-2) - - [Test cannot bind to a localhost port](#test-cannot-bind-to-a-localhost-port) + - [Signed commit fails with the agent refusing, and the overlay never appeared](#signed-commit-fails-with-the-agent-refusing-and-the-overlay-never-appeared) - [Symptom](#symptom-3) - [Root cause](#root-cause-3) - [Fix](#fix-3) - [Notes](#notes-3) - - [Docker / Podman command fails with a socket error](#docker--podman-command-fails-with-a-socket-error) + - [Test cannot bind to a localhost port](#test-cannot-bind-to-a-localhost-port) - [Symptom](#symptom-4) - [Root cause](#root-cause-4) - [Fix](#fix-4) - [Notes](#notes-4) - - [Temp files fail with "Read-only file system" under `/tmp`](#temp-files-fail-with-read-only-file-system-under-tmp) + - [Docker / Podman command fails with a socket error](#docker--podman-command-fails-with-a-socket-error) - [Symptom](#symptom-5) - [Root cause](#root-cause-5) - [Fix](#fix-5) - [Notes](#notes-5) - - [`gh` fails with TLS `OSStatus -26276` or `HTTP 401` inside the sandbox](#gh-fails-with-tls-osstatus--26276-or-http-401-inside-the-sandbox) + - [Temp files fail with "Read-only file system" under `/tmp`](#temp-files-fail-with-read-only-file-system-under-tmp) - [Symptom](#symptom-6) - [Root cause](#root-cause-6) - [Fix](#fix-6) - [Notes](#notes-6) + - [`gh` fails with TLS `OSStatus -26276` or `HTTP 401` inside the sandbox](#gh-fails-with-tls-osstatus--26276-or-http-401-inside-the-sandbox) + - [Symptom](#symptom-7) + - [Root cause](#root-cause-7) + - [Fix](#fix-7) + - [Notes](#notes-7) - [Adding a new entry](#adding-a-new-entry) @@ -453,6 +458,64 @@ session state. could not read the key file, here it cannot read the program. Both fail in well under a second, before the key is asked for anything. +## Signed commit fails with the agent refusing, and the overlay never appeared + +### Symptom + +A `git commit` the agent runs gets through every pre-commit hook and +then dies at the signature, with a write error from the wrapper +immediately before it and no touch window at any point: + +```text +error: /Users//.claude/scripts/gpg-touch-wrap-ssh-keygen: line 340: /tmp/magpie-gpg-touch/watcher.pid: Operation not permitted +Signing file /tmp/claude-/.git_signing_buffer_tmpXXXXXX +Couldn't sign message (signer): agent refused operation? +fatal: failed to write commit object +``` + +The same commit succeeds when run outside the sandbox, or from your own +terminal. + +### Root cause + +The overlay keeps its owners registry, window lease, pid and log files +in `$XDG_RUNTIME_DIR/magpie-gpg-touch`. macOS sets no +`XDG_RUNTIME_DIR`, and the fallback used to be `/tmp`, which is outside +the sandbox's write set. The wrapper cannot create its pid file, so no +watcher starts; nothing puts a window on screen; the key is never +touched; and gpg-agent gives up with `agent refused operation`. + +The error names the *watcher*, not the key, which is what makes this +read like a broken signing setup rather than a sandbox denial. + +### Fix + +Update the framework. The fallback is now +`${XDG_CACHE_HOME:-$HOME/.cache}/magpie-gpg-touch`, which is per-user, +not world-writable, and inside the reference `allowWrite`, so the +watcher starts under the sandbox with no widening. + +A stale `/tmp/magpie-gpg-touch/` left by an older version is harmless +and can be removed. + +Do **not** point the overlay at `$TMPDIR` instead. It differs between +the signing contexts that have to find one another — the agent's hooks +see the harness's scratch directory, a terminal `git` sees the login +one — and two contexts computing two runtime directories cannot share +an owners registry or a window lease. + +### Notes + +- The `/tmp` fallback was also a local-security weakness independent of + the sandbox: `/tmp` is world-writable, so another user on the machine + could pre-create the directory and sit on the pid files and the lock + the window is leased through. +- Distinct from the two entries above: there git could not read the key + or could not exec the wrapper, and both failed instantly. Here the + wrapper runs, the signature is genuinely attempted, and the failure + arrives only once the agent stops waiting for a touch that was never + prompted for. + ## Test cannot bind to a localhost port ### Symptom diff --git a/docs/setup/secure-agent-setup.md b/docs/setup/secure-agent-setup.md index 51db8e43..bd641ec2 100644 --- a/docs/setup/secure-agent-setup.md +++ b/docs/setup/secure-agent-setup.md @@ -2225,7 +2225,8 @@ hook's `disarm`, so a second signature in the same command — a rebase replaying several commits, a real signature after a hook ran something that merely looked like one — raises the window again. `MAGPIE_GPG_TOUCH_DEBUG=1` makes the watcher log to `$XDG_RUNTIME_DIR/magpie-gpg-touch/watcher.log` -(`/tmp/magpie-gpg-touch/` on macOS, which sets no `XDG_RUNTIME_DIR`). +(`~/.cache/magpie-gpg-touch/` on macOS, which sets no +`XDG_RUNTIME_DIR`; `$XDG_CACHE_HOME` is honoured when set). So does a marker file, `touch $XDG_RUNTIME_DIR/magpie-gpg-touch/debug` — the way to get a log out of the watcher the *hook* spawns, whose environment is the harness's own and takes no variable from your diff --git a/tools/agent-isolation/gpg-touch-overlay.sh b/tools/agent-isolation/gpg-touch-overlay.sh index 9588e49d..a176779a 100755 --- a/tools/agent-isolation/gpg-touch-overlay.sh +++ b/tools/agent-isolation/gpg-touch-overlay.sh @@ -133,7 +133,31 @@ readonly SHOW_DELAY=8 # polls a signature must block before showing (0.2s ea readonly MAX_WAIT=600 # seconds the watcher may live, whatever happens readonly POLL=0.2 -RUNTIME_DIR="${XDG_RUNTIME_DIR:-/tmp}/magpie-gpg-touch" +# Where the owners registry, the window lease and the watcher's pid and +# log files live. `XDG_RUNTIME_DIR` is the right home and is what Linux +# provides: per-user, 0700, tmpfs-backed, cleared at logout. +# +# macOS sets no `XDG_RUNTIME_DIR`, and the fallback used to be `/tmp`, +# which is wrong twice over. It is world-writable, so another local user +# can pre-create the directory and sit on the pid files and the lock the +# window is leased through. And it is outside the write set of the +# sandbox the framework ships, so a signed commit from a sandboxed agent +# dies at `watcher.pid: Operation not permitted` before the watcher ever +# starts -- no window, no touch, and gpg-agent refuses the signature. +# +# `$TMPDIR` is not the answer despite being per-user and writable: it +# differs between the signing contexts that have to find each other. The +# agent's hooks see the harness's scratch dir, a terminal `git` sees the +# login one. Two contexts computing two runtime dirs cannot share an +# owners registry or a window lease, which is the whole design. The +# cache directory is stable for a user whatever spawned the process, and +# everything kept here is regenerable -- pid files swept by the next arm, +# a lock reclaimed when its holder dies. +if [[ -n ${XDG_RUNTIME_DIR:-} ]]; then + RUNTIME_DIR="$XDG_RUNTIME_DIR/magpie-gpg-touch" +else + RUNTIME_DIR="${XDG_CACHE_HOME:-$HOME/.cache}/magpie-gpg-touch" +fi # One registration per signing context, never one pid file for all of # them. Two agent sessions sign at the same time often enough, and a @@ -760,6 +784,9 @@ case "${1:-}" in _gi_python) _gi_python ;; _tk_python) _tk_python ;; _gui_available) _gui_available ;; + # Test seam: report where state would be kept, so the fallback can be + # asserted against the script's own value rather than a copy of it. + _runtime_dir) printf '%s\n' "$RUNTIME_DIR"; exit 0 ;; _signing_in_flight) signing_in_flight ;; _agent_sockets) agent_sockets ;; _agent_socket_rows) agent_socket_rows "$(printf '%s\n' "${@:2}")" ;; diff --git a/tools/agent-isolation/sandbox-error-hint.sh b/tools/agent-isolation/sandbox-error-hint.sh index 50447c28..13a063b2 100755 --- a/tools/agent-isolation/sandbox-error-hint.sh +++ b/tools/agent-isolation/sandbox-error-hint.sh @@ -103,7 +103,12 @@ doc_path="docs/setup/sandbox-troubleshooting.md" match() { printf '%s' "$output" | grep -qE "$1"; } -if match 'Could not open a connection to your authentication agent|agent refused operation|ssh-add: error fetching identities for protocol|Permission denied \(publickey\)'; then +# Must precede the ssh-agent branch: this failure ends in "agent refused +# operation" too, and that branch would claim it and send the reader to +# the wrong entry. The write error names the watcher, not the key. +if match 'magpie-gpg-touch/[^ ]*: Operation not permitted'; then + hint="The touch overlay could not write its runtime state, so no watcher started and the key was never prompted for a touch. See ${doc_path}#signed-commit-fails-with-the-agent-refusing-and-the-overlay-never-appeared" +elif match 'Could not open a connection to your authentication agent|agent refused operation|ssh-add: error fetching identities for protocol|Permission denied \(publickey\)'; then hint="SSH agent / Yubikey appears unreachable from inside the sandbox. See ${doc_path}#ssh-agent--yubikey-appears-unreachable-from-inside-the-sandbox" elif match "cannot exec '[^']*gpg-touch-(wrap-[^']*|overlay\.sh wrap [^']*)': Operation not permitted|bash: [^ ]*gpg-touch-(wrap-[^ :]*|overlay\.sh): Operation not permitted"; then hint="git cannot start the touch-overlay wrapper from inside the sandbox (~/.claude/scripts/ is read-denied). See ${doc_path}#signed-commit-fails-with-cannot-exec-of-the-touch-overlay-wrapper" diff --git a/tools/agent-isolation/tests/test_gpg_touch_overlay.py b/tools/agent-isolation/tests/test_gpg_touch_overlay.py index 1256b6ad..db884d5e 100644 --- a/tools/agent-isolation/tests/test_gpg_touch_overlay.py +++ b/tools/agent-isolation/tests/test_gpg_touch_overlay.py @@ -190,6 +190,44 @@ def test_disarm_is_silent_when_nothing_is_armed(tmp_path: Path) -> None: assert result.stderr == "" +def _runtime_dir(env_overrides: dict[str, str]) -> str: + """Where the script itself says it keeps its state.""" + env = {k: v for k, v in os.environ.items() if not k.startswith("XDG_")} + env.update(env_overrides) + result = subprocess.run( + ["bash", str(SCRIPT), "_runtime_dir"], + capture_output=True, + text=True, + env=env, + ) + assert result.returncode == 0, result.stderr + return result.stdout.strip() + + +def test_runtime_dir_prefers_xdg_runtime_dir(tmp_path: Path) -> None: + assert _runtime_dir({"XDG_RUNTIME_DIR": str(tmp_path)}) == ( + f"{tmp_path}/magpie-gpg-touch" + ) + + +def test_runtime_dir_never_falls_back_to_world_writable_tmp(tmp_path: Path) -> None: + # /tmp is world-writable, so another local user can pre-create the + # directory and sit on the pid files and the window lease. It is also + # outside the write set of the sandbox the framework ships, which made + # a signed commit die at `watcher.pid: Operation not permitted` before + # the watcher ever started -- no window, no touch, refused signature. + resolved = _runtime_dir({"HOME": str(tmp_path)}) + assert resolved == f"{tmp_path}/.cache/magpie-gpg-touch" + assert resolved not in ("/tmp/magpie-gpg-touch", "/private/tmp/magpie-gpg-touch") + + +def test_runtime_dir_honours_xdg_cache_home(tmp_path: Path) -> None: + cache = tmp_path / "elsewhere" + assert _runtime_dir({"HOME": str(tmp_path), "XDG_CACHE_HOME": str(cache)}) == ( + f"{cache}/magpie-gpg-touch" + ) + + def test_unknown_mode_is_rejected() -> None: result = subprocess.run( ["bash", str(SCRIPT), "wibble"], capture_output=True, text=True diff --git a/tools/agent-isolation/tests/test_sandbox_error_hint.py b/tools/agent-isolation/tests/test_sandbox_error_hint.py index f320e366..5a6d3485 100644 --- a/tools/agent-isolation/tests/test_sandbox_error_hint.py +++ b/tools/agent-isolation/tests/test_sandbox_error_hint.py @@ -111,6 +111,25 @@ def test_touch_overlay_wrapper_symlinked_script_signature(self) -> None: assert result.returncode == 1 assert f"{DOC}#signed-commit-fails-with-cannot-exec-of-the-touch-overlay-wrapper" in result.stderr + def test_overlay_runtime_dir_denied_beats_the_ssh_agent_branch(self) -> None: + # This failure also ends in "agent refused operation", which the + # ssh-agent branch matches. The reader must land on the overlay + # entry instead: the key is fine, the watcher never started. + result = _run( + _bash( + stderr="error: /Users/alice/.claude/scripts/gpg-touch-wrap-ssh-keygen: " + "line 340: /tmp/magpie-gpg-touch/watcher.pid: Operation not permitted\n" + "Couldn't sign message (signer): agent refused operation?\n" + "fatal: failed to write commit object" + ) + ) + assert result.returncode == 1 + assert ( + f"{DOC}#signed-commit-fails-with-the-agent-refusing-and-the-overlay-never-appeared" + in result.stderr + ) + assert "ssh-agent--yubikey" not in result.stderr + def test_docker_signature(self) -> None: result = _run(_bash(stderr="Cannot connect to the Docker daemon at unix:///var/run/docker.sock")) assert result.returncode == 1 diff --git a/tools/spec-loop/.last-sync b/tools/spec-loop/.last-sync index b762e770..df2c2a69 100644 --- a/tools/spec-loop/.last-sync +++ b/tools/spec-loop/.last-sync @@ -1 +1 @@ -5a6e150981dd7f1660ad7178c409e2270f0d6b84 +0e7012bac91017e2255a4a34cdf7c47e84438767 diff --git a/tools/spec-loop/specs/agent-isolation-sandbox.md b/tools/spec-loop/specs/agent-isolation-sandbox.md index 372f4fe8..b1ae6796 100644 --- a/tools/spec-loop/specs/agent-isolation-sandbox.md +++ b/tools/spec-loop/specs/agent-isolation-sandbox.md @@ -60,7 +60,16 @@ existing sandbox grants can widen the baseline. See `docs/adapters/gemini.md`. the wrapper only runs the program and the hook's watcher shows the window, and across contexts the window is leased by atomic directory create, so only one watcher draws it and a lease left by a watcher - that died is reclaimed. The + that died is reclaimed. That registry, the lease, and the watcher's + pid and log files live in `$XDG_RUNTIME_DIR/magpie-gpg-touch`, else + `${XDG_CACHE_HOME:-$HOME/.cache}/magpie-gpg-touch` on a platform that + sets no `XDG_RUNTIME_DIR`. The fallback has to be per-user rather than + `/tmp`, which is world-writable and outside the reference + `allowWrite` — a sandboxed signed commit died there at + `watcher.pid: Operation not permitted` before any watcher started — + and it has to be stable across contexts rather than `$TMPDIR`, which + differs between the agent's hooks and a terminal `git` and would give + two contexts two registries that cannot see each other. The git the agent runs reads the same global config, so the wrapper's two files are a `sandbox.filesystem.allowRead` grant of their own (nothing wider under `~/.claude/`), or every sandboxed signed commit