Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions codex-rs/core/src/codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ use crate::config::Config;
use crate::config::Constrained;
use crate::config::ConstraintResult;
use crate::config::GhostSnapshotConfig;
use crate::config::types::ShellConfig;
use crate::config::types::ShellEnvironmentPolicy;
use crate::context_manager::ContextManager;
use crate::environment_context::EnvironmentContext;
Expand Down Expand Up @@ -382,6 +383,7 @@ pub(crate) struct TurnContext {
pub(crate) approval_policy: AskForApproval,
pub(crate) sandbox_policy: SandboxPolicy,
pub(crate) shell_environment_policy: ShellEnvironmentPolicy,
pub(crate) shell: ShellConfig,
pub(crate) tools_config: ToolsConfig,
pub(crate) ghost_snapshot: GhostSnapshotConfig,
pub(crate) final_output_json_schema: Option<Value>,
Expand Down Expand Up @@ -540,6 +542,7 @@ impl Session {
approval_policy: session_configuration.approval_policy.value(),
sandbox_policy: session_configuration.sandbox_policy.get().clone(),
shell_environment_policy: per_turn_config.shell_environment_policy.clone(),
shell: per_turn_config.shell.clone(),
tools_config,
ghost_snapshot: per_turn_config.ghost_snapshot.clone(),
final_output_json_schema: None,
Expand Down Expand Up @@ -2266,6 +2269,7 @@ async fn spawn_review_thread(
approval_policy: parent_turn_context.approval_policy,
sandbox_policy: parent_turn_context.sandbox_policy.clone(),
shell_environment_policy: parent_turn_context.shell_environment_policy.clone(),
shell: parent_turn_context.shell.clone(),
cwd: parent_turn_context.cwd.clone(),
final_output_json_schema: None,
codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(),
Expand Down
49 changes: 49 additions & 0 deletions codex-rs/core/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::config::types::OtelConfigToml;
use crate::config::types::OtelExporterKind;
use crate::config::types::SandboxWorkspaceWrite;
use crate::config::types::ScrollInputMode;
use crate::config::types::ShellConfig;
use crate::config::types::ShellEnvironmentPolicy;
use crate::config::types::ShellEnvironmentPolicyToml;
use crate::config::types::Tui;
Expand Down Expand Up @@ -130,6 +131,8 @@ pub struct Config {
pub forced_auto_mode_downgraded_on_windows: bool,

pub shell_environment_policy: ShellEnvironmentPolicy,
/// Shell execution defaults applied when tools do not specify overrides.
pub shell: ShellConfig,

/// When `true`, `AgentReasoning` events emitted by the backend will be
/// suppressed from the frontend output. This can reduce visual noise when
Expand Down Expand Up @@ -694,6 +697,9 @@ pub struct ConfigToml {

#[serde(default)]
pub shell_environment_policy: ShellEnvironmentPolicyToml,
/// Default shell configuration applied when tools do not override it.
#[serde(default)]
pub shell: ShellConfig,

/// Sandbox mode to use.
pub sandbox_mode: Option<SandboxMode>,
Expand Down Expand Up @@ -1227,6 +1233,7 @@ impl Config {
.clone();

let shell_environment_policy = cfg.shell_environment_policy.into();
let shell = cfg.shell;

let history = cfg.history.unwrap_or_default();

Expand Down Expand Up @@ -1340,6 +1347,7 @@ impl Config {
did_user_set_custom_approval_policy_or_sandbox_mode,
forced_auto_mode_downgraded_on_windows,
shell_environment_policy,
shell,
notify: cfg.notify,
user_instructions,
base_instructions,
Expand Down Expand Up @@ -1918,6 +1926,43 @@ trust_level = "trusted"
Ok(())
}

#[test]
fn shell_defaults_to_login_shell() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let cfg = ConfigToml::default();

let config = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
)?;

assert!(config.shell.default_login);

Ok(())
}

#[test]
fn shell_respects_configured_default_login() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let cfg = ConfigToml {
shell: ShellConfig {
default_login: false,
},
..Default::default()
};

let config = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
)?;

assert!(!config.shell.default_login);

Ok(())
}

#[test]
fn profile_legacy_toggles_override_base() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
Expand Down Expand Up @@ -3225,6 +3270,7 @@ model_verbosity = "high"
did_user_set_custom_approval_policy_or_sandbox_mode: true,
forced_auto_mode_downgraded_on_windows: false,
shell_environment_policy: ShellEnvironmentPolicy::default(),
shell: ShellConfig::default(),
user_instructions: None,
notify: None,
cwd: fixture.cwd(),
Expand Down Expand Up @@ -3310,6 +3356,7 @@ model_verbosity = "high"
did_user_set_custom_approval_policy_or_sandbox_mode: true,
forced_auto_mode_downgraded_on_windows: false,
shell_environment_policy: ShellEnvironmentPolicy::default(),
shell: ShellConfig::default(),
user_instructions: None,
notify: None,
cwd: fixture.cwd(),
Expand Down Expand Up @@ -3410,6 +3457,7 @@ model_verbosity = "high"
did_user_set_custom_approval_policy_or_sandbox_mode: true,
forced_auto_mode_downgraded_on_windows: false,
shell_environment_policy: ShellEnvironmentPolicy::default(),
shell: ShellConfig::default(),
user_instructions: None,
notify: None,
cwd: fixture.cwd(),
Expand Down Expand Up @@ -3496,6 +3544,7 @@ model_verbosity = "high"
did_user_set_custom_approval_policy_or_sandbox_mode: true,
forced_auto_mode_downgraded_on_windows: false,
shell_environment_policy: ShellEnvironmentPolicy::default(),
shell: ShellConfig::default(),
user_instructions: None,
notify: None,
cwd: fixture.cwd(),
Expand Down
18 changes: 18 additions & 0 deletions codex-rs/core/src/config/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,24 @@ impl Notice {
pub(crate) const TABLE_KEY: &'static str = "notice";
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
pub struct ShellConfig {
#[serde(default = "default_login_shell")]
pub default_login: bool,
}

const fn default_login_shell() -> bool {
true
}

impl Default for ShellConfig {
fn default() -> Self {
Self {
default_login: true,
}
}
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default)]
pub struct SandboxWorkspaceWrite {
#[serde(default)]
Expand Down
27 changes: 21 additions & 6 deletions codex-rs/core/src/tools/handlers/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,13 @@ 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 base_command(
shell: &Shell,
command: &str,
login: Option<bool>,
default_login: bool,
) -> Vec<String> {
let use_login_shell = login.unwrap_or(default_login);
shell.derive_exec_args(command, use_login_shell)
}

Expand All @@ -54,7 +59,12 @@ impl ShellCommandHandler {
turn_context: &TurnContext,
) -> ExecParams {
let shell = 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,
params.login,
turn_context.shell.default_login,
);

ExecParams {
command,
Expand Down Expand Up @@ -156,7 +166,12 @@ impl ToolHandler for ShellCommandHandler {
serde_json::from_str::<ShellCommandToolCallParams>(arguments)
.map(|params| {
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,
params.login,
invocation.turn.shell.default_login,
);
!is_known_safe_command(&command)
})
.unwrap_or(true)
Expand Down Expand Up @@ -400,14 +415,14 @@ mod tests {
};

let login_command =
ShellCommandHandler::base_command(&shell, "echo login shell", Some(true));
ShellCommandHandler::base_command(&shell, "echo login shell", Some(true), 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", Some(false), true);
assert_eq!(
non_login_command,
shell.derive_exec_args("echo non login shell", false)
Expand Down
79 changes: 59 additions & 20 deletions codex-rs/core/src/tools/handlers/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,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 @@ -63,10 +63,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 All @@ -89,7 +85,11 @@ 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 = get_command(
&params,
invocation.session.user_shell(),
invocation.turn.shell.default_login,
);
!is_known_safe_command(&command)
}

Expand Down Expand Up @@ -120,8 +120,7 @@ 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.shell.default_login);
let ExecCommandArgs {
workdir,
yield_time_ms,
Expand Down Expand Up @@ -225,16 +224,32 @@ impl ToolHandler for UnifiedExecHandler {
}
}

fn get_command(args: &ExecCommandArgs, session_shell: Arc<Shell>) -> Vec<String> {
let model_shell = args.shell.as_ref().map(|shell_str| {
fn resolve_login_choice(
requested_login: Option<bool>,
default_login: bool,
has_snapshot: bool,
) -> bool {
requested_login.unwrap_or(if has_snapshot { true } else { default_login })
}

fn get_command(
args: &ExecCommandArgs,
session_shell: Arc<Shell>,
default_login: bool,
) -> Vec<String> {
if let Some(shell_str) = &args.shell {
let mut shell = get_shell_by_model_provided_path(&PathBuf::from(shell_str));
shell.shell_snapshot = None;
shell
});

let shell = model_shell.as_ref().unwrap_or(session_shell.as_ref());
let use_login_shell = resolve_login_choice(args.login, default_login, false);
return shell.derive_exec_args(&args.cmd, use_login_shell);
}

shell.derive_exec_args(&args.cmd, args.login)
let use_login_shell = resolve_login_choice(
args.login,
default_login,
session_shell.shell_snapshot.is_some(),
);
session_shell.derive_exec_args(&args.cmd, use_login_shell)
}

fn format_response(response: &UnifiedExecResponse) -> String {
Expand Down Expand Up @@ -270,7 +285,11 @@ fn format_response(response: &UnifiedExecResponse) -> String {
mod tests {
use super::*;
use crate::shell::default_user_shell;
#[cfg(not(windows))]
use crate::shell_snapshot::ShellSnapshot;
use std::sync::Arc;
#[cfg(not(windows))]
use tempfile::TempDir;

#[test]
fn test_get_command_uses_default_shell_when_unspecified() -> anyhow::Result<()> {
Expand All @@ -280,7 +299,7 @@ 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);

assert_eq!(command.len(), 3);
assert_eq!(command[2], "echo hello");
Expand All @@ -295,7 +314,7 @@ 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);

assert_eq!(command.last(), Some(&"echo hello".to_string()));
if command
Expand All @@ -315,7 +334,7 @@ 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);

assert_eq!(command[2], "echo hello");
Ok(())
Expand All @@ -329,9 +348,29 @@ 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);

assert_eq!(command[2], "echo hello");
Ok(())
}

#[test]
#[cfg(not(windows))]
fn test_get_command_prefers_login_for_snapshot_defaults() {
let json = r#"{"cmd": "echo hello"}"#;

let args: ExecCommandArgs =
serde_json::from_str(json).expect("deserialize ExecCommandArgs");
let temp_dir = TempDir::new().expect("create temp dir");
let snapshot_path = temp_dir.path().join("snapshot.sh");
std::fs::write(&snapshot_path, "").expect("write snapshot");

let mut shell = default_user_shell();
shell.shell_snapshot = Some(Arc::new(ShellSnapshot {
path: snapshot_path,
}));

let command = get_command(&args, Arc::new(shell), false);
assert_eq!(command[1], "-lc");
}
}
Loading