From 2a6482d33f686cb33aabeb7f05e6e356c62f6116 Mon Sep 17 00:00:00 2001 From: Paolo Dettori Date: Fri, 1 May 2026 22:49:47 -0400 Subject: [PATCH 1/2] fix(sandbox): ensure read_write directories are writable by sandbox user Container images may ship /tmp as root:root 0700. The supervisor's prepare_filesystem only chowned newly-created directories, leaving existing ones inaccessible to the unprivileged sandbox user. When a read_write directory exists but is not writable by the target user (neither world-writable nor owned by the user with write bit), set its mode to 1777 (sticky world-writable). This fixes Claude Code failing with EACCES when creating its task directory under /tmp. Assisted-By: Claude (Anthropic AI) Signed-off-by: Paolo Dettori --- crates/openshell-sandbox/src/lib.rs | 58 ++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index 34ee80bb5..5cb26f82c 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -1856,7 +1856,9 @@ fn validate_sandbox_user(policy: &SandboxPolicy) -> Result<()> { /// still needs to be chowned to the sandbox user/group. Existing paths keep /// their image-defined ownership. #[cfg(unix)] -fn prepare_read_write_path(path: &std::path::Path) -> Result { +fn prepare_read_write_path(path: &std::path::Path, uid: Option) -> Result { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + // SECURITY: use symlink_metadata (lstat) to inspect each path *before* // calling chown. chown follows symlinks, so a malicious container image // could place a symlink (e.g. /sandbox -> /etc/shadow) to trick the @@ -1871,10 +1873,28 @@ fn prepare_read_write_path(path: &std::path::Path) -> Result { )); } - debug!( - path = %path.display(), - "Preserving ownership for existing read_write path" - ); + // Ensure existing read_write directories are accessible by the sandbox + // user. Container images may ship directories like /tmp as root:root + // 0700, which prevents the unprivileged sandbox user from writing. + if meta.is_dir() { + let mode = meta.permissions().mode(); + let owned_by_target = uid.is_some_and(|u| u.as_raw() == meta.st_uid()); + let world_writable = mode & 0o002 != 0; + let user_writable = owned_by_target && (mode & 0o200 != 0); + + if !world_writable && !user_writable { + let new_mode = 0o1777; + debug!( + path = %path.display(), + old_mode = format!("{:04o}", mode & 0o7777), + new_mode = format!("{:04o}", new_mode), + "Fixing permissions on existing read_write directory" + ); + std::fs::set_permissions(path, std::fs::Permissions::from_mode(new_mode)) + .into_diagnostic()?; + } + } + Ok(false) } else { debug!(path = %path.display(), "Creating read_write directory"); @@ -1930,8 +1950,9 @@ fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { }; // Create missing read_write paths and only chown the ones we created. + // For existing paths, ensure they are writable by the sandbox user. for path in &policy.filesystem.read_write { - if prepare_read_write_path(path)? { + if prepare_read_write_path(path, uid)? { debug!( path = %path.display(), ?uid, @@ -2725,7 +2746,7 @@ filesystem_policy: let dir = tempfile::tempdir().unwrap(); let missing = dir.path().join("missing").join("nested"); - assert!(prepare_read_write_path(&missing).unwrap()); + assert!(prepare_read_write_path(&missing, None).unwrap()); assert!(missing.is_dir()); } @@ -2736,7 +2757,7 @@ filesystem_policy: let existing = dir.path().join("existing"); std::fs::create_dir(&existing).unwrap(); - assert!(!prepare_read_write_path(&existing).unwrap()); + assert!(!prepare_read_write_path(&existing, None).unwrap()); assert!(existing.is_dir()); } @@ -2749,7 +2770,7 @@ filesystem_policy: std::fs::create_dir(&target).unwrap(); symlink(&target, &link).unwrap(); - let error = prepare_read_write_path(&link).unwrap_err(); + let error = prepare_read_write_path(&link, None).unwrap_err(); assert!( error .to_string() @@ -2792,4 +2813,23 @@ filesystem_policy: assert_eq!(after.uid(), before.uid()); assert_eq!(after.gid(), before.gid()); } + + #[cfg(unix)] + #[test] + fn prepare_read_write_path_fixes_inaccessible_tmp() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let tmp = dir.path().join("tmp"); + std::fs::create_dir(&tmp).unwrap(); + // Simulate container image shipping /tmp as root:root 0700 + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o700)).unwrap(); + + // Use a UID different from the owner to trigger the fix + let fake_uid = nix::unistd::Uid::from_raw(998); + assert!(!prepare_read_write_path(&tmp, Some(fake_uid)).unwrap()); + + let meta = std::fs::metadata(&tmp).unwrap(); + assert_eq!(meta.permissions().mode() & 0o7777, 0o1777); + } } From 7c4d1bcf143fdc08c32545b9b14a6b36760da151 Mon Sep 17 00:00:00 2001 From: Paolo Dettori Date: Fri, 1 May 2026 22:54:33 -0400 Subject: [PATCH 2/2] fix: use portable MetadataExt::uid() instead of st_uid() The st_uid() method is from std::os::linux::fs::MetadataExt while uid() is from std::os::unix::fs::MetadataExt which is portable across all Unix platforms. Assisted-By: Claude (Anthropic AI) Signed-off-by: Paolo Dettori --- crates/openshell-sandbox/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index 5cb26f82c..9dfb2bf9d 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -1878,7 +1878,7 @@ fn prepare_read_write_path(path: &std::path::Path, uid: Option // 0700, which prevents the unprivileged sandbox user from writing. if meta.is_dir() { let mode = meta.permissions().mode(); - let owned_by_target = uid.is_some_and(|u| u.as_raw() == meta.st_uid()); + let owned_by_target = uid.is_some_and(|u| u.as_raw() == meta.uid()); let world_writable = mode & 0o002 != 0; let user_writable = owned_by_target && (mode & 0o200 != 0);