diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index 8bb628d9da..1ffeaf131b 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -3,6 +3,7 @@ use crate::config::ConfigBuilder; use crate::context::ContextualUserFragment; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::environment_selection::TurnEnvironmentState; +use crate::session::turn_context::EnvironmentConfig; use crate::session::turn_context::TurnEnvironment; use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerStack; @@ -337,6 +338,9 @@ fn resolved_local_environments( PathUri::from_abs_path(&cwd), Vec::new(), /*shell*/ None, + EnvironmentConfig { + allow_login_shell: true, + }, )) }) .collect(), diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 433e50819a..1ebe44a749 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -22,6 +22,7 @@ use futures::future::BoxFuture; use futures::future::Shared; use tokio_util::task::AbortOnDropHandle; +use crate::session::turn_context::EnvironmentConfig; use crate::session::turn_context::TurnEnvironment; use crate::shell::Shell; use crate::shell_snapshot::ShellSnapshot; @@ -79,6 +80,8 @@ impl StartingTurnEnvironment { pub(crate) struct ThreadEnvironments { environment_manager: Arc, local_shell: Shell, + // Temporary thread-wide config source until environments supply their own config. + fallback_environment_config: EnvironmentConfig, shell_snapshot: ShellSnapshot, non_blocking_snapshots: bool, environments: ArcSwap>, @@ -89,6 +92,7 @@ impl ThreadEnvironments { pub(crate) fn new( environment_manager: Arc, local_shell: Shell, + fallback_environment_config: EnvironmentConfig, shell_snapshot: ShellSnapshot, current: TurnEnvironmentSnapshot, non_blocking_snapshots: bool, @@ -98,9 +102,11 @@ impl ThreadEnvironments { .environments .into_iter() .filter_map(|environment| { - let TurnEnvironmentState::Ready(environment) = environment else { + let TurnEnvironmentState::Ready(mut environment) = environment else { return None; }; + // Child threads can have a different shell policy from their parent. + environment.config = fallback_environment_config.clone(); let selection = environment.selection(); let selected_environment = Arc::clone(&environment.environment); let resolution: TurnEnvironmentResolution = @@ -116,6 +122,7 @@ impl ThreadEnvironments { Self { environment_manager, local_shell, + fallback_environment_config, shell_snapshot, non_blocking_snapshots, environments: ArcSwap::from_pointee(environments), @@ -166,6 +173,7 @@ impl ThreadEnvironments { selected_environment.clone(), Arc::clone(&environment), self.local_shell.clone(), + self.fallback_environment_config.clone(), self.shell_snapshot.clone(), ) .remote_handle(); @@ -265,6 +273,7 @@ impl ThreadEnvironments { selection: TurnEnvironmentSelection, environment: Arc, local_shell: Shell, + config: EnvironmentConfig, shell_snapshot: ShellSnapshot, ) -> BoxFuture<'static, TurnEnvironmentResult> { async move { @@ -300,6 +309,7 @@ impl ThreadEnvironments { selection.cwd, selection.workspace_roots, shell, + config, ); let task = shell_snapshot .build(turn_environment.clone()) @@ -506,6 +516,9 @@ mod tests { let turn_environments = Arc::new(ThreadEnvironments::new( environment_manager, crate::shell::default_user_shell(), + EnvironmentConfig { + allow_login_shell: true, + }, ShellSnapshot::disabled(), TurnEnvironmentSnapshot::default(), /*non_blocking_snapshots*/ false, @@ -649,7 +662,7 @@ url = "ws://127.0.0.1:8765" } #[tokio::test] - async fn local_environment_uses_configured_shell() { + async fn local_environment_uses_configured_shell_and_login_policy() { let cwd = AbsolutePathBuf::current_dir().expect("cwd"); let local_shell = Shell { shell_type: crate::shell::ShellType::Zsh, @@ -658,6 +671,9 @@ url = "ws://127.0.0.1:8765" let turn_environments = ThreadEnvironments::new( Arc::new(EnvironmentManager::default_for_tests()), local_shell.clone(), + EnvironmentConfig { + allow_login_shell: false, + }, ShellSnapshot::disabled(), TurnEnvironmentSnapshot::default(), /*non_blocking_snapshots*/ false, @@ -669,13 +685,10 @@ url = "ws://127.0.0.1:8765" }]); let snapshot = turn_environments.snapshot().await; + let environment = snapshot.primary().expect("local environment"); - assert_eq!( - snapshot - .primary() - .and_then(|environment| environment.shell.as_ref()), - Some(&local_shell) - ); + assert_eq!(environment.shell.as_ref(), Some(&local_shell)); + assert!(!environment.config.allow_login_shell); } #[tokio::test] @@ -797,6 +810,9 @@ url = "ws://127.0.0.1:8765" let environments = Arc::new(ThreadEnvironments::new( manager, crate::shell::default_user_shell(), + EnvironmentConfig { + allow_login_shell: true, + }, ShellSnapshot::disabled(), TurnEnvironmentSnapshot::default(), /*non_blocking_snapshots*/ false, @@ -850,6 +866,9 @@ url = "ws://127.0.0.1:8765" let turn_environments = ThreadEnvironments::new( manager, crate::shell::default_user_shell(), + EnvironmentConfig { + allow_login_shell: true, + }, ShellSnapshot::disabled(), TurnEnvironmentSnapshot::default(), /*non_blocking_snapshots*/ true, @@ -924,6 +943,9 @@ url = "ws://127.0.0.1:8765" let environments = ThreadEnvironments::new( Arc::clone(&manager), crate::shell::default_user_shell(), + EnvironmentConfig { + allow_login_shell: true, + }, ShellSnapshot::disabled(), TurnEnvironmentSnapshot::default(), /*non_blocking_snapshots*/ true, @@ -978,6 +1000,9 @@ url = "ws://127.0.0.1:8765" let environments = Arc::new(ThreadEnvironments::new( Arc::clone(&manager), crate::shell::default_user_shell(), + EnvironmentConfig { + allow_login_shell: true, + }, ShellSnapshot::disabled(), TurnEnvironmentSnapshot::default(), /*non_blocking_snapshots*/ true, @@ -1050,7 +1075,7 @@ url = "ws://127.0.0.1:8765" } #[tokio::test] - async fn inherited_environment_reuses_parent_handle() { + async fn inherited_environment_reuses_parent_handle_and_uses_child_login_policy() { let cwd = AbsolutePathBuf::current_dir().expect("cwd"); let selection = TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), @@ -1067,6 +1092,9 @@ url = "ws://127.0.0.1:8765" selection.cwd.clone(), Vec::new(), /*shell*/ None, + EnvironmentConfig { + allow_login_shell: true, + }, ); let manager = Arc::new(environment_manager_without_environments()); manager @@ -1079,6 +1107,9 @@ url = "ws://127.0.0.1:8765" let environments = ThreadEnvironments::new( manager, crate::shell::default_user_shell(), + EnvironmentConfig { + allow_login_shell: false, + }, ShellSnapshot::disabled(), TurnEnvironmentSnapshot { environments: vec![TurnEnvironmentState::Ready(inherited)], @@ -1089,13 +1120,9 @@ url = "ws://127.0.0.1:8765" environments.update_selections(std::slice::from_ref(&selection)); let snapshot = environments.snapshot().await; - assert!(Arc::ptr_eq( - &snapshot - .primary() - .expect("inherited environment") - .environment, - &inherited_environment, - )); + let inherited = snapshot.primary().expect("inherited environment"); + assert!(Arc::ptr_eq(&inherited.environment, &inherited_environment)); + assert!(!inherited.config.allow_login_shell); } #[tokio::test] @@ -1124,6 +1151,9 @@ url = "ws://127.0.0.1:8765" cwd_uri.clone(), Vec::new(), /*shell*/ None, + EnvironmentConfig { + allow_login_shell: true, + }, ))], }; let multiple = TurnEnvironmentSnapshot { @@ -1135,6 +1165,9 @@ url = "ws://127.0.0.1:8765" cwd_uri, Vec::new(), /*shell*/ None, + EnvironmentConfig { + allow_login_shell: true, + }, )), ], }; diff --git a/codex-rs/core/src/mcp_openai_file.rs b/codex-rs/core/src/mcp_openai_file.rs index 9075923115..1cc2e72a71 100644 --- a/codex-rs/core/src/mcp_openai_file.rs +++ b/codex-rs/core/src/mcp_openai_file.rs @@ -217,6 +217,7 @@ mod tests { use super::*; use crate::environment_selection::TurnEnvironmentState; use crate::session::tests::make_session_and_context; + use crate::session::turn_context::EnvironmentConfig; use crate::session::turn_context::TurnContext; use crate::session::turn_context::TurnEnvironment; use codex_utils_absolute_path::AbsolutePathBuf; @@ -239,6 +240,7 @@ mod tests { PathUri::from_abs_path(&cwd), Vec::new(), primary.shell.clone(), + primary.config.clone(), ); } @@ -323,6 +325,9 @@ mod tests { let environments = crate::environment_selection::ThreadEnvironments::new( session.services.turn_environments.environment_manager(), crate::shell::default_user_shell(), + EnvironmentConfig { + allow_login_shell: turn_context.config.permissions.allow_login_shell, + }, crate::shell_snapshot::ShellSnapshot::disabled(), Default::default(), /*non_blocking_snapshots*/ true, diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 4db1cdbd5c..c0d93364fd 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -6,6 +6,7 @@ use crate::session::step_context::StepContext; 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::EnvironmentConfig; use crate::session::turn_context::TurnEnvironment; use crate::state::ActiveTurn; use crate::test_support::models_manager_with_provider; @@ -1102,6 +1103,9 @@ async fn mcp_sandbox_cwd_uses_matching_server_environment_uri() -> anyhow::Resul secondary_cwd.clone(), Vec::new(), /*shell*/ None, + EnvironmentConfig { + allow_login_shell: true, + }, ))); let step_context = StepContext::for_test(Arc::new(turn_context)); diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 4b40cbc2e7..e6f5627920 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -5,6 +5,7 @@ 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::EnvironmentConfig; use crate::shell_snapshot::ShellSnapshot; use crate::skills::SkillError; use crate::state::ActiveTurn; @@ -939,6 +940,10 @@ impl Session { let turn_environments = Arc::new(ThreadEnvironments::new( environment_manager, default_shell.clone(), + // Temporary: preserve thread-level behavior until environments supply config. + EnvironmentConfig { + allow_login_shell: config.permissions.allow_login_shell, + }, shell_snapshot, inherited_environments.unwrap_or_default(), config.features.enabled(Feature::DeferredExecutor), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index c67aa14dd6..d2970beb45 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1,4 +1,5 @@ use super::mcp_refresh::McpRefresh; +use super::turn_context::EnvironmentConfig; use super::turn_context::TurnEnvironment; use super::*; use crate::agents_md_manager::AgentsMdManager; @@ -4869,6 +4870,12 @@ async fn resolved_environments_for_configuration( let turn_environments = ThreadEnvironments::new( Arc::clone(&environment_manager), default_user_shell(), + EnvironmentConfig { + allow_login_shell: session_configuration + .original_config_do_not_use + .permissions + .allow_login_shell, + }, ShellSnapshot::disabled(), TurnEnvironmentSnapshot::default(), /*non_blocking_snapshots*/ false, @@ -5667,6 +5674,9 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { let turn_environments = Arc::new(ThreadEnvironments::new( environment_manager, default_user_shell(), + EnvironmentConfig { + allow_login_shell: config.permissions.allow_login_shell, + }, ShellSnapshot::disabled(), resolved_environments, /*non_blocking_snapshots*/ false, @@ -6470,6 +6480,7 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir PathUri::from_abs_path(&environment_cwd), Vec::new(), current_environment.shell, + current_environment.config, )); let call_id = "call-1".to_string(); @@ -7131,6 +7142,7 @@ async fn primary_environment_uses_first_turn_environment() { second_cwd_uri.clone(), Vec::new(), /*shell*/ None, + first_environment.config.clone(), ))); assert_eq!( @@ -7856,6 +7868,9 @@ where let turn_environments = Arc::new(ThreadEnvironments::new( environment_manager, default_user_shell(), + EnvironmentConfig { + allow_login_shell: config.permissions.allow_login_shell, + }, ShellSnapshot::disabled(), resolved_turn_environments.clone(), /*non_blocking_snapshots*/ false, @@ -8446,6 +8461,7 @@ async fn conflicting_ready_environment_root_ids_keep_first_location() { local_environment.cwd().clone(), local_environment.workspace_roots().to_vec(), local_environment.shell.clone(), + local_environment.config.clone(), )); } let environments = TurnEnvironmentSnapshot { @@ -8656,6 +8672,7 @@ async fn record_context_updates_emits_environment_item_for_cwd_changes() { PathUri::from_abs_path(&cwd), Vec::new(), environment.shell, + environment.config, )); let update_items = @@ -8721,6 +8738,7 @@ async fn record_context_updates_omits_environment_item_when_disabled() { PathUri::from_abs_path(&test_path_buf("/new-repo").abs()), Vec::new(), environment.shell, + environment.config, )); let update_items = @@ -9172,6 +9190,7 @@ async fn turn_context_item_stores_local_cwd() { cwd, Vec::new(), environment.shell, + environment.config, )); #[allow(deprecated)] diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 600f736d69..30ae7e2d81 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -27,6 +27,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 EnvironmentConfig { + pub(crate) allow_login_shell: bool, +} + #[derive(Clone)] pub(crate) struct TurnEnvironment { pub(crate) environment_id: String, @@ -34,6 +40,7 @@ pub(crate) struct TurnEnvironment { cwd: PathUri, workspace_roots: Vec, pub(crate) shell: Option, + pub(crate) config: EnvironmentConfig, pub(crate) shell_snapshot: ShellSnapshotTask, } @@ -44,6 +51,7 @@ impl TurnEnvironment { cwd: PathUri, workspace_roots: Vec, shell: Option, + config: EnvironmentConfig, ) -> Self { Self { environment_id, @@ -51,6 +59,7 @@ impl TurnEnvironment { cwd, workspace_roots, shell, + config, shell_snapshot: futures::future::ready(None).boxed().shared(), } } @@ -90,6 +99,7 @@ impl std::fmt::Debug for TurnEnvironment { .field("cwd", &self.cwd) .field("workspace_roots", &self.workspace_roots) .field("shell", &self.shell) + .field("config", &self.config) .finish_non_exhaustive() } } diff --git a/codex-rs/core/src/tools/handlers/shell/shell_command.rs b/codex-rs/core/src/tools/handlers/shell/shell_command.rs index 2b4cc7e2ed..7b975e2f71 100644 --- a/codex-rs/core/src/tools/handlers/shell/shell_command.rs +++ b/codex-rs/core/src/tools/handlers/shell/shell_command.rs @@ -91,14 +91,14 @@ impl ShellCommandHandler { turn_context: &TurnContext, turn_environment: &TurnEnvironment, cwd: AbsolutePathBuf, - allow_login_shell: bool, ) -> Result { let session_shell = session.user_shell(); let shell = turn_environment .shell .as_ref() .unwrap_or(session_shell.as_ref()); - let use_login_shell = Self::resolve_use_login_shell(params.login, allow_login_shell)?; + let use_login_shell = + Self::resolve_use_login_shell(params.login, turn_environment.config.allow_login_shell)?; let command = Self::base_command(shell, ¶ms.command, use_login_shell); let mut env = create_env( @@ -214,7 +214,6 @@ impl ShellCommandHandler { turn.as_ref(), &turn_environment, cwd, - turn.config.permissions.allow_login_shell, )?; let shell_type = Some( turn_environment diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index f71f23ca1b..f7caa3a9e4 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -12,6 +12,7 @@ use crate::exec_env::inject_permission_profile_env; use crate::sandboxing::SandboxPermissions; use crate::session::step_context::StepContext; use crate::session::tests::make_session_and_context; +use crate::session::turn_context::EnvironmentConfig; use crate::session::turn_context::TurnEnvironment; use crate::shell::Shell; use crate::shell::ShellType; @@ -111,6 +112,9 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() { PathUri::from_abs_path(&selected_cwd), Vec::new(), Some(selected_shell), + EnvironmentConfig { + allow_login_shell: true, + }, ); let mut expected_env = create_env( &turn_context.config.permissions.shell_environment_policy, @@ -136,7 +140,6 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() { &turn_context, &selected_environment, expected_cwd.clone(), - /*allow_login_shell*/ true, ) .expect("login shells should be allowed"); @@ -192,10 +195,12 @@ fn shell_command_handler_respects_explicit_login_flag() { #[tokio::test] async fn shell_command_handler_defaults_to_non_login_when_disallowed() { let (session, turn_context) = make_session_and_context().await; - let turn_environment = turn_context + let mut turn_environment = turn_context .environments .primary() - .expect("primary environment"); + .expect("primary environment") + .clone(); + turn_environment.config.allow_login_shell = false; let cwd = turn_environment .cwd() .to_abs_path() @@ -215,9 +220,8 @@ async fn shell_command_handler_defaults_to_non_login_when_disallowed() { ¶ms, &session, &turn_context, - turn_environment, + &turn_environment, cwd, - /*allow_login_shell*/ false, ) .expect("non-login shells should still be allowed"); @@ -257,7 +261,6 @@ async fn shell_command_handler_rejects_justification_without_sandbox_permissions &turn_context, turn_environment, cwd, - /*allow_login_shell*/ false, ) .expect_err("justification without sandbox permissions should be rejected"); diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index 16eed42a80..d6d5ab699b 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -235,7 +235,7 @@ impl ExecCommandHandler { &args, shell, &shell_mode, - turn.config.permissions.allow_login_shell, + turn_environment.config.allow_login_shell, ) .map_err(FunctionCallError::RespondToModel)?; let command = resolved_command.command; diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index 9b107dd83d..626bb90414 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -9,6 +9,8 @@ use codex_utils_output_truncation::TruncationPolicy; use pretty_assertions::assert_eq; use std::sync::Arc; +use crate::environment_selection::TurnEnvironmentState; +use crate::function_tool::FunctionCallError; use crate::session::step_context::StepContext; use crate::session::tests::make_session_and_context; use crate::tools::context::ExecCommandToolOutput; @@ -17,6 +19,7 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::hook_names::HookToolName; use crate::tools::registry::CoreToolRuntime; +use crate::tools::registry::ToolExecutor; use crate::turn_diff_tracker::TurnDiffTracker; use tokio::sync::Mutex; @@ -168,6 +171,46 @@ fn test_get_command_rejects_explicit_login_when_disallowed() -> anyhow::Result<( Ok(()) } +#[tokio::test] +async fn exec_command_rejects_login_when_selected_environment_disallows_it() { + let (session, mut turn) = make_session_and_context().await; + assert!(turn.config.permissions.allow_login_shell); + let TurnEnvironmentState::Ready(environment) = turn + .environments + .environments + .first_mut() + .expect("primary environment") + else { + panic!("primary environment should be ready"); + }; + environment.config.allow_login_shell = false; + + let turn = Arc::new(turn); + let invocation = ToolInvocation { + session: session.into(), + step_context: StepContext::for_test(Arc::clone(&turn)), + turn, + cancellation_token: tokio_util::sync::CancellationToken::new(), + tracker: Arc::new(Mutex::new(TurnDiffTracker::new())), + call_id: "login-disallowed".to_string(), + tool_name: codex_tools::ToolName::plain("exec_command"), + source: ToolCallSource::Direct, + payload: ToolPayload::Function { + arguments: serde_json::json!({ "cmd": "echo hello", "login": true }).to_string(), + }, + }; + + let Err(FunctionCallError::RespondToModel(message)) = + ExecCommandHandler::default().handle(invocation).await + else { + panic!("expected login-shell rejection"); + }; + assert_eq!( + message, + "login shell is disabled by config; omit `login` or set it to false." + ); +} + #[test] fn test_get_command_rejects_explicit_shell_in_zsh_fork_mode() -> anyhow::Result<()> { let json = r#"{"cmd": "echo hello", "shell": "/bin/bash"}"#; diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 20efe1b549..675f409b72 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -273,6 +273,7 @@ mod tests { PathUri::from_abs_path(&cwd), Vec::new(), current.shell, + current.config, )); } 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 3a879de8c9..146e571a3b 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::session::turn_context::EnvironmentConfig; use crate::tools::sandboxing::SandboxAttempt; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::AdditionalPermissionProfile; @@ -21,6 +22,9 @@ fn test_turn_environment(environment_id: &str) -> crate::session::turn_context:: PathUri::from_abs_path(&std::env::temp_dir().abs()), Vec::new(), /*shell*/ None, + EnvironmentConfig { + allow_login_shell: true, + }, ) } diff --git a/codex-rs/core/src/tools/runtimes/shell_tests.rs b/codex-rs/core/src/tools/runtimes/shell_tests.rs index 7ae93ab918..b4f43b6a38 100644 --- a/codex-rs/core/src/tools/runtimes/shell_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::session::turn_context::EnvironmentConfig; use codex_exec_server::Environment; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; @@ -16,6 +17,9 @@ async fn approval_key_uses_path_uri_and_includes_environment_id() { PathUri::from_abs_path(&cwd), Vec::new(), /*shell*/ None, + EnvironmentConfig { + allow_login_shell: true, + }, ), shell_type: None, hook_command: "echo hello".to_string(), diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index a2781fb890..40bc938c22 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -549,6 +549,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt mod tests { use super::*; use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS; + use crate::session::turn_context::EnvironmentConfig; use crate::tools::sandboxing::ToolRuntime; use codex_exec_server::Environment; use codex_exec_server::LOCAL_ENVIRONMENT_ID; @@ -566,6 +567,9 @@ mod tests { cwd, Vec::new(), /*shell*/ None, + EnvironmentConfig { + allow_login_shell: true, + }, ) } diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 36ab4758fa..0dbb5c0580 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -760,7 +760,7 @@ fn add_core_tool_sources(context: &CoreToolPlanContext<'_>, registry: &mut ToolR if environment_mode.has_environment() { let include_environment_id = matches!(environment_mode, ToolEnvironmentMode::Multiple); registry.add(ExecCommandHandler::new(ExecCommandHandlerOptions { - allow_login_shell: turn_context.config.permissions.allow_login_shell, + allow_login_shell: any_environment_allows_login_shell(context.environments), exec_permission_approvals_enabled: false, include_environment_id, include_shell_parameter: unified_exec_should_include_shell_parameter( @@ -800,6 +800,12 @@ fn tool_environment_mode(environments: &TurnEnvironmentSnapshot) -> ToolEnvironm ToolEnvironmentMode::from_count(environments.turn_environments().count()) } +fn any_environment_allows_login_shell(environments: &TurnEnvironmentSnapshot) -> bool { + environments + .turn_environments() + .any(|environment| environment.config.allow_login_shell) +} + #[instrument(level = "trace", skip_all)] fn add_shell_tools(context: &CoreToolPlanContext<'_>, registry: &mut ToolRegistry) { let turn_context = context.turn_context; @@ -809,7 +815,7 @@ fn add_shell_tools(context: &CoreToolPlanContext<'_>, registry: &mut ToolRegistr return; } - let allow_login_shell = turn_context.config.permissions.allow_login_shell; + let allow_login_shell = any_environment_allows_login_shell(context.environments); let exec_permission_approvals_enabled = features.enabled(Feature::ExecPermissionApprovals); let include_environment_id = matches!(environment_mode, ToolEnvironmentMode::Multiple); let supports_shell_command = context.environments.single_local_environment().is_some(); diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 441eea6ed2..5d5adac913 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -661,6 +661,70 @@ async fn shell_family_registers_visible_unified_exec_and_hidden_legacy_shell() { assert!(has_parameter(plan.visible_spec("exec_command"), "shell")); } +#[tokio::test] +async fn login_shell_parameter_follows_selected_environment() { + for (tool_name, guardian) in [ + ("shell_command", false), + ("exec_command", false), + ("exec_command", true), + ] { + for allow_login_shell in [false, true] { + let plan = probe(|turn| { + set_feature(turn, Feature::ShellTool, /*enabled*/ true); + set_feature(turn, Feature::UnifiedExec, tool_name == "exec_command"); + set_feature(turn, Feature::ShellZshFork, /*enabled*/ false); + turn.model_info.shell_type = ConfigShellToolType::ShellCommand; + update_config(turn, |config| { + config.permissions.allow_login_shell = !allow_login_shell; + }); + let TurnEnvironmentState::Ready(environment) = turn + .environments + .environments + .first_mut() + .expect("primary environment") + else { + panic!("primary environment should be ready"); + }; + environment.config.allow_login_shell = allow_login_shell; + if guardian { + turn.session_source = codex_protocol::protocol::SessionSource::SubAgent( + codex_protocol::protocol::SubAgentSource::Other( + crate::guardian::GUARDIAN_REVIEWER_NAME.to_string(), + ), + ); + } + }) + .await; + + assert_eq!( + has_parameter(plan.visible_spec(tool_name), "login"), + allow_login_shell + ); + } + } +} + +#[tokio::test] +async fn login_shell_parameter_is_available_when_any_environment_allows_it() { + let plan = probe(|turn| { + set_features(turn, &[Feature::ShellTool, Feature::UnifiedExec]); + set_feature(turn, Feature::ShellZshFork, /*enabled*/ false); + update_config(turn, |config| { + config.permissions.allow_login_shell = false; + }); + duplicate_primary_environment(turn); + for (index, environment) in turn.environments.environments.iter_mut().enumerate() { + let TurnEnvironmentState::Ready(environment) = environment else { + panic!("environment should be ready"); + }; + environment.config.allow_login_shell = index == 1; + } + }) + .await; + + assert!(has_parameter(plan.visible_spec("exec_command"), "login")); +} + #[tokio::test] async fn shell_command_is_not_registered_without_a_single_local_environment() { let remote_environment = probe(|turn| { @@ -836,6 +900,9 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava remote_cwd, Vec::new(), /*shell*/ None, + crate::session::turn_context::EnvironmentConfig { + allow_login_shell: true, + }, ), )); }) diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 23f644da57..4848080c00 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -242,6 +242,58 @@ async fn create_workspace_directory( Ok(abs_path.into_path_buf()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn exec_command_hides_and_rejects_login_when_disabled() -> Result<()> { + skip_if_no_network!(Ok(())); + + let builder = test_codex().with_model("gpt-5.4").with_config(|config| { + config.permissions.allow_login_shell = false; + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + }); + let harness = TestCodexHarness::with_builder(builder).await?; + let call_id = "exec-command-login-disabled"; + let arguments = json!({ + "cmd": "echo should not run", + "login": true, + }); + let responses = mount_sse_sequence( + harness.server(), + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&arguments)?), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + harness.submit("run the command with login").await?; + + assert_eq!( + harness.function_call_stdout(call_id).await, + "login shell is disabled by config; omit `login` or set it to false." + ); + let request = responses.requests()[0].body_json(); + let exec_tool = request["tools"] + .as_array() + .expect("tools should be an array") + .iter() + .find(|tool| tool["name"] == "exec_command") + .expect("exec_command should be available"); + assert!(exec_tool["parameters"]["properties"].get("login").is_none()); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn unified_exec_intercepts_apply_patch_exec_command() -> Result<()> { skip_if_no_network!(Ok(()));