openai / openai/codex

AskForApproval::UnlessTrusted always prompts, bypassing writable-paths check

Open Beginner friendly
#43,430 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug CLI sandbox windows-os
Dominant language
Rust
Stars
125k
Forks
19.4k
PR merge metrics
PR metrics pending

Description

What version of Codex CLI is running?

codex-cli 0.147.0

What subscription do you have?

go

Which model were you using?

gpt-5.5

What platform is your computer?

Microsoft Windows NT 10.0.26200.0 x64

What terminal emulator and version are you using (if applicable)?

Windows Terminal 1.24.11911.0 (PowerShell)

Codex doctor report
not available
What issue are you seeing?

When the approval policy is AskForApproval::UnlessTrusted, the safety check function returns SafetyCheck::AskUser unconditionally, without first checking whether the patch's target paths are fully contained within writable roots. This means patches that would be silently auto-approved under AskForApproval::Never still force a user prompt under UnlessTrusted, even when they touch nothing outside writable directories.

Location: core/src/safety.rs, lines 47–51:

AskForApproval::UnlessTrusted => {
return SafetyCheck::AskUser;
}

The author's own inline TODO flags uncertainty about this branch, suggesting it may be an unintentional regression rather than deliberate design.

Expected behavior: Under UnlessTrusted, the writable-paths check should still run. The user should only be prompted (SafetyCheck::AskUser) if the patch touches paths outside the writable roots. Patches fully constrained to writable directories should be auto-approved, consistent with how AskForApproval::Never behaves for the same paths.

Actual behavior: Every patch under UnlessTrusted immediately short-circuits to SafetyCheck::AskUser, regardless of whether the writable-paths check would have passed.

Impact:

  • Inconsistent UX: users on UnlessTrusted are prompted far more often than the policy's intent suggests.
  • Trust in the approval model erodes if users are prompted for routine, safe writable-path edits.
  • Acknowledged by the original author as likely incorrect.

Suggested fix: Fall through to the writable-paths check before returning AskUser:

AskForApproval::UnlessTrusted => {
if !paths_are_writable(&patch_paths, &writable_roots) {
return SafetyCheck::AskUser;
}
}

This was found via source review, not a reproduced runtime crash — no local repro steps to include.

What steps can reproduce the bug?

Not reproduced via the CLI directly — this was found through source code review of core/src/safety.rs.

Based on the code, the bug should reproduce as follows:

  1. Set approval policy to AskForApproval::UnlessTrusted.
  2. Run a patch/command whose file changes are fully contained within a writable root (i.e., a patch that AskForApproval::Never would auto-approve).
  3. Observe that Codex still prompts for approval (SafetyCheck::AskUser), instead of auto-approving as the writable-paths check would allow.

Expected: step 3 should auto-approve when all target paths are writable.
Actual: step 3 always prompts, regardless of path writability.

What is the expected behavior?

Under AskForApproval::UnlessTrusted, the safety check should still run the writable-paths check before deciding whether to prompt the user. If a patch's target paths are fully contained within writable roots, it should be auto-approved — just as it would be under AskForApproval::Never for the same paths. The user should only see a SafetyCheck::AskUser prompt when the patch touches paths outside the writable roots.

Additional information

This appears to be a known/suspected issue rather than a newly discovered one — the code itself contains an inline TODO from the original author expressing uncertainty about this exact branch:

// TODO(ragona): I'm not sure this is actually correct? I believe in this case
// we want to continue to the writable paths check before asking the user.

This suggests the current behavior may be an unintentional regression rather than deliberate design, and the fix is likely low-risk: fall through to the existing writable-paths check (used by the Never arm) instead of returning early.

Happy to open a PR with the fix if a maintainer confirms the intended behavior described above.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in core/src/safety.rs at lines 47–51 and trace the safety check through the existing writable-paths logic. Verify the behavior for AskForApproval::UnlessTrusted with paths inside and outside writable roots. Done means writable paths avoid an unnecessary SafetyCheck::AskUser prompt, while paths outside writable roots still prompt.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.