mirror of
https://github.com/openai/codex.git
synced 2026-05-16 01:02:48 +00:00
fix(exec-policy) use is_known_safe_command less (#20305)
## Summary Restricts behavior of `is_known_safe_command` only to modes where it is explicitly part of the documented behavior: - when `environment_lacks_sandbox_protections` - in `AskForApproval::UnlessTrusted` Notably, as a result of this, escalations for commands that pass `is_known_safe_commands` are no longer auto-approved in AskForApproval::OnRequest or AskForApproval::Granular. ## Testing - [x] Updated unit tests - [x] Updated approvals scenario tests. --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -649,9 +649,6 @@ pub(crate) fn render_decision_for_unmatched_command(
|
||||
codex_shell_command::is_safe_command::is_safe_powershell_words(command)
|
||||
}
|
||||
};
|
||||
if is_known_safe && !used_complex_parsing {
|
||||
return Decision::Allow;
|
||||
}
|
||||
|
||||
// On Windows, ReadOnly sandbox is not a real sandbox, so special-case it
|
||||
// here.
|
||||
@@ -662,6 +659,14 @@ pub(crate) fn render_decision_for_unmatched_command(
|
||||
sandbox_cwd,
|
||||
);
|
||||
|
||||
if is_known_safe
|
||||
&& !used_complex_parsing
|
||||
&& (approval_policy == AskForApproval::UnlessTrusted
|
||||
|| environment_lacks_sandbox_protections)
|
||||
{
|
||||
return Decision::Allow;
|
||||
}
|
||||
|
||||
// If the command is flagged as dangerous or we have no sandbox protection,
|
||||
// we should never allow it to run without approval.
|
||||
//
|
||||
|
||||
@@ -1144,6 +1144,29 @@ fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn known_safe_on_request_still_prompts_for_restricted_sandbox_escalation() {
|
||||
let command = vec!["echo".to_string(), "hello".to_string()];
|
||||
|
||||
assert_eq!(
|
||||
Decision::Prompt,
|
||||
render_decision_for_unmatched_command(
|
||||
&command,
|
||||
UnmatchedCommandContext {
|
||||
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,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
},
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_cwd_write_profile_is_not_read_only() {
|
||||
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
|
||||
@@ -1230,6 +1253,55 @@ async fn exec_approval_requirement_prompts_for_inline_additional_permissions_und
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_approval_requirement_prompts_for_known_safe_escalation_under_on_request() {
|
||||
assert_exec_approval_requirement_for_command(
|
||||
ExecApprovalRequirementScenario {
|
||||
policy_src: None,
|
||||
command: vec!["echo".to_string(), "hello".to_string()],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
|
||||
file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"echo".to_string(),
|
||||
"hello".to_string(),
|
||||
])),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_approval_requirement_rejects_known_safe_escalation_when_granular_sandbox_is_disabled()
|
||||
{
|
||||
assert_exec_approval_requirement_for_command(
|
||||
ExecApprovalRequirementScenario {
|
||||
policy_src: None,
|
||||
command: vec!["echo".to_string(), "hello".to_string()],
|
||||
approval_policy: AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
|
||||
file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
reason: REJECT_SANDBOX_APPROVAL_REASON.to_string(),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_granular_sandbox_is_disabled()
|
||||
{
|
||||
|
||||
@@ -15,6 +15,7 @@ use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ExecApprovalRequestEvent;
|
||||
use codex_protocol::protocol::ExecPolicyAmendment;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
@@ -965,6 +966,46 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
output_contains: "rejected by user",
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "known_safe_escalation_on_request_requires_approval",
|
||||
approval_policy: OnRequest,
|
||||
sandbox_policy: workspace_write(false),
|
||||
action: ActionKind::RunCommand {
|
||||
command: "echo known-safe-escalation",
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
features: vec![],
|
||||
model_override: Some("gpt-5.2"),
|
||||
outcome: Outcome::ExecApprovalWithAmendment {
|
||||
decision: ReviewDecision::Denied,
|
||||
expected_reason: None,
|
||||
expected_execpolicy_amendment: Some(&["echo", "known-safe-escalation"]),
|
||||
},
|
||||
expectation: Expectation::CommandFailure {
|
||||
output_contains: "rejected by user",
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "known_safe_escalation_granular_sandbox_disabled_rejects",
|
||||
approval_policy: Granular(GranularApprovalConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
sandbox_policy: workspace_write(false),
|
||||
action: ActionKind::RunCommand {
|
||||
command: "echo known-safe-escalation-granular-disabled",
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
features: vec![],
|
||||
model_override: Some("gpt-5.2"),
|
||||
outcome: Outcome::Auto,
|
||||
expectation: Expectation::CommandFailure {
|
||||
output_contains: "you should not ask for escalated permissions",
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "cat_heredoc_file_redirect_prefix_rule_requires_escalation_approval",
|
||||
approval_policy: OnRequest,
|
||||
|
||||
Reference in New Issue
Block a user