mirror of
https://github.com/openai/codex.git
synced 2026-04-30 03:12:20 +03:00
core: adopt host_executable() rules in zsh-fork (#13046)
## Why [#12964](https://github.com/openai/codex/pull/12964) added `host_executable()` support to `codex-execpolicy`, but the zsh-fork interception path in `unix_escalation.rs` was still evaluating commands with the default exact-token matcher. That meant an intercepted absolute executable such as `/usr/bin/git status` could still miss basename rules like `prefix_rule(pattern = ["git", "status"])`, even when the policy also defined a matching `host_executable(name = "git", ...)` entry. This PR adopts the new matching behavior in the zsh-fork runtime only. That keeps the rollout intentionally narrow: zsh-fork already requires explicit user opt-in, so it is a safer first caller to exercise the new `host_executable()` scheme before expanding it to other execpolicy call sites. It also brings zsh-fork back in line with the current `prefix_rule()` execution model. Until prefix rules can carry their own permission profiles, a matched `prefix_rule()` is expected to rerun the intercepted command unsandboxed on `allow`, or after the user accepts `prompt`, instead of merely continuing inside the inherited shell sandbox. ## What Changed - added `evaluate_intercepted_exec_policy()` in `core/src/tools/runtimes/shell/unix_escalation.rs` to centralize execpolicy evaluation for intercepted commands - switched intercepted direct execs in the zsh-fork path to `check_multiple_with_options(...)` with `MatchOptions { resolve_host_executables: true }` - added `commands_for_intercepted_exec_policy()` so zsh-fork policy evaluation works from intercepted `(program, argv)` data instead of reconstructing a synthetic command before matching - left shell-wrapper parsing intentionally disabled by default behind `ENABLE_INTERCEPTED_EXEC_POLICY_SHELL_WRAPPER_PARSING`, so path-sensitive matching relies on later direct exec interception rather than shell-script parsing - made matched `prefix_rule()` decisions rerun intercepted commands with `EscalationExecution::Unsandboxed`, while unmatched-command fallback keeps the existing sandbox-preserving behavior - extracted the zsh-fork test harness into `core/tests/common/zsh_fork.rs` so both the skill-focused and approval-focused integration suites can exercise the same runtime setup - limited this change to the intercepted zsh-fork path rather than changing every execpolicy caller at once - added runtime coverage in `core/src/tools/runtimes/shell/unix_escalation_tests.rs` for allowed and disallowed `host_executable()` mappings and the wrapper-parsing modes - added integration coverage in `core/tests/suite/approvals.rs` to verify a saved `prefix_rule(pattern=["touch"], decision="allow")` reruns under zsh-fork outside a restrictive `WorkspaceWrite` sandbox --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13046). * #13065 * __->__ #13046
This commit is contained in:
@@ -21,6 +21,7 @@ pub mod responses;
|
||||
pub mod streaming_sse;
|
||||
pub mod test_codex;
|
||||
pub mod test_codex_exec;
|
||||
pub mod zsh_fork;
|
||||
|
||||
#[ctor]
|
||||
fn enable_deterministic_unified_exec_process_ids_for_tests() {
|
||||
|
||||
118
codex-rs/core/tests/common/zsh_fork.rs
Normal file
118
codex-rs/core/tests/common/zsh_fork.rs
Normal file
@@ -0,0 +1,118 @@
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::Constrained;
|
||||
use codex_core::features::Feature;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
|
||||
use crate::test_codex::TestCodex;
|
||||
use crate::test_codex::test_codex;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct ZshForkRuntime {
|
||||
zsh_path: PathBuf,
|
||||
main_execve_wrapper_exe: PathBuf,
|
||||
}
|
||||
|
||||
impl ZshForkRuntime {
|
||||
fn apply_to_config(
|
||||
&self,
|
||||
config: &mut Config,
|
||||
approval_policy: AskForApproval,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
) {
|
||||
config.features.enable(Feature::ShellTool);
|
||||
config.features.enable(Feature::ShellZshFork);
|
||||
config.zsh_path = Some(self.zsh_path.clone());
|
||||
config.main_execve_wrapper_exe = Some(self.main_execve_wrapper_exe.clone());
|
||||
config.permissions.allow_login_shell = false;
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy);
|
||||
}
|
||||
}
|
||||
|
||||
pub fn restrictive_workspace_write_policy() -> SandboxPolicy {
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: Vec::new(),
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn zsh_fork_runtime(test_name: &str) -> Result<Option<ZshForkRuntime>> {
|
||||
let Some(zsh_path) = find_test_zsh_path()? else {
|
||||
return Ok(None);
|
||||
};
|
||||
if !supports_exec_wrapper_intercept(&zsh_path) {
|
||||
eprintln!(
|
||||
"skipping {test_name}: zsh does not support EXEC_WRAPPER intercepts ({})",
|
||||
zsh_path.display()
|
||||
);
|
||||
return Ok(None);
|
||||
}
|
||||
let Ok(main_execve_wrapper_exe) = codex_utils_cargo_bin::cargo_bin("codex-execve-wrapper")
|
||||
else {
|
||||
eprintln!("skipping {test_name}: unable to resolve `codex-execve-wrapper` binary");
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
Ok(Some(ZshForkRuntime {
|
||||
zsh_path,
|
||||
main_execve_wrapper_exe,
|
||||
}))
|
||||
}
|
||||
|
||||
pub async fn build_zsh_fork_test<F>(
|
||||
server: &wiremock::MockServer,
|
||||
runtime: ZshForkRuntime,
|
||||
approval_policy: AskForApproval,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
pre_build_hook: F,
|
||||
) -> Result<TestCodex>
|
||||
where
|
||||
F: FnOnce(&Path) + Send + 'static,
|
||||
{
|
||||
let mut builder = test_codex()
|
||||
.with_pre_build_hook(pre_build_hook)
|
||||
.with_config(move |config| {
|
||||
runtime.apply_to_config(config, approval_policy, sandbox_policy);
|
||||
});
|
||||
builder.build(server).await
|
||||
}
|
||||
|
||||
fn find_test_zsh_path() -> Result<Option<PathBuf>> {
|
||||
let repo_root = codex_utils_cargo_bin::repo_root()?;
|
||||
let dotslash_zsh = repo_root.join("codex-rs/app-server/tests/suite/zsh");
|
||||
if !dotslash_zsh.is_file() {
|
||||
eprintln!(
|
||||
"skipping zsh-fork test: shared zsh DotSlash file not found at {}",
|
||||
dotslash_zsh.display()
|
||||
);
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
match crate::fetch_dotslash_file(&dotslash_zsh, None) {
|
||||
Ok(path) => Ok(Some(path)),
|
||||
Err(error) => {
|
||||
eprintln!("skipping zsh-fork test: failed to fetch zsh via dotslash: {error:#}");
|
||||
Ok(None)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn supports_exec_wrapper_intercept(zsh_path: &Path) -> bool {
|
||||
let status = std::process::Command::new(zsh_path)
|
||||
.arg("-fc")
|
||||
.arg("/usr/bin/true")
|
||||
.env("EXEC_WRAPPER", "/usr/bin/false")
|
||||
.status();
|
||||
match status {
|
||||
Ok(status) => !status.success(),
|
||||
Err(_) => false,
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user