[codex] require review for sandbox-writable exec paths

This commit is contained in:
Ollie Matthews
2026-04-29 15:54:05 -07:00
parent aaf88f1ba3
commit f810207e00
2 changed files with 86 additions and 9 deletions
+58 -9
View File
@@ -250,7 +250,16 @@ impl ExecPolicyManager {
// Keep heredoc prefix parsing for rule evaluation so existing
// allow/prompt/forbidden rules still apply, but avoid auto-derived
// amendments when only the heredoc fallback parser matched.
let auto_amendment_allowed = !used_complex_parsing;
let sandbox_writable_executable_requires_review = commands.iter().any(|cmd| {
sandbox_writable_executable_path_requires_review(
cmd,
file_system_sandbox_policy,
sandbox_cwd,
sandbox_permissions,
)
});
let auto_amendment_allowed =
!used_complex_parsing && !sandbox_writable_executable_requires_review;
let exec_policy_fallback = |cmd: &[String]| {
render_decision_for_unmatched_command(
approval_policy,
@@ -271,14 +280,18 @@ impl ExecPolicyManager {
&match_options,
);
let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule(
prefix_rule.as_ref(),
&evaluation.matched_rules,
exec_policy.as_ref(),
&commands,
&exec_policy_fallback,
&match_options,
);
let requested_amendment = if auto_amendment_allowed {
derive_requested_execpolicy_amendment_from_prefix_rule(
prefix_rule.as_ref(),
&evaluation.matched_rules,
exec_policy.as_ref(),
&commands,
&exec_policy_fallback,
&match_options,
)
} else {
None
};
match evaluation.decision {
Decision::Forbidden => ExecApprovalRequirement::Forbidden {
@@ -590,6 +603,15 @@ pub fn render_decision_for_unmatched_command(
sandbox_permissions: SandboxPermissions,
used_complex_parsing: bool,
) -> Decision {
if sandbox_writable_executable_path_requires_review(
command,
file_system_sandbox_policy,
sandbox_cwd,
sandbox_permissions,
) {
return Decision::Prompt;
}
if is_known_safe_command(command) && !used_complex_parsing {
return Decision::Allow;
}
@@ -678,6 +700,33 @@ pub fn render_decision_for_unmatched_command(
}
}
fn sandbox_writable_executable_path_requires_review(
command: &[String],
file_system_sandbox_policy: &FileSystemSandboxPolicy,
sandbox_cwd: &Path,
sandbox_permissions: SandboxPermissions,
) -> bool {
if !sandbox_permissions.requests_sandbox_override()
|| !matches!(
file_system_sandbox_policy.kind,
FileSystemSandboxKind::Restricted
)
|| file_system_sandbox_policy.has_full_disk_write_access()
{
return false;
}
let Some(program) = command.first() else {
return false;
};
let program_path = Path::new(program);
if !program_path.is_absolute() && program_path.components().count() <= 1 {
return false;
}
file_system_sandbox_policy.can_write_path_with_cwd(program_path, sandbox_cwd)
}
fn profile_is_managed_read_only(
permission_profile: &PermissionProfile,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
+28
View File
@@ -997,6 +997,34 @@ fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() {
);
}
#[tokio::test]
async fn sandbox_writable_safe_command_path_requires_approval_for_escalation() {
let command = vec!["/tmp/cat".to_string(), "/dev/null".to_string()];
let manager = ExecPolicyManager::default();
let requirement = manager
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::OnRequest,
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_workspace_write_policy(),
),
file_system_sandbox_policy: &workspace_write_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: SandboxPermissions::RequireEscalated,
prefix_rule: None,
})
.await;
assert_eq!(
requirement,
ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: None,
}
);
}
#[test]
fn managed_cwd_write_profile_is_not_read_only() {
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![