Repository navigation
fix(codex): refuse known storage relocation across native config layers - #311
Conversation
Extend the pre-effect storage boundary to the native Codex 0.162.1 layers that can select SQLite storage: system config and requirements files (including the legacy Unix managed_config.toml), macOS managed-profile values, and the project layers of the native process directory. Dry-run and queue pin the native process to the inspected home, refuse while the home's app-server control socket exists (its startup storage cannot be observed), and inspect the saved thread directory where the consuming writer starts. Wake inspects the thread directory before starting its app-server. Package defaults and enterprise cloud bundles remain uninspected and documented as limits; no relocated-storage support.
Run the queue storage preflight before a wake reserves its per-target budget, so storage refusals never consume a reservation. Refuse unreadable or malformed saved-thread metadata as consumer_directory_unverifiable instead of skipping the consumer check. Isolate Codex dispatch, wake, home and storage fixtures from machine-wide and ancestor configuration through a shared helper, and align the PWD expectation with the platform branch. Correct the documentation: dry-run starts no process, native TUIs auto-start a shared daemon by default, the control-socket refusal is a compatibility change, and the reviewed APIs do not attest daemon startup storage; list the uninspected layers.
Per the owner's compatibility policy, remove the refusal based on the presence of an app-server daemon control socket: native TUIs start such a daemon by default, and `codex queue` already selects a connectable daemon or an embedded server. session-peer neither probes the socket nor claims storage authority. Known relocation in system, managed, project, user and environment layers still refuses before any effect, and wake still runs these checks before reserving its budget. Document that the reviewed initialize, config/read and queue APIs offer no stable startup SQLite attestation, that dry-run and send checks do not remove the time gap, and that translated native homes do not support storage verification.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ 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. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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: bc3655882f
ℹ️ 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".
|
@codex review Please review the latest HEAD 7f109fb, including the fixes for all four prior findings and the subsequent cross-platform fixture corrections. Existing findings have implementation replies and are marked resolved; this is a request for a fresh review, not a claim of approval. Current-head CI: 28 checks successful. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f109fb367
ℹ️ 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".
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
|
Current-head CI attempt1 (run38690059740, head f812a0f) hit a native process crash: optional Relay Ubuntu/Python3.13 job116273649374 exited139 during test_lost_pair_commit_response_reconciles_without_reusing_secret, without a Python traceback. Three Relay matrix cells were fail-fast cancelled. Previous head was green and local combined tests pass, but neither proves this crash unrelated. Python/dependency versions matched the preceding green run; runner image differed, which is an observation, not a cause. Preserving the first failure and requesting one bounded same-SHA rerun of failed jobs. A pass means non-reproduction only, not root-cause resolution; repeated139 requires diagnostic stack collection and keeps the release gate blocked. No test/guard/dependency bypass or runtime workaround applied. |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Bounded same-SHA CI rerun completed successfully: run38690059740 attempt2, head f812a0f, all four Relay matrix cells pass and the aggregate release gate passes; latest checks28/28 success. Attempt1 exit139 remains an unexplained native-process crash and was not fixed by a source change; passing retry is non-reproduction only. No additional reruns or dependency/test/guard relaxation were used. Current-head Security Review found no security issues; Code Review is still running. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f812a0f18f
ℹ️ 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".
|
@codex review |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cca38080d5
ℹ️ 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".
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Sequential merge stage: updated integration head f01f23e includes current main3392c561 (#305 already merged). Feature diff patch-id is unchanged from reviewed1272b80 (stable patch-id0a85507013e8ed7ef086166e3afdfb2311167f0d). Current f01f23e CI28 successful; Code Review completed without new findings; Security Review still running. Waiting before normal protected-main squash merge; no bypass or native acceptance/issue completion claim. |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Summary
Latest head:
1272b80ca2c8df096ec9e890a10e55d60cee5f1d.Refs #268 — remains open for authoritative effective-storage and native platform acceptance. No runtime version bump or publication.
Bounded storage hardening with the owner's compatibility policy 3: daemon socket presence/connectivity does not block submission and session-peer adds no probe, forced embedded mode, strict flag or storage-authority JSON. Existing override refusal, stable-writer checks, inactive-home opt-in and unknown/no-resend boundaries remain.
Current owner-approved metadata-only discovery policy
The owner selected option1: standalone list must not execute Codex or query native versions. At ba884e6/1272b80, macOS public collection and direct discovery conservatively inspect ordinary plus forced preferences before SQLite access, even with --codex-bin. The documented tradeoff is conservative refusal of values a newer0.162.1 loader may ignore; precise version-aware listing is deferred, not implemented implicitly.
Errors remain failures rather than empty-success or alternate-DB fallback. Existing partial healthy home/agent rows and JSON/exit shapes remain intact. The diagnostic distinguishes a preference sqlite_home override from an unclassifiable known configuration layer (not necessarily CF), preserves the original reason and gives safe operator guidance without values, auto-deletion or DB switching. Internal send dry-run reuses its selected policy without another version query; public collection does not accept a private override.
Fifteen synthetic listing tests cover legacy stale DB refusal, intentional newer-version refusal, malformed HOME/CF configuration, explicit --codex-bin/no native calls, zero SQL before refusal, JSON/text error, partial results and internal policy reuse. Coordinator exact combined1.2 rehearsal5e536883567d9453b619acd18394a3533e055b3f:1256 tests pass/19skips; reproducible wheel/sdist builds, distribution41 pass/3PowerShellskips and isolated artifact installs pass. No new native/CF/MDM/model acceptance.
Independent static review: Claude Code2.1.300 requested/actual claude-opus-5-5 and isolated Codex CLI0.162.1 default (actual model unexposed), parallel stdin-only, no tools/network/tests/writes. Frozen ba884e6 input ba9aeecbd5b3fd1790b477ae52a8fb431c06bedb1ee8809f4963f145b5097b38 and exact1272b80 follow-up input4ea98ed1e54f012226bd167b1d0864480be01c7f93d7b8f2a11d2e4162cd39ea: all four processes exit0 with complete valid results. No confirmed mandatory defect. Coordinator checked conditional facts against all actual discover callers and reason-producing helpers: the managed reason is raised only by the preference classifier, with root key sqlite_home; generic file/parser failures retain the non-attributed diagnostic. Exact1272b80 GitHub CI28/28, Code Review and Security Review are complete with no new findings; all threads resolved. The six acceptance issues remain open and owner approval plus final-main/release-asset gates remain separate.
GitHub review corrections
.gitmarker, including non-directory metadata-only markers; a directory requiresHEAD. Without a marker, inspect cwd only. Do not open FIFO/device markers. Symlink or unreadable markers remain conservative refusals. Known customproject_root_markerssettings retain a documented ancestor superset. The earlier candidate's<cwd>/config.tomlassumption came from a stale loader comment; the pinned project-layer construction loads.codex/config.toml, not that extra cwd file.SHGetKnownFolderPath(FOLDERID_ProgramData)rather thanProgramDataenvironment values. Bound UTF-16 reads and free the allocation. Match the native fixed fallback on lookup failure; invalid/unbounded successful results refuse instead of silently checking another directory. Preserve native UTF-16 path representation.CI corrections
/projectvalues on Windows; keep malformed real metadata refusal..2sexecution/.5scleanup interval; model.3sstartup. Production clocks/budgets are unchanged. This does not establish the historical macOS failure's cause.Type
Validation
-X dev -W error::ResourceWarning: integratedf1b3ad6full suite 1154 OK, 19 skipped. The final narrow drive-bound validation/doc recommendation followed this full run; its focused Python 3.9 platform/handoff suite 52 OK, 1 skipped and documentation 61 PASS were rerun on the final tree. Exact-head distributed CI remains required.Independent CLI review
Claude Code 2.1.300, requested/response-confirmed
claude-opus-5-5; tools disabled, safe non-interactive mode, no session persistence. Codex CLI 0.162.1, default model with user configuration ignored, read-only sandbox and ephemeral execution; actual model name not exposed in captured response metadata. Separate parallel processes read identical immutable stdin, without sharing the other reviewer's conclusions. Reviewers ran no tests or repository/operational writes.Initial exact snapshot
7074e3efaab703e632be32620af0dcf7346d9c1e, input SHA256e44698340c4ce3be442c409dbfc88114f84f3c601d2e1d4ee2f0330ee4a93d39: addressed the metadata-only non-directory marker case and provided complete pinned project-layer construction evidence. Also strengthened native-directory anomaly handling and FFI ownership fixtures.Follow-up exact snapshot
f1b3ad6a692fcacfe2f170b5518e4952b98935d2, input SHA2565542e035812230e034d56a5b0ee07d2b6cee3084a6407315ad558c7a1b249a09: both reviewers reported no confirmed new mandatory defect. Claude's conditional marker-evidence question was checked by the coordinator against the complete pinned discover_project_root and project-root fallback: headless.gitdirectories are skipped, non-directory entries are recognized. The coordinator then addressed the shared optional drive-unbound Windows-path case and polished four-language wording, with focused tests; these final small feedback-driven edits were not externally rerun. All four CLI processes completed exit 0 with valid final responses and immutable input hashes. Static review is not native/platform acceptance.Remaining boundaries
092d3acd6bec3e3a14bdc7e7a2810ab628ab759d.Earlier wake-specific review and validation
At f812a0f, Codex187 tests (4 skips), core565 (3 skips), documentation61 and Python3.9 focused71 (1 skip) pass. Synthetic tests distinguish ordinary/forced CF reads, unsupported versions, active writer, custom markers, refusal before reservation and child effects, and preservation of a known queued result after a later wake refusal. No live CF/MDM/native test or effective-storage attestation is claimed.
Independent static review input SHA25687f1f2cb4e218a29d1e1cb0426e5099f992e1caca0d13aa75be0338fd3dc79db, exact f812a0f: parallel Claude Code2.1.300 requested/actual claude-opus-5-5 and isolated Codex CLI0.162.1 default (actual model not exposed), both exit0 with complete final responses and no mandatory defect found. No reviewer tools/network/tests/writes. Coordinator checked the conditional exception-inheritance question: CcPeerError inherits Exception, not ValueError. Optional synthetic test-strengthening suggestions are not live acceptance. Latest GitHub CI/code/security review remains pending until its exact-head results complete.
Earlier follow-up: ordinary-version policy and physical cwd
Code-review threads4279676944/4279676951 are addressed by09efcc2 and fixture/documentation follow-up cca3808. Ordinary macOS send/dry-run uses a single bounded version query (3s query +2s cleanup, existing owned-process output caps). Only the exact reviewed0.162.1 chooses forced-only preferences; old/unreviewed/unavailable/malformed evidence conservatively includes ordinary values, not blanket version rejection. Cleanup uncertainty refuses before queueing; interruption propagates after the cleanup attempt. Handoff reuses the existing query and deadline, preserving policy until submission. No new non-macOS query/cache/wake support is added.
Owned execution plans use physical cwd consistently for project scans, child cwd/PWD and wake resume; consumers additionally inspect distinct lexical spellings with the same marker boundary. Alias configurations can be conservatively refused, and
wake.cwdnow uses the physical spelling. Missing-directory/translated-native-home limits and daemon compatibility remain explicit; no storage attestation or TOCTOU elimination.Independent reviewers read identical frozen09efcc2 input SHA2566ac56afc35a6ee9366e46fd152f17a65479086a10b307feef7f56dbb306ba2fb. Claude Code2.1.300 requested/actual Opus5.5 and isolated Codex CLI0.162.1 default (model not exposed) both exited0 with complete final responses and no mandatory product defect. Feedback-driven follow-up cca3808 improves fixture isolation/non-vacuous assertions and four-language wording; the coordinator proved executable AST unchanged from09efcc2, excluding comments/docstrings. Reviewers ran no tools/tests/network or writes.
Validation: Codex206/core567/docs61 and Python3.9focused90 passed at09efcc2. Follow-up focused126/Python3.9focused42/docs61/generator/diff pass. A local combined rehearsal at e3dbd13 passed1240 tests/19skips plus reproducible builds and isolated installs; stderr contained a separately tracked asyncio shutdown diagnostic, so this is not a warning-free claim. A subsequent Relay-only diagnostic passed236 without that warning. Final combined revalidation after the test/docs follow-up is ongoing; exact-current-head GitHub CI and Code/Security Reviews are required.
Relay CI exit139 and the separate local shutdown diagnostics are tracked in #316, cause/relationship unresolved. The diagnostic-only faulthandler addition is not a fix. No suppressed test, relaxed gate/dependency, public main merge/tag/release or deployment.