apps/server/docs/ai-context/llm-router-codex-followups.md
Codex independent review (2026-05-15) on commit 1bb0aab2f returned 12
findings. Applied 3 HIGH inline (see commits after 1bb0aab2f). The 9
remaining findings are deferred — each evaluated through the AGENTS.md
"外部建议 3 问过滤" and tracked here per "拒绝时留痕" rule.
llmRouter.route() — fixed in
apps/server/src/routes/openai/v1/index.ts. c.req.raw.signal now flows
into both the router and the legacy fetch() fallback. Prevents the
"client disconnects but upstream keeps generating + burning paid quota"
leak.apps/server/src/services/domain/llm-router/router.ts. Every non-2xx fallback
path now calls response.body?.cancel() before continuing. Prevents
socket-pool exhaustion under fallback storms.apps/server/src/services/adapters/tts/azure.ts. Voice id is
regex-validated (^[a-z0-9-]+$/i) before SSML interpolation; invalid
values throw BAD_REQUEST. Prevents attribute-context breakout under
the server's Azure credential.loadFresh() can repopulate cache after invalidate()Severity: MEDIUM. Decision: defer to v1.x.
Rationale: race window is bounded by the Pub/Sub-driven invalidate
firing during a concurrent TTL-driven reload. In practice the operator
revokes a key, the active in-flight load was already reading the
pre-revocation config and so writes back the old key state. Worst case
the stale config lives until next TTL expiry (5s). Acceptable for v1
given the 5s SLO target — not a security hole, just a propagation hiccup.
Fix shape (generation counter on loadFresh) is well-known and cheap; do
it the next time someone touches config-loader.ts.
Severity: MEDIUM. Decision: defer.
Rationale: configKV's underlying Redis read is single-digit-ms in
practice; even a 10-request stampede is 10 cheap reads. The plan's
adversarial reviewer flagged this; we accepted because the cure
(promise-dedup) adds state with its own race window and rarely matters
at airi's current QPS. Revisit if airi.gen_ai.gateway.config.reload
spikes after a Pub/Sub burst.
fullChainTimeoutMs not enforced as a hard capSeverity: MEDIUM. Decision: apply in next iteration. Rationale: legit bug — per-attempt × N keys can exceed the configured 60s cap. Worth fixing but requires a route-level abort controller that composes with the per-attempt one; not a 5-minute change. Track here so it doesn't get lost.
Severity: MEDIUM. Decision: reject for v1. Rationale: codex flagged this without citing real evidence of any v1 provider doing it. The four configured providers (OpenRouter, Azure, DashScope, Volcengine) return proper HTTP status codes for errors. Adding body-shape detection across LLM + 3 TTS adapters adds complexity that guards against a hypothetical. "外部建议 3 问过滤" (c): would create duplicated parsing logic in every adapter. Revisit if a provider is added that does return 200-with-error.
encryptKey()Severity: MEDIUM. Decision: partial accept — fix in next iteration.
Rationale: codex is right that encryptKey leaves plaintextBytes
uncleared. Fix is try/finally { plaintextBytes.fill(0) } — apply in
v1.x. Note: rendered Authorization: Bearer <key> strings cannot be
zeroized once V8 has interned them as JS strings; that limitation is
fundamental and worth documenting in envelope-crypto.ts JSDoc.
seed-router-config.tsSeverity: MEDIUM. Decision: apply.
Rationale: legit security issue — keys appear in ps output and
shell history. Switch to env var input (OPENROUTER_KEY etc.) and
remove the --openrouter-key flag. Will apply in next iteration to the
seed script; the script is operator-only and runs locally so impact is
bounded but the principle is right.
e2e-llm-router.ts debug fetch logs auth header prefixSeverity: MEDIUM. Decision: apply now (trivial).
Rationale: legit. The 30-char prefix can identify accounts. Fix shape:
replace ${auth.slice(0, 30)}... with <set>. Apply in next commit.
current / previous master key not rejectedSeverity: LOW. Decision: apply. Rationale: defensive guard, ~3 lines. Will apply in next iteration.
Severity: LOW. Decision: defer. Rationale: Redis is on the trusted private network in our deployment model; HMAC was already deferred from the plan (P2 finding from ce-doc-review). Size guard is small but its security value is contingent on the same untrusted-Redis threat we explicitly accepted. Mark as follow-up when HMAC is added in the same iteration.