diff --git a/codex-rs/README.md b/codex-rs/README.md index 3d290b08f1..18bffd9f64 100644 --- a/codex-rs/README.md +++ b/codex-rs/README.md @@ -86,12 +86,6 @@ codex --sandbox danger-full-access ``` In `workspace-write`, Codex also includes `~/.codex/memories` in its writable roots so memory maintenance does not require an extra approval. -Set the following to allow Git metadata writes under writable roots while keeping Git config and hooks read-only: - -```toml -[sandbox_workspace_write] -allow_limited_git_writes = true -``` ## Code Organization diff --git a/codex-rs/app-server/src/request_processors/command_exec_processor.rs b/codex-rs/app-server/src/request_processors/command_exec_processor.rs index 5b14f0b0c0..9d3ca22f91 100644 --- a/codex-rs/app-server/src/request_processors/command_exec_processor.rs +++ b/codex-rs/app-server/src/request_processors/command_exec_processor.rs @@ -238,8 +238,11 @@ impl CommandExecRequestProcessor { .permissions .can_set_legacy_sandbox_policy(&policy, &sandbox_cwd) .map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?; - let file_system_sandbox_policy = + let mut file_system_sandbox_policy = codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, &sandbox_cwd); + if self.config.managed_allow_limited_git_writes() == Some(true) { + file_system_sandbox_policy.apply_managed_limited_git_writes(&sandbox_cwd); + } let network_sandbox_policy = codex_protocol::permissions::NetworkSandboxPolicy::from(&policy); let permission_profile = diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 86d64e4153..437089ba38 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -505,11 +505,13 @@ impl From for NetworkConstraints { #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct FilesystemRequirementsToml { pub deny_read: Option>, + pub allow_limited_git_writes: Option, } #[derive(Deserialize)] struct RawFilesystemRequirementsToml { deny_read: Option>, + allow_limited_git_writes: Option, description: Option, extends: Option, workspace_roots: Option, @@ -525,6 +527,7 @@ impl<'de> Deserialize<'de> for FilesystemRequirementsToml { let raw = RawFilesystemRequirementsToml::deserialize(deserializer)?; let RawFilesystemRequirementsToml { deny_read, + allow_limited_git_writes, description, extends, workspace_roots, @@ -543,7 +546,10 @@ impl<'de> Deserialize<'de> for FilesystemRequirementsToml { )); } - Ok(Self { deny_read }) + Ok(Self { + deny_read, + allow_limited_git_writes, + }) } } @@ -559,15 +565,18 @@ pub struct PermissionsRequirementsToml { #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct FilesystemConstraints { pub deny_read: Vec, + pub allow_limited_git_writes: Option, } impl From for FilesystemConstraints { fn from(value: PermissionsRequirementsToml) -> Self { - let deny_read = value - .filesystem - .and_then(|filesystem| filesystem.deny_read) - .unwrap_or_default(); - Self { deny_read } + let Some(filesystem) = value.filesystem else { + return Self::default(); + }; + Self { + deny_read: filesystem.deny_read.unwrap_or_default(), + allow_limited_git_writes: filesystem.allow_limited_git_writes, + } } } @@ -1994,6 +2003,7 @@ allowed_approvals_reviewers = ["user"] r#" [permissions.filesystem] deny_read = [{deny_read_0:?}, {deny_read_1:?}] + allow_limited_git_writes = true "# ); @@ -2008,6 +2018,7 @@ allowed_approvals_reviewers = ["user"] AbsolutePathBuf::from_absolute_path(deny_read_0)?.into(), AbsolutePathBuf::from_absolute_path(deny_read_1)?.into(), ], + allow_limited_git_writes: Some(true), }, RequirementSource::Unknown, )) @@ -2036,6 +2047,7 @@ allowed_approvals_reviewers = ["user"] FilesystemDenyReadPattern::from_input("./private/**/*.txt") .expect("normalize glob pattern"), ], + allow_limited_git_writes: None, }, RequirementSource::Unknown, )) diff --git a/codex-rs/config/src/permissions_toml.rs b/codex-rs/config/src/permissions_toml.rs index 0baec2a5cc..0ef2684a83 100644 --- a/codex-rs/config/src/permissions_toml.rs +++ b/codex-rs/config/src/permissions_toml.rs @@ -233,6 +233,8 @@ pub struct FilesystemPermissionsToml { /// platforms that snapshot glob matches before sandbox startup. #[schemars(range(min = 1))] pub glob_scan_max_depth: Option, + #[serde(default)] + pub allow_git: bool, #[serde(flatten)] pub entries: BTreeMap, } diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 9e6a867475..23934fae81 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -883,8 +883,6 @@ pub struct SandboxWorkspaceWrite { #[serde(default)] pub network_access: bool, #[serde(default)] - pub allow_limited_git_writes: bool, - #[serde(default)] pub exclude_tmpdir_env_var: bool, #[serde(default)] pub exclude_slash_tmp: bool, @@ -895,7 +893,7 @@ impl From for codex_app_server_protocol::SandboxSettings Self { writable_roots: sandbox_workspace_write.writable_roots, network_access: Some(sandbox_workspace_write.network_access), - allow_limited_git_writes: Some(sandbox_workspace_write.allow_limited_git_writes), + allow_limited_git_writes: None, exclude_tmpdir_env_var: Some(sandbox_workspace_write.exclude_tmpdir_env_var), exclude_slash_tmp: Some(sandbox_workspace_write.exclude_slash_tmp), } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 1ee26b80a0..6dd85dffc2 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2514,10 +2514,6 @@ "SandboxWorkspaceWrite": { "additionalProperties": false, "properties": { - "allow_limited_git_writes": { - "default": false, - "type": "boolean" - }, "exclude_slash_tmp": { "default": false, "type": "boolean" diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index d0097d7537..27ca34906d 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -1473,6 +1473,267 @@ extends = ":workspace" Ok(()) } +async fn load_config_with_system_filesystem_requirements( + tmp: &Path, + cwd: &Path, + user_config: &str, + requirements: &str, + permission_profile: Option, +) -> anyhow::Result { + let codex_home = tmp.join("home"); + tokio::fs::create_dir_all(&codex_home).await?; + tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), user_config).await?; + let requirements_path = tmp.join("requirements.toml"); + tokio::fs::write(&requirements_path, requirements).await?; + let cwd = AbsolutePathBuf::from_absolute_path(cwd)?; + let mut overrides = LoaderOverrides::without_managed_config_for_tests(); + overrides.system_requirements_path = Some(requirements_path); + Ok(ConfigBuilder::default() + .codex_home(codex_home) + .fallback_cwd(Some(cwd.to_path_buf())) + .harness_overrides(ConfigOverrides { + permission_profile, + ..Default::default() + }) + .loader_overrides(overrides) + .build() + .await?) +} + +#[tokio::test] +async fn system_filesystem_requirements_enable_limited_git_writes_for_all_profile_sources() +-> anyhow::Result<()> { + for (source, user_config, permission_profile, managed_profile) in [ + ( + ":workspace", + r#"default_permissions = ":workspace""#, + None, + "", + ), + ( + "local", + r#" +default_permissions = "local" + +[permissions.local] +extends = ":workspace" +"#, + None, + "", + ), + ( + "managed-standard", + r#"default_permissions = "managed-standard""#, + None, + r#" +[permissions.managed-standard] +extends = ":workspace" +"#, + ), + ( + "legacy workspace-write", + r#"sandbox_mode = "workspace-write""#, + None, + "", + ), + ( + "permission profile override", + "", + Some(PermissionProfile::workspace_write()), + "", + ), + ] { + let tmp = tempdir()?; + tokio::fs::create_dir_all(tmp.path().join(".git").join("hooks")).await?; + let requirements = format!( + r#" +[permissions.filesystem] +allow_limited_git_writes = true + +{managed_profile}"# + ); + let config = load_config_with_system_filesystem_requirements( + tmp.path(), + tmp.path(), + user_config, + &requirements, + permission_profile, + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + assert!( + policy.allow_limited_git_writes, + "expected system requirements to enable limited git writes for `{source}`" + ); + assert!( + policy.can_write_path_with_cwd(&tmp.path().join(".git").join("HEAD"), tmp.path()), + "expected system requirements to permit Git metadata writes for `{source}`" + ); + assert!( + !policy.can_write_path_with_cwd(&tmp.path().join(".git").join("config"), tmp.path()), + "expected Git config to remain protected for `{source}`" + ); + assert!( + !policy.can_write_path_with_cwd( + &tmp.path().join(".git").join("hooks").join("pre-commit"), + tmp.path(), + ), + "expected Git hooks to remain protected for `{source}`" + ); + } + Ok(()) +} + +#[tokio::test] +async fn system_filesystem_requirements_can_disable_profile_limited_git_writes() +-> anyhow::Result<()> { + let tmp = tempdir()?; + tokio::fs::create_dir_all(tmp.path().join(".git").join("hooks")).await?; + let config = load_config_with_system_filesystem_requirements( + tmp.path(), + tmp.path(), + r#" +default_permissions = "local" + +[permissions.local] +extends = ":workspace" + +[permissions.local.filesystem] +allow_limited_git_writes = true +"#, + r#" +[permissions.filesystem] +allow_limited_git_writes = false +"#, + None, + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + assert!( + !policy.allow_limited_git_writes, + "expected system requirements to override local limited git write opt-in" + ); + assert!( + !policy.can_write_path_with_cwd(&tmp.path().join(".git").join("HEAD"), tmp.path()), + "expected the disabled managed setting to protect Git metadata" + ); + Ok(()) +} + +#[tokio::test] +async fn system_filesystem_requirements_override_conflicting_profile_git_writes() +-> anyhow::Result<()> { + let tmp = tempdir()?; + tokio::fs::create_dir_all(tmp.path().join(".git").join("hooks")).await?; + let config = load_config_with_system_filesystem_requirements( + tmp.path(), + tmp.path(), + r#" +default_permissions = "local" + +[permissions.local] +extends = ":workspace" + +[permissions.local.filesystem.":workspace_roots"] +".git/config" = "write" +".git/hooks/pre-commit" = "write" +"#, + r#" +[permissions.filesystem] +allow_limited_git_writes = true +"#, + None, + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + assert!(policy.can_write_path_with_cwd(&tmp.path().join(".git/HEAD"), tmp.path())); + assert!(!policy.can_write_path_with_cwd(&tmp.path().join(".git/config"), tmp.path())); + assert!( + !policy.can_write_path_with_cwd(&tmp.path().join(".git/hooks/pre-commit"), tmp.path(),) + ); + Ok(()) +} + +#[tokio::test] +async fn system_filesystem_requirements_enable_linked_worktree_gitdir_writes() -> anyhow::Result<()> +{ + let tmp = tempdir()?; + let cwd = tmp.path().join("worktree"); + let gitdir = tmp.path().join("repo/.git/worktrees/feature"); + tokio::fs::create_dir_all(gitdir.join("hooks")).await?; + tokio::fs::create_dir_all(&cwd).await?; + tokio::fs::write(cwd.join(".git"), format!("gitdir: {}\n", gitdir.display())).await?; + let config = load_config_with_system_filesystem_requirements( + tmp.path(), + &cwd, + r#" +sandbox_mode = "workspace-write" + +[sandbox_workspace_write] +exclude_tmpdir_env_var = true +exclude_slash_tmp = true +"#, + r#" +[permissions.filesystem] +allow_limited_git_writes = true +"#, + None, + ) + .await?; + + let cwd = AbsolutePathBuf::from_absolute_path(&cwd)?; + let gitdir = AbsolutePathBuf::from_absolute_path(gitdir.canonicalize()?)?; + + let policy = config.permissions.file_system_sandbox_policy(); + assert!( + policy.can_write_path_with_cwd(gitdir.join("HEAD").as_path(), cwd.as_path()), + "expected linked gitdir metadata to be writable: {policy:#?}" + ); + assert!(!policy.can_write_path_with_cwd(gitdir.join("config").as_path(), cwd.as_path())); + assert!( + !policy.can_write_path_with_cwd(gitdir.join("hooks/pre-commit").as_path(), cwd.as_path(),) + ); + let gitdir_root = policy + .get_writable_roots_with_cwd(cwd.as_path()) + .into_iter() + .find(|root| root.root == gitdir) + .expect("linked gitdir should be exported as writable"); + assert!(!gitdir_root.is_path_writable(gitdir.join("config").as_path())); + assert!(!gitdir_root.is_path_writable(gitdir.join("hooks/pre-commit").as_path())); + Ok(()) +} + +#[tokio::test] +async fn system_filesystem_requirements_reject_unrestricted_profile_selection() -> anyhow::Result<()> +{ + let tmp = tempdir()?; + let config = load_config_with_system_filesystem_requirements( + tmp.path(), + tmp.path(), + r#"sandbox_mode = "danger-full-access""#, + r#" +[permissions.filesystem] +allow_limited_git_writes = true +"#, + None, + ) + .await?; + + assert!( + !config + .permissions + .file_system_sandbox_policy() + .has_full_disk_write_access() + ); + assert!(config.startup_warnings.iter().any(|warning| { + warning.contains("Configured value for `permission_profile` is disallowed by requirements") + })); + Ok(()) +} + #[tokio::test] async fn system_allowed_permissions_keep_builtin_permission_fallbacks() -> anyhow::Result<()> { for (trust_level, expected_profile) in [ diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index ff666b51e9..ab481e2e1a 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2148,7 +2148,11 @@ fn resolve_permission_config_syntax( fn apply_managed_filesystem_constraints( file_system_sandbox_policy: &mut FileSystemSandboxPolicy, filesystem_constraints: &codex_config::FilesystemConstraints, + cwd: &Path, ) { + if filesystem_constraints.allow_limited_git_writes == Some(true) { + file_system_sandbox_policy.apply_managed_limited_git_writes(cwd); + } for deny_read in &filesystem_constraints.deny_read { let deny_entry = if deny_read.contains_glob() { codex_protocol::permissions::FileSystemSandboxEntry { @@ -3274,7 +3278,8 @@ impl Config { value: filesystem_requirements, source: filesystem_requirements_source, }) = filesystem_requirements.as_ref() - && !filesystem_requirements.deny_read.is_empty() + && (!filesystem_requirements.deny_read.is_empty() + || filesystem_requirements.allow_limited_git_writes.is_some()) { let requirement_source = filesystem_requirements_source.clone(); constrained_permission_profile @@ -3365,6 +3370,7 @@ impl Config { apply_managed_filesystem_constraints( &mut effective_file_system_sandbox_policy, filesystem_requirements, + resolved_cwd.as_path(), ); } let effective_file_system_sandbox_policy = effective_file_system_sandbox_policy @@ -3687,6 +3693,15 @@ impl Config { .is_some() } + /// Global Git metadata write mode configured by filesystem requirements. + pub fn managed_allow_limited_git_writes(&self) -> Option { + self.config_layer_stack + .requirements() + .filesystem + .as_ref() + .and_then(|filesystem| filesystem.value.allow_limited_git_writes) + } + pub(crate) fn network_proxy_spec_for_active_permission_profile( &self, active_permission_profile: &ActivePermissionProfile, diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index b7292ce3fb..92569def14 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -250,10 +250,22 @@ pub(crate) fn compile_permission_profile( profile, inherited_profile_names, } = resolve_permission_profile(permissions, profile_name)?; + let allow_git = profile + .filesystem + .as_ref() + .is_some_and(|filesystem| filesystem.allow_git); let base_permissions = inherited_profile_names.iter().find_map(|name| { match name.as_str() { BUILT_IN_READ_ONLY_PROFILE => Some(PermissionProfile::read_only()), - BUILT_IN_WORKSPACE_PROFILE => Some(PermissionProfile::workspace_write()), + BUILT_IN_WORKSPACE_PROFILE => { + Some(PermissionProfile::workspace_write_with_limited_git_writes( + &[], + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + allow_git, + )) + } _ => None, } .map(|profile| profile.to_runtime_permissions()) @@ -316,6 +328,9 @@ pub(crate) fn compile_permission_profile( if let Some(glob_scan_max_depth) = glob_scan_max_depth { file_system_sandbox_policy.glob_scan_max_depth = Some(glob_scan_max_depth); } + if allow_git { + file_system_sandbox_policy.apply_managed_limited_git_writes(policy_cwd); + } let network_sandbox_policy = compile_network_sandbox_policy(profile.network.as_ref(), base_network_sandbox_policy); Ok((file_system_sandbox_policy, network_sandbox_policy)) diff --git a/codex-rs/core/src/config/permissions_tests.rs b/codex-rs/core/src/config/permissions_tests.rs index 41f4179ba8..77d1c9ff0f 100644 --- a/codex-rs/core/src/config/permissions_tests.rs +++ b/codex-rs/core/src/config/permissions_tests.rs @@ -542,6 +542,48 @@ fn glob_scan_max_depth_must_be_positive() { ); } +#[test] +fn workspace_parent_uses_profile_limited_git_write_setting() -> std::io::Result<()> { + let cwd = TempDir::new()?; + std::fs::create_dir_all(cwd.path().join(".git").join("hooks"))?; + let mut startup_warnings = Vec::new(); + let (file_system_policy, _) = compile_permission_profile( + &PermissionsToml { + entries: BTreeMap::from([( + "workspace".to_string(), + PermissionProfileToml { + description: None, + extends: Some(BUILT_IN_WORKSPACE_PROFILE.to_string()), + workspace_roots: None, + filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, + allow_limited_git_writes: true, + entries: BTreeMap::new(), + }), + network: None, + }, + )]), + }, + "workspace", + cwd.path(), + &mut startup_warnings, + )?; + + assert!( + file_system_policy + .can_write_path_with_cwd(&cwd.path().join(".git").join("HEAD"), cwd.path()) + ); + assert!( + !file_system_policy + .can_write_path_with_cwd(&cwd.path().join(".git").join("config"), cwd.path()) + ); + assert!(!file_system_policy.can_write_path_with_cwd( + &cwd.path().join(".git").join("hooks").join("pre-commit"), + cwd.path() + )); + Ok(()) +} + #[test] fn read_write_trailing_glob_suffix_compiles_as_subpath() -> std::io::Result<()> { let cwd = TempDir::new()?; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 12254f192d..094619a34c 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -584,6 +584,7 @@ impl Codex { approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, permission_profile_state: session_permission_profile_state_from_config(&config)?, + managed_allow_limited_git_writes: config.managed_allow_limited_git_writes(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), workspace_roots: config.workspace_roots.clone(), diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 995f07d701..768b433213 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -70,6 +70,7 @@ pub(crate) struct SessionConfiguration { /// active profile id, and profile-defined workspace roots in sync by using /// the methods below instead of mutating the fields independently. pub(super) permission_profile_state: PermissionProfileState, + pub(super) managed_allow_limited_git_writes: Option, pub(super) windows_sandbox_level: WindowsSandboxLevel, /// Absolute working directory that should be treated as the *root* of the @@ -171,6 +172,15 @@ impl SessionConfiguration { .network_sandbox_policy() } + fn apply_managed_limited_git_write_override( + &self, + file_system_sandbox_policy: &mut FileSystemSandboxPolicy, + ) { + if self.managed_allow_limited_git_writes == Some(true) { + file_system_sandbox_policy.apply_managed_limited_git_writes(self.cwd.as_path()); + } + } + pub(super) fn thread_config_snapshot(&self) -> ThreadConfigSnapshot { ThreadConfigSnapshot { model: self.collaboration_mode.model().to_string(), @@ -329,12 +339,14 @@ impl SessionConfiguration { next_configuration.original_config_do_not_use = Arc::new(config); } } else if let Some(sandbox_policy) = updates.sandbox_policy.clone() { - let file_system_sandbox_policy = + let mut file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_preserving_deny_entries( &sandbox_policy, &next_configuration.cwd, ¤t_file_system_sandbox_policy, ); + next_configuration + .apply_managed_limited_git_write_override(&mut file_system_sandbox_policy); let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); next_configuration .permission_profile_state @@ -352,12 +364,14 @@ impl SessionConfiguration { // Preserve richer split policies across cwd-only updates; only // rederive when the session is already using a structurally // cwd-bound legacy bridge. - let file_system_sandbox_policy = + let mut file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_preserving_deny_entries( ¤t_sandbox_policy, &next_configuration.cwd, ¤t_file_system_sandbox_policy, ); + next_configuration + .apply_managed_limited_git_write_override(&mut file_system_sandbox_policy); next_configuration .permission_profile_state .set_legacy_permission_profile( @@ -391,6 +405,7 @@ impl SessionConfiguration { file_system_sandbox_policy .preserve_deny_read_restrictions_from(existing_file_system_policy); } + self.apply_managed_limited_git_write_override(&mut file_system_sandbox_policy); let effective_permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( enforcement, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index d538400cc3..fa83eaa312 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -3085,6 +3085,7 @@ async fn set_rate_limits_retains_previous_credits() { approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, permission_profile_state: config.permissions.permission_profile_state().clone(), + managed_allow_limited_git_writes: None, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), workspace_roots: config.workspace_roots.clone(), @@ -3191,6 +3192,7 @@ async fn set_rate_limits_updates_plan_type_when_present() { approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, permission_profile_state: config.permissions.permission_profile_state().clone(), + managed_allow_limited_git_writes: None, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), workspace_roots: config.workspace_roots.clone(), @@ -3721,6 +3723,7 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, permission_profile_state: config.permissions.permission_profile_state().clone(), + managed_allow_limited_git_writes: None, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), workspace_roots: config.workspace_roots.clone(), @@ -3867,6 +3870,54 @@ async fn session_configuration_apply_permission_profile_preserves_existing_deny_ ); } +#[tokio::test] +async fn session_configuration_reapplies_managed_git_write_mode_on_profile_update() { + let mut session_configuration = make_session_configuration_for_tests().await; + let cwd = tempfile::tempdir().expect("create temp dir"); + session_configuration.cwd = cwd.path().abs(); + session_configuration.workspace_roots = vec![session_configuration.cwd.clone()]; + session_configuration.managed_allow_limited_git_writes = Some(true); + + let updated = session_configuration + .apply(&SessionSettingsUpdate { + permission_profile: Some(PermissionProfile::workspace_write()), + ..Default::default() + }) + .expect("permission profile update should honor managed filesystem setting"); + let policy = updated.file_system_sandbox_policy(); + + assert!(policy.can_write_path_with_cwd(&cwd.path().join(".git/HEAD"), cwd.path())); + assert!(!policy.can_write_path_with_cwd(&cwd.path().join(".git/config"), cwd.path())); + assert!(!policy.can_write_path_with_cwd(&cwd.path().join(".git/hooks/pre-commit"), cwd.path())); +} + +#[tokio::test] +async fn session_configuration_reapplies_managed_git_write_mode_on_sandbox_update() { + let mut session_configuration = make_session_configuration_for_tests().await; + let cwd = tempfile::tempdir().expect("create temp dir"); + session_configuration.cwd = cwd.path().abs(); + session_configuration.managed_allow_limited_git_writes = Some(false); + + let updated = session_configuration + .apply(&SessionSettingsUpdate { + sandbox_policy: Some(SandboxPolicy::WorkspaceWrite { + writable_roots: Vec::new(), + network_access: false, + allow_limited_git_writes: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }), + ..Default::default() + }) + .expect("sandbox policy update should honor managed filesystem setting"); + + assert!( + !updated + .file_system_sandbox_policy() + .can_write_path_with_cwd(&cwd.path().join(".git/HEAD"), cwd.path()) + ); +} + #[tokio::test] async fn session_configuration_apply_permission_profile_accepts_direct_write_roots() { let mut session_configuration = make_session_configuration_for_tests().await; @@ -4469,6 +4520,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() { approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, permission_profile_state: config.permissions.permission_profile_state().clone(), + managed_allow_limited_git_writes: None, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), workspace_roots: config.workspace_roots.clone(), @@ -4580,6 +4632,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, permission_profile_state: config.permissions.permission_profile_state().clone(), + managed_allow_limited_git_writes: None, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), workspace_roots: config.workspace_roots.clone(), @@ -4817,6 +4870,7 @@ async fn make_session_with_config_and_rx( approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, permission_profile_state: config.permissions.permission_profile_state().clone(), + managed_allow_limited_git_writes: None, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), workspace_roots: config.workspace_roots.clone(), @@ -4922,6 +4976,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, permission_profile_state: config.permissions.permission_profile_state().clone(), + managed_allow_limited_git_writes: None, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), workspace_roots: config.workspace_roots.clone(), @@ -6428,6 +6483,7 @@ where approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, permission_profile_state: config.permissions.permission_profile_state().clone(), + managed_allow_limited_git_writes: None, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), workspace_roots: config.workspace_roots.clone(), diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index ded838f84d..73cc7f458b 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -398,6 +398,72 @@ impl FileSystemSandboxPolicy { } } + /// Apply a managed Git metadata write mode while preserving protected + /// config and hook paths on an already compiled restricted policy. + pub fn apply_managed_limited_git_writes(&mut self, cwd: &Path) { + if !matches!(self.kind, FileSystemSandboxKind::Restricted) { + return; + } + + let writable_root_paths = dedup_absolute_paths( + self.resolved_entries_with_cwd(cwd) + .into_iter() + .filter(|entry| entry.access.can_write()) + .filter(|entry| self.can_write_path_with_cwd(entry.path.as_path(), cwd)) + .map(|entry| entry.path) + .collect(), + /*normalize_effective_paths*/ false, + ); + let linked_gitdirs = pointed_gitdirs_for_writable_roots(&writable_root_paths); + let protected_git_subpaths = dedup_absolute_paths( + writable_root_paths + .iter() + .flat_map(|root| managed_git_read_only_subpaths_for_writable_root(root, true)) + .chain( + linked_gitdirs + .iter() + .flat_map(protected_git_subpaths_when_writable), + ) + .collect(), + /*normalize_effective_paths*/ true, + ); + self.entries.retain(|entry| { + !entry.access.can_write() + || !is_managed_git_protected_write_entry(entry, cwd, &protected_git_subpaths) + }); + + self.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(Some(PathBuf::from( + PROTECTED_METADATA_GIT_PATH_NAME, + ))), + }, + access: FileSystemAccessMode::Write, + }); + for git_root in writable_root_paths + .iter() + .map(|root| root.join(PROTECTED_METADATA_GIT_PATH_NAME)) + .chain(linked_gitdirs) + { + self.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { path: git_root }, + access: FileSystemAccessMode::Write, + }); + } + + append_default_read_only_project_root_subpath_if_no_explicit_rule( + &mut self.entries, + ".git/config", + ); + append_default_read_only_project_root_subpath_if_no_explicit_rule( + &mut self.entries, + ".git/hooks", + ); + for git_subpath in protected_git_subpaths { + append_default_read_only_path_if_no_explicit_rule(&mut self.entries, git_subpath); + } + } + fn has_root_access(&self, predicate: impl Fn(FileSystemAccessMode) -> bool) -> bool { matches!(self.kind, FileSystemSandboxKind::Restricted) && self.entries.iter().any(|entry| { @@ -1607,6 +1673,29 @@ pub(crate) fn default_read_only_subpaths_for_writable_root( writable_root: &AbsolutePathBuf, protect_missing_dot_codex: bool, allow_limited_git_writes: bool, +) -> Vec { + let mut subpaths = + default_git_read_only_subpaths_for_writable_root(writable_root, allow_limited_git_writes); + let top_level_agents = writable_root.join(PROTECTED_METADATA_AGENTS_PATH_NAME); + 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. + let top_level_codex = writable_root.join(PROTECTED_METADATA_CODEX_PATH_NAME); + 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 default_git_read_only_subpaths_for_writable_root( + writable_root: &AbsolutePathBuf, + allow_limited_git_writes: bool, ) -> Vec { let mut subpaths: Vec = Vec::new(); let top_level_git = writable_root.join(PROTECTED_METADATA_GIT_PATH_NAME); @@ -1636,23 +1725,30 @@ pub(crate) fn default_read_only_subpaths_for_writable_root( } } - let top_level_agents = writable_root.join(PROTECTED_METADATA_AGENTS_PATH_NAME); - 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. - let top_level_codex = writable_root.join(PROTECTED_METADATA_CODEX_PATH_NAME); - 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 managed_git_read_only_subpaths_for_writable_root( + writable_root: &AbsolutePathBuf, + allow_limited_git_writes: bool, +) -> Vec { + let mut subpaths = + default_git_read_only_subpaths_for_writable_root(writable_root, allow_limited_git_writes); + if !allow_limited_git_writes { + subpaths.push(writable_root.join(PROTECTED_METADATA_GIT_PATH_NAME)); + } + dedup_absolute_paths(subpaths, /*normalize_effective_paths*/ false) +} + +fn pointed_gitdirs_for_writable_roots(writable_roots: &[AbsolutePathBuf]) -> Vec { + let gitdirs = writable_roots + .iter() + .filter_map(|writable_root| { + resolve_gitdir_from_file(&writable_root.join(PROTECTED_METADATA_GIT_PATH_NAME)) + }) + .collect(); + dedup_absolute_paths(gitdirs, /*normalize_effective_paths*/ true) +} /// Rebuilds the filesystem policy that legacy sandbox runtimes enforce for a /// concrete cwd. /// @@ -1781,6 +1877,89 @@ fn append_default_read_only_entry_if_no_explicit_rule( }); } +fn git_root_protection_children(path: &FileSystemPath) -> Option<[FileSystemPath; 2]> { + match path { + FileSystemPath::Special { + value: + FileSystemSpecialPath::ProjectRoots { + subpath: Some(subpath), + }, + } if subpath == Path::new(PROTECTED_METADATA_GIT_PATH_NAME) => Some([ + FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(Some(subpath.join("config"))), + }, + FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(Some(subpath.join("hooks"))), + }, + ]), + FileSystemPath::Path { .. } + | FileSystemPath::GlobPattern { .. } + | FileSystemPath::Special { .. } => None, + } +} + +fn git_child_protection_root(path: &FileSystemPath) -> Option { + match path { + FileSystemPath::Special { + value: + FileSystemSpecialPath::ProjectRoots { + subpath: Some(subpath), + }, + } if matches!( + subpath.as_path(), + path if path == Path::new(".git/config") || path == Path::new(".git/hooks") + ) => + { + Some(FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(Some(PathBuf::from( + PROTECTED_METADATA_GIT_PATH_NAME, + ))), + }) + } + FileSystemPath::Path { .. } + | FileSystemPath::GlobPattern { .. } + | FileSystemPath::Special { .. } => None, + } +} + +fn is_managed_git_protected_write_entry( + entry: &FileSystemSandboxEntry, + cwd: &Path, + protected_paths: &[AbsolutePathBuf], +) -> bool { + match &entry.path { + FileSystemPath::Path { path } => { + let path = normalize_effective_absolute_path(path.clone()); + protected_paths.iter().any(|protected_path| { + path.as_path().starts_with( + normalize_effective_absolute_path(protected_path.clone()).as_path(), + ) + }) + } + FileSystemPath::Special { + value: + FileSystemSpecialPath::ProjectRoots { + subpath: Some(subpath), + }, + } => [".git/config", ".git/hooks"] + .iter() + .any(|protected_subpath| subpath.starts_with(Path::new(protected_subpath))), + FileSystemPath::Special { .. } => resolve_file_system_path( + &entry.path, + AbsolutePathBuf::from_absolute_path(cwd).ok().as_ref(), + ) + .is_some_and(|path| { + let path = normalize_effective_absolute_path(path); + protected_paths.iter().any(|protected_path| { + path.as_path().starts_with( + normalize_effective_absolute_path(protected_path.clone()).as_path(), + ) + }) + }), + FileSystemPath::GlobPattern { .. } => false, + } +} + fn has_explicit_resolved_path_entry( entries: &[ResolvedFileSystemEntry], path: &AbsolutePathBuf, @@ -2234,6 +2413,160 @@ mod tests { assert!(!writable_roots[0].is_path_writable(&dot_codex_config)); } + #[test] + fn managed_limited_git_writes_replaces_default_workspace_git_protection() { + let cwd = TempDir::new().expect("tempdir"); + let dot_git_head = cwd.path().join(".git").join("HEAD"); + let dot_git_config = cwd.path().join(".git").join("config"); + let dot_git_hook = cwd.path().join(".git").join("hooks").join("pre-commit"); + let mut policy = FileSystemSandboxPolicy::workspace_write( + &[], + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ); + + policy.apply_managed_limited_git_writes(cwd.path()); + + assert!(policy.can_write_path_with_cwd(&dot_git_head, cwd.path())); + assert!(!policy.can_write_path_with_cwd(&dot_git_config, cwd.path())); + assert!(!policy.can_write_path_with_cwd(&dot_git_hook, cwd.path())); + } + + #[test] + fn managed_limited_git_writes_replaces_resolved_gitdir_protection() { + let temp = TempDir::new().expect("tempdir"); + let cwd = temp.path().join("worktree"); + let gitdir_path = temp.path().join("repo/.git/worktrees/feature"); + std::fs::create_dir_all(&cwd).expect("create worktree"); + std::fs::create_dir_all(gitdir_path.join("hooks")).expect("create gitdir"); + std::fs::write( + cwd.join(".git"), + format!("gitdir: {}\n", gitdir_path.display()), + ) + .expect("write gitdir pointer"); + let gitdir = normalize_effective_absolute_path( + AbsolutePathBuf::from_absolute_path(gitdir_path).expect("absolute gitdir"), + ); + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: Vec::new(), + network_access: false, + allow_limited_git_writes: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + let mut policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&sandbox_policy, &cwd); + + policy.apply_managed_limited_git_writes(&cwd); + + assert!( + policy.can_write_path_with_cwd(gitdir.join("HEAD").as_path(), &cwd), + "resolved gitdir metadata should be writable in limited mode: {policy:#?}" + ); + assert!(!policy.can_write_path_with_cwd(gitdir.join("config").as_path(), &cwd)); + assert!(!policy.can_write_path_with_cwd(gitdir.join("hooks/pre-commit").as_path(), &cwd)); + } + + #[test] + fn managed_limited_git_writes_keeps_explicit_nested_git_read_only() { + let cwd = TempDir::new().expect("tempdir"); + let explicit_git = AbsolutePathBuf::from_absolute_path(cwd.path().join("vendor/.git")) + .expect("absolute nested git path"); + let mut policy = FileSystemSandboxPolicy::workspace_write( + &[], + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ); + policy.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: explicit_git.clone(), + }, + access: FileSystemAccessMode::Read, + }); + + policy.apply_managed_limited_git_writes(cwd.path()); + + assert!(!policy.can_write_path_with_cwd(explicit_git.join("HEAD").as_path(), cwd.path())); + } + + #[test] + fn managed_limited_git_writes_override_conflicting_profile_writes() { + let cwd = TempDir::new().expect("tempdir"); + let mut policy = FileSystemSandboxPolicy::workspace_write( + &[], + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ); + policy.entries.extend([ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(Some(PathBuf::from(".git/config"))), + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(Some(PathBuf::from( + ".git/hooks/pre-commit", + ))), + }, + access: FileSystemAccessMode::Write, + }, + ]); + + policy.apply_managed_limited_git_writes(cwd.path()); + + assert!(policy.can_write_path_with_cwd(&cwd.path().join(".git/HEAD"), cwd.path())); + assert!(!policy.can_write_path_with_cwd(&cwd.path().join(".git/config"), cwd.path())); + assert!( + !policy.can_write_path_with_cwd(&cwd.path().join(".git/hooks/pre-commit"), cwd.path()) + ); + + policy.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::from_absolute_path(cwd.path().join(".git/HEAD")) + .expect("absolute git HEAD"), + }, + access: FileSystemAccessMode::Write, + }); + policy.apply_managed_limited_git_writes(cwd.path()); + + assert!(policy.can_write_path_with_cwd(&cwd.path().join(".git/HEAD"), cwd.path())); + } + + #[test] + fn limited_git_writes_export_linked_worktree_gitdir_root() { + let temp = TempDir::new().expect("tempdir"); + let cwd = temp.path().join("worktree"); + let gitdir_path = + AbsolutePathBuf::from_absolute_path(temp.path().join("repo/.git/worktrees/feature")) + .expect("absolute gitdir"); + std::fs::create_dir_all(&cwd).expect("create worktree"); + std::fs::create_dir_all(gitdir_path.join("hooks").as_path()).expect("create gitdir"); + std::fs::write( + cwd.join(".git"), + format!("gitdir: {}\n", gitdir_path.as_path().display()), + ) + .expect("write gitdir pointer"); + let gitdir = normalize_effective_absolute_path(gitdir_path); + let mut policy = FileSystemSandboxPolicy::workspace_write( + &[], + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ); + policy.apply_managed_limited_git_writes(&cwd); + + let gitdir_root = policy + .get_writable_roots_with_cwd(&cwd) + .into_iter() + .find(|root| root.root == gitdir) + .expect("linked gitdir should be an exported writable root"); + + assert!(gitdir_root.is_path_writable(gitdir.join("HEAD").as_path())); + assert!(!gitdir_root.is_path_writable(gitdir.join("config").as_path())); + assert!(!gitdir_root.is_path_writable(gitdir.join("hooks/pre-commit").as_path())); + } + #[test] fn legacy_workspace_write_projection_accepts_relative_cwd() { let relative_cwd = Path::new("workspace"); diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index c58a2f23a4..50f5b4ad8d 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -83,6 +83,7 @@ pub use crate::permissions::FileSystemSandboxKind; pub use crate::permissions::FileSystemSandboxPolicy; pub use crate::permissions::FileSystemSpecialPath; pub use crate::permissions::NetworkSandboxPolicy; +use crate::permissions::add_limited_git_writable_roots; use crate::permissions::default_read_only_subpaths_for_writable_root; pub use crate::request_permissions::RequestPermissionsArgs; pub use crate::request_user_input::RequestUserInputEvent; @@ -1128,7 +1129,7 @@ impl SandboxPolicy { // For each root, compute subpaths that should remain read-only. let cwd_root = AbsolutePathBuf::from_absolute_path(cwd).ok(); - roots + let mut roots = roots .into_iter() .map(|writable_root| { let protect_missing_dot_codex = cwd_root @@ -1144,7 +1145,11 @@ impl SandboxPolicy { root: writable_root, } }) - .collect() + .collect::>(); + if *allow_limited_git_writes { + add_limited_git_writable_roots(&mut roots); + } + roots } } } @@ -4431,6 +4436,52 @@ mod tests { ); } + #[test] + fn workspace_write_limited_git_writes_export_linked_gitdir_root() { + let temp = TempDir::new().expect("tempdir"); + let cwd = temp.path().join("worktree"); + let gitdir_path = + AbsolutePathBuf::from_absolute_path(temp.path().join("repo/.git/worktrees/feature")) + .expect("absolute gitdir"); + std::fs::create_dir_all(&cwd).expect("create worktree"); + std::fs::create_dir_all(gitdir_path.join("hooks").as_path()).expect("create gitdir"); + std::fs::write( + cwd.join(".git"), + format!("gitdir: {}\n", gitdir_path.as_path().display()), + ) + .expect("write gitdir pointer"); + let gitdir = AbsolutePathBuf::from_absolute_path( + gitdir_path + .as_path() + .canonicalize() + .expect("canonicalize gitdir"), + ) + .expect("absolute canonical gitdir"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: Vec::new(), + network_access: false, + allow_limited_git_writes: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + assert!(sandbox_policy_allows_write( + &policy, + gitdir.join("HEAD").as_path(), + &cwd, + )); + assert!(!sandbox_policy_allows_write( + &policy, + gitdir.join("config").as_path(), + &cwd, + )); + assert!(!sandbox_policy_allows_write( + &policy, + gitdir.join("hooks/pre-commit").as_path(), + &cwd, + )); + } + #[test] fn restricted_file_system_policy_treats_read_entries_as_read_only_subpaths() { let cwd = TempDir::new().expect("tempdir"); diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index c9d91eed55..fff9e5cf7a 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -242,6 +242,13 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec> { } if let Some(filesystem) = requirements.filesystem.as_ref() { + if let Some(allow_limited_git_writes) = filesystem.value.allow_limited_git_writes { + requirement_lines.push(requirement_line( + "permissions.filesystem.allow_limited_git_writes", + allow_limited_git_writes.to_string(), + Some(&filesystem.source), + )); + } let deny_read = join_or_empty( filesystem .value @@ -689,6 +696,7 @@ mod tests { filesystem: Some(Sourced::new( FilesystemConstraints { deny_read: vec![denied_path.clone().into()], + allow_limited_git_writes: Some(true), }, RequirementSource::SystemRequirementsToml { file: requirements_file.clone(), @@ -780,6 +788,15 @@ mod tests { assert!(rendered.contains( "experimental_network: enabled=true, domains={example.com=allow} (source: cloud requirements)" )); + assert!( + rendered.contains( + format!( + "permissions.filesystem.allow_limited_git_writes: true (source: {})", + requirements_file.as_path().display() + ) + .as_str(), + ) + ); assert!( rendered.contains( format!(