Skip to content

fix(sandbox): stop a stale path grant from breaking bash_exec entirely#117

Merged
juacker merged 2 commits into
mainfrom
fix/stale-grant-sandbox
Jul 23, 2026
Merged

fix(sandbox): stop a stale path grant from breaking bash_exec entirely#117
juacker merged 2 commits into
mainfrom
fix/stale-grant-sandbox

Conversation

@juacker

@juacker juacker commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the bug where one stale filesystem grant disabled bash_exec entirely for an agent.

Problem

An always-scoped path grant can outlive its target — e.g. the user deletes a workspace directory that was granted earlier. Both sandbox backends then hard-failed up front in validate_grants_exist(), so every sandboxed command aborted with:

Sandbox path grant does not exist: <path>

There is no agent-facing revoke tool, so the agent could never recover from its own stale grant (observed 2026-07-05; tracked in TODO under bugs/reliability).

Fix

Replace the hard fail with pruning: grants whose host path no longer exists are dropped from the profile (one tracing::warn! per drop) and the sandbox launches without them. The stale path is then simply absent inside the sandbox — the same ENOENT an unsandboxed shell would hit at use time. Pruning can only ever remove access, never widen it.

  • linux_bwrap: validate_grants_existprune_missing_grants, keeping the Flatpak host-namespace probe (flatpak-spawn --host test -e). Grant binds were already lenient (*-bind-try), so bwrap itself never needed the hard gate.
  • macos_seatbelt: same prune; add_grant_filters now also skips a grant that fails canonicalize (TOCTOU between prune and profile build) instead of erroring the command.
  • Workspace-root validation stays fatal on both backends — a missing workspace is a genuine error.

Tests

  • Prune unit tests on both backends (stale dropped, live kept — stale path derived from a dropped tempdir, not a hardcoded literal).
  • Seatbelt: profile builds despite a missing grant, and the stale path never appears in the rendered profile.
  • Linux: end-to-end regression — real bwrap launch with a missing grant must succeed.

cargo test --lib: 805 passed. cargo clippy --lib: clean.

Review

3 independent reviews (Codex / Claude / Minimax), all production_quality, zero blocker/major. Applied minor: derive stale test paths from dropped tempdirs. Accepted residual risk: the Flatpak fail-open probe branch is untested (would need to mock flatpak-spawn; behavior is fail-safe, bounded by *-bind-try).

Follow-up (out of scope, tracked in TODO)

  • Agent-facing grant revoke / self-heal so stale always grants can be cleaned from settings, plus a user-visible notice when a grant is pruned.

juacker added 2 commits July 23, 2026 11:41
An always-scoped filesystem grant can outlive its target (e.g. the user
deletes a workspace that was granted earlier). Both sandbox backends
hard-failed on that up front — validate_grants_exist() aborted EVERY
sandboxed command with "Sandbox path grant does not exist: <path>", so
one stale grant disabled bash_exec for the whole agent with no recovery
path (observed 2026-07-05).

Replace the hard fail with pruning: drop grants whose host path no
longer exists (warn per pruned grant) and launch the sandbox without
them. The stale path is then simply absent inside the sandbox — the
same ENOENT an unsandboxed shell would hit at use time. Pruning only
ever removes access, never widens it.

- linux_bwrap: validate_grants_exist -> prune_missing_grants (keeps the
  Flatpak host-namespace probe; grant binds were already lenient
  *-bind-try, so bwrap itself never needed the hard gate)
- macos_seatbelt: same prune; add_grant_filters now also skips a grant
  that vanishes between prune and profile build (TOCTOU) instead of
  erroring the command
- tests: prune keeps live grants / drops stale ones (both backends),
  seatbelt profile builds without the stale path, plus an end-to-end
  bwrap regression run with a missing grant

The agent-facing revoke tool for stale grants remains a separate
follow-up (tracked in TODO).
@juacker
juacker marked this pull request as ready for review July 23, 2026 10:01
@juacker
juacker merged commit 55b0666 into main Jul 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant