fix: harden symlinked output and project config writes#15730
fix: harden symlinked output and project config writes#15730viyatb-oai wants to merge 4 commits intomainfrom
Conversation
…d-project-config Co-authored-by: Codex <noreply@openai.com>
![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
codex/codex-rs/core/src/config_loader/mod.rs
Lines 782 to 785 in 4ef765a
Reject symlinked .codex directories before loading config
load_project_layers treats .codex as valid when metadata(...).is_dir() is true, but metadata follows symlinks. A symlinked .codex directory still passes and allows loading config.toml from outside the project, bypassing the new leaf-only symlink guard and undermining the hardening intent.
ℹ️ 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".
| match tokio::fs::symlink_metadata(&config_file).await { | ||
| Ok(metadata) => { | ||
| if metadata.file_type().is_symlink() { | ||
| let config_file_display = config_file.as_path().display(); | ||
| return Err(io::Error::new( | ||
| io::ErrorKind::InvalidInput, | ||
| format!("Project config file {config_file_display} must not be a symlink"), | ||
| )); | ||
| } | ||
| } | ||
| Err(err) => { | ||
| if err.kind() == io::ErrorKind::NotFound { | ||
| layers.push(project_layer_entry( | ||
| trust_context, | ||
| &dot_codex_abs, | ||
| &layer_dir, | ||
| TomlValue::Table(toml::map::Map::new()), | ||
| /*config_toml_exists*/ false, | ||
| )); | ||
| continue; | ||
| } | ||
| let config_file_display = config_file.as_path().display(); | ||
| return Err(io::Error::new( | ||
| err.kind(), | ||
| format!("Failed to inspect project config file {config_file_display}: {err}"), | ||
| )); | ||
| } | ||
| } | ||
| match tokio::fs::read_to_string(&config_file).await { |
There was a problem hiding this comment.
The code checks config.toml with symlink_metadata and then reads it in a later async call. This check-then-use gap allows replacing the file with a symlink between operations, so a symlinked config can still be read despite the new rejection logic.
Useful? React with 👍 / 👎.
viyatb-oai
changed the title
1 participant
Summary
--output-last-messagepaths incodex execwithO_NOFOLLOW.codex/config.tomlas a read-only leaf in split filesystem policiesRoot Cause
Bubblewrap protects
.codex/as a read-only directory, but writes to.codex/config.tomlcan still follow a symlink to a writable target outside the protected subtree. Separately,--output-last-messageused plainstd::fs::write, which follows symlinks when callers place the output path in a sandbox-writable location.Validation
just fmtcargo test -p codex-exec last_messagecargo test -p codex-protocolcargo test -p codex-linux-sandbox