feat(filesystem): add denyReadAlways for denies that beat allowRead#284
Open
smolyn wants to merge 3 commits into
Open
feat(filesystem): add denyReadAlways for denies that beat allowRead#284smolyn wants to merge 3 commits into
smolyn wants to merge 3 commits into
Conversation
When `network.allowAllDomains: true`, filterNetworkRequest short-circuits to allow after the deniedDomains check runs, so explicit denies still take effect. Intended for environments where filesystem restrictions are the primary boundary and enumerating every reachable domain is impractical (e.g. opening web docs lookups in an otherwise locked-down sandbox). The flag is optional and additive: existing configs are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Read restrictions today have only two layers — denyRead and allowRead — with allowRead unconditionally winning. That makes it impossible to broadly allow a directory (e.g. ~/src for cross-project work) while still keeping credential-style files (.env, *.pem, ~/.aws/credentials) denied within it. The write side already has the symmetric capability via denyWrite, so reads were the odd one out. New optional `denyReadAlways: string[]` in FilesystemConfigSchema. On macOS it emits Seatbelt (deny file-read*) rules AFTER the allowRead loop so they take precedence (last-match-wins). On Linux it emits bubblewrap binds (--tmpfs for dirs, --ro-bind /dev/null for files) after the allowRead re-bind loop for the same effect. denyReadAlways paths are also passed to generateMoveBlockingRules so mv/rename cannot bypass the deny by relocating the file. Glob handling reuses the existing helpers — patterns work the same as denyRead. Patterns with a leading "/" match globally (e.g. "/**/.env*"); without it they are CWD-relative, matching existing denyRead behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Adds an optional
filesystem.denyReadAlways: string[]schema field that emits read-deny rules with priority overallowRead. Designed to let users keep a broadallowRead(e.g.~/srcfor cross-project work) while still blocking credential-style files (.env,~/.aws/credentials,*.pem, …) inside.Motivation
Today, read restrictions have two layers:
denyRead— deny broad regionsallowRead— re-allow within denied regions (wins over denyRead)There is no way to express "deny X even though X is inside an allowed region."
denyRead: ["~/src/**/.env*"]is silently overridden whenallowRead: ["~/src"]is set, becausegenerateReadRules(macos-sandbox-utils.ts:224-298) emits Seatbelt rules inallow → deny → alloworder — last-match-wins makes allowRead the final word.The write side already has the symmetric capability:
denyWritebeatsallowWritevia the existingdenyWithinAllowchannel (sandbox-config.ts:239-254). This PR adds the equivalent for reads.Behavior
denyReadAlwaysallowRead(a.k.a.allowWithinDeny)denyRead(a.k.a.denyOnly)Example config:
{ \"filesystem\": { \"denyRead\": [\"/Users\"], \"allowRead\": [\"~/src\"], \"denyReadAlways\": [ \"/**/.env*\", \"/**/credentials\", \"/**/id_rsa*\", \"/**/id_ed25519*\" ] } }~/src/myproject/.env→ blocked (denyReadAlways).~/src/myproject/source.ts→ allowed (allowRead beats denyRead).~/Documents/note.md→ blocked (denyRead/Users, no override).Implementation
src/sandbox/sandbox-config.ts— adddenyReadAlwaystoFilesystemConfigSchema(optional, additive).src/sandbox/sandbox-schemas.ts— extend internalFsReadRestrictionConfigwithdenyAlways?: string[]; updated docstring to describe the three-layer model.src/sandbox/sandbox-manager.ts— plumb the field throughgetFsReadConfigand thecustomConfigoverride path, with the same Linux-glob-expansion treatment used fordenyRead.src/sandbox/macos-sandbox-utils.ts— third pass ingenerateReadRulesemitting(deny file-read* ...)after theallowWithinDenyloop so denies win. Also passdenyAlwayspaths togenerateMoveBlockingRulesso mv/rename cannot bypass.src/sandbox/linux-sandbox-utils.ts— third pass after the allowRead re-bind loop emitting--tmpfs <dir>or--ro-bind /dev/null <file>. Bubblewrap binds are applied in argument order, so later binds shadow earlier ones for the same destination.test/config-validation.test.ts— schema test confirmingdenyReadAlwaysvalidates.+128 / -5 lines across 6 files. All additive — existing configs unaffected.
Glob anchoring note
Glob patterns are CWD-relative unless they start with
/(the existingdenyReadhas the same behavior). The intended pattern for global credential matching is/**/.env*, which resolves to the regex^/(.*/)?\.env[^/]*$. On Linux, broad globs like/**/.env*hit the existing "too broad" guard inexpandGlobPatternand get skipped with a warning — same limitation as broaddenyReadglobs today. Linux users should narrow the patterns to e.g.~/src/**/.env*.Test plan
pnpm exec tsc --noEmitcleantest/config-validation.test.tsschema test passessrt --settings:~/src/srt-test/.envand~/src/srt-test/regular.txt.allowRead: [\"~/src\"]only: both readable.denyReadAlways: [\"/**/.env*\"]:.envblocked with EPERM,regular.txtstill readable. ✓Dependency note
This branch stacks on #283 (allowAllDomains) — the PR diff currently includes both features. After #283 merges I'll rebase this branch onto fresh
mainso the diff shows only the denyReadAlways change.🤖 Generated with Claude Code