Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 90 additions & 14 deletions src/sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ pub struct SandboxConfig {
pub mode: SandboxMode,
#[serde(default)]
pub writable_paths: Vec<PathBuf>,
#[serde(default)]
pub readable_paths: Vec<PathBuf>,
/// Environment variable names to forward from the parent process into worker
/// subprocesses. This is the escape hatch for self-hosted users who set env
/// vars in Docker/systemd but don't configure a secret store. When the secret
Expand All @@ -41,6 +43,7 @@ impl Default for SandboxConfig {
Self {
mode: SandboxMode::Enabled,
writable_paths: Vec::new(),
readable_paths: Vec::new(),
passthrough_env: Vec::new(),
project_paths: Vec::new(),
}
Expand Down Expand Up @@ -285,7 +288,7 @@ impl Sandbox {
}

/// Check whether a canonical path falls within the workspace or any
/// allowed writable path (user-configured or project-injected).
/// allowed path (workspace, writable paths, or readable paths).
///
/// Used by shell/file tools to relax the workspace boundary when
/// project paths are registered.
Expand All @@ -294,19 +297,56 @@ impl Sandbox {
.workspace
.canonicalize()
.unwrap_or_else(|_| self.workspace.clone());
let data_dir_canonical = self
.data_dir
.canonicalize()
.unwrap_or_else(|_| self.data_dir.clone());

if canonical.starts_with(&data_dir_canonical) {
return false;
}

if canonical.starts_with(&workspace_canonical) {
return true;
}

let config = self.config.load();
for path in config.all_writable_paths() {
let allowed = path.canonicalize().unwrap_or_else(|_| path.clone());
if allowed.starts_with(&data_dir_canonical) {
continue;
}
if canonical.starts_with(&allowed) {
return true;
}
}
for path in &config.readable_paths {
let allowed = path.canonicalize().unwrap_or_else(|_| path.clone());
if allowed.starts_with(&data_dir_canonical) {
continue;
}
if canonical.starts_with(&allowed) {
return !canonical.starts_with(&data_dir_canonical);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
false
}

/// Check whether a canonical path is writable under sandbox policy.
pub fn is_path_writable(&self, canonical: &Path) -> bool {
if !self.is_path_allowed(canonical) {
return false;
}
Comment thread
EZotoff marked this conversation as resolved.
Resolved
let config = self.config.load();
for path in &config.readable_paths {
let allowed = path.canonicalize().unwrap_or_else(|_| path.clone());
if canonical.starts_with(&allowed) {
return false;
}
}
true
}

/// True when OS-level containment is currently active.
///
/// If mode is enabled but no backend is available, this returns false
Expand Down Expand Up @@ -345,6 +385,12 @@ impl Sandbox {
push_unique_path(&mut paths, canonical);
}
}

for path in &config.readable_paths {
if let Ok(canonical) = path.canonicalize() {
push_unique_path(&mut paths, canonical);
}
}
}
InternalBackend::SandboxExec => {
for system_path in MACOS_READ_ONLY_SYSTEM_PATHS {
Expand All @@ -363,6 +409,10 @@ impl Sandbox {
for path in config.all_writable_paths() {
push_unique_path(&mut paths, canonicalize_or_self(path));
}

for path in &config.readable_paths {
push_unique_path(&mut paths, canonicalize_or_self(path));
}
}
InternalBackend::None => {}
}
Expand Down Expand Up @@ -545,47 +595,63 @@ impl Sandbox {
}
}

// 7. Mask agent data dir with an empty tmpfs to prevent reads/writes,
// 7. User-configured read-only paths
for path in &config.readable_paths {
match path.canonicalize() {
Ok(canonical) => {
cmd.arg("--ro-bind").arg(&canonical).arg(&canonical);
}
Err(error) => {
tracing::debug!(
path = %path.display(),
%error,
"skipping readable_path (does not exist or is unresolvable)"
);
}
}
}

// 8. Mask agent data dir with an empty tmpfs to prevent reads/writes,
// even when it overlaps with workspace-related paths.
cmd.arg("--tmpfs").arg(&self.data_dir);

// 8. Isolation flags
// 9. Isolation flags
cmd.arg("--unshare-pid");
cmd.arg("--new-session");
cmd.arg("--die-with-parent");

// 9. Clear all inherited environment variables. Workers must not see
// 10. Clear all inherited environment variables. Workers must not see
// system secrets (LLM API keys, messaging tokens) or SPACEBOT_* internals.
cmd.arg("--clearenv");

// 10. Working directory
// 11. Working directory
cmd.arg("--chdir").arg(working_dir);

// 11. Set PATH inside the sandbox
// 12. Set PATH inside the sandbox
cmd.arg("--setenv").arg("PATH").arg(path_env);

// 12. Set deterministic sandbox-local home/temp paths.
// 13. Set deterministic sandbox-local home/temp paths.
cmd.arg("--setenv")
.arg("HOME")
.arg(self.workspace.to_string_lossy().into_owned());
cmd.arg("--setenv").arg("TMPDIR").arg("/tmp");

// 12a. Suppress interactive prompts. CI=true prevents npm/npx/yarn
// 13a. Suppress interactive prompts. CI=true prevents npm/npx/yarn
// from prompting; DEBIAN_FRONTEND=noninteractive prevents apt-get.
// Shell tool runs without stdin, so interactive prompts always hang.
cmd.arg("--setenv").arg("CI").arg("true");
cmd.arg("--setenv")
.arg("DEBIAN_FRONTEND")
.arg("noninteractive");

// 13. Re-inject safe environment variables for basic process operation
// 14. Re-inject safe environment variables for basic process operation
for var_name in SAFE_ENV_VARS {
if let Ok(value) = std::env::var(var_name) {
cmd.arg("--setenv").arg(var_name).arg(value);
}
}

// 13. Re-inject tool secrets from the secret store.
// 15. Re-inject tool secrets from the secret store.
// Only tool-category secrets are injected; system secrets (LLM API keys,
// messaging tokens) never enter subprocess environments.
for (name, value) in tool_secrets {
Expand All @@ -596,7 +662,7 @@ impl Sandbox {
cmd.arg("--setenv").arg(name).arg(value);
}

// 14. Re-inject passthrough env vars (user-configured forwarding),
// 16. Re-inject passthrough env vars (user-configured forwarding),
// skipping any that would override hardened defaults.
for var_name in &config.passthrough_env {
if is_reserved_env_var(var_name) {
Expand All @@ -608,7 +674,7 @@ impl Sandbox {
}
}

// 15. Per-command env vars from tool caller (e.g. shell tool `env`).
// 17. Per-command env vars from tool caller (e.g. shell tool `env`).
// Injected via --setenv so they reach the inner sandboxed process.
for (name, value) in command_env {
if is_reserved_env_var(name) {
Expand All @@ -622,7 +688,7 @@ impl Sandbox {
cmd.arg("--setenv").arg(name).arg(value);
}

// 16. Worker keyring isolation (Linux) — give the child a fresh empty
// 18. Worker keyring isolation (Linux) — give the child a fresh empty
// session keyring so it cannot access the parent's keyring (which holds
// the master key for secret store encryption).
#[cfg(target_os = "linux")]
Expand All @@ -635,7 +701,7 @@ impl Sandbox {
}
}

// 17. The actual command
// 19. The actual command
cmd.arg("--").arg(program);
for arg in args {
cmd.arg(arg);
Expand Down Expand Up @@ -856,6 +922,15 @@ impl Sandbox {
));
}

for (index, path) in config.readable_paths.iter().enumerate() {
let canonical = canonicalize_or_self(path);
profile.push_str(&format!(
"; readable path {index}\n(allow file-read* (subpath \"{}\"))\n(deny file-write* (subpath \"{}\"))\n",
escape_sbpl_path(&canonical),
escape_sbpl_path(&canonical)
));
}

// /tmp writable
let tmp = canonicalize_or_self(Path::new("/tmp"));
profile.push_str(&format!(
Expand Down Expand Up @@ -1002,6 +1077,7 @@ mod tests {
let config = SandboxConfig::default();
assert_eq!(config.mode, SandboxMode::Enabled);
assert!(config.writable_paths.is_empty());
assert!(config.readable_paths.is_empty());
assert!(config.project_paths.is_empty());
assert!(config.passthrough_env.is_empty());
}
Expand Down
26 changes: 17 additions & 9 deletions src/tools/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,7 @@ impl FileContext {
Self { workspace, sandbox }
}

/// Resolve and validate a path.
///
/// Relative paths are resolved against the workspace root. When sandbox mode
/// is enabled, absolute paths must fall within the workspace and symlink
/// traversal is blocked. When sandbox is disabled, any readable/writable
/// path is accepted.
fn resolve_path(&self, raw: &str) -> Result<PathBuf, FileError> {
fn resolve_path_inner(&self, raw: &str, require_writable: bool) -> Result<PathBuf, FileError> {
let path = Path::new(raw);
let resolved = if path.is_absolute() {
path.to_path_buf()
Expand Down Expand Up @@ -62,6 +56,12 @@ impl FileContext {
)));
}

if require_writable && !self.sandbox.is_path_writable(&canonical) {
return Err(FileError(
"ACCESS DENIED: This path is mounted as read-only. You can read files here but cannot write or edit them.".to_string(),
));
}

let workspace_canonical = self
.workspace
.canonicalize()
Expand Down Expand Up @@ -90,6 +90,14 @@ impl FileContext {
Ok(canonical)
}

fn resolve_path(&self, raw: &str) -> Result<PathBuf, FileError> {
self.resolve_path_inner(raw, false)
}

fn resolve_writable_path(&self, raw: &str) -> Result<PathBuf, FileError> {
self.resolve_path_inner(raw, true)
}
Comment thread
EZotoff marked this conversation as resolved.
Resolved

// NOTE: Identity file protection (PROTECTED_FILES) has been removed.
// Identity files (SOUL.md, IDENTITY.md, ROLE.md) now live in the agent
// root directory, outside the workspace sandbox boundary. The sandbox
Expand Down Expand Up @@ -218,7 +226,7 @@ impl Tool for FileReadTool {
}

async fn call(&self, args: Self::Args) -> Result<Self::Output, Self::Error> {
let path = self.context.resolve_path(&args.path)?;
let path = self.context.resolve_writable_path(&args.path)?;

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_read is resolving paths with resolve_writable_path(), which will reject reads from SandboxConfig.readable_paths (read-only mounts). file_read should allow reading from read-only paths and only require writability for write/edit operations.

Suggested change
let path = self.context.resolve_writable_path(&args.path)?;
let path = self.context.resolve_readable_path(&args.path)?;

Copilot uses AI. Check for mistakes.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

let raw = tokio::fs::read_to_string(&path)
.await
Expand Down Expand Up @@ -325,7 +333,7 @@ impl Tool for FileWriteTool {
}

async fn call(&self, args: Self::Args) -> Result<Self::Output, Self::Error> {
let path = self.context.resolve_path(&args.path)?;
let path = self.context.resolve_writable_path(&args.path)?;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

// Ensure parent directory exists if requested
if args.create_dirs
Expand Down