Harden permission enforcement against sandbox bypasses

Close two ways the permission system could be bypassed:

- Workspace path traversal: normalize `.`/`..` lexically before the
  boundary prefix comparison so paths like `/workspace/../../etc` can no
  longer escape the sandbox. Fixed in both the runtime enforcer and the
  duplicate check in the tools PowerShell path classifier.
- read-only mode no longer trusts the leading token alone: reject shell
  metacharacters (chaining/substitution/redirect/pipe/subshell), drop
  interpreters and build drivers (python/node/ruby/cargo/rustc) from the
  allow-list, gate `git` to non-mutating subcommands, and reject `find`
  actions that execute or delete.

Adds regression tests for both holes. The pre-existing, unrelated
worker_boot git-metadata test failure is not affected by this change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Sam Lamrabte
2026-05-31 13:26:39 +02:00
parent 4d3dc5b873
commit e8c8ef1142
2 changed files with 207 additions and 20 deletions

View File

@@ -2571,6 +2571,20 @@ fn is_within_workspace(path: &str) -> bool {
let path = PathBuf::from(trimmed);
// Reject any parent-directory traversal. Callers never need `..` to refer
// to files inside the workspace, and `..` defeats both checks below: the
// relative branch only inspects the leading component, and the absolute
// branch's `canonicalize()` silently falls back to the literal `..` path
// when the target does not exist yet (e.g. a file about to be created).
// Returning false here is the safe direction: it classifies the command as
// requiring full-access permission rather than workspace-write.
if path
.components()
.any(|component| matches!(component, std::path::Component::ParentDir))
{
return false;
}
// If path is absolute, check if it starts with CWD
if path.is_absolute() {
if let Ok(cwd) = std::env::current_dir() {
@@ -2588,6 +2602,26 @@ fn run_powershell(input: PowerShellInput) -> Result<String, String> {
to_pretty_json(execute_powershell(input).map_err(|error| error.to_string())?)
}
#[cfg(test)]
mod workspace_traversal_guard_tests {
use super::is_within_workspace;
#[test]
fn rejects_parent_traversal_components() {
// Leading and embedded `..` must both be rejected (was previously a hole
// because only the leading component was inspected).
assert!(!is_within_workspace("../secrets"));
assert!(!is_within_workspace("src/../../etc/passwd"));
assert!(!is_within_workspace("a/b/../../../etc/crontab"));
}
#[test]
fn allows_plain_relative_paths() {
assert!(is_within_workspace("src/main.rs"));
assert!(is_within_workspace("Cargo.toml"));
}
}
fn to_pretty_json<T: serde::Serialize>(value: T) -> Result<String, String> {
serde_json::to_string_pretty(&value).map_err(|error| error.to_string())
}