mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
feat: add allow list to disable login shell
This commit is contained in:
@@ -1,7 +1,16 @@
|
||||
use crate::bash::parse_shell_lc_plain_commands;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
/// Plain commands (no redirects/subshell/etc.) that are safe to run without
|
||||
/// login-shell initialization.
|
||||
const NON_LOGIN_SHELL_ALLOWLIST: &[&str] = &[
|
||||
"cat", "cd", "echo", "false", "find", "grep", "head", "ls", "nl", "pwd", "rm",
|
||||
"rmdir", "sed", "tail", "true", "wc",
|
||||
];
|
||||
|
||||
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
|
||||
pub enum ShellType {
|
||||
Zsh,
|
||||
@@ -58,6 +67,32 @@ impl Shell {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Builds shell exec args for `script`, defaulting to login-shell semantics
|
||||
/// unless we detect a safe, allowlisted script. An explicit `login` flag
|
||||
/// overrides the default.
|
||||
pub fn build_command(&self, script: &str, login: Option<bool>) -> Vec<String> {
|
||||
let use_login_shell = login.unwrap_or_else(|| match self.shell_type {
|
||||
ShellType::PowerShell | ShellType::Cmd => true,
|
||||
ShellType::Zsh | ShellType::Bash | ShellType::Sh => {
|
||||
!uses_allowlisted_plain_commands(&self.derive_exec_args(script, true))
|
||||
}
|
||||
});
|
||||
self.derive_exec_args(script, use_login_shell)
|
||||
}
|
||||
}
|
||||
|
||||
fn uses_allowlisted_plain_commands(command: &[String]) -> bool {
|
||||
parse_shell_lc_plain_commands(command)
|
||||
.filter(|commands| !commands.is_empty())
|
||||
.is_some_and(|commands| {
|
||||
commands.iter().all(|cmd| {
|
||||
cmd.first()
|
||||
.and_then(|cmd0| Path::new(cmd0).file_name())
|
||||
.and_then(|name| name.to_str())
|
||||
.is_some_and(|cmd0| NON_LOGIN_SHELL_ALLOWLIST.contains(&cmd0))
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
@@ -374,6 +409,41 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allowlisted_bash_script_skips_login_shell() {
|
||||
let shell = Shell {
|
||||
shell_type: ShellType::Bash,
|
||||
shell_path: PathBuf::from("/bin/bash"),
|
||||
};
|
||||
let command = shell.build_command("cd foo && ls", None);
|
||||
assert_eq!(command[1], "-c");
|
||||
let command = shell.build_command("cd foo && cargo build", None);
|
||||
assert_eq!(command[1], "-lc");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_allowlisted_bash_script_keeps_login_shell() {
|
||||
let shell = Shell {
|
||||
shell_type: ShellType::Bash,
|
||||
shell_path: PathBuf::from("/bin/bash"),
|
||||
};
|
||||
let command = shell.build_command("python -c 'print(1)'", None);
|
||||
assert_eq!(command[1], "-lc");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_login_flag_is_respected() {
|
||||
let shell = Shell {
|
||||
shell_type: ShellType::Bash,
|
||||
shell_path: PathBuf::from("/bin/bash"),
|
||||
};
|
||||
let login_command = shell.build_command("echo hello", Some(true));
|
||||
assert_eq!(login_command[1], "-lc");
|
||||
|
||||
let non_login_command = shell.build_command("echo hello", Some(false));
|
||||
assert_eq!(non_login_command[1], "-c");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn can_run_on_shell_test() {
|
||||
let cmd = "echo \"Works\"";
|
||||
|
||||
@@ -49,8 +49,7 @@ impl ShellCommandHandler {
|
||||
turn_context: &TurnContext,
|
||||
) -> ExecParams {
|
||||
let shell = session.user_shell();
|
||||
let use_login_shell = true;
|
||||
let command = shell.derive_exec_args(¶ms.command, use_login_shell);
|
||||
let command = shell.build_command(¶ms.command, None);
|
||||
|
||||
ExecParams {
|
||||
command,
|
||||
|
||||
@@ -34,8 +34,8 @@ struct ExecCommandArgs {
|
||||
workdir: Option<String>,
|
||||
#[serde(default)]
|
||||
shell: Option<String>,
|
||||
#[serde(default = "default_login")]
|
||||
login: bool,
|
||||
#[serde(default)]
|
||||
login: Option<bool>,
|
||||
#[serde(default = "default_exec_yield_time_ms")]
|
||||
yield_time_ms: u64,
|
||||
#[serde(default)]
|
||||
@@ -66,10 +66,6 @@ fn default_write_stdin_yield_time_ms() -> u64 {
|
||||
250
|
||||
}
|
||||
|
||||
fn default_login() -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl ToolHandler for UnifiedExecHandler {
|
||||
fn kind(&self) -> ToolKind {
|
||||
@@ -260,7 +256,7 @@ fn get_command(args: &ExecCommandArgs) -> Vec<String> {
|
||||
default_user_shell()
|
||||
};
|
||||
|
||||
shell.derive_exec_args(&args.cmd, args.login)
|
||||
shell.build_command(&args.cmd, args.login)
|
||||
}
|
||||
|
||||
fn format_response(response: &UnifiedExecResponse) -> String {
|
||||
@@ -304,6 +300,7 @@ mod tests {
|
||||
serde_json::from_str(json).expect("deserialize ExecCommandArgs");
|
||||
|
||||
assert!(args.shell.is_none());
|
||||
assert!(args.login.is_none());
|
||||
|
||||
let command = get_command(&args);
|
||||
|
||||
@@ -325,6 +322,42 @@ mod tests {
|
||||
assert_eq!(command[2], "echo hello");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_get_command_defaults_to_non_login_shell_for_allowlisted_commands() {
|
||||
let json = r#"{"cmd": "echo hello", "shell": "/bin/bash"}"#;
|
||||
|
||||
let args: ExecCommandArgs =
|
||||
serde_json::from_str(json).expect("deserialize ExecCommandArgs");
|
||||
|
||||
let command = get_command(&args);
|
||||
|
||||
assert_eq!(command[1], "-c");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_get_command_keeps_login_shell_for_non_allowlisted_commands() {
|
||||
let json = r#"{"cmd": "python -c 'print(1)'", "shell": "/bin/bash"}"#;
|
||||
|
||||
let args: ExecCommandArgs =
|
||||
serde_json::from_str(json).expect("deserialize ExecCommandArgs");
|
||||
|
||||
let command = get_command(&args);
|
||||
|
||||
assert_eq!(command[1], "-lc");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_get_command_respects_explicit_login_flag() {
|
||||
let json = r#"{"cmd": "echo hello", "shell": "/bin/bash", "login": true}"#;
|
||||
|
||||
let args: ExecCommandArgs =
|
||||
serde_json::from_str(json).expect("deserialize ExecCommandArgs");
|
||||
|
||||
let command = get_command(&args);
|
||||
|
||||
assert_eq!(command[1], "-lc");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_get_command_respects_explicit_powershell_shell() {
|
||||
let json = r#"{"cmd": "echo hello", "shell": "powershell"}"#;
|
||||
|
||||
@@ -173,7 +173,7 @@ pub fn sandbox_network_env_var() -> &'static str {
|
||||
}
|
||||
|
||||
pub fn format_with_current_shell(command: &str) -> Vec<String> {
|
||||
codex_core::shell::default_user_shell().derive_exec_args(command, true)
|
||||
codex_core::shell::default_user_shell().build_command(command, None)
|
||||
}
|
||||
|
||||
pub fn format_with_current_shell_display(command: &str) -> String {
|
||||
|
||||
@@ -337,7 +337,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_command(&begin_event.command, "-lc", "/bin/echo hello unified exec");
|
||||
assert_command(&begin_event.command, "-c", "/bin/echo hello unified exec");
|
||||
|
||||
assert_eq!(begin_event.cwd, cwd.path());
|
||||
|
||||
|
||||
Reference in New Issue
Block a user