Files
ocp/lib/spawn-auth.mjs
T
2538233059 fix(server): serialize -p real-HOME token fallback + TTL-cache keychain + de-stale isolation decision (F3/F5/F6) (#150)
* fix(server): serialize -p real-HOME token fallback + TTL-cache keychain + de-stale isolation decision (F3/F5/F6)

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>

* fix(server): drain F3 fallback queue immediately — invalidate F5 keychain cache before the serialized re-check

Follow-up to the F3/F5/F6 fix (independent-review observation). F5's 30s keychain TTL cache could
make F3's post-refresh re-check see the stale (expiring) cached creds for up to ~30s, so a waiter
admitted right after the prior real-HOME holder's claude refreshed the keychain would needlessly
fall back to real HOME again instead of proceeding ISOLATED. Serialization safety was never at risk
(still one real-HOME spawn at a time, no double-refresh); only the drain-to-fast-path optimization
lagged.

Fix: invalidateKeychainReadCache() clears the F5 TTL cache; resolveSpawnDecision() calls it under
the fallback mutex, immediately before the re-check, so the admitted waiter reads FRESH keychain
state and drains to the isolated fast path at once. The extra keychain read happens only on the rare
real-HOME fallback path and only under the mutex (serialized, one at a time).

Alignment: unchanged from the parent commit — proxy-internal keychain/HOME-isolation process logic,
no cli.js analogue (cli.js citation DECLARED ABSENT, ALIGNMENT.md Rule 2), no endpoint/header/body,
/health shape unchanged, OCP still never performs a refresh_token grant itself (#112). 249 tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-07 22:50:52 +10:00

68 lines
3.2 KiB
JavaScript

// Pure, dependency-injected primitives for the `-p` spawn-token resolution + HOME-isolation
// layer. Extracted from server.mjs (findings F3 / F5 / F6, 2026-07-07) so the concurrency,
// caching and expiry logic is unit-testable WITHOUT booting the server or mocking execFileSync /
// child_process.spawn / fs. server.mjs owns all I/O (macOS keychain exec, process spawn, fs);
// this module owns only pure decision logic.
//
// ALIGNMENT NOTE: none of this touches the OAuth wire machinery (no endpoint / header / body).
// OCP still NEVER performs a refresh_token grant itself — these helpers only READ + GATE a token
// that some other process (the operator's real claude, or a spawned claude under the real HOME)
// refreshes. That property is load-bearing (issue #112) and preserved.
// Promise-chain mutex. `acquire()` resolves to a `release()` fn; the NEXT `acquire()` does not
// resolve until the current holder calls its `release()`. Serializes async critical sections
// without busy-waiting. release() is idempotent.
export function createSerialMutex() {
let tail = Promise.resolve();
return {
acquire() {
let release;
const gate = new Promise((r) => { release = r; });
const prev = tail;
tail = tail.then(() => gate);
// Hand the caller its release fn only after the previous holder has released.
return prev.then(() => {
let released = false;
return function releaseMutex() { if (!released) { released = true; release(); } };
});
},
};
}
// Short-TTL memo. `get(produce, now)` returns the cached value while `now - storedAt < ttlMs`,
// otherwise calls `produce()` and re-stores. A miss that produces null/undefined is STILL stored
// (so a genuinely-absent source is not re-probed on every call within the TTL window). `now` is
// injectable for testing.
export function createTtlCache({ ttlMs }) {
let value;
let at = -Infinity;
let has = false;
return {
get(produce, now = Date.now()) {
if (has && now - at < ttlMs) return value;
value = produce();
at = now;
has = true;
return value;
},
clear() { has = false; value = undefined; at = -Infinity; },
};
}
// Pure expiry gate. Returns true when `creds` carries a known expiry that is at/within `bufferMs`
// of `now`. Creds WITHOUT `expiresAt` (e.g. long-lived env tokens) are never treated as expiring.
// This gate is applied to the CACHED creds on EVERY use — which is precisely why a short-TTL
// keychain cache (createTtlCache) cannot reintroduce the #146 forever-stale-token regression: the
// cache bounds how often we re-READ the keychain, but the expiry decision is recomputed per use.
export function isTokenExpiring(creds, now = Date.now(), bufferMs = 300000) {
return !!(creds && creds.expiresAt && now + bufferMs >= creds.expiresAt);
}
// Order candidate keychain labels so the last-known-good label is tried first (avoids the
// wrong-label miss that doubles the `security` exec count on the hot path). Pure: performs no
// read. Returns a fresh array; input is not mutated.
export function orderLabelsLastGoodFirst(labels, lastGood) {
if (!lastGood || !labels.includes(lastGood)) return labels.slice();
return [lastGood, ...labels.filter((l) => l !== lastGood)];
}