From 52b8e318bd44b2f48f2da4a8beb4249fba9542b1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 17 Apr 2026 22:54:19 +0100 Subject: [PATCH] test: collapse gateway node authz hotspots --- src/gateway/server.node-pairing-authz.test.ts | 82 +++++++------- .../server.roles-allowlist-update.test.ts | 100 +++++++----------- 2 files changed, 82 insertions(+), 100 deletions(-) diff --git a/src/gateway/server.node-pairing-authz.test.ts b/src/gateway/server.node-pairing-authz.test.ts index bd9fdaf071a..17dbfb63f13 100644 --- 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>; + 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", }), diff --git a/src/gateway/server.roles-allowlist-update.test.ts b/src/gateway/server.roles-allowlist-update.test.ts index a05bb87aa87..2409250195d 100644 --- 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(); } }); });