Unbounded caches and missing timeouts in long-lived client/server paths

[bokelley]

From codex-security candidate verification. Ranked by real exploitability rather than by how the scan ordered them — and two of the six are recommended won't-fix, with reasons.

Highest amplification: canonical-reference resolver cache (candidate-96b469a344efc673)

src/lib/canonical-references/index.ts:232-247, :418-441. createCanonicalReferenceResolver creates a Map-backed cache with no max entries, max bytes, or TTL. Seller-supplied format_schema URIs are the intended input, and cloneResolved (:219-229) stores Uint8Array.from(body) + structuredClone(document) + text — roughly 3-4 MiB retained per unique reference against a DEFAULT_MAX_BODY_BYTES of 1 MiB. So ~1.4 GB of attacker transfer pins ~4 GB of buyer heap, keyed on attacker-varying URI + digest.

docs/guides/CANONICAL-REFERENCE-RESOLVER.md:62 documents the cache as "caller-owned … fresh per resolver", so the injectable-store pattern applies — but the zero-arg default is unbounded and that's what adopters get. Add maxEntries / maxBytes + TTL to the default.

A2A client cache has no eviction while its MCP sibling does (candidate-6d425da64771c133)

src/lib/protocols/a2a.ts:75-94. a2aClientCache is keyed by URL/auth/headers/signing-identity with no eviction outside a global closeConnections('a2a'). mcp-modern.ts:74 has MAX_CACHED_CONNECTIONS = 20 with evictLeastRecentlyUsed().

The asymmetry looks like an oversight rather than a decision: getOrCreateA2AClient already bypasses the cache for scoped fetchFn with a comment citing "short-lived closure identities do not grow the global cache without bound" — so growth was considered, just not capped on the main path. Mitigator: only successful agent-card discoveries are cached, so each entry costs a real round trip. Mirror the MCP LRU.

Missing timeout on the upstream helper (candidate-4b301f405c1f1118)

src/lib/server/upstream-helpers.ts:258-273. No AbortSignal, and res.text() buffers the whole body before truncating it for the error message. The missing timeout matters more than the missing size cap — a slow or hung upstream holds the connection indefinitely from inside a request handler. signal: AbortSignal.timeout(n) plus a byte-counted stream read.

Multi-host metadata Maps keyed on the Host header (candidate-6a8775efebc5c905) — recommend won't-fix, document instead

src/lib/server/serve.ts:355-401 memoizes function-form publicUrl, origin, and protected-resource metadata in three process-lifetime Maps. The key is genuinely attacker-controlled — serve.ts:892-908 returns the lowercased raw header with no allowlist, and the /.well-known/oauth-protected-resource branch that populates all three is explicitly auth-free (:402-410).

What stops it in practice: a resolver that throws or returns an invalid value never reaches .set(), so the adopter's own resolver is the de facto gate. Recommend an LRU cap only if cheap, but the real fix is a sentence in the JSDoc at :192-196 — which currently promises memoization without mentioning it is unbounded — telling adopters their multi-host resolver must reject unknown hosts.

Webhook idempotency-key store (candidate-b53f9915f7a988c3) — won't-fix as filed; the obvious fix is a correctness bug

src/lib/server/webhook-emitter.ts:57-64, :444-467. memoryWebhookKeyStore does retain every operation_id forever. But growth is linear with legitimate business volume at ~100 bytes/entry, and :41-49 is the documented "inject a durable backend" pattern.

The trap worth recording: the JSDoc calls key stability across retries "the load-bearing invariant the receiver-side dedup depends on." A naive LRU cap would evict a live operation and make resolveIdempotencyKey mint a new key on retry — converting a slow memory leak into silent duplicate-processing at the receiver. Only a TTL keyed to the retry/dedup window is safe here. Do not "fix" this with a max-entries cap.

Also

bin/adcp-async-handler.js:85-93 unbounded body buffering is filed with the CLI webhook auth work in #2426, since both live in the same function and want one edit.

[bokelley]

Triage

Classification: Bug (memory exhaustion / availability)
Bucket(s): library
Status: ready-for-human

What the experts said:

• security-reviewer: All five findings confirmed. Findings 1 (canonical-cache heap amplification) and 3 (upstream hang) are co-equal HIGH — Finding 1 requires a rogue seller with HTTPS infrastructure, Finding 3 can be triggered by any slow or hung upstream. Won't-fix calls for items 4 and 5 are sound. No CVE urgency — resource exhaustion only, no data access or cross-tenant exposure.
• code-reviewer: Fix 1 should use LRU (not TTL — keys are content-addressed, so TTL eviction just causes spurious re-fetches) and warrants a minor bump. Fix 2 is straightforward — mirror MCP's MAX_CACHED_CONNECTIONS = 20; add delete-and-reinsert on read for correct LRU ordering. Fix 3: scope to timeout-only (DEFAULT_REQUEST_TIMEOUT_MS = 60s for consistency); byte-bounded body read is separate scope. Fix 4: doc-alone is incomplete — a ~10-line LRU cap on the three serve.ts Maps is warranted as defense-in-depth.

My take: The findings and won't-fix reasoning are solid, but two items need a human call: Fix 3 introduces a breaking TimeoutError where calls previously hung indefinitely (minimum minor bump), and experts diverged on Fix 4 — doc-only vs doc + LRU cap. The bin/adcp-async-handler.js body-buffering item is explicitly folded into #2426.

@bokelley — can you confirm (a) bump strategy across the three fixable items, and (b) whether serve.ts gets the defense-in-depth LRU cap alongside the JSDoc update?

---

Triaged by Claude Code. Session: https://claude.ai/code/cse_01LJw33AyQSofmFXSLr3kixo

---

Generated by Claude Code