Skip to content
Merged
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
17 changes: 17 additions & 0 deletions codex-rs/core/src/codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,7 @@ impl TurnContext {
sandbox_policy: self.sandbox_policy.get(),
windows_sandbox_level: self.windows_sandbox_level,
})
.with_unified_exec_shell_mode(self.tools_config.unified_exec_shell_mode.clone())
.with_web_search_config(self.tools_config.web_search_config.clone())
.with_allow_login_shell(self.tools_config.allow_login_shell)
.with_agent_roles(config.agent_roles.clone());
Expand Down Expand Up @@ -1261,6 +1262,9 @@ impl Session {
session_telemetry: &SessionTelemetry,
provider: ModelProviderInfo,
session_configuration: &SessionConfiguration,
user_shell: &shell::Shell,
shell_zsh_path: Option<&PathBuf>,
main_execve_wrapper_exe: Option<&PathBuf>,
per_turn_config: Config,
model_info: ModelInfo,
models_manager: &ModelsManager,
Expand Down Expand Up @@ -1292,6 +1296,11 @@ impl Session {
sandbox_policy: session_configuration.sandbox_policy.get(),
windows_sandbox_level: session_configuration.windows_sandbox_level,
})
.with_unified_exec_shell_mode_for_session(
user_shell,
shell_zsh_path,
main_execve_wrapper_exe,
)
.with_web_search_config(per_turn_config.web_search_config.clone())
.with_allow_login_shell(per_turn_config.permissions.allow_login_shell)
.with_agent_roles(per_turn_config.agent_roles.clone());
Expand Down Expand Up @@ -2358,6 +2367,9 @@ impl Session {
&self.services.session_telemetry,
session_configuration.provider.clone(),
&session_configuration,
self.services.user_shell.as_ref(),
self.services.shell_zsh_path.as_ref(),
self.services.main_execve_wrapper_exe.as_ref(),
per_turn_config,
model_info,
&self.services.models_manager,
Expand Down Expand Up @@ -5269,6 +5281,11 @@ async fn spawn_review_thread(
sandbox_policy: parent_turn_context.sandbox_policy.get(),
windows_sandbox_level: parent_turn_context.windows_sandbox_level,
})
.with_unified_exec_shell_mode_for_session(
sess.services.user_shell.as_ref(),
sess.services.shell_zsh_path.as_ref(),
sess.services.main_execve_wrapper_exe.as_ref(),
)
.with_web_search_config(None)
.with_allow_login_shell(config.permissions.allow_login_shell)
.with_agent_roles(config.agent_roles.clone());
Expand Down
6 changes: 6 additions & 0 deletions codex-rs/core/src/codex_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2178,6 +2178,9 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
&session_telemetry,
session_configuration.provider.clone(),
&session_configuration,
services.user_shell.as_ref(),
services.shell_zsh_path.as_ref(),
services.main_execve_wrapper_exe.as_ref(),
per_turn_config,
model_info,
&models_manager,
Expand Down Expand Up @@ -2850,6 +2853,9 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
&session_telemetry,
session_configuration.provider.clone(),
&session_configuration,
services.user_shell.as_ref(),
services.shell_zsh_path.as_ref(),
services.main_execve_wrapper_exe.as_ref(),
per_turn_config,
model_info,
&models_manager,
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/memories/usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ fn shell_command_for_invocation(invocation: &ToolInvocation) -> Option<(Vec<Stri
let command = crate::tools::handlers::unified_exec::get_command(
&params,
invocation.session.user_shell(),
&invocation.turn.tools_config.unified_exec_shell_mode,
invocation.turn.tools_config.allow_login_shell,
)
.ok()?;
Expand Down
28 changes: 20 additions & 8 deletions codex-rs/core/src/tools/handlers/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::tools::handlers::parse_arguments_with_base_path;
use crate::tools::handlers::resolve_workdir_base_path;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
use crate::tools::spec::UnifiedExecShellMode;
use crate::unified_exec::ExecCommandRequest;
use crate::unified_exec::UnifiedExecContext;
use crate::unified_exec::UnifiedExecProcessManager;
Expand Down Expand Up @@ -107,6 +108,7 @@ impl ToolHandler for UnifiedExecHandler {
let command = match get_command(
&params,
invocation.session.user_shell(),
&invocation.turn.tools_config.unified_exec_shell_mode,
invocation.turn.tools_config.allow_login_shell,
) {
Ok(command) => command,
Expand Down Expand Up @@ -154,6 +156,7 @@ impl ToolHandler for UnifiedExecHandler {
let command = get_command(
&args,
session.user_shell(),
&turn.tools_config.unified_exec_shell_mode,
turn.tools_config.allow_login_shell,
)
.map_err(FunctionCallError::RespondToModel)?;
Expand Down Expand Up @@ -323,15 +326,9 @@ impl ToolHandler for UnifiedExecHandler {
pub(crate) fn get_command(
args: &ExecCommandArgs,
session_shell: Arc<Shell>,
shell_mode: &UnifiedExecShellMode,
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();
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(
Expand All @@ -342,7 +339,22 @@ pub(crate) fn get_command(
None => allow_login_shell,
};

Ok(shell.derive_exec_args(&args.cmd, use_login_shell))
match shell_mode {
UnifiedExecShellMode::Direct => {
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();
shell
});
let shell = model_shell.as_ref().unwrap_or(session_shell.as_ref());
Ok(shell.derive_exec_args(&args.cmd, use_login_shell))
}
UnifiedExecShellMode::ZshFork(zsh_fork_config) => Ok(vec![
zsh_fork_config.shell_zsh_path.to_string_lossy().to_string(),
if use_login_shell { "-lc" } else { "-c" }.to_string(),
args.cmd.clone(),
]),
}
}

#[cfg(test)]
Expand Down
78 changes: 68 additions & 10 deletions codex-rs/core/src/tools/handlers/unified_exec_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::*;
use crate::shell::default_user_shell;
use crate::tools::handlers::parse_arguments_with_base_path;
use crate::tools::handlers::resolve_workdir_base_path;
use crate::tools::spec::ZshForkConfig;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
Expand All @@ -18,8 +19,13 @@ fn test_get_command_uses_default_shell_when_unspecified() -> anyhow::Result<()>

assert!(args.shell.is_none());

let command =
get_command(&args, Arc::new(default_user_shell()), true).map_err(anyhow::Error::msg)?;
let command = get_command(
&args,
Arc::new(default_user_shell()),
&UnifiedExecShellMode::Direct,
true,
)
.map_err(anyhow::Error::msg)?;

assert_eq!(command.len(), 3);
assert_eq!(command[2], "echo hello");
Expand All @@ -34,8 +40,13 @@ fn test_get_command_respects_explicit_bash_shell() -> anyhow::Result<()> {

assert_eq!(args.shell.as_deref(), Some("/bin/bash"));

let command =
get_command(&args, Arc::new(default_user_shell()), true).map_err(anyhow::Error::msg)?;
let command = get_command(
&args,
Arc::new(default_user_shell()),
&UnifiedExecShellMode::Direct,
true,
)
.map_err(anyhow::Error::msg)?;

assert_eq!(command.last(), Some(&"echo hello".to_string()));
if command
Expand All @@ -55,8 +66,13 @@ fn test_get_command_respects_explicit_powershell_shell() -> anyhow::Result<()> {

assert_eq!(args.shell.as_deref(), Some("powershell"));

let command =
get_command(&args, Arc::new(default_user_shell()), true).map_err(anyhow::Error::msg)?;
let command = get_command(
&args,
Arc::new(default_user_shell()),
&UnifiedExecShellMode::Direct,
true,
)
.map_err(anyhow::Error::msg)?;

assert_eq!(command[2], "echo hello");
Ok(())
Expand All @@ -70,8 +86,13 @@ fn test_get_command_respects_explicit_cmd_shell() -> anyhow::Result<()> {

assert_eq!(args.shell.as_deref(), Some("cmd"));

let command =
get_command(&args, Arc::new(default_user_shell()), true).map_err(anyhow::Error::msg)?;
let command = get_command(
&args,
Arc::new(default_user_shell()),
&UnifiedExecShellMode::Direct,
true,
)
.map_err(anyhow::Error::msg)?;

assert_eq!(command[2], "echo hello");
Ok(())
Expand All @@ -82,8 +103,13 @@ 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");
let err = get_command(
&args,
Arc::new(default_user_shell()),
&UnifiedExecShellMode::Direct,
false,
)
.expect_err("explicit login should be rejected");

assert!(
err.contains("login shell is disabled by config"),
Expand All @@ -92,6 +118,38 @@ fn test_get_command_rejects_explicit_login_when_disallowed() -> anyhow::Result<(
Ok(())
}

#[test]
fn test_get_command_ignores_explicit_shell_in_zsh_fork_mode() -> anyhow::Result<()> {
let json = r#"{"cmd": "echo hello", "shell": "/bin/bash"}"#;
let args: ExecCommandArgs = parse_arguments(json)?;
let shell_zsh_path = AbsolutePathBuf::from_absolute_path(if cfg!(windows) {
r"C:\opt\codex\zsh"
} else {
"/opt/codex/zsh"
})?;
let shell_mode = UnifiedExecShellMode::ZshFork(ZshForkConfig {
shell_zsh_path: shell_zsh_path.clone(),
main_execve_wrapper_exe: AbsolutePathBuf::from_absolute_path(if cfg!(windows) {
r"C:\opt\codex\codex-execve-wrapper"
} else {
"/opt/codex/codex-execve-wrapper"
})?,
});

let command = get_command(&args, Arc::new(default_user_shell()), &shell_mode, true)
.map_err(anyhow::Error::msg)?;

assert_eq!(
command,
vec![
shell_zsh_path.to_string_lossy().to_string(),
"-lc".to_string(),
"echo hello".to_string()
]
);
Ok(())
}

#[test]
fn exec_command_args_resolve_relative_additional_permissions_against_workdir() -> anyhow::Result<()>
{
Expand Down
29 changes: 4 additions & 25 deletions codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,20 +226,9 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
_attempt: &SandboxAttempt<'_>,
ctx: &ToolCtx,
exec_request: ExecRequest,
shell_zsh_path: &std::path::Path,
main_execve_wrapper_exe: &std::path::Path,
) -> Result<Option<PreparedUnifiedExecZshFork>, ToolError> {
let Some(shell_zsh_path) = ctx.session.services.shell_zsh_path.as_ref() else {
tracing::warn!("ZshFork backend specified, but shell_zsh_path is not configured.");
return Ok(None);
};
if !ctx.session.features().enabled(Feature::ShellZshFork) {
tracing::warn!("ZshFork backend specified, but ShellZshFork feature is not enabled.");
return Ok(None);
}
if !matches!(ctx.session.user_shell().shell_type, ShellType::Zsh) {
tracing::warn!("ZshFork backend specified, but user shell is not Zsh.");
return Ok(None);
}

let parsed = match extract_shell_script(&exec_request.command) {
Ok(parsed) => parsed,
Err(err) => {
Expand Down Expand Up @@ -282,16 +271,6 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
codex_linux_sandbox_exe: ctx.turn.codex_linux_sandbox_exe.clone(),
use_legacy_landlock: ctx.turn.features.use_legacy_landlock(),
};
let main_execve_wrapper_exe = ctx
.session
.services
.main_execve_wrapper_exe
.clone()
.ok_or_else(|| {
ToolError::Rejected(
"zsh fork feature enabled, but execve wrapper is not configured".to_string(),
)
})?;
let escalation_policy = CoreShellActionProvider {
policy: Arc::clone(&exec_policy),
session: Arc::clone(&ctx.session),
Expand All @@ -312,8 +291,8 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
};

let escalate_server = EscalateServer::new(
shell_zsh_path.clone(),
main_execve_wrapper_exe,
shell_zsh_path.to_path_buf(),
main_execve_wrapper_exe.to_path_buf(),
escalation_policy,
);
let escalation_session = escalate_server
Expand Down
19 changes: 15 additions & 4 deletions codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::tools::runtimes::unified_exec::UnifiedExecRequest;
use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::ToolCtx;
use crate::tools::sandboxing::ToolError;
use crate::tools::spec::ZshForkConfig;
use crate::unified_exec::SpawnLifecycleHandle;

pub(crate) struct PreparedUnifiedExecSpawn {
Expand Down Expand Up @@ -37,8 +38,9 @@ pub(crate) async fn maybe_prepare_unified_exec(
attempt: &SandboxAttempt<'_>,
ctx: &ToolCtx,
exec_request: ExecRequest,
zsh_fork_config: &ZshForkConfig,
) -> Result<Option<PreparedUnifiedExecSpawn>, ToolError> {
imp::maybe_prepare_unified_exec(req, attempt, ctx, exec_request).await
imp::maybe_prepare_unified_exec(req, attempt, ctx, exec_request, zsh_fork_config).await
}

#[cfg(unix)]
Expand Down Expand Up @@ -83,9 +85,17 @@ mod imp {
attempt: &SandboxAttempt<'_>,
ctx: &ToolCtx,
exec_request: ExecRequest,
zsh_fork_config: &ZshForkConfig,
) -> Result<Option<PreparedUnifiedExecSpawn>, ToolError> {
let Some(prepared) =
unix_escalation::prepare_unified_exec_zsh_fork(req, attempt, ctx, exec_request).await?
let Some(prepared) = unix_escalation::prepare_unified_exec_zsh_fork(
req,
attempt,
ctx,
exec_request,
zsh_fork_config.shell_zsh_path.as_path(),
zsh_fork_config.main_execve_wrapper_exe.as_path(),
)
.await?
else {
return Ok(None);
};
Expand Down Expand Up @@ -118,8 +128,9 @@ mod imp {
attempt: &SandboxAttempt<'_>,
ctx: &ToolCtx,
exec_request: ExecRequest,
zsh_fork_config: &ZshForkConfig,
) -> Result<Option<PreparedUnifiedExecSpawn>, ToolError> {
let _ = (req, attempt, ctx, exec_request);
let _ = (req, attempt, ctx, exec_request, zsh_fork_config);
Ok(None)
}
}
Loading
Loading