From cb9a48e2f0f46177c1f944f4dad937817b485b13 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 27 Sep 2026 23:56:55 +0900 Subject: [PATCH 1/6] docs(devlog): plan release-train-4 picker CA recovery --- .../picker-ca/000_plan.md | 54 +++++++++++++++++++ .../picker-ca/010_ca_publication.md | 29 ++++++++++ .../picker-ca/020_desktop_continuity.md | 28 ++++++++++ .../picker-ca/030_codex_drift_heal.md | 29 ++++++++++ .../picker-ca/040_integration.md | 21 ++++++++ 5 files changed, 161 insertions(+) create mode 100644 devlog/_plan/260927_release_train_4/picker-ca/000_plan.md create mode 100644 devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md create mode 100644 devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md create mode 100644 devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md create mode 100644 devlog/_plan/260927_release_train_4/picker-ca/040_integration.md diff --git a/devlog/_plan/260927_release_train_4/picker-ca/000_plan.md b/devlog/_plan/260927_release_train_4/picker-ca/000_plan.md new file mode 100644 index 00000000000..538ce679b01 --- /dev/null +++ b/devlog/_plan/260927_release_train_4/picker-ca/000_plan.md @@ -0,0 +1,54 @@ +# Release train 4: picker CA continuity and Codex drift heal + +An applied Claude Desktop picker profile can retain an egress URL after CA rotation is refused. This unit keeps that URL serving blind CONNECT while the old public certificate awaits verified removal, then allows the existing picker enable flow to retry. It also narrows the Codex drift heal to the injected config surface so a stopped auto-refresh generation cannot finish a stale full catalog sync. The source and acceptance record is this unit; the external 2026-09-27 audit is a lead, not an authority. + +## Loop contract + +| Field | Decision | +| --- | --- | +| Archetype / trigger | Satisfy-spec repair for the release-train-4 R1–R3 blocker and B10 verification request. | +| Goal | Applied Desktop egress stays reachable without picker TLS arming until predecessor trust is gone; CA publication is serialized; pending removal survives process replacement; B10 drift heal respects generation and bounded write-lock waiting. | +| Non-goals | No `main`/`preview`, release, version, user keychain, real Desktop library, other lane checkout, provider, GUI, or unrelated client change. | +| Verifier | Isolated focused Bun tests exercise failed untrust, actual CONNECT through the applied URL, lock contention, distinct processes, journal retry, stopped generation, and drift healing; typecheck, `test:changed`, privacy, structure, docs build, exact-head PR CI, then dev CI. | +| Stop | All criteria have fresh evidence, required PR-head checks succeed, the authorized PR is merged into latest `dev`, and dev CI is checked. | +| Memory artifact | This numbered unit, PR Verification, and goalplan/receipt records. Detailed pre-fix security working notes remain in ignored `.tmp/`. | +| Terminal outcomes | DONE only after the stop condition; unresolved native/CI evidence is reported as a limitation, never inferred from Linux tests. | +| Escalation | A needed write outside this lane, actual keychain mutation, blocked security review, or an unmergeable required check requires coordinator direction. | +| Resource scope | Only this dedicated worktree, `gpt-6-sol` read-only reviewers, existing repository tools and GitHub PR/CI; no user-set token or time cap. | + +## Baseline and disposition + +Base is `origin/dev` `24b2f39b77` (fetched 2026-09-27); branch `codex/t4-picker-ca-release-blocker`. The original #6072 picker CA PR and #6074 drift-heal PR are closed and their carried implementations are already on `dev` through #6073/#6075. There is no open picker-CA PR or issue from the current targeted GitHub search. This unit **reimplements the defective edges** of those landed batches on current `dev`; it does not merge an open PR as-is, cherry-pick, or squash a foreign branch. Existing credit in the earlier carry stays in history; this corrective diff does not carry another author's unmerged work. + +Baseline in an isolated `HOME`, `OPENCODEX_HOME`, and `TMPDIR`: the three existing focused files passed 40/40, `bun run typecheck` passed, `bun run privacy:scan` passed, and `bun run structure:check` passed after `bun install --frozen-lockfile`. The focused tests do not assert the applied egress URL after failed untrust; `tests/claude-integration/claude-picker-runtime.test.ts:500` tests only the main intercept port. `src/claude/intercept/picker-ca.ts:89` conflates lock outcome with a void callback. The B10 tick calls full sync at `src/codex/catalog-auto-refresh.ts:86` and checks its generation only after that await at `:143`. + +## Design decisions + +| ID | Decision | Reason and rejected alternative | +| --- | --- | --- | +| T4-PICKER-CA-R1 | Bind a blind-only CONNECT relay on the applied profile's **actual** `egressProxyUrl` port when predecessor cleanup cannot complete; keep the row and previous selection. | A profile pivot requires ownership-sensitive Desktop writes, can discard retry intent, and may not repair the currently running app's pinned URL. Blind relay preserves network access without picker TLS termination. A foreign port holder cannot be replaced safely; report bind failure and preserve the row. | +| T4-PICKER-CA-R2 | Serialize read/decide/journal/publish under the existing picker CA SQLite lock; return a tagged lock result and never publish on lock failure. | A void callback returns `undefined` on success today. Publication outside the lock permits competing writers. | +| T4-PICKER-CA-R3 | Journal one outgoing **public** PEM plus SHA-1/SHA-256 before changing `ca.pem`; clear a prior pending item before a fresh rotation and acknowledge only after verified untrust. | The previous public certificate is otherwise lost across process replacement. Refusing a second rotation while one item is pending keeps a single record sufficient. No signing key is serialized. | +| T4-B10 | Use the existing config injector directly for drifted root keys, with its synchronous commit guard and 1-second write-lock wait. | Full `syncModelsToCodex` gathers/commits a catalog from a tick snapshot and exposes no generation guard or deadline option. The 1-second contract covers lock waiting, not the entire provider fetch; a direct injector avoids that fetch for a config-only heal. | + +## Architect consultation + +Read-only architect handle `01a0e33c-036c-7dd2-a83f-dfcce911055c` proposed R1 blind relay, R2 tagged lock/no unlocked write, and R3 public pending journal. Main accepted each decision, chose to keep `ensurePickerCa`'s existing return type and throw on unsafe/deferred publication, and added the separate B10 direct-injection decision from the bounded B10 source audit. The same architect reflected on this executable five-document roadmap: its first response was **MISALIGNED** on cached foreign-owner refusal and non-default catalog path preservation. Main amended [010](010_ca_publication.md), [020](020_desktop_continuity.md), and [030](030_codex_drift_heal.md); the second reflection was **ALIGNED**, with both pending drains and acknowledgement required before picker construction. Independent plan audit follows this consultation. + +Independent A audit first found the new documents invisible to its staged-diff reader; the five files were staged in the dedicated worktree. Its substantive round then found that a later controller enable could bypass startup cleanup and that the existing journal path getter mutates an invalid journal. The phase docs now close the former through default `ensurePickerCa` refusal while pending, and the latter through a bounded read-only journal lookup plus a regular/parsed catalog check. These are plan amendments pending the same architect's reflection and the reviewer's next audit. + +Architect reflection on those amendments found a further crash boundary: a pending record may have been written while its PEM remains published and the owner is still live if the subsequent CA replacement fails. Main amended [010](010_ca_publication.md) and [020](020_desktop_continuity.md) so cleanup defers rather than untrusting that incumbent. The independent audit must recheck this reachable fault path before code begins. + +The next reviewer pass found the planned multiple-pending test unreachable because publication refuses while an item is pending. Main simplified the data structure to one pending item and replaced that row with a sequential retry test; architect reflection and final audit follow. + +The same architect confirmed **ALIGNED** for the single-record change. The independent reviewer checked the staged five-document roadmap again, found no remaining blockers, and ended with **VERDICT: PASS**; `git diff --cached --check` also passed. This closes the docs-first design decision. The next cycle begins with serialized CA publication and its process tests in [010](010_ca_publication.md); source files remain unedited at this checkpoint. + +## Dependency order and review + +The first PABCD work phase records this roadmap only. [010](010_ca_publication.md) establishes publication and journal contracts; [020](020_desktop_continuity.md) consumes them for Desktop continuity; [030](030_codex_drift_heal.md) repairs the independent B10 path; [040](040_integration.md) synchronizes docs/invariants, runs gates, and delivers one ordinary PR to `dev`. Each phase rechecks its prewritten diff plan against the current tree before editing. All implementation remains one release-blocker PR because the final acceptance depends on the combined picker recovery and B10 check; no native GitHub stack is requested. + +## Verification and enforcement limits + +The focused commands name their target test files directly. `bun run typecheck` reads `src/**/*.ts` and `tests/**/*.ts` via `tsconfig.json`. `bun run structure:check` reads `structure/`; `bun run privacy:scan` scans the tracked tree. `bun run test:changed` uses the `dev` merge base's parsed import graph and cannot discover source-as-data, subprocess, or journal artifacts, so the focused files remain explicit. Baseline output is summarized above; `test:changed`, docs build, skill-surface check, and CI have not yet run on the repair. The seven concurrent release lanes make a full local suite disproportionately costly; the PR will list exact focused commands/results and leave full coverage to CI. + +The CA and generation guards are code-level controls, checked by isolated regression tests and required hosted CI (E3/E4). They do not prevent an arbitrary same-user process from altering its own files or guarantee an occupied egress port; native keychain denial remains simulated locally. There is no claimed unbypassable layer. A Mac workflow on an ordinary source-only PR is gated by `.github/workflows/ci.yml:680-687`; if skipped, an exact-head `macos-control` dispatch is the available explicit coverage path, and its actual job conclusion must be checked separately. diff --git a/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md new file mode 100644 index 00000000000..b26ee2f2057 --- /dev/null +++ b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md @@ -0,0 +1,29 @@ +# 010 — Serialized CA publication and public recovery record + +Depends on: `000_plan.md`. Work phase `wp1`, class C4. Source owner: `src/claude/intercept/picker-ca.ts`; consumer: `src/claude/intercept/runtime.ts` in phase 020. No production keychain, service, or home operation. + +## File changes (diff-level) + +| Path | Change | Before → after | +| --- | --- | --- | +| `src/claude/intercept/picker-ca.ts` | MODIFY | `underPickerCaLock` returns `T | undefined`, and void callbacks are repeated outside the lock → tagged `{ ok: true, value: T } | { ok: false, error }`, with a caller-visible failure and no unlocked publication. The cached and fresh paths both decide and publish only under the lock. A live foreign owner blocks a fresh publication **and makes a cached mismatched authority throw**; returning the cached CA after merely deferring publication would let callers arm it against a different on-disk owner. `processAuthorities` is set only after a successful locked path. | +| `src/claude/intercept/picker-ca.ts` | MODIFY | A replacement overwrites the last public PEM with no durable predecessor → under the same lock, parse the current public PEM, write one `{ sha1, sha256, certPem }` record to `pending-untrust.json` by private temporary file/rename **before** `ca.pem` changes, then publish `ca.pem` and `ca-owner.json`. Refuse another replacement while that record exists. Reject malformed or oversized pending data rather than dropping it. Expose a bounded read, a check of whether the pending PEM is still the published certificate with a matching **live** owner, and exact-entry acknowledgement under the lock. A missing record means no pending item. No `keyPem`, signing key, private path, or request data enters the record. | +| `src/claude/intercept/picker-ca.ts` | MODIFY | `ensurePickerCa(configDir)` returns a CA to controller/CLI/runtime even when predecessor cleanup is unresolved → keep its return type, but make its default call refuse any pending record or replacement requiring untrust. Add a narrow startup-only option that permits a fresh replacement **only when no prior pending record exists** and returns the CA with its newly queued predecessor for immediate cleanup in phase 020. The ordinary controller `enable` and picker-runtime material paths use the default and cannot trust or rearm a new CA before cleanup. | +| `tests/claude-integration/claude-picker-ca.test.ts` | MODIFY | Existing process-restart and foreign-owner tests check only the final certificate → test a successful void callback executes exactly once, an actual second process holding the SQLite lock causes zero publication outside it, and competing processes leave `ca.pem` and `ca-owner.json` with matching SHA-256 after serialization. Check the pending record contains only public PEM/fingerprints and is written before replacement; malformed record blocks publication; cached republish obeys the same lock. | + +## Conditional activation and field chain + +- Normal void callback: call the tagged lock helper with a side-effect counter, assert one increment and `ok: true` even though `value` is `undefined`. +- Busy lock: hold the actual picker SQLite lock in a child process, call `ensurePickerCa` in another process, assert no publication and an explicit deferred outcome; release then retry. No in-process stub substitutes for this contention check. +- Live foreign owner: publish from a live second process, call the cached `ensurePickerCa` in the first, and assert it refuses instead of returning an authority whose public PEM is no longer published. +- Prior certificate: start with a valid published public PEM, rotate, assert the record exists with its SHA-1/SHA-256 before the new `ca.pem` is visible. If the write fails, the old PEM remains published. +- Interrupted publication: fault-inject failure after recording pending but before replacing `ca.pem`; while the matching owner process is alive, a second process sees the pending PEM still actively published and **does not** untrust it. It defers picker construction/cleanup; after the owner exits, a later process retries removal. +- Bad record: stage malformed JSON and attempt rotation, assert no replacement and no private material. No parsing fallback silently erases recovery data. +- Sequential predecessors: an unresolved A blocks another publication; after exact A acknowledgement, a later B-to-C rotation may create B as the next single pending item. A mismatched acknowledgement leaves the original record intact. +- Later controller enable: leave a pending entry after a failed removal, invoke the real controller enable path, and assert no trust add or picker rearm. The default `ensurePickerCa` refusal is the gate, not a changed controller implementation. + +New field chain: the startup-only `ensurePickerCa` path creates the single pending entry; the JSON writer serializes it; the bounded reader validates its fingerprints against the PEM; phase 020 consumes it for `untrustPickerCa`, then the locked acknowledgement removes it. Ordinary `ensurePickerCa` callers consume only the no-pending state. The option is created only by `startClaudeIntercept`, is not serialized, and has no other consumer. A test-visible lock helper, if needed to observe the void callback, is a narrow code contract and not user configuration. + +## Proof before closing this phase + +Run isolated `bun test tests/claude-integration/claude-picker-ca.test.ts`, `bun run typecheck`, and the relevant source-as-data/process tests explicitly. Inspect the final CA/owner pair and journal bytes, and verify no `ca.key` or signing key appears anywhere under the fixture state root. Record exact commands and outcomes in the phase D note. diff --git a/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md b/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md new file mode 100644 index 00000000000..ca24537324c --- /dev/null +++ b/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md @@ -0,0 +1,28 @@ +# 020 — Applied Desktop egress continuity + +Depends on: `010_ca_publication.md`. Work phase `wp2`, class C4. Only the picker lifecycle and its tests change; the existing profile writer remains the row owner. + +## File changes (diff-level) + +| Path | Change | Before → after | +| --- | --- | --- | +| `src/claude/intercept/runtime.ts` | MODIFY | Read only a boolean `pickerProfileApplied`, rotate, and skip the entire picker proxy bind on failed untrust → retain the inspected applied profile's validated URL/port, clear any prior pending item **before** a fresh rotation, call phase 010's startup-only `ensurePickerCa` option, then clear the newly queued outgoing item. Before either removal, ask phase 010's locked owner check whether this pending PEM is still published by a live process; if so, defer without invoking `security` or acknowledging. A failed/deferred untrust, corrupt journal, or deferred publication blocks picker creation. Each safe removal uses a private temporary **public** PEM copy and calls `untrustPickerCa`; only a confirmed result permits exact-entry acknowledgement. Both cleanup passes and acknowledgements must finish before picker construction. | +| `src/claude/intercept/runtime.ts` | MODIFY | `pickerBlocked` leaves the applied profile's egress URL without a listener → bind `startConnectProxy` at the URL's actual loopback port with `interceptHosts: []` and a per-CONNECT decision that is always `{ kind: "blind" }`. This branch creates no picker runtime/controller/TLS leaf, performs no trust add, does not rewrite or remove the profile, and closes the relay in `stop`/error teardown. The live state reports the bound proxy port while the picker runtime remains null. Existing `offlinePickerStatus` may still say `proxy_unavailable`: that reason describes the picker feature/controller, not egress liveness; the runtime state and actual CONNECT prove degraded relay availability. If the port belongs to another process, do not take it over; preserve the row and report a degraded bind failure. | +| `tests/claude-integration/claude-picker-runtime.test.ts` | MODIFY | The failed-untrust test checks only the main intercept port → apply an owned Desktop profile first, read its `egressProxyUrl`, inject failed removal, CONNECT through that exact URL to a fake upstream, assert a 200 blind tunnel and no picker creation/trust addition, and confirm row ID plus previous selection and retry intent stay intact. Check normal startup still arms only after cleanup. | +| `tests/claude-integration/claude-picker-recovery.test.ts` | NEW | Start separate Bun processes with one isolated fixture home, fake `SecurityRunner`, and no actual `/usr/bin/security`. First startup fails removal; a new process retries the recorded old PEM before rotating again; success clears the record and permits picker creation. Also test controller enable with pending cleanup refuses a trust add. Register this `.test.ts` in both `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json`. | + +## Conditional activation + +- Failed old-CA removal with applied profile: inject a listed old fingerprint and removal failure; prove the profile's own URL answers CONNECT and that `claude.ai` is blind, not terminated. +- Same failure without applied profile: no extra open relay is needed; picker remains disarmed and the default intercept pair remains live. +- Process replacement: use a new OS process, not an in-process stop/start that reuses `processAuthorities`; show the old fingerprint is retried from the public journal. +- Partly published rotation: pending PEM still equals `ca.pem` and its matching owner PID is live; a second process must not remove trust for that incumbent or arm its own picker. When that owner exits, retry proceeds. +- Ack lock contention: simulated failed acknowledgement retains the record and keeps picker disarmed, even if keychain removal itself succeeded. +- Later explicit controller enable: while pending remains, the default `ensurePickerCa` refuses and the controller does not call `trustPickerCa` or `rearm`; this prevents a bypass of startup's cleanup order. +- Busy egress port: observe bind refusal, no other process disturbance, row still present for a later retry. + +The chosen blind relay leaves Claude Code traffic from Desktop on its normal upstream route during recovery; it never selects the main intercept listener on this egress port. The separate main intercept port retains its existing policy. This is deliberately narrower than enabling picker MITM while a predecessor may remain trusted. + +## Proof before closing this phase + +Run the affected picker runtime/recovery files under isolated `HOME`, `OPENCODEX_HOME`, `TMPDIR`, plus typecheck. Read the actual CONNECT response and journal state, not merely a mocked `startProxy` call. Do not invoke the user's keychain or installed Desktop. diff --git a/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md b/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md new file mode 100644 index 00000000000..5c39f0b4fe7 --- /dev/null +++ b/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md @@ -0,0 +1,29 @@ +# 030 — B10 drift heal cancellation boundary + +Depends on: `000_plan.md`. Work phase `wp3`, class C4 because it governs a background write into Codex configuration. The source change stays in `src/codex/catalog-auto-refresh.ts` and its tests. + +## File changes (diff-level) + +| Path | Change | Before → after | +| --- | --- | --- | +| `src/codex/catalog-auto-refresh.ts` | MODIFY | `healCodexConfigDrift(config)` calls `syncModelsToCodex` with the tick's stale snapshot, awaits provider discovery, and checks `generation` only later → call `injectCodexConfig` directly for missing injected URL root keys. Read `JOURNAL_PATH` with a **bounded, read-only** regular-file/JSON check and obtain its version-1 `injectedCatalogPath` without calling the mutating `journaledInjectedCatalogPath()` helper. Resolve a relative path against the Codex config home; accept only a readable regular catalog whose JSON parses as a catalog, otherwise try the current catalog path with the same test, otherwise pass `null`. Pass `lockTimeoutMs: TICK_DEADLINE_MS` and a synchronous `beforeClientWrite` guard. The guard checks captured timer generation and persisted config at the actual write boundary. A stale or stopped tick defers the heal without a catalog/cache write. Keep post-inject on-disk drift as the sole basis for `healed`. | +| `tests/codex-integration/catalog-auto-refresh-scheduler.test.ts` | MODIFY | Existing heal tests mock full sync and see only the requested port/log → assert direct injector options include the 1-second lock wait and commit guard; stop/restart or persist OFF/new picker order while a deferred injector is waiting, then invoke its guard and prove no stale write/log. A successful on-generation injection is reported healed only after the root key is observed. The ordinary catalog-only converge path remains unchanged. | + +## Boundary and activation + +The 1-second constant already documents a **commit-lock wait**, not a whole-tick deadline (`src/codex/catalog-auto-refresh.ts:27`; `src/codex/convergence.ts:640`). This phase does not claim a 1-second limit on configuration preparation. `syncModelsToCodex` has no cancellation or deadline option and can perform provider discovery before injection (`src/codex/sync.ts:99,256`); a post-await generation check cannot revoke those writes. A direct config injector already accepts `beforeClientWrite` and `lockTimeoutMs` (`src/codex/inject.ts:98-123`) and checks the guard under its write coordination (`:575-584`). It therefore fits this file's write scope. + +`codexConfigDrift` detects a missing journaled `openai_base_url` or realtime URL, **not** catalog-only loss (`src/codex/config-drift-heal.ts:57-79`). The chosen catalog path must preserve a non-default file that the journal says the last injection selected, provided it is readable and structurally valid; the Desktop rewrite may have removed `model_catalog_json` from `config.toml`. `JOURNAL_PATH` (`src/codex/journal.ts:17`) names the file, and `resolveCodexConfigPath` (`src/codex/paths.ts:142`) resolves a relative catalog path. The existing exported path getter is unsuitable for this background observer because its default `readJournal()` removes an invalid journal (`src/codex/journal.ts:237-254`); the lane cannot modify `journal.ts`, so this helper reads only the one needed field without cleanup. A missing/invalid/unreadable journaled catalog falls back to another verified catalog or `null`, never a nonexistent file. + +- Stopped generation: enter a deferred injector, call `stopCatalogAutoRefresh`, then trigger the guard and assert refusal before any stubbed write or success log. +- Changed configuration: edit persisted ON settings during the await and trigger the guard; stale values are not injected. Repeat for OFF intent. A later tick can retry from a fresh snapshot. +- External provider: a successful injector may intentionally avoid writing; recheck missing root keys and report `not-healed`, never infer success from the return value. +- Non-default catalog: persist a journaled existing path, remove `model_catalog_json` from the fixture TOML, trigger drift repair, and assert the injector receives that same path rather than the default. +- Invalid journal or path: preserve the journal bytes while the observer refuses them; reject a directory, symlink, unreadable or malformed catalog and fall back to a separately verified catalog or `null`. +- Lock contention: pass `lockTimeoutMs: 1000`, verify refusal is deferred; avoid a busy wait outside the injector. + +No new persisted field or enum is introduced; creation, serialization, deserialization and consumer chains are unchanged. A synchronous guard can be bypassed by direct non-tick callers, which retain their own contracts; this phase protects only the auto-refresh drift heal. The final enforcement layer is the injector commit guard, backed by focused tests and hosted CI; process termination during a partially completed external application is outside its guarantee. + +## Proof before closing this phase + +Run isolated `bun test tests/codex-integration/catalog-auto-refresh-scheduler.test.ts`, relevant direct-inject/cancellation regressions, and `bun run typecheck`. Document whether any full-sync call remains reachable from the drift branch. Keep any broader stale-model race found in the catalog-only path out of this lane's source edits and report it separately. diff --git a/devlog/_plan/260927_release_train_4/picker-ca/040_integration.md b/devlog/_plan/260927_release_train_4/picker-ca/040_integration.md new file mode 100644 index 00000000000..2bb6a39c10e --- /dev/null +++ b/devlog/_plan/260927_release_train_4/picker-ca/040_integration.md @@ -0,0 +1,21 @@ +# 040 — Source-of-truth, verification, and dev integration + +Depends on: `010_ca_publication.md`, `020_desktop_continuity.md`, and `030_codex_drift_heal.md`. Work phase `wp4`. Delivery is one ordinary PR from this lane branch to `dev`; no release or promotion action. + +## File changes (diff-level) + +| Path | Change | Before → after | +| --- | --- | --- | +| `structure/clients/claude-desktop.md` | MODIFY | A failed predecessor untrust is described only as disabling picker → document the blind egress relay, actual applied URL, preserved row/retry, public pending journal and process-restart retry, with no claim that picker MITM arms. | +| `structure/config.md` | MODIFY | Drift heal says the tick reruns standard full sync → document direct guarded config reinjection, generation cancellation and the write-lock deadline scope; catalog-only convergence remains separate. | +| `structure/overview.md` | MODIFY | No bound picker rotation/continuity invariant → add a narrowly worded `INV-PICKER-01` bound to a test file that actually covers the failed-untrust applied URL and journal retry. The test repeats the id in a comment. | +| `docs-site/src/content/docs/guides/claude-code.md` | MODIFY | Restart guidance omits failed predecessor cleanup → explain that Desktop remains connected through its profile proxy as a blind relay while picker aliases are unavailable, and that `picker status`/a later retry reflects recovery. English remains canonical; review translated versions for contradictions and adjust directly affected translations only. | +| `devlog/_plan/260927_release_train_4/picker-ca/` | MODIFY | Fill each numbered phase with the actual result, commands, PR/CI links and limitations. Keep unreleased security working detail in ignored `.tmp/`. | + +## Gates and exact-head evidence + +1. Rebase/merge latest `origin/dev` before push; inspect the union for file-size caps, locale/union/count drift, and touched-source doc map. Do not raise ratchet caps. +2. Run focused picker CA/runtime/recovery and Codex scheduler tests with isolated home; `bun run test:changed`; `bun run typecheck`; `bun run privacy:scan`; `bun run structure:check`; `bun run skill:surface:check`. Run docs-site frozen install/build if the guide changes. The local full suite may be omitted for seven-lane contention only with exact focused results and the CI coverage boundary in PR Verification. +3. Independently review the source, the direct CONNECT observation, journal contents, test home paths and private-key absence. No real user keychain, Desktop library, or service is touched. +4. Fill the repository PR template. Security-sensitive CA changes require explicit technical security review under `MAINTAINERS.md`. Verify the branch is current, every required check is **success on the PR's exact head**, and correct Codex/CodeRabbit findings are addressed. If macOS shards are skipped by the native path filter, run or request an exact-head `macos-control` workflow and report its own job result; a skipped job is not success. +5. Merge only this PR into `dev` under the delegated authority, record PR number/merge SHA and policy choice, then inspect the resulting dev CI. If red due to this change, repair via a new scoped PR and repeat the gate. Do not modify `main`, `preview`, version, release, or another lane. From 7e1ecbd732393d3f2612ea8f8ac9ec13e958cb9b Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 28 Sep 2026 00:28:56 +0900 Subject: [PATCH 2/6] WIP: serialize picker CA publication and preserve pending untrust --- .../picker-ca/010_ca_publication.md | 19 +- .../picker-ca/020_desktop_continuity.md | 2 +- src/claude/intercept/picker-ca.ts | 228 +++++++++++--- .../claude-picker-ca.test.ts | 284 +++++++++++++++--- 4 files changed, 438 insertions(+), 95 deletions(-) diff --git a/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md index b26ee2f2057..e7c56ecfd80 100644 --- a/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md +++ b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md @@ -6,23 +6,26 @@ Depends on: `000_plan.md`. Work phase `wp1`, class C4. Source owner: `src/claude | Path | Change | Before → after | | --- | --- | --- | -| `src/claude/intercept/picker-ca.ts` | MODIFY | `underPickerCaLock` returns `T | undefined`, and void callbacks are repeated outside the lock → tagged `{ ok: true, value: T } | { ok: false, error }`, with a caller-visible failure and no unlocked publication. The cached and fresh paths both decide and publish only under the lock. A live foreign owner blocks a fresh publication **and makes a cached mismatched authority throw**; returning the cached CA after merely deferring publication would let callers arm it against a different on-disk owner. `processAuthorities` is set only after a successful locked path. | -| `src/claude/intercept/picker-ca.ts` | MODIFY | A replacement overwrites the last public PEM with no durable predecessor → under the same lock, parse the current public PEM, write one `{ sha1, sha256, certPem }` record to `pending-untrust.json` by private temporary file/rename **before** `ca.pem` changes, then publish `ca.pem` and `ca-owner.json`. Refuse another replacement while that record exists. Reject malformed or oversized pending data rather than dropping it. Expose a bounded read, a check of whether the pending PEM is still the published certificate with a matching **live** owner, and exact-entry acknowledgement under the lock. A missing record means no pending item. No `keyPem`, signing key, private path, or request data enters the record. | -| `src/claude/intercept/picker-ca.ts` | MODIFY | `ensurePickerCa(configDir)` returns a CA to controller/CLI/runtime even when predecessor cleanup is unresolved → keep its return type, but make its default call refuse any pending record or replacement requiring untrust. Add a narrow startup-only option that permits a fresh replacement **only when no prior pending record exists** and returns the CA with its newly queued predecessor for immediate cleanup in phase 020. The ordinary controller `enable` and picker-runtime material paths use the default and cannot trust or rearm a new CA before cleanup. | -| `tests/claude-integration/claude-picker-ca.test.ts` | MODIFY | Existing process-restart and foreign-owner tests check only the final certificate → test a successful void callback executes exactly once, an actual second process holding the SQLite lock causes zero publication outside it, and competing processes leave `ca.pem` and `ca-owner.json` with matching SHA-256 after serialization. Check the pending record contains only public PEM/fingerprints and is written before replacement; malformed record blocks publication; cached republish obeys the same lock. | +| `src/claude/intercept/picker-ca.ts` | MODIFY | `underPickerCaLock` returns `T | undefined`, and void callbacks are repeated outside the lock → tagged `{ kind: "acquired", value: T } | { kind: "unavailable", error }`, with no unlocked publication. Track whether the callback entered: an exception after entry or during release propagates as a partial-operation failure, never as mere lock contention. The cached and fresh paths both decide and publish only under the lock. A live foreign owner blocks a fresh publication **and makes a cached mismatched authority throw**; returning the cached CA after merely deferring publication would let callers arm it against a different on-disk owner. Owner records include a process-start identity where the OS supports it, so a reused PID does not falsely count as the recorded owner; legacy records without it conservatively defer. `processAuthorities` is set only after successful locked publication. | +| `src/claude/intercept/picker-ca.ts` | MODIFY | A replacement overwrites the last public PEM with no durable predecessor → under the same lock, parse the current public PEM, write one `{ sha1, sha256, certPem }` record to `pending-untrust.json` by private temporary file/rename **before** `ca.pem` changes, then publish `ca.pem` and `ca-owner.json`. Public temp files use unique exclusive names and are cleaned after rename failure. Refuse another replacement while the record exists. Reject malformed, nonregular, or oversized pending data rather than dropping it. Expose a bounded read, a locked check of whether the pending PEM is still the published certificate with a matching **live** owner (including this PID), and exact-entry acknowledgement that rereads PEM and both hashes under the lock. A missing record means no pending item. No `keyPem`, signing key, private path, or request data enters the record. | +| `src/claude/intercept/picker-ca.ts` | MODIFY | `ensurePickerCa(configDir)` returns a CA to controller/CLI/runtime even when predecessor cleanup is unresolved → keep its `PickerCa` return type, but make its default call refuse any pending record or replacement requiring untrust. Add optional `{ rotation: "startup" }` that permits one fresh replacement **only when no prior pending record exists** and returns the CA with its newly queued predecessor for immediate cleanup in phase 020. Only `startClaudeIntercept` uses the option; controller `enable` and picker-runtime material paths use the default and cannot trust or rearm through pending cleanup. | +| `tests/claude-integration/claude-picker-ca.test.ts` | MODIFY | Existing cached-authority test at `:142-158` expects an ordinary call to overwrite a different valid certificate → change that case to an explicit refusal with no publication, while keeping the missing-file republish as a separate locked success. Existing process-restart and foreign-owner tests check only the final certificate → test a successful void callback executes exactly once, an actual second process holding the SQLite lock causes zero publication outside it, and competing processes leave `ca.pem` and `ca-owner.json` with matching SHA-256 after serialization. Check the pending record contains only public PEM/fingerprints and is written before replacement; malformed record blocks publication. | ## Conditional activation and field chain -- Normal void callback: call the tagged lock helper with a side-effect counter, assert one increment and `ok: true` even though `value` is `undefined`. +- Normal void callback: spy on `node:fs.renameSync` while calling public `ensurePickerCa` in a fresh Bun process. Exactly one `ca.pem` and one `ca-owner.json` replacement prove the locked publisher ran once; the current code produces two of each (four rename calls) with no lock contention. Avoid exporting a private helper only for the test. - Busy lock: hold the actual picker SQLite lock in a child process, call `ensurePickerCa` in another process, assert no publication and an explicit deferred outcome; release then retry. No in-process stub substitutes for this contention check. -- Live foreign owner: publish from a live second process, call the cached `ensurePickerCa` in the first, and assert it refuses instead of returning an authority whose public PEM is no longer published. +- Live foreign owner: present a controlled foreign PEM plus matching owner record with a live child PID, call the cached `ensurePickerCa` in the first process, and assert refusal. The new fresh-owner guard itself prevents a legitimate second `ensurePickerCa` from creating that state. Also run two real publishers against one state dir; one must defer while its peer remains live, and the final CA/owner pair must match. +- PID reuse: alter only the recorded start identity while keeping the same live PID and fingerprint; on macOS/Linux the owner check rejects the mismatch. A record from an older build with no start identity remains conservative. - Prior certificate: start with a valid published public PEM, rotate, assert the record exists with its SHA-1/SHA-256 before the new `ca.pem` is visible. If the write fails, the old PEM remains published. -- Interrupted publication: fault-inject failure after recording pending but before replacing `ca.pem`; while the matching owner process is alive, a second process sees the pending PEM still actively published and **does not** untrust it. It defers picker construction/cleanup; after the owner exits, a later process retries removal. +- Interrupted publication: fault-inject failure after recording pending but before replacing `ca.pem`; assert the outgoing public PEM stays published and pending is retained. The fresh-owner guard normally refuses rotation while that published owner is still alive, so a live-owner/pending match is a separate controlled fixture (for example a changed process identity after a crash), not a claimed normal result of this fault injection. Phase 020 must defer keychain removal for that fixture and retry after the owner exits. - Bad record: stage malformed JSON and attempt rotation, assert no replacement and no private material. No parsing fallback silently erases recovery data. - Sequential predecessors: an unresolved A blocks another publication; after exact A acknowledgement, a later B-to-C rotation may create B as the next single pending item. A mismatched acknowledgement leaves the original record intact. - Later controller enable: leave a pending entry after a failed removal, invoke the real controller enable path, and assert no trust add or picker rearm. The default `ensurePickerCa` refusal is the gate, not a changed controller implementation. -New field chain: the startup-only `ensurePickerCa` path creates the single pending entry; the JSON writer serializes it; the bounded reader validates its fingerprints against the PEM; phase 020 consumes it for `untrustPickerCa`, then the locked acknowledgement removes it. Ordinary `ensurePickerCa` callers consume only the no-pending state. The option is created only by `startClaudeIntercept`, is not serialized, and has no other consumer. A test-visible lock helper, if needed to observe the void callback, is a narrow code contract and not user configuration. +New field chain: the startup-only `ensurePickerCa` path creates the single pending entry; the JSON writer serializes it; the bounded reader validates its fingerprints against the PEM; phase 020 consumes it for `untrustPickerCa`, then the locked acknowledgement removes it. Ordinary `ensurePickerCa` callers consume only the no-pending state. The option is created only by `startClaudeIntercept`, is not serialized, and has no other consumer. The lock helper stays private; the public publication path supplies the test oracle. + +wp1 P stale check: `origin/dev` remains `24b2f39b77` and `picker-ca.ts` is unchanged since the docs-only plan. A scratch probe in an isolated `HOME`/`OPENCODEX_HOME`/`TMPDIR` observed **four** `renameSync` destinations on one uncontended `ensurePickerCa` call (two `ca.pem`, two `ca-owner.json`); the regression will require exactly two. The cached-authority test at `claude-picker-ca.test.ts:142-158` and second-process tests at `:162-215` assume unconditional replacement of a different valid/live certificate; update them to the new refusal/retry contract while preserving the still-safe missing-file republish assertion. ## Proof before closing this phase diff --git a/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md b/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md index ca24537324c..f7d8639789f 100644 --- a/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md +++ b/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md @@ -6,7 +6,7 @@ Depends on: `010_ca_publication.md`. Work phase `wp2`, class C4. Only the picker | Path | Change | Before → after | | --- | --- | --- | -| `src/claude/intercept/runtime.ts` | MODIFY | Read only a boolean `pickerProfileApplied`, rotate, and skip the entire picker proxy bind on failed untrust → retain the inspected applied profile's validated URL/port, clear any prior pending item **before** a fresh rotation, call phase 010's startup-only `ensurePickerCa` option, then clear the newly queued outgoing item. Before either removal, ask phase 010's locked owner check whether this pending PEM is still published by a live process; if so, defer without invoking `security` or acknowledging. A failed/deferred untrust, corrupt journal, or deferred publication blocks picker creation. Each safe removal uses a private temporary **public** PEM copy and calls `untrustPickerCa`; only a confirmed result permits exact-entry acknowledgement. Both cleanup passes and acknowledgements must finish before picker construction. | +| `src/claude/intercept/runtime.ts` | MODIFY | Read only a boolean `pickerProfileApplied`, rotate, and skip the entire picker proxy bind on failed untrust → retain the inspected applied profile's validated URL/port, clear any prior pending item **before** a fresh rotation, call `ensurePickerCa(configDir, { rotation: "startup" })`, then clear the newly queued outgoing item. Before either removal, ask phase 010's locked owner check whether this pending PEM is still published by a live process; if so, defer without invoking `security` or acknowledging. A failed/deferred untrust, corrupt journal, or deferred publication blocks picker creation. Each safe removal uses a private temporary **public** PEM copy and calls `untrustPickerCa`; only a confirmed result permits exact-entry acknowledgement. Both cleanup passes and acknowledgements must finish before picker construction. | | `src/claude/intercept/runtime.ts` | MODIFY | `pickerBlocked` leaves the applied profile's egress URL without a listener → bind `startConnectProxy` at the URL's actual loopback port with `interceptHosts: []` and a per-CONNECT decision that is always `{ kind: "blind" }`. This branch creates no picker runtime/controller/TLS leaf, performs no trust add, does not rewrite or remove the profile, and closes the relay in `stop`/error teardown. The live state reports the bound proxy port while the picker runtime remains null. Existing `offlinePickerStatus` may still say `proxy_unavailable`: that reason describes the picker feature/controller, not egress liveness; the runtime state and actual CONNECT prove degraded relay availability. If the port belongs to another process, do not take it over; preserve the row and report a degraded bind failure. | | `tests/claude-integration/claude-picker-runtime.test.ts` | MODIFY | The failed-untrust test checks only the main intercept port → apply an owned Desktop profile first, read its `egressProxyUrl`, inject failed removal, CONNECT through that exact URL to a fake upstream, assert a 200 blind tunnel and no picker creation/trust addition, and confirm row ID plus previous selection and retry intent stay intact. Check normal startup still arms only after cleanup. | | `tests/claude-integration/claude-picker-recovery.test.ts` | NEW | Start separate Bun processes with one isolated fixture home, fake `SecurityRunner`, and no actual `/usr/bin/security`. First startup fails removal; a new process retries the recorded old PEM before rotating again; success clears the record and permits picker creation. Also test controller enable with pending cleanup refuses a trust add. Register this `.test.ts` in both `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json`. | diff --git a/src/claude/intercept/picker-ca.ts b/src/claude/intercept/picker-ca.ts index b79f417a5c5..a14241b734c 100644 --- a/src/claude/intercept/picker-ca.ts +++ b/src/claude/intercept/picker-ca.ts @@ -1,5 +1,5 @@ -import { createHash, X509Certificate } from "node:crypto"; -import { chmodSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { createHash, randomUUID, X509Certificate } from "node:crypto"; +import { chmodSync, lstatSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { withClientLifecycleSync } from "../../client/lifecycle-lock"; import { @@ -15,13 +15,16 @@ export const PICKER_CA_COMMON_NAME = "opencodex Claude Desktop Picker CA"; export const PICKER_STATE_DIR = "claude-picker"; export interface PickerCa extends LocalInterceptCa { fingerprint: string } +export interface PendingPickerCaUntrust { certPem: string; sha1: string; sha256: string } const processAuthorities = new Map(); +const MAX_PENDING_CA_BYTES = 64 * 1024; export function pickerStateDir(configDir: string): string { return join(configDir, PICKER_STATE_DIR); } export function pickerCaCertPath(configDir: string): string { return join(pickerStateDir(configDir), "ca.pem"); } export function pickerLeafCertPath(configDir: string): string { return join(pickerStateDir(configDir), "leaf.pem"); } export function pickerCaOwnerPath(configDir: string): string { return join(pickerStateDir(configDir), "ca-owner.json"); } +export function pickerCaPendingUntrustPath(configDir: string): string { return join(pickerStateDir(configDir), "pending-untrust.json"); } function pickerCaLockPath(configDir: string): string { return join(pickerStateDir(configDir), "ca.lock.sqlite"); } export function pickerCaFingerprints(certPem: string): { sha1: string; sha256: string } { @@ -34,10 +37,14 @@ export function pickerCaFingerprints(certPem: string): { sha1: string; sha256: s /** Atomically publish a public certificate; these files carry no key material. */ function publishPem(path: string, pem: string): void { - const tmp = `${path}.${process.pid}.tmp`; - writeFileSync(tmp, pem, { mode: 0o644 }); - try { chmodSync(tmp, 0o644); } catch { /* best-effort on platforms without POSIX modes */ } - renameSync(tmp, path); + const tmp = `${path}.${process.pid}.${randomUUID()}.tmp`; + writeFileSync(tmp, pem, { flag: "wx", mode: 0o644 }); + try { + try { chmodSync(tmp, 0o644); } catch { /* best-effort on platforms without POSIX modes */ } + renameSync(tmp, path); + } finally { + rmSync(tmp, { force: true }); + } } /** @@ -47,11 +54,38 @@ function publishPem(path: string, pem: string): void { */ function publishAuthority(configDir: string, ca: PickerCa): void { publishPem(pickerCaCertPath(configDir), ca.certPem); - publishPem(pickerCaOwnerPath(configDir), JSON.stringify({ pid: process.pid, sha256: ca.fingerprint }) + "\n"); + publishPem(pickerCaOwnerPath(configDir), JSON.stringify({ + pid: process.pid, + startTime: processStartIdentity(process.pid), + sha256: ca.fingerprint, + }) + "\n"); +} + +/** The OS process start identity prevents a recycled PID from impersonating the recorded owner. */ +function processStartIdentity(pid: number): string | null { + if (process.platform === "linux") { + try { + const stat = readFileSync(`/proc/${pid}/stat`, "utf8"); + const afterCommand = stat.lastIndexOf(") "); + const ticks = afterCommand < 0 ? undefined : stat.slice(afterCommand + 2).trim().split(/\s+/)[19]; + return ticks && /^\d+$/.test(ticks) ? ticks : null; + } catch { return null; } + } + if (process.platform === "darwin") { + try { + const result = Bun.spawnSync(["/bin/ps", "-o", "lstart=", "-p", String(pid)], { + stdin: "ignore", stdout: "pipe", stderr: "ignore", + }); + const value = result.stdout.toString().trim(); + return result.exitCode === 0 && value.length > 0 && value.length <= 128 ? value : null; + } catch { return null; } + } + return null; } -function foreignProcessAlive(pid: number): boolean { - if (!Number.isInteger(pid) || pid <= 0 || pid === process.pid) return false; +function processAlive(pid: number): boolean { + if (!Number.isInteger(pid) || pid <= 0) return false; + if (pid === process.pid) return true; try { process.kill(pid, 0); return true; @@ -62,38 +96,142 @@ function foreignProcessAlive(pid: number): boolean { } /** - * True when the published certificate belongs to a different live process's authority. The owner + * True when the published certificate belongs to a live process's authority. The owner * record is only trusted while it describes the certificate actually on disk: a stale or * third-party `ca-owner.json` cannot shield a file that was tampered with after the owner wrote it. */ -function foreignLiveOwner(configDir: string, published: string): boolean { +function livePublishedOwner(configDir: string, published: string): boolean { try { const owner = JSON.parse(readFileSync(pickerCaOwnerPath(configDir), "utf8")) as unknown; if (owner === null || typeof owner !== "object") return false; - const { pid, sha256 } = owner as { pid?: unknown; sha256?: unknown }; + const { pid, sha256, startTime } = owner as { pid?: unknown; sha256?: unknown; startTime?: unknown }; if (typeof pid !== "number" || typeof sha256 !== "string") return false; if (sha256 !== pickerCaFingerprints(published).sha256) return false; - return foreignProcessAlive(pid); + if (!processAlive(pid)) return false; + // Older owner records have no start identity. Fail conservatively for those: deferring + // cleanup is safer than removing trust from a process that could still be serving. + if (startTime === undefined || startTime === null) return true; + if (typeof startTime !== "string" || startTime.length === 0 || startTime.length > 128) return false; + const actual = processStartIdentity(pid); + return actual === null || actual === startTime; } catch { return false; } } +function publicCertificate(pem: string): PendingPickerCaUntrust | null { + try { + const certPem = new X509Certificate(pem).toString(); + return { certPem, ...pickerCaFingerprints(certPem) }; + } catch { + return null; + } +} + +/** The pending record is public-only, but malformed or unsafe state must never be discarded. */ +export function readPendingPickerCaUntrust(configDir: string): PendingPickerCaUntrust | null { + const path = pickerCaPendingUntrustPath(configDir); + let stat; + try { stat = lstatSync(path); } + catch (error) { + if (error !== null && typeof error === "object" && (error as { code?: unknown }).code === "ENOENT") return null; + throw error; + } + if (!stat.isFile() || stat.nlink !== 1 || stat.size > MAX_PENDING_CA_BYTES) { + throw new Error("picker_ca_pending_untrust_unsafe"); + } + const raw = readFileSync(path, "utf8"); + if (Buffer.byteLength(raw) > MAX_PENDING_CA_BYTES) throw new Error("picker_ca_pending_untrust_unsafe"); + let value: unknown; + try { value = JSON.parse(raw); } + catch { throw new Error("picker_ca_pending_untrust_invalid"); } + if (value === null || typeof value !== "object" || Array.isArray(value)) { + throw new Error("picker_ca_pending_untrust_invalid"); + } + const pending = value as Partial; + if (Object.keys(pending).sort().join(",") !== "certPem,sha1,sha256" + || typeof pending.certPem !== "string" || typeof pending.sha1 !== "string" || typeof pending.sha256 !== "string") { + throw new Error("picker_ca_pending_untrust_invalid"); + } + const publicOnly = publicCertificate(pending.certPem); + if (!publicOnly || pending.certPem !== publicOnly.certPem + || pending.sha1 !== publicOnly.sha1 || pending.sha256 !== publicOnly.sha256) { + throw new Error("picker_ca_pending_untrust_invalid"); + } + return publicOnly; +} + +function writePendingPickerCaUntrust(configDir: string, pending: PendingPickerCaUntrust): void { + const path = pickerCaPendingUntrustPath(configDir); + const temporary = `${path}.${process.pid}.${randomUUID()}.tmp`; + writeFileSync(temporary, JSON.stringify(pending) + "\n", { flag: "wx", mode: 0o600 }); + try { + try { chmodSync(temporary, 0o600); } catch { /* best-effort on platforms without POSIX modes */ } + renameSync(temporary, path); + } finally { + rmSync(temporary, { force: true }); + } +} + /** - * `check` runs under a cross-process lock keyed to the picker state dir so the read-decide-publish - * sequence cannot interleave with a peer's. Returns undefined when the lock cannot be taken, and - * callers then run the same step without it. For a cached authority that step still defers to a - * live foreign owner. A fresh authority publishes unconditionally, with or without the lock; the - * peer it displaces sees the new owner record and stops republishing. + * A lock failure is separate from a successful void callback. Once the callback entered, an + * exception (including lock release failure) propagates: its effects may be partial. */ -function underPickerCaLock(configDir: string, check: () => T): T | undefined { +function underPickerCaLock(configDir: string, check: () => T): + | { kind: "acquired"; value: T } + | { kind: "unavailable"; error: unknown } { + let entered = false; try { - return withClientLifecycleSync(check, { lockPath: pickerCaLockPath(configDir) }); - } catch { - return undefined; + const value = withClientLifecycleSync(() => { + entered = true; + return check(); + }, { lockPath: pickerCaLockPath(configDir) }); + return { kind: "acquired", value }; + } catch (error) { + if (entered) throw error; + return { kind: "unavailable", error }; + } +} + +function lockedPickerCa(configDir: string, check: () => T): T { + const result = underPickerCaLock(configDir, check); + if (result.kind === "unavailable") throw result.error; + return result.value; +} + +function publishedPickerCa(configDir: string): string | null { + try { return readFileSync(pickerCaCertPath(configDir), "utf8"); } + catch (error) { + if (error !== null && typeof error === "object" && (error as { code?: unknown }).code === "ENOENT") return null; + throw error; } } +/** A pending CA may still be serving when journal publication preceded a failed PEM replacement. */ +export function pendingPickerCaHasLivePublishedOwner(configDir: string, pending: PendingPickerCaUntrust): boolean { + return lockedPickerCa(configDir, () => { + const current = readPendingPickerCaUntrust(configDir); + if (!current || current.sha1 !== pending.sha1 + || current.sha256 !== pending.sha256 || current.certPem !== pending.certPem) { + throw new Error("picker_ca_pending_untrust_changed"); + } + const published = publishedPickerCa(configDir); + return published !== null && publicCertificate(published)?.sha256 === pending.sha256 + && livePublishedOwner(configDir, published); + }); +} + +/** Acknowledgement never clears an entry another process created or replaced. */ +export function acknowledgePendingPickerCaUntrust(configDir: string, pending: PendingPickerCaUntrust): boolean { + return lockedPickerCa(configDir, () => { + const current = readPendingPickerCaUntrust(configDir); + if (!current || current.certPem !== pending.certPem + || current.sha1 !== pending.sha1 || current.sha256 !== pending.sha256) return false; + rmSync(pickerCaPendingUntrustPath(configDir)); + return true; + }); +} + /** * Drop the legacy exportable signing key, if one exists in the picker state dir. Releases before * the process-scoped authority persisted `ca.key` next to `ca.pem`; the removal is deliberately @@ -103,39 +241,33 @@ export function discardPickerCaKey(configDir: string): void { rmSync(join(pickerStateDir(configDir), "ca.key"), { force: true }); } -export function ensurePickerCa(configDir: string): PickerCa { +export function ensurePickerCa(configDir: string, options: { rotation?: "startup" } = {}): PickerCa { const dir = pickerStateDir(configDir); // The signing key must never survive this process: another process under the same user could // otherwise steal it and later take over the predictable loopback proxy. Drop a key left (or // restored) by an older release on every call, including cache hits. discardPickerCaKey(configDir); - const path = pickerCaCertPath(configDir); const cached = processAuthorities.get(dir); - if (cached) { - // The published certificate is this authority's public face. If it went missing, republish; - // if a *live* peer rotated it, defer to the owner record — republishing a certificate another - // process still serves would re-point trust inspection at an authority that process controls. - const republishUnlessForeignOwned = (): void => { - let published: string | null = null; - try { published = readFileSync(path, "utf8"); } catch { /* missing or unreadable: republish */ } - if (published === cached.certPem) return; - if (published !== null && foreignLiveOwner(configDir, published)) return; - mkdirSync(dir, { recursive: true, mode: 0o700 }); - publishAuthority(configDir, cached); - }; - if (underPickerCaLock(configDir, republishUnlessForeignOwned) === undefined) { - republishUnlessForeignOwned(); - } - return cached; - } - const ca = createCertificateAuthority({ commonName: PICKER_CA_COMMON_NAME, permittedDnsNames: [PICKER_HOST] }); - const pickerCa = { ...ca, fingerprint: pickerCaFingerprints(ca.certPem).sha256 }; + const pickerCa = cached ?? (() => { + const ca = createCertificateAuthority({ commonName: PICKER_CA_COMMON_NAME, permittedDnsNames: [PICKER_HOST] }); + return { ...ca, fingerprint: pickerCaFingerprints(ca.certPem).sha256 }; + })(); mkdirSync(dir, { recursive: true, mode: 0o700 }); - // This process is a fresh authority: publish unconditionally. A peer whose cert we just - // replaced will observe the divergence against our live owner record and stop republishing. - const publish = (): void => { publishAuthority(configDir, pickerCa); }; - if (underPickerCaLock(configDir, publish) === undefined) publish(); - processAuthorities.set(dir, pickerCa); + lockedPickerCa(configDir, () => { + if (readPendingPickerCaUntrust(configDir)) throw new Error("picker_ca_pending_untrust"); + const published = publishedPickerCa(configDir); + if (published === pickerCa.certPem) return; + if (published !== null && livePublishedOwner(configDir, published)) { + throw new Error("picker_ca_live_owner"); + } + const outgoing = published === null ? null : publicCertificate(published); + if (outgoing && outgoing.sha256 !== pickerCa.fingerprint) { + if (options.rotation !== "startup") throw new Error("picker_ca_rotation_requires_startup"); + writePendingPickerCaUntrust(configDir, outgoing); + } + publishAuthority(configDir, pickerCa); + }); + if (!cached) processAuthorities.set(dir, pickerCa); return pickerCa; } diff --git a/tests/claude-integration/claude-picker-ca.test.ts b/tests/claude-integration/claude-picker-ca.test.ts index 7d7d17d3386..6c12a7e543e 100644 --- a/tests/claude-integration/claude-picker-ca.test.ts +++ b/tests/claude-integration/claude-picker-ca.test.ts @@ -7,8 +7,9 @@ import { pathToFileURL } from "node:url"; import { connect, createServer } from "node:tls"; import { createCertificateAuthority, createLocalInterceptCa, issueServerLeaf } from "../../src/claude/intercept/local-ca"; import { - ensurePickerCa, issuePickerLeaf, pickerCaCertPath, pickerCaFingerprints, - pickerCaOwnerPath, pickerLeafCertPath, pickerStateDir, PICKER_CA_COMMON_NAME, PICKER_HOST, + acknowledgePendingPickerCaUntrust, ensurePickerCa, issuePickerLeaf, pickerCaCertPath, pickerCaFingerprints, + pickerCaOwnerPath, pickerCaPendingUntrustPath, pickerLeafCertPath, pickerStateDir, + pendingPickerCaHasLivePublishedOwner, readPendingPickerCaUntrust, PICKER_CA_COMMON_NAME, PICKER_HOST, } from "../../src/claude/intercept/picker-ca"; function tempDir(): string { return mkdtempSync(join(tmpdir(), "ocx-picker-ca-")); } @@ -139,81 +140,288 @@ test("picker authority keeps its private key in process memory and removes a leg expect(constraints(readFileSync(pickerCaCertPath(dir), "utf8"))?.dnsNames).toEqual([PICKER_HOST]); }); -test("a cached authority still removes a restored legacy key and republishes a stale certificate", () => { +test("a cached authority refuses a different certificate but republishes a missing one", () => { const dir = tempDir(); const stateDir = pickerStateDir(dir); const ca = ensurePickerCa(dir); - // Another process published a different certificate while a legacy key reappeared on disk. + // A different valid certificate needs startup rotation and verified untrust first. writeFileSync(join(stateDir, "ca.key"), "legacy-exportable-key\n"); - writeFileSync(pickerCaCertPath(dir), createCertificateAuthority({ + const other = createCertificateAuthority({ commonName: PICKER_CA_COMMON_NAME, permittedDnsNames: [PICKER_HOST], - }).certPem); - expect(ensurePickerCa(dir).fingerprint).toBe(ca.fingerprint); - expect(readFileSync(pickerCaCertPath(dir), "utf8")).toBe(ca.certPem); + }).certPem; + writeFileSync(pickerCaCertPath(dir), other); + expect(() => ensurePickerCa(dir)).toThrow("picker_ca_rotation_requires_startup"); + expect(readFileSync(pickerCaCertPath(dir), "utf8")).toBe(other); expect(existsSync(join(stateDir, "ca.key"))).toBe(false); - // A certificate that went missing entirely is republished the same way. + // A missing certificate has no predecessor to untrust and can be republished under the lock. rmSync(pickerCaCertPath(dir)); expect(ensurePickerCa(dir).fingerprint).toBe(ca.fingerprint); expect(readFileSync(pickerCaCertPath(dir), "utf8")).toBe(ca.certPem); }); const PICKER_CA_MODULE_URL = pathToFileURL(join(import.meta.dir, "../../src/claude/intercept/picker-ca.ts")).href; +const LIFECYCLE_LOCK_MODULE_URL = pathToFileURL(join(import.meta.dir, "../../src/client/lifecycle-lock.ts")).href; + +async function waitForFile(path: string): Promise { + for (let attempt = 0; attempt < 400; attempt += 1) { + if (existsSync(path)) return; + await Bun.sleep(5); + } + throw new Error(`fixture signal missing: ${path}`); +} + +test("one uncontended fresh authority publishes its certificate and owner exactly once", () => { + const dir = tempDir(); + const child = Bun.spawnSync({ + cmd: [process.execPath, "-e", + `import { spyOn } from "bun:test"; import * as fs from "node:fs";\n` + + `const renames = spyOn(fs, "renameSync");\n` + + `const { ensurePickerCa } = await import(${JSON.stringify(PICKER_CA_MODULE_URL)});\n` + + `ensurePickerCa(${JSON.stringify(dir)});\n` + + `process.stdout.write(JSON.stringify(renames.mock.calls.map(call => call[1])));`], + cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, + stdout: "pipe", + stderr: "pipe", + }); + expect(child.exitCode).toBe(0); + const published = JSON.parse(child.stdout.toString()) as string[]; + expect(published.filter(path => path === pickerCaCertPath(dir))).toHaveLength(1); + expect(published.filter(path => path === pickerCaOwnerPath(dir))).toHaveLength(1); +}); // The restart contract is process-scoped: a new process must mint its own authority, not reuse // the previous one's certificate. This needs a real second process — the in-process authority // cache would otherwise hand the same keypair back. -test("a second process mints a fresh authority and republishes it", () => { +test("a replacement process mints a fresh authority and records the outgoing public root", () => { const dir = tempDir(); - const ours = ensurePickerCa(dir); - const child = Bun.spawnSync({ + const first = Bun.spawnSync({ cmd: [process.execPath, "-e", `import { ensurePickerCa } from ${JSON.stringify(PICKER_CA_MODULE_URL)};\n` + `process.stdout.write(ensurePickerCa(${JSON.stringify(dir)}).fingerprint);`], cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, stdout: "pipe", stderr: "pipe", }); - expect(child.exitCode).toBe(0); - const childFingerprint = child.stdout.toString().trim(); - expect(childFingerprint).not.toBe(ours.fingerprint); - // The newer process's authority is the published one. - expect(pickerCaFingerprints(readFileSync(pickerCaCertPath(dir), "utf8")).sha256).toBe(childFingerprint); + expect(first.exitCode).toBe(0); + const firstFingerprint = first.stdout.toString().trim(); + const firstPem = readFileSync(pickerCaCertPath(dir), "utf8"); + const second = Bun.spawnSync({ + cmd: [process.execPath, "-e", + `import { ensurePickerCa } from ${JSON.stringify(PICKER_CA_MODULE_URL)};\n` + + `process.stdout.write(ensurePickerCa(${JSON.stringify(dir)}, { rotation: "startup" }).fingerprint);`], + cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, + stdout: "pipe", + stderr: "pipe", + }); + expect(second.exitCode).toBe(0); + const secondFingerprint = second.stdout.toString().trim(); + expect(secondFingerprint).not.toBe(firstFingerprint); + expect(pickerCaFingerprints(readFileSync(pickerCaCertPath(dir), "utf8")).sha256).toBe(secondFingerprint); + expect(readPendingPickerCaUntrust(dir)).toEqual({ certPem: firstPem, ...pickerCaFingerprints(firstPem) }); + expect(readFileSync(pickerCaPendingUntrustPath(dir), "utf8")).not.toContain("PRIVATE KEY"); }); test("a live foreign owner is never clobbered; a dead one is reclaimed", async () => { const dir = tempDir(); const ours = ensurePickerCa(dir); const child = Bun.spawn({ - cmd: [process.execPath, "-e", - `import { ensurePickerCa } from ${JSON.stringify(PICKER_CA_MODULE_URL)};\n` + - `ensurePickerCa(${JSON.stringify(dir)}); setInterval(() => {}, 60000);`], + cmd: [process.execPath, "-e", "setInterval(() => {}, 60000);"], cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, stdout: "pipe", stderr: "pipe", }); try { - // Wait until the child's authority is published with its owner record. - const ownerPath = pickerCaOwnerPath(dir); - let childFingerprint = ""; - for (let i = 0; i < 400; i += 1) { - try { - const owner = JSON.parse(readFileSync(ownerPath, "utf8")) as { pid?: number; sha256?: string }; - if (owner.pid === child.pid && typeof owner.sha256 === "string") { childFingerprint = owner.sha256; break; } - } catch { /* owner file not written yet */ } - await Bun.sleep(10); - } - expect(childFingerprint).not.toBe(""); - expect(childFingerprint).not.toBe(ours.fingerprint); - // A live foreign process owns the published certificate: this process must not republish its - // previously trusted authority over it. - ensurePickerCa(dir); - expect(pickerCaFingerprints(readFileSync(pickerCaCertPath(dir), "utf8")).sha256).toBe(childFingerprint); + // Simulate a foreign owner's matching publication; the new fresh-owner guard prevents a + // second ensurePickerCa process from creating this state while ours is alive. + const foreign = createCertificateAuthority({ commonName: PICKER_CA_COMMON_NAME, permittedDnsNames: [PICKER_HOST] }); + const foreignFingerprint = pickerCaFingerprints(foreign.certPem).sha256; + writeFileSync(pickerCaCertPath(dir), foreign.certPem); + writeFileSync(pickerCaOwnerPath(dir), JSON.stringify({ pid: child.pid, sha256: foreignFingerprint })); + expect(() => ensurePickerCa(dir)).toThrow("picker_ca_live_owner"); + expect(readFileSync(pickerCaCertPath(dir), "utf8")).toBe(foreign.certPem); child.kill(); await child.exited; - // Once the owner is gone the file is stale again and this process reclaims it. - ensurePickerCa(dir); + // A startup may reclaim after the owner exits, but retains its outgoing public root. + ensurePickerCa(dir, { rotation: "startup" }); expect(readFileSync(pickerCaCertPath(dir), "utf8")).toBe(ours.certPem); + expect(readPendingPickerCaUntrust(dir)?.sha256).toBe(foreignFingerprint); + } finally { + child.kill(); + } +}); + +test("a held picker CA lock never permits publication outside the critical section", async () => { + const dir = tempDir(); + const held = join(dir, "lock-held"); + const release = join(dir, "lock-release"); + const lockPath = join(pickerStateDir(dir), "ca.lock.sqlite"); + const child = Bun.spawn({ + cmd: [process.execPath, "-e", + `import { existsSync, writeFileSync } from "node:fs";\n` + + `import { withClientLifecycleSync } from ${JSON.stringify(LIFECYCLE_LOCK_MODULE_URL)};\n` + + `withClientLifecycleSync(() => {\n` + + ` writeFileSync(${JSON.stringify(held)}, "held");\n` + + ` const cell = new Int32Array(new SharedArrayBuffer(4));\n` + + ` const until = Date.now() + 5000;\n` + + ` while (!existsSync(${JSON.stringify(release)}) && Date.now() < until) Atomics.wait(cell, 0, 0, 20);\n` + + `}, { lockPath: ${JSON.stringify(lockPath)} });`], + cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, + stdout: "pipe", + stderr: "pipe", + }); + try { + await waitForFile(held); + expect(() => ensurePickerCa(dir)).toThrow("client_lifecycle_busy"); + expect(existsSync(pickerCaCertPath(dir))).toBe(false); + expect(existsSync(pickerCaOwnerPath(dir))).toBe(false); } finally { + writeFileSync(release, "release"); + await child.exited; child.kill(); } + expect(ensurePickerCa(dir).fingerprint).toBe(pickerCaFingerprints(readFileSync(pickerCaCertPath(dir), "utf8")).sha256); +}); + +test("two competing processes leave one matching public certificate and owner", async () => { + const dir = tempDir(); + const start = join(dir, "start"); + const release = join(dir, "release"); + const workers = [0, 1].map(index => { + const result = join(dir, `result-${index}.json`); + const child = Bun.spawn({ + cmd: [process.execPath, "-e", + `import { existsSync, writeFileSync } from "node:fs";\n` + + `import { ensurePickerCa } from ${JSON.stringify(PICKER_CA_MODULE_URL)};\n` + + `while (!existsSync(${JSON.stringify(start)})) await Bun.sleep(5);\n` + + `try {\n` + + ` const ca = ensurePickerCa(${JSON.stringify(dir)}, { rotation: "startup" });\n` + + ` writeFileSync(${JSON.stringify(result)}, JSON.stringify({ ok: true, sha256: ca.fingerprint, pid: process.pid }));\n` + + ` while (!existsSync(${JSON.stringify(release)})) await Bun.sleep(5);\n` + + `} catch (error) {\n` + + ` writeFileSync(${JSON.stringify(result)}, JSON.stringify({ ok: false, message: String(error) }));\n` + + `}`], + cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, + stdout: "pipe", + stderr: "pipe", + }); + return { child, result }; + }); + try { + writeFileSync(start, "start"); + for (const worker of workers) await waitForFile(worker.result); + const results = workers.map(worker => JSON.parse(readFileSync(worker.result, "utf8")) as { + ok: boolean; sha256?: string; pid?: number; + }); + expect(results.filter(result => result.ok)).toHaveLength(1); + const winner = results.find(result => result.ok)!; + const owner = JSON.parse(readFileSync(pickerCaOwnerPath(dir), "utf8")) as { + pid: number; startTime: string | null; sha256: string; + }; + expect(owner).toMatchObject({ pid: winner.pid, sha256: winner.sha256 }); + if (process.platform === "darwin" || process.platform === "linux") expect(owner.startTime).toBeTruthy(); + expect(pickerCaFingerprints(readFileSync(pickerCaCertPath(dir), "utf8")).sha256).toBe(owner.sha256); + // Contention alone can make the loser fail. A later process must independently see and + // refuse the live published owner, rather than relying on the racing failure. + const contender = Bun.spawnSync({ + cmd: [process.execPath, "-e", + `import { ensurePickerCa } from ${JSON.stringify(PICKER_CA_MODULE_URL)};\n` + + `try { ensurePickerCa(${JSON.stringify(dir)}, { rotation: "startup" }); process.stdout.write("unexpected success"); }\n` + + `catch (error) { process.stdout.write(String(error)); }`], + cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, + stdout: "pipe", + stderr: "pipe", + }); + expect(contender.exitCode).toBe(0); + expect(contender.stdout.toString()).toContain("picker_ca_live_owner"); + expect(pickerCaFingerprints(readFileSync(pickerCaCertPath(dir), "utf8")).sha256).toBe(owner.sha256); + } finally { + writeFileSync(release, "release"); + for (const worker of workers) { + await worker.child.exited; + worker.child.kill(); + } + } +}); + +test("pending untrust contains one canonical public PEM and clears only on an exact acknowledgement", () => { + const dir = tempDir(); + const ours = ensurePickerCa(dir); + const old = createCertificateAuthority({ commonName: PICKER_CA_COMMON_NAME, permittedDnsNames: [PICKER_HOST] }); + writeFileSync(pickerCaCertPath(dir), old.certPem); + expect(ensurePickerCa(dir, { rotation: "startup" }).fingerprint).toBe(ours.fingerprint); + const pending = readPendingPickerCaUntrust(dir)!; + expect(Object.keys(JSON.parse(readFileSync(pickerCaPendingUntrustPath(dir), "utf8"))).sort()) + .toEqual(["certPem", "sha1", "sha256"]); + expect(pending).toEqual({ certPem: old.certPem, ...pickerCaFingerprints(old.certPem) }); + expect(readFileSync(pickerCaPendingUntrustPath(dir), "utf8")).not.toContain("PRIVATE KEY"); + expect(() => ensurePickerCa(dir)).toThrow("picker_ca_pending_untrust"); + expect(acknowledgePendingPickerCaUntrust(dir, { ...pending, sha1: "0".repeat(40) })).toBe(false); + expect(readPendingPickerCaUntrust(dir)).toEqual(pending); + expect(acknowledgePendingPickerCaUntrust(dir, pending)).toBe(true); + expect(readPendingPickerCaUntrust(dir)).toBeNull(); + expect(ensurePickerCa(dir).fingerprint).toBe(ours.fingerprint); +}); + +test("a failed certificate replacement retains the public predecessor record and published PEM", () => { + const dir = tempDir(); + const first = Bun.spawnSync({ + cmd: [process.execPath, "-e", + `import { ensurePickerCa } from ${JSON.stringify(PICKER_CA_MODULE_URL)};\n` + + `ensurePickerCa(${JSON.stringify(dir)});`], + cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, + stdout: "pipe", + stderr: "pipe", + }); + expect(first.exitCode).toBe(0); + const priorPem = readFileSync(pickerCaCertPath(dir), "utf8"); + const failed = Bun.spawnSync({ + cmd: [process.execPath, "-e", + `import { spyOn } from "bun:test"; import * as fs from "node:fs";\n` + + `const rename = fs.renameSync;\n` + + `spyOn(fs, "renameSync").mockImplementation((from, to) => {\n` + + ` if (to === ${JSON.stringify(pickerCaCertPath(dir))}) throw new Error("injected CA rename failure");\n` + + ` return rename(from, to);\n` + + `});\n` + + `const { ensurePickerCa } = await import(${JSON.stringify(PICKER_CA_MODULE_URL)});\n` + + `try { ensurePickerCa(${JSON.stringify(dir)}, { rotation: "startup" }); process.stdout.write("unexpected success"); }\n` + + `catch (error) { process.stdout.write(String(error)); }`], + cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, + stdout: "pipe", + stderr: "pipe", + }); + expect(failed.exitCode).toBe(0); + expect(failed.stdout.toString()).toContain("injected CA rename failure"); + expect(readFileSync(pickerCaCertPath(dir), "utf8")).toBe(priorPem); + expect(readPendingPickerCaUntrust(dir)).toEqual({ certPem: priorPem, ...pickerCaFingerprints(priorPem) }); + expect(pendingPickerCaHasLivePublishedOwner(dir, readPendingPickerCaUntrust(dir)!)).toBe(false); +}); + +test("a malformed pending record blocks publication and an actively published pending root is deferred", () => { + const invalidDir = tempDir(); + mkdirSync(pickerStateDir(invalidDir), { recursive: true }); + writeFileSync(pickerCaPendingUntrustPath(invalidDir), "{malformed"); + expect(() => ensurePickerCa(invalidDir, { rotation: "startup" })).toThrow("picker_ca_pending_untrust_invalid"); + expect(existsSync(pickerCaCertPath(invalidDir))).toBe(false); + + const dir = tempDir(); + const ca = ensurePickerCa(dir); + const pending = { certPem: ca.certPem, ...pickerCaFingerprints(ca.certPem) }; + writeFileSync(pickerCaPendingUntrustPath(dir), JSON.stringify(pending)); + expect(pendingPickerCaHasLivePublishedOwner(dir, pending)).toBe(true); + const owner = JSON.parse(readFileSync(pickerCaOwnerPath(dir), "utf8")) as Record; + writeFileSync(pickerCaOwnerPath(dir), JSON.stringify({ ...owner, startTime: "recycled-pid" })); + if (process.platform === "darwin" || process.platform === "linux") { + expect(pendingPickerCaHasLivePublishedOwner(dir, pending)).toBe(false); + } + expect(() => ensurePickerCa(dir, { rotation: "startup" })).toThrow("picker_ca_pending_untrust"); + expect(readPendingPickerCaUntrust(dir)).toEqual(pending); }); From 87bd878a0d8dc916afc5e25a7a2d731d60428e78 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 28 Sep 2026 00:29:33 +0900 Subject: [PATCH 3/6] WIP: record picker CA lane handoff --- devlog/_plan/260927_release_train_4/picker-ca/_handoff.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 devlog/_plan/260927_release_train_4/picker-ca/_handoff.md diff --git a/devlog/_plan/260927_release_train_4/picker-ca/_handoff.md b/devlog/_plan/260927_release_train_4/picker-ca/_handoff.md new file mode 100644 index 00000000000..b0a46f998d0 --- /dev/null +++ b/devlog/_plan/260927_release_train_4/picker-ca/_handoff.md @@ -0,0 +1,7 @@ +# Picker CA lane handoff (2026-09-28) + +- **Branch/worktree:** `codex/t4-picker-ca-release-blocker` at `/Users/jun/.codex/worktrees/t4-picker-ca/opencodex`. Local HEAD before this handoff commit: `48d6dd7eed` (WIP CA implementation); docs-first roadmap: `c087a9d297`. Base `origin/dev` was `24b2f39b77` at last check. **No push, PR, issue/PR comment, closure, or merge.** +- **Loop:** goal remains active; CodexClaw FSM is at **C for wp1** with source bound to this worktree. The 000–040 roadmap is under this folder. The docs-first wp0 cycle closed; wp1 A audit passed. The initial wp1 code review found three items (predictable temp file, PID reuse, and a weak live-owner test); fixes are in the WIP commit. The last interdiff review was closed at the stop request before its verdict was read, so treat wp1 review as unresolved. +- **Implemented locally:** `picker-ca.ts` now separates lock acquisition from a void callback, never publishes unlocked, records one public PEM/fingerprint pending-untrust item before replacement, gates ordinary calls while cleanup is pending, checks live owner with OS start identity where available, and acknowledges an exact item under lock. Tests cover one publication, lock contention, two processes, public-only record, failed rename, malformed record, PID mismatch, and exact acknowledgement. +- **Observed checks:** isolated `bun test tests/claude-integration/claude-picker-ca.test.ts` 14/14; `bun run typecheck` exit 0; `bun run privacy:scan` exit 0; staged `gitleaks git --staged --redact` exit 0. A red pre-fix test observed two `ca.pem` publications; the fixed test observed one. No real keychain, Desktop library, service, or local proxy was touched. `test:changed` and full suite have **not** run. +- **Next:** inspect the latest WIP diff and independent review; finish wp1 C/D with truthful receipts. Implement wp2 in `runtime.ts`: drain pending public CA before/after rotation, defer untrust for a matching live owner, keep an applied profile's actual egress URL serving blind CONNECT on failure, and test actual process replacement plus row/retry intent. Then wp3 B10 guarded drift heal, wp4 structure/invariant/docs and exact-head PR/CI/dev merge. Per coordinator, run `test:changed`/full from a same-commit `/private/tmp/t4-picker-ca-verify` checkout, with isolated `HOME`, `OPENCODEX_HOME`, `CODEX_HOME`, and `TMPDIR`; do not bypass test cleanup guards. From 844168b9795d1e6176e8d43c30e708b3096eab22 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 28 Sep 2026 00:53:46 +0900 Subject: [PATCH 4/6] fix(claude): keep Desktop egress alive and retry picker CA untrust across restarts Serialize picker CA publication under the CA lock with a tagged lock result, journal the outgoing public certificate before rotation, drain it before and after startup rotation with acknowledgement only after a confirmed untrust, and serve an applied Desktop profile's egress port with a blind-only CONNECT relay while cleanup is incomplete. Route the Codex config drift heal through the injector directly with the tick's 1-second lock wait and a generation/config write guard. --- .../picker-ca/010_ca_publication.md | 9 + .../picker-ca/020_desktop_continuity.md | 8 + .../picker-ca/030_codex_drift_heal.md | 15 ++ .../picker-ca/_handoff.md | 7 +- .../src/content/docs/guides/claude-code.md | 8 + scripts/test-layout/layout.json | 1 + src/claude/intercept/picker-ca-cleanup.ts | 31 +++ src/claude/intercept/picker-ca.ts | 30 ++- src/claude/intercept/runtime.ts | 73 +++--- src/codex/catalog-auto-refresh.ts | 104 ++++++-- structure/clients/claude-desktop.md | 22 +- structure/config.md | 4 +- structure/overview.md | 12 + .../claude-picker-ca.test.ts | 67 ++++- .../claude-picker-recovery.test.ts | 136 ++++++++++ .../claude-picker-runtime.test.ts | 118 ++++++++- .../catalog-auto-refresh-scheduler.test.ts | 247 ++++++++++++++++-- tests/fixtures/test-layout-expected.json | 1 + 18 files changed, 802 insertions(+), 91 deletions(-) create mode 100644 src/claude/intercept/picker-ca-cleanup.ts create mode 100644 tests/claude-integration/claude-picker-recovery.test.ts diff --git a/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md index e7c56ecfd80..222a3461434 100644 --- a/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md +++ b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md @@ -30,3 +30,12 @@ wp1 P stale check: `origin/dev` remains `24b2f39b77` and `picker-ca.ts` is uncha ## Proof before closing this phase Run isolated `bun test tests/claude-integration/claude-picker-ca.test.ts`, `bun run typecheck`, and the relevant source-as-data/process tests explicitly. Inspect the final CA/owner pair and journal bytes, and verify no `ca.key` or signing key appears anywhere under the fixture state root. Record exact commands and outcomes in the phase D note. + +## Results (2026-09-28) + +Implemented as planned in `picker-ca.ts`, with startup draining in the new `picker-ca-cleanup.ts`. Independent `gpt-6-sol` assessments of the committed WIP found two further gaps that are now fixed: a cached ensure did not restore a missing `ca-owner.json` for this process's own published CA (now rewritten under the lock), and pending acknowledgement did not require a confirmed untrust result (it now takes the `untrustPickerCa` result and returns false unless `ok`; a failed removal leaves the record byte-identical). Temporary public files are created inside their cleanup `try`, so a partial write leaves nothing behind. + +**Owner record absent while the owner is alive (accepted limitation).** The second assessment showed that if `ca-owner.json` is deleted while its process is live, a second process may rotate that CA, and a pending record matching the still-published PEM is then drained. No code path deletes the owner record, and `publishAuthority` writes the PEM before the owner, so a crash in between leaves a dead owner, for which rotation is correct. The state is therefore reachable only when another same-user process edits `/claude-picker/`, which AGENTS.md already places outside what an in-process check can prevent. The suggested strict refusal ("no verifiable owner means defer") was rejected because every upgrade from the current `main` release starts with a published `ca.pem` and no owner record: startup rotation would be refused permanently, and after an interrupted publication the pending drain would defer forever, leaving the picker off for good. Running two proxies on one `OPENCODEX_HOME` is separately refused by the start lifecycle. + +Evidence: isolated focused runs from the leaves — `claude-picker-ca`, `claude-picker-runtime`, `claude-picker-recovery`, test-layout and file-size ratchet tests: 129 pass / 0 fail; `bun run typecheck` and `bun run privacy:scan` passed; the assessor observed pending-file mode `0600` and refusal of a symlinked record and found no new private-key write path. + diff --git a/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md b/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md index f7d8639789f..6eeaa4f83e5 100644 --- a/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md +++ b/devlog/_plan/260927_release_train_4/picker-ca/020_desktop_continuity.md @@ -23,6 +23,14 @@ Depends on: `010_ca_publication.md`. Work phase `wp2`, class C4. Only the picker The chosen blind relay leaves Claude Code traffic from Desktop on its normal upstream route during recovery; it never selects the main intercept listener on this egress port. The separate main intercept port retains its existing policy. This is deliberately narrower than enabling picker MITM while a predecessor may remain trusted. +## Results + +Startup now drains the public pending-untrust journal before rotation and again after rotation. Each removal uses a private temporary copy of the recorded public PEM; a failed removal, live published owner, corrupt record, or failed acknowledgement keeps the picker disarmed. An applied profile then receives a blind-only CONNECT relay on its validated, recorded egress port. Bind refusal leaves the row and retry record intact. Normal startup constructs the picker only after both cleanup passes finish. The wp1 follow-up repairs a missing owner record under the CA lock on a cached ensure, and acknowledgement now requires a successful untrust result. + +**R1 design choice.** Keep the applied row and bind a blind relay at its actual port. Restoring the original profile after an ownership check would change Desktop's selected row and its previous-selection history during a temporary CA cleanup failure. The relay preserves that durable choice and gives Desktop an opaque upstream path until a later startup can safely arm the picker. + +Verification: isolated `bun test tests/claude-integration/*picker*.test.ts tests/test-layout*.test.ts tests/ci-workflows/file-size-ratchet.test.ts` passed 129/129; isolated `bun run typecheck` and `bun run privacy:scan` passed. The recovery test uses separate Bun processes and fake keychain runners; the runtime test sends a real CONNECT through the profile URL and verifies bytes through a fake upstream. No live keychain or Desktop installation was touched. + ## Proof before closing this phase Run the affected picker runtime/recovery files under isolated `HOME`, `OPENCODEX_HOME`, `TMPDIR`, plus typecheck. Read the actual CONNECT response and journal state, not merely a mocked `startProxy` call. Do not invoke the user's keychain or installed Desktop. diff --git a/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md b/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md index 5c39f0b4fe7..83c49878416 100644 --- a/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md +++ b/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md @@ -27,3 +27,18 @@ No new persisted field or enum is introduced; creation, serialization, deseriali ## Proof before closing this phase Run isolated `bun test tests/codex-integration/catalog-auto-refresh-scheduler.test.ts`, relevant direct-inject/cancellation regressions, and `bun run typecheck`. Document whether any full-sync call remains reachable from the drift branch. Keep any broader stale-model race found in the catalog-only path out of this lane's source edits and report it separately. + +## Results + +The defect was present. The drift branch passed a captured config to `syncModelsToCodex` without a cancellation or lock-wait option (`src/codex/catalog-auto-refresh.ts`, formerly lines 58–77). Full sync can await provider discovery and write catalog/cache before injection (`src/codex/sync.ts:245–283`), while its injector calls carried neither guard nor tick deadline. A stop/restart or persisted settings edit during that await could therefore publish stale work. The plan's direct-inject approach remains the smallest correction within this lane: the drift branch now has no full-sync call (`src/codex/catalog-auto-refresh.ts:115–150`). The separate catalog-only convergence path remains unchanged and is outside this phase. + +The repair selects the last journaled catalog path through a bounded read-only version-1 journal read, validates a regular catalog JSON file, and falls back to a separately validated default or `null` (`src/codex/catalog-auto-refresh.ts:51–113`). It calls `injectCodexConfig` with `lockTimeoutMs: 1000` and a synchronous generation-plus-persisted-config guard (`:126–149`). The injector evaluates that guard at its commit boundary (`src/codex/inject.ts:574–584`). The tick suppresses stale-generation reporting, and a successful heal is reported only after the missing root keys are observed on disk (`src/codex/catalog-auto-refresh.ts:149–150,205–211`). This deadline bounds lock acquisition, not all injection preparation, as the plan already specified. + +Verification used a fresh isolated `HOME`, `OPENCODEX_HOME`, `CODEX_HOME`, and `TMPDIR` for each Bun command: + +- `bun test tests/codex-integration/catalog-auto-refresh-scheduler.test.ts`: 16 pass, 0 fail on the final run. The tests cover stopped/restarted generations, changed ON/OFF settings during a deferred injector, the 1000 ms option, a child-process check that the injector receives the journaled non-default catalog, invalid journal byte preservation, and on-disk heal observation. The direct-inject assertions and full-sync refusal would fail against the old branch. An intermediate run had 15 pass / 1 fail because the test expected `/var` while the Codex-home resolver canonicalized the macOS temp path to `/private/var`; the assertion now compares canonical paths. +- `bun test tests/codex-integration/catalog-auto-refresh-scheduler.test.ts tests/codex-integration/codex-config-drift-heal.test.ts tests/codex-integration/codex-inject-write-lock.test.ts tests/codex-integration/codex-sync-api.test.ts tests/codex-integration/codex-sync-response.test.ts tests/codex-integration/client-injection-guard.test.ts`: 75 pass, 0 fail. This run preceded the final on-disk assertion refinement; the focused suite was rerun afterward. +- `bun test tests/codex-integration/codex-inject.test.ts tests/codex-integration/codex-inject-integration.test.ts`: 160 pass, 0 fail. +- `bun run typecheck`: exit 0, rerun after the final test edit. + +The source-ownership map lists `structure/config.md` for `src/codex/` (`structure/INDEX.md:121`), and its scheduler paragraph at `structure/config.md:584` should be updated by the coordinator; that file is outside this lane's write scope. diff --git a/devlog/_plan/260927_release_train_4/picker-ca/_handoff.md b/devlog/_plan/260927_release_train_4/picker-ca/_handoff.md index b0a46f998d0..c8d8c0d0e6a 100644 --- a/devlog/_plan/260927_release_train_4/picker-ca/_handoff.md +++ b/devlog/_plan/260927_release_train_4/picker-ca/_handoff.md @@ -1,7 +1,4 @@ # Picker CA lane handoff (2026-09-28) -- **Branch/worktree:** `codex/t4-picker-ca-release-blocker` at `/Users/jun/.codex/worktrees/t4-picker-ca/opencodex`. Local HEAD before this handoff commit: `48d6dd7eed` (WIP CA implementation); docs-first roadmap: `c087a9d297`. Base `origin/dev` was `24b2f39b77` at last check. **No push, PR, issue/PR comment, closure, or merge.** -- **Loop:** goal remains active; CodexClaw FSM is at **C for wp1** with source bound to this worktree. The 000–040 roadmap is under this folder. The docs-first wp0 cycle closed; wp1 A audit passed. The initial wp1 code review found three items (predictable temp file, PID reuse, and a weak live-owner test); fixes are in the WIP commit. The last interdiff review was closed at the stop request before its verdict was read, so treat wp1 review as unresolved. -- **Implemented locally:** `picker-ca.ts` now separates lock acquisition from a void callback, never publishes unlocked, records one public PEM/fingerprint pending-untrust item before replacement, gates ordinary calls while cleanup is pending, checks live owner with OS start identity where available, and acknowledges an exact item under lock. Tests cover one publication, lock contention, two processes, public-only record, failed rename, malformed record, PID mismatch, and exact acknowledgement. -- **Observed checks:** isolated `bun test tests/claude-integration/claude-picker-ca.test.ts` 14/14; `bun run typecheck` exit 0; `bun run privacy:scan` exit 0; staged `gitleaks git --staged --redact` exit 0. A red pre-fix test observed two `ca.pem` publications; the fixed test observed one. No real keychain, Desktop library, service, or local proxy was touched. `test:changed` and full suite have **not** run. -- **Next:** inspect the latest WIP diff and independent review; finish wp1 C/D with truthful receipts. Implement wp2 in `runtime.ts`: drain pending public CA before/after rotation, defer untrust for a matching live owner, keep an applied profile's actual egress URL serving blind CONNECT on failure, and test actual process replacement plus row/retry intent. Then wp3 B10 guarded drift heal, wp4 structure/invariant/docs and exact-head PR/CI/dev merge. Per coordinator, run `test:changed`/full from a same-commit `/private/tmp/t4-picker-ca-verify` checkout, with isolated `HOME`, `OPENCODEX_HOME`, `CODEX_HOME`, and `TMPDIR`; do not bypass test cleanup guards. +- Branch/worktree: `codex/t4-picker-ca-release-blocker` at `/Users/jun/.codex/worktrees/t4-picker-ca/opencodex`, based on `origin/dev` `24b2f39b77`. +- wp1 (010), wp2 (020), wp3 (030) implemented with Results sections; structure docs, INV-PICKER-01/02 and the Claude Code guide updated (040). Remaining: exact-head PR CI, merge into `dev`, dev CI dispatch. diff --git a/docs-site/src/content/docs/guides/claude-code.md b/docs-site/src/content/docs/guides/claude-code.md index 9d369e28ac8..7056426f463 100644 --- a/docs-site/src/content/docs/guides/claude-code.md +++ b/docs-site/src/content/docs/guides/claude-code.md @@ -227,6 +227,14 @@ and its subdomains. Its signing key exists only inside the running OpenCodex pro OpenCodex restart publishes a fresh authority and macOS asks you to trust it again — approve the prompt, or later run `ocx claude desktop picker trust`, after each restart. +On restart OpenCodex first removes the previous authority from the keychain. If that removal fails +(for example because you decline the keychain prompt), the picker stays off for this run so two +authorities are never trusted side by side. Desktop keeps its network connection: the proxy address +in its profile still answers, but only as a plain relay that does not read claude.ai traffic, and the +picker lists Anthropic's own models until the removal succeeds. OpenCodex remembers which certificate +still needs removal and retries on the next restart; `ocx claude desktop picker status` shows the +picker as unavailable meanwhile. + While picker mode is on, Claude Desktop reaches the network through OpenCodex. If OpenCodex stops, Desktop is offline until you fully restart it or turn picker mode off. Check the state with `ocx claude desktop picker status`; use `ocx claude desktop picker trust` to repeat the trust step, diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 4bae502f37f..6fef66e5e07 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -468,6 +468,7 @@ "claude-picker-ca.test.ts": "claude-integration", "claude-picker-listener.test.ts": "claude-integration", "claude-picker-models.test.ts": "claude-integration", + "claude-picker-recovery.test.ts": "claude-integration", "claude-picker-runtime.test.ts": "claude-integration", "claude-picker-trust.test.ts": "claude-integration", "claude-desktop-remote-hub.test.ts": "claude-integration", diff --git a/src/claude/intercept/picker-ca-cleanup.ts b/src/claude/intercept/picker-ca-cleanup.ts new file mode 100644 index 00000000000..f2b01cc7134 --- /dev/null +++ b/src/claude/intercept/picker-ca-cleanup.ts @@ -0,0 +1,31 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + acknowledgePendingPickerCaUntrust, + pendingPickerCaHasLivePublishedOwner, + readPendingPickerCaUntrust, +} from "./picker-ca"; +import { untrustPickerCa, type SecurityRunner } from "./picker-trust"; + +/** Drain exactly the recorded public predecessor; keep its journal on any uncertainty. */ +export async function drainPendingPickerCaUntrust( + configDir: string, + security?: SecurityRunner, + platform?: NodeJS.Platform, +): Promise { + const pending = readPendingPickerCaUntrust(configDir); + if (!pending) return true; + if (pendingPickerCaHasLivePublishedOwner(configDir, pending)) return false; + + const privateDir = mkdtempSync(join(tmpdir(), "ocx-picker-untrust-")); + try { + const publicCopy = join(privateDir, "ca.pem"); + writeFileSync(publicCopy, pending.certPem, { mode: 0o600 }); + const result = await untrustPickerCa(publicCopy, pending.sha1, security, platform); + if (!result.ok) return false; + return acknowledgePendingPickerCaUntrust(configDir, pending, result); + } finally { + rmSync(privateDir, { recursive: true, force: true }); + } +} diff --git a/src/claude/intercept/picker-ca.ts b/src/claude/intercept/picker-ca.ts index a14241b734c..56ed9d87a8c 100644 --- a/src/claude/intercept/picker-ca.ts +++ b/src/claude/intercept/picker-ca.ts @@ -38,8 +38,8 @@ export function pickerCaFingerprints(certPem: string): { sha1: string; sha256: s /** Atomically publish a public certificate; these files carry no key material. */ function publishPem(path: string, pem: string): void { const tmp = `${path}.${process.pid}.${randomUUID()}.tmp`; - writeFileSync(tmp, pem, { flag: "wx", mode: 0o644 }); try { + writeFileSync(tmp, pem, { flag: "wx", mode: 0o644 }); try { chmodSync(tmp, 0o644); } catch { /* best-effort on platforms without POSIX modes */ } renameSync(tmp, path); } finally { @@ -54,6 +54,10 @@ function publishPem(path: string, pem: string): void { */ function publishAuthority(configDir: string, ca: PickerCa): void { publishPem(pickerCaCertPath(configDir), ca.certPem); + publishOwner(configDir, ca); +} + +function publishOwner(configDir: string, ca: PickerCa): void { publishPem(pickerCaOwnerPath(configDir), JSON.stringify({ pid: process.pid, startTime: processStartIdentity(process.pid), @@ -61,6 +65,14 @@ function publishAuthority(configDir: string, ca: PickerCa): void { }) + "\n"); } +function currentProcessOwnsPublishedCa(configDir: string, ca: PickerCa): boolean { + try { + const owner = JSON.parse(readFileSync(pickerCaOwnerPath(configDir), "utf8")) as Record; + return owner.pid === process.pid && owner.sha256 === ca.fingerprint + && owner.startTime === processStartIdentity(process.pid); + } catch { return false; } +} + /** The OS process start identity prevents a recycled PID from impersonating the recorded owner. */ function processStartIdentity(pid: number): string | null { if (process.platform === "linux") { @@ -164,8 +176,8 @@ export function readPendingPickerCaUntrust(configDir: string): PendingPickerCaUn function writePendingPickerCaUntrust(configDir: string, pending: PendingPickerCaUntrust): void { const path = pickerCaPendingUntrustPath(configDir); const temporary = `${path}.${process.pid}.${randomUUID()}.tmp`; - writeFileSync(temporary, JSON.stringify(pending) + "\n", { flag: "wx", mode: 0o600 }); try { + writeFileSync(temporary, JSON.stringify(pending) + "\n", { flag: "wx", mode: 0o600 }); try { chmodSync(temporary, 0o600); } catch { /* best-effort on platforms without POSIX modes */ } renameSync(temporary, path); } finally { @@ -222,7 +234,12 @@ export function pendingPickerCaHasLivePublishedOwner(configDir: string, pending: } /** Acknowledgement never clears an entry another process created or replaced. */ -export function acknowledgePendingPickerCaUntrust(configDir: string, pending: PendingPickerCaUntrust): boolean { +export function acknowledgePendingPickerCaUntrust( + configDir: string, + pending: PendingPickerCaUntrust, + confirmedUntrust: { ok: boolean }, +): boolean { + if (confirmedUntrust?.ok !== true) return false; return lockedPickerCa(configDir, () => { const current = readPendingPickerCaUntrust(configDir); if (!current || current.certPem !== pending.certPem @@ -256,7 +273,12 @@ export function ensurePickerCa(configDir: string, options: { rotation?: "startup lockedPickerCa(configDir, () => { if (readPendingPickerCaUntrust(configDir)) throw new Error("picker_ca_pending_untrust"); const published = publishedPickerCa(configDir); - if (published === pickerCa.certPem) return; + if (published === pickerCa.certPem) { + // The signing key is still in this process. A missing/stale owner file must not let a + // second process rotate this live authority after an otherwise harmless cached ensure. + if (!currentProcessOwnsPublishedCa(configDir, pickerCa)) publishOwner(configDir, pickerCa); + return; + } if (published !== null && livePublishedOwner(configDir, published)) { throw new Error("picker_ca_live_owner"); } diff --git a/src/claude/intercept/runtime.ts b/src/claude/intercept/runtime.ts index 346b31ef123..5f59242de56 100644 --- a/src/claude/intercept/runtime.ts +++ b/src/claude/intercept/runtime.ts @@ -1,7 +1,4 @@ import type { Server } from "bun"; -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; import type { OcxConfig } from "../../types"; import { getConfigDir } from "../../config/paths"; import type { DesktopPickerController } from "../desktop-picker"; @@ -10,7 +7,8 @@ import { classifyInterceptClient, interceptRouteFor } from "./client-class"; import { CLAUDE_INTERCEPT_HOSTS, isBrowserConnect, startConnectProxy, type ConnectProxyHandle } from "./connect-proxy"; import { startClaudeInterceptListener } from "./listener"; import { claudeInterceptCaCertPath, ensureLocalInterceptCaForStartup, issueLocalInterceptLeaf } from "./local-ca"; -import { discardPickerCaKey, ensurePickerCa, pickerCaCertPath, pickerCaFingerprints } from "./picker-ca"; +import { discardPickerCaKey, ensurePickerCa } from "./picker-ca"; +import { drainPendingPickerCaUntrust } from "./picker-ca-cleanup"; import type { PickerRouteInput } from "./picker-models"; import { createPickerRuntime, type CreatePickerRuntimeOptions, type PickerRuntime } from "./picker-runtime"; import type { SecurityRunner } from "./picker-trust"; @@ -193,46 +191,39 @@ export async function startClaudeIntercept(options: StartClaudeInterceptOptio // it in place — row id and its recorded previous selection included — so the picker proxy // can fail to bind without losing it, and the restore enable below only updates the proxy // URL inside the same row rather than recreating a selection around a placeholder pivot. - const pickerProfileApplied = inspectDesktopPickerProfile({ + const pickerProfile = inspectDesktopPickerProfile({ configDir, ...(options.pickerPlatform ? { platform: options.pickerPlatform } : {}), - }).kind === "applied"; - const oldCaPath = pickerCaCertPath(configDir); + }); + const pickerProfileApplied = pickerProfile.kind === "applied"; let pickerBlocked = false; - if (existsSync(oldCaPath)) { - // Rotation cleanup drops the *replaced* authority's keychain trust. A reused process - // authority keeps its trust — removing it would revoke picker access mid-flight and force - // a redundant keychain prompt. When removal of a genuinely different predecessor fails, - // the picker must not arm at all: the outgoing signing key would otherwise stay trusted - // beside the new authority, and the already-bound main intercept pair keeps serving alone. - // Capture the published bytes before ensurePickerCa replaces them: the untrust step below - // must remove trust for the *outgoing* certificate, so it needs the old file contents. - let publishedPem: string | undefined; - let publishedSha1: string | undefined; - try { - publishedPem = readFileSync(oldCaPath, "utf8"); - publishedSha1 = pickerCaFingerprints(publishedPem).sha1; - } catch { /* unreadable or malformed: nothing identifiable to remove */ } - const nextSha1 = pickerCaFingerprints(ensurePickerCa(configDir).certPem).sha1; - if (publishedPem !== undefined && publishedSha1 !== undefined && publishedSha1 !== nextSha1) { - const { untrustPickerCa } = await import("./picker-trust"); + try { + // A prior process may have died after publishing the journal but before cleanup. Finish + // that entry before rotation can replace it, then drain the newly queued predecessor. + const drain = () => drainPendingPickerCaUntrust(configDir, options.pickerSecurity, options.pickerPlatform); + if (!await drain()) pickerBlocked = true; + if (!pickerBlocked) { + ensurePickerCa(configDir, { rotation: "startup" }); + if (!await drain()) pickerBlocked = true; + } + } catch (error) { + pickerBlocked = true; + console.warn(`⚠ Claude Desktop picker CA cleanup deferred: ${error instanceof Error ? error.message : String(error)}`); + } + if (pickerBlocked) { + console.warn("⚠ Claude Desktop picker disabled: the previous certificate could not be untrusted"); + if (pickerProfile.kind === "applied") { + // Keep Desktop's actual pinned egress alive without ever constructing a TLS terminator. + const port = Number(new URL(pickerProfile.proxyUrl).port); try { - // remove-trusted-cert takes the certificate file; ca.pem now holds the replacement, - // so untrust from a private copy of the bytes that were actually trusted. - const privateDir = mkdtempSync(join(tmpdir(), "ocx-picker-untrust-")); - try { - const outgoing = join(privateDir, "ca.pem"); - writeFileSync(outgoing, publishedPem, { mode: 0o600 }); - const dropped = await untrustPickerCa(outgoing, publishedSha1, options.pickerSecurity, options.pickerPlatform); - pickerBlocked = !dropped.ok; - } finally { - rmSync(privateDir, { recursive: true, force: true }); - } - } catch { - pickerBlocked = true; - } - if (pickerBlocked) { - console.warn("⚠ Claude Desktop picker disabled: the previous certificate could not be untrusted"); + pickerProxy = await startProxy(port, { + interceptPort: listener.port!, + interceptHosts: [], + selectTunnel: () => ({ kind: "blind" }), + }); + pickerProxyLive = true; + } catch (error) { + console.warn(`⚠ Claude Desktop blind egress relay could not start: ${error instanceof Error ? error.message : String(error)}`); } } } @@ -324,7 +315,7 @@ export async function startClaudeIntercept(options: StartClaudeInterceptOptio const state: ClaudeInterceptState = { proxyPort: proxy.port, caCertPath: claudeInterceptCaCertPath(configDir), - pickerProxyPort: picker && pickerProxy ? pickerProxy.port : null, + pickerProxyPort: pickerProxy?.port ?? null, }; activeState = state; activePicker = picker; diff --git a/src/codex/catalog-auto-refresh.ts b/src/codex/catalog-auto-refresh.ts index 894cdbde6e5..3c0f6af36eb 100644 --- a/src/codex/catalog-auto-refresh.ts +++ b/src/codex/catalog-auto-refresh.ts @@ -29,9 +29,12 @@ const MIN_INTERVAL_MS = 15 * 60_000; * callers fail fast and defer (ConvergeRequest.mode) rather than holding the * write lock across a slow tick. */ +import { closeSync, constants, fstatSync, lstatSync, openSync, readSync } from "node:fs"; import type { OcxConfig } from "../types"; const TICK_DEADLINE_MS = 1_000; +const MAX_JOURNAL_BYTES = 1024 * 1024; +const MAX_CATALOG_BYTES = 64 * 1024 * 1024; let timer: ReturnType | null = null; let detachShutdownHook: (() => void) | null = null; @@ -45,24 +48,84 @@ let generation = 0; /** setInterval does not skip a firing while the previous callback is still awaiting. */ let inFlight = false; -/** - * Re-run the standard Codex sync when the injected config surface drifted (src/codex/config- - * drift-heal.ts). The gate mirrors syncCodexOnStartIfEnabled: a hub must not rewrite its own - * client, and the user's Codex OFF decision outlives restarts. A failed heal is silent here — - * the next tick retries, and the sync's own callers report their refusals. - * - * "healed" means the keys that were missing are back on disk. A sync can succeed without - * writing them (for example when an external provider now owns config.toml and the injector - * stands down), so the outcome is read from the file, not from the sync result. - */ -async function healCodexConfigDrift(config: OcxConfig): Promise<"none" | "healed" | "not-healed"> { - const [{ codexConfigDrift }, { journaledInjectedOpenaiBaseUrl, journaledInjectedRealtimeWsBaseUrl }, { shouldSyncCodexOnStart }, { syncModelsToCodex }] = +/** Read only a regular file, with a byte limit even if it grows after the stat. */ +function readBoundedRegularFile(path: string, maxBytes: number): string | null { + let fd: number | undefined; + try { + const entry = lstatSync(path); + if (!entry.isFile() || entry.size > maxBytes) return null; + fd = openSync(path, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0)); + const opened = fstatSync(fd); + if (!opened.isFile() || opened.size > maxBytes) return null; + const bytes = Buffer.alloc(opened.size + 1); + let length = 0; + while (length < bytes.length) { + const read = readSync(fd, bytes, length, bytes.length - length, null); + if (read === 0) break; + length += read; + } + return length > maxBytes ? null : bytes.subarray(0, length).toString("utf8"); + } catch { + return null; + } finally { + if (fd !== undefined) closeSync(fd); + } +} + +/** A background observation must not use journaledInjectedCatalogPath(), which cleans invalid journals. */ +function readJournaledCatalogPath(journalPath: string): string | null { + const bytes = readBoundedRegularFile(journalPath, MAX_JOURNAL_BYTES); + if (bytes === null) return null; + try { + const journal: unknown = JSON.parse(bytes); + if (!journal || typeof journal !== "object" || Array.isArray(journal)) return null; + const record = journal as Record; + return record.version === 1 && typeof record.injectedCatalogPath === "string" && record.injectedCatalogPath.trim() + ? record.injectedCatalogPath + : null; + } catch { + return null; + } +} + +function usableCatalogPath(path: string | null): string | null { + if (!path) return null; + const bytes = readBoundedRegularFile(path, MAX_CATALOG_BYTES); + if (bytes === null) return null; + try { + const catalog: unknown = JSON.parse(bytes); + return catalog && typeof catalog === "object" && Array.isArray((catalog as { models?: unknown }).models) + ? path : null; + } catch { + return null; + } +} + +/** Testable path decision with no journal mutation or catalog write. */ +export function selectDriftHealCatalogPath( + journalPath: string, + defaultCatalogPath: string, + resolvePath: (path: string) => string, +): string | null { + const recorded = readJournaledCatalogPath(journalPath); + return usableCatalogPath(recorded ? resolvePath(recorded) : null) + ?? usableCatalogPath(defaultCatalogPath); +} + +/** Repair only the missing config roots; catalog convergence remains the later tick step. */ +async function healCodexConfigDrift(config: OcxConfig, entryGeneration: number): Promise<"none" | "healed" | "not-healed"> { + const [{ codexConfigDrift }, { JOURNAL_PATH, journaledInjectedOpenaiBaseUrl, journaledInjectedRealtimeWsBaseUrl }, { shouldSyncCodexOnStart }, { injectCodexConfig }, { DEFAULT_CATALOG_PATH, resolveCodexConfigPath }, { loadConfig }] = await Promise.all([ import("./config-drift-heal"), import("./journal"), import("./desired-state"), - import("./sync"), + import("./inject"), + import("./paths"), + import("../config"), ]); + const capturedSettings = JSON.stringify(config); + const current = () => entryGeneration === generation && JSON.stringify(loadConfig()) === capturedSettings; + if (!current()) return "none"; if (!shouldSyncCodexOnStart(config)) return "none"; const journaled = { injectedOpenaiBaseUrl: journaledInjectedOpenaiBaseUrl({ readOnly: true }), @@ -71,9 +134,19 @@ async function healCodexConfigDrift(config: OcxConfig): Promise<"none" | "healed const drift = codexConfigDrift(() => journaled); if (!drift.drifted) return "none"; const { readRuntimePort } = await import("../config/process-state"); + if (!current()) return "none"; const runtime = readRuntimePort(process.pid); if (!runtime) return "not-healed"; - await syncModelsToCodex(runtime.port, config, null).catch(() => null); + const catalogPath = selectDriftHealCatalogPath(JOURNAL_PATH, DEFAULT_CATALOG_PATH, resolveCodexConfigPath); + if (!current()) return "none"; + await injectCodexConfig(runtime.port, config, { + catalogPath, + lockTimeoutMs: TICK_DEADLINE_MS, + beforeClientWrite: () => { + if (!current()) throw new Error("Catalog drift heal tick is stale"); + }, + }).catch(() => null); + if (!current()) return "none"; return codexConfigDrift(() => journaled).drifted ? "not-healed" : "healed"; } @@ -128,7 +201,8 @@ async function tick(): Promise { // report "no change" while Codex serves its native model picker. Re-injecting through the // standard sync rewrites the keys, re-journals the baseline, and only runs when the // integration is on and this install is allowed to manage its local client. - const heal = await healCodexConfigDrift(config); + const heal = await healCodexConfigDrift(config, entryGeneration); + if (entryGeneration !== generation) return; if (heal === "healed") { console.info("[catalog-auto-refresh] injected Codex config keys were rewritten externally; re-injected"); } else if (heal === "not-healed") { diff --git a/structure/clients/claude-desktop.md b/structure/clients/claude-desktop.md index 82d4445f722..c687dcad750 100644 --- a/structure/clients/claude-desktop.md +++ b/structure/clients/claude-desktop.md @@ -189,7 +189,27 @@ skips host-scoped trust settings, so `inspectPickerTrust` treats a current CA wh trust settings carry `kSecTrustSettingsPolicyString` as untrusted and the trust step replaces it; an export it cannot read makes trust `unknown`, which never arms. A rotated-out picker certificate is removed from the login keychain as its replacement is published, and a failed removal stops the -picker arming. The relay verifies the upstream +picker arming. Publication of `ca.pem` and `ca-owner.json` happens only inside the +`ca.lock.sqlite` lock (`picker-ca.ts`): lock acquisition is reported separately from the +callback, so a busy lock publishes nothing, and a missing or mismatched owner record for our own +certificate is rewritten under the lock so a second process cannot rotate out a live owner's +authority. The owner record carries the OS process start identity where the platform exposes one, +so a reused PID does not count as the live owner. +Before a startup rotation replaces `ca.pem`, the outgoing certificate's **public** PEM and its +SHA-1/SHA-256 go to `pending-untrust.json` (mode 0600, no key material); only one such record may +exist, and a default `ensurePickerCa` call (the controller's enable/trust path) refuses while it +does. Startup (`runtime.ts` via `picker-ca-cleanup.ts`) drains that record before and after +rotation: it defers without calling `security` while the recorded certificate is still published by +a live owner, untrusts a private temporary copy of the public PEM otherwise, and acknowledges the +exact record only after a confirmed removal, so a failure survives process replacement and is +retried by the next start. While the drain is incomplete and a Desktop picker profile is applied, the +lifecycle binds a blind-only CONNECT relay (`interceptHosts: []`, every tunnel blind) on the +profile's recorded `egressProxyUrl` port instead of the picker: Desktop keeps its network path, no +TLS is terminated, no trust is added, and the profile row, its previous selection and the retry +intent stay untouched. A port held by another process is not taken over; the relay start fails with +a warning and the row stays for the next start. The relay is chosen over restoring the pre-picker +profile because a restore is an ownership-sensitive Desktop write that would discard the retry +intent and cannot repair the URL a running Desktop already pinned. The `claude.ai` relay verifies the upstream certificate, streams every body and upgrade unchanged, and rewrites only the bootstrap response's local Code picker surfaces, `ccd` (what the Desktop Code tab reads) and its `code` fallback, never the remote `ccr` (`picker-bootstrap.ts`), failing open to the original bytes; the model list diff --git a/structure/config.md b/structure/config.md index 1ae6fc51842..7e93c97e534 100644 --- a/structure/config.md +++ b/structure/config.md @@ -264,8 +264,8 @@ base-url-less table is the Codex desktop app's own native-routing placeholder (s each app-managed rewrite writes `model_provider = "custom"` plus such a table, stripping the injected root keys alongside), which re-injects cleanly because the injector strips a root `model_provider` line first. Recovery is drift detection (`src/codex/config-drift-heal.ts`): the -auto-refresh tick re-runs the standard sync when a root key the journal says was injected is -missing on disk (presence only; a present key with another value is left alone). +auto-refresh tick re-injects the config when a root key the journal says was injected is missing on disk (presence only; a present key with another value is left alone). +The heal calls the injector directly (no provider discovery or catalog write), waits at most the tick's 1-second commit-lock deadline, and its `beforeClientWrite` guard refuses the write once the timer generation changed or the persisted config no longer matches the tick's snapshot; the catalog path is a bounded read-only lookup of the journal's `injectedCatalogPath` (a regular, parseable catalog) falling back to the default or none, and "healed" is reported only after the keys are observed on disk. `ocx sync` and `ocx restore back` run the injector's non-writing preflight before provider discovery or catalog/cache replacement. Deterministic config and ownership refusals therefore diff --git a/structure/overview.md b/structure/overview.md index 6e6ef5d7c76..c31d9d628e9 100644 --- a/structure/overview.md +++ b/structure/overview.md @@ -210,6 +210,18 @@ still cover the rule, which is a judgement only review makes. there is none, no icon is claimed, the window is shown on launch whatever the launch origin, and closing it quits through the same drain; see [`desktop-shell.md`](desktop-shell.md). Enforced by `tests/clients/desktop-tray-availability.test.ts`. +- **INV-PICKER-01** — The Claude Desktop picker never terminates `claude.ai` TLS while a replaced + picker certificate may still be trusted. When predecessor untrust fails, is deferred, or its + record is unreadable, startup builds no picker and adds no trust; if a picker profile is applied, + a blind-only CONNECT relay serves the profile's recorded egress port so Desktop stays connected, + and the profile row, previous selection and retry intent are left in place; see + [`claude-desktop.md`](clients/claude-desktop.md). + Enforced by `tests/claude-integration/claude-picker-runtime.test.ts`. +- **INV-PICKER-02** — A startup rotation records the outgoing picker certificate's public PEM and + fingerprints (no key material) before replacing `ca.pem`, and removes that record only after a + confirmed keychain untrust, so a later process retries it; a controller enable refuses while it + exists; see [`claude-desktop.md`](clients/claude-desktop.md). + Enforced by `tests/claude-integration/claude-picker-recovery.test.ts`. CI enumerates that domain layout through `scripts/ci/run-bun-test-batches.sh`. Its default general scope and 12-file/120-second process shape leave the dedicated Linux storage-policy and api-usage diff --git a/tests/claude-integration/claude-picker-ca.test.ts b/tests/claude-integration/claude-picker-ca.test.ts index 6c12a7e543e..cba76e90093 100644 --- a/tests/claude-integration/claude-picker-ca.test.ts +++ b/tests/claude-integration/claude-picker-ca.test.ts @@ -6,6 +6,7 @@ import { join } from "node:path"; import { pathToFileURL } from "node:url"; import { connect, createServer } from "node:tls"; import { createCertificateAuthority, createLocalInterceptCa, issueServerLeaf } from "../../src/claude/intercept/local-ca"; +import { drainPendingPickerCaUntrust } from "../../src/claude/intercept/picker-ca-cleanup"; import { acknowledgePendingPickerCaUntrust, ensurePickerCa, issuePickerLeaf, pickerCaCertPath, pickerCaFingerprints, pickerCaOwnerPath, pickerCaPendingUntrustPath, pickerLeafCertPath, pickerStateDir, @@ -362,13 +363,75 @@ test("pending untrust contains one canonical public PEM and clears only on an ex expect(pending).toEqual({ certPem: old.certPem, ...pickerCaFingerprints(old.certPem) }); expect(readFileSync(pickerCaPendingUntrustPath(dir), "utf8")).not.toContain("PRIVATE KEY"); expect(() => ensurePickerCa(dir)).toThrow("picker_ca_pending_untrust"); - expect(acknowledgePendingPickerCaUntrust(dir, { ...pending, sha1: "0".repeat(40) })).toBe(false); + expect(acknowledgePendingPickerCaUntrust(dir, { ...pending, sha1: "0".repeat(40) }, { ok: true })).toBe(false); expect(readPendingPickerCaUntrust(dir)).toEqual(pending); - expect(acknowledgePendingPickerCaUntrust(dir, pending)).toBe(true); + expect(acknowledgePendingPickerCaUntrust(dir, pending, { ok: false })).toBe(false); + expect(readPendingPickerCaUntrust(dir)).toEqual(pending); + expect(acknowledgePendingPickerCaUntrust(dir, pending, { ok: true })).toBe(true); expect(readPendingPickerCaUntrust(dir)).toBeNull(); expect(ensurePickerCa(dir).fingerprint).toBe(ours.fingerprint); }); +test("failed untrust leaves the pending record byte-for-byte intact", async () => { + const dir = tempDir(); + ensurePickerCa(dir); + const old = createCertificateAuthority({ commonName: PICKER_CA_COMMON_NAME, permittedDnsNames: [PICKER_HOST] }); + writeFileSync(pickerCaCertPath(dir), old.certPem); + ensurePickerCa(dir, { rotation: "startup" }); + const path = pickerCaPendingUntrustPath(dir); + const before = readFileSync(path); + const sha1 = pickerCaFingerprints(old.certPem).sha1; + const calls: string[] = []; + const drained = await drainPendingPickerCaUntrust(dir, async args => { + calls.push(args[0]!); + return args[0] === "find-certificate" + ? { code: 0, stdout: `SHA-1 hash: ${sha1}\n`, stderr: "" } + : { code: 1, stdout: "", stderr: "" }; + }, "darwin"); + expect(drained).toBe(false); + expect(calls).toContain("remove-trusted-cert"); + expect(readFileSync(path)).toEqual(before); +}); + +test("cached ensure repairs a missing owner so a peer cannot rotate a live CA", async () => { + const dir = tempDir(); + const ready = join(dir, "ready"); + const release = join(dir, "release"); + const child = Bun.spawn({ + cmd: [process.execPath, "-e", + `import { existsSync, unlinkSync, writeFileSync } from "node:fs";\n` + + `import { ensurePickerCa, pickerCaOwnerPath } from ${JSON.stringify(PICKER_CA_MODULE_URL)};\n` + + `ensurePickerCa(${JSON.stringify(dir)});\n` + + `unlinkSync(pickerCaOwnerPath(${JSON.stringify(dir)}));\n` + + `ensurePickerCa(${JSON.stringify(dir)});\n` + + `writeFileSync(${JSON.stringify(ready)}, "ready");\n` + + `while (!existsSync(${JSON.stringify(release)})) await Bun.sleep(5);`], + cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, + stdout: "pipe", stderr: "pipe", + }); + try { + await waitForFile(ready); + expect(existsSync(pickerCaOwnerPath(dir))).toBe(true); + const published = readFileSync(pickerCaCertPath(dir), "utf8"); + const peer = Bun.spawnSync({ + cmd: [process.execPath, "-e", + `import { ensurePickerCa } from ${JSON.stringify(PICKER_CA_MODULE_URL)};\n` + + `try { ensurePickerCa(${JSON.stringify(dir)}, { rotation: "startup" }); process.stdout.write("rotated"); }\n` + + `catch (error) { process.stdout.write(String(error)); }`], + cwd: dir, + env: { ...process.env, HOME: dir, OPENCODEX_HOME: dir, TMPDIR: dir }, + stdout: "pipe", stderr: "pipe", + }); + expect(peer.exitCode).toBe(0); + expect(peer.stdout.toString()).toContain("picker_ca_live_owner"); + expect(readFileSync(pickerCaCertPath(dir), "utf8")).toBe(published); + } finally { + writeFileSync(release, "release"); + await child.exited; + } +}); + test("a failed certificate replacement retains the public predecessor record and published PEM", () => { const dir = tempDir(); const first = Bun.spawnSync({ diff --git a/tests/claude-integration/claude-picker-recovery.test.ts b/tests/claude-integration/claude-picker-recovery.test.ts new file mode 100644 index 00000000000..4172b6acd9e --- /dev/null +++ b/tests/claude-integration/claude-picker-recovery.test.ts @@ -0,0 +1,136 @@ +// INV-PICKER-02: the outgoing public picker CA survives process replacement until a confirmed untrust clears it. +import { expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { createServer } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { createDesktopPickerController } from "../../src/claude/desktop-picker"; +import { createCertificateAuthority } from "../../src/claude/intercept/local-ca"; +import { + ensurePickerCa, pickerCaCertPath, pickerCaFingerprints, pickerCaPendingUntrustPath, + readPendingPickerCaUntrust, PICKER_CA_COMMON_NAME, PICKER_HOST, +} from "../../src/claude/intercept/picker-ca"; +import type { PickerRuntime } from "../../src/claude/intercept/picker-runtime"; +import type { OcxConfig } from "../../src/types"; + +const runtimeUrl = pathToFileURL(join(import.meta.dir, "../../src/claude/intercept/runtime.ts")).href; +const caUrl = pathToFileURL(join(import.meta.dir, "../../src/claude/intercept/picker-ca.ts")).href; + +async function freePort(): Promise { + const server = createServer(); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("missing port"); + await new Promise(resolve => server.close(() => resolve())); + return address.port; +} + +function replacement(root: string, port: number, fail: boolean, fingerprints: string[]) { + const source = ` + import { readFileSync } from "node:fs"; + import { startClaudeIntercept, getClaudePickerRuntime } from ${JSON.stringify(runtimeUrl)}; + import { pickerCaFingerprints } from ${JSON.stringify(caUrl)}; + const root = ${JSON.stringify(root)}; + const trusted = new Set(${JSON.stringify(fingerprints)}); + const attempts = []; + const security = async args => { + if (args[0] === "find-certificate") return { code: 0, stdout: [...trusted].map(sha => "SHA-1 hash: " + sha).join("\\n"), stderr: "" }; + if (args[0] === "remove-trusted-cert") { + const sha = pickerCaFingerprints(readFileSync(args[1], "utf8")).sha1; + attempts.push(sha); + if (${fail}) return { code: 1, stdout: "", stderr: "" }; + trusted.delete(sha); + } + if (args[0] === "delete-certificate" && !${fail}) trusted.delete(args[2]); + return { code: ${fail} && args[0] === "delete-certificate" ? 1 : 0, stdout: "", stderr: "" }; + }; + let created = false; + const handle = await startClaudeIntercept({ + config: { port: 10100, providers: {}, defaultProvider: "openai", claudeCode: { intercept: { port: ${port} } } }, + publicPort: 10100, configDir: root, dispatch: async () => new Response("unused"), + loadPickerRoutes: async () => ({ nativeSlugs: [], routedModels: [] }), + createPicker: () => { created = true; return { selectTunnel: () => ({ kind: "blind" }), start: async () => {}, stop: async () => {} }; }, + pickerSecurity: security, pickerPlatform: "darwin", + }); + const result = { created, active: getClaudePickerRuntime() !== null, attempts, pickerPort: handle?.pickerProxyPort ?? null }; + await handle?.stop(); + process.stdout.write(JSON.stringify(result)); + `; + const child = Bun.spawnSync({ + cmd: [process.execPath, "-e", source], + cwd: root, + env: { ...process.env, HOME: root, OPENCODEX_HOME: root, CODEX_HOME: join(root, "codex"), TMPDIR: root }, + stdout: "pipe", stderr: "pipe", + }); + expect(child.exitCode, child.stderr.toString()).toBe(0); + return JSON.parse(child.stdout.toString()) as { created: boolean; active: boolean; attempts: string[]; pickerPort: number | null }; +} + +test("a replacement process retries the recorded predecessor before rotating and arming", { timeout: 30_000 }, async () => { + const root = mkdtempSync(join(tmpdir(), "ocx-picker-recovery-")); + try { + mkdirSync(join(root, "codex")); + const foreign = createCertificateAuthority({ commonName: PICKER_CA_COMMON_NAME, permittedDnsNames: [PICKER_HOST] }); + const stateDir = join(root, "claude-picker"); + await Bun.write(join(stateDir, "ca.pem"), foreign.certPem); + const foreignSha1 = pickerCaFingerprints(foreign.certPem).sha1; + const port = await freePort(); + const first = replacement(root, port, true, [foreignSha1]); + expect(first).toMatchObject({ created: false, active: false, attempts: [foreignSha1] }); + expect(readPendingPickerCaUntrust(root)?.sha1).toBe(foreignSha1); + const firstProcessSha1 = pickerCaFingerprints(readFileSync(pickerCaCertPath(root), "utf8")).sha1; + expect(firstProcessSha1).not.toBe(foreignSha1); + const second = replacement(root, port, false, [foreignSha1, firstProcessSha1]); + expect(second).toMatchObject({ created: true, active: true, attempts: [foreignSha1, firstProcessSha1] }); + expect(second.pickerPort).toBe(port + 1); + expect(readPendingPickerCaUntrust(root)).toBeNull(); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("controller enable with pending cleanup does not request trust", async () => { + const root = mkdtempSync(join(tmpdir(), "ocx-picker-pending-enable-")); + try { + const ca = ensurePickerCa(root); + writeFileSync(pickerCaPendingUntrustPath(root), JSON.stringify({ certPem: ca.certPem, ...pickerCaFingerprints(ca.certPem) })); + const calls: string[] = []; + const runtime = { status: () => ({ + desired: true, supported: true, trust: "untrusted", listenerReady: false, effective: false, + latched: false, reason: "trust_untrusted", models: 0, snapshotAt: null, lastBootstrapAt: null, + }), rearm: async () => { throw new Error("must not rearm"); } } as unknown as PickerRuntime; + const current = { port: 10100, providers: {}, defaultProvider: "openai", + clientIntegrations: { "claude-desktop": true }, claudeCode: { desktopMode: "first-party", intercept: { picker: true } }, + } as OcxConfig; + const controller = createDesktopPickerController({ + runtime, readConfig: () => current, persistPreference: () => true, proxyPort: () => 10201, + configDir: root, platform: "darwin", security: async args => { + calls.push(args[0]!); + return { code: 0, stdout: "", stderr: "" }; + }, + }); + expect((await controller.enable({ persist: false, context: "server" })).effective).toBe(false); + expect(calls).toEqual([]); + expect(readPendingPickerCaUntrust(root)).not.toBeNull(); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("replacement defers a pending certificate still published by a live owner", async () => { + const root = mkdtempSync(join(tmpdir(), "ocx-picker-live-owner-")); + try { + mkdirSync(join(root, "codex")); + const ca = ensurePickerCa(root); + const pending = { certPem: ca.certPem, ...pickerCaFingerprints(ca.certPem) }; + writeFileSync(pickerCaPendingUntrustPath(root), JSON.stringify(pending)); + const port = await freePort(); + const peer = replacement(root, port, false, [pending.sha1]); + expect(peer).toMatchObject({ created: false, active: false, attempts: [] }); + expect(readPendingPickerCaUntrust(root)).toEqual(pending); + expect(pickerCaFingerprints(readFileSync(pickerCaCertPath(root), "utf8")).sha1).toBe(pending.sha1); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/tests/claude-integration/claude-picker-runtime.test.ts b/tests/claude-integration/claude-picker-runtime.test.ts index d94a176dfaa..1c00020e340 100644 --- a/tests/claude-integration/claude-picker-runtime.test.ts +++ b/tests/claude-integration/claude-picker-runtime.test.ts @@ -6,7 +6,8 @@ import { join } from "node:path"; import { applyDesktopFirstParty } from "../../src/claude/desktop-first-party"; import { applyDesktopPickerProfile, inspectDesktopPickerProfile } from "../../src/claude/desktop-picker-profile"; import { claudeDesktopIntegrationEnabled } from "../../src/codex/desired-state"; -import { ensurePickerCa, pickerCaCertPath, pickerCaFingerprints, pickerStateDir, PICKER_CA_COMMON_NAME, PICKER_HOST } from "../../src/claude/intercept/picker-ca"; +import { ensurePickerCa, pickerCaCertPath, pickerCaFingerprints, pickerCaPendingUntrustPath, pickerStateDir, PICKER_CA_COMMON_NAME, PICKER_HOST } from "../../src/claude/intercept/picker-ca"; +import { startConnectProxy } from "../../src/claude/intercept/connect-proxy"; import { createCertificateAuthority } from "../../src/claude/intercept/local-ca"; import { readClaudeInterceptProxyToken } from "../../src/claude/intercept/proxy-auth"; import type { PickerListenerOptions } from "../../src/claude/intercept/picker-listener"; @@ -293,6 +294,25 @@ function connectStatusLine(port: number, host: string, userAgent?: string): Prom }); } +function blindRoundTrip(port: number): Promise { + return new Promise((resolve, reject) => { + const socket = connect({ host: "127.0.0.1", port }); + let received = ""; + const timer = setTimeout(() => { socket.destroy(); reject(new Error("blind tunnel timed out")); }, 5000); + socket.once("connect", () => socket.write("CONNECT fake.invalid:443 HTTP/1.1\r\nHost: fake.invalid:443\r\nUser-Agent: Mozilla/5.0\r\n\r\n")); + socket.on("data", chunk => { + received += chunk.toString(); + if (received.includes("\r\n\r\n") && !received.includes("tunnel-payload")) socket.write("tunnel-payload"); + if (received.includes("tunnel-payload")) { + clearTimeout(timer); + socket.destroy(); + resolve(received); + } + }); + socket.once("error", error => { clearTimeout(timer); reject(error); }); + }); +} + describe("startClaudeIntercept wiring", () => { function interceptOptions(port: number, createPicker: Parameters[0]["createPicker"]) { return { @@ -497,11 +517,27 @@ describe("startClaudeIntercept wiring", () => { expect(getClaudePickerRuntime()).toBeNull(); }); + // INV-PICKER-01: a failed predecessor untrust keeps the applied profile's egress URL serving blind CONNECT, never picker TLS. test("a failed rotation untrust refuses the picker but keeps the intercept pair serving", async () => { const port = await freePortPair(); + const pickerPort = port + 1; const stateDir = pickerStateDir(root); // Legacy state: a published certificate and the exportable signing key it paired with. ensurePickerCa(root); + const library = join(root, "desktop-library"); + mkdirSync(library, { recursive: true }); + writeFileSync(join(library, "_meta.json"), JSON.stringify({ + appliedId: "previous-profile", entries: [{ id: "previous-profile", name: "Personal" }], + })); + writeFileSync(join(library, "previous-profile.json"), "{}\n"); + expect(applyDesktopPickerProfile({ configDir: root, platform: "darwin", proxyPort: pickerPort }).ok).toBe(true); + const appliedBefore = inspectDesktopPickerProfile({ configDir: root, platform: "darwin" }); + if (appliedBefore.kind !== "applied") throw new Error("profile did not apply"); + const stateBefore = readFileSync(join(stateDir, "profile-state.json"), "utf8"); + const egressPort = Number(new URL(appliedBefore.proxyUrl).port); + const upstream = createServer(socket => socket.on("data", bytes => socket.write(bytes))); + await new Promise(resolve => upstream.listen(0, "127.0.0.1", resolve)); + const upstreamPort = (upstream.address() as { port: number }).port; writeFileSync(join(stateDir, "ca.key"), "legacy-exportable-key\n"); // A different authority than this process will publish — e.g. written by a since-exited peer — // must actually leave the keychain before the replacement arms. @@ -509,7 +545,9 @@ describe("startClaudeIntercept wiring", () => { writeFileSync(pickerCaCertPath(root), foreign.certPem); const foreignSha1 = pickerCaFingerprints(foreign.certPem).sha1; // The keychain still trusts the foreign certificate; every removal attempt fails. + const securityCalls: string[] = []; const broken: SecurityRunner = async args => { + securityCalls.push(args[0]!); if (args[0] === "find-certificate") { return { code: 0, stdout: `SHA-1 hash: ${foreignSha1}\n`, stderr: "" }; } @@ -523,6 +561,10 @@ describe("startClaudeIntercept wiring", () => { dispatch: async () => new Response("unused"), loadPickerRoutes: async () => ({ nativeSlugs: [], routedModels: [] }), createPicker: () => { pickerCreated = true; throw new Error("must not start"); }, + startProxy: (boundPort, options) => startConnectProxy(boundPort, { + ...options, + ...(boundPort === egressPort ? { dialUpstream: () => connect({ host: "127.0.0.1", port: upstreamPort }) } : {}), + }), pickerSecurity: broken, pickerPlatform: "darwin", }); @@ -533,13 +575,87 @@ describe("startClaudeIntercept wiring", () => { // Failing closed: the picker never arms while a foreign signing key stays trusted. expect(pickerCreated).toBe(false); expect(getClaudePickerRuntime()).toBeNull(); + expect(securityCalls).not.toContain("add-trusted-cert"); + expect(handle?.pickerProxyPort).toBe(egressPort); + expect(await blindRoundTrip(egressPort)).toContain("HTTP/1.1 200 Connection Established\r\n\r\ntunnel-payload"); + expect(inspectDesktopPickerProfile({ configDir: root, platform: "darwin" })).toEqual(appliedBefore); + expect(readFileSync(join(stateDir, "profile-state.json"), "utf8")).toBe(stateBefore); + expect(JSON.parse(stateBefore).previousAppliedId).toBe("previous-profile"); + expect(existsSync(pickerCaPendingUntrustPath(root))).toBe(true); // The main intercept proxy stayed bound and still terminates api.anthropic.com CONNECTs. expect(await canBind(port)).toBe(false); expect(await connectStatusLine(port, "api.anthropic.com")).toContain("200"); } finally { await handle?.stop(); + await new Promise(resolve => upstream.close(() => resolve())); } expect(getClaudePickerRuntime()).toBeNull(); + expect(await canBind(egressPort)).toBe(true); + }); + + test("a busy applied egress port leaves the failed-cleanup profile and journal intact", async () => { + const port = await freePortPair(); + const pickerPort = port + 1; + ensurePickerCa(root); + expect(applyDesktopPickerProfile({ configDir: root, platform: "darwin", proxyPort: pickerPort }).ok).toBe(true); + const profileBefore = inspectDesktopPickerProfile({ configDir: root, platform: "darwin" }); + const stateBefore = readFileSync(join(pickerStateDir(root), "profile-state.json"), "utf8"); + const foreign = createCertificateAuthority({ commonName: PICKER_CA_COMMON_NAME, permittedDnsNames: [PICKER_HOST] }); + writeFileSync(pickerCaCertPath(root), foreign.certPem); + const sha1 = pickerCaFingerprints(foreign.certPem).sha1; + const squatter = createServer(socket => socket.end("held-by-peer")); + await new Promise(resolve => squatter.listen(pickerPort, "127.0.0.1", resolve)); + let handle; + try { + handle = await startClaudeIntercept({ + config: config({ claudeCode: { intercept: { port } } }), publicPort: 10100, configDir: root, + dispatch: async () => new Response("unused"), + loadPickerRoutes: async () => ({ nativeSlugs: [], routedModels: [] }), + createPicker: () => { throw new Error("must not create picker"); }, + pickerSecurity: async args => args[0] === "find-certificate" + ? { code: 0, stdout: `SHA-1 hash: ${sha1}\n`, stderr: "" } + : { code: 1, stdout: "", stderr: "" }, + pickerPlatform: "darwin", + }); + expect(handle?.pickerProxyPort).toBeNull(); + expect(getClaudePickerRuntime()).toBeNull(); + expect(squatter.listening).toBe(true); + expect(inspectDesktopPickerProfile({ configDir: root, platform: "darwin" })).toEqual(profileBefore); + expect(readFileSync(join(pickerStateDir(root), "profile-state.json"), "utf8")).toBe(stateBefore); + expect(existsSync(pickerCaPendingUntrustPath(root))).toBe(true); + } finally { + await handle?.stop(); + await new Promise(resolve => squatter.close(() => resolve())); + } + }); + + test("a corrupt pending record keeps an applied egress blind without a keychain call", async () => { + const port = await freePortPair(); + const pickerPort = port + 1; + ensurePickerCa(root); + expect(applyDesktopPickerProfile({ configDir: root, platform: "darwin", proxyPort: pickerPort }).ok).toBe(true); + writeFileSync(pickerCaPendingUntrustPath(root), "{corrupt"); + let created = false; + const calls: string[] = []; + const handle = await startClaudeIntercept({ + config: config({ claudeCode: { intercept: { port } } }), publicPort: 10100, configDir: root, + dispatch: async () => new Response("unused"), + loadPickerRoutes: async () => ({ nativeSlugs: [], routedModels: [] }), + createPicker: () => { created = true; throw new Error("must not create picker"); }, + pickerSecurity: async args => { + calls.push(args[0]!); + return { code: 0, stdout: "", stderr: "" }; + }, + pickerPlatform: "darwin", + }); + try { + expect(handle?.pickerProxyPort).toBe(pickerPort); + expect(created).toBe(false); + expect(calls).toEqual([]); + expect(readFileSync(pickerCaPendingUntrustPath(root), "utf8")).toBe("{corrupt"); + } finally { + await handle?.stop(); + } }); test("a restart preserves the picker row identity and its recorded previous selection", { timeout: 30_000 }, async () => { diff --git a/tests/codex-integration/catalog-auto-refresh-scheduler.test.ts b/tests/codex-integration/catalog-auto-refresh-scheduler.test.ts index 329a04b14c6..a946e6b6869 100644 --- a/tests/codex-integration/catalog-auto-refresh-scheduler.test.ts +++ b/tests/codex-integration/catalog-auto-refresh-scheduler.test.ts @@ -1,13 +1,16 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; -import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, readFileSync, realpathSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { fileURLToPath } from "node:url"; import { catalogAutoRefreshIntervalForTests, catalogAutoRefreshTickCountForTests, isCatalogAutoRefreshRunning, resetCatalogAutoRefreshForTests, runCatalogAutoRefreshTickForTests, + selectDriftHealCatalogPath, startCatalogAutoRefresh, stopCatalogAutoRefresh, } from "../../src/codex/catalog-auto-refresh"; @@ -16,8 +19,10 @@ import type { CatalogOnlyOutcome } from "../../src/codex/convergence-types"; import * as managementConvergence from "../../src/codex/management-convergence"; import { CATALOG_AUTO_REFRESH_MIN_INTERVAL_MS, + armDetachedConfigBaseline, getConfigPath, getDefaultConfig, + loadConfig, saveConfigPreservingClaudeCode, } from "../../src/config"; import type { OcxConfig } from "../../src/types"; @@ -249,49 +254,251 @@ describe("catalog auto-refresh scheduler", () => { }); describe("catalog auto-refresh drift heal", () => { - // The heal loads its collaborators through dynamic imports; stub each so the test pins the - // tick glue (gate, drift, live runtime port, sync, outcome) without touching a real home. - async function runHealTick(afterSyncDrifted: boolean) { + test.each([ + ["Codex OFF", (config: OcxConfig) => { config.clientIntegrations = { ...config.clientIntegrations, codex: false }; }], + ["picker order", (config: OcxConfig) => { config.codexAccountPriorities = { __main__: 5 }; }], + ] as const)("real persisted config stays stable and a later %s edit blocks the client write", async (_label, edit) => { const drift = await import("../../src/codex/config-drift-heal"); const desired = await import("../../src/codex/desired-state"); const processState = await import("../../src/config/process-state"); + const inject = await import("../../src/codex/inject"); + writeCatalogAutoRefreshConfig({ enabled: true, intervalMinutes: 60 }); + const fixture = JSON.parse(readFileSync(getConfigPath(), "utf8")) as Record; + fixture.apiKeys = [{ key: "fixture-key", name: "fixture", createdAt: "2026-01-01T00:00:00.000Z" }]; + // The load path assigns a stable id to this legacy row without writing config.json. + (fixture.providers as Record>).xai!.modelCosts = { + "grok-fixture": { input: 1, output: 2, cacheRead: 0.5, cacheWrite: 0.5 }, + }; + writeFileSync(getConfigPath(), JSON.stringify(fixture), "utf8"); + const persistedBefore = readFileSync(getConfigPath(), "utf8"); + const firstLoad = loadConfig(); + const captured = JSON.stringify(firstLoad); + armDetachedConfigBaseline(firstLoad); + expect(JSON.stringify(firstLoad)).toBe(captured); + expect(JSON.stringify(loadConfig())).toBe(captured); + expect(readFileSync(getConfigPath(), "utf8")).toBe(persistedBefore); + + let editBeforeWrite = false; + let injectorCalls = 0; + let allowedWrites = 0; + let refusedWrites = 0; + const spies = [ + spyOn(desired, "shouldSyncCodexOnStart").mockReturnValue(true), + spyOn(drift, "codexConfigDrift").mockReturnValue({ drifted: true, missingKeys: ["openai_base_url"] }), + spyOn(processState, "readRuntimePort").mockReturnValue({ pid: process.pid, port: 43_210 } as never), + spyOn(inject, "injectCodexConfig").mockImplementation((async (_port, _config, options) => { + injectorCalls += 1; + if (editBeforeWrite) { + const onDisk = JSON.parse(readFileSync(getConfigPath(), "utf8")) as OcxConfig; + edit(onDisk); + writeFileSync(getConfigPath(), JSON.stringify(onDisk), "utf8"); + } + try { + options?.beforeClientWrite?.(); + allowedWrites += 1; + } catch { + refusedWrites += 1; + } + return { success: true, message: "fixture" }; + }) as typeof inject.injectCodexConfig), + ]; + try { + await runCatalogAutoRefreshTickForTests(); + expect(injectorCalls).toBe(1); + expect(allowedWrites).toBe(1); + expect(refusedWrites).toBe(0); + editBeforeWrite = true; + await runCatalogAutoRefreshTickForTests(); + expect(injectorCalls).toBe(2); + expect(allowedWrites).toBe(1); + expect(refusedWrites).toBe(1); + } finally { + for (const spy of spies) spy.mockRestore(); + } + }); + + // Mock the writer and drift observer: a test must never reach a real Codex home. + async function runHealTick(afterInjectDrifted: boolean) { + const drift = await import("../../src/codex/config-drift-heal"); + const observeDrift = drift.codexConfigDrift; + const desired = await import("../../src/codex/desired-state"); + const processState = await import("../../src/config/process-state"); const sync = await import("../../src/codex/sync"); - let driftCalls = 0; - const syncedPorts: number[] = []; + const inject = await import("../../src/codex/inject"); + const configPath = join(openCodexHome, "healed-config.toml"); + const injected: Array<{ port: number; lockTimeoutMs: number | undefined }> = []; const info: string[] = []; const spies = [ spyOn(desired, "shouldSyncCodexOnStart").mockReturnValue(true), - spyOn(drift, "codexConfigDrift").mockImplementation(() => { - driftCalls += 1; - const drifted = driftCalls === 1 || afterSyncDrifted; - return { drifted, missingKeys: drifted ? ["openai_base_url"] : [] }; - }), + spyOn(drift, "codexConfigDrift").mockImplementation(() => observeDrift( + () => ({ injectedOpenaiBaseUrl: "http://127.0.0.1:43210/backend-api/codex" }), + configPath, + )), spyOn(processState, "readRuntimePort").mockReturnValue({ pid: process.pid, port: 43_210 } as never), - spyOn(sync, "syncModelsToCodex").mockImplementation((async (port: number) => { - syncedPorts.push(port); - return { ok: true } as never; - }) as never), + spyOn(sync, "syncModelsToCodex").mockImplementation((async () => { throw new Error("full sync must not run"); }) as never), + spyOn(inject, "injectCodexConfig").mockImplementation((async (port, _config, options) => { + options?.beforeClientWrite?.(); + injected.push({ port, lockTimeoutMs: options?.lockTimeoutMs }); + if (!afterInjectDrifted) writeFileSync(configPath, 'openai_base_url = "http://127.0.0.1:43210/backend-api/codex"\n'); + return { success: true, message: "fixture" }; + }) as typeof inject.injectCodexConfig), spyOn(console, "info").mockImplementation((...args: unknown[]) => { info.push(args.join(" ")); }), ]; try { writeCatalogAutoRefreshConfig({ enabled: true, intervalMinutes: 60 }); + writeFileSync(configPath, 'model = "gpt-5"\n'); await runCatalogAutoRefreshTickForTests(); } finally { for (const spy of spies) spy.mockRestore(); } - return { syncedPorts, info }; + return { injected, info }; } - test("re-injects through the live runtime port and reports it only when the keys return", async () => { + test("injects through the live runtime port with a one-second lock wait and reports observed keys", async () => { const healed = await runHealTick(false); - expect(healed.syncedPorts).toEqual([43_210]); + expect(healed.injected).toEqual([{ port: 43_210, lockTimeoutMs: 1000 }]); expect(healed.info.some(line => line.includes("re-injected"))).toBe(true); }); - test("a sync that leaves the keys missing is not reported as a heal", async () => { + test("an injector that leaves the keys missing is not reported as a heal", async () => { const ceded = await runHealTick(true); - expect(ceded.syncedPorts).toEqual([43_210]); + expect(ceded.injected).toEqual([{ port: 43_210, lockTimeoutMs: 1000 }]); expect(ceded.info.some(line => line.includes("; re-injected"))).toBe(false); expect(ceded.info.some(line => line.includes("not re-injected this tick"))).toBe(true); }); + + test.each(["stop", "restart", "settings", "off"])("a deferred injector cannot write or log success after %s", async (change) => { + const drift = await import("../../src/codex/config-drift-heal"); + const desired = await import("../../src/codex/desired-state"); + const processState = await import("../../src/config/process-state"); + const sync = await import("../../src/codex/sync"); + const inject = await import("../../src/codex/inject"); + let entered!: () => void; + const enteredInjector = new Promise(resolve => { entered = resolve; }); + let release!: () => void; + const released = new Promise(resolve => { release = resolve; }); + let writes = 0; + const info: string[] = []; + const spies = [ + spyOn(desired, "shouldSyncCodexOnStart").mockReturnValue(true), + spyOn(drift, "codexConfigDrift").mockReturnValue({ drifted: true, missingKeys: ["openai_base_url"] }), + spyOn(processState, "readRuntimePort").mockReturnValue({ pid: process.pid, port: 43_210 } as never), + spyOn(sync, "syncModelsToCodex").mockImplementation((async () => { + entered(); + await released; + writes += 1; + return { ok: true } as never; + }) as never), + spyOn(inject, "injectCodexConfig").mockImplementation((async (_port, _config, options) => { + entered(); + await released; + try { + options?.beforeClientWrite?.(); + writes += 1; + } catch { /* stale tick refused at the write boundary */ } + return { success: true, message: "fixture" }; + }) as typeof inject.injectCodexConfig), + spyOn(console, "info").mockImplementation((...args: unknown[]) => { info.push(args.join(" ")); }), + ]; + try { + writeCatalogAutoRefreshConfig({ enabled: true, intervalMinutes: 60 }); + startCatalogAutoRefresh(); + const tick = runCatalogAutoRefreshTickForTests(); + await enteredInjector; + if (change === "stop" || change === "restart") { + stopCatalogAutoRefresh(); + if (change === "restart") startCatalogAutoRefresh(); + } else { + const persisted = JSON.parse(readFileSync(getConfigPath(), "utf8")) as OcxConfig; + if (change === "off") persisted.clientIntegrations = { ...persisted.clientIntegrations, codex: false }; + else persisted.injectionModel = "changed-while-healing"; + writeFileSync(getConfigPath(), JSON.stringify(persisted), "utf8"); + } + release(); + await tick; + expect(writes).toBe(0); + expect(info.some(line => line.includes("re-injected"))).toBe(false); + } finally { + release(); + for (const spy of spies) spy.mockRestore(); + } + }); + + test("a valid non-default journaled catalog survives a removed config path", () => { + const journalPath = join(openCodexHome, "journal.json"); + const defaultPath = join(openCodexHome, "opencodex-catalog.json"); + const alternatePath = join(openCodexHome, "alternate-catalog.json"); + writeFileSync(defaultPath, JSON.stringify({ models: [] })); + writeFileSync(alternatePath, JSON.stringify({ models: [{ slug: "alternate" }] })); + writeFileSync(journalPath, JSON.stringify({ version: 1, injectedCatalogPath: "alternate-catalog.json" })); + expect(selectDriftHealCatalogPath(journalPath, defaultPath, path => join(openCodexHome, path))).toBe(alternatePath); + }); + + test("the drift branch passes a journaled non-default catalog to the injector", () => { + const root = mkdtempSync(join(tmpdir(), "ocx-heal-catalog-child-")); + const home = join(root, "home"); + const ocx = join(root, "ocx"); + const codex = join(root, "codex"); + const tmp = join(root, "tmp"); + for (const directory of [home, ocx, codex, tmp]) mkdirSync(directory); + const schedulerPath = fileURLToPath(new URL("../../src/codex/catalog-auto-refresh.ts", import.meta.url)); + const driftPath = fileURLToPath(new URL("../../src/codex/config-drift-heal.ts", import.meta.url)); + const desiredPath = fileURLToPath(new URL("../../src/codex/desired-state.ts", import.meta.url)); + const processStatePath = fileURLToPath(new URL("../../src/config/process-state.ts", import.meta.url)); + const syncPath = fileURLToPath(new URL("../../src/codex/sync.ts", import.meta.url)); + const injectPath = fileURLToPath(new URL("../../src/codex/inject.ts", import.meta.url)); + const configPath = fileURLToPath(new URL("../../src/config.ts", import.meta.url)); + const script = ` + const { spyOn } = require("bun:test"); + const fs = require("node:fs"); + const path = require("node:path"); + const configModule = require(${JSON.stringify(configPath)}); + const scheduler = require(${JSON.stringify(schedulerPath)}); + const drift = require(${JSON.stringify(driftPath)}); + const desired = require(${JSON.stringify(desiredPath)}); + const processState = require(${JSON.stringify(processStatePath)}); + const sync = require(${JSON.stringify(syncPath)}); + const inject = require(${JSON.stringify(injectPath)}); + const catalogPath = path.join(process.env.CODEX_HOME, "alternate-catalog.json"); + fs.writeFileSync(catalogPath, JSON.stringify({ models: [{ slug: "alternate" }] })); + fs.writeFileSync(path.join(process.env.CODEX_HOME, "config.toml"), 'model = "gpt-5"\\n'); + fs.writeFileSync(path.join(process.env.CODEX_HOME, "opencodex-journal.json"), JSON.stringify({ version: 1, injectedCatalogPath: "alternate-catalog.json" })); + fs.writeFileSync(configModule.getConfigPath(), JSON.stringify({ ...configModule.getDefaultConfig(), defaultProvider: "xai", providers: { xai: { adapter: "openai-responses", baseUrl: "https://api.x.ai/v1" } }, catalogAutoRefresh: { enabled: true, intervalMinutes: 60 } })); + spyOn(desired, "shouldSyncCodexOnStart").mockReturnValue(true); + spyOn(drift, "codexConfigDrift").mockReturnValue({ drifted: true, missingKeys: ["openai_base_url"] }); + spyOn(processState, "readRuntimePort").mockReturnValue({ pid: process.pid, port: 43210 }); + spyOn(sync, "syncModelsToCodex").mockImplementation(async () => { throw new Error("full sync called"); }); + let received = null; + spyOn(inject, "injectCodexConfig").mockImplementation(async (_port, _config, options) => { received = options.catalogPath; options.beforeClientWrite(); return { success: true, message: "fixture" }; }); + spyOn(console, "info").mockImplementation(() => {}); + await scheduler.runCatalogAutoRefreshTickForTests(); + process.stdout.write(JSON.stringify({ received })); + `; + try { + const result = spawnSync(process.execPath, ["--eval", script], { + cwd: process.cwd(), + env: { ...process.env, HOME: home, OPENCODEX_HOME: ocx, CODEX_HOME: codex, TMPDIR: tmp }, + encoding: "utf8", + timeout: 20_000, + }); + expect(result.status).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ received: join(realpathSync.native(codex), "alternate-catalog.json") }); + } finally { + removeTreeWithRetry(root); + } + }); + + test("invalid journal bytes and unsafe catalogs are read without mutation", () => { + const journalPath = join(openCodexHome, "journal.json"); + const defaultPath = join(openCodexHome, "opencodex-catalog.json"); + writeFileSync(defaultPath, JSON.stringify({ models: [] })); + writeFileSync(journalPath, "{invalid journal bytes"); + expect(selectDriftHealCatalogPath(journalPath, defaultPath, path => join(openCodexHome, path))).toBe(defaultPath); + expect(readFileSync(journalPath, "utf8")).toBe("{invalid journal bytes"); + writeFileSync(journalPath, JSON.stringify({ version: 1, injectedCatalogPath: "linked.json" })); + symlinkSync(defaultPath, join(openCodexHome, "linked.json")); + expect(selectDriftHealCatalogPath(journalPath, defaultPath, path => join(openCodexHome, path))).toBe(defaultPath); + writeFileSync(defaultPath, "not a catalog"); + expect(selectDriftHealCatalogPath(journalPath, defaultPath, path => join(openCodexHome, path))).toBeNull(); + }); }); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 4fe30686d8b..d3c768cc8fb 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -309,6 +309,7 @@ "claude-picker-ca.test.ts": "claude-integration", "claude-picker-listener.test.ts": "claude-integration", "claude-picker-models.test.ts": "claude-integration", + "claude-picker-recovery.test.ts": "claude-integration", "claude-picker-runtime.test.ts": "claude-integration", "claude-picker-trust.test.ts": "claude-integration", "claude-desktop-remote-hub.test.ts": "claude-integration", From 26474d95b975c64faf1c76802b01669df2857c5d Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 28 Sep 2026 01:51:29 +0900 Subject: [PATCH 5/6] fix: defer drift heal without a catalog and expire reused legacy picker CA owners Address PR review: a drift heal with no usable catalog no longer strips model_catalog_json (the next tick retries once a catalog exists), and a legacy owner record without a start identity no longer blocks rotation forever when its PID was reused (macOS start time vs record mtime). --- .../picker-ca/010_ca_publication.md | 1 + .../picker-ca/030_codex_drift_heal.md | 2 + src/claude/intercept/picker-ca.ts | 28 +++++++-- src/codex/catalog-auto-refresh.ts | 3 + structure/clients/claude-desktop.md | 3 +- structure/config.md | 2 +- .../claude-picker-ca.test.ts | 26 +++++++- .../catalog-auto-refresh-scheduler.test.ts | 60 +++++++++++++++++++ 8 files changed, 117 insertions(+), 8 deletions(-) diff --git a/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md index 222a3461434..fa6dc208377 100644 --- a/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md +++ b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md @@ -39,3 +39,4 @@ Implemented as planned in `picker-ca.ts`, with startup draining in the new `pick Evidence: isolated focused runs from the leaves — `claude-picker-ca`, `claude-picker-runtime`, `claude-picker-recovery`, test-layout and file-size ratchet tests: 129 pass / 0 fail; `bun run typecheck` and `bun run privacy:scan` passed; the assessor observed pending-file mode `0600` and refusal of a symlinked record and found no new private-key write path. +PR review follow-up: a legacy owner row with no start identity is now considered stale on macOS when `/bin/ps` supplies an absolute process start time more than 2 seconds after `ca-owner.json` was written; missing/unparseable time or mtime remains conservative. A new live-PID regression failed before the fix and now proves that old owner files permit startup rotation while newer ones still refuse it. Isolated `bun test tests/claude-integration/*picker*.test.ts` passed 103/103, and `bun run typecheck` passed. diff --git a/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md b/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md index 83c49878416..7653d34282f 100644 --- a/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md +++ b/devlog/_plan/260927_release_train_4/picker-ca/030_codex_drift_heal.md @@ -42,3 +42,5 @@ Verification used a fresh isolated `HOME`, `OPENCODEX_HOME`, `CODEX_HOME`, and ` - `bun run typecheck`: exit 0, rerun after the final test edit. The source-ownership map lists `structure/config.md` for `src/codex/` (`structure/INDEX.md:121`), and its scheduler paragraph at `structure/config.md:584` should be updated by the coordinator; that file is outside this lane's write scope. + +PR review follow-up: when neither the journaled nor default catalog is usable, drift healing now defers injection and leaves the missing routing roots in place. Catalog-only convergence may create the catalog in that tick, and the next tick retries injection with the new path. The new two-tick regression failed before the fix and passes afterward; isolated scheduler tests passed 19/19, test-layout and file-size ratchet tests passed 27/27, and `bun run typecheck` passed. diff --git a/src/claude/intercept/picker-ca.ts b/src/claude/intercept/picker-ca.ts index 56ed9d87a8c..0a745bc8a5d 100644 --- a/src/claude/intercept/picker-ca.ts +++ b/src/claude/intercept/picker-ca.ts @@ -1,5 +1,5 @@ import { createHash, randomUUID, X509Certificate } from "node:crypto"; -import { chmodSync, lstatSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, lstatSync, mkdirSync, readFileSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { withClientLifecycleSync } from "../../client/lifecycle-lock"; import { @@ -19,6 +19,7 @@ export interface PendingPickerCaUntrust { certPem: string; sha1: string; sha256: const processAuthorities = new Map(); const MAX_PENDING_CA_BYTES = 64 * 1024; +const LEGACY_OWNER_MTIME_TOLERANCE_MS = 2_000; export function pickerStateDir(configDir: string): string { return join(configDir, PICKER_STATE_DIR); } export function pickerCaCertPath(configDir: string): string { return join(pickerStateDir(configDir), "ca.pem"); } @@ -74,7 +75,7 @@ function currentProcessOwnsPublishedCa(configDir: string, ca: PickerCa): boolean } /** The OS process start identity prevents a recycled PID from impersonating the recorded owner. */ -function processStartIdentity(pid: number): string | null { +function processStartIdentity(pid: number, utc = false): string | null { if (process.platform === "linux") { try { const stat = readFileSync(`/proc/${pid}/stat`, "utf8"); @@ -87,6 +88,7 @@ function processStartIdentity(pid: number): string | null { try { const result = Bun.spawnSync(["/bin/ps", "-o", "lstart=", "-p", String(pid)], { stdin: "ignore", stdout: "pipe", stderr: "ignore", + ...(utc ? { env: { ...process.env, TZ: "UTC" } } : {}), }); const value = result.stdout.toString().trim(); return result.exitCode === 0 && value.length > 0 && value.length <= 128 ? value : null; @@ -95,6 +97,15 @@ function processStartIdentity(pid: number): string | null { return null; } +/** Darwin's lstart is local wall-clock time; Linux's /proc start ticks are not comparable to mtime. */ +function processStartEpochMs(pid: number): number | null { + if (process.platform !== "darwin") return null; + const identity = processStartIdentity(pid, true); + if (identity === null) return null; + const epoch = Date.parse(`${identity} UTC`); + return Number.isFinite(epoch) ? epoch : null; +} + function processAlive(pid: number): boolean { if (!Number.isInteger(pid) || pid <= 0) return false; if (pid === process.pid) return true; @@ -120,9 +131,16 @@ function livePublishedOwner(configDir: string, published: string): boolean { if (typeof pid !== "number" || typeof sha256 !== "string") return false; if (sha256 !== pickerCaFingerprints(published).sha256) return false; if (!processAlive(pid)) return false; - // Older owner records have no start identity. Fail conservatively for those: deferring - // cleanup is safer than removing trust from a process that could still be serving. - if (startTime === undefined || startTime === null) return true; + // Older records have no start identity. On Darwin, wall-clock start after the owner + // file proves PID reuse; otherwise preserve the conservative live-owner decision. + if (startTime === undefined || startTime === null) { + const startedAt = processStartEpochMs(pid); + if (startedAt === null) return true; + let mtime: number; + try { mtime = statSync(pickerCaOwnerPath(configDir)).mtimeMs; } + catch { return true; } + return !Number.isFinite(mtime) || startedAt <= mtime + LEGACY_OWNER_MTIME_TOLERANCE_MS; + } if (typeof startTime !== "string" || startTime.length === 0 || startTime.length > 128) return false; const actual = processStartIdentity(pid); return actual === null || actual === startTime; diff --git a/src/codex/catalog-auto-refresh.ts b/src/codex/catalog-auto-refresh.ts index 3c0f6af36eb..afc72410c83 100644 --- a/src/codex/catalog-auto-refresh.ts +++ b/src/codex/catalog-auto-refresh.ts @@ -139,6 +139,9 @@ async function healCodexConfigDrift(config: OcxConfig, entryGeneration: number): if (!runtime) return "not-healed"; const catalogPath = selectDriftHealCatalogPath(JOURNAL_PATH, DEFAULT_CATALOG_PATH, resolveCodexConfigPath); if (!current()) return "none"; + // Keep the missing routing roots visible to the next tick. Catalog-only convergence below + // may recreate the file now, but it does not re-inject config.toml in this tick. + if (catalogPath === null) return "not-healed"; await injectCodexConfig(runtime.port, config, { catalogPath, lockTimeoutMs: TICK_DEADLINE_MS, diff --git a/structure/clients/claude-desktop.md b/structure/clients/claude-desktop.md index c687dcad750..8c47457d96b 100644 --- a/structure/clients/claude-desktop.md +++ b/structure/clients/claude-desktop.md @@ -194,7 +194,8 @@ picker arming. Publication of `ca.pem` and `ca-owner.json` happens only inside t callback, so a busy lock publishes nothing, and a missing or mismatched owner record for our own certificate is rewritten under the lock so a second process cannot rotate out a live owner's authority. The owner record carries the OS process start identity where the platform exposes one, -so a reused PID does not count as the live owner. +so a reused PID does not count as the live owner; an older record without one still counts as live +unless, on macOS, the PID's process started after the record was written. Before a startup rotation replaces `ca.pem`, the outgoing certificate's **public** PEM and its SHA-1/SHA-256 go to `pending-untrust.json` (mode 0600, no key material); only one such record may exist, and a default `ensurePickerCa` call (the controller's enable/trust path) refuses while it diff --git a/structure/config.md b/structure/config.md index 7e93c97e534..e476b51a00a 100644 --- a/structure/config.md +++ b/structure/config.md @@ -265,7 +265,7 @@ each app-managed rewrite writes `model_provider = "custom"` plus such a table, s injected root keys alongside), which re-injects cleanly because the injector strips a root `model_provider` line first. Recovery is drift detection (`src/codex/config-drift-heal.ts`): the auto-refresh tick re-injects the config when a root key the journal says was injected is missing on disk (presence only; a present key with another value is left alone). -The heal calls the injector directly (no provider discovery or catalog write), waits at most the tick's 1-second commit-lock deadline, and its `beforeClientWrite` guard refuses the write once the timer generation changed or the persisted config no longer matches the tick's snapshot; the catalog path is a bounded read-only lookup of the journal's `injectedCatalogPath` (a regular, parseable catalog) falling back to the default or none, and "healed" is reported only after the keys are observed on disk. +The heal calls the injector directly (no provider discovery or catalog write), waits at most the tick's 1-second commit-lock deadline, and its `beforeClientWrite` guard refuses the write once the timer generation changed or the persisted config no longer matches the tick's snapshot; the catalog path is a bounded read-only lookup of the journal's `injectedCatalogPath` (a regular, parseable catalog) falling back to the default; with no usable catalog the heal is deferred to a later tick, and "healed" is reported only after the keys are observed on disk. `ocx sync` and `ocx restore back` run the injector's non-writing preflight before provider discovery or catalog/cache replacement. Deterministic config and ownership refusals therefore diff --git a/tests/claude-integration/claude-picker-ca.test.ts b/tests/claude-integration/claude-picker-ca.test.ts index cba76e90093..12afd082588 100644 --- a/tests/claude-integration/claude-picker-ca.test.ts +++ b/tests/claude-integration/claude-picker-ca.test.ts @@ -1,6 +1,6 @@ import { expect, test } from "bun:test"; import { X509Certificate } from "node:crypto"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { pathToFileURL } from "node:url"; @@ -255,6 +255,30 @@ test("a live foreign owner is never clobbered; a dead one is reclaimed", async ( } }); +test("a legacy live PID is reclaimed only when it started after the owner file", () => { + if (process.platform !== "darwin") return; // Linux's /proc start ticks are not wall-clock time. + const dir = tempDir(); + const ours = ensurePickerCa(dir); + const foreign = createCertificateAuthority({ commonName: PICKER_CA_COMMON_NAME, permittedDnsNames: [PICKER_HOST] }); + const ownerPath = pickerCaOwnerPath(dir); + writeFileSync(pickerCaCertPath(dir), foreign.certPem); + writeFileSync(ownerPath, JSON.stringify({ pid: process.pid, sha256: pickerCaFingerprints(foreign.certPem).sha256 })); + + utimesSync(ownerPath, new Date(0), new Date(0)); + ensurePickerCa(dir, { rotation: "startup" }); + expect(readFileSync(pickerCaCertPath(dir), "utf8")).toBe(ours.certPem); + expect(readPendingPickerCaUntrust(dir)?.certPem).toBe(foreign.certPem); + + const newerDir = tempDir(); + ensurePickerCa(newerDir); + writeFileSync(pickerCaCertPath(newerDir), foreign.certPem); + writeFileSync(pickerCaOwnerPath(newerDir), JSON.stringify({ pid: process.pid, sha256: pickerCaFingerprints(foreign.certPem).sha256 })); + const future = new Date(Date.now() + 10_000); + utimesSync(pickerCaOwnerPath(newerDir), future, future); + expect(() => ensurePickerCa(newerDir, { rotation: "startup" })).toThrow("picker_ca_live_owner"); + expect(readFileSync(pickerCaCertPath(newerDir), "utf8")).toBe(foreign.certPem); +}); + test("a held picker CA lock never permits publication outside the critical section", async () => { const dir = tempDir(); const held = join(dir, "lock-held"); diff --git a/tests/codex-integration/catalog-auto-refresh-scheduler.test.ts b/tests/codex-integration/catalog-auto-refresh-scheduler.test.ts index a946e6b6869..0024f78a87a 100644 --- a/tests/codex-integration/catalog-auto-refresh-scheduler.test.ts +++ b/tests/codex-integration/catalog-auto-refresh-scheduler.test.ts @@ -17,6 +17,7 @@ import { import { lastCatalogAutoRefreshOutcome, resetCatalogAutoRefreshStatusForTests } from "../../src/codex/catalog-refresh-status"; import type { CatalogOnlyOutcome } from "../../src/codex/convergence-types"; import * as managementConvergence from "../../src/codex/management-convergence"; +import { DEFAULT_CATALOG_PATH } from "../../src/codex/paths"; import { CATALOG_AUTO_REFRESH_MIN_INTERVAL_MS, armDetachedConfigBaseline, @@ -67,6 +68,7 @@ beforeEach(() => { openCodexHome = mkdtempSync(join(tmpdir(), "ocx-catalog-auto-refresh-")); process.env.OPENCODEX_HOME = openCodexHome; isolatedCodexHome = installIsolatedCodexHome("ocx-catalog-auto-refresh-codex-"); + writeFileSync(DEFAULT_CATALOG_PATH, JSON.stringify({ models: [] }), "utf8"); resetCatalogAutoRefreshForTests(); resetCatalogAutoRefreshStatusForTests(); convergeFactoryCalls = 0; @@ -488,6 +490,64 @@ describe("catalog auto-refresh drift heal", () => { } }); + test("a missing catalog defers injection until catalog convergence creates one", () => { + const root = mkdtempSync(join(tmpdir(), "ocx-heal-missing-catalog-")); + const home = join(root, "home"); + const ocx = join(root, "ocx"); + const codex = join(root, "codex"); + const tmp = join(root, "tmp"); + for (const directory of [home, ocx, codex, tmp]) mkdirSync(directory); + const source = (name: string) => fileURLToPath(new URL(`../../src/${name}`, import.meta.url)); + const script = ` + const { spyOn } = require("bun:test"); + const fs = require("node:fs"); + const path = require("node:path"); + const config = require(${JSON.stringify(source("config.ts"))}); + const scheduler = require(${JSON.stringify(source("codex/catalog-auto-refresh.ts"))}); + const drift = require(${JSON.stringify(source("codex/config-drift-heal.ts"))}); + const desired = require(${JSON.stringify(source("codex/desired-state.ts"))}); + const processState = require(${JSON.stringify(source("config/process-state.ts"))}); + const inject = require(${JSON.stringify(source("codex/inject.ts"))}); + const management = require(${JSON.stringify(source("codex/management-convergence.ts"))}); + const catalog = path.join(process.env.CODEX_HOME, "opencodex-catalog.json"); + fs.writeFileSync(path.join(process.env.CODEX_HOME, "config.toml"), 'model = "gpt-5"\\n'); + fs.writeFileSync(config.getConfigPath(), JSON.stringify({ ...config.getDefaultConfig(), defaultProvider: "xai", providers: { xai: { adapter: "openai-responses", baseUrl: "https://api.x.ai/v1" } }, catalogAutoRefresh: { enabled: true, intervalMinutes: 60 } })); + spyOn(desired, "shouldSyncCodexOnStart").mockReturnValue(true); + spyOn(drift, "codexConfigDrift").mockReturnValue({ drifted: true, missingKeys: ["openai_base_url"] }); + spyOn(processState, "readRuntimePort").mockReturnValue({ pid: process.pid, port: 43210 }); + const received = []; + spyOn(inject, "injectCodexConfig").mockImplementation(async (_port, _config, options) => { received.push(options.catalogPath); return { success: true, message: "fixture" }; }); + let converges = 0; + spyOn(management, "createManagementConvergeCodex").mockImplementation(() => async () => { + if (++converges === 1) fs.writeFileSync(catalog, JSON.stringify({ models: [] })); + return { kind: "catalog-only", changed: false, catalogRefresh: { status: "committed", changed: false, degraded: false, notices: [] } }; + }); + const info = []; + spyOn(console, "info").mockImplementation((message) => info.push(message)); + await scheduler.runCatalogAutoRefreshTickForTests(); + const first = { received: [...received], converges, catalogExists: fs.existsSync(catalog), + rootsStillMissing: !fs.readFileSync(path.join(process.env.CODEX_HOME, "config.toml"), "utf8").includes("openai_base_url"), + deferred: info.some(line => line.includes("not re-injected this tick")) }; + await scheduler.runCatalogAutoRefreshTickForTests(); + process.stdout.write(JSON.stringify({ first, received, converges })); + `; + try { + const result = spawnSync(process.execPath, ["--eval", script], { + cwd: process.cwd(), + env: { ...process.env, HOME: home, OPENCODEX_HOME: ocx, CODEX_HOME: codex, TMPDIR: tmp }, + encoding: "utf8", timeout: 20_000, + }); + expect(result.status).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + first: { received: [], converges: 1, catalogExists: true, rootsStillMissing: true, deferred: true }, + received: [join(realpathSync.native(codex), "opencodex-catalog.json")], + converges: 2, + }); + } finally { + removeTreeWithRetry(root); + } + }); + test("invalid journal bytes and unsafe catalogs are read without mutation", () => { const journalPath = join(openCodexHome, "journal.json"); const defaultPath = join(openCodexHome, "opencodex-catalog.json"); From 187fc2853d1bbde530ddd65b9d69638f2f3cbe50 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 28 Sep 2026 03:18:29 +0900 Subject: [PATCH 6/6] docs(devlog): escape union pipes in the picker CA plan table --- .../260927_release_train_4/picker-ca/010_ca_publication.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md index fa6dc208377..699212cb296 100644 --- a/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md +++ b/devlog/_plan/260927_release_train_4/picker-ca/010_ca_publication.md @@ -6,7 +6,7 @@ Depends on: `000_plan.md`. Work phase `wp1`, class C4. Source owner: `src/claude | Path | Change | Before → after | | --- | --- | --- | -| `src/claude/intercept/picker-ca.ts` | MODIFY | `underPickerCaLock` returns `T | undefined`, and void callbacks are repeated outside the lock → tagged `{ kind: "acquired", value: T } | { kind: "unavailable", error }`, with no unlocked publication. Track whether the callback entered: an exception after entry or during release propagates as a partial-operation failure, never as mere lock contention. The cached and fresh paths both decide and publish only under the lock. A live foreign owner blocks a fresh publication **and makes a cached mismatched authority throw**; returning the cached CA after merely deferring publication would let callers arm it against a different on-disk owner. Owner records include a process-start identity where the OS supports it, so a reused PID does not falsely count as the recorded owner; legacy records without it conservatively defer. `processAuthorities` is set only after successful locked publication. | +| `src/claude/intercept/picker-ca.ts` | MODIFY | `underPickerCaLock` returns `T \| undefined`, and void callbacks are repeated outside the lock → tagged `{ kind: "acquired", value: T } \| { kind: "unavailable", error }`, with no unlocked publication. Track whether the callback entered: an exception after entry or during release propagates as a partial-operation failure, never as mere lock contention. The cached and fresh paths both decide and publish only under the lock. A live foreign owner blocks a fresh publication **and makes a cached mismatched authority throw**; returning the cached CA after merely deferring publication would let callers arm it against a different on-disk owner. Owner records include a process-start identity where the OS supports it, so a reused PID does not falsely count as the recorded owner; legacy records without it conservatively defer. `processAuthorities` is set only after successful locked publication. | | `src/claude/intercept/picker-ca.ts` | MODIFY | A replacement overwrites the last public PEM with no durable predecessor → under the same lock, parse the current public PEM, write one `{ sha1, sha256, certPem }` record to `pending-untrust.json` by private temporary file/rename **before** `ca.pem` changes, then publish `ca.pem` and `ca-owner.json`. Public temp files use unique exclusive names and are cleaned after rename failure. Refuse another replacement while the record exists. Reject malformed, nonregular, or oversized pending data rather than dropping it. Expose a bounded read, a locked check of whether the pending PEM is still the published certificate with a matching **live** owner (including this PID), and exact-entry acknowledgement that rereads PEM and both hashes under the lock. A missing record means no pending item. No `keyPem`, signing key, private path, or request data enters the record. | | `src/claude/intercept/picker-ca.ts` | MODIFY | `ensurePickerCa(configDir)` returns a CA to controller/CLI/runtime even when predecessor cleanup is unresolved → keep its `PickerCa` return type, but make its default call refuse any pending record or replacement requiring untrust. Add optional `{ rotation: "startup" }` that permits one fresh replacement **only when no prior pending record exists** and returns the CA with its newly queued predecessor for immediate cleanup in phase 020. Only `startClaudeIntercept` uses the option; controller `enable` and picker-runtime material paths use the default and cannot trust or rearm through pending cleanup. | | `tests/claude-integration/claude-picker-ca.test.ts` | MODIFY | Existing cached-authority test at `:142-158` expects an ordinary call to overwrite a different valid certificate → change that case to an explicit refusal with no publication, while keeping the missing-file republish as a separate locked success. Existing process-restart and foreign-owner tests check only the final certificate → test a successful void callback executes exactly once, an actual second process holding the SQLite lock causes zero publication outside it, and competing processes leave `ca.pem` and `ca-owner.json` with matching SHA-256 after serialization. Check the pending record contains only public PEM/fingerprints and is written before replacement; malformed record blocks publication. |