From 4369a27938f1719d5f0e0b1cd8329d347063ff54 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 27 Feb 2026 13:04:11 -0800 Subject: [PATCH 1/2] core: adopt host_executable() rules in zsh-fork --- .../tools/runtimes/shell/unix_escalation.rs | 141 +++++++++--- .../runtimes/shell/unix_escalation_tests.rs | 208 +++++++++++++++++- 2 files changed, 317 insertions(+), 32 deletions(-) diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index ae1df34ece..5288fd8e82 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -16,6 +16,8 @@ use crate::tools::sandboxing::SandboxablePreference; use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use codex_execpolicy::Decision; +use codex_execpolicy::Evaluation; +use codex_execpolicy::MatchOptions; use codex_execpolicy::Policy; use codex_execpolicy::RuleMatch; use codex_protocol::config_types::WindowsSandboxLevel; @@ -431,6 +433,12 @@ impl CoreShellActionProvider { } } +// Shell-wrapper parsing is weaker than direct exec interception because it can +// only see the script text, not the final resolved executable path. Keep it +// disabled by default so path-sensitive rules rely on the later authoritative +// execve interception. +const ENABLE_INTERCEPTED_EXEC_POLICY_SHELL_WRAPPER_PARSING: bool = false; + #[async_trait::async_trait] impl EscalationPolicy for CoreShellActionProvider { async fn determine_action( @@ -493,28 +501,17 @@ impl EscalationPolicy for CoreShellActionProvider { .await; } - let command = join_program_and_argv(program, argv); - let (commands, used_complex_parsing) = - if let Some(commands) = parse_shell_lc_plain_commands(&command) { - (commands, false) - } else if let Some(single_command) = parse_shell_lc_single_command_prefix(&command) { - (vec![single_command], true) - } else { - (vec![command.clone()], false) - }; - - let fallback = |cmd: &[String]| { - crate::exec_policy::render_decision_for_unmatched_command( - self.approval_policy, - &self.sandbox_policy, - cmd, - self.sandbox_permissions, - used_complex_parsing, - ) - }; let evaluation = { let policy = self.policy.read().await; - policy.check_multiple(commands.iter(), &fallback) + evaluate_intercepted_exec_policy( + &policy, + program, + argv, + self.approval_policy, + &self.sandbox_policy, + self.sandbox_permissions, + ENABLE_INTERCEPTED_EXEC_POLICY_SHELL_WRAPPER_PARSING, + ) }; // When true, means the Evaluation was due to *.rules, not the // fallback function. @@ -528,16 +525,20 @@ impl EscalationPolicy for CoreShellActionProvider { } else { DecisionSource::UnmatchedCommandFallback }; - let escalation_execution = Self::shell_request_escalation_execution( - self.sandbox_permissions, - &self.sandbox_policy, - self.prompt_permissions.as_ref(), - self.turn - .config - .permissions - .macos_seatbelt_profile_extensions - .as_ref(), - ); + let escalation_execution = match decision_source { + DecisionSource::PrefixRule => EscalationExecution::Unsandboxed, + DecisionSource::UnmatchedCommandFallback => Self::shell_request_escalation_execution( + self.sandbox_permissions, + &self.sandbox_policy, + self.prompt_permissions.as_ref(), + self.turn + .config + .permissions + .macos_seatbelt_profile_extensions + .as_ref(), + ), + DecisionSource::SkillScript { .. } => unreachable!("handled above"), + }; self.process_decision( evaluation.decision, needs_escalation, @@ -552,6 +553,86 @@ impl EscalationPolicy for CoreShellActionProvider { } } +fn evaluate_intercepted_exec_policy( + policy: &Policy, + program: &AbsolutePathBuf, + argv: &[String], + approval_policy: AskForApproval, + sandbox_policy: &SandboxPolicy, + sandbox_permissions: SandboxPermissions, + enable_intercepted_exec_policy_shell_wrapper_parsing: bool, +) -> Evaluation { + let CandidateCommands { + commands, + used_complex_parsing, + } = if enable_intercepted_exec_policy_shell_wrapper_parsing { + // In this codepath, the first argument in `commands` could be a bare + // name like `find` instead of an absolute path like `/usr/bin/find`. + // It could also be a shell built-in like `echo`. + commands_for_intercepted_exec_policy(program, argv) + } else { + // In this codepath, `commands` has a single entry where the program + // is always an absolute path. + CandidateCommands { + commands: vec![join_program_and_argv(program, argv)], + used_complex_parsing: false, + } + }; + + let fallback = |cmd: &[String]| { + crate::exec_policy::render_decision_for_unmatched_command( + approval_policy, + sandbox_policy, + cmd, + sandbox_permissions, + used_complex_parsing, + ) + }; + + policy.check_multiple_with_options( + commands.iter(), + &fallback, + &MatchOptions { + resolve_host_executables: true, + }, + ) +} + +struct CandidateCommands { + commands: Vec>, + used_complex_parsing: bool, +} + +fn commands_for_intercepted_exec_policy( + program: &AbsolutePathBuf, + argv: &[String], +) -> CandidateCommands { + if let [_, flag, script] = argv { + let shell_command = [ + program.to_string_lossy().to_string(), + flag.clone(), + script.clone(), + ]; + if let Some(commands) = parse_shell_lc_plain_commands(&shell_command) { + return CandidateCommands { + commands, + used_complex_parsing: false, + }; + } + if let Some(single_command) = parse_shell_lc_single_command_prefix(&shell_command) { + return CandidateCommands { + commands: vec![single_command], + used_complex_parsing: true, + }; + } + } + + CandidateCommands { + commands: vec![join_program_and_argv(program, argv)], + used_complex_parsing: false, + } +} + struct CoreShellCommandExecutor { command: Vec, cwd: PathBuf, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 519608a43b..b88f5b0542 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -2,6 +2,8 @@ use super::CoreShellActionProvider; #[cfg(target_os = "macos")] use super::CoreShellCommandExecutor; use super::ParsedShellCommand; +use super::commands_for_intercepted_exec_policy; +use super::evaluate_intercepted_exec_policy; use super::extract_shell_script; use super::join_program_and_argv; use super::map_exec_result; @@ -12,14 +14,16 @@ use crate::config::Permissions; #[cfg(target_os = "macos")] use crate::config::types::ShellEnvironmentPolicy; use crate::exec::SandboxType; -#[cfg(target_os = "macos")] use crate::protocol::AskForApproval; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; -#[cfg(target_os = "macos")] use crate::sandboxing::SandboxPermissions; #[cfg(target_os = "macos")] use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; +use codex_execpolicy::Decision; +use codex_execpolicy::Evaluation; +use codex_execpolicy::PolicyParser; +use codex_execpolicy::RuleMatch; #[cfg(target_os = "macos")] use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::FileSystemPermissions; @@ -36,8 +40,25 @@ use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; #[cfg(target_os = "macos")] use std::collections::HashMap; +use std::path::PathBuf; use std::time::Duration; +fn host_absolute_path(segments: &[&str]) -> String { + let mut path = if cfg!(windows) { + PathBuf::from(r"C:\") + } else { + PathBuf::from("/") + }; + for segment in segments { + path.push(segment); + } + path.to_string_lossy().into_owned() +} + +fn starlark_string(value: &str) -> String { + value.replace('\\', "\\\\").replace('"', "\\\"") +} + #[test] fn extract_shell_script_preserves_login_flag() { assert_eq!( @@ -126,6 +147,24 @@ fn join_program_and_argv_replaces_original_argv_zero() { ); } +#[test] +fn commands_for_intercepted_exec_policy_parses_plain_shell_wrappers() { + let program = AbsolutePathBuf::try_from(host_absolute_path(&["bin", "bash"])).unwrap(); + let candidate_commands = commands_for_intercepted_exec_policy( + &program, + &["not-bash".into(), "-lc".into(), "git status && pwd".into()], + ); + + assert_eq!( + candidate_commands.commands, + vec![ + vec!["git".to_string(), "status".to_string()], + vec!["pwd".to_string()], + ] + ); + assert!(!candidate_commands.used_complex_parsing); +} + #[test] fn map_exec_result_preserves_stdout_and_stderr() { let out = map_exec_result( @@ -203,6 +242,171 @@ fn shell_request_escalation_execution_is_explicit() { ); } +#[test] +fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_parsing_disabled() { + let policy_src = r#"prefix_rule(pattern = ["npm", "publish"], decision = "prompt")"#; + let mut parser = PolicyParser::new(); + parser.parse("test.rules", policy_src).unwrap(); + let policy = parser.build(); + let program = AbsolutePathBuf::try_from(host_absolute_path(&["bin", "zsh"])).unwrap(); + + let enable_intercepted_exec_policy_shell_wrapper_parsing = false; + let evaluation = evaluate_intercepted_exec_policy( + &policy, + &program, + &[ + "zsh".to_string(), + "-lc".to_string(), + "npm publish".to_string(), + ], + AskForApproval::OnRequest, + &SandboxPolicy::new_read_only_policy(), + SandboxPermissions::UseDefault, + enable_intercepted_exec_policy_shell_wrapper_parsing, + ); + + assert!( + matches!( + evaluation.matched_rules.as_slice(), + [RuleMatch::HeuristicsRuleMatch { command, decision: Decision::Allow }] + if command == &vec![ + program.to_string_lossy().to_string(), + "-lc".to_string(), + "npm publish".to_string(), + ] + ), + r#"This is allowed because when shell wrapper parsing is disabled, +the policy evaluation does not try to parse the shell command and instead +matches the whole command line with the resolved program path, which in this +case is `/bin/zsh` followed by some arguments. + +Because there is no policy rule for `/bin/zsh` or `zsh`, the decision is to +allow the command and let the sandbox be responsible for enforcing any +restrictions. + +That said, if /bin/zsh is the zsh-fork, then the execve wrapper should +ultimately intercept the `npm publish` command and apply the policy rules to it. +"# + ); +} + +#[test] +fn evaluate_intercepted_exec_policy_matches_inner_shell_commands_when_enabled() { + let policy_src = r#"prefix_rule(pattern = ["npm", "publish"], decision = "prompt")"#; + let mut parser = PolicyParser::new(); + parser.parse("test.rules", policy_src).unwrap(); + let policy = parser.build(); + let program = AbsolutePathBuf::try_from(host_absolute_path(&["bin", "bash"])).unwrap(); + + let enable_intercepted_exec_policy_shell_wrapper_parsing = true; + let evaluation = evaluate_intercepted_exec_policy( + &policy, + &program, + &[ + "bash".to_string(), + "-lc".to_string(), + "npm publish".to_string(), + ], + AskForApproval::OnRequest, + &SandboxPolicy::new_read_only_policy(), + SandboxPermissions::UseDefault, + enable_intercepted_exec_policy_shell_wrapper_parsing, + ); + + assert_eq!( + evaluation, + Evaluation { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["npm".to_string(), "publish".to_string()], + decision: Decision::Prompt, + resolved_program: None, + justification: None, + }], + } + ); +} + +#[test] +fn intercepted_exec_policy_uses_host_executable_mappings() { + let git_path = host_absolute_path(&["usr", "bin", "git"]); + let git_path_literal = starlark_string(&git_path); + let policy_src = format!( + r#" +prefix_rule(pattern = ["git", "status"], decision = "prompt") +host_executable(name = "git", paths = ["{git_path_literal}"]) +"# + ); + let mut parser = PolicyParser::new(); + parser.parse("test.rules", &policy_src).unwrap(); + let policy = parser.build(); + let program = AbsolutePathBuf::try_from(git_path).unwrap(); + + let evaluation = evaluate_intercepted_exec_policy( + &policy, + &program, + &["git".to_string(), "status".to_string()], + AskForApproval::OnRequest, + &SandboxPolicy::new_read_only_policy(), + SandboxPermissions::UseDefault, + false, + ); + + assert_eq!( + evaluation, + Evaluation { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["git".to_string(), "status".to_string()], + decision: Decision::Prompt, + resolved_program: Some(program), + justification: None, + }], + } + ); + assert!(CoreShellActionProvider::decision_driven_by_policy( + &evaluation.matched_rules, + evaluation.decision + )); +} + +#[test] +fn intercepted_exec_policy_rejects_disallowed_host_executable_mapping() { + let allowed_git = host_absolute_path(&["usr", "bin", "git"]); + let other_git = host_absolute_path(&["opt", "homebrew", "bin", "git"]); + let allowed_git_literal = starlark_string(&allowed_git); + let policy_src = format!( + r#" +prefix_rule(pattern = ["git", "status"], decision = "prompt") +host_executable(name = "git", paths = ["{allowed_git_literal}"]) +"# + ); + let mut parser = PolicyParser::new(); + parser.parse("test.rules", &policy_src).unwrap(); + let policy = parser.build(); + let program = AbsolutePathBuf::try_from(other_git.clone()).unwrap(); + + let evaluation = evaluate_intercepted_exec_policy( + &policy, + &program, + &["git".to_string(), "status".to_string()], + AskForApproval::OnRequest, + &SandboxPolicy::new_read_only_policy(), + SandboxPermissions::UseDefault, + false, + ); + + assert!(matches!( + evaluation.matched_rules.as_slice(), + [RuleMatch::HeuristicsRuleMatch { command, .. }] + if command == &vec![other_git, "status".to_string()] + )); + assert!(!CoreShellActionProvider::decision_driven_by_policy( + &evaluation.matched_rules, + evaluation.decision + )); +} + #[cfg(target_os = "macos")] #[tokio::test] async fn prepare_escalated_exec_turn_default_preserves_macos_seatbelt_extensions() { From 93f803fd66b8aace6cf3c40a7349a8c47933f8ee Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 27 Feb 2026 16:20:54 -0800 Subject: [PATCH 2/2] core: resolve host_executable() rules during preflight --- codex-rs/core/src/exec_policy.rs | 135 ++++++++++++++++++++++++++++++- 1 file changed, 133 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index b781f0bc7d..69a7cb933d 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -13,6 +13,7 @@ use codex_execpolicy::AmendError; use codex_execpolicy::Decision; use codex_execpolicy::Error as ExecPolicyRuleError; use codex_execpolicy::Evaluation; +use codex_execpolicy::MatchOptions; use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use codex_execpolicy::PolicyParser; @@ -221,7 +222,14 @@ impl ExecPolicyManager { used_complex_parsing, ) }; - let evaluation = exec_policy.check_multiple(commands.iter(), &exec_policy_fallback); + let match_options = MatchOptions { + resolve_host_executables: true, + }; + let evaluation = exec_policy.check_multiple_with_options( + commands.iter(), + &exec_policy_fallback, + &match_options, + ); let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule( prefix_rule.as_ref(), @@ -229,6 +237,7 @@ impl ExecPolicyManager { exec_policy.as_ref(), &commands, &exec_policy_fallback, + &match_options, ); match evaluation.decision { @@ -630,6 +639,7 @@ fn derive_requested_execpolicy_amendment_from_prefix_rule( exec_policy: &Policy, commands: &[Vec], exec_policy_fallback: &impl Fn(&[String]) -> Decision, + match_options: &MatchOptions, ) -> Option { let prefix_rule = prefix_rule?; if prefix_rule.is_empty() { @@ -656,6 +666,7 @@ fn derive_requested_execpolicy_amendment_from_prefix_rule( &amendment.command, commands, exec_policy_fallback, + match_options, ) { Some(amendment) } else { @@ -668,6 +679,7 @@ fn prefix_rule_would_approve_all_commands( prefix_rule: &[String], commands: &[Vec], exec_policy_fallback: &impl Fn(&[String]) -> Decision, + match_options: &MatchOptions, ) -> bool { let mut policy_with_prefix_rule = exec_policy.clone(); if policy_with_prefix_rule @@ -679,7 +691,7 @@ fn prefix_rule_would_approve_all_commands( commands.iter().all(|command| { policy_with_prefix_rule - .check(command, exec_policy_fallback) + .check_with_options(command, exec_policy_fallback, match_options) .decision == Decision::Allow }) @@ -849,6 +861,15 @@ mod tests { path.to_string_lossy().into_owned() } + fn host_program_path(name: &str) -> String { + let executable_name = if cfg!(windows) { + format!("{name}.exe") + } else { + name.to_string() + }; + host_absolute_path(&["usr", "bin", &executable_name]) + } + fn starlark_string(value: &str) -> String { value.replace('\\', "\\\\").replace('"', "\\\"") } @@ -1398,6 +1419,115 @@ prefix_rule( ); } + #[tokio::test] + async fn absolute_path_exec_approval_requirement_matches_host_executable_rules() { + let git_path = host_program_path("git"); + let git_path_literal = starlark_string(&git_path); + let policy_src = format!( + r#" +host_executable(name = "git", paths = ["{git_path_literal}"]) +prefix_rule(pattern=["git"], decision="allow") +"# + ); + let mut parser = PolicyParser::new(); + parser + .parse("test.rules", &policy_src) + .expect("parse policy"); + let manager = ExecPolicyManager::new(Arc::new(parser.build())); + let command = vec![git_path, "status".to_string()]; + + let requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::UnlessTrusted, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, + } + ); + } + + #[tokio::test] + async fn absolute_path_exec_approval_requirement_ignores_disallowed_host_executable_paths() { + let allowed_git_path = host_program_path("git"); + let disallowed_git_path = host_absolute_path(&[ + "opt", + "homebrew", + "bin", + if cfg!(windows) { "git.exe" } else { "git" }, + ]); + let allowed_git_path_literal = starlark_string(&allowed_git_path); + let policy_src = format!( + r#" +host_executable(name = "git", paths = ["{allowed_git_path_literal}"]) +prefix_rule(pattern=["git"], decision="prompt") +"# + ); + let mut parser = PolicyParser::new(); + parser + .parse("test.rules", &policy_src) + .expect("parse policy"); + let manager = ExecPolicyManager::new(Arc::new(parser.build())); + let command = vec![disallowed_git_path, "status".to_string()]; + + let requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::UnlessTrusted, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), + } + ); + } + + #[tokio::test] + async fn requested_prefix_rule_can_approve_absolute_path_commands() { + let command = vec![ + host_program_path("cargo"), + "install".to_string(), + "cargo-insta".to_string(), + ]; + let manager = ExecPolicyManager::default(); + + let requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::UnlessTrusted, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ + "cargo".to_string(), + "install".to_string(), + ])), + } + ); + } + #[tokio::test] async fn exec_approval_requirement_respects_approval_policy() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; @@ -1952,6 +2082,7 @@ prefix_rule( &Policy::empty(), &commands, &|_: &[String]| Decision::Allow, + &MatchOptions::default(), ) }