feat(allowedpaths): permit writes through sandbox open#206
Draft
feat(allowedpaths): permit writes through sandbox open#206
Conversation
Relax Sandbox.Open from read-only to read+write. The underlying os.Root.OpenFile already validates writes safely via openat, so the sandbox needed only to widen its flag check rather than build new machinery. The cross-root symlink fallback stays read-only — following a symlink that escapes its os.Root and then performing O_CREATE/O_TRUNC is the classic TOCTOU footgun, where the link target can be swapped between resolution and open. Writes therefore stay inside a single os.Root. Open flags are also tightened to an explicit allowlist (O_RDONLY|O_WRONLY|O_RDWR|O_APPEND|O_CREATE|O_EXCL|O_TRUNC) so unknown or platform-specific bits cannot slip through. This is the foundation of a demo-only stack — file-target redirects (>, >>) are still blocked at the parser, so no user-visible surface gains write capability in this commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
5 tasks
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
1 participant
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
Sandbox.Openfrom read-only to read+write. The underlyingos.Root.OpenFilealready validates writes safely viaopenat, so this is a flag-check widening, not new machinery.O_RDONLY|O_WRONLY|O_RDWR|O_APPEND|O_CREATE|O_EXCL|O_TRUNC); anything else (O_DIRECTORY,O_NOFOLLOW,O_SYNC,O_NONBLOCK, …) returnsErrPermission.os.Rootand then performingO_CREATE/O_TRUNC/O_APPEND/write opens is the classic TOCTOU footgun — the link target can be swapped between resolution and open. Writes must therefore stay inside a singleos.Root.interp.AllowedPathsdoc andREADME.md/SHELL_FEATURES.mdto reflect the new read+write semantics.Demo-only stack — not for main merge
This branch is the foundation of a demo-only stack and is not expected to merge to main. Follow-up branches (
>redirection, atruncatebuiltin,logrotatevia host exec) build on top.As a result, this PR ships a sandbox capability with no in-tree consumer — file-target output redirects (
>,>>) are still rejected byinterp/validate.go, and no builtin gains write capability. Reviewers should not expect user-visible behavior changes here.Test plan
make fmtcleango test ./allowedpaths/... ./interp/... -timeout 120sgo test ./... -timeout 300s(full suite, includingTestVerificationAllowedpathsCleanPass)allowedpaths/sandbox_test.go:TestSandboxWriteAllowedPath—O_CREATE|O_WRONLYinside the allowlist creates the fileTestSandboxWriteOutsideAllowedPath— same flags outside returnsErrPermission, no file createdTestSandboxAppend—O_APPENDappends correctlyTestSandboxTruncate—O_TRUNCreplaces contentTestSandboxWriteThroughSymlinkEscapeRejected— symlink-inside-allowlist pointing outside is denied for writes (TOCTOU defense), and the target is not createdTestSandboxWriteRejectsUnknownFlag— unknown flag bit rejectedTestSandboxOpenReadStillWorks— read-only path unaffectedTestAllowedPathsRedirectOutside(and other existing redirect/cat tests) still pass — file-target redirection remains blocked at the parser.🤖 Generated with Claude Code