fix: honor parent approvals for intercepted execs

This commit is contained in:
Michael Bolin
2026-06-05 09:11:54 -07:00
parent b558b79f5a
commit 896feb97bb
3 changed files with 549 additions and 29 deletions

View File

@@ -17,6 +17,7 @@ use crate::shell::ShellType;
use crate::tools::runtimes::build_sandbox_command;
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
use crate::tools::runtimes::prepend_zsh_fork_bin_to_path;
use crate::tools::sandboxing::ExecApprovalRequirement;
use crate::tools::sandboxing::PermissionRequestPayload;
use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::ToolCtx;
@@ -100,6 +101,19 @@ fn approval_sandbox_permissions(
}
}
fn parent_approved_sandbox_override(
sandbox_permissions: SandboxPermissions,
additional_permissions_preapproved: bool,
exec_approval_requirement: &ExecApprovalRequirement,
) -> bool {
sandbox_permissions.requests_sandbox_override()
&& (additional_permissions_preapproved
|| matches!(
exec_approval_requirement,
ExecApprovalRequirement::NeedsApproval { .. }
))
}
pub(super) async fn try_run_zsh_fork(
req: &ShellRequest,
attempt: &SandboxAttempt<'_>,
@@ -228,6 +242,11 @@ pub(super) async fn try_run_zsh_fork(
sandbox_permissions: req.sandbox_permissions,
approval_sandbox_permissions,
prompt_permissions: req.additional_permissions.clone(),
parent_sandbox_override_approved: parent_approved_sandbox_override(
req.sandbox_permissions,
req.additional_permissions_preapproved,
&req.exec_approval_requirement,
),
stopwatch: stopwatch.clone(),
};
@@ -304,6 +323,11 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
req.additional_permissions_preapproved,
),
prompt_permissions: req.additional_permissions.clone(),
parent_sandbox_override_approved: parent_approved_sandbox_override(
req.sandbox_permissions,
req.additional_permissions_preapproved,
&req.exec_approval_requirement,
),
stopwatch: Stopwatch::unlimited(),
};
@@ -336,6 +360,7 @@ struct CoreShellActionProvider {
sandbox_permissions: SandboxPermissions,
approval_sandbox_permissions: SandboxPermissions,
prompt_permissions: Option<AdditionalPermissionProfile>,
parent_sandbox_override_approved: bool,
stopwatch: Stopwatch,
}
@@ -639,6 +664,19 @@ impl EscalationPolicy for CoreShellActionProvider {
SandboxPermissions::RequireEscalated => unsandboxed_allowed,
SandboxPermissions::WithAdditionalPermissions => true,
};
// The parent shell/unified-exec approval already covered unmatched
// prompts caused by its sandbox override. Explicit exec-policy rules
// still keep their own decisions. Guardian-routed turns keep the
// prompt so Guardian can review each intercepted execve independently.
let decision = if self.parent_sandbox_override_approved
&& !routes_approval_to_guardian(&self.turn)
&& evaluation.decision == Decision::Prompt
&& !decision_driven_by_policy
{
Decision::Allow
} else {
evaluation.decision
};
let decision_source = if decision_driven_by_policy {
DecisionSource::PrefixRule
@@ -656,7 +694,7 @@ impl EscalationPolicy for CoreShellActionProvider {
),
};
self.process_decision(
evaluation.decision,
decision,
needs_escalation,
program,
argv,

View File

@@ -11,6 +11,7 @@ use crate::config::Constrained;
use crate::sandboxing::SandboxPermissions;
use crate::session::tests::make_session_and_context;
use anyhow::Context;
use codex_config::types::ApprovalsReviewer;
use codex_execpolicy::Decision;
use codex_execpolicy::Evaluation;
use codex_execpolicy::PolicyParser;
@@ -153,6 +154,42 @@ fn approval_sandbox_permissions_only_downgrades_preapproved_additional_permissio
);
}
#[test]
fn parent_approved_sandbox_override_tracks_parent_approval_sources() {
assert!(super::parent_approved_sandbox_override(
SandboxPermissions::RequireEscalated,
/*additional_permissions_preapproved*/ false,
&crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: None,
},
));
assert!(super::parent_approved_sandbox_override(
SandboxPermissions::WithAdditionalPermissions,
/*additional_permissions_preapproved*/ true,
&crate::tools::sandboxing::ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
},
));
assert!(!super::parent_approved_sandbox_override(
SandboxPermissions::UseDefault,
/*additional_permissions_preapproved*/ false,
&crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: None,
},
));
assert!(!super::parent_approved_sandbox_override(
SandboxPermissions::RequireEscalated,
/*additional_permissions_preapproved*/ false,
&crate::tools::sandboxing::ExecApprovalRequirement::Skip {
bypass_sandbox: true,
proposed_execpolicy_amendment: None,
},
));
}
#[test]
fn extract_shell_script_preserves_login_flag() {
assert_eq!(
@@ -428,6 +465,7 @@ async fn preapproved_additional_permissions_escalate_intercepted_exec() -> anyho
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
approval_sandbox_permissions: SandboxPermissions::UseDefault,
prompt_permissions: Some(requested_permissions),
parent_sandbox_override_approved: true,
stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)),
};
@@ -449,6 +487,143 @@ async fn preapproved_additional_permissions_escalate_intercepted_exec() -> anyho
Ok(())
}
#[tokio::test]
async fn parent_require_escalated_approval_escalates_intercepted_exec() -> anyhow::Result<()> {
let (session, turn_context) = make_session_and_context().await;
let workdir = test_sandbox_cwd();
let provider = CoreShellActionProvider {
policy: Arc::new(RwLock::new(codex_execpolicy::Policy::empty())),
session: Arc::new(session),
turn: Arc::new(turn_context),
call_id: "parent-require-escalated".to_string(),
tool_name: GuardianCommandSource::Shell,
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::workspace_write(),
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
sandbox_policy_cwd: workdir.clone(),
sandbox_permissions: SandboxPermissions::RequireEscalated,
approval_sandbox_permissions: SandboxPermissions::RequireEscalated,
prompt_permissions: None,
parent_sandbox_override_approved: true,
stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)),
};
let action = codex_shell_escalation::EscalationPolicy::determine_action(
&provider,
&AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?,
&["curl".to_string(), "example.com".to_string()],
&workdir,
)
.await?;
assert_eq!(
action,
codex_shell_escalation::EscalationDecision::Escalate(EscalationExecution::Unsandboxed)
);
Ok(())
}
#[tokio::test]
async fn parent_approval_does_not_bypass_guardian_routed_exec_prompt() -> anyhow::Result<()> {
let (session, mut turn_context) = make_session_and_context().await;
let approval_policy = AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: false,
rules: true,
skill_approval: true,
request_permissions: true,
mcp_elicitations: true,
});
turn_context.approval_policy = Constrained::allow_any(approval_policy);
let mut config = (*turn_context.config).clone();
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
turn_context.config = Arc::new(config);
let workdir = test_sandbox_cwd();
let provider = CoreShellActionProvider {
policy: Arc::new(RwLock::new(codex_execpolicy::Policy::empty())),
session: Arc::new(session),
turn: Arc::new(turn_context),
call_id: "parent-guardian-routed".to_string(),
tool_name: GuardianCommandSource::Shell,
approval_policy,
permission_profile: PermissionProfile::workspace_write(),
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
sandbox_policy_cwd: workdir.clone(),
sandbox_permissions: SandboxPermissions::RequireEscalated,
approval_sandbox_permissions: SandboxPermissions::RequireEscalated,
prompt_permissions: None,
parent_sandbox_override_approved: true,
stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)),
};
let action = codex_shell_escalation::EscalationPolicy::determine_action(
&provider,
&AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?,
&["curl".to_string(), "example.com".to_string()],
&workdir,
)
.await?;
assert_eq!(
action,
codex_shell_escalation::EscalationDecision::Deny {
reason: Some("Execution forbidden by policy".to_string()),
}
);
Ok(())
}
#[tokio::test]
async fn parent_approval_does_not_override_intercepted_exec_policy_prompt() -> anyhow::Result<()> {
let (session, turn_context) = make_session_and_context().await;
let mut parser = PolicyParser::new();
parser.parse(
"test.rules",
r#"prefix_rule(pattern = ["curl"], decision = "prompt")"#,
)?;
let workdir = test_sandbox_cwd();
let provider = CoreShellActionProvider {
policy: Arc::new(RwLock::new(parser.build())),
session: Arc::new(session),
turn: Arc::new(turn_context),
call_id: "parent-policy-prompt".to_string(),
tool_name: GuardianCommandSource::Shell,
approval_policy: AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: true,
rules: false,
skill_approval: true,
request_permissions: true,
mcp_elicitations: true,
}),
permission_profile: PermissionProfile::workspace_write(),
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
sandbox_policy_cwd: workdir.clone(),
sandbox_permissions: SandboxPermissions::RequireEscalated,
approval_sandbox_permissions: SandboxPermissions::RequireEscalated,
prompt_permissions: None,
parent_sandbox_override_approved: true,
stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)),
};
let action = codex_shell_escalation::EscalationPolicy::determine_action(
&provider,
&AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?,
&["curl".to_string(), "example.com".to_string()],
&workdir,
)
.await?;
assert_eq!(
action,
codex_shell_escalation::EscalationDecision::Deny {
reason: Some("Execution forbidden by policy".to_string()),
}
);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Result<()> {
let (session, mut turn_context) = make_session_and_context().await;
@@ -561,6 +736,7 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul
sandbox_permissions: SandboxPermissions::RequireEscalated,
approval_sandbox_permissions: SandboxPermissions::RequireEscalated,
prompt_permissions: None,
parent_sandbox_override_approved: false,
stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)),
};
@@ -775,6 +951,7 @@ prefix_rule(pattern = ["{cat_path_literal}"], decision = "allow")
sandbox_permissions: SandboxPermissions::UseDefault,
approval_sandbox_permissions: SandboxPermissions::UseDefault,
prompt_permissions: None,
parent_sandbox_override_approved: false,
stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)),
};
@@ -818,6 +995,7 @@ async fn denied_reads_keep_granular_sandbox_rejection_for_escalation() -> anyhow
sandbox_permissions: SandboxPermissions::RequireEscalated,
approval_sandbox_permissions: SandboxPermissions::RequireEscalated,
prompt_permissions: None,
parent_sandbox_override_approved: false,
stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)),
};

View File

@@ -2,6 +2,8 @@
use anyhow::Context;
use anyhow::Result;
use codex_config::permissions_toml::FilesystemPermissionToml;
use codex_config::permissions_toml::PermissionProfileToml;
use codex_config::types::ApprovalsReviewer;
use codex_core::CodexThread;
use codex_core::config::Constrained;
@@ -56,6 +58,7 @@ use std::env;
use std::fs;
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
@@ -370,6 +373,66 @@ fn exec_command_event(
Ok(ev_function_call(call_id, "exec_command", &args_str))
}
fn denied_read_permission_profile(denied_path: &Path) -> Result<PermissionProfile> {
let mut profile = toml::from_str::<PermissionProfileToml>(
r#"
[filesystem]
"/" = "read"
":project_roots" = "write"
[network]
enabled = false
"#,
)
.context("test permission profile should deserialize")?;
let filesystem = profile
.filesystem
.as_mut()
.context("test permission profile should include filesystem entries")?;
filesystem.entries.insert(
denied_path.to_string_lossy().to_string(),
FilesystemPermissionToml::Access(FileSystemAccessMode::Deny),
);
let entries = filesystem
.entries
.iter()
.map(|(path, permission)| {
let FilesystemPermissionToml::Access(access) = permission else {
anyhow::bail!("unexpected scoped filesystem permission in test profile: {path}");
};
let path = match path.as_str() {
"/" => FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
":project_roots" => FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
_ if *access == FileSystemAccessMode::Deny => FileSystemPath::GlobPattern {
pattern: path.clone(),
},
_ => anyhow::bail!("unexpected filesystem entry in test profile: {path}"),
};
Ok(FileSystemSandboxEntry {
path,
access: *access,
})
})
.collect::<Result<Vec<_>>>()?;
let mut file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(entries);
file_system_sandbox_policy.glob_scan_max_depth = filesystem.glob_scan_max_depth;
assert!(
file_system_sandbox_policy.has_denied_read_restrictions(),
"test must exercise a permission profile with denied reads"
);
Ok(PermissionProfile::from_runtime_permissions(
&file_system_sandbox_policy,
NetworkSandboxPolicy::Restricted,
))
}
#[derive(Clone)]
enum Expectation {
FileCreated {
@@ -2815,34 +2878,7 @@ async fn unified_exec_zsh_fork_parent_approval_preserves_denied_reads() -> Resul
let denied_path = denied_dir.path().join("secret.env");
let secret = "unified-exec-zsh-fork-denied-read-secret";
fs::write(&denied_path, format!("{secret}\n"))?;
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: denied_path.to_string_lossy().to_string(),
},
access: FileSystemAccessMode::Deny,
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
]);
assert!(
file_system_sandbox_policy.has_denied_read_restrictions(),
"test must exercise a permission profile with denied reads"
);
let permission_profile = PermissionProfile::from_runtime_permissions(
&file_system_sandbox_policy,
NetworkSandboxPolicy::Restricted,
);
let permission_profile = denied_read_permission_profile(&denied_path)?;
let approval_policy = AskForApproval::OnRequest;
let command = format!("cat {denied_path:?}");
@@ -2942,6 +2978,274 @@ async fn unified_exec_zsh_fork_parent_approval_preserves_denied_reads() -> Resul
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[cfg(unix)]
async fn unified_exec_zsh_fork_parent_approval_escalates_intercepted_exec() -> Result<()> {
skip_if_no_network!(Ok(()));
let Some(runtime) = zsh_fork_runtime("unified-exec zsh-fork parent approval test")? else {
return Ok(());
};
let approval_policy = AskForApproval::OnRequest;
let permission_profile = restrictive_workspace_write_profile();
let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?;
let outside_path = outside_dir
.path()
.join("unified-exec-zsh-fork-parent-approval.txt");
let command = format!("printf hi > {outside_path:?}");
let server = start_mock_server().await;
let outside_path_for_hook = outside_path.clone();
let test = build_unified_exec_zsh_fork_test(
&server,
runtime,
approval_policy,
permission_profile,
move |_home| {
let _ = fs::remove_file(&outside_path_for_hook);
},
)
.await?;
let call_id = "uexec-zsh-fork-parent-approval";
let event = exec_command_event(
call_id,
&command,
Some(30_000),
SandboxPermissions::RequireEscalated,
Some("write outside the workspace for the test"),
)?;
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-uexec-zsh-fork-parent-approval-1"),
event,
ev_completed("resp-uexec-zsh-fork-parent-approval-1"),
]),
)
.await;
let results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-uexec-zsh-fork-parent-approval-1", "done"),
ev_completed("resp-uexec-zsh-fork-parent-approval-2"),
]),
)
.await;
let session_model = test.session_configured.model.clone();
let (sandbox_policy, permission_profile) = turn_permission_fields(
test.session_configured.permission_profile.clone(),
test.cwd.path(),
);
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "run approved unified exec through zsh fork".into(),
text_elements: Vec::new(),
}],
environments: None,
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
cwd: Some(test.cwd.path().to_path_buf()),
approval_policy: Some(approval_policy),
approvals_reviewer: Some(ApprovalsReviewer::User),
sandbox_policy: Some(sandbox_policy),
permission_profile,
collaboration_mode: Some(codex_protocol::config_types::CollaborationMode {
mode: codex_protocol::config_types::ModeKind::Default,
settings: codex_protocol::config_types::Settings {
model: session_model,
reasoning_effort: None,
developer_instructions: None,
},
}),
..Default::default()
},
})
.await?;
let approval = expect_exec_approval(&test, &command).await;
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion_without_approval(&test).await;
let result = parse_result(&results.single_request().function_call_output(call_id));
assert_eq!(
result.exit_code.unwrap_or(0),
0,
"approved unified exec zsh-fork command should complete: {}",
result.stdout
);
let contents = fs::read_to_string(&outside_path)
.with_context(|| format!("read {}", outside_path.display()))?;
assert_eq!(contents, "hi");
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[cfg(unix)]
async fn unified_exec_zsh_fork_parent_approval_keeps_explicit_prompt_rule() -> Result<()> {
skip_if_no_network!(Ok(()));
let Some(runtime) = zsh_fork_runtime("unified-exec zsh-fork prompt rule approval test")? else {
return Ok(());
};
let approval_policy = AskForApproval::OnRequest;
let permission_profile = restrictive_workspace_write_profile();
let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?;
let outside_path = outside_dir
.path()
.join("unified-exec-zsh-fork-explicit-prompt-rule.txt");
let command = format!("touch {outside_path:?}");
let rules = r#"prefix_rule(pattern=["touch"], decision="prompt")"#.to_string();
let server = start_mock_server().await;
let outside_path_for_hook = outside_path.clone();
let test = build_unified_exec_zsh_fork_test(
&server,
runtime,
approval_policy,
permission_profile,
move |home| {
let _ = fs::remove_file(&outside_path_for_hook);
let rules_dir = home.join("rules");
fs::create_dir_all(&rules_dir).unwrap();
fs::write(rules_dir.join("default.rules"), &rules).unwrap();
},
)
.await?;
let call_id = "uexec-zsh-fork-parent-approval-explicit-prompt-rule";
let event = exec_command_event(
call_id,
&command,
Some(30_000),
SandboxPermissions::RequireEscalated,
Some("write outside the workspace for the test"),
)?;
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-uexec-zsh-fork-prompt-rule-1"),
event,
ev_completed("resp-uexec-zsh-fork-prompt-rule-1"),
]),
)
.await;
let results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-uexec-zsh-fork-prompt-rule-1", "done"),
ev_completed("resp-uexec-zsh-fork-prompt-rule-2"),
]),
)
.await;
let session_model = test.session_configured.model.clone();
let (sandbox_policy, permission_profile) = turn_permission_fields(
test.session_configured.permission_profile.clone(),
test.cwd.path(),
);
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "run approved unified exec prompt rule through zsh fork".into(),
text_elements: Vec::new(),
}],
environments: None,
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
cwd: Some(test.cwd.path().to_path_buf()),
approval_policy: Some(approval_policy),
approvals_reviewer: Some(ApprovalsReviewer::User),
sandbox_policy: Some(sandbox_policy),
permission_profile,
collaboration_mode: Some(codex_protocol::config_types::CollaborationMode {
mode: codex_protocol::config_types::ModeKind::Default,
settings: codex_protocol::config_types::Settings {
model: session_model,
reasoning_effort: None,
developer_instructions: None,
},
}),
..Default::default()
},
})
.await?;
let parent_approval = expect_exec_approval(&test, &command).await;
test.codex
.submit(Op::ExecApproval {
id: parent_approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
let approval_event = wait_for_event_with_timeout(
&test.codex,
|event| {
matches!(
event,
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
)
},
Duration::from_secs(10),
)
.await;
let EventMsg::ExecApprovalRequest(inner_approval) = approval_event else {
panic!("expected explicit prompt rule approval before completion");
};
assert!(
inner_approval
.command
.iter()
.any(|arg| arg.ends_with("/touch"))
&& inner_approval
.command
.iter()
.any(|arg| arg == outside_path.to_string_lossy().as_ref()),
"expected explicit prompt rule approval for intercepted touch, got: {:?}",
inner_approval.command
);
test.codex
.submit(Op::ExecApproval {
id: inner_approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
let result = parse_result(&results.single_request().function_call_output(call_id));
assert_eq!(
result.exit_code.unwrap_or(0),
0,
"approved unified exec zsh-fork prompt-rule command should complete: {}",
result.stdout
);
assert!(
outside_path.exists(),
"approved intercepted touch should create the out-of-workspace file"
);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(unix)]
async fn invalid_requested_prefix_rule_falls_back_for_compound_command() -> Result<()> {