web-terminal/docs/REVIEW_RELAY_SECURITY.md

Security Audit — relay-e2e + relay-auth (pre-production go-live gate)

Date: 2026-07-02 · Branch: security/relay-auth-audit-fixes · Prior verdict: approve-with-changes Method: multi-agent deep audit — 11 security dimensions fanned out across both packages, every candidate finding adversarially verified by ≥3 independent skeptic lenses (exploitability / crypto-spec / refuter, majority vote), a second "repeat-review" deep-dive pass, a per-package completeness critic, then dedup + severity re-rank. 99 agents, ~6.9M tokens. The orchestrator independently read 100% of both src/ trees to adjudicate.

Verdict

relay-e2e (crypto core): PASS — no changes required. Every candidate against the E2E core (replay-key nonce reuse, sequence/handshake ordering, key-hygiene, X25519 low-order) was refuted on verification: the deterministic-nonce discipline is safe because the direction split + the real agent-side createReplaySealer (in term-relay/agent) each own an independent monotonic counter; recv accept() is a strict-successor guard whose pre-auth commit cannot cause a replay-accept.

relay-auth: APPROVE-WITH-CHANGES — 5 confirmed findings (2 HIGH, 1 MED, 2 LOW), all in the human-auth / enforcement layer. All fixes are contained in relay-auth; the P5-local types (AuthenticatedPrincipal, StepUpPolicy) are extendable without touching any frozen contract.

Refuted (verified NOT exploitable — recorded so they are not re-litigated)

• E2E replay K_content nonce reuse — real consumer keeps an independent monotonic seq (no reset-to-0).
• mTLS verifyChain missing EKU/keyUsage — every abuse cert fails the later mandatory SPIFFE-SAN → enrolled-&-non-revoked-host registry gate.
• SPIFFE trust-domain not pinned on verify — gated by chain-valid + registry; defense-in-depth only.
• Reattach host-scope "bypass" — token.host === requestedHostId bind at decide.ts:62 blocks it; session↔host binding is enforced at connect by P1/P3. (Kept as a cheap D-i-D hardening note.)
• TOTP non-constant-time compare / device-proof acct canonicalization / DPoP ±future-iat — LOW, refuted.

---

Confirmed findings + fix plan

F1 · HIGH — Step-up factor downgrade (human/stepup/stepup.ts:33)

needsStepUp() tests policy.requiredMethod ∈ principal.amr and freshness via a single method-agnostic stepUpAt. amr is cumulative across login + every step-up, so a passkey requirement is satisfied by a passkey used at login, while the freshness timestamp can be produced by a weaker step-up (e.g. phished TOTP). The phishing-resistant control silently downgrades. Fix: bind freshness to the method. Add stepUpMethod to the principal; needsStepUp requires stepUpMethod === requiredMethod AND fresh; recordStepUp stamps the method. (Group A)

F2 · HIGH — Mandatory step-up skipped on the new-session path (enforce/onUpgrade.ts:128)

The gate is wrapped in if (ctx.principal !== null), but the production caller (term-relay/data-plane/upgrade.ts:105) hardcodes principal: null on both connect and reattach (session principal exists only after authz). Result: step-up never runs in production — a valid DPoP-bound token opens a root shell with the mandatory passkey ceremony never performed. Deny-by-default is violated (absence of proof → allow). Fix: make step-up host-driven and fail-closed: when stepUpPolicyFor(host).required, DENY (403 step_up_required) unless an authenticated principal proves fresh step-up. Applies to connect and reattach. Default v0.9 policy is required:false → existing non-step-up hosts unaffected. Behavioral note: once a host sets required:true, the P1 caller must supply a step-up principal; until then such hosts correctly deny. (Group A)

F3 · MEDIUM — WebAuthn assertion not bound to the stored credential (human/webauthn/authenticate.ts:40)

finishAuthentication forwards only signCount to the verifier — never cred.publicKey / cred.credentialId, and there is no credentialId cross-check. Any reasonable verifier can only check challenge/origin/rpId, so an assertion from a different registered credential authenticates. Fix: add credentialPublicKey + expectedCredentialId to the verifier interface & forward them; assert resp.credentialId === cred.credentialId before trusting verified. (Group B)

F4 · LOW — DPoP verify throws (unhandled) → INV15 gate fails un-audited (capability/verify.ts:142)

importEd25519PublicRaw(rawPub) (and readCnfJkt) run outside try/catch; a thumbprint-matching but malformed key makes verifyDpopProof reject instead of returning false. Callers (decide.ts:57, onUpgrade.ts:112) don't catch → the sole authz gate throws an unhandled exception and emits no deny AuditEvent (audit-evasion + repeatable unhandled-rejection DoS). Fail-closed but off-contract. Fix (defense-in-depth): (a) wrap the risky calls in verifyDpopProof → return false; (b) wrap the verifyDpopProof call in coreAuthorize → clean audited deny(401); (c) validate cnfJkt format at issue. (Group C)

F5 · LOW — WebAuthn verified signCount discarded → clone detection inert (human/webauthn/authenticate.ts:52)

The advanced newSignCount is used only for the regression check and never returned/persisted, so the guard forever compares against the registration-time count — a cloned authenticator is never detected. Fix: return newSignCount (updated credential) so the caller can persist it. (Group B)

---

Execution plan (multi-agent, loop mode)

• Foundation (barrier): types.ts — add StepUpPolicy.required: boolean + AuthenticatedPrincipal.stepUpMethod?: AuthMethod|null (+ Zod). Optional stepUpMethod ⇒ no ripple to other principal constructors; absent ⇒ treated as null ⇒ needs-step-up (fail-safe).
• Fix groups (parallel, file-disjoint):
  • Group A (F1,F2): stepup.ts, enforce/onUpgrade.ts, test/{stepup,enforce}.test.ts, test/tripwire/cross-tenant.test.ts.
  • Group B (F3,F5): human/webauthn/{verifier,authenticate}.ts, test/webauthn.test.ts.
  • Group C (F4): capability/{verify,issue}.ts, authz/decide.ts, test/capability.test.ts.
• Verify (loop until green): tsc --noEmit + full vitest run in relay-auth; repair any breakage; repeat. Baseline to preserve/extend: relay-e2e 76, relay-auth 104.
• Re-audit (parallel adversarial): one verifier per finding re-reads the fixed code and confirms the specific exploit is closed.

Each fix ships with a regression test encoding the exploit (F2: STRICT+null→403; F1: fresh TOTP ≠ passkey; F3: credentialId mismatch→reject; F4: malformed-jwk DPoP→false+audited deny; F5: newSignCount surfaced).

---

Second independent review (2026-07-02, Fable 5) — RECORD CORRECTION

A second review (fix-regression hunt + fresh-angle sweep + adversarial re-attack of refuted items), run on a different model for reviewer diversity, returned:

• Fix-regression: 0 — the 5 fixes (F1–F5) survive independent scrutiny, no regressions.
• Re-attack of refuted/D-i-D items: 0 survived — mTLS EKU, SPIFFE domain, reattach host-scope, TOTP replay, device-proof canonicalization are re-confirmed non-exploitable.
• 1 new HIGH — which the first audit had REFUTED in error (a false-negative). Corrected below.

F6 · HIGH — Recoverable K_content replay key reuses (key, nonce) across sealer generations

⚠️ This corrects the earlier "relay-e2e PASS" verdict — relay-e2e's replay surface has a real nonce-reuse hazard. relay-e2e/src/replay-key.ts deriveContentKey keys K_content = HKDF(hostContentSecret, salt=sessionId, info=const) — deterministic, and intended to be recoverable/stable per (host, sessionId). The nonce is deterministic f(seq) and sealReplayFrame is stateless (no SequenceGuard). The sole real consumer, agent/src/e2e/replaySeal.ts:41 createReplaySealer, holds let seq = 0n in memory and resets to 0 on every reconstruction (agent restart/redeploy, or a new attach), while hostContentSecret is persisted 0600 (Keystore) and sessionId is stable (localStorage). Unlike the LIVE h2c path — safe because each reconnect runs a fresh ECDH → brand-new keys — the replay path reuses the same K_content with seq restarting at 0 ⇒ two generations of distinct plaintext sealed under identical (key, nonce). An untrusted relay (INV2 sees ciphertext) that observes both generations recovers plaintext-XOR (terminal output: commands/tokens/code) and, for AES-256-GCM, recovers the GHASH auth-subkey → forgery, which openReplayCiphertext (no cross-frame replay check) will accept. Why round 1 missed it: the round-1 verifier located let seq = 0n, correctly noted it's an independent counter, but wrongly concluded "safe" by analogy to the live path — missing that K_content (unlike the live ephemeral keys) is STABLE across restarts, so the seq reset does collide. Round 2's dedicated re-attack + orchestrator's direct read of replaySeal.ts/hostEndpoint.ts confirm the exploit. Fix options (design decision — touches a frozen contract and/or the agent package, both outside the two originally-scoped packages):

• (A) Agent-side durable seq — persist the replay SequenceGuard with the ring buffer / Keystore so seq never resets for a given (sessionId, K_content). Keeps K_content recoverable; blast radius = agent/ only. Requires durable monotonic-seq guarantee across restart.
• (B) Per-generation epoch in K_content — add an epoch/generation to ReplayKeyParams (frozen in relay-contracts/src/e2e/types.ts), mixed into the HKDF salt, and carry it with each frame so the browser re-derives the right key. Cryptographically robust (fresh key per generation) but changes a frozen contract + relay-e2e + relay-web (browser re-derivation) + agent.
• Regression test (either path): two sealer generations for the same (secret, sessionId) must never emit two frames sharing (key, nonce).

RESOLUTION — FIXED (option B, epoch-in-key), 2026-07-02. Implemented across all 4 packages: ReplayKeyParams (relay-contracts) gains a required epoch: string; deriveContentKey (relay-e2e) folds it into the HKDF salt as utf8(sessionId) ‖ 0x1F ‖ utf8(epoch) (unit-separator, no concat aliasing); createReplaySealer (agent) mints a fresh randomUUID() epoch per generation and exposes it; ReplaySource (relay-web) carries epoch and the browser re-derives with it. A fresh epoch per generation ⇒ a fresh key, so a seq=0 reset after restart/re-attach can never collide with a prior generation's (key, nonce); recoverability within a generation (same epoch ⇒ same key) is preserved. Verified: all 4 packages tsc-clean and green (relay-contracts 81, relay-e2e 78 [+2 F6 regressions], agent 133, relay-web 99); re-audit confirms closure (regression proves two generations share the seq=0 nonce yet derive different keys). Residual (fail-closed, not reuse): the ring-buffer transport (P1/P2) must persist each generation's epoch and serve the epoch matching those exact frames; documented as a TODO at relay-web/.../manage-page.ts (loadReplay is still a throwing stub — the replay path is not wired end-to-end yet, so F6 was latent).

---

Dynamic end-to-end security harness (e2e/, 2026-07-02)

A cross-package harness (e2e/) now wires the real P5(auth) + P4(crypto) + P2(agent replay) exports through in-memory seams and an untrusted-relay "RelaySpy" attacker vantage — every security check runs the production code path; only true I/O boundaries (registries/buckets/revocation/audit/sockets) are faked. buildRelayWorld() exposes the full flow (issue cap → upgrade/authz → handshake → sealed session via relay → reattach → revoke). 21 tests pass, tsc clean. This upgrades the findings from static + unit validation to dynamic validation — the attacker actually attempts each exploit:

Attack (dynamic)	Asserted defense
F1 step-up factor downgrade	fresh TOTP step-up → needsStepUp true + upgrade 403; passkey → allowed
F2 step-up skipped (fail-open)	STRICT + principal:null → 403 step_up_required + audit; required:false+null → allowed
F3 WebAuthn credential binding	credentialId mismatch → WebAuthnError even with a verifier that returns verified:true
F4 DPoP unhandled throw	malformed thumbprint-matching jwk → verifyDpopProof resolves false (no throw) → clean audited dpop_proof_failed deny
F6 replay (key,nonce) reuse	two sealer generations → same seq-0 nonce but different key ⇒ ciphertext+tag differ; cross-gen open throws; within-gen recovers
MITM	wrong agentPubkey / tampered hostEphPub → FingerprintMismatchError, no keys derived
reflection / replay / reorder	c2h→client.open rejects; dup/out-of-order open throws (SequenceGuard)
INV2	RelaySpy ciphertext (live + replay path) never contains the plaintext marker
cross-tenant (INV1)	acct-A token at acct-B host → 403 cross_tenant + cross-tenant-attempt audit
capability single-use	same jti twice (fresh DPoP) → 2nd 403 token_replayed

Not dynamically covered: F5 (signCount persistence) is a return-shape change, asserted by its relay-auth unit regression. The harness fakes the P1 relay/transport (RelaySpy) rather than running term-relay's mux and the control-plane HTTP — a true multi-process/browser e2e would need the (not-yet-built) run tooling.