The file tools (Read/Write/Edit) open paths inside the SDK process after the
can_use_tool callback validates them, leaving a check-vs-open window. Fully
closing it atomically needs openat2(RESOLVE_BENEATH) file tools replacing the
SDK's in-process ones (tracked as follow-up; the Edit tool's file-state-cache
contract makes that a non-trivial reimplementation).
This test pins the fact that makes the window unreachable under the current
model: an exploit needs a concurrent mutator, the only agent-controlled source
of concurrency is a backgrounded process, and bwrap's PID namespace
(--unshare-all) makes the bash command PID 1 — so the kernel reaps any
backgrounded job when the single bash call exits. The test backgrounds a writer
that would keep appending for ~2s and asserts its output stops the moment the
call returns. With sequential tool execution and per-session bind isolation,
no surviving concurrent mutator exists to win the race.
This is an honest mitigation with evidence, not a claim that the in-process
open is atomic.
The SDK's built-in Bash tool ran commands as a bare subprocess with only
current_dir set — no OS boundary. On a server that let an agent shell read
/etc/shadow, cd /, write anywhere the process uid could, and reach the
network; the phase-1 path validation only guards the file tools, not bash.
(The SDK's SandboxSettings is dead config: Agent::new never reads it.)
Replace it with SandboxedBashTool — same name and input schema, registered
via custom_tools so it overrides the built-in by name — that confines the
command with bubblewrap, gated by permission mode:
- Default / AcceptEdits (review modes): run inside a bwrap namespace with the
system mounted read-only, the session sandbox dir bind-mounted rw at its
real path and made the cwd, /etc //root //home and all other host paths
absent, and --unshare-all (network included). Filesystem escape and network
egress are blocked by the kernel, not by convention.
- FullAccess (explicitly dangerous): unconfined, same as before, but every
command is audit-logged with a warning.
Fail-closed: in a review mode, if bwrap is unavailable the command is refused
rather than silently run unconfined — no fabricated boundary.
Verified end-to-end in tests/agent_bash_sandbox.rs by invoking the tool:
under Default mode `cat /etc/passwd` sees no host passwd, `ls /` exposes no
etc//root//home, a write to /etc/cron.d/x fails (and the host file is never
created), and the network is unreachable; in-sandbox work succeeds and lands
in the real session dir; FullAccess can see the host. bwrap 0.11.2 confirmed
functional on this host (root, CAP_SYS_ADMIN, user namespaces enabled).
Phase-1 suites stay green.
validate_path_within_cwd used exists() to decide whether a path was new.
exists() follows symlinks, so a *dangling* symlink (target missing) reported
false and fell into the not-yet-existing branch, which only bound-checked the
nearest existing ancestor — never the symlink itself. An approved
`ln -s /etc/cron.d/pwn evil` inside the sandbox would therefore pass
validation, and a subsequent Write to `evil` would follow the link and create
a file outside the sandbox. In AcceptEdits mode this chained cleanly: approve
one innocuous-looking symlink (Execute), then writes auto-allow.
Resolve every path component with lstat (symlink_metadata, which does not
follow links) instead of exists():
- Final component: if it is a symlink (dangling or live), resolve its real
target via read_link + canonicalize-nearest-ancestor and reject unless the
target stays within the sandbox.
- Ancestor walk: an intermediate dangling symlink directory is likewise
resolved and bound-checked rather than skipped.
Live symlinks were already caught by canonicalize; this closes the dangling
case. It does not close the check-vs-open TOCTOU (the open happens inside the
SDK tool and would need O_NOFOLLOW/openat2 there); tracked for phase 2 along
with OS-level bash isolation.
Covered by tests/agent_sandbox.rs (dangling symlink to absolute and relative
targets, write tool through a dangling link, write through an intermediate
dangling symlink dir, all rejected; dangling link pointing inside and ordinary
new files still allowed) plus mirrored in-crate unit tests.
The previous approval registry used a oneshot channel, so two independent
signals decided an approval: the HTTP handler's result (from Sender::send's
bool) and the agent loop's allow/deny (from which timeout arm fired). At the
300s boundary these could disagree — a late /approve could see send() return
Ok (receiver not yet dropped) and report ok:true to the user, while the
timeout's final poll simultaneously fired and denied the tool. The frontend
was told "approved" while nothing ran. The mirror case (ok:false but allowed)
was impossible, so it never executed an unapproved tool, but the user-facing
signal could lie.
Replace the channel with a single source of truth: each entry is an
AtomicU8 state machine (Pending -> Approved/Denied/TimedOut) plus a Notify.
The decision is made by exactly one compare_exchange from Pending — whoever
wins (approver or timeout) settles it, everyone else observes a losing CAS
and no-ops. resolve() now returns whether *its* CAS won, so /approve tells
the frontend the truth; the loop decides purely from the settled atomic.
They read the same atomic and cannot disagree.
A PendingGuard removes the registry entry on drop (normal completion or
aborted/disconnected session), so there is no leak on any path. The callback
also emits an approval_resolved SSE event with the final decision so a late
click — already told ok:false — can dismiss its modal with the real outcome.
Verified with an integration test that races resolve() against a near-zero
timeout 5000x and asserts ok==true iff the executed decision is Approved
(never a mixed state). Run via tests/ because the in-crate lib test binary
is currently blocked by unrelated AppConfig test fixtures in other modules.
boos