Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions codex-rs/core/src/shell.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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\"";
Expand Down
3 changes: 1 addition & 2 deletions codex-rs/core/src/tools/handlers/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(&params.command, use_login_shell);
let command = shell.build_command(&params.command, None);

ExecParams {
command,
Expand Down
47 changes: 40 additions & 7 deletions codex-rs/core/src/tools/handlers/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);

Expand All @@ -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"}"#;
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/tests/common/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/tests/suite/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
Loading