Skip to content

fix(exec-policy) use is_known_safe_command less#20305

Merged
owenlin0 merged 3 commits into
mainfrom
dh--exec-policy-no-safe-command
May 11, 2026
Merged

fix(exec-policy) use is_known_safe_command less#20305
owenlin0 merged 3 commits into
mainfrom
dh--exec-policy-no-safe-command

Conversation

@dylan-hurd-oai
Copy link
Copy Markdown
Collaborator

@dylan-hurd-oai dylan-hurd-oai commented Apr 30, 2026

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

  • Updated unit tests
  • Updated approvals scenario tests.

@dylan-hurd-oai dylan-hurd-oai requested a review from a team as a code owner April 30, 2026 02:16
Copy link
Copy Markdown
Contributor

@evawong-oai evawong-oai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review done. I approve this PR.

Non blocking: while reviewing this file I noticed a pre existing issue in exec policy loading. A malformed rules file currently falls back to an empty policy, which can drop explicit rules for the session. This does not look introduced by this PR, and this PR narrows the approval bypass surface, so I would not block this change on it. It should be tracked as a separate follow up.

Research done:

  1. Reviewed PR 20305 from base f3e0a64 to head 116859a.
  2. Triage covered the three changed files. The runtime review focused on the exec policy implementation. The test files were used as behavior evidence.
  3. Confirmed the PR makes known safe command auto allow narrower. Under OnRequest, known safe sandbox escalation now prompts. Under Granular with sandbox approval disabled, it is forbidden.
  4. Found one Medium reviewed file issue. It appears pre existing and not introduced by this PR.
  5. Dedupe was skipped because there was one finding. Rerank completed. High severity corruption and RCE validation was skipped because there were no high severity corruption or RCE findings.
  6. GitHub checks are passing on the current head.

@dylan-hurd-oai dylan-hurd-oai force-pushed the dh--exec-policy-no-safe-command branch from 116859a to 227b697 Compare April 30, 2026 06:23
Base automatically changed from dh--heredoc-redirect-patch to main May 1, 2026 01:05
@dylan-hurd-oai dylan-hurd-oai force-pushed the dh--exec-policy-no-safe-command branch from b3d1147 to 0a9e00a Compare May 1, 2026 03:09
dylan-hurd-oai added a commit that referenced this pull request May 1, 2026
Co-authored-by: Codex <noreply@openai.com>
dylan-hurd-oai added a commit that referenced this pull request May 5, 2026
Co-authored-by: Codex <noreply@openai.com>
@dylan-hurd-oai dylan-hurd-oai force-pushed the dh--exec-policy-no-safe-command branch from 4027374 to 7c62701 Compare May 5, 2026 20:15
@dylan-hurd-oai dylan-hurd-oai force-pushed the dh--exec-policy-no-safe-command branch from 7c62701 to d6e4688 Compare May 11, 2026 03:14
@owenlin0 owenlin0 merged commit e783dab into main May 11, 2026
27 checks passed
@owenlin0 owenlin0 deleted the dh--exec-policy-no-safe-command branch May 11, 2026 18:37
@github-actions github-actions Bot locked and limited conversation to collaborators May 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants