diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index 2821de5a8b..71b55d9094 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -49,6 +49,7 @@ use codex_protocol::config_types::WebSearchToolConfig; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ReasoningEffort; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; @@ -641,15 +642,15 @@ pub struct GhostSnapshotToml { } impl ConfigToml { - /// Derive the effective sandbox policy from the configuration. - pub async fn derive_sandbox_policy( + /// Derive the effective permission profile from the configuration. + pub async fn derive_permission_profile( &self, sandbox_mode_override: Option, profile_sandbox_mode: Option, windows_sandbox_level: WindowsSandboxLevel, active_project: Option<&ProjectConfig>, permission_profile_constraint: Option<&crate::Constrained>, - ) -> SandboxPolicy { + ) -> PermissionProfile { let sandbox_mode_was_explicit = sandbox_mode_override.is_some() || profile_sandbox_mode.is_some() || self.sandbox_mode.is_some(); @@ -677,50 +678,72 @@ impl ConfigToml { }) }) .unwrap_or_default(); - let mut sandbox_policy = match resolved_sandbox_mode { - SandboxMode::ReadOnly => SandboxPolicy::new_read_only_policy(), + let workspace_write_unsupported = cfg!(target_os = "windows") + // If the experimental Windows sandbox is enabled, do not force a downgrade. + && windows_sandbox_level == WindowsSandboxLevel::Disabled + && matches!(resolved_sandbox_mode, SandboxMode::WorkspaceWrite); + let mut permission_profile = match resolved_sandbox_mode { + SandboxMode::ReadOnly => PermissionProfile::read_only(), SandboxMode::WorkspaceWrite => match self.sandbox_workspace_write.as_ref() { Some(SandboxWorkspaceWrite { writable_roots, network_access, exclude_tmpdir_env_var, exclude_slash_tmp, - }) => SandboxPolicy::WorkspaceWrite { - writable_roots: writable_roots.clone(), - network_access: *network_access, - exclude_tmpdir_env_var: *exclude_tmpdir_env_var, - exclude_slash_tmp: *exclude_slash_tmp, - }, - None => SandboxPolicy::new_workspace_write_policy(), + }) => { + let network_policy = if *network_access { + NetworkSandboxPolicy::Enabled + } else { + NetworkSandboxPolicy::Restricted + }; + PermissionProfile::workspace_write_with( + writable_roots, + network_policy, + *exclude_tmpdir_env_var, + *exclude_slash_tmp, + ) + } + None => PermissionProfile::workspace_write(), }, - SandboxMode::DangerFullAccess => SandboxPolicy::DangerFullAccess, + SandboxMode::DangerFullAccess => PermissionProfile::Disabled, }; - let downgrade_workspace_write_if_unsupported = |policy: &mut SandboxPolicy| { - if cfg!(target_os = "windows") - // If the experimental Windows sandbox is enabled, do not force a downgrade. - && windows_sandbox_level == WindowsSandboxLevel::Disabled - && matches!(&*policy, SandboxPolicy::WorkspaceWrite { .. }) - { - *policy = SandboxPolicy::new_read_only_policy(); - } - }; - if matches!(resolved_sandbox_mode, SandboxMode::WorkspaceWrite) { - downgrade_workspace_write_if_unsupported(&mut sandbox_policy); + if workspace_write_unsupported { + permission_profile = PermissionProfile::read_only(); } if !sandbox_mode_was_explicit && let Some(constraint) = permission_profile_constraint - && let Err(err) = constraint.can_set(&PermissionProfile::from_legacy_sandbox_policy( - &sandbox_policy, - )) + && let Err(err) = constraint.can_set(&permission_profile) { tracing::warn!( error = %err, "default sandbox policy is disallowed by requirements; falling back to required default" ); - sandbox_policy = SandboxPolicy::new_read_only_policy(); - downgrade_workspace_write_if_unsupported(&mut sandbox_policy); + permission_profile = PermissionProfile::read_only(); } - sandbox_policy + permission_profile + } + + /// Derive the legacy sandbox projection from configuration. + /// + /// New callers should use [`Self::derive_permission_profile`] instead. + pub async fn derive_sandbox_policy( + &self, + sandbox_mode_override: Option, + profile_sandbox_mode: Option, + windows_sandbox_level: WindowsSandboxLevel, + active_project: Option<&ProjectConfig>, + permission_profile_constraint: Option<&crate::Constrained>, + ) -> SandboxPolicy { + self.derive_permission_profile( + sandbox_mode_override, + profile_sandbox_mode, + windows_sandbox_level, + active_project, + permission_profile_constraint, + ) + .await + .to_legacy_sandbox_policy(Path::new("/")) + .expect("legacy config-derived permission profile should be bridgeable") } /// Resolves the cwd to an existing project, or returns None if ConfigToml diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index d0ea8980bf..7d0bb613ec 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1748,7 +1748,7 @@ exclude_slash_tmp = true } #[tokio::test] -async fn legacy_sandbox_mode_config_builds_split_policies_without_drift() -> std::io::Result<()> { +async fn legacy_sandbox_mode_builds_profiles_with_compatible_projection() -> std::io::Result<()> { let codex_home = TempDir::new()?; let cwd = TempDir::new()?; let extra_root = test_absolute_path("/tmp/legacy-extra-root"); @@ -1793,26 +1793,73 @@ exclude_slash_tmp = true ) .await?; - let sandbox_policy = &config.legacy_sandbox_policy(); + let sandbox_policy = config.legacy_sandbox_policy(); + let file_system_policy = config.permissions.file_system_sandbox_policy(); + let network_policy = config.permissions.network_sandbox_policy(); + assert_eq!( - config.permissions.file_system_sandbox_policy(), - FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(sandbox_policy, cwd.path()), - "case `{name}` should preserve filesystem semantics from legacy config" - ); - assert_eq!( - config.permissions.network_sandbox_policy(), - NetworkSandboxPolicy::from(sandbox_policy), + network_policy, + NetworkSandboxPolicy::from(&sandbox_policy), "case `{name}` should preserve network semantics from legacy config" ); assert_eq!( - config - .permissions - .file_system_sandbox_policy() - .to_legacy_sandbox_policy(config.permissions.network_sandbox_policy(), cwd.path()) + file_system_policy + .to_legacy_sandbox_policy(network_policy, cwd.path()) .unwrap_or_else(|err| panic!("case `{name}` should round-trip: {err}")), - sandbox_policy.clone(), - "case `{name}` should round-trip through split policies without drift" + sandbox_policy, + "case `{name}` should preserve its legacy compatibility projection" ); + + match name.as_str() { + "danger-full-access" | "read-only" => { + assert_eq!( + file_system_policy, + FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( + &sandbox_policy, + cwd.path() + ), + "case `{name}` should match the legacy filesystem projection exactly" + ); + } + "workspace-write" => { + assert!( + file_system_policy + .entries + .contains(&FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }) + ); + assert!( + file_system_policy + .entries + .contains(&FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: extra_root.clone(), + }, + access: FileSystemAccessMode::Write, + }) + ); + assert!( + file_system_policy + .can_write_path_with_cwd(cwd.path().join(".git").as_path(), cwd.path()), + "legacy workspace-write should allow git init to create .git" + ); + assert!( + file_system_policy + .can_write_path_with_cwd(cwd.path().join(".agents").as_path(), cwd.path()), + "legacy workspace-write should allow creating .agents" + ); + assert!( + !file_system_policy + .can_write_path_with_cwd(cwd.path().join(".codex").as_path(), cwd.path()), + "legacy workspace-write should protect .codex before it exists" + ); + } + _ => unreachable!("unexpected test case `{name}`"), + } } Ok(()) diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 9635034dcc..e34390bdb0 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1965,8 +1965,8 @@ impl Config { ) } else { let configured_network_proxy_config = NetworkProxyConfig::default(); - let mut sandbox_policy = cfg - .derive_sandbox_policy( + let mut permission_profile = cfg + .derive_permission_profile( sandbox_mode, config_profile.sandbox_mode, windows_sandbox_level, @@ -1974,24 +1974,34 @@ impl Config { Some(&constrained_permission_profile), ) .await; - if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy { - for path in &additional_writable_roots { - if !writable_roots.iter().any(|existing| existing == path) { - writable_roots.push(path.clone()); - } - } - } - let file_system_sandbox_policy = - FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( + let (mut file_system_sandbox_policy, network_sandbox_policy) = + permission_profile.to_runtime_permissions(); + let sandbox_policy = compatibility_sandbox_policy_for_permission_profile( + &permission_profile, + &file_system_sandbox_policy, + network_sandbox_policy, + resolved_cwd.as_path(), + ); + permission_profile = PermissionProfile::from_legacy_sandbox_policy_for_cwd( &sandbox_policy, resolved_cwd.as_path(), ); - let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); - let permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - network_sandbox_policy, - ); + (file_system_sandbox_policy, _) = permission_profile.to_runtime_permissions(); + if matches!(permission_profile.enforcement(), SandboxEnforcement::Managed) + && file_system_sandbox_policy.can_write_path_with_cwd( + resolved_cwd.as_path(), + resolved_cwd.as_path(), + ) + && !file_system_sandbox_policy.has_full_disk_write_access() + { + file_system_sandbox_policy = file_system_sandbox_policy + .with_additional_legacy_workspace_writable_roots(&additional_writable_roots); + permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( + permission_profile.enforcement(), + &file_system_sandbox_policy, + network_sandbox_policy, + ); + } ( configured_network_proxy_config, permission_profile, diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 7da986efda..0ffbefed27 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -404,55 +404,30 @@ impl PermissionProfile { /// Managed workspace-write filesystem access with restricted network access. pub fn workspace_write() -> Self { + Self::workspace_write_with( + &[], + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ) + } + + /// Managed workspace-write filesystem access with the legacy + /// `sandbox_workspace_write` knobs applied directly to the profile. + pub fn workspace_write_with( + writable_roots: &[AbsolutePathBuf], + network: NetworkSandboxPolicy, + exclude_tmpdir_env_var: bool, + exclude_slash_tmp: bool, + ) -> Self { + let file_system = FileSystemSandboxPolicy::legacy_workspace_write( + writable_roots, + exclude_tmpdir_env_var, + exclude_slash_tmp, + ); Self::Managed { - file_system: ManagedFileSystemPermissions::Restricted { - entries: vec![ - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Root, - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::CurrentWorkingDirectory, - }, - access: FileSystemAccessMode::Write, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::SlashTmp, - }, - access: FileSystemAccessMode::Write, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Tmpdir, - }, - access: FileSystemAccessMode::Write, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::project_roots(Some(".git".into())), - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::project_roots(Some(".agents".into())), - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::project_roots(Some(".codex".into())), - }, - access: FileSystemAccessMode::Read, - }, - ], - glob_scan_max_depth: None, - }, - network: NetworkSandboxPolicy::Restricted, + file_system: ManagedFileSystemPermissions::from_sandbox_policy(&file_system), + network, } } @@ -504,6 +479,14 @@ impl PermissionProfile { ) } + pub fn from_legacy_sandbox_policy_for_cwd(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self { + Self::from_runtime_permissions_with_enforcement( + SandboxEnforcement::from_legacy_sandbox_policy(sandbox_policy), + &FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(sandbox_policy, cwd), + NetworkSandboxPolicy::from(sandbox_policy), + ) + } + pub fn enforcement(&self) -> SandboxEnforcement { match self { Self::Managed { .. } => SandboxEnforcement::Managed, diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 63c2e3c4f7..f85cf661df 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -418,7 +418,6 @@ impl FileSystemSandboxPolicy { /// can preserve the active cwd binding until the policy is actually /// resolved for a turn or command. pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy) -> Self { - let mut file_system_policy = Self::from(sandbox_policy); let SandboxPolicy::WorkspaceWrite { writable_roots, exclude_tmpdir_env_var, @@ -426,26 +425,34 @@ impl FileSystemSandboxPolicy { .. } = sandbox_policy else { - return file_system_policy; + return Self::from(sandbox_policy); }; + Self::legacy_workspace_write(writable_roots, *exclude_tmpdir_env_var, *exclude_slash_tmp) + } + + /// Filesystem policy matching legacy `WorkspaceWrite` semantics without + /// requiring callers to construct a legacy [`SandboxPolicy`] first. + pub fn legacy_workspace_write( + writable_roots: &[AbsolutePathBuf], + exclude_tmpdir_env_var: bool, + exclude_slash_tmp: bool, + ) -> Self { + let mut file_system_policy = legacy_workspace_write_base_policy( + writable_roots, + exclude_tmpdir_env_var, + exclude_slash_tmp, + ); + prune_read_entries_under_writable_roots( &mut file_system_policy.entries, &legacy_non_cwd_writable_roots( writable_roots, - *exclude_tmpdir_env_var, - *exclude_slash_tmp, + exclude_tmpdir_env_var, + exclude_slash_tmp, ), ); - append_default_read_only_project_root_subpath_if_no_explicit_rule( - &mut file_system_policy.entries, - ".git", - ); - append_default_read_only_project_root_subpath_if_no_explicit_rule( - &mut file_system_policy.entries, - ".agents", - ); append_default_read_only_project_root_subpath_if_no_explicit_rule( &mut file_system_policy.entries, ".codex", @@ -613,6 +620,44 @@ impl FileSystemSandboxPolicy { self } + /// Add roots using legacy `WorkspaceWrite` behavior. + /// + /// Unlike [`Self::with_additional_writable_roots`], this mirrors legacy + /// writable-roots semantics by adding exact roots even when they are + /// already writable through `:cwd`, and by adding the default read-only + /// protected subpaths for each new root. + pub fn with_additional_legacy_workspace_writable_roots( + mut self, + additional_writable_roots: &[AbsolutePathBuf], + ) -> Self { + if !matches!(self.kind, FileSystemSandboxKind::Restricted) { + return self; + } + + for path in additional_writable_roots { + if !self.entries.iter().any(|entry| { + entry.access.can_write() + && matches!(&entry.path, FileSystemPath::Path { path: existing } if existing == path) + }) { + self.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { path: path.clone() }, + access: FileSystemAccessMode::Write, + }); + } + + for protected_path in default_read_only_subpaths_for_writable_root( + path, /*protect_missing_dot_codex*/ false, + ) { + append_default_read_only_path_if_no_explicit_rule( + &mut self.entries, + protected_path, + ); + } + } + + self + } + pub fn needs_direct_runtime_enforcement( &self, network_policy: NetworkSandboxPolicy, @@ -991,51 +1036,61 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy { exclude_tmpdir_env_var, exclude_slash_tmp, .. - } => { - let mut entries = vec![FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Root, - }, - access: FileSystemAccessMode::Read, - }]; - - entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::CurrentWorkingDirectory, - }, - access: FileSystemAccessMode::Write, - }); - if !exclude_slash_tmp { - entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::SlashTmp, - }, - access: FileSystemAccessMode::Write, - }); - } - if !exclude_tmpdir_env_var { - entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Tmpdir, - }, - access: FileSystemAccessMode::Write, - }); - } - entries.extend( - writable_roots - .iter() - .cloned() - .map(|path| FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, - access: FileSystemAccessMode::Write, - }), - ); - FileSystemSandboxPolicy::restricted(entries) - } + } => legacy_workspace_write_base_policy( + writable_roots, + *exclude_tmpdir_env_var, + *exclude_slash_tmp, + ), } } } +fn legacy_workspace_write_base_policy( + writable_roots: &[AbsolutePathBuf], + exclude_tmpdir_env_var: bool, + exclude_slash_tmp: bool, +) -> FileSystemSandboxPolicy { + let mut entries = vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }]; + + entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }); + if !exclude_slash_tmp { + entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::SlashTmp, + }, + access: FileSystemAccessMode::Write, + }); + } + if !exclude_tmpdir_env_var { + entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + access: FileSystemAccessMode::Write, + }); + } + entries.extend( + writable_roots + .iter() + .cloned() + .map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Write, + }), + ); + FileSystemSandboxPolicy::restricted(entries) +} + fn resolve_file_system_path( path: &FileSystemPath, cwd: Option<&AbsolutePathBuf>, @@ -1589,18 +1644,6 @@ mod tests { }, access: FileSystemAccessMode::Write, }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::project_roots(Some(".git".into())), - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::project_roots(Some(".agents".into())), - }, - access: FileSystemAccessMode::Read, - }, FileSystemSandboxEntry { path: FileSystemPath::Special { value: FileSystemSpecialPath::project_roots(Some(".codex".into())), @@ -1611,6 +1654,35 @@ mod tests { ); } + #[test] + fn legacy_workspace_write_projection_allows_missing_git_and_agents() { + let cwd = TempDir::new().expect("tempdir"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: Vec::new(), + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + let file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy); + + assert!( + file_system_policy + .can_write_path_with_cwd(cwd.path().join(".git").as_path(), cwd.path()), + "legacy workspace-write must allow git init to create .git" + ); + assert!( + file_system_policy + .can_write_path_with_cwd(cwd.path().join(".agents").as_path(), cwd.path()), + "legacy workspace-write must allow creating .agents" + ); + assert!( + !file_system_policy + .can_write_path_with_cwd(cwd.path().join(".codex").as_path(), cwd.path()), + "legacy workspace-write still protects .codex before it exists" + ); + } + #[cfg(unix)] #[test] fn writable_roots_skip_default_dot_codex_when_explicit_user_rule_exists() { @@ -2380,6 +2452,47 @@ mod tests { ); } + #[test] + fn with_additional_legacy_workspace_writable_roots_protects_metadata() { + let temp_dir = TempDir::new().expect("tempdir"); + let extra = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("extra")) + .expect("resolve extra root"); + std::fs::create_dir_all(extra.join(".git")).expect("create .git dir"); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }]); + + let actual = + policy.with_additional_legacy_workspace_writable_roots(std::slice::from_ref(&extra)); + + assert_eq!( + actual, + FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: extra.clone() + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: extra.join(".git") + }, + access: FileSystemAccessMode::Read, + }, + ]) + ); + } + #[test] fn file_system_access_mode_orders_by_conflict_precedence() { assert!(FileSystemAccessMode::Write > FileSystemAccessMode::Read);