feat(cli): enforce /add-dir via sandbox writable roots (⚠ security — please review)#166
Open
oratis wants to merge 1 commit into
Open
feat(cli): enforce /add-dir via sandbox writable roots (⚠ security — please review)#166oratis wants to merge 1 commit into
oratis wants to merge 1 commit into
Conversation
…itive) /add-dir only printed a message; the settings.permissions.additionalDirectories field was declared but consumed nowhere. Now: - /add-dir validates the path is an existing directory and PERSISTS it to permissions.additionalDirectories (dedup) in the user settings; with no args it lists the current set. - New core helper withAdditionalWritableDirs(sandbox, dirs) folds those dirs into the sandbox's filesystem.allowWrite. The REPL + headless build their sandboxConfig through it, so the sandboxed Bash tool can write to added dirs (beyond cwd). No-op when the sandbox is off; never mutates input. The file tools (Read/Write/Edit/Glob/Grep) already accept any absolute path, so this only changes the SANDBOX boundary for Bash — which is the security-relevant surface. Holding this PR for review rather than auto-merging. Tests: withAdditionalWritableDirs (add/dedup/undefined-safe) + /add-dir (persist / reject-missing / list). core 646 · cli 143. Co-Authored-By: Claude Opus 4.8 (1M context) <[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
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.
Security-sensitive — opening for review, not auto-merging.
/add-dironly printed a message;settings.permissions.additionalDirectorieswas declared but consumed nowhere. Now:/add-dir <path>validates the path is an existing directory and persists it topermissions.additionalDirectories(deduped) in the user settings; with no args it lists the current set.withAdditionalWritableDirs(sandbox, dirs)folds those dirs into the sandbox'sfilesystem.allowWrite. The REPL + headless build theirsandboxConfigthrough it, so the sandboxed Bash tool can write to added dirs (beyond cwd). No-op when the sandbox is off; never mutates input.Why this is the right boundary
The file tools (Read/Write/Edit/Glob/Grep) already accept any absolute path () — there's no cwd containment to "enforce". The only thing that restricts writes is the sandbox (for Bash). So
/add-direnforcement = expanding the sandbox's writable roots, which is exactly the user's intent. That's the security-relevant change — hence review.Tests
withAdditionalWritableDirs(add / dedup / undefined-safe / seeds empty) +/add-dir(persist validated dir / reject missing / list). core 646 · cli 143, typecheck +format:checkclean.🤖 Generated with Claude Code