diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index 1703978f0e..f5dd78977f 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -5,7 +5,6 @@ use crate::context::ContextualUserFragment; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::environment_selection::TurnEnvironmentState; use crate::session::turn_context::TurnEnvironment; -use crate::session::turn_context::TurnEnvironmentConfig; use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerStack; use codex_config::ConfigRequirements; @@ -23,6 +22,7 @@ use codex_exec_server::RemoveOptions; use codex_extension_api::UserInstructions; use codex_features::Feature; use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::EnvironmentConfigState; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_utils_absolute_path::AbsolutePathBuf; @@ -345,12 +345,12 @@ fn resolved_local_environments( .expect("local environment"), ), /*shell*/ None, - TurnEnvironmentConfig { + EnvironmentConfig { allow_login_shell: true, permission_profile: PermissionProfileSnapshot::legacy( PermissionProfile::read_only(), ), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }, )) }) diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index bbf2e428a4..d256bd03f4 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -31,7 +31,6 @@ use tokio_util::task::AbortOnDropHandle; use crate::session::turn_context::ShellSnapshotTask; use crate::session::turn_context::TurnEnvironment; -use crate::session::turn_context::TurnEnvironmentConfig; use crate::shell::Shell; use crate::shell_snapshot::ShellSnapshot; @@ -67,7 +66,7 @@ struct ResolvedEnvironment { #[derive(Clone)] struct SelectedTurnEnvironment { selection: TurnEnvironmentSelection, - config: TurnEnvironmentConfig, + config: EnvironmentConfig, environment: Arc, // Selection clones share one listener; the final handle drop aborts it. connection_events_task: Option>>, @@ -77,7 +76,7 @@ struct SelectedTurnEnvironment { #[derive(Clone)] pub(crate) struct StartingTurnEnvironment { pub(crate) selection: TurnEnvironmentSelection, - config: TurnEnvironmentConfig, + config: EnvironmentConfig, resolution: TurnEnvironmentResolution, } @@ -85,17 +84,11 @@ impl SelectedTurnEnvironment { fn apply_configuration( &mut self, selection_config: EnvironmentConfigState, - thread_config: &TurnEnvironmentConfig, + thread_config: &EnvironmentConfig, ) { self.config = match &selection_config { EnvironmentConfigState::FromThread => thread_config.clone(), - EnvironmentConfigState::Ready(config) => TurnEnvironmentConfig { - allow_login_shell: config.allow_login_shell, - // temp read from thread_config; will go away once perms on EnvironmentConfig, - // then we can just assign passed config directly - permission_profile: thread_config.permission_profile.clone(), - selected_capability_roots: Some(config.selected_capability_roots.clone()), - }, + EnvironmentConfigState::Ready(config) => config.clone(), EnvironmentConfigState::Pending => { unreachable!("pending environment configuration is not supported yet") } @@ -133,7 +126,7 @@ impl ThreadEnvironments { pub(crate) fn new( environment_manager: Arc, local_shell: Shell, - thread_environment_config: TurnEnvironmentConfig, + thread_environment_config: EnvironmentConfig, shell_snapshot: ShellSnapshot, current: TurnEnvironmentSnapshot, non_blocking_snapshots: bool, @@ -186,7 +179,7 @@ impl ThreadEnvironments { pub(crate) fn update_selections( &self, environments: &[TurnEnvironmentSelection], - thread_environment_config: &TurnEnvironmentConfig, + thread_environment_config: &EnvironmentConfig, ) { let previous = self.environments.load(); let mut seen_environment_ids = HashSet::with_capacity(environments.len()); @@ -306,7 +299,7 @@ impl ThreadEnvironments { .unwrap_or_default() } - pub(crate) fn update_environment_configs(&self, config: &TurnEnvironmentConfig) { + pub(crate) fn update_environment_configs(&self, config: &EnvironmentConfig) { let environments = self .environments .load() @@ -353,9 +346,8 @@ impl ThreadEnvironments { let environments = self.environments.load(); let mut selected_capability_roots = thread_selected_capability_roots.to_vec(); for environment in environments.iter() { - if let Some(roots) = &environment.config.selected_capability_roots { - selected_capability_roots.extend(roots.iter().cloned()); - } + selected_capability_roots + .extend(environment.config.selected_capability_roots.iter().cloned()); } let mut seen_root_ids = HashSet::with_capacity(selected_capability_roots.len()); selected_capability_roots.retain(|root| seen_root_ids.insert(root.id.clone())); @@ -706,11 +698,11 @@ mod tests { use super::*; - fn test_environment_config() -> TurnEnvironmentConfig { - TurnEnvironmentConfig { + fn test_environment_config() -> EnvironmentConfig { + EnvironmentConfig { allow_login_shell: true, permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), } } @@ -874,14 +866,14 @@ url = "ws://127.0.0.1:8765" shell_type: crate::shell::ShellType::Zsh, shell_path: std::path::PathBuf::from("/configured/zsh"), }; - let expected_config = TurnEnvironmentConfig { + let expected_config = EnvironmentConfig { allow_login_shell: false, permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots( PermissionProfile::read_only(), ActivePermissionProfile::read_only(), vec![cwd.join("profile-root")], ), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }; let turn_environments = ThreadEnvironments::new( Arc::new(EnvironmentManager::default_for_tests()), @@ -1075,14 +1067,14 @@ url = "ws://127.0.0.1:8765" .await, ); let cwd = AbsolutePathBuf::current_dir().expect("cwd"); - let expected_config = TurnEnvironmentConfig { + let expected_config = EnvironmentConfig { allow_login_shell: false, permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots( PermissionProfile::read_only(), ActivePermissionProfile::read_only(), vec![cwd.join("profile-root")], ), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }; let cwd = PathUri::from_abs_path(&cwd); let remote = TurnEnvironmentSelection { @@ -1244,7 +1236,7 @@ url = "ws://127.0.0.1:8765" /*connect_timeout*/ None, ) .expect("replacement environment"); - let next_config = TurnEnvironmentConfig { + let next_config = EnvironmentConfig { allow_login_shell: false, ..test_environment_config() }; @@ -1394,14 +1386,14 @@ url = "ws://127.0.0.1:8765" /*connect_timeout*/ None, ) .expect("replacement environment"); - let child_config = TurnEnvironmentConfig { + let child_config = EnvironmentConfig { allow_login_shell: false, permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots( PermissionProfile::read_only(), ActivePermissionProfile::read_only(), vec![cwd.join("child-profile-root")], ), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }; let environments = ThreadEnvironments::new( manager, @@ -1423,7 +1415,7 @@ url = "ws://127.0.0.1:8765" } #[tokio::test] - async fn installed_environment_config_is_inherited_and_reset_for_new_cwd() { + async fn owner_environment_config_is_inherited_and_reset_for_new_cwd() { let cwd = AbsolutePathBuf::current_dir().expect("cwd"); let selection = TurnEnvironmentSelection { environment_id: LOCAL_ENVIRONMENT_ID.to_string(), @@ -1441,17 +1433,16 @@ url = "ws://127.0.0.1:8765" let manager = Arc::new(EnvironmentManager::default_for_tests()); let parent = resolve_turn_environments(Arc::clone(&manager), std::slice::from_ref(&selection)).await; + let parent_owner_config = EnvironmentConfig { + allow_login_shell: false, + permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()), + selected_capability_roots: vec![root("parent-root")], + }; parent - .environment_ready( - &selection, - EnvironmentConfig { - allow_login_shell: false, - selected_capability_roots: vec![root("parent-root")], - }, - ) + .environment_ready(&selection, parent_owner_config.clone()) .expect("install environment config"); - let child_thread_config = TurnEnvironmentConfig { + let child_thread_config = EnvironmentConfig { permission_profile: PermissionProfileSnapshot::legacy( PermissionProfile::workspace_write(), ), @@ -1468,14 +1459,12 @@ url = "ws://127.0.0.1:8765" let child_snapshot = child.snapshot().await; let child_environment = child_snapshot.primary().expect("child environment"); + let cleared_parent_owner_config = EnvironmentConfig { + selected_capability_roots: Vec::new(), + ..parent_owner_config.clone() + }; parent - .environment_ready( - &selection, - EnvironmentConfig { - allow_login_shell: false, - selected_capability_roots: Vec::new(), - }, - ) + .environment_ready(&selection, cleared_parent_owner_config.clone()) .expect("clear parent roots"); let cleared_snapshot = parent.snapshot().await; assert_eq!( @@ -1483,20 +1472,9 @@ url = "ws://127.0.0.1:8765" .primary() .expect("environment with cleared roots") .config, - TurnEnvironmentConfig { - allow_login_shell: false, - selected_capability_roots: Some(Vec::new()), - ..test_environment_config() - } - ); - assert_eq!( - child_environment.config, - TurnEnvironmentConfig { - allow_login_shell: false, - selected_capability_roots: Some(vec![root("parent-root")]), - ..child_thread_config - } + cleared_parent_owner_config ); + assert_eq!(child_environment.config, parent_owner_config); let changed_selection = TurnEnvironmentSelection { cwd: PathUri::from_abs_path(&cwd.join("changed")), diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 381ba8f81a..9d548e5867 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -21,6 +21,7 @@ use codex_protocol::models::BaseInstructionsProvenance; use codex_protocol::models::ContentItem; use codex_protocol::models::ImageDetail; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::PermissionProfileSnapshot; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::InputModality; @@ -28,6 +29,7 @@ use codex_protocol::openai_models::ModelMessages; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::CodexErrorInfo; +use codex_protocol::protocol::EnvironmentConfigState; use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; @@ -1050,8 +1052,23 @@ async fn run_review_on_session( .total_token_usage() .await .unwrap_or_default(); - let guardian_permission_profile = params.spawn_config.permissions.permission_profile().clone(); - let parent_turn_environments = params.parent_context.environments().to_selections(); + let guardian_permission_snapshot = params + .spawn_config + .permissions + .permission_profile_state() + .snapshot(); + let mut parent_turn_environments = params.parent_context.environments().to_selections(); + // FromThread inherits the Guardian profile below. Once Core normalizes FromThread + // to Ready(config), every selection will carry explicit config and this conditional can be + // removed. + for selection in &mut parent_turn_environments { + if let EnvironmentConfigState::Ready(config) = &mut selection.config { + config.permission_profile = + PermissionProfileSnapshot::legacy(read_only_guardian_permission_profile( + config.permission_profile.permission_profile(), + )); + } + } // TODO(anp): Migrate guardian review thread settings to a PathUri fallback cwd so foreign // parent environments do not fall back to the host-native config cwd. let parent_turn_legacy_fallback_cwd = params @@ -1071,7 +1088,7 @@ async fn run_review_on_session( )), approval_policy: Some(AskForApproval::Never), sandbox_policy: None, - permission_profile: Some(guardian_permission_profile), + permission_profile: Some(guardian_permission_snapshot.permission_profile().clone()), summary: Some(params.reasoning_summary), personality: params.personality, collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { @@ -1275,6 +1292,16 @@ fn event_matches_turn(event: &Event, expected_turn_id: &str) -> bool { } } +fn read_only_guardian_permission_profile( + permission_profile: &PermissionProfile, +) -> PermissionProfile { + permission_profile + .intersect_with_read_only() + .unwrap_or(PermissionProfile::External { + network: codex_protocol::permissions::NetworkSandboxPolicy::Restricted, + }) +} + pub(crate) fn build_guardian_review_session_config( parent_config: &Config, live_network_config: Option, @@ -1303,13 +1330,8 @@ pub(crate) fn build_guardian_review_session_config( guardian_config.notify = None; guardian_config.developer_instructions = None; guardian_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never); - let guardian_permission_profile = parent_config - .permissions - .permission_profile() - .intersect_with_read_only() - .unwrap_or(PermissionProfile::External { - network: codex_protocol::permissions::NetworkSandboxPolicy::Restricted, - }); + let guardian_permission_profile = + read_only_guardian_permission_profile(parent_config.permissions.permission_profile()); guardian_config .permissions .set_permission_profile(guardian_permission_profile) diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 55bcf165a7..7195ebb7b2 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -7,7 +7,6 @@ use crate::session::tests::make_session_and_context; use crate::session::tests::make_session_and_context_with_rx; use crate::session::tests::mcp_config_for_test; use crate::session::turn_context::TurnEnvironment; -use crate::session::turn_context::TurnEnvironmentConfig; use crate::state::ActiveTurn; use crate::test_support::models_manager_with_provider; use crate::tools::hook_names::HookToolName; @@ -26,6 +25,7 @@ use codex_hooks::HooksConfig; use codex_model_provider::create_model_provider; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::EnvironmentConfigState; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::GranularApprovalConfig; @@ -1116,14 +1116,14 @@ async fn mcp_sandbox_cwd_uses_matching_server_environment_uri() -> anyhow::Resul }, environment, /*shell*/ None, - TurnEnvironmentConfig { + EnvironmentConfig { allow_login_shell: true, permission_profile: turn_context .config .permissions .permission_profile_state() .snapshot(), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }, ))); diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 1690591239..cfddb30f17 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -445,13 +445,11 @@ impl Session { .selected_capability_roots .iter() .cloned() - .chain(environments.turn_environments().flat_map(|environment| { - environment - .config - .selected_capability_roots - .clone() - .unwrap_or_else(|| environment.environment.selected_capability_roots()) - })) + .chain( + environments + .turn_environments() + .flat_map(|environment| environment.config.selected_capability_roots.clone()), + ) .enumerate() { if let Some(kept_location) = root_locations_by_id.get(&root.id) { diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 84829d90f1..dfd6ca1bb9 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -5,7 +5,6 @@ use crate::agents_md_manager::AgentsMdManager; use crate::config::ConstraintError; use crate::environment_selection::ThreadEnvironments; use crate::environment_selection::TurnEnvironmentSnapshot; -use crate::session::turn_context::TurnEnvironmentConfig; use crate::shell_snapshot::ShellSnapshot; use crate::state::ActiveTurn; use codex_extension_api::ExtensionDataInit; @@ -20,6 +19,7 @@ use codex_protocol::config_types::ServiceTier; use codex_protocol::mcp::ClientMcpExtensions; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSpecialPath; +use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::HookCompletedEvent; use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::ThreadHistoryMode; @@ -136,14 +136,14 @@ impl SessionConfiguration { &self.permission_profile_state } - pub(super) fn turn_environment_config(&self) -> TurnEnvironmentConfig { - TurnEnvironmentConfig { + pub(super) fn turn_environment_config(&self) -> EnvironmentConfig { + EnvironmentConfig { allow_login_shell: self .original_config_do_not_use .permissions .allow_login_shell, permission_profile: self.permission_profile_state.snapshot(), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), } } @@ -1070,7 +1070,6 @@ impl Session { let turn_environments = Arc::new(ThreadEnvironments::new( environment_manager, default_shell.clone(), - // Temporary: preserve thread-level behavior until environments supply config. session_configuration.turn_environment_config(), shell_snapshot, inherited_environments.unwrap_or_default(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index d6f1a0bc9b..46f351e0bb 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -8598,25 +8598,10 @@ async fn mcp_refresh_updates_plugin_auth_mode_before_checking_pending_state() { ); } -struct PendingNoiseConnectProvider; - -impl codex_exec_server::NoiseRendezvousConnectProvider for PendingNoiseConnectProvider { - fn connect_bundle( - &self, - _: codex_exec_server::NoiseChannelPublicKey, - ) -> futures::future::BoxFuture< - '_, - Result, - > { - Box::pin(futures::future::pending()) - } -} - #[tokio::test] #[tracing_test::traced_test] async fn conflicting_ready_environment_root_ids_keep_first_location() { let (session, turn_context) = make_session_and_context().await; - let environment_manager = session.services.turn_environments.environment_manager(); let selected_root = |environment_id: &str, path: &str| codex_protocol::capabilities::SelectedCapabilityRoot { id: "shared-root".to_string(), @@ -8639,29 +8624,21 @@ async fn conflicting_ready_environment_root_ids_keep_first_location() { environment_id, .. } = &selected_root.location; - let provider = Arc::new(PendingNoiseConnectProvider); - let environment = environment_manager - .materialize_pending_noise_environment(environment_id.clone(), provider.clone()) - .expect("materialize deferred environment"); - environment_manager - .report_environment_provisioning_status( - environment_id.clone(), - Ok(codex_exec_server::EnvironmentReadyInfo { - selected_capability_roots: vec![selected_root.clone()], - }), - provider, - ) - .expect("report environment ready"); + let mut environment_config = local_environment.config.clone(); + environment_config.selected_capability_roots = vec![selected_root.clone()]; turn_environments.push(TurnEnvironment::new( TurnEnvironmentSelection { environment_id: environment_id.clone(), cwd: local_environment.cwd().clone(), workspace_roots: local_environment.workspace_roots().to_vec(), - config: EnvironmentConfigState::FromThread, + config: EnvironmentConfigState::Ready(environment_config.clone()), }, - environment, + Arc::new( + codex_exec_server::Environment::create_for_tests(/*exec_server_url*/ None) + .expect("create test environment"), + ), local_environment.shell.clone(), - local_environment.config.clone(), + environment_config, )); } let environments = TurnEnvironmentSnapshot { diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 2a968e65af..a11bef12f4 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -11,10 +11,10 @@ use codex_file_system::FileSystemSandboxContext; use codex_model_provider::SharedModelProvider; use codex_protocol::SessionId; use codex_protocol::ThreadId; -use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER; use codex_protocol::openai_models::ModelInfo; +use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::ThreadHistoryMode; @@ -31,21 +31,12 @@ use tracing::instrument; pub(crate) type ShellSnapshotTask = Shared>>>; -/// Effective per-environment config; fields move here as executor config is migrated. -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct TurnEnvironmentConfig { - pub(crate) allow_login_shell: bool, - pub(crate) permission_profile: PermissionProfileSnapshot, - /// None preserves legacy executor roots; Some, including empty, is owner-installed. - pub(crate) selected_capability_roots: Option>, -} - #[derive(Clone)] pub(crate) struct TurnEnvironment { pub(crate) selection: TurnEnvironmentSelection, pub(crate) environment: Arc, pub(crate) shell: Option, - pub(crate) config: TurnEnvironmentConfig, + pub(crate) config: EnvironmentConfig, pub(crate) shell_snapshot: ShellSnapshotTask, } @@ -54,7 +45,7 @@ impl TurnEnvironment { selection: TurnEnvironmentSelection, environment: Arc, shell: Option, - config: TurnEnvironmentConfig, + config: EnvironmentConfig, ) -> Self { Self { selection, diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index 742083e247..1a7fed0194 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -14,7 +14,6 @@ use crate::sandboxing::SandboxPermissions; use crate::session::step_context::StepContext; use crate::session::tests::make_session_and_context; use crate::session::turn_context::TurnEnvironment; -use crate::session::turn_context::TurnEnvironmentConfig; use crate::shell::Shell; use crate::shell::ShellType; use crate::tools::context::FunctionToolOutput; @@ -25,6 +24,7 @@ use crate::tools::handlers::ShellCommandHandler; use crate::tools::hook_names::HookToolName; use crate::tools::registry::CoreToolRuntime; use crate::turn_diff_tracker::TurnDiffTracker; +use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::EnvironmentConfigState; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_shell_command::is_safe_command::is_known_safe_command; @@ -119,13 +119,13 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() { .environment, ), Some(selected_shell), - TurnEnvironmentConfig { + EnvironmentConfig { allow_login_shell: true, permission_profile: PermissionProfileSnapshot::active( permission_profile, active_permission_profile.clone(), ), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }, ); let mut expected_env = create_env( diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index 08ddeaa815..b62882c471 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -1,12 +1,12 @@ use super::*; use crate::config::PermissionProfileSnapshot; -use crate::session::turn_context::TurnEnvironmentConfig; use crate::tools::sandboxing::SandboxAttempt; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::EnvironmentConfigState; use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::TurnEnvironmentSelection; @@ -28,10 +28,10 @@ fn test_turn_environment(environment_id: &str) -> crate::session::turn_context:: }, std::sync::Arc::new(codex_exec_server::Environment::default_for_tests()), /*shell*/ None, - TurnEnvironmentConfig { + EnvironmentConfig { allow_login_shell: true, permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }, ) } diff --git a/codex-rs/core/src/tools/runtimes/shell_tests.rs b/codex-rs/core/src/tools/runtimes/shell_tests.rs index 1fe33bb8b2..a336983a53 100644 --- a/codex-rs/core/src/tools/runtimes/shell_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell_tests.rs @@ -1,9 +1,9 @@ use super::*; use crate::config::PermissionProfileSnapshot; -use crate::session::turn_context::TurnEnvironmentConfig; use crate::tools::approvals::ApprovalCacheKey; use codex_exec_server::Environment; use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::EnvironmentConfigState; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_utils_path_uri::PathUri; @@ -25,12 +25,12 @@ async fn approval_key_uses_path_uri_and_includes_environment_id() { }, Arc::new(Environment::default_for_tests()), /*shell*/ None, - TurnEnvironmentConfig { + EnvironmentConfig { allow_login_shell: true, permission_profile: PermissionProfileSnapshot::legacy( PermissionProfile::read_only(), ), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }, ), shell_type: None, diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 21ca88c384..b49843feba 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -515,11 +515,11 @@ mod tests { use super::*; use crate::config::PermissionProfileSnapshot; use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS; - use crate::session::turn_context::TurnEnvironmentConfig; use crate::tools::sandboxing::ToolRuntime; use codex_exec_server::Environment; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_protocol::models::PermissionProfile; + use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::EnvironmentConfigState; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_tools::ZshForkConfig; @@ -540,12 +540,12 @@ mod tests { }, Arc::new(Environment::default_for_tests()), /*shell*/ None, - TurnEnvironmentConfig { + EnvironmentConfig { allow_login_shell: true, permission_profile: PermissionProfileSnapshot::legacy( PermissionProfile::read_only(), ), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }, ) } diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index ddc34326e3..8eca4327aa 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -917,14 +917,14 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava .expect("remote test environment"), ), /*shell*/ None, - crate::session::turn_context::TurnEnvironmentConfig { + codex_protocol::protocol::EnvironmentConfig { allow_login_shell: true, permission_profile: turn .config .permissions .permission_profile_state() .snapshot(), - selected_capability_roots: None, + selected_capability_roots: Vec::new(), }, ), )); diff --git a/codex-rs/core/tests/suite/guardian_review.rs b/codex-rs/core/tests/suite/guardian_review.rs index 6c940cc56e..757990ba2e 100644 --- a/codex-rs/core/tests/suite/guardian_review.rs +++ b/codex-rs/core/tests/suite/guardian_review.rs @@ -25,6 +25,7 @@ use codex_login::CodexAuth; use codex_protocol::ThreadId; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::PermissionProfileSnapshot; use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::permissions::FileSystemAccessMode; @@ -33,6 +34,8 @@ use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::EnvironmentConfig; +use codex_protocol::protocol::EnvironmentConfigState; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::SandboxPolicy; @@ -511,6 +514,19 @@ async fn guardian_session_is_reused_for_consecutive_tool_reviews_without_prewarm ], ) .await; + let mut parent_environments = local_selections(test.config.cwd.clone()); + let parent_environment_config = EnvironmentConfig { + allow_login_shell: test.config.permissions.allow_login_shell, + permission_profile: PermissionProfileSnapshot::legacy( + test.config.permissions.permission_profile().clone(), + ), + selected_capability_roots: Vec::new(), + }; + parent_environments + .environments + .first_mut() + .expect("local environment selection") + .config = EnvironmentConfigState::Ready(parent_environment_config); test.codex .start_or_steer_turn( @@ -519,7 +535,7 @@ async fn guardian_session_is_reused_for_consecutive_tool_reviews_without_prewarm text_elements: Vec::new(), }]) .with_thread_settings(ThreadSettingsOverrides { - environments: Some(local_selections(test.config.cwd.clone())), + environments: Some(parent_environments), approval_policy: Some(approval_policy), approvals_reviewer: Some(ApprovalsReviewer::AutoReview), ..Default::default() diff --git a/codex-rs/core/tests/suite/mcp_tool_exposure.rs b/codex-rs/core/tests/suite/mcp_tool_exposure.rs index c08c83335a..5a666df086 100644 --- a/codex-rs/core/tests/suite/mcp_tool_exposure.rs +++ b/codex-rs/core/tests/suite/mcp_tool_exposure.rs @@ -18,6 +18,7 @@ use codex_mcp::McpResourceClient; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::PermissionProfileSnapshot; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; @@ -478,6 +479,9 @@ async fn root_reconciliation_reuses_pending_apps_startup() -> Result<()> { &selection, EnvironmentConfig { allow_login_shell: false, + permission_profile: PermissionProfileSnapshot::legacy( + test.config.permissions.permission_profile().clone(), + ), selected_capability_roots: vec![SelectedCapabilityRoot { id: "calendar-root".to_string(), location: CapabilityRootLocation::Environment { diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 89c250aa9b..69604aab33 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -46,6 +46,7 @@ use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::PermissionProfileSnapshot; use codex_protocol::models::SandboxPermissions; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; @@ -435,6 +436,105 @@ async fn explicit_remote_shell_runs_in_remote_cwd() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn ready_environment_permissions_override_thread_permissions() -> Result<()> { + const CALL_ID: &str = "attachment-specific-permissions"; + const FILE_NAME: &str = "attachment-read-only-marker.txt"; + + skip_if_target_windows!( + Ok(()), + "Windows sandbox enforcement is covered by the platform-specific suite" + ); + + let server = start_mock_server().await; + let mut builder = test_codex().with_config(|config| { + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + config + .permissions + .set_permission_profile(PermissionProfile::workspace_write()) + .expect("thread should allow workspace writes"); + }); + let test = builder.build_with_auto_env(&server).await?; + let mut selection = test.executor_environment().selection().clone(); + let marker = selection.cwd.join(FILE_NAME)?; + selection.config = EnvironmentConfigState::Ready(EnvironmentConfig { + allow_login_shell: test.config.permissions.allow_login_shell, + permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()), + selected_capability_roots: Vec::new(), + }); + + let (shell, command) = match test_target_os() { + TestTargetOs::Linux => ( + "bash", + format!( + "if printf blocked > {FILE_NAME}; then echo WRITE_SUCCEEDED; else echo WRITE_DENIED; fi" + ), + ), + TestTargetOs::MacOs => ( + "zsh", + format!( + "if printf blocked > {FILE_NAME}; then echo WRITE_SUCCEEDED; else echo WRITE_DENIED; fi" + ), + ), + TestTargetOs::Windows => ( + "powershell", + format!( + "try {{ Set-Content -Path '{FILE_NAME}' -Value blocked -ErrorAction Stop; Write-Output WRITE_SUCCEEDED }} catch {{ Write-Output WRITE_DENIED }}" + ), + ), + }; + let arguments = serde_json::to_string(&json!({ + "cmd": command, + "shell": shell, + "login": false, + "yield_time_ms": 10_000, + "sandbox_permissions": SandboxPermissions::UseDefault, + }))?; + let response_mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(CALL_ID, "exec_command", &arguments), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + test.submit_turn_with_environments("try to write a file", Some(vec![selection])) + .await?; + + let output = response_mock + .last_request() + .context("model should receive the command output")? + .function_call_output_text(CALL_ID) + .context("shell tool result should be present")?; + assert!( + output.contains("WRITE_DENIED"), + "unexpected output: {output}" + ); + assert!(!output.contains("WRITE_SUCCEEDED")); + assert!( + test.fs() + .read_file_text(&marker, /*sandbox*/ None) + .await + .is_err(), + "read-only attachment unexpectedly wrote {FILE_NAME}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn step_world_state_gates_deferred_prompt_independently_of_host_config() -> Result<()> { for deferred_executor_enabled in [false, true] { @@ -1007,11 +1107,14 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment( path: selection.cwd.clone(), }, }; + let permission_profile = + PermissionProfileSnapshot::legacy(test.config.permissions.permission_profile().clone()); for config in [ EnvironmentConfigState::Pending, EnvironmentConfigState::Ready(EnvironmentConfig { allow_login_shell: false, + permission_profile: permission_profile.clone(), selected_capability_roots: vec![root("duplicate"), root("duplicate")], }), ] { @@ -1045,6 +1148,7 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment( environments: Some(vec![TurnEnvironmentSelection { config: EnvironmentConfigState::Ready(EnvironmentConfig { allow_login_shell: true, + permission_profile: permission_profile.clone(), selected_capability_roots: vec![root("startup-root"), root("second-root")], }), ..selection.clone() @@ -1062,6 +1166,7 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment( vec![TurnEnvironmentSelection { config: EnvironmentConfigState::Ready(EnvironmentConfig { allow_login_shell: false, + permission_profile: permission_profile.clone(), selected_capability_roots: vec![root("first-root")], }), ..selection.clone() @@ -1118,6 +1223,7 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment( vec![TurnEnvironmentSelection { config: EnvironmentConfigState::Ready(EnvironmentConfig { allow_login_shell: false, + permission_profile: permission_profile.clone(), selected_capability_roots: vec![root("first-updated-root")], }), ..selection.clone() @@ -1283,7 +1389,7 @@ async fn ready_before_selection_exposes_remote_tools_and_capability_context_afte .report_environment_provisioning_status( REMOTE_ENVIRONMENT_ID.to_string(), Ok(EnvironmentReadyInfo { - selected_capability_roots: vec![ready_root], + selected_capability_roots: Vec::new(), }), Arc::new(ReadyNoiseConnectProvider { websocket_url: format!("{rendezvous_url}/relay?role=harness"), @@ -1327,7 +1433,13 @@ async fn ready_before_selection_exposes_remote_tools_and_capability_context_afte environment_id: REMOTE_ENVIRONMENT_ID.to_string(), cwd: PathUri::from_abs_path(&test.config.cwd), workspace_roots: vec![PathUri::from_abs_path(&test.config.cwd)], - config: EnvironmentConfigState::FromThread, + config: EnvironmentConfigState::Ready(EnvironmentConfig { + allow_login_shell: true, + permission_profile: PermissionProfileSnapshot::legacy( + test.config.permissions.permission_profile().clone(), + ), + selected_capability_roots: vec![ready_root], + }), }]), ) .await?; diff --git a/codex-rs/protocol/src/environment.rs b/codex-rs/protocol/src/environment.rs index 54a55f3af4..3ed127c310 100644 --- a/codex-rs/protocol/src/environment.rs +++ b/codex-rs/protocol/src/environment.rs @@ -1,4 +1,5 @@ use crate::capabilities::SelectedCapabilityRoot; +use crate::models::PermissionProfileSnapshot; /// Configuration supplied for a thread's selected environment. #[derive(Clone, Debug, PartialEq, Eq)] @@ -11,11 +12,13 @@ pub enum EnvironmentConfigState { Ready(EnvironmentConfig), } -/// Resolved configuration supplied by the owner of a thread/environment attachment. +/// Resolved configuration for a thread/environment attachment. #[derive(Clone, Debug, PartialEq, Eq)] pub struct EnvironmentConfig { /// Whether shell tools may start login shells in this environment. pub allow_login_shell: bool, + /// Resolved permissions for this thread's environment attachment. + pub permission_profile: PermissionProfileSnapshot, /// Capability roots selected for this thread's environment attachment. pub selected_capability_roots: Vec, }