feat(sandbox): make it a real axis, and turn it on by default - #226
Merged
Conversation
Findings F5/F6 in docs/THREE_WAY_REVIEW.md: the sandbox was off unless configured, and a single `mode` expressed both what a command may touch and how it gets approved. Codex separates those; the alignment plan §5.5 says to, and hadn't. The axis. `sandbox.mode` in settings and `--sandbox` on the CLI take read-only / workspace-write / danger-full-access, orthogonal to `--mode`. The legacy `enabled` boolean still works (true → workspace-write, false → danger-full-access) and `mode` wins when both are set. Every host now defaults to workspace-write via RuntimeHost; library callers of wrapBashCommand keep the old "off unless configured" behaviour unless they pass defaultMode, so embedders can't be silently sandboxed by an upgrade. Why it wasn't usable. On macOS the profile is `(deny default)` with no rule for cwd, so an enabled sandbox denied reads of the project directory itself — `cat src/a.ts` failed inside it, while Linux bound cwd read-write. Nobody hit this because nobody could turn it on. Reads become allow-by-default. The read allowlist did not survive real commands: git couldn't resolve Xcode's developer dir under /Applications, nothing could open /dev/null, temp writes failed because SBPL `subpath` doesn't match the directory node itself, and ~/.gitconfig was denied. Each failure surfaces inside an agent as a confusing permission error. Writes and network stay deny-by-default; well-known credential stores (~/.ssh, ~/.aws, ~/.gnupg, ~/.netrc, ~/.config/gh, the DeepCode credentials file, Keychains) are denied for reading, and filesystem.denyRead still applies last. Package-manager caches are writable — denying ~/.npm turns `npm install` into a permission error while protecting a content-addressed cache. A linked worktree's git dirs live outside the workspace and are added too, or every git command fails in the worktrees EnterWorktree creates. Verified on macOS rather than asserted: under workspace-write, workspace read/write, temp writes, git, node, npm install, tsc and vitest all succeed while writes outside the workspace and reads of ~/.ssh are denied; read-only additionally denies workspace writes. The end-to-end attack test moved its target out of $TMPDIR, which the profile intentionally allows — it had been passing for the wrong reason. Co-Authored-By: Claude Opus 5 <[email protected]>
This was referenced Aug 3, 2026
oratis
added a commit
that referenced
this pull request
Aug 9, 2026
`buildLinuxBwrapArgs` ended with an unconditional `--bind <cwd> <cwd>`. bwrap applies binds in order and the last one wins, so under `read-only` the `--ro-bind-try` that `sandboxConfigForMode` had correctly asked for was overwritten a few arguments later. The mode resolved right, the profile said the right thing, and a command could still write to the workspace. macOS never had this: `buildMacOsProfile` grants writes only from `allowWrite`, which read-only leaves empty. #226 introduced the mode axis and live-verified it on macOS — where it caught the mirror-image bug, a profile that denied reads of the project directory. This is the half nobody looked at, which is exactly what THREE_WAY_REVIEW recorded under 仍未做. Found by writing the missing test rather than by reading the code: the bwrap integration suite only ever exercised the legacy `enabled: true` shape, so no test had spawned bwrap in a named mode to see what a command could actually do. The mode resolution was unit-tested and the arguments were argument-tested, and between those two correct halves the behaviour was wrong. An absent mode keeps the read-write bind, so callers on the legacy shape are unaffected. Linux CI now sets DC_REQUIRE_BWRAP=1. These are the only tests that observe what the sandbox does rather than what it builds, and they self-skip — a green suite that skipped every real enforcement check is how this survived. Co-Authored-By: Claude Opus 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Findings F5/F6 in
docs/THREE_WAY_REVIEW.md. The sandbox was off unless configured, and onemodeexpressed both what a command may touch and how it gets approved. Codex separates those;CODEX_ALIGNMENT_PLAN.md§5.5 says to, and it hadn't been done.The axis
sandbox.mode(settings) and--sandbox(CLI) takeread-only|workspace-write|danger-full-access, orthogonal to--mode. So "never ask me, but keep writes inside the workspace" is now expressible:The legacy
enabledboolean keeps working (true→ workspace-write,false→ danger-full-access);modewins when both are set. Every host defaults to workspace-write throughRuntimeHost. Library callers ofwrapBashCommandkeep the old "off unless configured" behaviour unless they passdefaultMode, so embedding DeepCode can't become silently sandboxed by an upgrade./statusand--helpshow the resolved mode.Why it wasn't usable before
On macOS the profile is
(deny default)with no rule for cwd — an enabled sandbox denied reads of the project directory itself, socat src/a.tsfailed inside it while the Linux path bound cwd read-write. Nobody hit it because nobody could turn the sandbox on.Reads become allow-by-default
The read allowlist did not survive contact with real commands. Each of these was a separate, silent failure:
gitcouldn't resolve Xcode's active developer dir (/Applicationswasn't readable) → "No developer tools were found"/dev/null→fatal: could not open '/dev/null'subpathdoesn't match the directory node, so/tmpand/private/tmpneeded literal reads~/.gitconfigwas deniedWrites and network stay deny-by-default — that is what the sandbox is actually for. Credential stores (
~/.ssh,~/.aws,~/.gnupg,~/.netrc,~/.docker/config.json,~/.config/gh,~/.deepcode/credentials.json,~/Library/Keychains) are denied for reading, andfilesystem.denyReadstill applies last, so a stricter posture remains expressible — it just isn't what has to be right forlsto work. This trade-off is written up indocs/security-model.md.Package-manager caches (
~/.npm,~/.cache,~/.cargo, …) are writable: denying them turnsnpm installinto a permission error while protecting a content-addressed cache. A linked worktree's git dirs live outside the workspace and are added too — otherwise every git command fails inside the worktreesEnterWorktreecreates.Verified, not asserted
Run against the real
sandbox-execon macOS:$TMPDIR//tmpgit status/git lognode,tsc,vitest,npm install~/.gitconfig~/.ssh/<key>The end-to-end attack test had its target inside
$TMPDIR, which the profile intentionally allows — it was passing for the wrong reason and now writes outside the temp allowance.19 new policy unit tests + 4 CLI flag tests.
pnpm typecheck·lint·format:checkclean; core 739/16 skipped · cli 208 · desktop 75 · server 41 · protocol 24 · vscode 12 · lsp 13 · scripts 21.Linux is unchanged beyond sharing the mode resolution — bwrap already bound cwd read-write. The bwrap path deserves the same empirical pass on a Linux host before we lean on it.
🤖 Generated with Claude Code