AskForApproval::UnlessTrusted always prompts, bypassing writable-paths check
Nobody has claimed this yet.
- 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:
- Set approval policy to AskForApproval::UnlessTrusted.
- Run a patch/command whose file changes are fully contained within a writable root (i.e., a patch that AskForApproval::Never would auto-approve).
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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