fix(permissions): fix symlinked writable roots in sandbox permissions#15981
fix(permissions): fix symlinked writable roots in sandbox permissions#15981viyatb-oai merged 18 commits intomainfrom
Conversation
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 532001fabb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: Codex noreply@openai.com
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5072f6d2fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
etraut-openai
left a comment
There was a problem hiding this comment.
Whew, keeping track of when paths are already canonicalized and when they are not is difficult. We should think about whether there's a way to make this clearer through variable names or even types, but that's beyond the scope of this PR. I'm going to have to place some trust in codex's reviews because I can't say with confidence that there's no bug here. I did spend quite a bit of time reviewing, and I didn't find any issues. This will be a nice fix to get in place!
Summary
Root cause
Permission normalization canonicalized symlinked writable roots and cwd to their real targets too early. That drifted policy checks away from the logical paths the sandboxed process can actually address, while bwrap still needed the real targets for mounts. The mismatch caused shell and apply_patch failures on symlinked writable roots.
Impact
Fixes #15781.
Also fixes #17079:
Related to #15157:
This should also fix #14672, #14694, #14715, and #15725:
TMPDIR#14672, apply_patch fails when writable roots are symlinked (bwrap bind error) #14694, and apply_patch still fails with symlinked ~/.codex (v0.115.0-alpha.23) #14715 are the same Linux symlinked-writable-root/bwrap family as shell write denial vs apply_patch success on symlinked paths #15781.Notes