From 87b6e80217c6bcfe3aa3facb9754b842ea837545 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 27 Apr 2026 13:41:37 -0700 Subject: [PATCH] permissions: derive config defaults as profiles --- codex-rs/config/src/config_toml.rs | 71 ++++--- codex-rs/core/src/config/config_tests.rs | 249 ++++++++++++++++------- codex-rs/core/src/config/mod.rs | 44 ++-- codex-rs/protocol/src/models.rs | 82 ++++---- codex-rs/protocol/src/permissions.rs | 194 ++++++++++++++---- 5 files changed, 418 insertions(+), 222 deletions(-) diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index 2821de5a8b..d40d9eb29e 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -49,8 +49,8 @@ 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; use codex_utils_path::normalize_for_path_comparison; use schemars::JsonSchema; @@ -641,15 +641,19 @@ 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 legacy sandbox config. + /// + /// Call this only after ruling out `default_permissions`: named + /// `[permissions]` profiles must be compiled through the permissions + /// profile pipeline, not reconstructed from `sandbox_mode`. + 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 +681,53 @@ impl ConfigToml { }) }) .unwrap_or_default(); - let mut sandbox_policy = match resolved_sandbox_mode { - SandboxMode::ReadOnly => SandboxPolicy::new_read_only_policy(), + let effective_sandbox_mode = if 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) + { + SandboxMode::ReadOnly + } else { + resolved_sandbox_mode + }; + + let permission_profile = match effective_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 !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); + PermissionProfile::read_only() + } else { + permission_profile } - sandbox_policy } /// 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 e25c83e58d..21a86dcadd 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -133,6 +133,34 @@ fn http_mcp(url: &str) -> McpServerConfig { } } +async fn derive_legacy_sandbox_policy_for_test( + cfg: &ConfigToml, + sandbox_mode_override: Option, + profile_sandbox_mode: Option, + windows_sandbox_level: WindowsSandboxLevel, + active_project: Option<&ProjectConfig>, + permission_profile_constraint: Option<&Constrained>, +) -> SandboxPolicy { + let permission_profile = cfg + .derive_permission_profile( + sandbox_mode_override, + profile_sandbox_mode, + windows_sandbox_level, + active_project, + permission_profile_constraint, + ) + .await; + permission_profile + .to_legacy_sandbox_policy(Path::new("/")) + .unwrap_or_else(|err| { + tracing::warn!( + error = %err, + "derived permission profile cannot be represented as a legacy sandbox policy; falling back to read-only" + ); + SandboxPolicy::new_read_only_policy() + }) +} + #[tokio::test] async fn load_config_normalizes_relative_cwd_override() -> std::io::Result<()> { let expected_cwd = AbsolutePathBuf::relative_to_current_dir("nested")?; @@ -1630,15 +1658,15 @@ network_access = false # This should be ignored. let sandbox_full_access_cfg = toml::from_str::(sandbox_full_access) .expect("TOML deserialization should succeed"); let sandbox_mode_override = None; - let resolution = sandbox_full_access_cfg - .derive_sandbox_policy( - sandbox_mode_override, - /*profile_sandbox_mode*/ None, - WindowsSandboxLevel::Disabled, - /*active_project*/ None, - /*permission_profile_constraint*/ None, - ) - .await; + let resolution = derive_legacy_sandbox_policy_for_test( + &sandbox_full_access_cfg, + sandbox_mode_override, + /*profile_sandbox_mode*/ None, + WindowsSandboxLevel::Disabled, + /*active_project*/ None, + /*permission_profile_constraint*/ None, + ) + .await; assert_eq!(resolution, SandboxPolicy::DangerFullAccess); let sandbox_read_only = r#" @@ -1651,15 +1679,15 @@ network_access = true # This should be ignored. let sandbox_read_only_cfg = toml::from_str::(sandbox_read_only) .expect("TOML deserialization should succeed"); let sandbox_mode_override = None; - let resolution = sandbox_read_only_cfg - .derive_sandbox_policy( - sandbox_mode_override, - /*profile_sandbox_mode*/ None, - WindowsSandboxLevel::Disabled, - /*active_project*/ None, - /*permission_profile_constraint*/ None, - ) - .await; + let resolution = derive_legacy_sandbox_policy_for_test( + &sandbox_read_only_cfg, + sandbox_mode_override, + /*profile_sandbox_mode*/ None, + WindowsSandboxLevel::Disabled, + /*active_project*/ None, + /*permission_profile_constraint*/ None, + ) + .await; assert_eq!(resolution, SandboxPolicy::new_read_only_policy()); let writable_root = test_absolute_path("/my/workspace"); @@ -1683,15 +1711,15 @@ trust_level = "trusted" let sandbox_workspace_write_cfg = toml::from_str::(&sandbox_workspace_write) .expect("TOML deserialization should succeed"); let sandbox_mode_override = None; - let resolution = sandbox_workspace_write_cfg - .derive_sandbox_policy( - sandbox_mode_override, - /*profile_sandbox_mode*/ None, - WindowsSandboxLevel::Disabled, - /*active_project*/ None, - /*permission_profile_constraint*/ None, - ) - .await; + let resolution = derive_legacy_sandbox_policy_for_test( + &sandbox_workspace_write_cfg, + sandbox_mode_override, + /*profile_sandbox_mode*/ None, + WindowsSandboxLevel::Disabled, + /*active_project*/ None, + /*permission_profile_constraint*/ None, + ) + .await; if cfg!(target_os = "windows") { assert_eq!(resolution, SandboxPolicy::new_read_only_policy()); } else { @@ -1723,15 +1751,15 @@ exclude_slash_tmp = true let sandbox_workspace_write_cfg = toml::from_str::(&sandbox_workspace_write) .expect("TOML deserialization should succeed"); let sandbox_mode_override = None; - let resolution = sandbox_workspace_write_cfg - .derive_sandbox_policy( - sandbox_mode_override, - /*profile_sandbox_mode*/ None, - WindowsSandboxLevel::Disabled, - /*active_project*/ None, - /*permission_profile_constraint*/ None, - ) - .await; + let resolution = derive_legacy_sandbox_policy_for_test( + &sandbox_workspace_write_cfg, + sandbox_mode_override, + /*profile_sandbox_mode*/ None, + WindowsSandboxLevel::Disabled, + /*active_project*/ None, + /*permission_profile_constraint*/ None, + ) + .await; if cfg!(target_os = "windows") { assert_eq!(resolution, SandboxPolicy::new_read_only_policy()); } else { @@ -1748,7 +1776,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 +1821,91 @@ 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" => { + if cfg!(target_os = "windows") { + assert_eq!( + sandbox_policy, + SandboxPolicy::new_read_only_policy(), + "legacy workspace-write should keep the existing Windows downgrade when \ + the experimental Windows sandbox is disabled" + ); + assert_eq!( + file_system_policy, + FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( + &sandbox_policy, + cwd.path() + ), + "downgraded workspace-write should match the legacy read-only projection" + ); + continue; + } + assert!( + file_system_policy + .entries + .contains(&FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(/*subpath*/ None), + }, + access: FileSystemAccessMode::Write, + }) + ); + assert!( + file_system_policy + .entries + .contains(&FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: extra_root.clone(), + }, + access: FileSystemAccessMode::Write, + }) + ); + for subpath in [".git", ".agents", ".codex"] { + assert!( + file_system_policy + .entries + .contains(&FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(Some( + subpath.into() + )), + }, + access: FileSystemAccessMode::Read, + }), + "case `{name}` should preserve `{subpath}` as a symbolic project-root \ + metadata carveout" + ); + } + } + _ => unreachable!("unexpected test case `{name}`"), + } } Ok(()) @@ -6310,15 +6403,15 @@ trust_level = "untrusted" trust_level: Some(TrustLevel::Untrusted), }; - let resolution = cfg - .derive_sandbox_policy( - /*sandbox_mode_override*/ None, - /*profile_sandbox_mode*/ None, - WindowsSandboxLevel::Disabled, - Some(&active_project), - /*permission_profile_constraint*/ None, - ) - .await; + let resolution = derive_legacy_sandbox_policy_for_test( + &cfg, + /*sandbox_mode_override*/ None, + /*profile_sandbox_mode*/ None, + WindowsSandboxLevel::Disabled, + Some(&active_project), + /*permission_profile_constraint*/ None, + ) + .await; // Verify that untrusted projects get WorkspaceWrite (or ReadOnly on Windows due to downgrade) if cfg!(target_os = "windows") { @@ -6367,15 +6460,15 @@ async fn derive_sandbox_policy_falls_back_to_read_only_for_implicit_defaults() - } })?; - let resolution = cfg - .derive_sandbox_policy( - /*sandbox_mode_override*/ None, - /*profile_sandbox_mode*/ None, - WindowsSandboxLevel::Disabled, - Some(&active_project), - Some(&constrained), - ) - .await; + let resolution = derive_legacy_sandbox_policy_for_test( + &cfg, + /*sandbox_mode_override*/ None, + /*profile_sandbox_mode*/ None, + WindowsSandboxLevel::Disabled, + Some(&active_project), + Some(&constrained), + ) + .await; assert_eq!(resolution, SandboxPolicy::new_read_only_policy()); Ok(()) @@ -6423,15 +6516,15 @@ async fn derive_sandbox_policy_preserves_windows_downgrade_for_unsupported_fallb }, )?; - let resolution = cfg - .derive_sandbox_policy( - /*sandbox_mode_override*/ None, - /*profile_sandbox_mode*/ None, - WindowsSandboxLevel::Disabled, - Some(&active_project), - Some(&constrained), - ) - .await; + let resolution = derive_legacy_sandbox_policy_for_test( + &cfg, + /*sandbox_mode_override*/ None, + /*profile_sandbox_mode*/ None, + WindowsSandboxLevel::Disabled, + Some(&active_project), + Some(&constrained), + ) + .await; if cfg!(target_os = "windows") { assert_eq!(resolution, SandboxPolicy::new_read_only_policy()); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 47f2e4dd9f..34c3634687 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1974,8 +1974,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, @@ -1983,24 +1983,30 @@ 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()); - } - } + if let Err(err) = permission_profile.to_legacy_sandbox_policy(resolved_cwd.as_path()) { + tracing::warn!( + error = %err, + "derived permission profile cannot be represented as a legacy sandbox policy; falling back to read-only" + ); + permission_profile = PermissionProfile::read_only(); + } + let (mut file_system_sandbox_policy, network_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, + ); } - let file_system_sandbox_policy = - FileSystemSandboxPolicy::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, - ); ( configured_network_proxy_config, permission_profile, diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 8f285159db..994303636e 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -408,55 +408,33 @@ impl PermissionProfile { /// The returned profile contains symbolic `:project_roots` entries that /// must be resolved against the active permission root before enforcement. 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. + /// + /// The returned profile contains symbolic `:project_roots` entries that + /// must be resolved against the active permission root before enforcement. + pub fn workspace_write_with( + writable_roots: &[AbsolutePathBuf], + network: NetworkSandboxPolicy, + exclude_tmpdir_env_var: bool, + exclude_slash_tmp: bool, + ) -> Self { + let file_system = FileSystemSandboxPolicy::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::project_roots(/*subpath*/ None), - }, - 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, } } @@ -508,6 +486,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 7d389a9519..7df2328615 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -418,7 +418,6 @@ impl FileSystemSandboxPolicy { /// the profile independent of the concrete root until it is 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,15 +425,31 @@ impl FileSystemSandboxPolicy { .. } = sandbox_policy else { - return file_system_policy; + return Self::from(sandbox_policy); }; + Self::workspace_write(writable_roots, *exclude_tmpdir_env_var, *exclude_slash_tmp) + } + + /// Filesystem policy matching `WorkspaceWrite` semantics without requiring + /// callers to construct a legacy [`SandboxPolicy`] first. + pub fn 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, ), ); @@ -635,6 +650,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 `:project_roots`, 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, @@ -1008,51 +1061,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::project_roots(/*subpath*/ None), - }, - 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::project_roots(/*subpath*/ None), + }, + 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>, @@ -2402,6 +2465,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::project_roots(/*subpath*/ None), + }, + 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::project_roots(/*subpath*/ None), + }, + 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);