From 42a055cc3b4b1a924415d6aa02db51e19342fa1e Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Fri, 3 Jul 2026 04:25:30 -0700 Subject: [PATCH] Restore one-shot policy for inspected PowerShell wrappers --- codex-rs/core/src/exec_policy.rs | 250 +++++- codex-rs/core/src/exec_policy_powershell.rs | 34 +- .../core/src/exec_policy_powershell_tests.rs | 726 +++++++++++++++--- codex-rs/core/src/exec_policy_tests.rs | 108 ++- codex-rs/core/src/tools/sandboxing.rs | 7 - codex-rs/core/tests/suite/exec_policy.rs | 122 ++- 6 files changed, 1035 insertions(+), 212 deletions(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index d301474166..54afe95668 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -170,28 +170,49 @@ fn is_policy_match(rule_match: &RuleMatch) -> bool { } } +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +struct PromptCauses { + rules: bool, + sandbox: bool, +} + +fn prompt_causes(matched_rules: &[RuleMatch]) -> PromptCauses { + matched_rules + .iter() + .fold(PromptCauses::default(), |mut causes, rule_match| { + match rule_match { + RuleMatch::PrefixRuleMatch { + decision: Decision::Prompt, + .. + } => causes.rules = true, + RuleMatch::HeuristicsRuleMatch { + decision: Decision::Prompt, + .. + } => causes.sandbox = true, + RuleMatch::PrefixRuleMatch { .. } | RuleMatch::HeuristicsRuleMatch { .. } => {} + } + causes + }) +} + /// Returns a rejection reason when `approval_policy` disallows surfacing the /// current prompt to the user. /// -/// `prompt_is_rule` distinguishes policy-rule prompts from sandbox/escalation -/// prompts so granular `rules` and `sandbox_approval` settings are honored -/// independently. When both are present, policy-rule prompts take precedence. -pub(crate) fn prompt_is_rejected_by_policy( +/// `causes` retains every represented policy-rule and sandbox/escalation +/// category so granular settings are honored independently. When both are +/// present and disabled, policy-rule prompts take precedence. +fn prompt_is_rejected_by_policy( approval_policy: AskForApproval, - prompt_is_rule: bool, + causes: PromptCauses, ) -> Option<&'static str> { match approval_policy { AskForApproval::Never => Some(PROMPT_CONFLICT_REASON), AskForApproval::OnRequest => None, AskForApproval::UnlessTrusted => None, AskForApproval::Granular(granular_config) => { - if prompt_is_rule { - if !granular_config.allows_rules_approval() { - Some(REJECT_RULES_APPROVAL_REASON) - } else { - None - } - } else if !granular_config.allows_sandbox_approval() { + if causes.rules && !granular_config.allows_rules_approval() { + Some(REJECT_RULES_APPROVAL_REASON) + } else if causes.sandbox && !granular_config.allows_sandbox_approval() { Some(REJECT_SANDBOX_APPROVAL_REASON) } else { None @@ -290,7 +311,24 @@ impl ExecPolicyManager { Some(powershell_policy::PreparedPowerShell::Terminal(requirement)) => { return requirement; } - Some(powershell_policy::PreparedPowerShell::Parsed(parsed)) => (Some(parsed), true), + Some(powershell_policy::PreparedPowerShell::Parsed(parsed)) => { + if let Some(outer_argv) = parsed.untrusted_outer_argv() { + return create_untrusted_powershell_approval_requirement( + exec_policy.as_ref(), + outer_argv, + parsed.commands(), + UnmatchedCommandContext { + approval_policy, + permission_profile: &permission_profile, + windows_sandbox_level, + sandbox_permissions, + used_complex_parsing: false, + command_origin: ExecPolicyCommandOrigin::PowerShell, + }, + ); + } + (Some(parsed), true) + } Some(powershell_policy::PreparedPowerShell::Unsupported) => (None, true), None => (None, false), }; @@ -378,32 +416,28 @@ impl ExecPolicyManager { .max() .unwrap_or(Decision::Forbidden); - let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy(); - let managed_filesystem_restrictions = - profile_has_managed_filesystem_restrictions(&permission_profile); - let permission_delta_requires_outer = match (&permission_profile, sandbox_permissions) { - (PermissionProfile::Disabled, _) => false, - (_, SandboxPermissions::WithAdditionalPermissions) => true, - (PermissionProfile::External { .. }, SandboxPermissions::RequireEscalated) => true, - (PermissionProfile::Managed { .. }, SandboxPermissions::RequireEscalated) => { - unsandboxed_execution_allowed(&file_system_sandbox_policy) - } - (_, SandboxPermissions::UseDefault) => false, - }; + let permission_delta_requires_outer = + permission_delta_requires_outer_authority(&permission_profile, sandbox_permissions); let parsed_powershell_needs_outer_approval = parsed_powershell_outer.is_some() && !exact_outer_allow && (permission_delta_requires_outer - || (cfg!(windows) - && windows_sandbox_level == WindowsSandboxLevel::Disabled - && managed_filesystem_restrictions)); - if evaluation.decision == Decision::Allow && parsed_powershell_needs_outer_approval { - evaluation.decision = Decision::Prompt; + || missing_managed_windows_sandbox_backend( + &permission_profile, + windows_sandbox_level, + )); + if evaluation.decision != Decision::Forbidden && parsed_powershell_needs_outer_approval { evaluation .matched_rules .push(RuleMatch::HeuristicsRuleMatch { command: command.to_vec(), decision: Decision::Prompt, }); + evaluation.decision = evaluation + .matched_rules + .iter() + .map(RuleMatch::decision) + .max() + .unwrap_or(Decision::Forbidden); } let requested_amendment = if parsed_powershell_outer.is_some() { @@ -426,10 +460,8 @@ impl ExecPolicyManager { reason: derive_forbidden_reason(command, &evaluation), }, Decision::Prompt => { - let prompt_is_rule = evaluation.matched_rules.iter().any(|rule_match| { - is_policy_match(rule_match) && rule_match.decision() == Decision::Prompt - }); - match prompt_is_rejected_by_policy(approval_policy, prompt_is_rule) { + let causes = prompt_causes(&evaluation.matched_rules); + match prompt_is_rejected_by_policy(approval_policy, causes) { Some(reason) => ExecApprovalRequirement::Forbidden { reason: reason.to_string(), }, @@ -438,7 +470,7 @@ impl ExecPolicyManager { proposed_execpolicy_amendment: requested_amendment.or_else(|| { match ( parsed_powershell_outer, - prompt_is_rule, + causes.rules, auto_amendment_allowed, ) { (Some(outer), false, _) => { @@ -583,6 +615,130 @@ impl ExecPolicyManager { } } +#[cfg(windows)] +fn create_untrusted_powershell_approval_requirement( + exec_policy: &Policy, + outer_argv: &[String], + commands: &[Vec], + context: UnmatchedCommandContext<'_>, +) -> ExecApprovalRequirement { + let match_options = MatchOptions { + resolve_host_executables: true, + }; + let inner_fallback = + |command: &[String]| render_decision_for_unmatched_command(command, context); + + let mut matched_rules = Vec::new(); + let mut every_inner_explicit_allow = !commands.is_empty(); + for command in commands { + let inner_matches = exec_policy.matches_for_command_with_options( + command, + Some(&inner_fallback), + &match_options, + ); + every_inner_explicit_allow &= !command.is_empty() + && inner_matches.iter().any(|rule_match| { + matches!( + rule_match, + RuleMatch::PrefixRuleMatch { + decision: Decision::Allow, + .. + } + ) + }); + matched_rules.extend(inner_matches); + } + + let outer_fallback = |_command: &[String]| match context.approval_policy { + AskForApproval::Never => Decision::Forbidden, + AskForApproval::OnRequest | AskForApproval::UnlessTrusted | AskForApproval::Granular(_) => { + Decision::Prompt + } + }; + matched_rules.extend( + exec_policy + .matches_for_command_with_restrictive_host_rules(outer_argv, Some(&outer_fallback)), + ); + + let raw_match_options = MatchOptions { + resolve_host_executables: false, + }; + let raw_full_outer_allow = exec_policy + .matches_for_command_with_options( + outer_argv, + /*heuristics_fallback*/ None, + &raw_match_options, + ) + .iter() + .any(|rule_match| { + matches!( + rule_match, + RuleMatch::PrefixRuleMatch { + matched_prefix, + decision: Decision::Allow, + .. + } if matched_prefix.len() == outer_argv.len() + ) + }); + let composed_full_authority = raw_full_outer_allow && every_inner_explicit_allow; + + let mut evaluation = Evaluation { + decision: matched_rules + .iter() + .map(RuleMatch::decision) + .max() + .unwrap_or(Decision::Forbidden), + matched_rules, + }; + let permission_or_backend_gate = permission_delta_requires_outer_authority( + context.permission_profile, + context.sandbox_permissions, + ) || missing_managed_windows_sandbox_backend( + context.permission_profile, + context.windows_sandbox_level, + ); + if evaluation.decision != Decision::Forbidden + && permission_or_backend_gate + && !composed_full_authority + { + evaluation + .matched_rules + .push(RuleMatch::HeuristicsRuleMatch { + command: outer_argv.to_vec(), + decision: Decision::Prompt, + }); + evaluation.decision = evaluation + .matched_rules + .iter() + .map(RuleMatch::decision) + .max() + .unwrap_or(Decision::Forbidden); + } + + match evaluation.decision { + Decision::Forbidden => ExecApprovalRequirement::Forbidden { + reason: derive_forbidden_reason(outer_argv, &evaluation), + }, + Decision::Prompt => { + match prompt_is_rejected_by_policy( + context.approval_policy, + prompt_causes(&evaluation.matched_rules), + ) { + Some(reason) => ExecApprovalRequirement::Forbidden { + reason: reason.to_string(), + }, + None => ExecApprovalRequirement::NeedsOneShotApproval { + reason: derive_prompt_reason(outer_argv, &evaluation), + }, + } + } + Decision::Allow => ExecApprovalRequirement::Skip { + bypass_sandbox: composed_full_authority, + proposed_execpolicy_amendment: None, + }, + } +} + impl Default for ExecPolicyManager { fn default() -> Self { Self::new(Arc::new(Policy::empty())) @@ -865,6 +1021,30 @@ fn profile_has_managed_filesystem_restrictions(permission_profile: &PermissionPr && !file_system_sandbox_policy.has_full_disk_write_access() } +fn permission_delta_requires_outer_authority( + permission_profile: &PermissionProfile, + sandbox_permissions: SandboxPermissions, +) -> bool { + match (permission_profile, sandbox_permissions) { + (PermissionProfile::Disabled, _) => false, + (_, SandboxPermissions::WithAdditionalPermissions) => true, + (PermissionProfile::External { .. }, SandboxPermissions::RequireEscalated) => true, + (PermissionProfile::Managed { .. }, SandboxPermissions::RequireEscalated) => { + unsandboxed_execution_allowed(&permission_profile.file_system_sandbox_policy()) + } + (_, SandboxPermissions::UseDefault) => false, + } +} + +fn missing_managed_windows_sandbox_backend( + permission_profile: &PermissionProfile, + windows_sandbox_level: WindowsSandboxLevel, +) -> bool { + cfg!(windows) + && windows_sandbox_level == WindowsSandboxLevel::Disabled + && profile_has_managed_filesystem_restrictions(permission_profile) +} + fn default_policy_path(codex_home: &Path) -> PathBuf { codex_home.join(RULES_DIR_NAME).join(DEFAULT_POLICY_FILE) } diff --git a/codex-rs/core/src/exec_policy_powershell.rs b/codex-rs/core/src/exec_policy_powershell.rs index 34dd868787..67b6b4a4a9 100644 --- a/codex-rs/core/src/exec_policy_powershell.rs +++ b/codex-rs/core/src/exec_policy_powershell.rs @@ -10,8 +10,14 @@ pub(super) enum PreparedPowerShell { Unsupported, } +enum RuntimeTrust { + Trusted, + Untrusted { outer_argv: Vec }, +} + pub(super) struct ParsedPowerShell { commands: Vec>, + runtime: RuntimeTrust, } pub(super) fn prepare(command: &[String]) -> Option { @@ -27,18 +33,35 @@ pub(super) fn prepare_classified( match parsed { PowerShellExecPolicyParse::TrustedRuntime { outcome: PowerShellExecPolicyParseOutcome::Commands(commands), - } => Some(PreparedPowerShell::Parsed(ParsedPowerShell { commands })), + } => Some(PreparedPowerShell::Parsed(ParsedPowerShell { + commands, + runtime: RuntimeTrust::Trusted, + })), PowerShellExecPolicyParse::TrustedRuntime { outcome: PowerShellExecPolicyParseOutcome::Unsupported, } => Some(PreparedPowerShell::Unsupported), PowerShellExecPolicyParse::TrustedRuntime { outcome: PowerShellExecPolicyParseOutcome::Failed, } => Some(forbidden(command, "the protected PowerShell parser failed")), + PowerShellExecPolicyParse::UntrustedRuntime { + outcome: PowerShellExecPolicyParseOutcome::Commands(commands), + } if !commands.is_empty() + && commands + .iter() + .all(|inner| !inner.is_empty() && inner.iter().all(|word| !word.is_empty())) => + { + Some(PreparedPowerShell::Parsed(ParsedPowerShell { + commands, + runtime: RuntimeTrust::Untrusted { + outer_argv: command.to_vec(), + }, + })) + } PowerShellExecPolicyParse::UntrustedRuntime { outcome: PowerShellExecPolicyParseOutcome::Commands(_), } => Some(forbidden( command, - "the PowerShell runtime is not a protected system executable", + "the protected system parser returned an empty command while inspecting an untrusted PowerShell wrapper", )), PowerShellExecPolicyParse::UntrustedRuntime { outcome: PowerShellExecPolicyParseOutcome::Unsupported, @@ -65,4 +88,11 @@ impl ParsedPowerShell { pub(super) fn commands(&self) -> &[Vec] { &self.commands } + + pub(super) fn untrusted_outer_argv(&self) -> Option<&[String]> { + match &self.runtime { + RuntimeTrust::Trusted => None, + RuntimeTrust::Untrusted { outer_argv } => Some(outer_argv), + } + } } diff --git a/codex-rs/core/src/exec_policy_powershell_tests.rs b/codex-rs/core/src/exec_policy_powershell_tests.rs index c28a34cdb4..1d19457cd9 100644 --- a/codex-rs/core/src/exec_policy_powershell_tests.rs +++ b/codex-rs/core/src/exec_policy_powershell_tests.rs @@ -19,6 +19,23 @@ fn powershell_command(script: &str) -> Vec { ] } +fn untrusted_powershell_command(script: &str) -> Vec { + vec![ + "powershell.exe".to_string(), + "-Command".to_string(), + script.to_string(), + ] +} + +fn prefix_rule_for(command: &[String], decision: &str) -> String { + let pattern = command + .iter() + .map(|word| format!(r#""{}""#, starlark_string(word))) + .collect::>() + .join(", "); + format!(r#"prefix_rule(pattern=[{pattern}], decision="{decision}")"#) +} + fn outer_result(command: &[String], prompt: bool) -> ExecApprovalRequirement { let amendment = Some(ExecPolicyAmendment::new(command.to_vec())); if prompt { @@ -63,113 +80,646 @@ async fn requirement( permission_profile: PermissionProfile, sandbox_permissions: SandboxPermissions, ) -> ExecApprovalRequirement { - exec_approval_requirement_for_command(ExecApprovalRequirementScenario { - policy_src: policy_src.map(str::to_owned), - command: command.to_vec(), + requirement_with_options( + policy_src, + command, approval_policy, permission_profile, + WindowsSandboxLevel::RestrictedToken, sandbox_permissions, - prefix_rule: None, - }) + /*prefix_rule*/ None, + ) .await } -#[tokio::test] -async fn rejects_untrusted_powershell_across_approval_and_sandbox_modes() { - let trusted = trusted_windows_powershell(); +async fn requirement_with_options( + policy_src: Option<&str>, + command: &[String], + approval_policy: AskForApproval, + permission_profile: PermissionProfile, + windows_sandbox_level: WindowsSandboxLevel, + sandbox_permissions: SandboxPermissions, + prefix_rule: Option>, +) -> ExecApprovalRequirement { + let policy = policy_from_src(policy_src); + ExecPolicyManager::new(policy) + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command, + approval_policy, + permission_profile, + windows_sandbox_level, + sandbox_permissions, + prefix_rule, + }) + .await +} + +fn composed_untrusted_requirement( + policy_src: Option<&str>, + outer_argv: &[String], + commands: &[Vec], + approval_policy: AskForApproval, + permission_profile: &PermissionProfile, + windows_sandbox_level: WindowsSandboxLevel, + sandbox_permissions: SandboxPermissions, +) -> ExecApprovalRequirement { + let policy = policy_from_src(policy_src); + create_untrusted_powershell_approval_requirement( + policy.as_ref(), + outer_argv, + commands, + UnmatchedCommandContext { + approval_policy, + permission_profile, + windows_sandbox_level, + sandbox_permissions, + used_complex_parsing: false, + command_origin: ExecPolicyCommandOrigin::PowerShell, + }, + ) +} + +fn one_shot(reason: Option) -> ExecApprovalRequirement { + ExecApprovalRequirement::NeedsOneShotApproval { reason } +} + +fn untrusted_skip(bypass_sandbox: bool) -> ExecApprovalRequirement { + ExecApprovalRequirement::Skip { + bypass_sandbox, + proposed_execpolicy_amendment: None, + } +} + +#[test] +fn untrusted_parsed_state_retains_exact_outer_argv_and_trusted_state_does_not() { + let outer_argv = vec_str(&[ + r"\\?\C:\Workspace\PoWeRsHeLl.ExE. ", + "-NoProfile", + "-Command", + "echo First; Write-Output SECOND", + ]); + let commands = vec![ + vec_str(&["echo", "First"]), + vec_str(&["Write-Output", "SECOND"]), + ]; + + let Some(powershell_policy::PreparedPowerShell::Parsed(untrusted)) = + powershell_policy::prepare_classified( + &outer_argv, + PowerShellExecPolicyParse::UntrustedRuntime { + outcome: PowerShellExecPolicyParseOutcome::Commands(commands.clone()), + }, + ) + else { + panic!("nonempty inspected untrusted PowerShell should remain policy-evaluable"); + }; + pretty_assertions::assert_eq!(untrusted.commands(), commands); + pretty_assertions::assert_eq!( + untrusted.untrusted_outer_argv(), + Some(outer_argv.as_slice()) + ); + + let Some(powershell_policy::PreparedPowerShell::Parsed(trusted)) = + powershell_policy::prepare_classified( + &outer_argv, + PowerShellExecPolicyParse::TrustedRuntime { + outcome: PowerShellExecPolicyParseOutcome::Commands(vec![vec_str(&[ + "Get-Location", + ])]), + }, + ) + else { + panic!("trusted parsed PowerShell should remain parsed"); + }; + pretty_assertions::assert_eq!(trusted.untrusted_outer_argv(), None); +} + +#[test] +fn untrusted_opaque_failed_and_empty_parser_states_remain_terminal() { + let command = untrusted_powershell_command("echo blocked"); + let rendered = render_shlex_command(&command); let cases = [ ( - "untrusted parsed runtime", - vec_str(&["powershell.EXE.CmD", "-Command", "echo allowed"]), - Some(r#"prefix_rule(pattern=["echo"], decision="allow")"#), - "the PowerShell runtime is not a protected system executable", + PowerShellExecPolicyParseOutcome::Commands(Vec::new()), + "the protected system parser returned an empty command while inspecting an untrusted PowerShell wrapper", ), ( - "untrusted opaque body", - vec_str(&[ - "powershell.exe", - "-NonInteractive", - "-Command", - "echo blocked", - ]), - None, + PowerShellExecPolicyParseOutcome::Commands(vec![Vec::new()]), + "the protected system parser returned an empty command while inspecting an untrusted PowerShell wrapper", + ), + ( + PowerShellExecPolicyParseOutcome::Commands(vec![vec![String::new()]]), + "the protected system parser returned an empty command while inspecting an untrusted PowerShell wrapper", + ), + ( + PowerShellExecPolicyParseOutcome::Unsupported, "an untrusted PowerShell wrapper could not be inspected with the protected system parser", ), ( - "full outer rule for bare runtime", - vec_str(&["powershell.exe", "-Command", "echo allowed"]), - Some( - r#"prefix_rule(pattern=["powershell.exe", "-Command", "echo allowed"], decision="allow")"#, - ), - "the PowerShell runtime is not a protected system executable", + PowerShellExecPolicyParseOutcome::Failed, + "the protected system parser failed while inspecting an untrusted PowerShell wrapper", ), - ( - "verbatim path alias", - vec![ - format!(r"\\?\{trusted}."), - "-Command".to_string(), - "echo allowed".to_string(), - ], - Some(r#"prefix_rule(pattern=["echo"], decision="allow")"#), - "the PowerShell runtime is not a protected system executable", - ), - ( - "device path alias", - vec![ - format!(r"\\.\{trusted} "), - "-Command".to_string(), - "echo allowed".to_string(), - ], - Some(r#"prefix_rule(pattern=["echo"], decision="allow")"#), - "the PowerShell runtime is not a protected system executable", - ), - ]; - let policies = [ - AskForApproval::Never, - AskForApproval::OnRequest, - AskForApproval::UnlessTrusted, - granular(/*rules*/ false, /*sandbox_approval*/ false), - granular(/*rules*/ false, /*sandbox_approval*/ true), - granular(/*rules*/ true, /*sandbox_approval*/ false), - granular(/*rules*/ true, /*sandbox_approval*/ true), - ]; - let profiles = [ - PermissionProfile::Disabled, - PermissionProfile::read_only(), - PermissionProfile::default(), - PermissionProfile::External { - network: NetworkSandboxPolicy::Restricted, - }, ]; - for (name, command, policy_src, reason) in cases { - let rendered = render_shlex_command(&command); - for approval_policy in policies { - for permission_profile in &profiles { - let requirement = - exec_approval_requirement_for_command(ExecApprovalRequirementScenario { - policy_src: policy_src.map(str::to_owned), - command: command.clone(), - approval_policy, - permission_profile: permission_profile.clone(), - sandbox_permissions: SandboxPermissions::UseDefault, - prefix_rule: None, - }) - .await; - - pretty_assertions::assert_eq!( - requirement, - ExecApprovalRequirement::Forbidden { - reason: format!("`{rendered}` rejected: {reason}"), - }, - "{} with {approval_policy:?} and {permission_profile:?}", - name - ); - } - } + for (outcome, reason) in cases { + let Some(powershell_policy::PreparedPowerShell::Terminal(requirement)) = + powershell_policy::prepare_classified( + &command, + PowerShellExecPolicyParse::UntrustedRuntime { outcome }, + ) + else { + panic!("untrusted opaque, failed, and empty states must remain terminal"); + }; + pretty_assertions::assert_eq!( + requirement, + ExecApprovalRequirement::Forbidden { + reason: format!("`{rendered}` rejected: {reason}"), + }, + ); } } +#[test] +fn untrusted_outer_and_every_inner_command_compose_with_strictest_wins() { + let outer = vec_str(&[r"C:\workspace\powershell.exe", "-Command", "echo allowed"]); + let rendered = render_shlex_command(&outer); + let echo = vec_str(&["echo", "allowed"]); + let later = vec_str(&["madeup-later", "value"]); + let full_outer_allow = prefix_rule_for(&outer, "allow"); + let short_outer_allow = prefix_rule_for(&outer[..1], "allow"); + let outer_prompt = prefix_rule_for(&outer, "prompt"); + let outer_forbidden = prefix_rule_for(&outer, "forbidden"); + let echo_allow = prefix_rule_for(&echo[..1], "allow"); + let echo_prompt = prefix_rule_for(&echo[..1], "prompt"); + let echo_forbidden = prefix_rule_for(&echo[..1], "forbidden"); + let later_prompt = prefix_rule_for(&later[..1], "prompt"); + let later_forbidden = prefix_rule_for(&later[..1], "forbidden"); + let policy_prompt_reason = Some(format!("`{rendered}` requires approval by policy")); + + let cases = vec![ + ( + "complete outer and every inner explicit allow", + format!("{full_outer_allow}\n{echo_allow}"), + vec![echo.clone()], + untrusted_skip(true), + ), + ( + "short outer allow remains sandbox preserving", + format!("{short_outer_allow}\n{echo_allow}"), + vec![echo.clone()], + untrusted_skip(false), + ), + ( + "heuristic inner allow cannot establish full authority", + full_outer_allow.clone(), + vec![vec_str(&["Get-Location"])], + untrusted_skip(false), + ), + ( + "inner prompt dominates outer allow", + format!("{full_outer_allow}\n{echo_prompt}"), + vec![echo.clone()], + one_shot(policy_prompt_reason.clone()), + ), + ( + "outer prompt dominates inner allow", + format!("{outer_prompt}\n{echo_allow}"), + vec![echo.clone()], + one_shot(policy_prompt_reason.clone()), + ), + ( + "unmatched outer prompts even with inner allow", + echo_allow.clone(), + vec![echo.clone()], + one_shot(None), + ), + ( + "outer forbidden is terminal", + format!("{outer_forbidden}\n{echo_allow}"), + vec![echo.clone()], + ExecApprovalRequirement::Forbidden { + reason: format!( + "`{rendered}` rejected: policy forbids commands starting with `{rendered}`" + ), + }, + ), + ( + "inner forbidden is terminal", + format!("{full_outer_allow}\n{echo_forbidden}"), + vec![echo.clone()], + ExecApprovalRequirement::Forbidden { + reason: format!( + "`{rendered}` rejected: policy forbids commands starting with `echo`" + ), + }, + ), + ( + "later statement prompt participates", + format!("{full_outer_allow}\n{echo_allow}\n{later_prompt}"), + vec![echo.clone(), later.clone()], + one_shot(policy_prompt_reason.clone()), + ), + ( + "later statement forbidden participates", + format!("{full_outer_allow}\n{echo_allow}\n{later_forbidden}"), + vec![echo.clone(), later.clone()], + ExecApprovalRequirement::Forbidden { + reason: format!( + "`{rendered}` rejected: policy forbids commands starting with `madeup-later`" + ), + }, + ), + ( + "nested wrapper does not inherit outer authority", + full_outer_allow, + vec![vec_str(&["powershell.exe", "-Command", "Get-Location"])], + untrusted_skip(false), + ), + ]; + + for (name, policy_src, commands, expected) in cases { + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&policy_src), + &outer, + &commands, + AskForApproval::OnRequest, + &PermissionProfile::read_only(), + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + ), + expected, + "{name}", + ); + } +} + +#[test] +fn untrusted_unmatched_outer_and_mixed_prompt_causes_follow_approval_policy() { + let outer = untrusted_powershell_command("echo allowed"); + let echo = vec_str(&["echo", "allowed"]); + let echo_allow = prefix_rule_for(&echo[..1], "allow"); + + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&echo_allow), + &outer, + std::slice::from_ref(&echo), + AskForApproval::Never, + &PermissionProfile::Disabled, + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + ), + ExecApprovalRequirement::Forbidden { + reason: format!( + "`{}` rejected: blocked by policy", + render_shlex_command(&outer) + ), + }, + ); + for approval_policy in [AskForApproval::OnRequest, AskForApproval::UnlessTrusted] { + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&echo_allow), + &outer, + std::slice::from_ref(&echo), + approval_policy, + &PermissionProfile::Disabled, + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + ), + one_shot(None), + "{approval_policy:?}", + ); + } + + let outer_prompt = prefix_rule_for(&outer, "prompt"); + let mixed_policy = format!("{outer_prompt}\n{echo_allow}"); + for (rules, sandbox_approval, expected) in [ + ( + false, + false, + ExecApprovalRequirement::Forbidden { + reason: REJECT_RULES_APPROVAL_REASON.to_string(), + }, + ), + ( + false, + true, + ExecApprovalRequirement::Forbidden { + reason: REJECT_RULES_APPROVAL_REASON.to_string(), + }, + ), + ( + true, + false, + ExecApprovalRequirement::Forbidden { + reason: REJECT_SANDBOX_APPROVAL_REASON.to_string(), + }, + ), + ( + true, + true, + one_shot(Some(format!( + "`{}` requires approval by policy", + render_shlex_command(&outer) + ))), + ), + ] { + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&mixed_policy), + &outer, + std::slice::from_ref(&echo), + granular(rules, sandbox_approval), + &PermissionProfile::read_only(), + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::RequireEscalated, + ), + expected, + "rules={rules}, sandbox_approval={sandbox_approval}", + ); + } + + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&mixed_policy), + &outer, + std::slice::from_ref(&echo), + granular(/*rules*/ true, /*sandbox_approval*/ false), + &PermissionProfile::read_only(), + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + ), + one_shot(Some(format!( + "`{}` requires approval by policy", + render_shlex_command(&outer) + ))), + "a rule-only prompt does not require sandbox approval", + ); + + let full_outer_allow = prefix_rule_for(&outer, "allow"); + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&full_outer_allow), + &outer, + std::slice::from_ref(&echo), + granular(/*rules*/ false, /*sandbox_approval*/ true), + &PermissionProfile::read_only(), + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::RequireEscalated, + ), + one_shot(None), + "a sandbox-only prompt does not require rules approval", + ); +} + +#[test] +fn untrusted_permission_and_windows_backend_gates_require_composed_authority() { + use SandboxPermissions as SP; + use WindowsSandboxLevel as WSL; + + let outer = untrusted_powershell_command("Get-Content Cargo.toml"); + let inner = vec_str(&["Get-Content", "Cargo.toml"]); + let partial_policy = format!( + "{}\n{}", + prefix_rule_for(&outer[..1], "allow"), + prefix_rule_for(&inner[..1], "allow") + ); + let full_policy = format!( + "{}\n{}", + prefix_rule_for(&outer, "allow"), + prefix_rule_for(&inner[..1], "allow") + ); + let denied_read_profile = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: "**/*.env".to_string(), + }, + access: FileSystemAccessMode::Deny, + }]), + NetworkSandboxPolicy::Restricted, + ); + let external_profile = PermissionProfile::External { + network: NetworkSandboxPolicy::Restricted, + }; + + for (name, profile, level, permissions, prompts) in [ + ( + "managed default", + PermissionProfile::read_only(), + WSL::RestrictedToken, + SP::UseDefault, + false, + ), + ( + "managed escalation", + PermissionProfile::read_only(), + WSL::RestrictedToken, + SP::RequireEscalated, + true, + ), + ( + "additional permissions", + PermissionProfile::read_only(), + WSL::RestrictedToken, + SP::WithAdditionalPermissions, + true, + ), + ( + "missing managed backend", + PermissionProfile::read_only(), + WSL::Disabled, + SP::UseDefault, + true, + ), + ( + "denied-read escalation", + denied_read_profile, + WSL::RestrictedToken, + SP::RequireEscalated, + false, + ), + ( + "disabled profile", + PermissionProfile::Disabled, + WSL::Disabled, + SP::RequireEscalated, + false, + ), + ( + "external default", + external_profile.clone(), + WSL::RestrictedToken, + SP::UseDefault, + false, + ), + ( + "external escalation", + external_profile, + WSL::RestrictedToken, + SP::RequireEscalated, + true, + ), + ] { + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&partial_policy), + &outer, + std::slice::from_ref(&inner), + AskForApproval::OnRequest, + &profile, + level, + permissions, + ), + if prompts { + one_shot(None) + } else { + untrusted_skip(false) + }, + "{name}", + ); + } + + for (level, permissions) in [ + (WSL::RestrictedToken, SP::RequireEscalated), + (WSL::RestrictedToken, SP::WithAdditionalPermissions), + (WSL::Disabled, SP::UseDefault), + ] { + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&full_policy), + &outer, + std::slice::from_ref(&inner), + AskForApproval::OnRequest, + &PermissionProfile::read_only(), + level, + permissions, + ), + untrusted_skip(true), + "composed authority should cover {level:?} and {permissions:?}", + ); + } +} + +#[test] +fn untrusted_wrapper_identity_uses_exact_outer_and_restrictive_basename_rules() { + let path_a = vec_str(&[r"C:\workspace-a\powershell.exe", "-Command", "echo allowed"]); + let path_b = vec_str(&[r"C:\workspace-b\powershell.exe", "-Command", "echo allowed"]); + let inner = vec_str(&["echo", "allowed"]); + let inner_allow = prefix_rule_for(&inner[..1], "allow"); + let basename_allow = prefix_rule_for(&vec_str(&["powershell.exe"]), "allow"); + let exact_a_allow = prefix_rule_for(&path_a, "allow"); + let basename_prompt = prefix_rule_for(&vec_str(&["powershell.exe"]), "prompt"); + let basename_forbidden = prefix_rule_for(&vec_str(&["powershell.exe"]), "forbidden"); + let mut extended = path_a.clone(); + extended.insert(1, "-NoProfile".to_string()); + let namespace_outer = vec_str(&[ + r"\\?\C:\attacker\powershell.exe.", + "-Command", + "echo allowed", + ]); + let namespace_policy = format!( + "{}\nhost_executable(name = \"powershell\", paths = [\"C:\\\\trusted\\\\powershell.exe\"])\n{inner_allow}", + prefix_rule_for(&vec_str(&["powershell.exe."]), "allow") + ); + let rendered_a = render_shlex_command(&path_a); + let cases = vec![ + ( + "basename Allow", + path_a.clone(), + format!("{basename_allow}\n{inner_allow}"), + one_shot(None), + ), + ( + "path A rule against B", + path_b, + format!("{exact_a_allow}\n{inner_allow}"), + one_shot(None), + ), + ( + "basename Prompt", + path_a.clone(), + format!("{exact_a_allow}\n{basename_prompt}\n{inner_allow}"), + one_shot(Some(format!("`{rendered_a}` requires approval by policy"))), + ), + ( + "basename Forbidden", + path_a, + format!("{exact_a_allow}\n{basename_forbidden}\n{inner_allow}"), + ExecApprovalRequirement::Forbidden { + reason: format!( + "`{rendered_a}` rejected: policy forbids commands starting with `powershell.exe`" + ), + }, + ), + ( + "extended argv", + extended, + format!("{exact_a_allow}\n{inner_allow}"), + one_shot(None), + ), + ( + "mapped namespace", + namespace_outer, + namespace_policy, + one_shot(None), + ), + ]; + + for (name, outer, policy_src, expected) in cases { + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&policy_src), + &outer, + std::slice::from_ref(&inner), + AskForApproval::OnRequest, + &PermissionProfile::read_only(), + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + ), + expected, + "{name}", + ); + } +} + +#[tokio::test] +async fn untrusted_parsed_results_ignore_requested_amendments() { + let command = untrusted_powershell_command("echo allowed"); + let inner_allow = prefix_rule_for(&vec_str(&["echo"]), "allow"); + let requested_prefix = Some(vec_str(&["echo"])); + + pretty_assertions::assert_eq!( + requirement_with_options( + /*policy_src*/ None, + &command, + AskForApproval::OnRequest, + PermissionProfile::read_only(), + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + requested_prefix.clone(), + ) + .await, + one_shot(None), + ); + + let full_policy = format!("{}\n{inner_allow}", prefix_rule_for(&command, "allow")); + pretty_assertions::assert_eq!( + requirement_with_options( + Some(&full_policy), + &command, + AskForApproval::OnRequest, + PermissionProfile::read_only(), + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + requested_prefix, + ) + .await, + untrusted_skip(true), + ); +} + #[test] fn parser_failures_are_terminal_for_trusted_and_untrusted_runtimes() { let command = powershell_command("Get-Content Cargo.toml"); diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index 6042df5958..a84cfa6ca8 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -1315,79 +1315,61 @@ async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_gra } #[tokio::test] -async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision() { +async fn mixed_rule_and_sandbox_prompt_requires_every_granular_category_in_either_order() { let policy_src = r#"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![ - "bash".to_string(), - "-lc".to_string(), - "git status && madeup-cmd".to_string(), + let cases = [ + ("rule then sandbox", "git status && madeup-cmd"), + ("sandbox then rule", "madeup-cmd && git status"), ]; - let requirement = manager - .create_exec_approval_requirement_for_command(ExecApprovalRequest { - command: &command, - approval_policy: AskForApproval::Granular(GranularApprovalConfig { - sandbox_approval: true, - rules: true, - skill_approval: true, - request_permissions: true, - mcp_elicitations: true, - }), - permission_profile: PermissionProfile::read_only(), - windows_sandbox_level: WindowsSandboxLevel::Disabled, - sandbox_permissions: SandboxPermissions::RequireEscalated, - prefix_rule: None, - }) - .await; + for (order, script) in cases { + let command = vec!["bash".to_string(), "-lc".to_string(), script.to_string()]; + for (rules, sandbox_approval, expected_rejection) in [ + (false, false, Some(REJECT_RULES_APPROVAL_REASON)), + (false, true, Some(REJECT_RULES_APPROVAL_REASON)), + (true, false, Some(REJECT_SANDBOX_APPROVAL_REASON)), + (true, true, None), + ] { + let requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::Granular(GranularApprovalConfig { + sandbox_approval, + rules, + skill_approval: true, + request_permissions: true, + mcp_elicitations: true, + }), + permission_profile: PermissionProfile::read_only(), + windows_sandbox_level: WindowsSandboxLevel::Disabled, + sandbox_permissions: SandboxPermissions::RequireEscalated, + prefix_rule: None, + }) + .await; + let expected = match expected_rejection { + Some(reason) => ExecApprovalRequirement::Forbidden { + reason: reason.to_string(), + }, + None => ExecApprovalRequirement::NeedsApproval { + reason: Some(format!( + "`{}` requires approval by policy", + render_shlex_command(&command) + )), + proposed_execpolicy_amendment: None, + }, + }; - assert!(matches!( - requirement, - ExecApprovalRequirement::NeedsApproval { .. } - )); -} - -#[tokio::test] -async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled() { - let policy_src = r#"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![ - "bash".to_string(), - "-lc".to_string(), - "git status && madeup-cmd".to_string(), - ]; - - let requirement = manager - .create_exec_approval_requirement_for_command(ExecApprovalRequest { - command: &command, - approval_policy: AskForApproval::Granular(GranularApprovalConfig { - sandbox_approval: true, - rules: false, - skill_approval: true, - request_permissions: true, - mcp_elicitations: true, - }), - permission_profile: PermissionProfile::read_only(), - windows_sandbox_level: WindowsSandboxLevel::Disabled, - sandbox_permissions: SandboxPermissions::RequireEscalated, - prefix_rule: None, - }) - .await; - - assert_eq!( - requirement, - ExecApprovalRequirement::Forbidden { - reason: REJECT_RULES_APPROVAL_REASON.to_string(), + assert_eq!( + requirement, expected, + "{order} with rules={rules} and sandbox_approval={sandbox_approval}", + ); } - ); + } } #[tokio::test] diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 27a9d8aaed..0a22aa2995 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -196,13 +196,6 @@ pub(crate) enum ExecApprovalRequirement { /// Approval is required for this invocation, but the decision must not be /// reused from or written to the session approval cache. #[cfg_attr(not(windows), allow(dead_code))] - #[cfg_attr( - all(windows, not(test)), - expect( - dead_code, - reason = "constructed by the later stacked Windows policy layer" - ) - )] NeedsOneShotApproval { reason: Option }, /// Execution forbidden for this tool call. Forbidden { reason: String }, diff --git a/codex-rs/core/tests/suite/exec_policy.rs b/codex-rs/core/tests/suite/exec_policy.rs index 8ebdcf41cd..00e01b5049 100644 --- a/codex-rs/core/tests/suite/exec_policy.rs +++ b/codex-rs/core/tests/suite/exec_policy.rs @@ -8,7 +8,11 @@ use codex_protocol::config_types::Settings; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; +#[cfg(windows)] +use codex_protocol::protocol::ExecApprovalPurpose; use codex_protocol::protocol::Op; +#[cfg(windows)] +use codex_protocol::protocol::ReviewDecision; use codex_protocol::user_input::UserInput; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -107,7 +111,8 @@ fn enable_unified_exec(config: &mut codex_core::config::Config) { #[cfg(windows)] #[tokio::test] -async fn unified_exec_workspace_powershell_path_is_rejected_before_execution() -> Result<()> { +async fn unified_exec_workspace_powershell_path_requires_one_shot_approval_before_execution() +-> Result<()> { let server = start_mock_server().await; let mut builder = test_codex().with_config(enable_unified_exec); let test = builder.build(&server).await?; @@ -120,10 +125,24 @@ async fn unified_exec_workspace_powershell_path_is_rejected_before_execution() - .expect("the test workspace path must be valid UTF-8") .to_string(); - let call_id = "unified-exec-workspace-powershell-rejected"; + let sentinel = test.config.cwd.join("workspace-powershell-started.txt"); + let marker = "WORKSPACE_POWERSHELL_STARTED"; + let sentinel_arg = sentinel + .to_str() + .expect("the sentinel path must be valid UTF-8") + .replace('\'', "''"); + let script = + format!("Set-Content -LiteralPath '{sentinel_arg}' started; Write-Output '{marker}'"); + let expected_command = vec![ + fake_powershell.clone(), + "-NoProfile".to_string(), + "-Command".to_string(), + script.clone(), + ]; + let call_id = "unified-exec-workspace-powershell-one-shot"; let args = json!({ "shell": fake_powershell.clone(), - "cmd": "Get-Location", + "cmd": script, "yield_time_ms": 1_000, }); mount_sse_once( @@ -153,19 +172,88 @@ async fn unified_exec_workspace_powershell_path_is_rejected_before_execution() - ) .await?; - let event = wait_for_event(&test.codex, |event| { - matches!( - event, - EventMsg::ExecApprovalRequest(_) - | EventMsg::ExecCommandBegin(_) - | EventMsg::TurnComplete(_) - ) - }) - .await; - assert!( - matches!(event, EventMsg::TurnComplete(_)), - "workspace PowerShell must be rejected before approval or execution, got {event:?}" + let approval = loop { + let event = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) + | EventMsg::ExecCommandBegin(_) + | EventMsg::ExecCommandEnd(_) + | EventMsg::TurnComplete(_) + ) + }) + .await; + assert!( + !sentinel.exists(), + "the requested workspace PowerShell must not start before approval" + ); + match event { + EventMsg::ExecCommandBegin(begin) => { + assert_eq!(begin.call_id, call_id); + assert_eq!(begin.command, expected_command); + } + EventMsg::ExecApprovalRequest(approval) => break approval, + EventMsg::ExecCommandEnd(end) => { + panic!("workspace PowerShell completed before approval: {end:?}") + } + EventMsg::TurnComplete(_) => panic!("expected one-shot approval before completion"), + _ => unreachable!(), + } + }; + assert_eq!(approval.call_id, call_id); + assert_eq!(approval.command, expected_command); + assert_eq!(approval.cwd, test.config.cwd); + let approval_id = approval + .approval_id + .clone() + .expect("one-shot approval must carry a callback ID"); + assert_ne!(approval_id, call_id); + assert_eq!( + approval.approval_purpose, + Some(ExecApprovalPurpose::Initial) ); + assert_eq!( + approval.effective_approval_purpose(), + ExecApprovalPurpose::Initial + ); + assert_eq!( + approval.effective_available_decisions(), + vec![ReviewDecision::Approved, ReviewDecision::Abort] + ); + assert_eq!(approval.proposed_execpolicy_amendment, None); + assert!(!sentinel.exists()); + + test.codex + .submit(Op::ExecApproval { + id: approval_id, + turn_id: Some(approval.turn_id), + decision: ReviewDecision::Abort, + }) + .await?; + + let mut command_end = None; + loop { + match wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ExecCommandEnd(_) | EventMsg::TurnComplete(_) + ) + }) + .await + { + EventMsg::ExecCommandEnd(end) => command_end = Some(end), + EventMsg::TurnComplete(_) => break, + _ => unreachable!(), + } + } + assert!( + !sentinel.exists(), + "aborting the one-shot approval must never start the requested workspace PowerShell" + ); + if let Some(end) = command_end { + assert_ne!(end.exit_code, 0, "aborted command must not report success"); + assert!(!end.aggregated_output.contains(marker)); + } let output_item = results_mock.single_request().function_call_output(call_id); let output = output_item @@ -173,8 +261,8 @@ async fn unified_exec_workspace_powershell_path_is_rejected_before_execution() - .and_then(Value::as_str) .expect("function call output should include a string output payload"); assert!( - output.contains("the PowerShell runtime is not a protected system executable"), - "unexpected output: {output}" + !output.contains(marker), + "unexpected successful output: {output}" ); Ok(())