feat(api): short-lived HMAC stream tickets for WS/SSE auth (#425 follow-up)

packages/api/.env.example

@@ -63,3 +63,24 @@ FLY_API_KEY=
 BATCH_LABEL=batch-auto-dev
 # Whether to group issues by milestone when processing batches
 BATCH_BY_MILESTONE=false
+
+# API Authentication (ENG-1671)
+# Accepted API keys for /api/* (CSV takes precedence over the single-key form)
+MULTIPLAI_API_KEYS=
+MULTIPLAI_API_KEY=
+
+# Stream tickets (follow-up to PR #425)
+# SSE/WebSocket clients that cannot set an Authorization header mint a
+# short-lived, single-purpose ticket via `POST /api/auth/ticket` (header-authed)
+# and pass it as `?ticket=` on /api/logs/stream or /api/ws/tasks.
+# Ticket lifetime in ms (default 60000). Keep short — a leaked URL is only
+# valid for this long.
+MULTIPLAI_TICKET_TTL_MS=60000
+# Optional independent signing secret. If unset, the ticket key is DERIVED from
+# MULTIPLAI_API_KEYS/MULTIPLAI_API_KEY, so rotating the API keys invalidates
+# outstanding tickets. Set this to rotate tickets independently of API keys.
+MULTIPLAI_TICKET_SECRET=
+# Legacy compat: accept `?token=<raw API key>` on the stream paths (PR #425).
+# Default OFF (0). A raw reusable key in the URL leaks through logs/Referer/
+# history — only set to 1 for a short migration window.
+ALLOW_QUERY_TOKEN=0

packages/api/src/core/auth.test.ts

@@ -8,6 +8,7 @@ import {
   isValidToken,
   resetAuthWarningForTests,
 } from "./auth";
+import { issueTicket, clearUsedTicketsForTests } from "./ticket";
 
 const ORIGINAL_ENV = {
   MULTIPLAI_API_KEYS: process.env.MULTIPLAI_API_KEYS,
@@ -46,6 +47,9 @@ function req(path: string, headers: Record<string, string> = {}): Request {
 
 beforeEach(() => {
   resetAuthWarningForTests();
+  delete process.env.ALLOW_QUERY_TOKEN;
+  delete process.env.MULTIPLAI_TICKET_SECRET;
+  clearUsedTicketsForTests();
   setEnv({ MULTIPLAI_API_KEY: "secret-key-1" });
 });
 
@@ -177,32 +181,101 @@ describe("authMiddleware", () => {
     }
   });
 
-  test("SSE accepts ?token= query param", () => {
+  // Follow-up to PR #425: raw `?token=<API key>` in the query string is now
+  // OFF by default (it leaks a reusable key through logs/Referer/history).
+  // Stream paths authenticate with a short-lived `?ticket=` instead; raw token
+  // is honored only under the ALLOW_QUERY_TOKEN=1 migration flag.
+  test("SSE rejects raw ?token= by default (ALLOW_QUERY_TOKEN off)", () => {
+    delete process.env.ALLOW_QUERY_TOKEN;
     const res = authMiddleware(req("/api/logs/stream?token=secret-key-1"));
-    expect(res).toBeNull();
+    expect(res!.status).toBe(401);
   });
 
-  test("SSE rejects bad ?token=", () => {
-    const res = authMiddleware(req("/api/logs/stream?token=bad"));
-    expect(res!.status).toBe(401);
+  test("SSE accepts raw ?token= only when ALLOW_QUERY_TOKEN=1", () => {
+    process.env.ALLOW_QUERY_TOKEN = "1";
+    try {
+      const ok = authMiddleware(req("/api/logs/stream?token=secret-key-1"));
+      expect(ok).toBeNull();
+      const bad = authMiddleware(req("/api/logs/stream?token=nope"));
+      expect(bad!.status).toBe(401);
+    } finally {
+      delete process.env.ALLOW_QUERY_TOKEN;
+    }
+  });
+
+  test("SSE rejects bad ?token= even with ALLOW_QUERY_TOKEN=1", () => {
+    process.env.ALLOW_QUERY_TOKEN = "1";
+    try {
+      const res = authMiddleware(req("/api/logs/stream?token=bad"));
+      expect(res!.status).toBe(401);
+    } finally {
+      delete process.env.ALLOW_QUERY_TOKEN;
+    }
   });
 
-  test("authMiddleware accepts ?token= for WS path pattern (middleware-only; see index.test.ts for the real upgrade path)", () => {
-    // NOTE: this only proves authMiddleware() itself accepts a valid
-    // ?token= for /api/ws/tasks. It does NOT prove the WebSocket upgrade
-    // is actually authenticated in production — index.ts's Bun.serve
-    // fetch handler calls server.upgrade() for this path, which bypasses
-    // handleRequest()/authMiddleware() entirely unless index.ts itself
-    // invokes authMiddleware() first (see index.ts + index.test.ts).
+  test("WS rejects raw ?token= by default (ALLOW_QUERY_TOKEN off)", () => {
+    delete process.env.ALLOW_QUERY_TOKEN;
     const res = authMiddleware(req("/api/ws/tasks?token=secret-key-1"));
+    expect(res!.status).toBe(401);
+  });
+
+  test("SSE accepts a valid ?ticket= (purpose sse)", () => {
+    clearUsedTicketsForTests();
+    const { ticket } = issueTicket("sse")!;
+    const res = authMiddleware(req(`/api/logs/stream?ticket=${ticket}`));
+    expect(res).toBeNull();
+  });
+
+  test("WS accepts a valid ?ticket= (purpose ws)", () => {
+    clearUsedTicketsForTests();
+    const { ticket } = issueTicket("ws")!;
+    const res = authMiddleware(req(`/api/ws/tasks?ticket=${ticket}`));
     expect(res).toBeNull();
   });
 
-  test("?token= is NOT accepted on regular API paths", () => {
-    const res = authMiddleware(req("/api/tasks?token=secret-key-1"));
+  test("a ticket minted for one stream purpose is rejected on the other path", () => {
+    clearUsedTicketsForTests();
+    const { ticket } = issueTicket("ws")!;
+    // ws-purpose ticket used on the SSE path -> bad_purpose -> 401
+    const res = authMiddleware(req(`/api/logs/stream?ticket=${ticket}`));
+    expect(res!.status).toBe(401);
+  });
+
+  test("a tampered ticket is rejected (bad signature)", () => {
+    clearUsedTicketsForTests();
+    const { ticket } = issueTicket("ws")!;
+    const tampered = ticket.slice(0, -2) + (ticket.endsWith("a") ? "b" : "a");
+    const res = authMiddleware(req(`/api/ws/tasks?ticket=${tampered}`));
+    expect(res!.status).toBe(401);
+  });
+
+  test("a single-use ticket cannot be replayed", () => {
+    clearUsedTicketsForTests();
+    const { ticket } = issueTicket("ws")!;
+    expect(authMiddleware(req(`/api/ws/tasks?ticket=${ticket}`))).toBeNull();
+    // Second use of the same ticket is rejected by the jti single-use cache.
+    expect(
+      authMiddleware(req(`/api/ws/tasks?ticket=${ticket}`))!.status,
+    ).toBe(401);
+  });
+
+  test("?ticket= is NOT accepted on regular API paths", () => {
+    clearUsedTicketsForTests();
+    const { ticket } = issueTicket("ws")!;
+    const res = authMiddleware(req(`/api/tasks?ticket=${ticket}`));
     expect(res!.status).toBe(401);
   });
 
+  test("?token= is NOT accepted on regular API paths (even with ALLOW_QUERY_TOKEN=1)", () => {
+    process.env.ALLOW_QUERY_TOKEN = "1";
+    try {
+      const res = authMiddleware(req("/api/tasks?token=secret-key-1"));
+      expect(res!.status).toBe(401);
+    } finally {
+      delete process.env.ALLOW_QUERY_TOKEN;
+    }
+  });
+
   test("rejects Authorization header with scheme but no token", () => {
     const res = authMiddleware(req("/api/tasks", { authorization: "Bearer" }));
     expect(res!.status).toBe(401);

packages/api/src/core/auth.ts

@@ -21,12 +21,31 @@
  */
 
 import { createHash, timingSafeEqual } from "node:crypto";
+import { validateTicket, type TicketPurpose } from "./ticket";
 
 /** Paths under /api/ that never require authentication */
 const PUBLIC_API_PATHS = new Set(["/api/health"]);
 
-/** Paths that may authenticate via ?token= query param (SSE / WebSocket) */
-const QUERY_TOKEN_PATHS = new Set(["/api/logs/stream", "/api/ws/tasks"]);
+/**
+ * Paths that may authenticate via a short-lived `?ticket=` query param, and
+ * the single ticket purpose each accepts. SSE/WebSocket clients (EventSource,
+ * the browser WebSocket API) cannot set an Authorization header, so they mint
+ * a ticket via POST /api/auth/ticket (header-authed) and pass it here.
+ */
+const QUERY_TICKET_PATHS = new Map<string, TicketPurpose>([
+  ["/api/logs/stream", "sse"],
+  ["/api/ws/tasks", "ws"],
+]);
+
+/**
+ * Whether the legacy `?token=<raw API key>` query-string auth from PR #425 is
+ * still accepted on the stream paths. Default OFF: a raw, long-lived API key in
+ * a URL leaks through logs/Referer/history. Set ALLOW_QUERY_TOKEN=1 only for a
+ * transitional window while clients migrate to `?ticket=`.
+ */
+export function isRawQueryTokenAllowed(): boolean {
+  return process.env.ALLOW_QUERY_TOKEN === "1";
+}
 
 let warnedNoKeys = false;
 
@@ -150,24 +169,42 @@ export function authMiddleware(req: Request): Response | null {
     return null;
   }
 
-  let token = extractBearerToken(req);
-
-  // SSE / WebSocket clients cannot always set headers; accept ?token=.
-  // KNOWN RISK (tracked, not fully resolved by this PR): a query string
-  // is commonly captured in reverse-proxy/access logs, APM tools, and
-  // browser history, which can leak this reusable API key outside the
-  // Authorization header's usual handling. We intentionally never log
-  // the full request URL with query string for QUERY_TOKEN_PATHS (see
-  // callers) to reduce exposure. Follow-up: issue a short-lived,
-  // scope-limited token for the SSE/WS handshake instead of accepting
-  // the primary API key verbatim in the URL (tracked separately).
-  if (!token && QUERY_TOKEN_PATHS.has(path)) {
-    token = url.searchParams.get("token");
+  const token = extractBearerToken(req);
+
+  // Header auth always wins and works on every /api/* path.
+  if (token && isValidToken(token, keys)) {
+    return null;
   }
 
-  if (!token || !isValidToken(token, keys)) {
-    return unauthorizedResponse();
+  // SSE / WebSocket clients cannot set an Authorization header. On the stream
+  // paths they authenticate with a short-lived, single-purpose `?ticket=`
+  // (follow-up to PR #425): the ticket is HMAC-signed and expires in ~60s, so
+  // a leaked URL is worthless almost immediately and cannot be replayed
+  // against another route. The ticket is validated for the exact purpose bound
+  // to this path (ws vs sse), so a ticket minted for one stream cannot be used
+  // on the other.
+  const ticketPurpose = QUERY_TICKET_PATHS.get(path);
+  if (ticketPurpose) {
+    const ticket = url.searchParams.get("ticket");
+    if (ticket) {
+      const result = validateTicket(ticket, ticketPurpose, { markUsed: true });
+      if (result.valid) {
+        return null;
+      }
+      // Fall through to 401 on an invalid/expired/replayed ticket.
+    }
+
+    // Legacy compat: `?token=<raw API key>` (PR #425) is only honored when the
+    // operator explicitly opts in via ALLOW_QUERY_TOKEN=1 during migration.
+    // Default OFF because a raw reusable API key in the URL leaks through
+    // access logs, APM, Referer headers, and browser history.
+    if (isRawQueryTokenAllowed()) {
+      const rawToken = url.searchParams.get("token");
+      if (rawToken && isValidToken(rawToken, keys)) {
+        return null;
+      }
+    }
   }
 
-  return null;
+  return unauthorizedResponse();
 }