diff --git a/codex-rs/core/src/tools/handlers/artifacts.rs b/codex-rs/core/src/tools/handlers/artifacts.rs index f046021ee6..42d6eb61e5 100644 --- a/codex-rs/core/src/tools/handlers/artifacts.rs +++ b/codex-rs/core/src/tools/handlers/artifacts.rs @@ -292,6 +292,17 @@ async fn prepare_artifact_build( cwd: turn.cwd.clone(), staged_script: source_path.clone(), }; + let initial_approval_requirement = session + .services + .exec_policy + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: turn.approval_policy.value(), + sandbox_policy: turn.sandbox_policy.get(), + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }) + .await; let escalation_approval_requirement = session .services .exec_policy @@ -303,23 +314,10 @@ async fn prepare_artifact_build( prefix_rule: None, }) .await; - let escalation_approval_requirement = match escalation_approval_requirement { - crate::tools::sandboxing::ExecApprovalRequirement::Skip { bypass_sandbox, .. } => { - crate::tools::sandboxing::ExecApprovalRequirement::Skip { - bypass_sandbox, - proposed_execpolicy_amendment: None, - } - } - crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { reason, .. } => { - crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { - reason, - proposed_execpolicy_amendment: None, - } - } - crate::tools::sandboxing::ExecApprovalRequirement::Forbidden { reason } => { - crate::tools::sandboxing::ExecApprovalRequirement::Forbidden { reason } - } - }; + let initial_approval_requirement = + clear_proposed_execpolicy_amendment(initial_approval_requirement); + let escalation_approval_requirement = + clear_proposed_execpolicy_amendment(escalation_approval_requirement); let env = build_artifact_env( &installed_runtime, @@ -335,12 +333,35 @@ async fn prepare_artifact_build( timeout_ms: Some(timeout_ms), env, approval_key, + initial_approval_requirement, escalation_approval_requirement, }, _source_dir: source_dir, }) } +fn clear_proposed_execpolicy_amendment( + requirement: crate::tools::sandboxing::ExecApprovalRequirement, +) -> crate::tools::sandboxing::ExecApprovalRequirement { + match requirement { + crate::tools::sandboxing::ExecApprovalRequirement::Skip { bypass_sandbox, .. } => { + crate::tools::sandboxing::ExecApprovalRequirement::Skip { + bypass_sandbox, + proposed_execpolicy_amendment: None, + } + } + crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { reason, .. } => { + crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { + reason, + proposed_execpolicy_amendment: None, + } + } + crate::tools::sandboxing::ExecApprovalRequirement::Forbidden { reason } => { + crate::tools::sandboxing::ExecApprovalRequirement::Forbidden { reason } + } + } +} + async fn ensure_artifact_build_launcher(codex_home: &Path) -> Result { let launcher_path = artifact_build_launcher_path(codex_home); match fs::read_to_string(&launcher_path).await { @@ -525,7 +546,10 @@ fn format_artifact_stderr(output: &ExecToolCallOutput) -> String { mod tests { use super::*; use crate::codex::make_session_and_context; + use crate::config::Constrained; use crate::exec::StreamOutput; + use crate::protocol::AskForApproval; + use crate::protocol::SandboxPolicy; use codex_artifacts::RuntimeEntrypoints; use codex_artifacts::RuntimePathEntry; use pretty_assertions::assert_eq; @@ -787,7 +811,9 @@ mod tests { #[tokio::test] async fn prepare_artifact_build_uses_script_specific_approval_key_without_execpolicy_rule() { - let (session, turn) = make_session_and_context().await; + let (session, mut turn) = make_session_and_context().await; + turn.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + turn.sandbox_policy = Constrained::allow_any(SandboxPolicy::new_read_only_policy()); let runtime = codex_artifacts::InstalledArtifactRuntime::new( PathBuf::from("/runtime"), PINNED_ARTIFACT_RUNTIME_VERSION.to_string(), @@ -844,5 +870,19 @@ mod tests { .proposed_execpolicy_amendment() .is_none() ); + assert_eq!( + prepared.request.initial_approval_requirement, + crate::tools::sandboxing::ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + } + ); + assert_eq!( + prepared.request.escalation_approval_requirement, + crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + } + ); } } diff --git a/codex-rs/core/src/tools/runtimes/artifacts.rs b/codex-rs/core/src/tools/runtimes/artifacts.rs index 1722c6dfaa..cd0c2a73c4 100644 --- a/codex-rs/core/src/tools/runtimes/artifacts.rs +++ b/codex-rs/core/src/tools/runtimes/artifacts.rs @@ -36,6 +36,7 @@ pub(crate) struct ArtifactExecRequest { pub(crate) timeout_ms: Option, pub(crate) env: HashMap, pub(crate) approval_key: ArtifactApprovalKey, + pub(crate) initial_approval_requirement: ExecApprovalRequirement, pub(crate) escalation_approval_requirement: ExecApprovalRequirement, } @@ -77,10 +78,14 @@ impl Approvable for ArtifactRuntime { let command = req.command.clone(); let cwd = req.cwd.clone(); let approval_keys = self.approval_keys(req); - let escalation_approval_requirement = req.escalation_approval_requirement.clone(); + let approval_requirement = if retry_reason.is_some() { + req.escalation_approval_requirement.clone() + } else { + req.initial_approval_requirement.clone() + }; Box::pin(async move { if matches!( - escalation_approval_requirement, + approval_requirement, ExecApprovalRequirement::Forbidden { .. } ) { return ReviewDecision::Denied; @@ -100,7 +105,7 @@ impl Approvable for ArtifactRuntime { cwd, retry_reason, None, - escalation_approval_requirement + approval_requirement .proposed_execpolicy_amendment() .cloned(), None, @@ -127,13 +132,13 @@ impl Approvable for ArtifactRuntime { &self, req: &ArtifactExecRequest, ) -> Option { - Some(req.escalation_approval_requirement.clone()) + Some(req.initial_approval_requirement.clone()) } fn sandbox_mode_for_first_attempt(&self, req: &ArtifactExecRequest) -> SandboxOverride { sandbox_override_for_first_attempt( SandboxPermissions::UseDefault, - &req.escalation_approval_requirement, + &req.initial_approval_requirement, ) } } @@ -194,6 +199,10 @@ mod tests { cwd: PathBuf::from("/tmp"), staged_script: PathBuf::from("/tmp/source.mjs"), }, + initial_approval_requirement: ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, escalation_approval_requirement: ExecApprovalRequirement::Skip { bypass_sandbox: false, proposed_execpolicy_amendment: None, @@ -255,6 +264,10 @@ mod tests { cwd: PathBuf::from("/tmp"), staged_script: PathBuf::from("/tmp/source-one.mjs"), }, + initial_approval_requirement: ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, escalation_approval_requirement: ExecApprovalRequirement::Skip { bypass_sandbox: false, proposed_execpolicy_amendment: None, @@ -277,6 +290,10 @@ mod tests { cwd: PathBuf::from("/tmp"), staged_script: PathBuf::from("/tmp/source-two.mjs"), }, + initial_approval_requirement: ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, escalation_approval_requirement: ExecApprovalRequirement::Skip { bypass_sandbox: false, proposed_execpolicy_amendment: None, @@ -290,7 +307,7 @@ mod tests { } #[test] - fn exec_approval_requirement_uses_request_requirement() { + fn exec_approval_requirement_uses_initial_requirement() { let runtime = ArtifactRuntime; let req = ArtifactExecRequest { command: vec![ @@ -309,21 +326,24 @@ mod tests { cwd: PathBuf::from("/tmp"), staged_script: PathBuf::from("/tmp/source.mjs"), }, + initial_approval_requirement: ExecApprovalRequirement::Forbidden { + reason: "blocked before first attempt".to_string(), + }, escalation_approval_requirement: ExecApprovalRequirement::Forbidden { - reason: "blocked by policy".to_string(), + reason: "blocked on retry".to_string(), }, }; assert_eq!( runtime.exec_approval_requirement(&req), Some(ExecApprovalRequirement::Forbidden { - reason: "blocked by policy".to_string(), + reason: "blocked before first attempt".to_string(), }) ); } #[test] - fn sandbox_mode_for_first_attempt_honors_bypass_sandbox_requirement() { + fn sandbox_mode_for_first_attempt_uses_initial_requirement() { let runtime = ArtifactRuntime; let req = ArtifactExecRequest { command: vec![ @@ -342,6 +362,10 @@ mod tests { cwd: PathBuf::from("/tmp"), staged_script: PathBuf::from("/tmp/source.mjs"), }, + initial_approval_requirement: ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, escalation_approval_requirement: ExecApprovalRequirement::Skip { bypass_sandbox: true, proposed_execpolicy_amendment: None, @@ -350,7 +374,7 @@ mod tests { assert_eq!( runtime.sandbox_mode_for_first_attempt(&req), - SandboxOverride::BypassSandboxFirstAttempt + SandboxOverride::NoOverride ); } }