Repository navigation
feat(security): allow disabling path sandbox via CGC_ALLOW_ALL_PATHS - #1726
Conversation
Dynamic multi-repository indexing, containerized setups, and script-driven integrations across variable filesystem locations make maintaining CGC_ALLOWED_ROOTS for every target path restrictive and cumbersome. Set CGC_ALLOW_ALL_PATHS=true or CGC_ALLOWED_ROOTS=* to skip path sandbox checks entirely. Every call site funnels through is_path_allowed(), so the bypass is centralized there. Default behavior is unchanged: sandboxing to cwd and CGC_ALLOWED_ROOTS stays on unless explicitly disabled. Fixes CodeGraphContext#1719.
|
@C0deRatoR is attempting to deploy a commit to the shashankss1205's projects Team on Vercel. A member of the Team first needs to authorize it. |
Shashankss1205
left a comment
Shashankss1205
left a comment
There was a problem hiding this comment.
Good shape for this. The key design decision — putting the bypass in is_path_allowed() rather than at each call site — is the right one: every sandbox consumer funnels through that single function, so there's exactly one place to audit and no chance of a call site that silently forgets the new flag.
Two things I checked specifically before approving:
- Default stays closed.
is_path_sandbox_disabled()returnsFalsefor an unset variable, an empty string, and anything outside{1, true, yes}— so a typo likeCGC_ALLOW_ALL_PATHS=turekeeps the sandbox on rather than failing open. That's the correct direction to fail. - The
CGC_ALLOWED_ROOTS=\"*\"handling doesn't leak. Theenv_roots != \"*\"guard inget_allowed_roots()stops*being parsed as a literal directory name, which would otherwise have produced a nonexistent root that silently matched nothing.
Agreed on leaving the config-file surface out — none of the sandbox call sites read config_manager today, and adding that plumbing here would have widened the diff well past the fix. The doc note in MCP_TOOLS.md is appropriately blunt about when to use it.
Verified locally against the full unit + integration suite: 1596 passed, 8 skipped, including the existing CWE-22 traversal suite.
github-project-automation
Bot
moved this from Backlog tasks
to Done
in CGC Progress Board
Shashankss1205
added
gssoc:approved
2 participants
Fixes #1719
What does this PR do?
Adds a way to fully disable path sandboxing, as requested in #1719, for workflows where maintaining
CGC_ALLOWED_ROOTSfor every target path is impractical (dynamic multi-repository indexing, containerized setups, script-driven integrations across variable filesystem locations).How it works
Every path-sandbox call site funnels through
is_path_allowed()inpath_sandbox.py, so the bypass is centralized in one place:CGC_ALLOW_ALL_PATHS=true(or1/yes), orCGC_ALLOWED_ROOTS=*either one makes
is_path_allowed()permit any path, skipping the cwd/CGC_ALLOWED_ROOTScheck for indexing, watching, bundle load, and context-switch tools alike.Default behavior is unchanged — sandboxing to cwd +
CGC_ALLOWED_ROOTSstays on unless one of these is explicitly set, matching the "default behavior should remain secure" requirement from the issue.Scoped to the environment-variable mechanism the issue proposes. Left the config-file equivalent (
allow_all_paths: true/sandbox.enabled: false) out of this PR since none of the sandbox call sites currently read fromconfig_manager's.env-based config at all — happy to add it as a follow-up if maintainers want that surface too.How did you test it?
Added unit tests in
tests/unit/utils/test_path_sandbox_security.pycovering: default-deny outside cwd, bypass viaCGC_ALLOW_ALL_PATHS=true, bypass viaCGC_ALLOWED_ROOTS=*, and that cwd is still allowed by default.pytest tests/unit/utils/test_path_sandbox_security.py tests/unit/tools/test_cwe22_path_traversal.py— 20 passed, no regressions in the existing CWE-22 suite.ruff checkon the changed files — clean.Also updated
docs/MCP_TOOLS.md's path-sandbox section with the new env vars.