Skip to content

fix(#182): read-only commands may read through dependency links; the Claude hook no longer claims Seatbelt - #226

Open
Lotus015 wants to merge 1 commit into
mainfrom
fix/182-dep-read-classification
Open

Lotus015 wants to merge 1 commit into
mainfrom
fix/182-dep-read-classification

Conversation

@Lotus015

Copy link
Copy Markdown
Collaborator

Fixes #182

Summary

#189 fixed the refusal text. This PR covers the two items the issue thread left open.

1. Dependency reads are allowed on every lane; writes are still refused.
Before this change, any operand that reached a manager-owned dependency link (node_modules, .venv, vendor) was allowed only when the command ran under Seatbelt. As the design note on #182 explains, the guard can't tell a read from a write by looking at a path token. So on Linux/hosted lanes, ls node_modules/.bin, ls ./node_modules and cat node_modules/x were all refused. That accounts for 3 of the 7 refusals in run-116-mubbgmhx.

The guard now looks at the command that owns the operand, using option (a) from the design note. A small allowlist of commands that have no option to write or spawn on a file operand may read through the link without the sandbox: cat head tail ls wc grep egrep fgrep diff cmp stat du readlink realpath test [ md5sum sha1sum sha256sum shasum.

  • Only the bare name counts. A project-local scripts/cat is not treated as the system cat.
  • The classification resets at every |, && and ;. So in cat x | tee node_modules/y, the tee operand is still refused.
  • Redirection targets and cd targets never get the exception.
  • Everything else still needs Seatbelt to reach the link: cp, mv, tee, touch, ln, find, sort -o and the opaque interpreters.
  • The link's real external target is still refused when it is named directly.
  • Running a tool through the link (node_modules/.bin/vitest …) already passed the guard. A test now pins that behaviour on the portable guard.

2. Lane-parity check: the Claude hook was more permissive than intended on macOS.
The hook called hasMacosWriteSandbox(), which is true on any Mac with sandbox-exec. But Claude Code runs Bash itself, outside baro's Seatbelt profile. As a result, on macOS the Claude lane accepted:

  • cp README.md node_modules/fx/index.js, a write into the dependency dir that every concurrent story shares;
  • node -e inline code with no sandbox at all.

I reproduced both against hookShellAccessContext before changing anything. "Does this command run sandboxed?" is now a field on ShellAccessContext (writeSandbox):

  • the native lane sets it from the sandbox it actually spawns, and prepareShellSandbox reads the same field, so the two can't drift;
  • the hook always sets it to false.

Behaviour change on macOS: the Claude lane now refuses node -e / python -c / perl -e / ruby -e inline code, the same way Linux lanes already do. Because of item 1, dependency reads stay allowed there.

3. Refusal texts.

isDependencyReadCommand keeps its set inline and is added to HOOK_SOURCE_GRAPH. This follows the file-header contract that the hook source is serialized with fn.toString().

Test plan

  • npm run typecheck -w @baro/orchestrator: clean
  • test/story-tool-containment.test.ts, all pass. Changes:
    • updated: dependency reads through the link pass on every platform, and the direct target and a same-named untrusted link are still refused;
    • new: on the portable (Linux) guard, ls/test/grep -r/head | wc through the link and running node_modules/.bin/<tool> work, while cp/mv/| tee/>/touch/ln/find -delete/sort -o/cd into the link are refused and the dependency file is left untouched;
    • new: a project-local scripts/cat gets no read exception.
  • test/execution/shell-containment.test.ts, all pass. New test: the hook context reports writeSandbox: false; reads through the link are allowed; cp, redirection and node -e are refused. The serialized hook gives the same verdicts.
  • test/harness/hook-bridge.test.ts: pass
  • npm run test:fast: 1636/1644 pass. The failures are timing-sensitive tests on a heavily loaded machine: run-architect-awake-budget, verification/command-timing, continuous-gate-runner and provider-ownership-manifest. The first three fail the same way on unmodified origin/main, checked by stashing this change. provider-ownership-manifest passed when re-run on its own. No Rust changes, so cargo was not run.
  • Repro on a hosted (Linux/Fargate) run: expect no node_modules escape refusals for read-only commands.

Not in this PR

  • rg is not on the read allowlist because rg --pre <cmd> runs a command per file. It could be added later with a --pre denial.
  • I did not ask whether the Claude lane should get a real write sandbox rather than a fail-closed guard. That would bring back node -e on macOS for that lane.

Generated with Devin

…ng Seatbelt

Without the write sandbox the guard refused every operand that reached a
manager-owned dependency link, so cheap hosted lanes paid a model round for
`ls node_modules/.bin`. Read-only commands (cat, ls, grep, head, ...) may now
read through the link on every lane; writes, redirections, and any other
command still need Seatbelt.

The lane-parity check found the Claude hook asked hasMacosWriteSandbox(),
which is true on macOS even though Claude Code runs Bash outside baro's
profile: that lane allowed `cp x node_modules/...` and `node -e` with no
sandbox at all. Whether a command runs sandboxed is now a field of the access
context: the native lane sets it from the sandbox it actually spawns, the hook
sets it false.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant