diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 8bd675920..5a556f207 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -1162,35 +1162,10 @@ async fn fork_startup_context_then_first_turn_diff_snapshot() -> anyhow::Result< }) .await?; wait_for_event(&initial.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - // The parent rollout writer drains asynchronously after turn completion. - // Wait until the persisted JSONL includes the source user turn before forking from it. - let mut source_history_persisted = false; - for _ in 0..100 { - let history = RolloutRecorder::get_rollout_history(&rollout_path).await; - source_history_persisted = history.ok().is_some_and(|history| { - history.get_rollout_items().into_iter().any(|item| { - matches!( - item, - RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) - if role == "user" - && content.iter().any(|content_item| { - matches!( - content_item, - ContentItem::InputText { text } if text == "fork seed" - ) - }) - ) - }) - }); - if source_history_persisted { - break; - } - sleep(StdDuration::from_millis(10)).await; - } - assert!( - source_history_persisted, - "source rollout should contain the completed pre-fork user turn before forking" - ); + // Forking reads the persisted rollout JSONL, so force the completed source turn to disk + // before snapshotting from it. + initial.codex.ensure_rollout_materialized().await; + initial.codex.flush_rollout().await; let mut fork_config = initial.config.clone(); fork_config.permissions.approval_policy = diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 9fa706911..0635718c9 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -74,6 +74,16 @@ impl CodexThread { self.codex.shutdown_and_wait().await } + #[doc(hidden)] + pub async fn ensure_rollout_materialized(&self) { + self.codex.session.ensure_rollout_materialized().await; + } + + #[doc(hidden)] + pub async fn flush_rollout(&self) { + self.codex.session.flush_rollout().await; + } + pub async fn submit_with_trace( &self, op: Op, diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 074db95e1..07532e5a1 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -595,6 +595,19 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() { }, ]); + #[cfg(windows)] + let expected_deny_write_paths = vec![ + codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(cwd.join(".codex")) + .expect("absolute .codex"), + codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs) + .expect("absolute docs"), + ]; + #[cfg(not(windows))] + let expected_deny_write_paths = vec![ + codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs) + .expect("absolute docs"), + ]; + assert_eq!( resolve_windows_restricted_token_filesystem_overlay( SandboxType::WindowsRestrictedToken, @@ -605,10 +618,7 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() { WindowsSandboxLevel::RestrictedToken, ), Ok(Some(WindowsRestrictedTokenFilesystemOverlay { - additional_deny_write_paths: vec![ - codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs) - .expect("absolute docs"), - ], + additional_deny_write_paths: expected_deny_write_paths, })) ); } diff --git a/codex-rs/core/src/safety_tests.rs b/codex-rs/core/src/safety_tests.rs index 3d05664ba..e7f55693d 100644 --- a/codex-rs/core/src/safety_tests.rs +++ b/codex-rs/core/src/safety_tests.rs @@ -252,3 +252,37 @@ fn explicit_read_only_subpaths_prevent_auto_approval_for_external_sandbox() { SafetyCheck::AskUser, ); } + +#[test] +fn missing_project_dot_codex_config_requires_approval() { + let tmp = TempDir::new().unwrap(); + let cwd = tmp.path().to_path_buf(); + let action = + ApplyPatchAction::new_add_for_test(&cwd.join(".codex").join("config.toml"), "".to_string()); + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: Default::default(), + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + let file_system_sandbox_policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, &cwd); + + assert!(!is_write_patch_constrained_to_writable_paths( + &action, + &file_system_sandbox_policy, + &cwd, + )); + assert_eq!( + assess_patch_safety( + &action, + AskForApproval::OnRequest, + &sandbox_policy, + &file_system_sandbox_policy, + &cwd, + WindowsSandboxLevel::Disabled, + ), + SafetyCheck::AskUser, + ); +} diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index 744a9998f..196fc68e8 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -118,6 +118,7 @@ async fn python_multiprocessing_lock_works_under_sandbox() { #[cfg(target_os = "linux")] let sandbox_env = match linux_sandbox_test_env().await { Some(env) => env, + // Skip on Linux hosts where Landlock cannot actually be enforced. None => return, }; #[cfg(not(target_os = "linux"))] @@ -312,6 +313,78 @@ async fn sandbox_distinguishes_command_and_policy_cwds() { assert!(allowed_exists, "allowed path should exist"); } +#[tokio::test] +async fn sandbox_blocks_first_time_dot_codex_creation() { + core_test_support::skip_if_sandbox!(); + #[cfg(target_os = "linux")] + let sandbox_env = match linux_sandbox_test_env().await { + Some(env) => env, + None => return, + }; + #[cfg(not(target_os = "linux"))] + let sandbox_env = HashMap::new(); + + let temp = tempfile::tempdir().expect("should be able to create temp dir"); + let repo_root = temp.path().join("repo"); + create_dir_all(&repo_root).await.expect("mkdir repo"); + let dot_codex = repo_root.join(".codex"); + let config_toml = dot_codex.join("config.toml"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: Default::default(), + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + let mut child = spawn_command_under_sandbox( + vec![ + "bash".to_string(), + "-lc".to_string(), + "mkdir -p .codex && echo 'sandbox_mode = \"danger-full-access\"' > .codex/config.toml" + .to_string(), + ], + repo_root.clone(), + &policy, + repo_root.as_path(), + StdioPolicy::RedirectForShellTool, + sandbox_env, + ) + .await + .expect("should spawn command creating .codex"); + + let status = child.wait().await.expect("should wait for .codex command"); + assert!( + !status.success(), + "sandbox unexpectedly allowed first-time .codex creation: {status:?}" + ); + let dot_codex_metadata = tokio::fs::symlink_metadata(&dot_codex).await; + if let Ok(metadata) = dot_codex_metadata { + assert!( + !metadata.is_dir(), + "{} should not be creatable as a directory", + dot_codex.display() + ); + } else if let Err(err) = &dot_codex_metadata { + assert_eq!( + err.kind(), + io::ErrorKind::NotFound, + "unexpected metadata error for {}: {err}", + dot_codex.display() + ); + } + let config_toml_exists = match tokio::fs::try_exists(&config_toml).await { + Ok(exists) => exists, + Err(err) if err.kind() == io::ErrorKind::NotADirectory => false, + Err(err) => panic!("try_exists {} failed: {err}", config_toml.display()), + }; + assert!( + !config_toml_exists, + "{} should not have been created", + config_toml.display() + ); +} + fn unix_sock_body() { unsafe { let mut fds = [0i32; 2]; diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 05e7b58e2..dfc5b1282 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -770,14 +770,25 @@ mod tests { assert_eq!( args.args, vec![ + // Start from a read-only view of the full filesystem. "--ro-bind".to_string(), "/".to_string(), "/".to_string(), + // Recreate a writable /dev inside the sandbox. "--dev".to_string(), "/dev".to_string(), + // Make the writable root itself writable again. "--bind".to_string(), "/".to_string(), "/".to_string(), + // Mask the default protected .codex subpath under that writable + // root. Because the root is `/` in this test, the carveout path + // appears as `/.codex`. + "--ro-bind".to_string(), + "/dev/null".to_string(), + "/.codex".to_string(), + // Rebind /dev after the root bind so device nodes remain + // writable/usable inside the writable root. "--bind".to_string(), "/dev".to_string(), "/dev".to_string(), diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 0a5b33702..978334f69 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -264,7 +264,7 @@ impl FileSystemSandboxPolicy { /// into split filesystem policy. pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self { let mut file_system_policy = Self::from(sandbox_policy); - if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) { + if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = sandbox_policy { let legacy_writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); file_system_policy.entries.retain(|entry| { if entry.access != FileSystemAccessMode::Read { @@ -278,6 +278,28 @@ impl FileSystemSandboxPolicy { FileSystemPath::Special { .. } => true, } }); + + if let Ok(cwd_root) = AbsolutePathBuf::from_absolute_path(cwd) { + for protected_path in default_read_only_subpaths_for_writable_root( + &cwd_root, /*protect_missing_dot_codex*/ true, + ) { + append_default_read_only_path_if_no_explicit_rule( + &mut file_system_policy.entries, + protected_path, + ); + } + } + for writable_root in writable_roots { + for protected_path in default_read_only_subpaths_for_writable_root( + writable_root, + /*protect_missing_dot_codex*/ false, + ) { + append_default_read_only_path_if_no_explicit_rule( + &mut file_system_policy.entries, + protected_path, + ); + } + } } file_system_policy @@ -454,7 +476,14 @@ impl FileSystemSandboxPolicy { .iter() .filter(|path| normalize_effective_absolute_path((*path).clone()) == root) .collect(); - let mut read_only_subpaths = default_read_only_subpaths_for_writable_root(&root); + let protect_missing_dot_codex = AbsolutePathBuf::from_absolute_path(cwd) + .ok() + .is_some_and(|cwd| normalize_effective_absolute_path(cwd) == root); + let mut read_only_subpaths: Vec = + default_read_only_subpaths_for_writable_root(&root, protect_missing_dot_codex) + .into_iter() + .filter(|path| !has_explicit_resolved_path_entry(&resolved_entries, path)) + .collect(); // Narrower explicit non-write entries carve out broader writable roots. // More specific write entries still remain writable because they appear // as separate WritableRoot values and are checked independently. @@ -1068,6 +1097,7 @@ fn normalize_effective_absolute_path(path: AbsolutePathBuf) -> AbsolutePathBuf { fn default_read_only_subpaths_for_writable_root( writable_root: &AbsolutePathBuf, + protect_missing_dot_codex: bool, ) -> Vec { let mut subpaths: Vec = Vec::new(); #[allow(clippy::expect_used)] @@ -1089,19 +1119,69 @@ fn default_read_only_subpaths_for_writable_root( subpaths.push(top_level_git); } - // Make .agents/skills and .codex/config.toml and related files read-only - // to the agent, by default. - for subdir in &[".agents", ".codex"] { - #[allow(clippy::expect_used)] - let top_level_codex = writable_root.join(subdir).expect("valid relative path"); - if top_level_codex.as_path().is_dir() { - subpaths.push(top_level_codex); - } + #[allow(clippy::expect_used)] + let top_level_agents = writable_root.join(".agents").expect("valid relative path"); + if top_level_agents.as_path().is_dir() { + subpaths.push(top_level_agents); + } + + // Keep top-level project metadata under .codex read-only to the agent by + // default. For the workspace root itself, protect it even before the + // directory exists so first-time creation still goes through the + // protected-path approval flow. + #[allow(clippy::expect_used)] + let top_level_codex = writable_root.join(".codex").expect("valid relative path"); + if protect_missing_dot_codex || top_level_codex.as_path().is_dir() { + subpaths.push(top_level_codex); } dedup_absolute_paths(subpaths, /*normalize_effective_paths*/ false) } +fn append_path_entry_if_missing( + entries: &mut Vec, + path: AbsolutePathBuf, + access: FileSystemAccessMode, +) { + if entries.iter().any(|entry| { + entry.access == access + && matches!( + &entry.path, + FileSystemPath::Path { path: existing } if existing == &path + ) + }) { + return; + } + + entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access, + }); +} + +fn append_default_read_only_path_if_no_explicit_rule( + entries: &mut Vec, + path: AbsolutePathBuf, +) { + if entries.iter().any(|entry| { + matches!( + &entry.path, + FileSystemPath::Path { path: existing } if existing == &path + ) + }) { + return; + } + + append_path_entry_if_missing(entries, path, FileSystemAccessMode::Read); +} + +fn has_explicit_resolved_path_entry( + entries: &[ResolvedFileSystemEntry], + path: &AbsolutePathBuf, +) -> bool { + entries.iter().any(|entry| &entry.path == path) +} + fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool { path.as_path().is_file() && path.as_path().file_name() == Some(OsStr::new(".git")) } @@ -1211,6 +1291,148 @@ mod tests { Ok(()) } + #[cfg(unix)] + #[test] + fn writable_roots_proactively_protect_missing_dot_codex() { + let cwd = TempDir::new().expect("tempdir"); + let expected_root = AbsolutePathBuf::from_absolute_path( + cwd.path().canonicalize().expect("canonicalize cwd"), + ) + .expect("absolute canonical root"); + let expected_dot_codex = expected_root.join(".codex").expect("expected .codex path"); + + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }]); + + let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + assert_eq!(writable_roots.len(), 1); + assert_eq!(writable_roots[0].root, expected_root); + assert!( + writable_roots[0] + .read_only_subpaths + .contains(&expected_dot_codex) + ); + } + + #[cfg(unix)] + #[test] + fn writable_roots_skip_default_dot_codex_when_explicit_user_rule_exists() { + let cwd = TempDir::new().expect("tempdir"); + let expected_root = AbsolutePathBuf::from_absolute_path( + cwd.path().canonicalize().expect("canonicalize cwd"), + ) + .expect("absolute canonical root"); + let explicit_dot_codex = expected_root.join(".codex").expect("expected .codex path"); + + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: explicit_dot_codex.clone(), + }, + access: FileSystemAccessMode::Write, + }, + ]); + + let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + let workspace_root = writable_roots + .iter() + .find(|root| root.root == expected_root) + .expect("workspace writable root"); + assert!( + !workspace_root + .read_only_subpaths + .contains(&explicit_dot_codex), + "explicit .codex rule should win over the default protected carveout" + ); + assert!( + policy.can_write_path_with_cwd( + explicit_dot_codex + .join("config.toml") + .expect("config.toml") + .as_path(), + cwd.path() + ) + ); + } + + #[test] + fn legacy_workspace_write_projection_blocks_missing_dot_codex_writes() { + let cwd = TempDir::new().expect("tempdir"); + let dot_codex_config = cwd.path().join(".codex").join("config.toml"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![], + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + let file_system_policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd.path()); + + assert!(!file_system_policy.can_write_path_with_cwd(&dot_codex_config, cwd.path())); + } + + #[test] + fn legacy_workspace_write_projection_accepts_relative_cwd() { + let relative_cwd = Path::new("workspace"); + let expected_dot_codex = AbsolutePathBuf::from_absolute_path( + std::env::current_dir() + .expect("current dir") + .join(relative_cwd) + .join(".codex"), + ) + .expect("absolute dot codex"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![], + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + let file_system_policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, relative_cwd); + + assert_eq!( + file_system_policy, + FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: expected_dot_codex, + }, + access: FileSystemAccessMode::Read, + }, + ]) + ); + assert!( + !file_system_policy + .can_write_path_with_cwd(Path::new(".codex/config.toml"), relative_cwd,) + ); + } + #[cfg(unix)] #[test] fn effective_runtime_roots_canonicalize_symlinked_paths() { @@ -1695,8 +1917,10 @@ mod tests { policy.needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),) ); - let legacy_workspace_write = - FileSystemSandboxPolicy::from(&SandboxPolicy::new_workspace_write_policy()); + let legacy_workspace_write = FileSystemSandboxPolicy::from_legacy_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + cwd.path(), + ); assert!( !legacy_workspace_write .needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),) diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2e67fa5d7..71c9cbda6 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1064,13 +1064,20 @@ impl SandboxPolicy { } // For each root, compute subpaths that should remain read-only. + let cwd_root = AbsolutePathBuf::from_absolute_path(cwd).ok(); roots .into_iter() - .map(|writable_root| WritableRoot { - read_only_subpaths: default_read_only_subpaths_for_writable_root( - &writable_root, - ), - root: writable_root, + .map(|writable_root| { + let protect_missing_dot_codex = cwd_root + .as_ref() + .is_some_and(|cwd_root| cwd_root == &writable_root); + WritableRoot { + read_only_subpaths: default_read_only_subpaths_for_writable_root( + &writable_root, + protect_missing_dot_codex, + ), + root: writable_root, + } }) .collect() } @@ -1080,6 +1087,7 @@ impl SandboxPolicy { fn default_read_only_subpaths_for_writable_root( writable_root: &AbsolutePathBuf, + protect_missing_dot_codex: bool, ) -> Vec { let mut subpaths: Vec = Vec::new(); #[allow(clippy::expect_used)] @@ -1101,14 +1109,20 @@ fn default_read_only_subpaths_for_writable_root( subpaths.push(top_level_git); } - // Make .agents/skills and .codex/config.toml and related files read-only - // to the agent, by default. - for subdir in &[".agents", ".codex"] { - #[allow(clippy::expect_used)] - let top_level_codex = writable_root.join(subdir).expect("valid relative path"); - if top_level_codex.as_path().is_dir() { - subpaths.push(top_level_codex); - } + #[allow(clippy::expect_used)] + let top_level_agents = writable_root.join(".agents").expect("valid relative path"); + if top_level_agents.as_path().is_dir() { + subpaths.push(top_level_agents); + } + + // Keep top-level project metadata under .codex read-only to the agent by + // default. For the workspace root itself, protect it even before the + // directory exists so first-time creation still goes through the + // protected-path approval flow. + #[allow(clippy::expect_used)] + let top_level_codex = writable_root.join(".codex").expect("valid relative path"); + if protect_missing_dot_codex || top_level_codex.as_path().is_dir() { + subpaths.push(top_level_codex); } let mut deduped = Vec::with_capacity(subpaths.len()); @@ -4095,6 +4109,8 @@ mod tests { let expected_docs_public = AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs/public")) .expect("canonical docs/public"); + let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex")) + .expect("canonical .codex"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -4116,7 +4132,13 @@ mod tests { assert_eq!( sorted_writable_roots(policy.get_writable_roots_with_cwd(cwd.path())), vec![ - (canonical_cwd, vec![expected_docs.to_path_buf()]), + ( + canonical_cwd, + vec![ + expected_dot_codex.to_path_buf(), + expected_docs.to_path_buf() + ], + ), (expected_docs_public.to_path_buf(), Vec::new()), ] ); @@ -4128,6 +4150,8 @@ mod tests { let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd"); + let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex")) + .expect("canonical .codex"); let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], read_only_access: ReadOnlyAccess::Restricted { @@ -4144,7 +4168,7 @@ mod tests { FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd.path()) .get_writable_roots_with_cwd(cwd.path()) ), - vec![(canonical_cwd, Vec::new())] + vec![(canonical_cwd, vec![expected_dot_codex.to_path_buf()])] ); } diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index e7cca3d2a..e6ff71d0d 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -330,8 +330,14 @@ fn build_seatbelt_access_policy( { let excluded_subpath = normalize_path_for_sandbox(excluded_subpath.as_path()).unwrap_or(excluded_subpath); - let excluded_param = format!("{param_prefix}_{index}_RO_{excluded_index}"); + let excluded_param = format!("{param_prefix}_{index}_EXCLUDED_{excluded_index}"); params.push((excluded_param.clone(), excluded_subpath.into_path_buf())); + // Exclude both the exact protected path and anything beneath it. + // `subpath` alone leaves a gap for first-time creation of the + // protected directory itself, such as `mkdir .codex`. + require_parts.push(format!( + "(require-not (literal (param \"{excluded_param}\")))" + )); require_parts.push(format!( "(require-not (subpath (param \"{excluded_param}\")))" )); diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index 925285763..e508a028c 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -129,22 +129,40 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access() let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/")); let unreadable_root = unreadable_roots.first().expect("expected unreadable root"); assert!( - policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"), + policy.contains("(require-not (literal (param \"READABLE_ROOT_0_EXCLUDED_0\")))"), + "expected exact read carveout in policy:\n{policy}" + ); + assert!( + policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_EXCLUDED_0\")))"), "expected read carveout in policy:\n{policy}" ); assert!( - policy.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_RO_0\")))"), + policy.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_0\")))"), + "expected exact write carveout in policy:\n{policy}" + ); + assert!( + policy.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_0\")))"), "expected write carveout in policy:\n{policy}" ); assert!( - args.iter() - .any(|arg| arg == &format!("-DREADABLE_ROOT_0_RO_0={}", unreadable_root.display())), + args.iter().any( + |arg| arg == &format!("-DREADABLE_ROOT_0_EXCLUDED_0={}", unreadable_root.display()) + ), "expected read carveout parameter in args: {args:#?}" ); - assert!( - args.iter() - .any(|arg| arg == &format!("-DWRITABLE_ROOT_0_RO_0={}", unreadable_root.display())), - "expected write carveout parameter in args: {args:#?}" + let writable_definitions: Vec = args + .iter() + .filter(|arg| arg.starts_with("-DWRITABLE_ROOT_")) + .cloned() + .collect(); + assert_eq!( + writable_definitions, + vec![ + "-DWRITABLE_ROOT_0=/".to_string(), + "-DWRITABLE_ROOT_0_EXCLUDED_0=/.codex".to_string(), + format!("-DWRITABLE_ROOT_0_EXCLUDED_1={}", unreadable_root.display()), + ], + "unexpected write carveout parameters in args: {args:#?}" ); } @@ -179,7 +197,11 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() { let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/")); let unreadable_root = unreadable_roots.first().expect("expected unreadable root"); assert!( - policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"), + policy.contains("(require-not (literal (param \"READABLE_ROOT_0_EXCLUDED_0\")))"), + "expected exact read carveout in policy:\n{policy}" + ); + assert!( + policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_EXCLUDED_0\")))"), "expected read carveout in policy:\n{policy}" ); assert!( @@ -188,8 +210,9 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() { "expected readable root parameter in args: {args:#?}" ); assert!( - args.iter() - .any(|arg| arg == &format!("-DREADABLE_ROOT_0_RO_0={}", unreadable_root.display())), + args.iter().any( + |arg| arg == &format!("-DREADABLE_ROOT_0_EXCLUDED_0={}", unreadable_root.display()) + ), "expected read carveout parameter in args: {args:#?}" ); } @@ -314,14 +337,17 @@ fn seatbelt_legacy_workspace_write_nested_readable_root_stays_writable() { /*extensions*/ None, ); - let docs_param = format!("-DWRITABLE_ROOT_0_RO_0={}", docs.as_path().display()); assert!( - !seatbelt_policy_arg(&args).contains("WRITABLE_ROOT_0_RO_0"), - "legacy workspace-write readable roots under cwd should not become seatbelt carveouts:\n{args:#?}" + !args + .iter() + .any(|arg| arg.ends_with(&format!("={}", docs.as_path().display()))), + "legacy workspace-write readable roots under cwd should not become seatbelt carveouts:\n{args:#?}", ); assert!( - !args.iter().any(|arg| arg == &docs_param), - "unexpected seatbelt carveout parameter for redundant legacy readable root: {args:#?}" + args.iter() + .any(|arg| arg.starts_with("-DWRITABLE_ROOT_0_EXCLUDED_") + && arg.ends_with("/workspace/.codex")), + "expected proactive .codex carveout for cwd root: {args:#?}", ); } @@ -647,30 +673,24 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { /*extensions*/ None, ); - // Build the expected policy text using a raw string for readability. - // Note that the policy includes: - // - the base policy, - // - read-only access to the filesystem, - // - write access to WRITABLE_ROOT_0 (but not its .git or .codex), WRITABLE_ROOT_1, and cwd as WRITABLE_ROOT_2. - let expected_policy = format!( - r#"{MACOS_SEATBELT_BASE_POLICY} -; allow read-only file operations -(allow file-read*) -(allow file-write* -(subpath (param "WRITABLE_ROOT_0")) (require-all (subpath (param "WRITABLE_ROOT_1")) (require-not (subpath (param "WRITABLE_ROOT_1_RO_0"))) (require-not (subpath (param "WRITABLE_ROOT_1_RO_1"))) ) (subpath (param "WRITABLE_ROOT_2")) -) - -; macOS permission profile extensions -(allow ipc-posix-shm-read* (ipc-posix-name-prefix "apple.cfprefs.")) -(allow mach-lookup - (global-name "com.apple.cfprefsd.daemon") - (global-name "com.apple.cfprefsd.agent") - (local-name "com.apple.cfprefsd.agent")) -(allow user-preference-read) -"#, + let policy_text = seatbelt_policy_arg(&args); + assert!( + policy_text.contains("(require-all (subpath (param \"WRITABLE_ROOT_0\"))"), + "expected cwd writable root to carry protected carveouts:\n{policy_text}", + ); + assert!( + policy_text.contains("WRITABLE_ROOT_0_EXCLUDED_0"), + "expected cwd .codex carveout in policy:\n{policy_text}", + ); + assert!( + policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_0") + && policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_1"), + "expected explicit writable root .git/.codex carveouts in policy:\n{policy_text}", + ); + assert!( + policy_text.contains("(subpath (param \"WRITABLE_ROOT_2\"))"), + "expected second explicit writable root grant in policy:\n{policy_text}", ); - - assert_eq!(seatbelt_policy_arg(&args), expected_policy); let expected_definitions = [ format!( @@ -679,16 +699,23 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { .expect("canonicalize cwd") .to_string_lossy() ), + format!( + "-DWRITABLE_ROOT_0_EXCLUDED_0={}", + cwd.canonicalize() + .expect("canonicalize cwd") + .join(".codex") + .display() + ), format!( "-DWRITABLE_ROOT_1={}", vulnerable_root_canonical.to_string_lossy() ), format!( - "-DWRITABLE_ROOT_1_RO_0={}", + "-DWRITABLE_ROOT_1_EXCLUDED_0={}", dot_git_canonical.to_string_lossy() ), format!( - "-DWRITABLE_ROOT_1_RO_1={}", + "-DWRITABLE_ROOT_1_EXCLUDED_1={}", dot_codex_canonical.to_string_lossy() ), format!( @@ -696,12 +723,15 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { empty_root_canonical.to_string_lossy() ), ]; - for expected_definition in expected_definitions { - assert!( - args.contains(&expected_definition), - "expected definition arg `{expected_definition}` in {args:#?}" - ); - } + let writable_definitions: Vec = args + .iter() + .filter(|arg| arg.starts_with("-DWRITABLE_ROOT_")) + .cloned() + .collect(); + assert_eq!( + writable_definitions, expected_definitions, + "unexpected writable-root parameter definitions in {args:#?}" + ); for (key, value) in macos_dir_params() { let expected_definition = format!("-D{key}={}", value.to_string_lossy()); assert!( @@ -818,6 +848,60 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { ); } +#[test] +fn create_seatbelt_args_block_first_time_dot_codex_creation_with_exact_and_descendant_carveouts() { + let tmp = TempDir::new().expect("tempdir"); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(&repo_root).expect("create repo root"); + + Command::new("git") + .arg("init") + .arg(".") + .current_dir(&repo_root) + .output() + .expect("git init ."); + + let dot_codex = repo_root.join(".codex"); + let config_toml = dot_codex.join("config.toml"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![repo_root.as_path().try_into().expect("absolute repo root")], + read_only_access: Default::default(), + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + let shell_command: Vec = [ + "bash", + "-c", + "mkdir -p \"$1\" && echo 'sandbox_mode = \"danger-full-access\"' > \"$2\"", + "bash", + dot_codex.to_string_lossy().as_ref(), + config_toml.to_string_lossy().as_ref(), + ] + .iter() + .map(std::string::ToString::to_string) + .collect(); + let args = create_seatbelt_command_args_with_extensions( + shell_command, + &policy, + repo_root.as_path(), + /*enforce_managed_network*/ false, + /*network*/ None, + /*extensions*/ None, + ); + + let policy_text = seatbelt_policy_arg(&args); + assert!( + policy_text.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"), + "expected exact .codex carveout in policy:\n{policy_text}" + ); + assert!( + policy_text.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"), + "expected descendant .codex carveout in policy:\n{policy_text}" + ); +} + #[test] fn create_seatbelt_args_with_read_only_git_pointer_file() { let tmp = TempDir::new().expect("tempdir"); @@ -971,7 +1055,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { .map(|p| p.to_string_lossy().to_string()); let tempdir_policy_entry = if tmpdir_env_var.is_some() { - r#" (subpath (param "WRITABLE_ROOT_2"))"# + r#" (require-all (subpath (param "WRITABLE_ROOT_2")) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_1"))) )"# } else { "" }; @@ -986,7 +1070,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { ; allow read-only file operations (allow file-read*) (allow file-write* -(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_RO_1"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry} +(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_1"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry} ) ; macOS permission profile extensions @@ -1007,11 +1091,11 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { vulnerable_root_canonical.to_string_lossy() ), format!( - "-DWRITABLE_ROOT_0_RO_0={}", + "-DWRITABLE_ROOT_0_EXCLUDED_0={}", dot_git_canonical.to_string_lossy() ), format!( - "-DWRITABLE_ROOT_0_RO_1={}", + "-DWRITABLE_ROOT_0_EXCLUDED_1={}", dot_codex_canonical.to_string_lossy() ), format!( @@ -1025,6 +1109,14 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { if let Some(p) = tmpdir_env_var { expected_args.push(format!("-DWRITABLE_ROOT_2={p}")); + expected_args.push(format!( + "-DWRITABLE_ROOT_2_EXCLUDED_0={}", + dot_git_canonical.to_string_lossy() + )); + expected_args.push(format!( + "-DWRITABLE_ROOT_2_EXCLUDED_1={}", + dot_codex_canonical.to_string_lossy() + )); } expected_args.extend(