fix(settings): strip repository workspace_paths in project sanitization - #173
Merged
Conversation
sanitize_project_settings removed every executable and provider surface from repository settings but left config.workspace_paths intact, and merge() concatenates that Vec — so an untrusted <project>/.coven-code/settings.json could append arbitrary roots (/, ~/.ssh) to the default-mode read allowlist and widen plain-headless read access beyond the project. Unlike the read/write tool paths, this needed no LSP or command execution. Only the user's own settings layers may contribute read roots now; a future explicit trust-this-project policy can relax that deliberately rather than by default. Regression test proves the sanitizer strips repo-provided paths and that a merge preserves exactly the trusted global roots. Follow-up from the coven-code #170 review (Cave bead cave-smv6v). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR tightens project-settings sanitization in claurst_core::config::Settings to prevent untrusted repository config from widening default-mode filesystem read allowlists via workspace_paths, aligning with the “repo settings are untrusted until explicitly trusted” security contract introduced in PR #170.
Changes:
- Clear
config.workspace_pathsinsideSettings::sanitize_project_settings()so repository-local settings cannot contribute additional read roots. - Add a regression test ensuring sanitized project settings do not contribute
workspace_paths, and that merging trusted global + sanitized project preserves only trusted roots.
Suppressed comments (1)
src-rust/crates/core/src/lib.rs:2370
- The new regression test only covers
workspace_paths, but the same trust boundary applies toconfig.additional_dirs(it is included inToolContext::permission_allowed_roots()alongsideworkspace_paths). Extending this test to includeadditional_dirswould prevent regressions where repository settings widen read roots via--add-dir-equivalent config.
let global = Settings {
config: Config {
workspace_paths: vec![PathBuf::from("/home/user/trusted-workspace")],
..Default::default()
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1851
to
+1857
| // workspace_paths widen the default-mode read allowlist, and merge() | ||
| // concatenates them — an untrusted repository could add arbitrary | ||
| // roots (e.g. "/" or "~") and read outside the project in plain | ||
| // headless mode. Only the user's own settings layers may contribute | ||
| // read roots; a future explicit trust-this-project policy can relax | ||
| // this deliberately rather than by default. | ||
| settings.config.workspace_paths.clear(); |
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
Security follow-up from the #170 review (tracked as Cave bead cave-smv6v):
Settings::sanitize_project_settingsremoved every executable and provider surface from repository settings but leftconfig.workspace_pathsintact — andmerge()concatenates that Vec. An untrusted<project>/.coven-code/settings.jsoncould therefore append arbitrary roots (/,~/.ssh) to the default-mode read allowlist, widening plain-headless read access beyond the project with no LSP or command execution involved.The sanitizer now clears
workspace_paths, so only the user's own settings layers (shared/global) contribute read roots. A future explicit trust-this-project policy can relax that deliberately rather than by default — matching the sanitizer's existing "untrusted until the user chooses to trust" contract.Verification
sanitize_strips_repository_workspace_paths: the sanitizer strips repo-provided paths, and a merge of trusted-global + sanitized-project preserves exactly the trusted roots.config::test module: 31/31 pass;cargo fmt --checkclean.🤖 Generated with Claude Code