mirror of
https://github.com/openai/codex.git
synced 2026-09-14 11:57:03 +00:00
fix: zsh-shell-fork was not honoring sandbox
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,17 @@ 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(),
|
||||
)),
|
||||
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(
|
||||
|
||||
@@ -558,3 +558,91 @@ permissions:
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn shell_zsh_fork_still_enforces_workspace_write_sandbox() -> Result<()> {
|
||||
use codex_config::Constrained;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let Some(zsh_path) = find_test_zsh_path()? else {
|
||||
return Ok(());
|
||||
};
|
||||
if !supports_exec_wrapper_intercept(&zsh_path) {
|
||||
eprintln!(
|
||||
"skipping zsh-fork sandbox test: zsh does not support EXEC_WRAPPER intercepts ({})",
|
||||
zsh_path.display()
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
let Ok(main_execve_wrapper_exe) = codex_utils_cargo_bin::cargo_bin("codex-execve-wrapper")
|
||||
else {
|
||||
eprintln!(
|
||||
"skipping zsh-fork sandbox test: unable to resolve `codex-execve-wrapper` binary"
|
||||
);
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let tool_call_id = "zsh-fork-workspace-write-deny";
|
||||
let outside_path = "/tmp/codex-zsh-fork-workspace-write-deny.txt";
|
||||
let workspace_write_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: Vec::new(),
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
let policy_for_config = workspace_write_policy.clone();
|
||||
let _ = fs::remove_file(outside_path);
|
||||
let mut builder = test_codex()
|
||||
.with_pre_build_hook(move |_| {
|
||||
let _ = fs::remove_file(outside_path);
|
||||
})
|
||||
.with_config(move |config| {
|
||||
config.features.enable(Feature::ShellTool);
|
||||
config.features.enable(Feature::ShellZshFork);
|
||||
config.zsh_path = Some(zsh_path.clone());
|
||||
config.main_execve_wrapper_exe = Some(main_execve_wrapper_exe);
|
||||
config.permissions.allow_login_shell = false;
|
||||
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never);
|
||||
config.permissions.sandbox_policy = Constrained::allow_any(policy_for_config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let command = format!("touch {outside_path}");
|
||||
let arguments = shell_command_arguments(&command)?;
|
||||
let mocks =
|
||||
mount_function_call_agent_response(&server, tool_call_id, &arguments, "shell_command")
|
||||
.await;
|
||||
|
||||
submit_turn_with_policies(
|
||||
&test,
|
||||
"write outside workspace with zsh fork",
|
||||
AskForApproval::Never,
|
||||
workspace_write_policy,
|
||||
)
|
||||
.await?;
|
||||
|
||||
wait_for_turn_complete_without_skill_approval(&test).await;
|
||||
|
||||
let call_output = mocks
|
||||
.completion
|
||||
.single_request()
|
||||
.function_call_output(tool_call_id);
|
||||
let output = call_output["output"].as_str().unwrap_or_default();
|
||||
assert!(
|
||||
output.contains("Permission denied")
|
||||
|| output.contains("Operation not permitted")
|
||||
|| output.contains("Read-only file system"),
|
||||
"expected sandbox denial, got output: {output:?}"
|
||||
);
|
||||
assert!(
|
||||
!Path::new(outside_path).exists(),
|
||||
"command should not write outside workspace under WorkspaceWrite policy"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user