test: collapse gateway node authz hotspots

2026-05-06 05:30:42 +00:00 · 2026-04-17 22:54:19 +01:00
parent ca34c7cd7b
commit 52b8e318bd
2 changed files with 82 additions and 100 deletions
--- a/src/gateway/server.node-pairing-authz.test.ts
+++ b/src/gateway/server.node-pairing-authz.test.ts
@@ -1,6 +1,11 @@
 import { describe, expect, test } from "vitest";
 import { WebSocket } from "ws";
-import { approveNodePairing, listNodePairing, requestNodePairing } from "../infra/node-pairing.js";
+import {
+  approveNodePairing,
+  getPairedNode,
+  listNodePairing,
+  requestNodePairing,
+} from "../infra/node-pairing.js";
 import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
 import {
  issueOperatorToken,
@@ -40,13 +45,15 @@ async function connectNodeClient(params: {
 }

 async function expectPairingApprovalRejected(params: {
+  started: Awaited<ReturnType<typeof startServerWithClient>>;
+  nodeId: string;
  approverName: string;
  tokenScopes: string[];
  connectedScopes: string[];
  requestCommands?: string[];
  expectedMessage: string;
 }) {
-  const started = await startServerWithClient("secret");
+  const { started } = params;
  const approver = await issueOperatorToken({
    name: params.approverName,
    approvedScopes: ["operator.admin"],
@@ -58,7 +65,7 @@ async function expectPairingApprovalRejected(params: {
  let pairingWs: WebSocket | undefined;
  try {
    const request = await requestNodePairing({
-      nodeId: "node-approve-target",
+      nodeId: params.nodeId,
      platform: "darwin",
      ...(params.requestCommands ? { commands: params.requestCommands } : {}),
    });
@@ -77,14 +84,9 @@ async function expectPairingApprovalRejected(params: {
    expect(approve.ok).toBe(false);
    expect(approve.error?.message).toBe(params.expectedMessage);

-    await expect(
-      import("../infra/node-pairing.js").then((m) => m.getPairedNode("node-approve-target")),
-    ).resolves.toBeNull();
+    await expect(getPairedNode(params.nodeId)).resolves.toBeNull();
  } finally {
    pairingWs?.close();
-    started.ws.close();
-    await started.server.close();
-    started.envSnapshot.restore();
  }
 }

@@ -182,38 +184,38 @@ async function expectRePairingRequest(params: {
 }

 describe("gateway node pairing authorization", () => {
-  test("requires operator.admin for exec-capable node pairing approvals", async () => {
-    await expectPairingApprovalRejected({
-      approverName: "node-pair-approve-pairing-only",
-      tokenScopes: ["operator.pairing"],
-      connectedScopes: ["operator.pairing"],
-      requestCommands: ["system.run"],
-      expectedMessage: "missing scope: operator.admin",
-    });
-  });
-
-  test("requires operator.pairing before node pairing approvals", async () => {
-    await expectPairingApprovalRejected({
-      approverName: "node-pair-approve-attacker",
-      tokenScopes: ["operator.write"],
-      connectedScopes: ["operator.write"],
-      requestCommands: ["system.run"],
-      expectedMessage: "missing scope: operator.pairing",
-    });
-  });
-
-  test("allows pairing-only operators to approve commandless node requests", async () => {
+  test("enforces node pairing approval scopes", async () => {
    const started = await startServerWithClient("secret");
-    const approver = await issueOperatorToken({
-      name: "node-pair-approve-commandless",
-      approvedScopes: ["operator.admin"],
-      tokenScopes: ["operator.pairing"],
-      clientId: GATEWAY_CLIENT_NAMES.TEST,
-      clientMode: GATEWAY_CLIENT_MODES.TEST,
-    });
-
    let pairingWs: WebSocket | undefined;
    try {
+      await expectPairingApprovalRejected({
+        started,
+        nodeId: "node-approve-reject-admin",
+        approverName: "node-pair-approve-pairing-only",
+        tokenScopes: ["operator.pairing"],
+        connectedScopes: ["operator.pairing"],
+        requestCommands: ["system.run"],
+        expectedMessage: "missing scope: operator.admin",
+      });
+
+      await expectPairingApprovalRejected({
+        started,
+        nodeId: "node-approve-reject-pairing",
+        approverName: "node-pair-approve-attacker",
+        tokenScopes: ["operator.write"],
+        connectedScopes: ["operator.write"],
+        requestCommands: ["system.run"],
+        expectedMessage: "missing scope: operator.pairing",
+      });
+
+      const approver = await issueOperatorToken({
+        name: "node-pair-approve-commandless",
+        approvedScopes: ["operator.admin"],
+        tokenScopes: ["operator.pairing"],
+        clientId: GATEWAY_CLIENT_NAMES.TEST,
+        clientMode: GATEWAY_CLIENT_MODES.TEST,
+      });
+
      const request = await requestNodePairing({
        nodeId: "node-approve-target",
        platform: "darwin",
@@ -237,9 +239,7 @@ describe("gateway node pairing authorization", () => {
      expect(approve.payload?.requestId).toBe(request.request.requestId);
      expect(approve.payload?.node?.nodeId).toBe("node-approve-target");

-      await expect(
-        import("../infra/node-pairing.js").then((m) => m.getPairedNode("node-approve-target")),
-      ).resolves.toEqual(
+      await expect(getPairedNode("node-approve-target")).resolves.toEqual(
        expect.objectContaining({
          nodeId: "node-approve-target",
        }),
--- a/src/gateway/server.roles-allowlist-update.test.ts
+++ b/src/gateway/server.roles-allowlist-update.test.ts
@@ -4,6 +4,9 @@ import path from "node:path";
 import { describe, expect, test, vi } from "vitest";
 import { WebSocket } from "ws";
 import type { DeviceIdentity } from "../infra/device-identity.js";
+import { loadOrCreateDeviceIdentity } from "../infra/device-identity.js";
+import { approveDevicePairing, listDevicePairing } from "../infra/device-pairing.js";
+import { approveNodePairing, requestNodePairing } from "../infra/node-pairing.js";
 import { resolveRestartSentinelPath } from "../infra/restart-sentinel.js";
 import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
 import type { GatewayClient } from "./client.js";
@@ -68,7 +71,6 @@ const connectNodeClient = async (params: {
 };

 const approveAllPendingPairings = async () => {
-  const { approveDevicePairing, listDevicePairing } = await import("../infra/device-pairing.js");
  const list = await listDevicePairing();
  for (const pending of list.pending) {
    await approveDevicePairing(pending.requestId, {
@@ -119,7 +121,6 @@ const connectNodeClientWithNodePairing = async (

  await provisionalClient.stopAndWait();

-  const { approveNodePairing, requestNodePairing } = await import("../infra/node-pairing.js");
  const request = await requestNodePairing({
    nodeId,
    displayName: params.displayName,
@@ -271,7 +272,6 @@ describe("gateway update.run", () => {

 describe("gateway node command allowlist", () => {
  test("enforces command allowlists across node clients", async () => {
-    const { loadOrCreateDeviceIdentity } = await import("../infra/device-identity.js");
    const waitForConnectedCount = async (count: number) => {
      await expect
        .poll(async () => {
@@ -441,7 +441,6 @@ describe("gateway node command allowlist", () => {
  });

  test("records only allowlisted commands in pending node pairing requests", async () => {
-    const { loadOrCreateDeviceIdentity } = await import("../infra/device-identity.js");
    const deviceIdentityPath = path.join(
      os.tmpdir(),
      `openclaw-allowlisted-pending-${Date.now()}-${Math.random().toString(36).slice(2)}.json`,
@@ -481,7 +480,6 @@ describe("gateway node command allowlist", () => {
  });

  test("rejects reconnect metadata spoof for paired node devices", async () => {
-    const { loadOrCreateDeviceIdentity } = await import("../infra/device-identity.js");
    const deviceIdentityPath = path.join(
      os.tmpdir(),
      `openclaw-spoof-test-device-${Date.now()}-${Math.random().toString(36).slice(2)}.json`,
@@ -528,65 +526,49 @@ describe("gateway node command allowlist", () => {
  });

  test("filters system.run for confusable iOS metadata at connect time", async () => {
-    const { loadOrCreateDeviceIdentity } = await import("../infra/device-identity.js");
-    const cases = [
-      {
-        label: "dotted-i-platform",
-        platform: "İOS",
-        deviceFamily: "iPhone",
-      },
-      {
-        label: "greek-omicron-family",
+    const deviceIdentityPath = path.join(
+      os.tmpdir(),
+      `openclaw-confusable-node-greek-omicron-${Date.now()}-${Math.random().toString(36).slice(2)}.json`,
+    );
+    const deviceIdentity = loadOrCreateDeviceIdentity(deviceIdentityPath);
+    const displayName = "node-greek-omicron-family";
+
+    let client: GatewayClient | undefined;
+    try {
+      client = await connectNodeClientWithNodePairing({
+        port,
+        commands: ["system.run", "canvas.snapshot"],
        platform: "ios",
        deviceFamily: "iPhοne",
-      },
-    ] as const;
+        instanceId: displayName,
+        displayName,
+        deviceIdentity,
+      });

-    for (const testCase of cases) {
-      const deviceIdentityPath = path.join(
-        os.tmpdir(),
-        `openclaw-confusable-node-${testCase.label}-${Date.now()}-${Math.random().toString(36).slice(2)}.json`,
-      );
-      const deviceIdentity = loadOrCreateDeviceIdentity(deviceIdentityPath);
-      const displayName = `node-${testCase.label}`;
+      await expect
+        .poll(
+          async () => {
+            const node = await findConnectedNodeByDisplayName(displayName);
+            return node?.commands?.toSorted() ?? [];
+          },
+          { timeout: 2_000, interval: 10 },
+        )
+        .toEqual(["canvas.snapshot"]);

-      let client: GatewayClient | undefined;
-      try {
-        client = await connectNodeClientWithNodePairing({
-          port,
-          commands: ["system.run", "canvas.snapshot"],
-          platform: testCase.platform,
-          deviceFamily: testCase.deviceFamily,
-          instanceId: displayName,
-          displayName,
-          deviceIdentity,
-        });
+      const node = await findConnectedNodeByDisplayName(displayName);
+      const nodeId = node?.nodeId ?? "";
+      expect(nodeId).toBeTruthy();

-        await expect
-          .poll(
-            async () => {
-              const node = await findConnectedNodeByDisplayName(displayName);
-              return node?.commands?.toSorted() ?? [];
-            },
-            { timeout: 2_000, interval: 10 },
-          )
-          .toEqual(["canvas.snapshot"]);
-
-        const node = await findConnectedNodeByDisplayName(displayName);
-        const nodeId = node?.nodeId ?? "";
-        expect(nodeId).toBeTruthy();
-
-        const systemRunRes = await rpcReq(ws, "node.invoke", {
-          nodeId,
-          command: "system.run",
-          params: { command: "echo blocked" },
-          idempotencyKey: `allowlist-confusable-${testCase.label}`,
-        });
-        expect(systemRunRes.ok).toBe(false);
-        expect(systemRunRes.error?.message ?? "").toContain("node command not allowed");
-      } finally {
-        await client?.stopAndWait();
-      }
+      const systemRunRes = await rpcReq(ws, "node.invoke", {
+        nodeId,
+        command: "system.run",
+        params: { command: "echo blocked" },
+        idempotencyKey: "allowlist-confusable-greek-omicron",
+      });
+      expect(systemRunRes.ok).toBe(false);
+      expect(systemRunRes.error?.message ?? "").toContain("node command not allowed");
+    } finally {
+      await client?.stopAndWait();
    }
  });
 });