feat: add config allow_login_shell (#12312)

codex-rs/core/src/tools/handlers/shell.rs

@@ -63,8 +63,20 @@ impl ShellHandler {
 }
 
 impl ShellCommandHandler {
-    fn base_command(shell: &Shell, command: &str, login: Option<bool>) -> Vec<String> {
-        let use_login_shell = login.unwrap_or(true);
+    fn resolve_use_login_shell(
+        login: Option<bool>,
+        allow_login_shell: bool,
+    ) -> Result<bool, FunctionCallError> {
+        if !allow_login_shell && login == Some(true) {
+            return Err(FunctionCallError::RespondToModel(
+                "login shell is disabled by config; omit `login` or set it to false.".to_string(),
+            ));
+        }
+
+        Ok(login.unwrap_or(allow_login_shell))
+    }
+
+    fn base_command(shell: &Shell, command: &str, use_login_shell: bool) -> Vec<String> {
         shell.derive_exec_args(command, use_login_shell)
     }
 
@@ -73,11 +85,13 @@ impl ShellCommandHandler {
         session: &crate::codex::Session,
         turn_context: &TurnContext,
         thread_id: ThreadId,
-    ) -> ExecParams {
+        allow_login_shell: bool,
+    ) -> Result<ExecParams, FunctionCallError> {
         let shell = session.user_shell();
-        let command = Self::base_command(shell.as_ref(), &params.command, params.login);
+        let use_login_shell = Self::resolve_use_login_shell(params.login, allow_login_shell)?;
+        let command = Self::base_command(shell.as_ref(), &params.command, use_login_shell);
 
-        ExecParams {
+        Ok(ExecParams {
             command,
             cwd: turn_context.resolve_path(params.workdir.clone()),
             expiration: params.timeout_ms.into(),
@@ -87,7 +101,7 @@ impl ShellCommandHandler {
             windows_sandbox_level: turn_context.windows_sandbox_level,
             justification: params.justification.clone(),
             arg0: None,
-        }
+        })
     }
 }
 
@@ -183,8 +197,15 @@ impl ToolHandler for ShellCommandHandler {
 
         serde_json::from_str::<ShellCommandToolCallParams>(arguments)
             .map(|params| {
+                let use_login_shell = match Self::resolve_use_login_shell(
+                    params.login,
+                    invocation.turn.tools_config.allow_login_shell,
+                ) {
+                    Ok(use_login_shell) => use_login_shell,
+                    Err(_) => return true,
+                };
                 let shell = invocation.session.user_shell();
-                let command = Self::base_command(shell.as_ref(), &params.command, params.login);
+                let command = Self::base_command(shell.as_ref(), &params.command, use_login_shell);
                 !is_known_safe_command(&command)
             })
             .unwrap_or(true)
@@ -213,7 +234,8 @@ impl ToolHandler for ShellCommandHandler {
             session.as_ref(),
             turn.as_ref(),
             session.conversation_id,
-        );
+            turn.tools_config.allow_login_shell,
+        )?;
         ShellHandler::run_exec_like(RunExecLikeArgs {
             tool_name,
             exec_params,
@@ -439,7 +461,9 @@ mod tests {
             &session,
             &turn_context,
             session.conversation_id,
-        );
+            true,
+        )
+        .expect("login shells should be allowed");
 
         // ExecParams cannot derive Eq due to the CancellationToken field, so we manually compare the fields.
         assert_eq!(exec_params.command, expected_command);
@@ -464,18 +488,57 @@ mod tests {
             shell_snapshot,
         };
 
-        let login_command =
-            ShellCommandHandler::base_command(&shell, "echo login shell", Some(true));
+        let login_command = ShellCommandHandler::base_command(&shell, "echo login shell", true);
         assert_eq!(
             login_command,
             shell.derive_exec_args("echo login shell", true)
         );
 
         let non_login_command =
-            ShellCommandHandler::base_command(&shell, "echo non login shell", Some(false));
+            ShellCommandHandler::base_command(&shell, "echo non login shell", false);
         assert_eq!(
             non_login_command,
             shell.derive_exec_args("echo non login shell", false)
         );
     }
+
+    #[tokio::test]
+    async fn shell_command_handler_defaults_to_non_login_when_disallowed() {
+        let (session, turn_context) = make_session_and_context().await;
+        let params = ShellCommandToolCallParams {
+            command: "echo hello".to_string(),
+            workdir: None,
+            login: None,
+            timeout_ms: None,
+            sandbox_permissions: None,
+            prefix_rule: None,
+            justification: None,
+        };
+
+        let exec_params = ShellCommandHandler::to_exec_params(
+            &params,
+            &session,
+            &turn_context,
+            session.conversation_id,
+            false,
+        )
+        .expect("non-login shells should still be allowed");
+
+        assert_eq!(
+            exec_params.command,
+            session.user_shell().derive_exec_args("echo hello", false)
+        );
+    }
+
+    #[test]
+    fn shell_command_handler_rejects_login_when_disallowed() {
+        let err = ShellCommandHandler::resolve_use_login_shell(Some(true), false)
+            .expect_err("explicit login should be rejected");
+
+        assert!(
+            err.to_string()
+                .contains("login shell is disabled by config"),
+            "unexpected error: {err}"
+        );
+    }
 }

codex-rs/core/src/tools/handlers/unified_exec.rs

@@ -32,8 +32,8 @@ pub(crate) struct ExecCommandArgs {
     pub(crate) workdir: Option<String>,
     #[serde(default)]
     shell: Option<String>,
-    #[serde(default = "default_login")]
-    login: bool,
+    #[serde(default)]
+    login: Option<bool>,
     #[serde(default = "default_tty")]
     tty: bool,
     #[serde(default = "default_exec_yield_time_ms")]
@@ -68,10 +68,6 @@ fn default_write_stdin_yield_time_ms() -> u64 {
     250
 }
 
-fn default_login() -> bool {
-    true
-}
-
 fn default_tty() -> bool {
     false
 }
@@ -98,7 +94,14 @@ impl ToolHandler for UnifiedExecHandler {
         let Ok(params) = serde_json::from_str::<ExecCommandArgs>(arguments) else {
             return true;
         };
-        let command = get_command(&params, invocation.session.user_shell());
+        let command = match get_command(
+            &params,
+            invocation.session.user_shell(),
+            invocation.turn.tools_config.allow_login_shell,
+        ) {
+            Ok(command) => command,
+            Err(_) => return true,
+        };
         !is_known_safe_command(&command)
     }
 
@@ -129,7 +132,12 @@ impl ToolHandler for UnifiedExecHandler {
             "exec_command" => {
                 let args: ExecCommandArgs = parse_arguments(&arguments)?;
                 let process_id = manager.allocate_process_id().await;
-                let command = get_command(&args, session.user_shell());
+                let command = get_command(
+                    &args,
+                    session.user_shell(),
+                    turn.tools_config.allow_login_shell,
+                )
+                .map_err(FunctionCallError::RespondToModel)?;
 
                 let ExecCommandArgs {
                     workdir,
@@ -238,7 +246,11 @@ impl ToolHandler for UnifiedExecHandler {
     }
 }
 
-pub(crate) fn get_command(args: &ExecCommandArgs, session_shell: Arc<Shell>) -> Vec<String> {
+pub(crate) fn get_command(
+    args: &ExecCommandArgs,
+    session_shell: Arc<Shell>,
+    allow_login_shell: bool,
+) -> Result<Vec<String>, String> {
     let model_shell = args.shell.as_ref().map(|shell_str| {
         let mut shell = get_shell_by_model_provided_path(&PathBuf::from(shell_str));
         shell.shell_snapshot = crate::shell::empty_shell_snapshot_receiver();
@@ -246,8 +258,17 @@ pub(crate) fn get_command(args: &ExecCommandArgs, session_shell: Arc<Shell>) ->
     });
 
     let shell = model_shell.as_ref().unwrap_or(session_shell.as_ref());
+    let use_login_shell = match args.login {
+        Some(true) if !allow_login_shell => {
+            return Err(
+                "login shell is disabled by config; omit `login` or set it to false.".to_string(),
+            );
+        }
+        Some(use_login_shell) => use_login_shell,
+        None => allow_login_shell,
+    };
 
-    shell.derive_exec_args(&args.cmd, args.login)
+    Ok(shell.derive_exec_args(&args.cmd, use_login_shell))
 }
 
 fn format_response(response: &UnifiedExecResponse) -> String {
@@ -283,6 +304,7 @@ fn format_response(response: &UnifiedExecResponse) -> String {
 mod tests {
     use super::*;
     use crate::shell::default_user_shell;
+    use pretty_assertions::assert_eq;
     use std::sync::Arc;
 
     #[test]
@@ -293,7 +315,8 @@ mod tests {
 
         assert!(args.shell.is_none());
 
-        let command = get_command(&args, Arc::new(default_user_shell()));
+        let command =
+            get_command(&args, Arc::new(default_user_shell()), true).map_err(anyhow::Error::msg)?;
 
         assert_eq!(command.len(), 3);
         assert_eq!(command[2], "echo hello");
@@ -308,7 +331,8 @@ mod tests {
 
         assert_eq!(args.shell.as_deref(), Some("/bin/bash"));
 
-        let command = get_command(&args, Arc::new(default_user_shell()));
+        let command =
+            get_command(&args, Arc::new(default_user_shell()), true).map_err(anyhow::Error::msg)?;
 
         assert_eq!(command.last(), Some(&"echo hello".to_string()));
         if command
@@ -328,7 +352,8 @@ mod tests {
 
         assert_eq!(args.shell.as_deref(), Some("powershell"));
 
-        let command = get_command(&args, Arc::new(default_user_shell()));
+        let command =
+            get_command(&args, Arc::new(default_user_shell()), true).map_err(anyhow::Error::msg)?;
 
         assert_eq!(command[2], "echo hello");
         Ok(())
@@ -342,9 +367,25 @@ mod tests {
 
         assert_eq!(args.shell.as_deref(), Some("cmd"));
 
-        let command = get_command(&args, Arc::new(default_user_shell()));
+        let command =
+            get_command(&args, Arc::new(default_user_shell()), true).map_err(anyhow::Error::msg)?;
 
         assert_eq!(command[2], "echo hello");
         Ok(())
     }
+
+    #[test]
+    fn test_get_command_rejects_explicit_login_when_disallowed() -> anyhow::Result<()> {
+        let json = r#"{"cmd": "echo hello", "login": true}"#;
+
+        let args: ExecCommandArgs = parse_arguments(json)?;
+        let err = get_command(&args, Arc::new(default_user_shell()), false)
+            .expect_err("explicit login should be rejected");
+
+        assert!(
+            err.contains("login shell is disabled by config"),
+            "unexpected error: {err}"
+        );
+        Ok(())
+    }
 }

codex-rs/core/src/tools/spec.rs

@@ -35,6 +35,7 @@ const SEARCH_TOOL_BM25_DESCRIPTION_TEMPLATE: &str =
 #[derive(Debug, Clone)]
 pub(crate) struct ToolsConfig {
     pub shell_type: ConfigShellToolType,
+    pub allow_login_shell: bool,
     pub apply_patch_tool_type: Option<ApplyPatchToolType>,
     pub web_search_mode: Option<WebSearchMode>,
     pub agent_roles: BTreeMap<String, AgentRoleConfig>,
@@ -96,6 +97,7 @@ impl ToolsConfig {
 
         Self {
             shell_type,
+            allow_login_shell: true,
             apply_patch_tool_type,
             web_search_mode: *web_search_mode,
             agent_roles: BTreeMap::new(),
@@ -112,6 +114,11 @@ impl ToolsConfig {
         self.agent_roles = agent_roles;
         self
     }
+
+    pub fn with_allow_login_shell(mut self, allow_login_shell: bool) -> Self {
+        self.allow_login_shell = allow_login_shell;
+        self
+    }
 }
 
 /// Generic JSON‑Schema subset needed for our tool definitions
@@ -211,7 +218,7 @@ fn create_approval_parameters() -> BTreeMap<String, JsonSchema> {
     properties
 }
 
-fn create_exec_command_tool() -> ToolSpec {
+fn create_exec_command_tool(allow_login_shell: bool) -> ToolSpec {
     let mut properties = BTreeMap::from([
         (
             "cmd".to_string(),
@@ -234,14 +241,6 @@ fn create_exec_command_tool() -> ToolSpec {
                 description: Some("Shell binary to launch. Defaults to the user's default shell.".to_string()),
             },
         ),
-        (
-            "login".to_string(),
-            JsonSchema::Boolean {
-                description: Some(
-                    "Whether to run the shell with -l/-i semantics. Defaults to true.".to_string(),
-                ),
-            },
-        ),
         (
             "tty".to_string(),
             JsonSchema::Boolean {
@@ -269,6 +268,16 @@ fn create_exec_command_tool() -> ToolSpec {
             },
         ),
     ]);
+    if allow_login_shell {
+        properties.insert(
+            "login".to_string(),
+            JsonSchema::Boolean {
+                description: Some(
+                    "Whether to run the shell with -l/-i semantics. Defaults to true.".to_string(),
+                ),
+            },
+        );
+    }
     properties.extend(create_approval_parameters());
 
     ToolSpec::Function(ResponsesApiTool {
@@ -385,7 +394,7 @@ Examples of valid command strings:
     })
 }
 
-fn create_shell_command_tool() -> ToolSpec {
+fn create_shell_command_tool(allow_login_shell: bool) -> ToolSpec {
     let mut properties = BTreeMap::from([
         (
             "command".to_string(),
@@ -402,6 +411,14 @@ fn create_shell_command_tool() -> ToolSpec {
             },
         ),
         (
+            "timeout_ms".to_string(),
+            JsonSchema::Number {
+                description: Some("The timeout for the command in milliseconds".to_string()),
+            },
+        ),
+    ]);
+    if allow_login_shell {
+        properties.insert(
             "login".to_string(),
             JsonSchema::Boolean {
                 description: Some(
@@ -409,14 +426,8 @@ fn create_shell_command_tool() -> ToolSpec {
                         .to_string(),
                 ),
             },
-        ),
-        (
-            "timeout_ms".to_string(),
-            JsonSchema::Number {
-                description: Some("The timeout for the command in milliseconds".to_string()),
-            },
-        ),
-    ]);
+        );
+    }
     properties.extend(create_approval_parameters());
 
     let description = if cfg!(windows) {
@@ -1462,7 +1473,10 @@ pub(crate) fn build_specs(
             builder.push_spec_with_parallel_support(ToolSpec::LocalShell {}, true);
         }
         ConfigShellToolType::UnifiedExec => {
-            builder.push_spec_with_parallel_support(create_exec_command_tool(), true);
+            builder.push_spec_with_parallel_support(
+                create_exec_command_tool(config.allow_login_shell),
+                true,
+            );
             builder.push_spec(create_write_stdin_tool());
             builder.register_handler("exec_command", unified_exec_handler.clone());
             builder.register_handler("write_stdin", unified_exec_handler);
@@ -1471,7 +1485,10 @@ pub(crate) fn build_specs(
             // Do nothing.
         }
         ConfigShellToolType::ShellCommand => {
-            builder.push_spec_with_parallel_support(create_shell_command_tool(), true);
+            builder.push_spec_with_parallel_support(
+                create_shell_command_tool(config.allow_login_shell),
+                true,
+            );
         }
     }
 
@@ -1816,7 +1833,7 @@ mod tests {
         // Build expected from the same helpers used by the builder.
         let mut expected: BTreeMap<String, ToolSpec> = BTreeMap::from([]);
         for spec in [
-            create_exec_command_tool(),
+            create_exec_command_tool(true),
             create_write_stdin_tool(),
             PLAN_TOOL.clone(),
             create_request_user_input_tool(),
@@ -2790,7 +2807,7 @@ Examples of valid command strings:
 
     #[test]
     fn test_shell_command_tool() {
-        let tool = super::create_shell_command_tool();
+        let tool = super::create_shell_command_tool(true);
         let ToolSpec::Function(ResponsesApiTool {
             description, name, ..
         }) = &tool