mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
fix: enforce sandbox envelope for zsh fork execution (#12800)
## Why Zsh fork execution was still able to bypass the `WorkspaceWrite` model in edge cases because the fork path reconstructed command execution without preserving sandbox wrappers, and command extraction only accepted shell invocations in a narrow positional shape. This can allow commands to run with broader filesystem access than expected, which breaks the sandbox safety model. ## What changed - Preserved the sandboxed `ExecRequest` produced by `attempt.env_for(...)` when entering the zsh fork path in [`unix_escalation.rs`](https://github.com/openai/codex/blob/main/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs). - Updated `CoreShellCommandExecutor` to execute the sandboxed command and working directory captured from `attempt.env_for(...)`, instead of re-running a freshly reconstructed shell command. - Made zsh-fork script extraction robust to wrapped invocations by scanning command arguments for `-c`/`-lc` rather than only matching the first positional form. - Added unit tests in `unix_escalation.rs` to lock in wrapper-tolerant parsing behavior and keep unsupported shell forms rejected. - Tightened the regression in [`skill_approval.rs`](https://github.com/openai/codex/blob/main/codex-rs/core/tests/suite/skill_approval.rs): - `shell_zsh_fork_still_enforces_workspace_write_sandbox` now uses an explicit `WorkspaceWrite` policy with `exclude_tmpdir_env_var: true` and `exclude_slash_tmp: true`. - The test attempts to write to `/tmp/...`, which is only reliably outside writable roots with those explicit exclusions set. ## Verification - Added and passed the new unit tests around `extract_shell_script` parsing behavior with wrapped command shapes. - `extract_shell_script_supports_wrapped_command_prefixes` - `extract_shell_script_rejects_unsupported_shell_invocation` - Verified the regression with the focused integration test: `shell_zsh_fork_still_enforces_workspace_write_sandbox`. ## Manual Testing Prior to this change, if I ran Codex via: ``` just codex --config zsh_path=/Users/mbolin/code/codex2/codex-rs/app-server/tests/suite/zsh --enable shell_zsh_fork ``` and asked: ``` what is the output of /bin/ps ``` it would run it, even though the default sandbox should prevent the agent from running `/bin/ps` because it is setuid on MacOS. But with this change, I now see the expected failure because it is blocked by the sandbox: ``` /bin/ps exited with status 1 and produced no output in this environment. ```
This commit is contained in:
@@ -72,14 +72,9 @@ pub(super) async fn try_run_zsh_fork(
|
||||
let sandbox_exec_request = attempt
|
||||
.env_for(spec, req.network.as_ref())
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
// Keep env/network/sandbox metadata from `attempt.env_for()`, but build the
|
||||
// script from the original shell argv. `attempt.env_for()` may wrap the
|
||||
// command with `sandbox-exec` on macOS, and passing those wrapper flags
|
||||
// (`-p`, `-D...`) through zsh breaks the zsh-fork path before subcommand
|
||||
// approval runs.
|
||||
let crate::sandboxing::ExecRequest {
|
||||
command: _sandbox_command,
|
||||
cwd: _sandbox_cwd,
|
||||
command,
|
||||
cwd: sandbox_cwd,
|
||||
env: sandbox_env,
|
||||
network: sandbox_network,
|
||||
expiration: _sandbox_expiration,
|
||||
@@ -90,7 +85,7 @@ pub(super) async fn try_run_zsh_fork(
|
||||
justification,
|
||||
arg0,
|
||||
} = sandbox_exec_request;
|
||||
let ParsedShellCommand { script, login } = extract_shell_script(command)?;
|
||||
let ParsedShellCommand { script, login } = extract_shell_script(&command)?;
|
||||
let effective_timeout = Duration::from_millis(
|
||||
req.timeout_ms
|
||||
.unwrap_or(crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS),
|
||||
@@ -99,6 +94,8 @@ pub(super) async fn try_run_zsh_fork(
|
||||
ctx.session.services.exec_policy.current().as_ref().clone(),
|
||||
));
|
||||
let command_executor = CoreShellCommandExecutor {
|
||||
command,
|
||||
cwd: sandbox_cwd,
|
||||
sandbox_policy,
|
||||
sandbox,
|
||||
env: sandbox_env,
|
||||
@@ -438,6 +435,8 @@ impl EscalationPolicy for CoreShellActionProvider {
|
||||
}
|
||||
|
||||
struct CoreShellCommandExecutor {
|
||||
command: Vec<String>,
|
||||
cwd: PathBuf,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
sandbox: SandboxType,
|
||||
env: HashMap<String, String>,
|
||||
@@ -452,8 +451,8 @@ struct CoreShellCommandExecutor {
|
||||
impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
async fn run(
|
||||
&self,
|
||||
command: Vec<String>,
|
||||
cwd: PathBuf,
|
||||
_command: Vec<String>,
|
||||
_cwd: PathBuf,
|
||||
env: HashMap<String, String>,
|
||||
cancel_rx: CancellationToken,
|
||||
) -> anyhow::Result<ExecResult> {
|
||||
@@ -466,8 +465,8 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
|
||||
let result = crate::sandboxing::execute_env(
|
||||
crate::sandboxing::ExecRequest {
|
||||
command,
|
||||
cwd,
|
||||
command: self.command.clone(),
|
||||
cwd: self.cwd.clone(),
|
||||
env: exec_env,
|
||||
network: self.network.clone(),
|
||||
expiration: ExecExpiration::Cancellation(cancel_rx),
|
||||
@@ -500,19 +499,20 @@ struct ParsedShellCommand {
|
||||
}
|
||||
|
||||
fn extract_shell_script(command: &[String]) -> Result<ParsedShellCommand, ToolError> {
|
||||
match command {
|
||||
[_, flag, script, ..] if flag == "-c" => Ok(ParsedShellCommand {
|
||||
script: script.clone(),
|
||||
login: false,
|
||||
}),
|
||||
[_, flag, script, ..] if flag == "-lc" => Ok(ParsedShellCommand {
|
||||
script: script.clone(),
|
||||
login: true,
|
||||
}),
|
||||
_ => Err(ToolError::Rejected(
|
||||
"unexpected shell command format for zsh-fork execution".to_string(),
|
||||
)),
|
||||
// Commands reaching zsh-fork can be wrapped by environment/sandbox helpers, so
|
||||
// we search for the first `-c`/`-lc` triple anywhere in the argv rather
|
||||
// than assuming it is the first positional form.
|
||||
if let Some((script, login)) = command.windows(3).find_map(|parts| match parts {
|
||||
[_, flag, script] if flag == "-c" => Some((script.to_owned(), false)),
|
||||
[_, flag, script] if flag == "-lc" => Some((script.to_owned(), true)),
|
||||
_ => None,
|
||||
}) {
|
||||
return Ok(ParsedShellCommand { script, login });
|
||||
}
|
||||
|
||||
Err(ToolError::Rejected(
|
||||
"unexpected shell command format for zsh-fork execution".to_string(),
|
||||
))
|
||||
}
|
||||
|
||||
fn map_exec_result(
|
||||
@@ -586,6 +586,58 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_shell_script_supports_wrapped_command_prefixes() {
|
||||
assert_eq!(
|
||||
extract_shell_script(&[
|
||||
"/usr/bin/env".into(),
|
||||
"CODEX_EXECVE_WRAPPER=1".into(),
|
||||
"/bin/zsh".into(),
|
||||
"-lc".into(),
|
||||
"echo hello".into()
|
||||
])
|
||||
.unwrap(),
|
||||
ParsedShellCommand {
|
||||
script: "echo hello".to_string(),
|
||||
login: true,
|
||||
}
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
extract_shell_script(&[
|
||||
"sandbox-exec".into(),
|
||||
"-p".into(),
|
||||
"sandbox_policy".into(),
|
||||
"/bin/zsh".into(),
|
||||
"-c".into(),
|
||||
"pwd".into(),
|
||||
])
|
||||
.unwrap(),
|
||||
ParsedShellCommand {
|
||||
script: "pwd".to_string(),
|
||||
login: false,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_shell_script_rejects_unsupported_shell_invocation() {
|
||||
let err = extract_shell_script(&[
|
||||
"sandbox-exec".into(),
|
||||
"-fc".into(),
|
||||
"echo not supported".into(),
|
||||
])
|
||||
.unwrap_err();
|
||||
assert!(matches!(err, super::ToolError::Rejected(_)));
|
||||
assert_eq!(
|
||||
match err {
|
||||
super::ToolError::Rejected(reason) => reason,
|
||||
_ => "".to_string(),
|
||||
},
|
||||
"unexpected shell command format for zsh-fork execution"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn join_program_and_argv_replaces_original_argv_zero() {
|
||||
assert_eq!(
|
||||
|
||||
Reference in New Issue
Block a user