Skip to content

Codex live cwd from session rollout: close .hypignore leak for subscription route (LLP 0083) - #266

Merged
philcunliffe merged 2 commits into
masterfrom
fix/issue-257
Jul 7, 2026
Merged

Codex live cwd from session rollout: close .hypignore leak for subscription route (LLP 0083)#266
philcunliffe merged 2 commits into
masterfrom
fix/issue-257

Conversation

@philcunliffe

@philcunliffephilcunliffe commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Problem

ChatGPT-subscription-routed Codex traffic (provider='chatgpt', /backend-api/codex/*) recorded every row with cwd = NULL, so the .hypignore folder policy was a silent no-op for a whole traffic class — enforcement failed open (LLP 0049 R1 leak). The Codex live projector resolved cwd only from the in-flight request (x-codex-turn-metadata header, then body cwd/metadata.cwd/metadata.user_id.cwd) and skipped the policy check when none was found. The API-key route happens to carry cwd in-band, so this enrichment was never built; the subscription protocol has no such field and codex-tui does not send the header. It also diverged from backfill, which reads the rollout and does honor the policy for the same session.

The cwd existed locally the whole time: Codex writes session_meta.cwd into its rollout (<sessionsDir>/…/rollout-<ts>-<session_id>.jsonl, line 1) at session start, for both auth modes.

Fix

Give the Codex live projector the rollout-based cwd fallback (symmetric to the Claude session-context.jsonl sidecar):

  • codex/src/rollout-cwd.js — new createRolloutCwdResolver: finds the rollout whose filename embeds the session id (via the shared sessionIdFromPath), reads the first session_meta line's cwd, caches per session id (incl. misses) so the capture hot path stays bounded (LLP 0049 R6). First-line-only, synchronous — matches the adapter's existing synchronous usage-policy seam.
  • exchange-projector.js — in-band cwd stays the fast path; the rollout is consulted lazily (??) only on a miss and only for real Codex sessions. The one resolved cwd feeds both the .hypignore drop and the stamped row, so live rows now carry the cwd backfill sees (closes the live/backfill inconsistency).
  • index.js — wires the resolver to <codexHome>/sessions.
  • backfill.js — exports sessionIdFromPath for reuse.

Docs

  • LLP 0083 (decision) mints the choice (rollout as the live cwd source over waiting on the client / accepting the leak).
  • LLP 0049 / 0050 carry forward-refs; nothing they decided changes.

Tests

test/plugins/codex-rollout-cwd.test.js: the primary regression asserts a subscription-route exchange with no in-band cwd is now .hypignore-dropped via the rollout cwd and stamps that cwd on the row (both failed on pre-fix code); plus a fast-path test and createRolloutCwdResolver unit tests over a real on-disk rollout.

Checks: npm test (1873 pass / 0 fail / 1 pre-existing skip), npm run typecheck (clean), npm run build:types (clean).

Fixes#257

…iption route (LLP 0083)
ChatGPT-subscription-routed Codex traffic (provider='chatgpt',
/backend-api/codex/*) carries no in-band cwd — codex-tui sends no
x-codex-turn-metadata header and the subscription protocol has no
metadata.cwd — so the live exchange projector recorded cwd = NULL on every
row and `.hypignore` folder policy failed OPEN for the whole traffic class
(LLP 0049 R1 leak). It also diverged from backfill, which reads the rollout
and DOES honor the policy for the same session.
Give the Codex live projector the enrichment it turned out to need: when the
request carries no cwd, resolve it from the session's rollout
(session_meta.cwd, the same source backfill reads), keyed on the session id
the adapter already resolves and matched via the shared sessionIdFromPath
helper. The in-band value stays the fast path (lazy `??`, no scan when
present); the rollout is read first-line-only and cached per session id, so
the capture hot path stays bounded (LLP 0049 R6). The one resolved cwd feeds
both the .hypignore drop and the stamped row, so live rows now carry the cwd
backfill sees.
- codex/src/rollout-cwd.js: new createRolloutCwdResolver (sync, cached).
- codex/src/exchange-projector.js: rollout cwd fallback; resolveRecordedContext
now takes the resolved cwd.
- codex/src/index.js: wire the resolver to <codexHome>/sessions.
- codex/src/backfill.js: export sessionIdFromPath for reuse.
- LLP 0083 (decision) mints the choice; LLP 0049/0050 carry forward-refs.
- test/plugins/codex-rollout-cwd.test.js: regression (fail-open -> dropped)
plus resolver unit tests.
Fixes#257
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

🔍 neutral review (Codex + Claude) — request_changes

Round 1, head 551b8b4. The fix is functionally correct and green: Claude traced all six correctness axes in the code — rollout resolution via the shared sessionIdFromPath, lazy in-band-first fallback (no scan when the request carries cwd), policy/row consistency via a single resolved cwd fed to both the .hypignore drop and resolveRecordedContext, synchronous-seam safety, and fail-open-on-genuinely-absent — and found no correctness defect; the regression test genuinely fails pre-fix / passes post-fix; LLP 0083 + the forward-refs to 0049/0050 are honest.

Codex (independent second family) surfaced two robustness/scale issues in the new rollout-cwd.js resolver that are worth fixing before merge — both matter more than usual because this is a fail-open-prevention fix:

Major 1 — a transient miss is cached as fail-open forever (rollout-cwd.js:46-49)

resolve() caches everyundefined result — including a not-yet-written rollout (a session-start race) or a transient read error (EMFILE/EIO) — permanently for that session id. So a session whose rollout is momentarily unavailable on its first exchange keeps recording cwd = NULL (→ .hypignore fails open) for its whole life, even once the rollout is present and readable on the next exchange. That silently reintroduces the exact race-based fail-open this PR (and sibling #258) set out to close.
Fix: cache a stable not-found (a clean scan that found no matching rollout), but make a transient error / not-yet-present result retryable — e.g. don't cache read/parse errors, or give negative entries a short TTL (the usage-policy resolver already uses a 5s TTL for exactly this reason). Positive results stay cached.

Major 2 — first lookup walks the whole sessions tree, oldest-first (rollout-cwd.js:85-113)

findRolloutFile does a full recursive readdirSync DFS of <sessionsDir> on the first exchange of each session (on the projector path), and the sorted-ascending + unshift traversal explores the oldest date dirs first — so the currently-active session's rollout (newest date dir) is found last, after walking the entire history. Unbounded in history size; worst-order for the common case.
Fix: walk newest-first (most-recent date dir first) so the active session resolves immediately, and/or bound the search (the filename embeds the timestamp — a date-scoped lookup or an index avoids the full-tree walk). Still cache the resolved path.

Neither is a happy-path correctness bug; both harden a policy-enforcement fix so it can't silently regress or scale badly. Verdict: request_changes — neutral is applying these fixes and will re-review the new head (round 2).

Automated neutral review (Codex = independent second family). Findings only — the branch is yours; neutral hasn't modified or merged it.

…okup (PR #266 review round 1)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

🔍 neutral review round 2 — approve

Round-2 verification of the round-1 fixes — head 2bef434, additive on 551b8b4 (no force-push). Both round-1 majors are correctly resolved and introduce no new issues; the fixed resolver + new tests were read directly. CI green, mergeable.

Round-1 findings — resolved

  • Major 1 (a transient miss cached as fail-open forever) ✓ — the cache now stores { cwd, expiresAt }: a resolved cwd is trusted for the session's life (Infinity), a miss only for a 5s TTL (NEGATIVE_CACHE_TTL_MS, mirroring the usage-policy resolver's idiom), so a not-yet-written or momentarily-unreadable rollout (race / EMFILE) is re-resolved on a later exchange instead of being fixed at NULL. Verified: positive→permanent, miss→TTL, expired-miss re-scans; clock injectable. New tests cover missing-then-present and a transient EMFILE retry.
  • Major 2 (whole-tree walk, oldest-first) ✓ — findRolloutFile now sorts entries descending and DFS-visits the newest date dirs first, returning on first match, so the active session (newest dir) resolves after touching only the newest branch; older/dormant sessions still resolve by continuing the walk; still fail-safe on unreadable dirs. readdirSync is injectable, and a test asserts the older branch isn't scanned.

Also fixed an inline import('node:fs').Dirent type (repo's no-inline-import-types rule) via RolloutDirent / RolloutCwdResolverOptions interfaces; the doc comments and @ref LLP 0049#R6 / LLP 0083 were kept honest.

(Round 2 was a focused verification of the bounded fix against the actual code — the round-1 Codex + Claude dual pass raised these findings; this round confirms they're resolved with no regressions. CI's test flaked once on the pre-existing, unrelated util-fs-atomic mtime test and passed on re-run.)

Verdict: approve. Round-1 findings resolved, no new findings. Review rounds complete (N=2). Held for a human to merge — merging closes #257 (Fixes #257). neutral does not merge.

Automated neutral review.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subscription-routed Codex records cwd=NULL: .hypignore fails open for a whole traffic class (live projector should enrich from rollouts)

1 participant

@philcunliffe