mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-21 21:15:09 +00:00
taodengandClaude Fable 5
d81e0d6b08
Three audit findings in the -p spawn-token resolution + HOME-isolation layer. All are infra/ process changes to how OCP READS and GATES an OAuth token it already holds; none touch the OAuth wire machinery. F3 (MEDIUM) — expiry-window fallback herds concurrent -p spawns into real HOME. When the keychain token is within 5 min of expiry, resolveSpawnToken() returns null and every concurrent spawn simultaneously falls back to the real HOME; each spawned claude then races a refresh_token grant against the SAME single-use refresh token — rotating it out from under the others and the operator's real claude (the credential-fork hazard, #112/#146 class). Fix: a promise-chain mutex (createSerialMutex) serializes ONLY the real-HOME fallback — one such spawn at a time. When a serialized waiter is admitted (prior holder torn down → its claude has refreshed the keychain), it re-runs resolveSpawnToken(): a now-fresh token means it proceeds ISOLATED instead of real-HOME, so the queue drains to the fast path. Isolated spawns never touch the mutex. F5 (LOW-MED) — per-spawn double keychain exec on the hot path. getOAuthCredentials() sync-exec'd `security find-generic-password` up to twice (wrong label first), worst case 5s×2, blocking the event loop and stalling in-flight SSE streams. Fix: (a) memoize the last-good keychain label and try it first (orderLabelsLastGoodFirst); (b) a 30s TTL cache of the read (createTtlCache). This does NOT reintroduce the #146 forever-memoized regression: the TTL bounds only how often we re-READ the keychain; resolveSpawnToken() still applies the 5-min expiry gate (isTokenExpiring) to the CACHED creds on EVERY use, so a token expiring within the window is still rejected → real-HOME fallback. Call sites stay synchronous (no async conversion). F6 (LOW) — memoized isolation decision goes stale; /health could misreport. getSpawnHomeMode() memoized the isolated/real-home decision forever: credentials appearing after startup never enabled isolation; deleting ~/.ocp/spawn-home at runtime ENOENT'd every isolated spawn until restart; during an expiry stint /health reported isolated:true while spawns ran real- HOME. Fix: re-evaluate the decision per spawn (cheap now that F5 caches the keychain read); ensureSpawnHome() re-verifies + re-prepares the scratch dir per isolated spawn; and /health now reports the EFFECTIVE decision (token presence AND expiry gate). The /health field set is UNCHANGED — no field added/removed/renamed — only the values are made truthful. Alignment: - Class: Not a wire/endpoint change for the spawn-token layer + Class B (B.2) for /health. - cli.js citation: DECLARED ABSENT. cli.js does NOT perform OCP's spawn-token resolution, HOME isolation, keychain caching, or fallback serialization — these are proxy-internal process concerns with no cli.js analogue, so no Class A cli.js:NNNN citation exists or is required (ALIGNMENT.md Rule 2: this is proxy infra, not an invented forwarded endpoint/header/body). - The /health change is authorized by ADR 0006 (grandfathered B.2 as of v3.16.4) and is a behaviour-preserving contract change: same fields, truthful values. - OCP still NEVER performs a refresh_token grant itself — that property is preserved; the fix only serializes/gates reads of a token refreshed by the spawned or real claude (#112). - No new endpoint, header, or request/response field. alignment.yml blacklist unaffected. Tests: extracted the pure primitives to lib/spawn-auth.mjs and added 11 unit tests (mutex serialization order + idempotent release; TTL cache freshness + null-miss; expiry gate; label ordering; and the combined invariant that the TTL cache respects the expiry gate). node --check clean; 249 tests pass (238 + 11). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>