From 729c31bcf49237d033ce43b2d49ee2d80e2087bc Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Fri, 3 Jul 2026 14:59:58 -0700 Subject: [PATCH 1/4] fix(core): require exact outer argv authority --- codex-rs/core/src/exec_policy.rs | 2 +- codex-rs/core/src/exec_policy_windows_tests.rs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index d301474166..10ebb38381 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -365,7 +365,7 @@ impl ExecPolicyManager { matched_prefix, decision: Decision::Allow, .. - } if matched_prefix.len() == outer.len() + } if matched_prefix.as_slice() == outer ) }) }); diff --git a/codex-rs/core/src/exec_policy_windows_tests.rs b/codex-rs/core/src/exec_policy_windows_tests.rs index 491190554a..0b8b94d558 100644 --- a/codex-rs/core/src/exec_policy_windows_tests.rs +++ b/codex-rs/core/src/exec_policy_windows_tests.rs @@ -146,7 +146,11 @@ async fn only_exact_outer_rules_can_bypass_runtime_resolution() { "host_executable(name = \"powershell\", paths = [\"{executable}\"])\n\ prefix_rule(pattern=[\"powershell\", {rest}], decision=\"allow\")" ), - Some(true), + Some(false), + ), + ( + format!("prefix_rule(pattern=[\"powershell\", {rest}], decision=\"allow\")"), + Some(false), ), ( format!( From e32f317912f37f4b10a05247605e1c8a5821fff6 Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Fri, 3 Jul 2026 15:11:37 -0700 Subject: [PATCH 2/4] fix(execpolicy): preserve restrictive namespace rules --- .../core/src/exec_policy_windows_tests.rs | 70 +++++++++++++++++++ codex-rs/execpolicy/README.md | 2 +- codex-rs/execpolicy/src/policy.rs | 2 +- codex-rs/execpolicy/tests/basic.rs | 47 +++++++++---- 4 files changed, 104 insertions(+), 17 deletions(-) diff --git a/codex-rs/core/src/exec_policy_windows_tests.rs b/codex-rs/core/src/exec_policy_windows_tests.rs index 286efec211..1ee9c04d01 100644 --- a/codex-rs/core/src/exec_policy_windows_tests.rs +++ b/codex-rs/core/src/exec_policy_windows_tests.rs @@ -403,6 +403,76 @@ host_executable(name = "git", paths = ["C:\\trusted\\git.exe"]) ); } +#[tokio::test] +async fn mapped_namespace_literal_restrictions_apply_to_direct_and_inner_commands() { + for (kind, command) in [ + ( + "direct", + vec![ + r"\\?\C:\attacker\git.exe.".to_string(), + "status".to_string(), + ], + ), + ( + "PowerShell inner", + powershell_command(r"\\?\C:\attacker\git.exe. status"), + ), + ] { + let rendered = render_shlex_command(&command); + + assert_eq!( + windows_requirement( + String::new(), + &command, + AskForApproval::OnRequest, + PermissionProfile::Disabled, + WindowsSandboxLevel::Disabled, + SandboxPermissions::UseDefault, + ) + .await, + skip_outer(&command, false), + "unmatched {kind} command must take the allow fallback", + ); + + for (wire_decision, expected) in [ + ( + "prompt", + ExecApprovalRequirement::NeedsApproval { + reason: Some(format!("`{rendered}` requires approval by policy")), + proposed_execpolicy_amendment: None, + }, + ), + ( + "forbidden", + ExecApprovalRequirement::Forbidden { + reason: format!( + "`{rendered}` rejected: policy forbids commands starting with `git.exe.`" + ), + }, + ), + ] { + assert_eq!( + windows_requirement( + format!( + r#" +prefix_rule(pattern = ["git.exe."], decision = "{wire_decision}") +host_executable(name = "git", paths = ["C:\\trusted\\git.exe"]) +"# + ), + &command, + AskForApproval::OnRequest, + PermissionProfile::Disabled, + WindowsSandboxLevel::Disabled, + SandboxPermissions::UseDefault, + ) + .await, + expected, + "{kind}/{wire_decision}", + ); + } + } +} + #[test] fn commands_for_exec_policy_keeps_bare_powershell_alias_opaque() { let command = vec![ diff --git a/codex-rs/execpolicy/README.md b/codex-rs/execpolicy/README.md index 77c986f0fb..0dd116aaf5 100644 --- a/codex-rs/execpolicy/README.md +++ b/codex-rs/execpolicy/README.md @@ -43,7 +43,7 @@ host_executable( - With host-executable resolution enabled, if no exact rule matches, execpolicy may fall back from `/usr/bin/git` to basename rules for `git`. - If `host_executable(name="git", ...)` exists, basename fallback is only allowed for listed absolute paths. - If no `host_executable()` entry exists for a basename, basename fallback is allowed. - - Windows verbatim/device namespace paths cannot be listed by `host_executable`; use an ordinary absolute spelling or an exact prefix rule. If either a namespace command's literal basename or its ordinary normalized alias has host-executable metadata, the namespace spelling does not inherit basename rules. With no corresponding metadata, its literal basename rule may still match. Exact full-path rules always retain their authored decision. + - Windows verbatim/device namespace paths cannot be listed by `host_executable`; use an ordinary absolute spelling or an exact prefix rule. If either a namespace command's literal basename or its ordinary normalized alias has host-executable metadata, basename `allow` is filtered while basename `prompt` and `forbidden` remain in force without a `resolvedProgram`. With no corresponding metadata, its literal basename rule may still match with its authored decision. Exact full-path rules always retain their authored decision. - The library's restrictive basename overlay is separate from ordinary fallback: it ignores host-executable path lists for `prompt` and `forbidden`, but always discards basename `allow`. ## CLI diff --git a/codex-rs/execpolicy/src/policy.rs b/codex-rs/execpolicy/src/policy.rs index dd0c86a5d4..84292a2d38 100644 --- a/codex-rs/execpolicy/src/policy.rs +++ b/codex-rs/execpolicy/src/policy.rs @@ -361,7 +361,7 @@ impl Policy { .host_executables_by_name .contains_key(&normalized_basename) { - return Vec::new(); + return self.match_restrictive_basename_rules(cmd); } } let Ok(program) = AbsolutePathBuf::try_from(first.clone()) else { diff --git a/codex-rs/execpolicy/tests/basic.rs b/codex-rs/execpolicy/tests/basic.rs index f97fa6fc29..bfe03f5118 100644 --- a/codex-rs/execpolicy/tests/basic.rs +++ b/codex-rs/execpolicy/tests/basic.rs @@ -1126,13 +1126,13 @@ host_executable(name = "git", paths = ["{}"]) #[cfg(windows)] #[test] -fn namespace_literal_aliases_do_not_bypass_equivalent_host_mapping() -> Result<()> { +fn mapped_namespace_literal_aliases_filter_allow_and_preserve_restrictions() -> Result<()> { let options = MatchOptions { resolve_host_executables: true, }; for executable_suffix in [".exe", ".cmd", ".bat", ".com"] { - for literal_tail in [".", " ", "...", " ", ". . ", ".. . "] { + for literal_tail in ["", ".", " ", "...", " ", ". . ", ".. . "] { let mixed_case = executable_suffix == ".com" && literal_tail == ".. . "; let raw_stem = if mixed_case { "GiT" } else { "git" }; let raw_suffix = if mixed_case { @@ -1144,7 +1144,7 @@ fn namespace_literal_aliases_do_not_bypass_equivalent_host_mapping() -> Result<( let literal_rule_key = raw_literal_name.to_ascii_lowercase(); let ordinary_name = format!("git{executable_suffix}"); let trusted_path = format!(r"C:\trusted\{ordinary_name}"); - let mapping_sources = [ + let mut mapping_sources = vec![ format!( r#"host_executable(name = "git", paths = ["{}"] )"#, starlark_string(&trusted_path) @@ -1156,6 +1156,8 @@ fn namespace_literal_aliases_do_not_bypass_equivalent_host_mapping() -> Result<( format!(r#"host_executable(name = "{literal_rule_key}", paths = [])"#), format!(r#"host_executable(name = "{raw_literal_name}", paths = [])"#), ]; + mapping_sources.sort(); + mapping_sources.dedup(); let namespace_paths = [ format!(r"\\?\C:\attacker\{raw_literal_name}"), format!(r"\\?\UNC\server\share\{raw_literal_name}"), @@ -1164,23 +1166,38 @@ fn namespace_literal_aliases_do_not_bypass_equivalent_host_mapping() -> Result<( ]; for mapping_source in mapping_sources { - let policy = parse_policy(&format!( - r#" -prefix_rule(pattern = ["{literal_rule_key}"], decision = "allow") + for (wire_decision, decision) in [ + ("allow", Decision::Allow), + ("prompt", Decision::Prompt), + ("forbidden", Decision::Forbidden), + ] { + let policy = parse_policy(&format!( + r#" +prefix_rule(pattern = ["{literal_rule_key}"], decision = "{wire_decision}") {mapping_source} "# - ))?; - for executable in &namespace_paths { - assert!( - policy - .matches_for_command_with_options( + ))?; + for executable in &namespace_paths { + let expected = if decision == Decision::Allow { + Vec::new() + } else { + vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec![literal_rule_key.clone()], + decision, + resolved_program: None, + justification: None, + }] + }; + assert_eq!( + policy.matches_for_command_with_options( &[executable.clone(), "status".to_string()], None, &options, - ) - .is_empty(), - "suffix={executable_suffix:?} tail={literal_tail:?} mapping={mapping_source:?} executable={executable:?}" - ); + ), + expected, + "suffix={executable_suffix:?} tail={literal_tail:?} decision={wire_decision:?} mapping={mapping_source:?} executable={executable:?}" + ); + } } } } From 7a02f212e9145e6d3d8b6986e220ff4f4403fd68 Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Fri, 3 Jul 2026 15:15:55 -0700 Subject: [PATCH 3/4] fix(core): require authority without filesystem containment --- codex-rs/core/src/exec_policy.rs | 24 ++- .../core/src/exec_policy_powershell_tests.rs | 154 +++++++++++++++++- 2 files changed, 173 insertions(+), 5 deletions(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 54afe95668..021512fd42 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -690,15 +690,13 @@ fn create_untrusted_powershell_approval_requirement( .unwrap_or(Decision::Forbidden), matched_rules, }; - let permission_or_backend_gate = permission_delta_requires_outer_authority( + let requires_composed_authority = untrusted_composition_requires_full_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 + && requires_composed_authority && !composed_full_authority { evaluation @@ -1036,6 +1034,24 @@ fn permission_delta_requires_outer_authority( } } +#[cfg_attr(not(windows), allow(dead_code))] +fn untrusted_composition_requires_full_authority( + permission_profile: &PermissionProfile, + sandbox_permissions: SandboxPermissions, + windows_sandbox_level: WindowsSandboxLevel, +) -> bool { + let lacks_filesystem_containment = match permission_profile { + PermissionProfile::Disabled => true, + PermissionProfile::Managed { .. } => { + !profile_has_managed_filesystem_restrictions(permission_profile) + } + PermissionProfile::External { .. } => false, + }; + lacks_filesystem_containment + || permission_delta_requires_outer_authority(permission_profile, sandbox_permissions) + || missing_managed_windows_sandbox_backend(permission_profile, windows_sandbox_level) +} + fn missing_managed_windows_sandbox_backend( permission_profile: &PermissionProfile, windows_sandbox_level: WindowsSandboxLevel, diff --git a/codex-rs/core/src/exec_policy_powershell_tests.rs b/codex-rs/core/src/exec_policy_powershell_tests.rs index 5f1f7ffb60..86be11c179 100644 --- a/codex-rs/core/src/exec_policy_powershell_tests.rs +++ b/codex-rs/core/src/exec_policy_powershell_tests.rs @@ -545,7 +545,7 @@ fn untrusted_permission_and_windows_backend_gates_require_composed_authority() { PermissionProfile::Disabled, WSL::Disabled, SP::RequireEscalated, - false, + true, ), ( "external default", @@ -602,6 +602,158 @@ fn untrusted_permission_and_windows_backend_gates_require_composed_authority() { } } +#[test] +fn untrusted_without_filesystem_containment_requires_complete_composed_authority() { + let outer = untrusted_powershell_command("Get-Location"); + let inner = vec_str(&["Get-Location"]); + let partial_policy = format!( + "{}\n{}", + prefix_rule_for(&outer[..1], "allow"), + prefix_rule_for(&inner, "allow") + ); + let full_policy = format!( + "{}\n{}", + prefix_rule_for(&outer, "allow"), + prefix_rule_for(&inner, "allow") + ); + let disabled = PermissionProfile::Disabled; + + for (name, approval_policy, expected) in [ + ( + "on request", + AskForApproval::OnRequest, + one_shot(/*reason*/ None), + ), + ( + "unless trusted", + AskForApproval::UnlessTrusted, + one_shot(/*reason*/ None), + ), + ( + "never", + AskForApproval::Never, + ExecApprovalRequirement::Forbidden { + reason: PROMPT_CONFLICT_REASON.to_string(), + }, + ), + ( + "granular sandbox approval disabled", + granular(/*rules*/ false, /*sandbox_approval*/ false), + ExecApprovalRequirement::Forbidden { + reason: REJECT_SANDBOX_APPROVAL_REASON.to_string(), + }, + ), + ( + "granular sandbox approval enabled", + granular(/*rules*/ false, /*sandbox_approval*/ true), + one_shot(/*reason*/ None), + ), + ] { + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&partial_policy), + &outer, + std::slice::from_ref(&inner), + approval_policy, + &disabled, + WindowsSandboxLevel::Disabled, + SandboxPermissions::UseDefault, + ), + expected, + "{name}", + ); + } + + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&prefix_rule_for(&outer, "allow")), + &outer, + std::slice::from_ref(&inner), + AskForApproval::OnRequest, + &disabled, + WindowsSandboxLevel::Disabled, + SandboxPermissions::UseDefault, + ), + one_shot(/*reason*/ None), + "a heuristic-safe inner command is not explicit authority without a sandbox", + ); + + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&full_policy), + &outer, + std::slice::from_ref(&inner), + AskForApproval::OnRequest, + &disabled, + WindowsSandboxLevel::Disabled, + SandboxPermissions::UseDefault, + ), + untrusted_skip(/*bypass_sandbox*/ true), + "complete composed authority remains sufficient without a sandbox", + ); + + let managed_unrestricted = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::unrestricted(), + NetworkSandboxPolicy::Enabled, + ); + let managed_full_disk_write = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }]), + NetworkSandboxPolicy::Enabled, + ); + for (name, profile) in [ + ("managed unrestricted", managed_unrestricted), + ("managed full-disk write", managed_full_disk_write), + ] { + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&partial_policy), + &outer, + std::slice::from_ref(&inner), + AskForApproval::OnRequest, + &profile, + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + ), + one_shot(/*reason*/ None), + "partial authority must prompt for {name}", + ); + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&full_policy), + &outer, + std::slice::from_ref(&inner), + AskForApproval::OnRequest, + &profile, + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + ), + untrusted_skip(/*bypass_sandbox*/ true), + "complete authority remains sufficient for {name}", + ); + } + + pretty_assertions::assert_eq!( + composed_untrusted_requirement( + Some(&partial_policy), + &outer, + std::slice::from_ref(&inner), + AskForApproval::OnRequest, + &PermissionProfile::External { + network: NetworkSandboxPolicy::Restricted, + }, + WindowsSandboxLevel::RestrictedToken, + SandboxPermissions::UseDefault, + ), + untrusted_skip(/*bypass_sandbox*/ false), + "an externally enforced sandbox keeps existing partial-authority behavior", + ); +} + #[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"]); From 2aac4aa37fd8e35525dc8e0e66656474bbd814ea Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Fri, 3 Jul 2026 15:35:15 -0700 Subject: [PATCH 4/4] test(core): cover sandbox retry callback identity --- codex-rs/core/tests/suite/approvals.rs | 113 +++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index cc33286f16..4c7c85cd87 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -20,6 +20,7 @@ use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::ApplyPatchApprovalRequestEvent; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::ExecApprovalPurpose; use codex_protocol::protocol::ExecApprovalRequestEvent; use codex_protocol::protocol::ExecPolicyAmendment; use codex_protocol::protocol::GranularApprovalConfig; @@ -36,6 +37,7 @@ use core_test_support::responses::ev_function_call_with_namespace; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_once; use core_test_support::responses::mount_sse_once_match; +use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; @@ -2008,6 +2010,117 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[cfg(unix)] +async fn sandbox_retry_uses_fresh_callback_identity_and_one_shot_decisions() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let approval_policy = AskForApproval::Granular(GranularApprovalConfig { + sandbox_approval: true, + rules: true, + skill_approval: true, + request_permissions: true, + mcp_elicitations: true, + }); + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let sandbox_policy_for_config = sandbox_policy.clone(); + let test = test_codex() + .with_model("gpt-5.4") + .with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + config + .features + .enable(Feature::UnifiedExec) + .expect("enable unified exec"); + config + .set_legacy_sandbox_policy(sandbox_policy_for_config) + .expect("set sandbox policy"); + }) + .build(&server) + .await?; + + let call_id = "sandbox-retry-callback-identity"; + let target = test.cwd.path().join("sandbox-retry-callback-identity.txt"); + let _ = fs::remove_file(&target); + let command = format!("touch {target:?}"); + let event = exec_command_event( + call_id, + &command, + /*yield_time_ms*/ Some(30_000), + SandboxPermissions::UseDefault, + /*justification*/ None, + )?; + + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-sandbox-retry-1"), + event, + ev_completed("resp-sandbox-retry-1"), + ]), + sse(vec![ + ev_assistant_message("msg-sandbox-retry-1", "done"), + ev_completed("resp-sandbox-retry-2"), + ]), + ], + ) + .await; + + submit_turn( + &test, + "retry the sandbox-denied write", + approval_policy, + sandbox_policy, + ) + .await?; + + let approval = expect_exec_approval(&test, &command).await; + let approval_id = approval + .approval_id + .clone() + .unwrap_or_else(|| panic!("sandbox retry must carry a callback ID: {approval:?}")); + assert_eq!(approval.call_id, call_id); + assert_ne!(approval_id, call_id); + assert_eq!( + approval.approval_purpose, + Some(ExecApprovalPurpose::SandboxRetry) + ); + assert_eq!( + approval.effective_available_decisions(), + vec![ReviewDecision::Approved, ReviewDecision::Abort] + ); + assert_eq!( + approval.reason.as_deref(), + Some("command failed; retry without sandbox?") + ); + assert!( + !target.exists(), + "sandbox-denied write must not complete before retry approval" + ); + + test.codex + .submit(Op::ExecApproval { + id: approval_id, + turn_id: Some(approval.turn_id), + decision: ReviewDecision::Approved, + }) + .await?; + wait_for_completion(&test).await; + + let output_item = responses.requests()[1].function_call_output(call_id); + let result = parse_result(&output_item); + assert_eq!(result.exit_code, Some(0), "approved retry must succeed"); + assert!( + target.exists(), + "approved retry must create the target file" + ); + fs::remove_file(target)?; + + Ok(()) +} + #[tokio::test(flavor = "current_thread")] #[cfg(unix)] async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file() -> Result<()> {