[sandbox] Enforce protected workspace metadata paths#19846
Conversation
a99da8c to
5b59ee3
Compare
01771db to
11fa852
Compare
5b8dd2e to
0f697bc
Compare
11fa852 to
ab4b378
Compare
There was a problem hiding this comment.
💡 Codex Review
https://github.com/openai/codex/blob/ab4b378d678768326e770272994017d56c3b7c31/codex-rs/protocol/src/permissions.rs#L726-L728
Include preserved-write semantics in enforcement bridging check
needs_direct_runtime_enforcement decides compatibility by comparing semantic signatures, but preserved-name write denials are enforced in can_write_path_with_cwd and are not represented in the bridged legacy model. This can return false and skip direct enforcement even when .git/.agents/.codex writes should be blocked.
ℹ️ 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".
| fn first_preserved_component(path: &Path) -> Option<(AbsolutePathBuf, &'static str)> { | ||
| let mut candidate = PathBuf::new(); | ||
| for component in path.components() { | ||
| candidate.push(component.as_os_str()); | ||
| if let Some(preserved_name) = preserved_path_name(component.as_os_str()) { | ||
| let absolute = AbsolutePathBuf::from_absolute_path(candidate).ok()?; | ||
| return Some((absolute, preserved_name)); |
There was a problem hiding this comment.
Restrict preserved-name blocking to root-level metadata
can_write_path_with_cwd calls first_preserved_component, which scans all path components and denies writes whenever any component is .git, .agents, or .codex unless there is a deeper explicit rule. This blocks legitimate nested repo writes (e.g. /tmp/repo/.git/...) under otherwise writable roots, so common flows like cloning in /tmp regress.
Useful? React with 👍 / 👎.
8e60a50 to
0b263d0
Compare
0b263d0 to
3ed97f5
Compare
|
Fixed in 3ed97f5. The active contract is now represented on
The stale inline nested repo concern is also covered by the current contract. This only protects the first component under each writable root, so normal nested work below a regular directory is not blocked by this field. Validation passed locally, and the devbox 46 case suite passed with |
viyatb-oai
left a comment
There was a problem hiding this comment.
can you preserve comments from the old code?
Summary
Make FileSystemSandboxPolicy the semantic source of truth for project root metadata protection. Under writable roots,
.git,.codex, and.agentsstay protected unless user policy grants an explicit write rule for that metadata path.Scope
protected_metadata_namestoWritableRoot.FileSystemSandboxPolicy::can_write_path_with_cwdto reject protected metadata writes under writable roots unless explicitly allowed..git,.codex, and.agents.Reviewer Focus
Stack
Validation