diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 34c41d44b0..a4d7915441 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -530,6 +530,62 @@ fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::io::Re Ok(()) } +#[test] +fn default_permissions_profile_allows_split_tmpdir_writes() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let cfg = ConfigToml { + default_permissions: Some("workspace".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([( + "workspace".to_string(), + PermissionProfileToml { + filesystem: Some(FilesystemPermissionsToml { + entries: BTreeMap::from([ + ( + ":minimal".to_string(), + FilesystemPermissionToml::Access(FileSystemAccessMode::Read), + ), + ( + ":tmpdir".to_string(), + FilesystemPermissionToml::Access(FileSystemAccessMode::Write), + ), + ]), + }), + network: None, + }, + )]), + }), + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.path().to_path_buf(), + )?; + + assert_eq!( + config.permissions.sandbox_policy.get(), + &SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: Vec::new(), + }, + network_access: false, + } + ); + assert!(config.permissions.file_system_sandbox_policy.can_write_path_with_cwd( + Path::new("/tmp/codex-profile-check"), + cwd.path() + )); + Ok(()) +} + #[test] fn permissions_profiles_require_default_permissions() -> std::io::Result<()> { let codex_home = TempDir::new()?; @@ -571,13 +627,13 @@ fn permissions_profiles_require_default_permissions() -> std::io::Result<()> { } #[test] -fn permissions_profiles_reject_writes_outside_workspace_root() -> std::io::Result<()> { +fn permissions_profiles_allow_writes_outside_workspace_root() -> std::io::Result<()> { let codex_home = TempDir::new()?; let cwd = TempDir::new()?; std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; let external_write_path = if cfg!(windows) { r"C:\temp" } else { "/tmp" }; - let err = Config::load_from_base_config_with_overrides( + let config = Config::load_from_base_config_with_overrides( ConfigToml { default_permissions: Some("workspace".to_string()), permissions: Some(PermissionsToml { @@ -601,15 +657,22 @@ fn permissions_profiles_reject_writes_outside_workspace_root() -> std::io::Resul ..Default::default() }, codex_home.path().to_path_buf(), - ) - .expect_err("writes outside the workspace root should be rejected"); + )?; - assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); - assert!( - err.to_string() - .contains("filesystem writes outside the workspace root"), - "{err}" + assert_eq!( + config.permissions.sandbox_policy.get(), + &SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + } ); + assert!(config.permissions.file_system_sandbox_policy.can_write_path_with_cwd( + Path::new(external_write_path), + cwd.path() + )); Ok(()) } diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 0a5b337029..4b02a2f34d 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -114,6 +114,10 @@ impl FileSystemSpecialPath { } } +#[cfg(target_os = "macos")] +const MACOS_TMPDIR_WRITE_BUNDLE: [&str; 4] = + ["/tmp", "/private/tmp", "/var/tmp", "/private/var/tmp"]; + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] pub struct FileSystemSandboxEntry { pub path: FileSystemPath, @@ -591,6 +595,10 @@ impl FileSystemSandboxPolicy { if entry.access.can_write() { if cwd_absolute.as_ref().is_some_and(|cwd| cwd == path) { workspace_root_writable = true; + } else if path_matches_tmpdir_env_var(path) { + tmpdir_writable = true; + } else if path.as_path() == Path::new("/tmp") { + slash_tmp_writable = true; } else { writable_roots.push(path.clone()); } @@ -615,50 +623,49 @@ impl FileSystemSandboxPolicy { FileSystemSpecialPath::CurrentWorkingDirectory => { if entry.access.can_write() { workspace_root_writable = true; - } else if entry.access.can_read() - && let Some(path) = resolve_file_system_special_path( + } else if entry.access.can_read() { + readable_roots.extend(resolve_file_system_special_paths( value, cwd_absolute.as_ref(), - ) - { - readable_roots.push(path); + )); } } FileSystemSpecialPath::ProjectRoots { subpath } => { if subpath.is_none() && entry.access.can_write() { workspace_root_writable = true; - } else if let Some(path) = - resolve_file_system_special_path(value, cwd_absolute.as_ref()) - { + } else { + let resolved_paths = resolve_file_system_special_paths( + value, + cwd_absolute.as_ref(), + ); if entry.access.can_write() { - writable_roots.push(path); + writable_roots.extend(resolved_paths); } else if entry.access.can_read() { - readable_roots.push(path); + readable_roots.extend(resolved_paths); } } } FileSystemSpecialPath::Tmpdir => { if entry.access.can_write() { tmpdir_writable = true; - } else if entry.access.can_read() - && let Some(path) = resolve_file_system_special_path( + if cfg!(target_os = "macos") { + slash_tmp_writable = true; + } + } else if entry.access.can_read() { + readable_roots.extend(resolve_file_system_special_paths( value, cwd_absolute.as_ref(), - ) - { - readable_roots.push(path); + )); } } FileSystemSpecialPath::SlashTmp => { if entry.access.can_write() { slash_tmp_writable = true; - } else if entry.access.can_read() - && let Some(path) = resolve_file_system_special_path( + } else if entry.access.can_read() { + readable_roots.extend(resolve_file_system_special_paths( value, cwd_absolute.as_ref(), - ) - { - readable_roots.push(path); + )); } } FileSystemSpecialPath::Unknown { .. } => {} @@ -699,11 +706,6 @@ impl FileSystemSandboxPolicy { exclude_tmpdir_env_var: !tmpdir_writable, exclude_slash_tmp: !slash_tmp_writable, } - } else if !writable_roots.is_empty() || tmpdir_writable || slash_tmp_writable { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "permissions profile requests filesystem writes outside the workspace root, which is not supported until the runtime enforces FileSystemSandboxPolicy directly", - )); } else { SandboxPolicy::ReadOnly { access: read_only_access, @@ -718,13 +720,13 @@ impl FileSystemSandboxPolicy { let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok(); self.entries .iter() - .filter_map(|entry| { - resolve_entry_path(&entry.path, cwd_absolute.as_ref()).map(|path| { - ResolvedFileSystemEntry { + .flat_map(|entry| { + resolve_entry_paths(&entry.path, cwd_absolute.as_ref()) + .into_iter() + .map(|path| ResolvedFileSystemEntry { path, access: entry.access, - } - }) + }) }) .collect() } @@ -844,12 +846,12 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy { }); } if !exclude_tmpdir_env_var { - entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Tmpdir, - }, - access: FileSystemAccessMode::Write, - }); + if let Some(tmpdir_path) = resolved_tmpdir_env_path() { + entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { path: tmpdir_path }, + access: FileSystemAccessMode::Write, + }); + } } entries.extend( writable_roots @@ -866,25 +868,22 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy { } } -fn resolve_file_system_path( - path: &FileSystemPath, - cwd: Option<&AbsolutePathBuf>, -) -> Option { +fn resolve_file_system_paths(path: &FileSystemPath, cwd: Option<&AbsolutePathBuf>) -> Vec { match path { - FileSystemPath::Path { path } => Some(path.clone()), - FileSystemPath::Special { value } => resolve_file_system_special_path(value, cwd), + FileSystemPath::Path { path } => vec![path.clone()], + FileSystemPath::Special { value } => resolve_file_system_special_paths(value, cwd), } } -fn resolve_entry_path( +fn resolve_entry_paths( path: &FileSystemPath, cwd: Option<&AbsolutePathBuf>, -) -> Option { +) -> Vec { match path { FileSystemPath::Special { value: FileSystemSpecialPath::Root, - } => cwd.map(absolute_root_path_for_cwd), - _ => resolve_file_system_path(path, cwd), + } => cwd.map(absolute_root_path_for_cwd).into_iter().collect(), + _ => resolve_file_system_paths(path, cwd), } } @@ -988,47 +987,77 @@ fn absolute_root_path_for_cwd(cwd: &AbsolutePathBuf) -> AbsolutePathBuf { .unwrap_or_else(|err| panic!("cwd root must be an absolute path: {err}")) } -fn resolve_file_system_special_path( +fn resolve_file_system_special_paths( value: &FileSystemSpecialPath, cwd: Option<&AbsolutePathBuf>, -) -> Option { +) -> Vec { match value { FileSystemSpecialPath::Root | FileSystemSpecialPath::Minimal - | FileSystemSpecialPath::Unknown { .. } => None, + | FileSystemSpecialPath::Unknown { .. } => Vec::new(), FileSystemSpecialPath::CurrentWorkingDirectory => { - let cwd = cwd?; - Some(cwd.clone()) + let Some(cwd) = cwd else { + return Vec::new(); + }; + vec![cwd.clone()] } FileSystemSpecialPath::ProjectRoots { subpath } => { - let cwd = cwd?; + let Some(cwd) = cwd else { + return Vec::new(); + }; match subpath.as_ref() { - Some(subpath) => { - AbsolutePathBuf::resolve_path_against_base(subpath, cwd.as_path()).ok() - } - None => Some(cwd.clone()), - } - } - FileSystemSpecialPath::Tmpdir => { - let tmpdir = std::env::var_os("TMPDIR")?; - if tmpdir.is_empty() { - None - } else { - let tmpdir = AbsolutePathBuf::from_absolute_path(PathBuf::from(tmpdir)).ok()?; - Some(tmpdir) + Some(subpath) => AbsolutePathBuf::resolve_path_against_base(subpath, cwd.as_path()) + .ok() + .into_iter() + .collect(), + None => vec![cwd.clone()], } } + FileSystemSpecialPath::Tmpdir => resolved_tmpdir_paths(), FileSystemSpecialPath::SlashTmp => { #[allow(clippy::expect_used)] let slash_tmp = AbsolutePathBuf::from_absolute_path("/tmp").expect("/tmp is absolute"); if !slash_tmp.as_path().is_dir() { - return None; + return Vec::new(); } - Some(slash_tmp) + vec![slash_tmp] } } } +fn resolved_tmpdir_paths() -> Vec { + let mut paths = Vec::new(); + + if let Some(tmpdir) = resolved_tmpdir_env_path() { + paths.push(tmpdir); + } + + #[cfg(target_os = "macos")] + for path in MACOS_TMPDIR_WRITE_BUNDLE { + #[allow(clippy::expect_used)] + let absolute_path = + AbsolutePathBuf::from_absolute_path(path).expect("macOS temp path is absolute"); + if absolute_path.as_path().is_dir() { + paths.push(absolute_path); + } + } + + dedup_absolute_paths(paths, /*normalize_effective_paths*/ false) +} + +fn resolved_tmpdir_env_path() -> Option { + let tmpdir = std::env::var_os("TMPDIR")?; + if tmpdir.is_empty() { + None + } else { + AbsolutePathBuf::from_absolute_path(PathBuf::from(tmpdir)).ok() + } +} + +fn path_matches_tmpdir_env_var(path: &AbsolutePathBuf) -> bool { + resolved_tmpdir_env_path().as_ref() == Some(path) +} + fn dedup_absolute_paths( paths: Vec, normalize_effective_paths: bool, @@ -1211,6 +1240,61 @@ mod tests { Ok(()) } + #[test] + fn split_write_paths_bridge_to_read_only_legacy_policy() -> std::io::Result<()> { + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + access: FileSystemAccessMode::Write, + }]); + + let sandbox_policy = policy.to_legacy_sandbox_policy( + NetworkSandboxPolicy::Restricted, + Path::new("/tmp/workspace"), + )?; + + assert_eq!( + sandbox_policy, + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + } + ); + Ok(()) + } + + #[cfg(target_os = "macos")] + #[test] + fn tmpdir_special_path_matches_macos_temp_bundle() { + let cwd = TempDir::new().expect("tempdir"); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + access: FileSystemAccessMode::Write, + }]); + + for path in [ + Path::new("/tmp/codex-temp-file"), + Path::new("/private/tmp/codex-temp-file"), + Path::new("/var/tmp/codex-temp-file"), + Path::new("/private/var/tmp/codex-temp-file"), + ] { + assert!(policy.can_write_path_with_cwd(path, cwd.path()), "{path:?}"); + } + + if let Some(tmpdir) = std::env::var_os("TMPDIR") + && !tmpdir.is_empty() + { + let tmpdir_path = PathBuf::from(tmpdir).join("codex-temp-file"); + assert!(policy.can_write_path_with_cwd(&tmpdir_path, cwd.path())); + } + } + #[cfg(unix)] #[test] fn effective_runtime_roots_canonicalize_symlinked_paths() { @@ -1257,18 +1341,14 @@ mod tests { ); let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + #[cfg(not(target_os = "macos"))] assert_eq!(writable_roots.len(), 1); - assert_eq!(writable_roots[0].root, expected_root); - assert!( - writable_roots[0] - .read_only_subpaths - .contains(&expected_blocked) - ); - assert!( - writable_roots[0] - .read_only_subpaths - .contains(&expected_codex) - ); + let tmpdir_root = writable_roots + .iter() + .find(|root| root.root == expected_root) + .expect("expected tmpdir writable root"); + assert!(tmpdir_root.read_only_subpaths.contains(&expected_blocked)); + assert!(tmpdir_root.read_only_subpaths.contains(&expected_codex)); } #[cfg(unix)] @@ -1606,18 +1686,14 @@ mod tests { ); let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + #[cfg(not(target_os = "macos"))] assert_eq!(writable_roots.len(), 1); - assert_eq!(writable_roots[0].root, expected_root); - assert!( - writable_roots[0] - .read_only_subpaths - .contains(&expected_blocked) - ); - assert!( - writable_roots[0] - .read_only_subpaths - .contains(&expected_codex) - ); + let tmpdir_root = writable_roots + .iter() + .find(|root| root.root == expected_root) + .expect("expected tmpdir writable root"); + assert!(tmpdir_root.read_only_subpaths.contains(&expected_blocked)); + assert!(tmpdir_root.read_only_subpaths.contains(&expected_codex)); } #[test] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 6856f19417..2077048a98 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -4166,14 +4166,19 @@ mod tests { access: FileSystemAccessMode::Write, }]); - let err = policy + let actual = policy .to_legacy_sandbox_policy(NetworkSandboxPolicy::Restricted, cwd) - .expect_err("non-workspace writes should be rejected"); + .expect("split writes should fall back to a conservative legacy policy"); - assert!( - err.to_string() - .contains("filesystem writes outside the workspace root"), - "{err}" + assert_eq!( + actual, + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + } ); } diff --git a/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl b/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl index 0e3a7bb2f2..118c93dc21 100644 --- a/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl +++ b/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl @@ -90,12 +90,6 @@ (allow file-read* file-test-existence file-write-data file-ioctl (literal "/dev/dtracehelper")) -; Scratch space so tools can create temp files. -(allow file-read* file-test-existence file-write* (subpath "/tmp")) -(allow file-read* file-write* (subpath "/private/tmp")) -(allow file-read* file-write* (subpath "/var/tmp")) -(allow file-read* file-write* (subpath "/private/var/tmp")) - ; Allow reading standard config directories. (allow file-read* (subpath "/etc")) (allow file-read* (subpath "/private/etc")) diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index 925285763e..64ed084887 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -194,6 +194,65 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() { ); } +#[test] +fn restricted_platform_defaults_do_not_include_temp_writes() { + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Minimal, + }, + access: FileSystemAccessMode::Read, + }]); + + let args = create_seatbelt_command_args_for_policies_with_extensions( + vec!["/bin/true".to_string()], + &file_system_policy, + NetworkSandboxPolicy::Restricted, + Path::new("/"), + false, + None, + None, + ); + + let policy = seatbelt_policy_arg(&args); + for path in ["/tmp", "/private/tmp", "/var/tmp", "/private/var/tmp"] { + assert!( + !policy.contains(&format!("file-write* (subpath \"{path}\")")), + "unexpected implicit temp write for {path}:\n{policy}" + ); + } +} + +#[test] +fn explicit_tmpdir_write_policy_adds_writable_roots() { + let cwd = TempDir::new().expect("tempdir"); + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + access: FileSystemAccessMode::Write, + }]); + + let args = create_seatbelt_command_args_for_policies_with_extensions( + vec!["/bin/true".to_string()], + &file_system_policy, + NetworkSandboxPolicy::Restricted, + cwd.path(), + false, + None, + None, + ); + + let writable_roots = file_system_policy.get_writable_roots_with_cwd(cwd.path()); + assert!(!writable_roots.is_empty()); + for (index, writable_root) in writable_roots.iter().enumerate() { + let expected = format!("-DWRITABLE_ROOT_{index}={}", writable_root.root.display()); + assert!( + args.iter().any(|arg| arg == &expected), + "missing writable root {expected} in args: {args:#?}" + ); + } +} + #[test] fn seatbelt_args_include_macos_permission_extensions() { let cwd = std::env::temp_dir();