Repository navigation
Enforce worker write roots and verify policy on reopen - #2122
Conversation
🦋 Changeset detectedLatest commit: cf917c9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
538845d to
c7b58bc
Compare
1a5401d to
a1e206e
Compare
c7b58bc to
992ad95
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (9)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The durable reopen policy check introduces a compatibility regression for implicit remote environments; see the inline finding. Keep the strict-policy checks while preserving non-strict relocation. Hosted checks remain partly pending. Static review only; no local code or tests were run.
a1e206e to
b78ae6d
Compare
992ad95 to
a7fea1b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b78ae6d4d6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The executor-layer changes are clean and the earlier remote-relocation blocker is fixed. The readiness race reported in #2122 (comment) is a false positive: the pinned Core process event log snapshots retained events and subscribes live under the publication mutex, so an already-exited probe is replayed (https://github.com/openai/codex/blob/7c2ce90716335c889a5076ded9a630459f9c9899/codex-rs/exec-server/src/process.rs#L145). This stacked head still inherits #2121 (comment); carry that fix through before merging. No additional executor finding.
Static review only; no candidate code or tests executed. Hosted CI: pending: test (22), test (22); no failed checks observed.
a7fea1b to
1a3eb41
Compare
b78ae6d to
6ea6424
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ea6424f62
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed the helper-path, implicit remote-reopen and receipt-validation changes; the upstream late-event issue is fixed. No actionable finding. The network-only checkpoint report overlooks the separate TypeScript network_policy_digest check before native reconnect. The environment-MCP report assumes a sandboxed launch that the pinned implementation does not perform: the guide explicitly treats this as a separately trusted application process, independent of model-visible executor tools. The empty-receipt report does not establish a naturally produced failing case beyond tampering with trusted rollout metadata.
Static review only; no candidate code or tests executed. Hosted CI: no pending checks observed; no failed checks observed.
1a3eb41 to
90ec6f7
Compare
6ea6424 to
28ec6fd
Compare
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved the exact current increment after coordinator and independent static reviews. The local storage guard now resolves sessions/date/archive aliases and checks the selected rollout path on resume and the live writer path before replacing grants. Path overlap and hard-link checks include compressed alternatives/plain materialization while preserving disjoint relocations. Validation precedes writer admission and committing new grants. Reviewed regressions and hosted artifact selection; no actionable findings.
Hosted CI on these new commits remains pending. No candidate code or local tests executed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7659110be5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The shared physical write-root and storage protections are sound: filesystem identities cover alternate paths, checklist aliases and hard links are checked, and private runtime symlinks fail closed. Disjoint storage and write grants remain supported.
Coordinator and independent static reviews of actual old/new file contents found no actionable issue. Hosted CI on this new head remains pending; prior-head results do not qualify it. No candidate code or local tests executed.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the refreshed head: 30 of 31 actual changed paths match upstream base. The remaining alpha-release workflow change uses established Linux/Windows runner groups for the canonical repository and preserves macOS/fork runners. Previously reviewed worker policy changes are intact. No new actionable findings after coordinator and independent static review. Hosted CI for this new head remains in progress; no candidate code or local tests executed.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed cf917c9. The Linux long-path helper scan safely falls back to component-wise directory opens, rejects descendant symlinks, preserves distinct mount views, and keeps the directory descriptor alive through entry inspection. The patch and manifest propagation are consistent with the previously reviewed stack. No new actionable findings. Hosted CI remains in progress; this was static review only, with no candidate code or local tests executed.
Requirements.
REQ732covers executor configuration and startup,REQ733covers owned cancellation,REQ734covers execution evidence, andREQ736covers isolation.Review order. #2120 (controls) → #2121 (receipts) → #2122 (executor) → #2123 (recovery) → #2124 (worker example) → #2125 (docs) → #2137 (automatic compaction during cancellation).
Summary
A coding worker needs to run commands under a bounded write policy and know when its processes have stopped. This layer adds local execution on supported Linux and macOS hosts. The first commit enforces the policy in Core. The second exposes it through the SDK and tests the public interface.
Closing a session prevents a delayed executor result from sending an ordinary HTTP or WebSocket follow-up to the model. New sessions can still issue requests.
The native build applies the Core patches in the order listed in
packages/agents/native/patches/series.json. The Linux shutdown regression exercises the resulting branch that joins all managed local commands.On macOS, shutdown waits for the starting wrapper to report its kernel process identity or for its control channel to finish. It can then resume a paused wrapper. Success requires both the drain acknowledgment and the wrapper's exit within the shutdown deadline.
A session saved before its first model request can reopen without a sampling receipt. After sampling, an explicit network policy or strict write grant needs recorded evidence. The SDK checks the owner of surviving modern receipt fields before allowing a legacy reopen. Older implicit remote sessions can reopen when every retained Core context matches the original default read-only or single-workspace policy. If all modern receipt fields were erased, a strict session that recorded the same Core context as an older implicit session can be indistinguishable from it. The caller can reopen that history only under a compatible implicit policy.
A writable root can expose private session history in either direction. It can contain a relocated sessions directory, or it can sit inside one and receive the next rollout. On Linux and macOS, Core compares directory identities and their ancestors so alternate spellings reach the same check. Disjoint storage relocations remain supported. On open, resume, and policy replacement, the SDK also checks the selected rollout file and the files directly inside its private checklist directory. The SDK refuses symbolic links inside its temporary runtime home.
The stacked package workflow now covers native code and patches, SDK runtime code, native tests and the package workflows themselves. The release note describes local write-root changes on both supported platforms and the opening-only grant on remote macOS.
Test plan
In an unprivileged Linux guest, the installed JavaScript package passed four SQLite checks covering direct and relocated database aliases at open and during write-root replacement. A separate installed Linux test passed with a disjoint workspace and durable reopen. On macOS, the installed package rejected the existing helper alias and still ran Code Mode from a disjoint workspace. macOS installed helper test
Five shared control-channel tests passed after the queued-identity regression failed on the previous source. They exercised the socket protocol with a simulated failed signal on Linux.
Focused SDK checks cover session close, delayed callbacks, model-step limits and successor turns. The package workflow has a separate installed-consumer check. The wrapper passed service-delegation, terminal, filesystem and network tests on macOS 15 and 26. macOS 15 run · macOS 26 run · Installed JS package run
On the old package a sandbox command changed a real rollout through a writable subdirectory with an alternate spelling. The corrected Linux package passed 13 selected cases on case-sensitive ext4 and three on case-insensitive ext4, including rejecting that subdirectory on open and policy replacement. The case-sensitive results combine 12 original passes with a rerun after fixing one test assertion about a permitted session open. The full executor-receipt suite passed all 19 cases with the corrected legacy fixture and a previously built package.
Issue number
The owning requirement IDs appear above.