diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py index 79eddff65..ba2d0c3ba 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py @@ -21,4 +21,4 @@ for installs that do not carry the framework source (see `isolated.py`). """ -FRAMEWORK_FINGERPRINT = "sha256:be5231e45d7c001e" +FRAMEWORK_FINGERPRINT = "sha256:1590e3d94aa8c3c0" diff --git a/tools/agent-isolation/gpg-touch-overlay.sh b/tools/agent-isolation/gpg-touch-overlay.sh index 64396155d..0534eae08 100755 --- a/tools/agent-isolation/gpg-touch-overlay.sh +++ b/tools/agent-isolation/gpg-touch-overlay.sh @@ -515,7 +515,7 @@ _write_context() { } _forget_context() { - rm -f "$CONTEXT_DIR/$1" 2>/dev/null || true + rm -f "$CONTEXT_DIR/$1" "$CONTEXT_DIR/$1.signer" 2>/dev/null || true } # A context whose owner is gone left its watcher behind, and a watcher @@ -785,6 +785,7 @@ wrap() { # raced or skipped. MAGPIE_GPG_TOUCH_WRAPPED_SIGNER=$signer MAGPIE_GPG_TOUCH_PARENT=$$ \ MAGPIE_GPG_TOUCH_CONTEXT="$CONTEXT_DIR/$id" \ + MAGPIE_GPG_TOUCH_SIGNER_FILE="$CONTEXT_DIR/$id.signer" \ "${SESSION_LAUNCHER[@]}" "$SELF" _watch >>"$log" 2>&1 & own=$! _register "$id" "$$" "$own" @@ -798,10 +799,20 @@ wrap() { # window it spawned, the pid itself for a watcher too young to have # called setsid (a signature that returns in milliseconds ends # before it has). - trap 'kill -- -'"$own"' '"$own"' 2>/dev/null; rm -f "'"$OWNERS_DIR/$id"'" "'"$CONTEXT_DIR/$id"'" 2>/dev/null; _cleanup_if_idle' EXIT + trap 'kill -- -'"$own"' '"$own"' 2>/dev/null; rm -f "'"$OWNERS_DIR/$id"'" "'"$CONTEXT_DIR/$id"'" "'"$CONTEXT_DIR/$id.signer"'" 2>/dev/null; _cleanup_if_idle' EXIT - "$real" "$@" - return $? + local status=0 + if [[ -n $signer ]]; then + # A new foreground shell publishes its own pid; exec preserves it. + # The wrapper remains the lifetime owner and keeps stdin attached. + # Publication failure must never prevent the signer from running. + "$BASH" -c 'printf "%s\n" "$$" 2>/dev/null >"$1" || :; shift; exec "$@"' \ + _ "$CONTEXT_DIR/$id.signer" "$real" "$@" || status=$? + else + "$real" "$@" || status=$? + fi + rm -f "$CONTEXT_DIR/$id.signer" 2>/dev/null + return "$status" } # ------------------------------------------------------------ watcher --- @@ -819,6 +830,16 @@ overlay_signers="" # its pid fails with EPERM, which would read as the signature having ended. signer_pids() { if [[ -n ${MAGPIE_GPG_TOUCH_WRAPPED_SIGNER:-} && -n ${MAGPIE_GPG_TOUCH_PARENT:-} ]]; then + local pid="" + if [[ -n ${MAGPIE_GPG_TOUCH_SIGNER_FILE:-} && -r $MAGPIE_GPG_TOUCH_SIGNER_FILE ]]; then + read -r pid <"$MAGPIE_GPG_TOUCH_SIGNER_FILE" || true + if [[ $pid =~ ^[0-9]+$ ]] && kill -0 "$pid" 2>/dev/null; then + printf '%s\n' "$pid" + return 0 + fi + fi + # A missing or failed publication costs an extra reminder, never + # an empty authentication marker that could hide another touch. printf '%s\n' "$MAGPIE_GPG_TOUCH_PARENT" return 0 fi @@ -952,7 +973,7 @@ _watch() { # The loop itself is silent, so a log is only worth having as a trace. _debugging && set -x - local i blocked=0 sockets baseline rows + local i blocked=0 sockets baseline rows dismissed # Connections to the agent that already exist are somebody else's — # a stuck client, a session in another terminal — and are not what @@ -990,6 +1011,16 @@ _watch() { else blocked=0 hide_overlay + # An empty dismissal answered the authentication that ended, + # not another one starting within the TTL. The existing lease + # protects the check and removal from a concurrent signer + # publishing a nonempty dismissal. Leave those markers alone. + if [[ -e $DISMISSED_MARKER ]] && _lease_acquire; then + if dismissed="$(cat "$DISMISSED_MARKER" 2>/dev/null)" && [[ -z $dismissed ]]; then + rm -f "$DISMISSED_MARKER" 2>/dev/null + fi + _lease_release + fi # Nothing is signing here: a window that closed unanswered was # for the signature that just ended. reshow_at=0 diff --git a/tools/agent-isolation/tests/test_gpg_touch_overlay.py b/tools/agent-isolation/tests/test_gpg_touch_overlay.py index 5c9757721..3a45c8bf1 100644 --- a/tools/agent-isolation/tests/test_gpg_touch_overlay.py +++ b/tools/agent-isolation/tests/test_gpg_touch_overlay.py @@ -30,6 +30,7 @@ import json import os import shutil +import signal import socket import subprocess import tempfile @@ -918,7 +919,7 @@ def test_wrap_passes_a_verify_call_straight_through(tmp_path: Path) -> None: shutil.which("git") is None or shutil.which("ssh-keygen") is None, reason="needs git and ssh-keygen", ) -def test_git_signs_a_commit_through_the_wrapper(tmp_path: Path) -> None: +def test_git_signs_a_commit_through_the_wrapper(tmp_path: Path, process_named) -> None: """End to end: `gpg.ssh.program` names the symlink and git commits. A software key stands in for the hardware one, so the real @@ -2012,3 +2013,210 @@ def test_no_dismissal_outlasts_its_minute(tmp_path: Path, process_named) -> None assert not _dismissal_holds(tmp_path, f"{bystander.pid}\n", age_minutes=2), ( "a dismissal outlived its minute because a named process stayed alive" ) + + +def _wrapped_test_signer(tmp_path: Path, window: Path) -> tuple[subprocess.Popen, Path, dict]: + """Run a real named child through the wrapper, without a key or display.""" + binary_dir = tmp_path / "bin" + binary_dir.mkdir() + named_dir = tmp_path / "named" + named_dir.mkdir() + named = named_dir / "ssh-keygen" + named.symlink_to(shutil.which("sleep")) + pid_file = tmp_path / "actual-signer" + program = binary_dir / "ssh-keygen" + program.write_text(f'#!/bin/sh\nprintf "%s\\n" "$$" > "{pid_file}"\nexec "{named}" 30\n') + program.chmod(0o755) + env = { + **os.environ, + **NO_CARD, + "PATH": f"{binary_dir}:{os.environ['PATH']}", + "XDG_RUNTIME_DIR": str(tmp_path / "runtime"), + "MAGPIE_GPG_TOUCH_DRY_RUN": "1", + "MAGPIE_GPG_TOUCH_OVERLAY": str(window), + } + proc = subprocess.Popen( + ["bash", str(SCRIPT), "wrap", "ssh-keygen", "-Y", "sign"], + env=env, + start_new_session=True, + ) + return proc, pid_file, env + + +def _end_wrapped_test_signer(proc: subprocess.Popen) -> None: + """Also end an orphaned signer after the abnormal-owner test.""" + try: + os.killpg(proc.pid, signal.SIGTERM) + except ProcessLookupError: + # The group already exited; proc.wait() below still reaps the wrapper. + pass + proc.wait(timeout=5) + + +@_needs_a_quiet_signer +def test_wrapped_and_hook_watchers_share_one_dismissal(tmp_path: Path, process_named) -> None: + """Both watchers answer one real signature, then remind for the next.""" + window, shown = _fake_window(tmp_path, 0) + wrapper, actual, _env = _wrapped_test_signer(tmp_path, window) + hook = None + marker = tmp_path / "runtime" / "magpie-gpg-touch" / "dismissed" + try: + assert _wait_for(actual.exists, timeout=5) + signer = int(actual.read_text()) + assert subprocess.run(["pgrep", "-x", "ssh-keygen"], capture_output=True, check=False).returncode == 0 + hook = _start_watcher(tmp_path, window) + assert _wait_for(marker.exists, timeout=10) + # Give the second watcher several polls to reject an inconsistent + # marker. This fails on the base by observing the duplicate window. + time.sleep(2) + assert wrapper.poll() is None and hook.poll() is None + assert _showings(shown) == 1, "the other watcher re-showed the dismissed signature" + assert marker.read_text().split() == [str(signer)] + os.kill(signer, signal.SIGTERM) + wrapper.wait(timeout=5) + process_named("gpg", seconds=30) + assert _wait_for(lambda: _showings(shown) >= 2, timeout=10), "the next signature got no reminder" + finally: + _end_wrapped_test_signer(wrapper) + if hook is not None: + hook.terminate() + hook.wait(timeout=5) + + +@_needs_a_quiet_signer +def test_each_ssh_authentication_gets_its_own_reminder(tmp_path: Path, process_named) -> None: + """Actual accepted Unix connections end and restart inside the TTL.""" + window, shown = _fake_window(tmp_path, 0) + path = str(tmp_path / "agent.sock") + runtime = tmp_path / "runtime" / "magpie-gpg-touch" + runtime.mkdir(parents=True) + trace = tmp_path / "trace" + env = { + **os.environ, + "XDG_RUNTIME_DIR": str(tmp_path / "runtime"), + "SSH_AUTH_SOCK": path, + "MAGPIE_GPG_TOUCH_OVERLAY": str(window), + "MAGPIE_GPG_TOUCH_DEBUG": "1", + } + with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as server, trace.open("w") as output: + server.bind(path) + server.listen() + watcher = subprocess.Popen(["bash", str(SCRIPT), "_watch"], env=env, stderr=output) + try: + assert _wait_for(lambda: "+ baseline=" in trace.read_text(), timeout=5) + with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as client: + client.connect(path) + accepted, _address = server.accept() + with accepted: + assert _wait_for((runtime / "dismissed").exists, timeout=10) + assert _showings(shown) == 1 + offset = len(trace.read_text()) + # Wait for the watcher's actual idle branch, not a guessed delay. + assert _wait_for(lambda: "+ reshow_at=0" in trace.read_text()[offset:], timeout=5) + with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as client: + client.connect(path) + accepted, _address = server.accept() + with accepted: + assert _wait_for(lambda: _showings(shown) >= 2, timeout=10), ( + "the first authentication's empty dismissal hid the second" + ) + finally: + watcher.terminate() + watcher.wait(timeout=5) + + +@_needs_a_quiet_signer +@pytest.mark.parametrize("abnormal", [False, True], ids=["group-signal", "dead-owner-sweep"]) +def test_wrapped_signer_record_is_cleaned_up(tmp_path: Path, process_named, abnormal: bool) -> None: + """Normal signalling or sweeping a dead owner forgets its signer PID.""" + window, _shown = _fake_window(tmp_path, 0) + wrapper, actual, env = _wrapped_test_signer(tmp_path, window) + state = tmp_path / "runtime" / "magpie-gpg-touch" + record = state / "context" / f"p-{wrapper.pid}.signer" + registration = state / "owners" / f"p-{wrapper.pid}" + try: + assert _wait_for(actual.exists, timeout=5) + assert _wait_for(record.exists, timeout=5) + assert record.read_text().strip() == actual.read_text().strip() + if abnormal: + wrapper.kill() + wrapper.wait(timeout=5) + subprocess.run( + ["bash", str(SCRIPT), "disarm"], + input=json.dumps({"session_id": "cleanup-other-owner"}), + text=True, + env=env, + check=True, + ) + else: + os.killpg(wrapper.pid, signal.SIGTERM) + wrapper.wait(timeout=5) + assert not record.exists() + assert not registration.exists() + finally: + _end_wrapped_test_signer(wrapper) + + +@_needs_a_quiet_signer +def test_failed_signer_publication_preserves_input_status_and_reminder(tmp_path: Path, process_named) -> None: + """An obstructed context directory costs no input, status or reminder.""" + window, shown = _fake_window(tmp_path, 0) + state = tmp_path / "runtime" / "magpie-gpg-touch" + state.mkdir(parents=True) + (state / "context").write_text("obstruct publication without relying on permissions") + binary_dir = tmp_path / "bin" + binary_dir.mkdir() + received = tmp_path / "received" + program = binary_dir / "gpg" + program.write_text(f'#!/bin/sh\ncat > "{received}"\nsleep 4\nexit 7\n') + program.chmod(0o755) + env = { + **os.environ, + **NO_CARD, + "PATH": f"{binary_dir}:{os.environ['PATH']}", + "XDG_RUNTIME_DIR": str(tmp_path / "runtime"), + "MAGPIE_GPG_TOUCH_DRY_RUN": "1", + "MAGPIE_GPG_TOUCH_OVERLAY": str(window), + } + wrapper = subprocess.Popen( + ["bash", str(SCRIPT), "wrap", "gpg", "-bsau"], + env=env, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + start_new_session=True, + ) + try: + wrapper.stdin.write("synthetic signing input\n") + wrapper.stdin.close() + wrapper.stdin = None + marker = state / "dismissed" + assert _wait_for(marker.exists, timeout=3) + assert marker.read_text().split() == [str(wrapper.pid)] + assert _showings(shown) >= 1 + _stdout, stderr = wrapper.communicate(timeout=6) + assert wrapper.returncode == 7 + assert received.read_text() == "synthetic signing input\n" + assert stderr == "" + finally: + _end_wrapped_test_signer(wrapper) + + +@_needs_a_quiet_signer +def test_idle_watcher_preserves_a_nonempty_signer_dismissal(tmp_path: Path, process_named) -> None: + """Only empty auth markers are cleared; a named live bystander remains.""" + window, _shown = _fake_window(tmp_path, 0) + state = tmp_path / "runtime" / "magpie-gpg-touch" + state.mkdir(parents=True) + marker = state / "dismissed" + with subprocess.Popen(["sleep", "30"]) as bystander: + marker.write_text(f"{bystander.pid}\n") + watcher = _start_watcher(tmp_path, window) + try: + time.sleep(1) + assert marker.read_text() == f"{bystander.pid}\n" + finally: + watcher.terminate() + watcher.wait(timeout=5) + bystander.terminate() diff --git a/tools/spec-loop/specs/agent-isolation-sandbox.md b/tools/spec-loop/specs/agent-isolation-sandbox.md index f5365ff9e..79790ea85 100644 --- a/tools/spec-loop/specs/agent-isolation-sandbox.md +++ b/tools/spec-loop/specs/agent-isolation-sandbox.md @@ -174,7 +174,12 @@ See `docs/adapters/goose.md`. names the signer processes it was made for, stops counting when one of them ends or an unnamed signer starts, and never lasts past `DISMISS_TTL_MIN`, so the next signature asks again even while other - sessions stay armed. + sessions stay armed. Wrapped and hook watchers name the same real signer + PID; the wrapper retains its own PID for watcher ownership. A failed child + PID publication falls back to the owner, costing an extra reminder rather + than hiding a touch. An empty ssh-authentication dismissal is cleared when + the watcher observes the authentication end, under the shared window lease; + a nonempty signer dismissal is preserved (#1834). A window that closed unanswered (it lost the keyboard and exited with `OVERLAY_UNANSWERED`, could not be drawn, or crashed) is no dismissal and is shown again after `RESHOW_DELAY` while the signature still blocks