diff --git a/codex-rs/core/src/session/environment.rs b/codex-rs/core/src/session/environment.rs index e4469afbc9..c096d3ae13 100644 --- a/codex-rs/core/src/session/environment.rs +++ b/codex-rs/core/src/session/environment.rs @@ -10,6 +10,10 @@ use codex_features::Feature; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::error::CodexErr; use codex_protocol::error::Result as CodexResult; +use codex_protocol::models::PermissionProfile; +use codex_protocol::models::SandboxEnforcement; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AgentStatus; use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::EnvironmentConfigState; @@ -183,7 +187,52 @@ impl Session { ensure_configs_stay_owner_provided(current_environments, &environments.environments)?; } - current.apply(updates, current_environments) + if updates.permission_profile.is_none() && updates.sandbox_policy.is_none() { + return current.apply(updates, current_environments); + } + + // Normalize a copy so preview leaves compatibility input available for admission. + let mut updates = updates.clone(); + let sandbox_policy = updates.sandbox_policy.take(); + let current_file_system = current.file_system_sandbox_policy(current_environments); + if let Some(profile) = updates.permission_profile.take() { + // Infer identity from the requested profile before merging current denials. + updates.active_permission_profile = updates.active_permission_profile.or_else(|| { + (profile == current.permission_profile()) + .then(|| current.active_permission_profile()) + .flatten() + }); + let (mut file_system, network) = profile.to_runtime_permissions(); + file_system.preserve_deny_read_restrictions_from(¤t_file_system); + updates.permission_profile = Some( + PermissionProfile::from_runtime_permissions_with_enforcement( + profile.enforcement(), + &file_system, + network, + ), + ); + } else if let Some(policy) = sandbox_policy { + let cwd = updates.environments.as_ref().map_or_else( + || current.cwd(), + |environments| &environments.legacy_fallback_cwd, + ); + let file_system = + FileSystemSandboxPolicy::from_legacy_sandbox_policy_preserving_deny_entries( + &policy, + cwd, + ¤t_file_system, + ); + updates.permission_profile = Some( + PermissionProfile::from_runtime_permissions_with_enforcement( + SandboxEnforcement::from_legacy_sandbox_policy(&policy), + &file_system, + NetworkSandboxPolicy::from(&policy), + ), + ); + updates.active_permission_profile = None; + updates.profile_workspace_roots = Some(Vec::new()); + } + current.apply(&updates, current_environments) } /// Activates the environments accepted for a new task. Configuration may have arrived for diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 6ab58bc676..933dc85a1c 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -433,6 +433,7 @@ impl SessionConfiguration { } } + /// Applies fields normalized against this configuration at admission. pub(super) fn apply( &self, updates: &SessionSettingsUpdate, @@ -481,20 +482,17 @@ impl SessionConfiguration { } if let Some(permission_profile) = updates.permission_profile.clone() { - let active_permission_profile = - updates.active_permission_profile.clone().or_else(|| { - if permission_profile == self.permission_profile() { - self.active_permission_profile() - } else { - None - } - }); - next_configuration.set_permission_profile_projection( - permission_profile, - active_permission_profile, - updates.profile_workspace_roots.clone().unwrap_or_default(), - Some(¤t_file_system_sandbox_policy), - )?; + let snapshot = match updates.active_permission_profile.clone() { + Some(active) => PermissionProfileSnapshot::active_with_profile_workspace_roots( + permission_profile, + active, + updates.profile_workspace_roots.clone().unwrap_or_default(), + ), + None => PermissionProfileSnapshot::legacy(permission_profile), + }; + next_configuration + .permission_profile_state + .set_permission_profile_snapshot(snapshot)?; if let Some(active_permission_profile) = next_configuration.active_permission_profile() { let mut config = (*next_configuration.original_config_do_not_use).clone(); @@ -522,23 +520,6 @@ 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 = - FileSystemSandboxPolicy::from_legacy_sandbox_policy_preserving_deny_entries( - &sandbox_policy, - next_configuration.cwd(), - ¤t_file_system_sandbox_policy, - ); - let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); - next_configuration - .permission_profile_state - .set_legacy_permission_profile( - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - network_sandbox_policy, - ), - )?; } else if cwd_changed && file_system_policy_has_rebindable_project_root_write { // Compatibility projection can resolve filesystem paths. Only compute it // when a cwd-bound legacy policy might need rebinding. @@ -624,42 +605,6 @@ impl SessionConfiguration { } Ok(next_configuration) } - - fn set_permission_profile_projection( - &mut self, - permission_profile: PermissionProfile, - active_permission_profile: Option, - profile_workspace_roots: Vec, - preserve_deny_reads_from: Option<&FileSystemSandboxPolicy>, - ) -> ConstraintResult<()> { - let enforcement = permission_profile.enforcement(); - let (mut file_system_sandbox_policy, network_sandbox_policy) = - permission_profile.to_runtime_permissions(); - if let Some(existing_file_system_policy) = preserve_deny_reads_from { - file_system_sandbox_policy - .preserve_deny_read_restrictions_from(existing_file_system_policy); - } - let effective_permission_profile = - PermissionProfile::from_runtime_permissions_with_enforcement( - enforcement, - &file_system_sandbox_policy, - network_sandbox_policy, - ); - - let permission_snapshot = match active_permission_profile { - Some(active_permission_profile) => { - PermissionProfileSnapshot::active_with_profile_workspace_roots( - effective_permission_profile, - active_permission_profile, - profile_workspace_roots, - ) - } - None => PermissionProfileSnapshot::legacy(effective_permission_profile), - }; - - self.permission_profile_state - .set_permission_profile_snapshot(permission_snapshot) - } } /// The configuration and public snapshot published by one settings commit. @@ -679,6 +624,7 @@ pub(crate) struct SessionSettingsUpdate { pub(crate) environments: Option, pub(crate) runtime_workspace_roots: Option>, pub(crate) profile_workspace_roots: Option>, + /// Compatibility input consumed when normalizing settings at admission. pub(crate) sandbox_policy: Option, pub(crate) permission_profile: Option, pub(crate) active_permission_profile: Option, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index fd53a60943..0b3313840c 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5538,11 +5538,19 @@ async fn session_configuration_apply_preserves_profile_file_system_policy_on_cwd #[tokio::test] async fn session_configuration_apply_permission_profile_preserves_existing_deny_read_entries() { - let mut session_configuration = make_session_configuration_for_tests().await; - let cwd = tempfile::tempdir().expect("create temp dir"); - session_configuration.legacy_fallback_cwd = cwd.path().abs(); + const GLOB_SCAN_MAX_DEPTH: usize = 2; - let workspace_policy = SandboxPolicy::new_workspace_write_policy(); + let (session, _turn_context) = make_session_and_context().await; + let mut config = session.state.lock().await.session_configuration.clone(); + let cwd = tempfile::tempdir().expect("create temp dir"); + config.legacy_fallback_cwd = cwd.path().abs(); + + let workspace_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![config.cwd().clone()], + network_access: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + }; let deny_entry = FileSystemSandboxEntry { path: FileSystemPath::GlobPattern { pattern: "**/*.env".to_string(), @@ -5550,48 +5558,136 @@ async fn session_configuration_apply_permission_profile_preserves_existing_deny_ access: FileSystemAccessMode::Deny, missing_path_behavior: None, }; - let mut existing_file_system_policy = - FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( - &workspace_policy, - session_configuration.cwd().as_path(), - ); - existing_file_system_policy.glob_scan_max_depth = Some(2); - existing_file_system_policy.entries.push(deny_entry.clone()); - session_configuration - .set_permission_profile_for_tests( - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&workspace_policy), - &existing_file_system_policy, - NetworkSandboxPolicy::Restricted, - ), - ) - .expect("set permission profile"); - - let requested_file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( + let requested = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( &workspace_policy, - session_configuration.cwd().as_path(), + config.cwd().as_path(), ); - let permission_profile = codex_protocol::models::PermissionProfile::from_runtime_permissions( - &requested_file_system_policy, - NetworkSandboxPolicy::Restricted, - ); - let updated = session_configuration - .apply( + let mut existing = requested.clone(); + existing.glob_scan_max_depth = Some(GLOB_SCAN_MAX_DEPTH); + existing.entries.push(deny_entry.clone()); + config + .set_permission_profile_for_tests(PermissionProfile::from_runtime_permissions( + &existing, + NetworkSandboxPolicy::Restricted, + )) + .unwrap(); + + let permission_profile = + PermissionProfile::from_runtime_permissions(&requested, NetworkSandboxPolicy::Restricted); + let mut expected = requested.materialize_project_roots_with_workspace_roots(&[]); + expected.glob_scan_max_depth = Some(GLOB_SCAN_MAX_DEPTH); + expected.entries.push(deny_entry); + for updates in [ + SessionSettingsUpdate { + permission_profile: Some(permission_profile), + ..Default::default() + }, + SessionSettingsUpdate { + sandbox_policy: Some(workspace_policy.clone()), + ..Default::default() + }, + ] { + let updated = session.apply_session_settings(&config, &updates).unwrap(); + assert_eq!(updated.file_system_sandbox_policy(&[]), expected); + } + let next_cwd = tempfile::tempdir().unwrap(); + let git_dir = cwd.path().join("git-metadata"); + std::fs::create_dir(&git_dir).unwrap(); + std::fs::write( + next_cwd.path().join(".git"), + format!("gitdir: {}\n", git_dir.display()), + ) + .unwrap(); + let updated = session + .apply_session_settings( + &config, &SessionSettingsUpdate { - permission_profile: Some(permission_profile), + environments: Some(TurnEnvironmentSelections::new( + next_cwd.path().abs(), + Vec::new(), + )), + sandbox_policy: Some(workspace_policy), ..Default::default() }, - &[], ) - .expect("permission profile update should succeed"); - - let mut expected_file_system_policy = - requested_file_system_policy.materialize_project_roots_with_workspace_roots(&[]); - expected_file_system_policy.glob_scan_max_depth = Some(2); - expected_file_system_policy.entries.push(deny_entry); + .unwrap(); + let filesystem = updated.file_system_sandbox_policy(&[]); assert_eq!( - updated.file_system_sandbox_policy(&[]), - expected_file_system_policy + ( + updated.cwd(), + filesystem.can_write_local_path_with_cwd(&git_dir, next_cwd.path()), + filesystem + .can_write_local_path_with_cwd(&cwd.path().join("ordinary-file"), next_cwd.path()), + ), + (&next_cwd.path().abs(), false, true) + ); +} + +#[tokio::test] +async fn session_settings_preview_preserves_legacy_input_and_identity_clearing() { + let (session, context) = make_session_and_context().await; + let profile = PermissionProfile::read_only(); + let selected = PermissionProfileSnapshot::active_with_profile_workspace_roots( + profile.clone(), + ActivePermissionProfile::read_only(), + vec![ + context + .initial_environments + .primary() + .unwrap() + .cwd() + .clone() + .into(), + ], + ); + session + .state + .lock() + .await + .session_configuration + .permission_profile_state + .set_permission_profile_snapshot(selected.clone()) + .unwrap(); + let canonical = session + .update_settings(SessionSettingsUpdate { + permission_profile: Some(profile.clone()), + profile_workspace_roots: Some(selected.profile_workspace_roots().to_vec()), + ..Default::default() + }) + .await + .unwrap(); + assert_eq!( + canonical.configuration.permission_profile_state.snapshot(), + selected + ); + + let legacy = SessionSettingsUpdate { + sandbox_policy: Some(SandboxPolicy::new_read_only_policy()), + ..Default::default() + }; + let preview = session.preview_settings(&legacy).await.unwrap(); + assert_eq!( + ( + preview.permission_profile, + preview.active_permission_profile, + preview.profile_workspace_roots + ), + (profile.clone(), None, Vec::new()), + ); + assert_eq!( + session + .state + .lock() + .await + .session_configuration + .permission_profile_state + .snapshot(), + selected + ); + let committed = session.update_settings(legacy).await.unwrap(); + assert_eq!( + committed.configuration.permission_profile_state.snapshot(), + PermissionProfileSnapshot::legacy(profile), ); }