feat(auth): harden OAuth state generation and callback validation (closes #1562)

generateOAuthState() now throws when crypto.getRandomValues is unavailable
instead of silently degrading to a non-cryptographic Math.random() fallback —
OAuth state is a CSRF token and must be unpredictable.

The web /oauth/callback handler now rejects a returned `state` param that does
not parse to the expected 64-char-hex authId shape (a forgery indicator),
surfacing a clear error toast instead of proceeding with an undefined session.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

clients/web/src/App.test.tsx

@@ -1589,3 +1589,56 @@ describe("App history pin/replay", () => {
     );
   });
 });
+
+// The `/oauth/callback` handler must reject a returned `state` that does not
+// parse to the expected 64-char-hex authId shape (a forgery indicator) instead
+// of silently proceeding. See #1562.
+describe("App OAuth callback state validation", () => {
+  const originalUrl = window.location.href;
+
+  beforeEach(() => {
+    notificationsMock.show.mockClear();
+    window.sessionStorage.clear();
+  });
+
+  afterEach(() => {
+    window.history.replaceState({}, "", originalUrl);
+  });
+
+  it("rejects an unparseable state param with a clear error toast", async () => {
+    window.history.replaceState(
+      {},
+      "",
+      "/oauth/callback?code=abc123&state=not-a-valid-state",
+    );
+
+    renderWithMantine(<App />);
+
+    await waitFor(() =>
+      expect(notificationsMock.show).toHaveBeenCalledWith(
+        expect.objectContaining({
+          title: "OAuth callback rejected",
+          color: "red",
+        }),
+      ),
+    );
+  });
+
+  it("does not reject when the state param parses to a valid authId", async () => {
+    // A well-formed 64-char-hex state is accepted; the handler proceeds past
+    // the rejection guard (it fails later for other reasons in this harness,
+    // but never shows the 'OAuth callback rejected' toast).
+    window.history.replaceState(
+      {},
+      "",
+      `/oauth/callback?code=abc123&state=${"a".repeat(64)}`,
+    );
+
+    renderWithMantine(<App />);
+
+    await waitFor(() => expect(notificationsMock.show).toHaveBeenCalled());
+    expect(notificationsMock.show).not.toHaveBeenCalledWith(
+      expect.objectContaining({ title: "OAuth callback rejected" }),
+    );
+  });
+});

clients/web/src/App.tsx

@@ -1472,10 +1472,15 @@ function App() {
     // session key the pre-redirect page saved the fetch log under, so the
     // rebuilt client can restore those `auth` entries. Read it before the
     // URL is cleared below.
+    //
+    // CSRF hardening: a `state` that is present but does not parse to our
+    // expected 64-char-hex authId shape is a forgery indicator — the value
+    // did not originate from `generateOAuthState`. Reject the callback with a
+    // clear error instead of silently proceeding with an undefined sessionId.
     const stateParam = new URLSearchParams(window.location.search).get("state");
-    const sessionId = stateParam
-      ? (parseOAuthState(stateParam)?.authId ?? undefined)
-      : undefined;
+    const parsedState = stateParam ? parseOAuthState(stateParam) : null;
+    const stateRejected = stateParam !== null && parsedState === null;
+    const sessionId = parsedState?.authId ?? undefined;
     const pendingId =
       window.sessionStorage.getItem(OAUTH_PENDING_SERVER_KEY) ?? undefined;
     window.sessionStorage.removeItem(OAUTH_PENDING_SERVER_KEY);
@@ -1484,6 +1489,16 @@ function App() {
     // the (now single-use) authorization code through the exchange again.
     window.history.replaceState({}, "", "/");
 
+    if (stateRejected) {
+      notifications.show({
+        title: "OAuth callback rejected",
+        message:
+          "OAuth callback carried an unrecognized state parameter; rejecting to prevent a cross-site request. Please try connecting again.",
+        color: "red",
+      });
+      return;
+    }
+
     if (!params.successful) {
       notifications.show({
         title: "OAuth authorization failed",

clients/web/src/test/core/auth/utils.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect } from "vitest";
+import { describe, it, expect, afterEach } from "vitest";
 import {
   parseHttpUrl,
   parseOAuthCallbackParams,
@@ -51,6 +51,17 @@ describe("parseOAuthCallbackParams", () => {
 });
 
 describe("generateOAuthState", () => {
+  const originalCrypto = globalThis.crypto;
+
+  afterEach(() => {
+    // Restore the real WebCrypto after tests that stub/remove it.
+    Object.defineProperty(globalThis, "crypto", {
+      configurable: true,
+      writable: true,
+      value: originalCrypto,
+    });
+  });
+
   it("should generate 64-char hex state", () => {
     const state = generateOAuthState();
     expect(state).toMatch(/^[a-f0-9]{64}$/i);
@@ -61,6 +72,31 @@ describe("generateOAuthState", () => {
     const s2 = generateOAuthState();
     expect(s1).not.toBe(s2);
   });
+
+  it("throws when the crypto global is entirely absent (no silent Math.random fallback)", () => {
+    // Simulate an exotic runtime with no WebCrypto at all. Rather than minting
+    // a guessable CSRF token, generateOAuthState must fail loudly.
+    Object.defineProperty(globalThis, "crypto", {
+      configurable: true,
+      writable: true,
+      value: undefined,
+    });
+    expect(() => generateOAuthState()).toThrow(
+      /crypto\.getRandomValues is not available/,
+    );
+  });
+
+  it("throws when crypto.getRandomValues is missing", () => {
+    // crypto exists but lacks getRandomValues — still refuse to degrade.
+    Object.defineProperty(globalThis, "crypto", {
+      configurable: true,
+      writable: true,
+      value: {},
+    });
+    expect(() => generateOAuthState()).toThrow(
+      /crypto\.getRandomValues is not available/,
+    );
+  });
 });
 
 describe("parseOAuthState", () => {