never let sandbox write to .codex/ or .codex/.sandbox/#8683
never let sandbox write to .codex/ or .codex/.sandbox/#8683iceweasel-oai merged 1 commit intomainfrom
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 👍 / 👎.
There was a problem hiding this comment.
I think this edge case is probably okay. However, what's the expected behavior if a user runs codex in ~/.codex?
There was a problem hiding this comment.
with this PR, we never make ~/.codex/ or ~/.codex/.sandbox/ a writable root for the sandbox. If the user runs codex in one of those directories, writes in the cwd will fail in the sandbox.
There was a problem hiding this comment.
Let's update docs accordingly, since this differs from behavior on other platforms.
iceweasel-oai
deleted the
dev/iceweasel/no-restricted-sandbox-dir-write
branch
2 participants
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.