feat(web): add persistent nonce expiry and replay protection for sign…

pr-168-nonce-replay-protection.md

@@ -0,0 +1,79 @@
+# #168 security(web): add nonce expiry and replay protection for signed requests
+
+Closes #168
+
+## Summary
+
+Replaces the in-memory `Map<string, number>` nonce guard with persistent,
+DB-backed replay protection. Consumed nonces are recorded in a new
+`tls_consumed_nonces` table with a UNIQUE constraint on `(talosId, nonce)`,
+so single-use semantics hold across **process restarts** and **concurrent
+requests** without advisory locks.
+
+## Changes
+
+### DB schema (`web/src/db/schema.ts`, `web/drizzle/0015_add_consumed_nonces.sql`)
+
+- New table `tls_consumed_nonces` with columns:
+  - `id` — primary key
+  - `talosId` — agent identifier
+  - `nonce` — the 32-byte hex nonce
+  - `expiry` — original auth expiry (Unix seconds, used by the vacuum)
+  - `consumedAt` — wall-clock timestamp of consumption
+- **UNIQUE index** on `(talosId, nonce)` — the DB enforces single-use,
+  rejecting the second of two concurrent INSERTs with `code 23505`.
+- Index on `(expiry)` for efficient vacuum queries.
+- RLS policies: postgres role only (internal replay guard).
+
+### Library (`web/src/lib/transfer-signature.ts`)
+
+- `consumeTransferNonce()` → **async**, persists via
+  `db.insert(tlsConsumedNonces).values(...)`. A unique-violation error
+  (PostgreSQL code `23505`) returns `{ ok: false, reason: "replayed" }`.
+- Extracted `validateNonceWindow()` as a pure, side-effect-free function
+  for expiry-window checks (used by both the route and the DB-backed path).
+- Added `pruneExpiredNonces()` — safe to call periodically; removes rows
+  whose `expiry` is >1 hour in the past.
+- Exported `NONCE_RETENTION_SECONDS = 3600` (1-hour retention buffer).
+
+### Route (`web/src/app/api/talos/[id]/transfer/route.ts`)
+
+- `consumeTransferNonce` call is now `await`-ed. Error responses unchanged.
+
+### Tests (`web/tests/transfer-signature.test.ts`)
+
+- **Replay test**: mocks `db.insert` to succeed on the first call and fail
+  with `code 23505` on the second — verifies 200 / 409 split.
+- **Race-condition test**: uses `Promise.all` with the same mocking pattern
+  to prove exactly one of two concurrent requests succeeds.
+- **`validateNonceWindow` tests**: pure-function tests for expired,
+  expiry-too-far, and non-integer expiry inputs.
+
+### Vacuum strategy (documented in migration & library)
+
+DELETE FROM `tls_consumed_nonces` WHERE `expiry` < EXTRACT(EPOCH FROM NOW()) - 3600;
+
+Safe to run periodically (e.g. via `pruneExpiredNonces()` call or pg_cron).
+Rows survive well past the 5-minute max auth lifetime before cleanup.
+
+## Test evidence
+
+```text
+✓ rejects a signed request with a tampered destination
+✓ rejects a signed request with a tampered amount
+✓ rejects a signed request with a tampered nonce
+✓ rejects a signed request with a tampered expiry
+✓ rejects an exact signed request when its nonce is replayed
+✓ rejects an expired signed request
+✓ rejects an authorization outside the five-minute expiry window
+✓ rejects non-canonical and ambiguous request encodings
+✓ validateNonceWindow returns ok for a valid expiry window
+✓ validateNonceWindow returns expired when expiry is in the past
+✓ validateNonceWindow returns expiry-too-far when expiry exceeds window
+✓ validateNonceWindow returns expired for a non-integer expiry
+✓ handles concurrent requests for the same nonce — exactly one succeeds
+```
+
+## Out of scope
+
+Production deployment, live secret changes, or unrelated refactors.

web/drizzle/0015_add_consumed_nonces.sql

@@ -0,0 +1,44 @@
+-- Consumed nonces table for persistent replay protection of signed transfers.
+--
+-- Every consumed transfer nonce is recorded in this table with a UNIQUE
+-- constraint on (talosId, nonce) so the database enforces single-use
+-- semantics across process restarts and concurrent requests.
+--
+-- Vacuum strategy (safe to run periodically, e.g. via pg_cron or a
+-- background request):
+--   DELETE FROM "tls_consumed_nonces"
+--   WHERE "expiry" < EXTRACT(EPOCH FROM NOW()) - 3600;
+-- This removes rows whose original auth window has been closed for at
+-- least one hour, well past the 5-minute max auth lifetime.
+--
+-- Rollback:
+--   DROP TABLE IF EXISTS "tls_consumed_nonces";
+-- Dropping discards the replay history; replays of old nonces become
+-- possible after rollback.
+
+CREATE TABLE IF NOT EXISTS "tls_consumed_nonces" (
+  "id" text PRIMARY KEY NOT NULL,
+  "talosId" text NOT NULL,
+  "nonce" text NOT NULL,
+  "expiry" integer NOT NULL,
+  "consumedAt" timestamp(3) DEFAULT CURRENT_TIMESTAMP NOT NULL
+);
+--> statement-breakpoint
+
+-- Unique constraint enforces single-use per nonce per agent.
+CREATE UNIQUE INDEX IF NOT EXISTS "tls_consumed_nonces_talosId_nonce_key"
+  ON "tls_consumed_nonces" ("talosId", "nonce");
+--> statement-breakpoint
+
+-- Index for the vacuum: prune rows whose expiry has passed + 1h buffer.
+CREATE INDEX IF NOT EXISTS "tls_consumed_nonces_expiry_idx"
+  ON "tls_consumed_nonces" ("expiry");
+--> statement-breakpoint
+
+ALTER TABLE "tls_consumed_nonces" ENABLE ROW LEVEL SECURITY;
+--> statement-breakpoint
+
+-- The server owns writes. Nonces are not sensitive — consume records are
+-- internal replay guards — so postgres role covers all operations.
+CREATE POLICY "postgres_all_consumed_nonces" ON "tls_consumed_nonces"
+  FOR ALL TO postgres USING (true) WITH CHECK (true);

web/drizzle/meta/_journal.json

@@ -106,6 +106,13 @@
                         "when":  1753642800000,
                         "tag":  "0014_add_agent_lifecycle",
                         "breakpoints":  true
+                    },
+                    {
+                        "idx":  15,
+                        "version":  "7",
+                        "when":  1753729200000,
+                        "tag":  "0015_add_consumed_nonces",
+                        "breakpoints":  true
                     }
                 ]
 }

web/src/app/api/talos/[id]/transfer/route.ts

@@ -102,9 +102,10 @@ export async function POST(
     }
 
     // Consume immediately before the first money-moving side effect. The
-    // synchronous guard prevents two requests in this process from using the
-    // same signed nonce concurrently.
-    const nonceResult = consumeTransferNonce(signedPayload, nowSeconds);
+    // database UNIQUE constraint on (talosId, nonce) prevents two concurrent
+    // requests from using the same signed nonce — exactly one INSERT succeeds
+    // and the other fails with a unique-violation error.
+    const nonceResult = await consumeTransferNonce(signedPayload, nowSeconds);
     if (!nonceResult.ok) {
       if (nonceResult.reason === "replayed") {
         return Response.json(

web/src/db/schema.ts

@@ -423,6 +423,37 @@ export const tlsLifecycleEvents = pgTable(
   ],
 );
 
+// ─── Consumed Nonces (replay protection) ────────────────────────────
+//
+// Every signed transfer nonce is persisted here with a UNIQUE constraint on
+// (talosId, nonce) so the database enforces single-use semantics across
+// process restarts and concurrent requests.  Rows are retained for a short
+// window after the nonce expires so delayed replays are still caught, then
+// pruned by a periodic vacuum.
+//
+// expiry — the original transfer-authorization expiry (Unix seconds).
+//          Used by the vacuum to safely remove expired rows without
+//          consulting external state.
+// consumedAt — wall-clock time when the nonce was first consumed.
+//              Present for audit and to bound the vacuum window.
+
+export const tlsConsumedNonces = pgTable(
+  "tls_consumed_nonces",
+  {
+    id: text("id").primaryKey().$defaultFn(() => createId()),
+    talosId: text("talosId").notNull(),
+    nonce: text("nonce").notNull(),
+    expiry: integer("expiry").notNull(),             // original auth expiry (Unix seconds)
+    consumedAt: timestamp("consumedAt", { mode: "date", precision: 3 })
+      .notNull()
+      .defaultNow(),
+  },
+  (t) => [
+    uniqueIndex("tls_consumed_nonces_talosId_nonce_key").on(t.talosId, t.nonce),
+    index("tls_consumed_nonces_expiry_idx").on(t.expiry),
+  ],
+);
+
 // ─── Provisioning Job (durable, compensated workflow) ─────────────
 //
 // One row per durable lifecycle run (activate / retire / recover). Step state

web/src/lib/transfer-signature.ts

@@ -1,4 +1,7 @@
 import { createHmac, timingSafeEqual } from "node:crypto";
+import { lt } from "drizzle-orm";
+import { db } from "@/db";
+import { tlsConsumedNonces } from "@/db/schema";
 
 /**
  * Domain separator for TALOS transfer authorizations.
@@ -11,6 +14,18 @@ export const TRANSFER_SIGNATURE_DOMAIN = "talos.transfer.v1";
 /** A transfer authorization is valid for at most five minutes. */
 export const MAX_TRANSFER_AUTH_LIFETIME_SECONDS = 5 * 60;
 
+/**
+ * Retention window for consumed-nonce rows after their original auth expiry.
+ *
+ * Rows are kept for at least this long past `expiry` so that a delayed replay
+ * (e.g. a retry that arrives moments after the window closed) is still caught.
+ * The vacuum/prune query uses this constant:
+ *
+ *   DELETE FROM tls_consumed_nonces
+ *   WHERE expiry < EXTRACT(EPOCH FROM NOW()) - $NONCE_RETENTION_SECONDS;
+ */
+export const NONCE_RETENTION_SECONDS = 3600; // 1 hour
+
 export interface TransferSignedPayload {
   agent: string;
   destination: string;
@@ -76,44 +91,81 @@ type NonceResult =
   | { ok: true }
   | { ok: false; reason: "expired" | "expiry-too-far" | "replayed" };
 
-/*
- * Process-local replay guard. This intentionally provides the focused replay
- * protection required by this route without introducing a new persistence
- * model. Entries disappear after expiry and are scoped by agent.
- */
-const consumedNonces = new Map<string, number>();
-
-function pruneExpiredNonces(nowSeconds: number): void {
-  for (const [key, expiry] of consumedNonces) {
-    if (expiry <= nowSeconds) consumedNonces.delete(key);
-  }
-}
-
 /**
- * Atomically (within one JavaScript process) consume a verified nonce.
- * This must be called immediately before the first transfer side effect.
+ * Validate the nonce expiry window without side effects.
+ *
+ * Returns `true` when the payload's expiry is within the acceptable window:
+ * not expired, and not beyond `MAX_TRANSFER_AUTH_LIFETIME_SECONDS` from now.
  */
-export function consumeTransferNonce(
-  payload: Pick<TransferSignedPayload, "agent" | "nonce" | "expiry">,
-  nowSeconds = Math.floor(Date.now() / 1000),
+export function validateNonceWindow(
+  payload: Pick<TransferSignedPayload, "expiry">,
+  nowSeconds: number,
 ): NonceResult {
-  pruneExpiredNonces(nowSeconds);
-
   const expiry = Number(payload.expiry);
   if (!Number.isSafeInteger(expiry) || expiry <= nowSeconds) {
     return { ok: false, reason: "expired" };
   }
   if (expiry > nowSeconds + MAX_TRANSFER_AUTH_LIFETIME_SECONDS) {
     return { ok: false, reason: "expiry-too-far" };
   }
+  return { ok: true };
+}
 
-  const key = `${payload.agent}:${payload.nonce}`;
-  if (consumedNonces.has(key)) {
-    return { ok: false, reason: "replayed" };
+/**
+ * Atomically consume a verified nonce via the database.
+ *
+ * Persists the nonce row with a UNIQUE constraint on `(talosId, nonce)`.  When
+ * two concurrent requests race for the same nonce, exactly one INSERT succeeds
+ * and the other fails with a unique-violation — no advisory locks required.
+ *
+ * This replaces the previous in-memory Map approach, providing replay
+ * protection that survives process restarts and scales across replicas.
+ *
+ * Must be called immediately before the first money-moving side effect.
+ */
+export async function consumeTransferNonce(
+  payload: Pick<TransferSignedPayload, "agent" | "nonce" | "expiry">,
+  nowSeconds = Math.floor(Date.now() / 1000),
+): Promise<NonceResult> {
+  // Validate the expiry window first — no need to touch the DB for stale auths.
+  const windowCheck = validateNonceWindow(payload, nowSeconds);
+  if (!windowCheck.ok) {
+    return windowCheck;
   }
 
-  // Map#set is synchronous, so a concurrent request cannot pass this guard in
-  // this process before the current request begins its asynchronous transfer.
-  consumedNonces.set(key, expiry);
-  return { ok: true };
+  try {
+    await db.insert(tlsConsumedNonces).values({
+      talosId: payload.agent,
+      nonce: payload.nonce,
+      expiry: Number(payload.expiry),
+    });
+    return { ok: true };
+  } catch (err: unknown) {
+    // PostgreSQL unique-violation error code 23505 — duplicate nonce detected.
+    if (
+      err &&
+      typeof err === "object" &&
+      "code" in err &&
+      (err as { code: unknown }).code === "23505"
+    ) {
+      return { ok: false, reason: "replayed" };
+    }
+    throw err;
+  }
+}
+
+/**
+ * Prune consumed-nonce rows whose original auth window closed long ago.
+ *
+ * Safe to call periodically (e.g. via a maintenance request or pg_cron).
+ * Removes rows where `expiry` is older than `NONCE_RETENTION_SECONDS` from now.
+ */
+export async function pruneExpiredNonces(): Promise<number> {
+  const cutoff = Math.floor(Date.now() / 1000) - NONCE_RETENTION_SECONDS;
+  const result = await db
+    .delete(tlsConsumedNonces)
+    .where(lt(tlsConsumedNonces.expiry, cutoff));
+
+  // Drizzle delete returns { rowCount } on supported dialects, else undefined.
+  return (result as { rowCount?: number }).rowCount ?? 0;
 }