refactor(gateway): split browser auth hardening paths

2026-03-08 06:54:24 +00:00 · 2026-02-26 01:36:52 +01:00
parent 8f8e46d898
commit 20c2db2103
4 changed files with 240 additions and 89 deletions
--- a/src/gateway/server.auth.browser-hardening.test.ts
+++ b/src/gateway/server.auth.browser-hardening.test.ts
@@ -0,0 +1,155 @@
+import { randomUUID } from "node:crypto";
+import os from "node:os";
+import path from "node:path";
+import { describe, expect, test } from "vitest";
+import { WebSocket } from "ws";
+import {
+  loadOrCreateDeviceIdentity,
+  publicKeyRawBase64UrlFromPem,
+  signDevicePayload,
+} from "../infra/device-identity.js";
+import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
+import { buildDeviceAuthPayload } from "./device-auth.js";
+import {
+  connectReq,
+  installGatewayTestHooks,
+  readConnectChallengeNonce,
+  testState,
+  trackConnectChallengeNonce,
+  withGatewayServer,
+} from "./test-helpers.js";
+
+installGatewayTestHooks({ scope: "suite" });
+
+const TEST_OPERATOR_CLIENT = {
+  id: GATEWAY_CLIENT_NAMES.TEST,
+  version: "1.0.0",
+  platform: "test",
+  mode: GATEWAY_CLIENT_MODES.TEST,
+};
+
+const originForPort = (port: number) => `http://127.0.0.1:${port}`;
+
+const openWs = async (port: number, headers?: Record<string, string>) => {
+  const ws = new WebSocket(`ws://127.0.0.1:${port}`, headers ? { headers } : undefined);
+  trackConnectChallengeNonce(ws);
+  await new Promise<void>((resolve) => ws.once("open", resolve));
+  return ws;
+};
+
+async function createSignedDevice(params: {
+  token: string;
+  scopes: string[];
+  clientId: string;
+  clientMode: string;
+  identityPath?: string;
+  nonce: string;
+  signedAtMs?: number;
+}) {
+  const identity = params.identityPath
+    ? loadOrCreateDeviceIdentity(params.identityPath)
+    : loadOrCreateDeviceIdentity();
+  const signedAtMs = params.signedAtMs ?? Date.now();
+  const payload = buildDeviceAuthPayload({
+    deviceId: identity.deviceId,
+    clientId: params.clientId,
+    clientMode: params.clientMode,
+    role: "operator",
+    scopes: params.scopes,
+    signedAtMs,
+    token: params.token,
+    nonce: params.nonce,
+  });
+  return {
+    identity,
+    device: {
+      id: identity.deviceId,
+      publicKey: publicKeyRawBase64UrlFromPem(identity.publicKeyPem),
+      signature: signDevicePayload(identity.privateKeyPem, payload),
+      signedAt: signedAtMs,
+      nonce: params.nonce,
+    },
+  };
+}
+
+describe("gateway auth browser hardening", () => {
+  test("rejects non-local browser origins for non-control-ui clients", async () => {
+    testState.gatewayAuth = { mode: "token", token: "secret" };
+    await withGatewayServer(async ({ port }) => {
+      const ws = await openWs(port, { origin: "https://attacker.example" });
+      try {
+        const res = await connectReq(ws, {
+          token: "secret",
+          client: TEST_OPERATOR_CLIENT,
+        });
+        expect(res.ok).toBe(false);
+        expect(res.error?.message ?? "").toContain("origin not allowed");
+      } finally {
+        ws.close();
+      }
+    });
+  });
+
+  test("rate-limits browser-origin auth failures on loopback even when loopback exemption is enabled", async () => {
+    testState.gatewayAuth = {
+      mode: "token",
+      token: "secret",
+      rateLimit: { maxAttempts: 1, windowMs: 60_000, lockoutMs: 60_000, exemptLoopback: true },
+    };
+    await withGatewayServer(async ({ port }) => {
+      const firstWs = await openWs(port, { origin: originForPort(port) });
+      try {
+        const first = await connectReq(firstWs, { token: "wrong" });
+        expect(first.ok).toBe(false);
+        expect(first.error?.message ?? "").not.toContain("retry later");
+      } finally {
+        firstWs.close();
+      }
+
+      const secondWs = await openWs(port, { origin: originForPort(port) });
+      try {
+        const second = await connectReq(secondWs, { token: "wrong" });
+        expect(second.ok).toBe(false);
+        expect(second.error?.message ?? "").toContain("retry later");
+      } finally {
+        secondWs.close();
+      }
+    });
+  });
+
+  test("does not silently auto-pair non-control-ui browser clients on loopback", async () => {
+    const { listDevicePairing } = await import("../infra/device-pairing.js");
+    testState.gatewayAuth = { mode: "token", token: "secret" };
+
+    await withGatewayServer(async ({ port }) => {
+      const browserWs = await openWs(port, { origin: originForPort(port) });
+      try {
+        const nonce = await readConnectChallengeNonce(browserWs);
+        expect(typeof nonce).toBe("string");
+        const { identity, device } = await createSignedDevice({
+          token: "secret",
+          scopes: ["operator.admin"],
+          clientId: TEST_OPERATOR_CLIENT.id,
+          clientMode: TEST_OPERATOR_CLIENT.mode,
+          identityPath: path.join(os.tmpdir(), `openclaw-browser-device-${randomUUID()}.json`),
+          nonce: String(nonce ?? ""),
+        });
+        const res = await connectReq(browserWs, {
+          token: "secret",
+          scopes: ["operator.admin"],
+          client: TEST_OPERATOR_CLIENT,
+          device,
+        });
+        expect(res.ok).toBe(false);
+        expect(res.error?.message ?? "").toContain("pairing required");
+
+        const pairing = await listDevicePairing();
+        const pending = pairing.pending.find((entry) => entry.deviceId === identity.deviceId);
+        expect(pending).toBeTruthy();
+        expect(pending?.silent).toBe(false);
+      } finally {
+        browserWs.close();
+      }
+    });
+  });
+});
--- a/src/gateway/server.auth.test.ts
+++ b/src/gateway/server.auth.test.ts
@@ -672,17 +672,6 @@ describe("gateway server auth/connect", () => {
      ws.close();
    });

-    test("rejects non-local browser origins for non-control-ui clients", async () => {
-      const ws = await openWs(port, { origin: "https://attacker.example" });
-      const res = await connectReq(ws, {
-        token: "secret",
-        client: TEST_OPERATOR_CLIENT,
-      });
-      expect(res.ok).toBe(false);
-      expect(res.error?.message ?? "").toContain("origin not allowed");
-      ws.close();
-    });
-
    test("returns control ui hint when token is missing", async () => {
      const ws = await openWs(port, { origin: originForPort(port) });
      const res = await connectReq(ws, {
@@ -712,27 +701,6 @@ describe("gateway server auth/connect", () => {
      );
      ws.close();
    });
-
-    test("rate-limits browser-origin auth failures on loopback even when loopback exemption is enabled", async () => {
-      testState.gatewayAuth = {
-        mode: "token",
-        token: "secret",
-        rateLimit: { maxAttempts: 1, windowMs: 60_000, lockoutMs: 60_000, exemptLoopback: true },
-      };
-      await withGatewayServer(async ({ port }) => {
-        const firstWs = await openWs(port, { origin: originForPort(port) });
-        const first = await connectReq(firstWs, { token: "wrong" });
-        expect(first.ok).toBe(false);
-        expect(first.error?.message ?? "").not.toContain("retry later");
-        firstWs.close();
-
-        const secondWs = await openWs(port, { origin: originForPort(port) });
-        const second = await connectReq(secondWs, { token: "wrong" });
-        expect(second.ok).toBe(false);
-        expect(second.error?.message ?? "").toContain("retry later");
-        secondWs.close();
-      });
-    });
  });

  describe("explicit none auth", () => {
@@ -1246,43 +1214,6 @@ describe("gateway server auth/connect", () => {
    restoreGatewayToken(prevToken);
  });

-  test("does not silently auto-pair non-control-ui browser clients on loopback", async () => {
-    const { listDevicePairing } = await import("../infra/device-pairing.js");
-    const { randomUUID } = await import("node:crypto");
-    const os = await import("node:os");
-    const path = await import("node:path");
-    const { server, ws, port, prevToken } = await startServerWithClient("secret");
-    ws.close();
-
-    const browserWs = await openWs(port, { origin: originForPort(port) });
-    const nonce = await readConnectChallengeNonce(browserWs);
-    const { identity, device } = await createSignedDevice({
-      token: "secret",
-      scopes: ["operator.admin"],
-      clientId: TEST_OPERATOR_CLIENT.id,
-      clientMode: TEST_OPERATOR_CLIENT.mode,
-      identityPath: path.join(os.tmpdir(), `openclaw-browser-device-${randomUUID()}.json`),
-      nonce,
-    });
-    const res = await connectReq(browserWs, {
-      token: "secret",
-      scopes: ["operator.admin"],
-      client: TEST_OPERATOR_CLIENT,
-      device,
-    });
-    expect(res.ok).toBe(false);
-    expect(res.error?.message ?? "").toContain("pairing required");
-
-    const pairing = await listDevicePairing();
-    const pending = pairing.pending.find((entry) => entry.deviceId === identity.deviceId);
-    expect(pending).toBeTruthy();
-    expect(pending?.silent).toBe(false);
-
-    browserWs.close();
-    await server.close();
-    restoreGatewayToken(prevToken);
-  });
-
  test("merges remote node/operator pairing requests for the same unpaired device", async () => {
    const { mkdtemp } = await import("node:fs/promises");
    const { tmpdir } = await import("node:os");
--- a/src/gateway/server.impl.ts
+++ b/src/gateway/server.impl.ts
@@ -110,6 +110,21 @@ const logWsControl = log.child("ws");
 const gatewayRuntime = runtimeForLogger(log);
 const canvasRuntime = runtimeForLogger(logCanvas);

+type AuthRateLimitConfig = Parameters<typeof createAuthRateLimiter>[0];
+
+function createGatewayAuthRateLimiters(rateLimitConfig: AuthRateLimitConfig | undefined): {
+  rateLimiter?: AuthRateLimiter;
+  browserRateLimiter: AuthRateLimiter;
+} {
+  const rateLimiter = rateLimitConfig ? createAuthRateLimiter(rateLimitConfig) : undefined;
+  // Browser-origin WS auth attempts always use loopback-non-exempt throttling.
+  const browserRateLimiter = createAuthRateLimiter({
+    ...rateLimitConfig,
+    exemptLoopback: false,
+  });
+  return { rateLimiter, browserRateLimiter };
+}
+
 export type GatewayServer = {
  close: (opts?: { reason?: string; restartExpectedMs?: number | null }) => Promise<void>;
 };
@@ -311,16 +326,10 @@ export async function startGatewayServer(
  let hooksConfig = runtimeConfig.hooksConfig;
  const canvasHostEnabled = runtimeConfig.canvasHostEnabled;

-  // Create auth rate limiter only when explicitly configured.
+  // Create auth rate limiters used by connect/auth flows.
  const rateLimitConfig = cfgAtStart.gateway?.auth?.rateLimit;
-  const authRateLimiter: AuthRateLimiter | undefined = rateLimitConfig
-    ? createAuthRateLimiter(rateLimitConfig)
-    : undefined;
-  // Always keep a browser-origin fallback limiter for WS auth attempts.
-  const browserAuthRateLimiter: AuthRateLimiter = createAuthRateLimiter({
-    ...rateLimitConfig,
-    exemptLoopback: false,
-  });
+  const { rateLimiter: authRateLimiter, browserRateLimiter: browserAuthRateLimiter } =
+    createGatewayAuthRateLimiters(rateLimitConfig);

  let controlUiRootState: ControlUiRootState | undefined;
  if (controlUiRootOverride) {
--- a/src/gateway/server/ws-connection/message-handler.ts
+++ b/src/gateway/server/ws-connection/message-handler.ts
@@ -83,6 +83,52 @@ import { isUnauthorizedRoleError, UnauthorizedFloodGuard } from "./unauthorized-
 type SubsystemLogger = ReturnType<typeof createSubsystemLogger>;

 const DEVICE_SIGNATURE_SKEW_MS = 2 * 60 * 1000;
+const BROWSER_ORIGIN_LOOPBACK_RATE_LIMIT_IP = "198.18.0.1";
+
+type HandshakeBrowserSecurityContext = {
+  hasBrowserOriginHeader: boolean;
+  enforceOriginCheckForAnyClient: boolean;
+  rateLimitClientIp: string | undefined;
+  authRateLimiter?: AuthRateLimiter;
+};
+
+function resolveHandshakeBrowserSecurityContext(params: {
+  requestOrigin?: string;
+  hasProxyHeaders: boolean;
+  clientIp: string | undefined;
+  rateLimiter?: AuthRateLimiter;
+  browserRateLimiter?: AuthRateLimiter;
+}): HandshakeBrowserSecurityContext {
+  const hasBrowserOriginHeader = Boolean(
+    params.requestOrigin && params.requestOrigin.trim() !== "",
+  );
+  return {
+    hasBrowserOriginHeader,
+    enforceOriginCheckForAnyClient: hasBrowserOriginHeader && !params.hasProxyHeaders,
+    rateLimitClientIp:
+      hasBrowserOriginHeader && isLoopbackAddress(params.clientIp)
+        ? BROWSER_ORIGIN_LOOPBACK_RATE_LIMIT_IP
+        : params.clientIp,
+    authRateLimiter:
+      hasBrowserOriginHeader && params.browserRateLimiter
+        ? params.browserRateLimiter
+        : params.rateLimiter,
+  };
+}
+
+function shouldAllowSilentLocalPairing(params: {
+  isLocalClient: boolean;
+  hasBrowserOriginHeader: boolean;
+  isControlUi: boolean;
+  isWebchat: boolean;
+  reason: "not-paired" | "role-upgrade" | "scope-upgrade";
+}): boolean {
+  return (
+    params.isLocalClient &&
+    (!params.hasBrowserOriginHeader || params.isControlUi || params.isWebchat) &&
+    (params.reason === "not-paired" || params.reason === "scope-upgrade")
+  );
+}

 export function attachGatewayWsMessageHandler(params: {
  socket: WebSocket;
@@ -195,12 +241,19 @@ export function attachGatewayWsMessageHandler(params: {

  const isWebchatConnect = (p: ConnectParams | null | undefined) => isWebchatClient(p?.client);
  const unauthorizedFloodGuard = new UnauthorizedFloodGuard();
-  const hasBrowserOriginHeader = Boolean(requestOrigin && requestOrigin.trim() !== "");
-  const enforceBrowserOriginForAnyClient = hasBrowserOriginHeader && !hasProxyHeaders;
-  const browserRateLimitClientIp =
-    hasBrowserOriginHeader && isLoopbackAddress(clientIp) ? "198.18.0.1" : clientIp;
-  const authRateLimiter =
-    hasBrowserOriginHeader && browserRateLimiter ? browserRateLimiter : rateLimiter;
+  const browserSecurity = resolveHandshakeBrowserSecurityContext({
+    requestOrigin,
+    hasProxyHeaders,
+    clientIp,
+    rateLimiter,
+    browserRateLimiter,
+  });
+  const {
+    hasBrowserOriginHeader,
+    enforceOriginCheckForAnyClient,
+    rateLimitClientIp: browserRateLimitClientIp,
+    authRateLimiter,
+  } = browserSecurity;

  socket.on("message", async (data) => {
    if (isClosed()) {
@@ -338,7 +391,7 @@ export function attachGatewayWsMessageHandler(params: {

        const isControlUi = connectParams.client.id === GATEWAY_CLIENT_IDS.CONTROL_UI;
        const isWebchat = isWebchatConnect(connectParams);
-        if (enforceBrowserOriginForAnyClient || isControlUi || isWebchat) {
+        if (enforceOriginCheckForAnyClient || isControlUi || isWebchat) {
          const originCheck = checkBrowserOrigin({
            requestHost,
            origin: requestOrigin,
@@ -622,10 +675,13 @@ export function attachGatewayWsMessageHandler(params: {
          const requirePairing = async (
            reason: "not-paired" | "role-upgrade" | "scope-upgrade",
          ) => {
-            const allowSilentLocalPairing =
-              isLocalClient &&
-              (!hasBrowserOriginHeader || isControlUi || isWebchat) &&
-              (reason === "not-paired" || reason === "scope-upgrade");
+            const allowSilentLocalPairing = shouldAllowSilentLocalPairing({
+              isLocalClient,
+              hasBrowserOriginHeader,
+              isControlUi,
+              isWebchat,
+              reason,
+            });
            const pairing = await requestDevicePairing({
              deviceId: device.id,
              publicKey: devicePublicKey,