never let sandbox write to .codex/ or .codex/.sandbox/ #8683
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75b36be64c
ℹ️ 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".
| roots.retain(|root| { | ||
| let key = crate::audit::normalize_path_key(root); | ||
| key != codex_home_key && key != sbx_dir_key && !key.starts_with(&sbx_dir_prefix) |
There was a problem hiding this comment.
Because filter_sensitive_write_roots now strips CODEX_HOME/.sandbox from every payload, the common refresh-only path won’t touch that directory. On machines that already ran setup before this change (where .sandbox was granted capability write via write_roots), those ACEs will persist indefinitely since refresh-only does not call lock_sandbox_dir to reset the DACL. That means upgrading doesn’t actually revoke sandbox write access to .sandbox unless a full setup is forced. Consider explicitly re-locking/removing ACLs during refresh or bumping the setup version to guarantee a full setup run.
Useful? React with 👍 / 👎.
Never treat .codex or .codex/.sandbox as a workspace root.
Handle write permissions to .codex/.sandbox in a single method so that the sandbox setup/runner can write logs and other setup files to that directory.