fix(gateway): stop pinning node commands to pairing state

This commit is contained in:
Peter Steinberger
2026-04-01 18:25:31 +09:00
parent fe57ee513f
commit f6317fb747
12 changed files with 71 additions and 222 deletions

View File

@@ -19,7 +19,6 @@ export function renderPendingPairingRequestsTable(params: {
IP: r.remoteIp ?? "",
Requested:
typeof r.ts === "number" ? formatTimeAgo(Math.max(0, now - r.ts)) : theme.muted("unknown"),
Repair: r.repairReason ? theme.warn(r.repairReason) : r.isRepair ? theme.warn("yes") : "",
}));
return {
heading: theme.heading("Pending"),
@@ -30,7 +29,6 @@ export function renderPendingPairingRequestsTable(params: {
{ key: "Node", header: "Node", minWidth: 14, flex: true },
{ key: "IP", header: "IP", minWidth: 10 },
{ key: "Requested", header: "Requested", minWidth: 12 },
{ key: "Repair", header: "Repair", minWidth: 12 },
],
rows,
}).trimEnd(),

View File

@@ -198,4 +198,41 @@ describe("gateway/node-catalog", () => {
}),
);
});
it("prefers the live command surface for connected nodes", () => {
const catalog = createKnownNodeCatalog({
pairedDevices: [],
pairedNodes: [
{
nodeId: "mac-1",
token: "node-token",
platform: "darwin",
caps: ["system"],
commands: ["system.run"],
createdAtMs: 1,
approvedAtMs: 123,
},
],
connectedNodes: [
{
nodeId: "mac-1",
connId: "conn-1",
client: {} as never,
displayName: "Mac",
platform: "darwin",
caps: ["canvas"],
commands: ["canvas.snapshot"],
connectedAtMs: 1,
},
],
});
expect(getKnownNode(catalog, "mac-1")).toEqual(
expect.objectContaining({
caps: ["canvas"],
commands: ["canvas.snapshot"],
connected: true,
}),
);
});
});

View File

@@ -106,8 +106,10 @@ function buildEffectiveKnownNode(entry: {
deviceFamily: live?.deviceFamily ?? nodePairing?.deviceFamily,
modelIdentifier: live?.modelIdentifier ?? nodePairing?.modelIdentifier,
remoteIp: live?.remoteIp ?? nodePairing?.remoteIp ?? devicePairing?.remoteIp,
caps: uniqueSortedStrings(live?.caps, nodePairing?.caps),
commands: uniqueSortedStrings(live?.commands, nodePairing?.commands),
caps: live ? uniqueSortedStrings(live.caps) : uniqueSortedStrings(nodePairing?.caps),
commands: live
? uniqueSortedStrings(live.commands)
: uniqueSortedStrings(nodePairing?.commands),
pathEnv: live?.pathEnv,
permissions: live?.permissions ?? nodePairing?.permissions,
connectedAtMs: live?.connectedAtMs,

View File

@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { diffApprovedNodeCommands, normalizeDeclaredNodeCommands } from "./node-command-policy.js";
import { normalizeDeclaredNodeCommands } from "./node-command-policy.js";
describe("gateway/node-command-policy", () => {
it("normalizes declared node commands against the allowlist", () => {
@@ -11,22 +11,4 @@ describe("gateway/node-command-policy", () => {
}),
).toEqual(["canvas.snapshot", "system.run"]);
});
it("reports command drift against the approved node command set", () => {
const allowlist = new Set(["canvas.snapshot", "system.run", "system.which"]);
expect(
diffApprovedNodeCommands({
declaredCommands: ["canvas.snapshot", "system.run"],
approvedCommands: ["canvas.snapshot", "system.which"],
allowlist,
}),
).toEqual({
declared: ["canvas.snapshot", "system.run"],
approved: ["canvas.snapshot", "system.which"],
missingApproved: ["system.run"],
extraApproved: ["system.which"],
effective: ["canvas.snapshot"],
needsRepair: true,
});
});
});

View File

@@ -217,43 +217,6 @@ export function normalizeDeclaredNodeCommands(params: {
);
}
export type NodeApprovedCommandDiff = {
declared: string[];
approved: string[];
missingApproved: string[];
extraApproved: string[];
effective: string[];
needsRepair: boolean;
};
export function diffApprovedNodeCommands(params: {
declaredCommands?: readonly string[];
approvedCommands?: readonly string[];
allowlist: Set<string>;
}): NodeApprovedCommandDiff {
const declared = normalizeDeclaredNodeCommands({
declaredCommands: params.declaredCommands,
allowlist: params.allowlist,
});
const approved = normalizeDeclaredNodeCommands({
declaredCommands: params.approvedCommands,
allowlist: params.allowlist,
});
const approvedSet = new Set(approved);
const declaredSet = new Set(declared);
const missingApproved = declared.filter((command) => !approvedSet.has(command));
const extraApproved = approved.filter((command) => !declaredSet.has(command));
const effective = declared.filter((command) => approvedSet.has(command));
return {
declared,
approved,
missingApproved,
extraApproved,
effective,
needsRepair: missingApproved.length > 0,
};
}
export function isNodeCommandAllowed(params: {
command: string;
declaredCommands?: string[];

View File

@@ -5,9 +5,8 @@ import type {
NodePairingRequestInput,
} from "../infra/node-pairing.js";
import {
diffApprovedNodeCommands,
normalizeDeclaredNodeCommands,
resolveNodeCommandAllowlist,
type NodeApprovedCommandDiff,
} from "./node-command-policy.js";
import type { ConnectParams } from "./protocol/index.js";
@@ -19,7 +18,6 @@ type PendingNodePairingResult = {
export type NodeConnectPairingReconcileResult = {
nodeId: string;
commandDiff: NodeApprovedCommandDiff;
effectiveCommands: string[];
pendingPairing?: PendingNodePairingResult;
};
@@ -29,7 +27,6 @@ function buildNodePairingRequestInput(params: {
connectParams: ConnectParams;
commands: string[];
remoteIp?: string;
repairReason?: NodePairingRequestInput["repairReason"];
}): NodePairingRequestInput {
return {
nodeId: params.nodeId,
@@ -41,7 +38,6 @@ function buildNodePairingRequestInput(params: {
caps: params.connectParams.caps,
commands: params.commands,
remoteIp: params.remoteIp,
repairReason: params.repairReason,
};
}
@@ -57,11 +53,10 @@ export async function reconcileNodePairingOnConnect(params: {
platform: params.connectParams.client.platform,
deviceFamily: params.connectParams.client.deviceFamily,
});
const commandDiff = diffApprovedNodeCommands({
const declared = normalizeDeclaredNodeCommands({
declaredCommands: Array.isArray(params.connectParams.commands)
? params.connectParams.commands
: [],
approvedCommands: params.pairedNode?.commands,
allowlist,
});
@@ -70,39 +65,19 @@ export async function reconcileNodePairingOnConnect(params: {
buildNodePairingRequestInput({
nodeId,
connectParams: params.connectParams,
commands: commandDiff.declared,
commands: declared,
remoteIp: params.reportedClientIp,
}),
);
return {
nodeId,
commandDiff,
effectiveCommands: [],
pendingPairing,
};
}
if (commandDiff.needsRepair) {
const pendingPairing = await params.requestPairing(
buildNodePairingRequestInput({
nodeId,
connectParams: params.connectParams,
commands: commandDiff.declared,
remoteIp: params.reportedClientIp,
repairReason: "approved-command-drift",
}),
);
return {
nodeId,
commandDiff,
effectiveCommands: commandDiff.effective,
effectiveCommands: declared,
pendingPairing,
};
}
return {
nodeId,
commandDiff,
effectiveCommands: commandDiff.effective,
effectiveCommands: declared,
};
}

View File

@@ -51,7 +51,6 @@ export const DevicePairRequestedEventSchema = Type.Object(
remoteIp: Type.Optional(NonEmptyString),
silent: Type.Optional(Type.Boolean()),
isRepair: Type.Optional(Type.Boolean()),
repairReason: Type.Optional(NonEmptyString),
ts: Type.Integer({ minimum: 0 }),
},
{ additionalProperties: false },

View File

@@ -1,11 +1,6 @@
import { describe, expect, test } from "vitest";
import { WebSocket } from "ws";
import {
approveNodePairing,
getPairedNode,
listNodePairing,
requestNodePairing,
} from "../infra/node-pairing.js";
import { approveNodePairing, listNodePairing, requestNodePairing } from "../infra/node-pairing.js";
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
import {
issueOperatorToken,
@@ -77,7 +72,9 @@ describe("gateway node pairing authorization", () => {
expect(approve.ok).toBe(false);
expect(approve.error?.message).toBe("missing scope: operator.write");
await expect(getPairedNode("node-approve-target")).resolves.toBeNull();
await expect(
import("../infra/node-pairing.js").then((m) => m.getPairedNode("node-approve-target")),
).resolves.toBeNull();
} finally {
pairingWs?.close();
started.ws.close();
@@ -118,7 +115,9 @@ describe("gateway node pairing authorization", () => {
expect(approve.ok).toBe(false);
expect(approve.error?.message).toBe("missing scope: operator.admin");
await expect(getPairedNode("node-approve-target")).resolves.toBeNull();
await expect(
import("../infra/node-pairing.js").then((m) => m.getPairedNode("node-approve-target")),
).resolves.toBeNull();
} finally {
pairingWs?.close();
started.ws.close();
@@ -127,7 +126,7 @@ describe("gateway node pairing authorization", () => {
}
});
test("pins connected node commands to the approved pairing record", async () => {
test("does not pin connected node commands to the approved pairing record", async () => {
const started = await startServerWithClient("secret");
const pairedNode = await pairDeviceIdentity({
name: "node-command-pin",
@@ -175,27 +174,19 @@ describe("gateway node pairing authorization", () => {
(entry) => entry.nodeId === pairedNode.identity.deviceId && entry.connected,
);
if (
JSON.stringify(node?.commands?.toSorted() ?? []) === JSON.stringify(["canvas.snapshot"])
JSON.stringify(node?.commands?.toSorted() ?? []) ===
JSON.stringify(["canvas.snapshot", "system.run"])
) {
break;
}
await new Promise((resolve) => setTimeout(resolve, 25));
}
const connectedNode = lastNodes.find(
(entry) => entry.nodeId === pairedNode.identity.deviceId && entry.connected,
);
expect(connectedNode?.commands?.toSorted(), JSON.stringify(lastNodes)).toEqual([
"canvas.snapshot",
]);
const invoke = await rpcReq(controlWs, "node.invoke", {
nodeId: pairedNode.identity.deviceId,
command: "system.run",
params: { command: "echo blocked" },
idempotencyKey: "node-command-pin",
});
expect(invoke.ok).toBe(false);
expect(invoke.error?.message ?? "").toContain("node command not allowed");
expect(
lastNodes
.find((entry) => entry.nodeId === pairedNode.identity.deviceId && entry.connected)
?.commands?.toSorted(),
JSON.stringify(lastNodes),
).toEqual(["canvas.snapshot", "system.run"]);
} finally {
controlWs?.close();
await firstClient?.stopAndWait();
@@ -206,7 +197,7 @@ describe("gateway node pairing authorization", () => {
}
});
test("requests repair pairing and restores approved commands after reconnect", async () => {
test("does not request repair pairing when a paired node reconnects with more commands", async () => {
const started = await startServerWithClient("secret");
const pairedNode = await pairDeviceIdentity({
name: "node-command-empty",
@@ -237,51 +228,6 @@ describe("gateway node pairing authorization", () => {
const deadline = Date.now() + 2_000;
let lastNodes: Array<{ nodeId: string; connected?: boolean; commands?: string[] }> = [];
while (Date.now() < deadline) {
const list = await rpcReq<{
nodes?: Array<{ nodeId: string; connected?: boolean; commands?: string[] }>;
}>(controlWs, "node.list", {});
lastNodes = list.payload?.nodes ?? [];
const node = lastNodes.find(
(entry) => entry.nodeId === pairedNode.identity.deviceId && entry.connected,
);
if ((node?.commands?.length ?? 0) === 0) {
break;
}
await new Promise((resolve) => setTimeout(resolve, 25));
}
const connectedNode = lastNodes.find(
(entry) => entry.nodeId === pairedNode.identity.deviceId && entry.connected,
);
expect(connectedNode?.commands ?? [], JSON.stringify(lastNodes)).toEqual([]);
const repairDeadline = Date.now() + 2_000;
let repairRequestId = "";
while (Date.now() < repairDeadline) {
const pairing = await listNodePairing();
const repair = pairing.pending.find(
(entry) => entry.nodeId === pairedNode.identity.deviceId,
);
if (repair) {
repairRequestId = repair.requestId;
expect(repair.isRepair).toBe(true);
expect(repair.repairReason).toBe("approved-command-drift");
expect(repair.commands).toEqual(["canvas.snapshot", "system.run"]);
break;
}
await new Promise((resolve) => setTimeout(resolve, 25));
}
expect(repairRequestId).toBeTruthy();
await approveNodePairing(repairRequestId);
await nodeClient.stopAndWait();
nodeClient = await connectNodeClient({
port: started.port,
deviceIdentity: pairedNode.identity,
commands: ["canvas.snapshot", "system.run"],
});
const restoredDeadline = Date.now() + 2_000;
while (Date.now() < restoredDeadline) {
const list = await rpcReq<{
nodes?: Array<{ nodeId: string; connected?: boolean; commands?: string[] }>;
}>(controlWs, "node.list", {});
@@ -297,7 +243,6 @@ describe("gateway node pairing authorization", () => {
}
await new Promise((resolve) => setTimeout(resolve, 25));
}
const repairedNode = lastNodes.find(
(entry) => entry.nodeId === pairedNode.identity.deviceId && entry.connected,
);
@@ -306,9 +251,9 @@ describe("gateway node pairing authorization", () => {
"system.run",
]);
await expect(getPairedNode(pairedNode.identity.deviceId)).resolves.toEqual(
await expect(listNodePairing()).resolves.toEqual(
expect.objectContaining({
commands: ["canvas.snapshot", "system.run"],
pending: [],
}),
);
} finally {

View File

@@ -381,7 +381,7 @@ describe("gateway node command allowlist", () => {
}
});
test("blocks all declared commands until node pairing exists", async () => {
test("keeps allowlisted declared commands available before node pairing exists", async () => {
const findConnectedNode = async (displayName: string) => {
const listRes = await rpcReq<{
nodes?: Array<{
@@ -413,7 +413,7 @@ describe("gateway node command allowlist", () => {
const node = await findConnectedNode(displayName);
return node?.commands?.toSorted() ?? [];
}, FAST_WAIT_OPTS)
.toEqual([]);
.toEqual(["canvas.snapshot", "system.run"]);
const node = await findConnectedNode(displayName);
const nodeId = node?.nodeId ?? "";
@@ -431,24 +431,6 @@ describe("gateway node command allowlist", () => {
}),
]),
);
const canvasRes = await rpcReq(ws, "node.invoke", {
nodeId,
command: "canvas.snapshot",
params: { format: "png" },
idempotencyKey: "allowlist-device-paired-only-canvas",
});
expect(canvasRes.ok).toBe(false);
expect(canvasRes.error?.message ?? "").toContain("node command not allowed");
const systemRunRes = await rpcReq(ws, "node.invoke", {
nodeId,
command: "system.run",
params: { command: "echo blocked" },
idempotencyKey: "allowlist-device-paired-only-system-run",
});
expect(systemRunRes.ok).toBe(false);
expect(systemRunRes.error?.message ?? "").toContain("node command not allowed");
} finally {
await nodeClient?.stopAndWait();
}

View File

@@ -50,7 +50,7 @@ describe("node pairing tokens", () => {
expect(second.request.requestId).toBe(first.request.requestId);
});
test("refreshes pending requests with newer commands and repair metadata", async () => {
test("refreshes pending requests with newer commands", async () => {
const baseDir = await mkdtemp(join(tmpdir(), "openclaw-node-pairing-"));
const first = await requestNodePairing(
{
@@ -60,7 +60,6 @@ describe("node pairing tokens", () => {
},
baseDir,
);
await approveNodePairing(first.request.requestId, baseDir);
const second = await requestNodePairing(
{
@@ -81,14 +80,12 @@ describe("node pairing tokens", () => {
baseDir,
);
expect(second.created).toBe(true);
expect(second.request.isRepair).toBe(true);
expect(second.request.repairReason).toBe("paired-node-refresh");
expect(second.created).toBe(false);
expect(second.request.requestId).toBe(first.request.requestId);
expect(third.created).toBe(false);
expect(third.request.requestId).toBe(second.request.requestId);
expect(third.request.displayName).toBe("Updated Node");
expect(third.request.commands).toEqual(["canvas.snapshot", "system.run", "system.which"]);
expect(third.request.repairReason).toBe("paired-node-refresh");
});
test("generates base64url node tokens with 256-bit entropy output length", async () => {

View File

@@ -29,18 +29,13 @@ export type NodeDeclaredSurface = {
export type NodeApprovedSurface = NodeDeclaredSurface;
export type NodePairingRepairReason = "approved-command-drift" | "paired-node-refresh";
export type NodePairingRequestInput = NodeDeclaredSurface & {
silent?: boolean;
repairReason?: NodePairingRepairReason;
};
export type NodePairingPendingRequest = NodePairingRequestInput & {
requestId: string;
silent?: boolean;
isRepair?: boolean;
repairReason?: NodePairingRepairReason;
ts: number;
};
@@ -77,24 +72,10 @@ function normalizeStringList(values?: string[]): string[] | undefined {
return normalized.length > 0 ? normalized : [];
}
function resolveNodePairingRepairReason(params: {
existingPairedNode: boolean;
requestedRepairReason?: NodePairingRepairReason;
}): NodePairingRepairReason | undefined {
if (params.requestedRepairReason) {
return params.requestedRepairReason;
}
if (params.existingPairedNode) {
return "paired-node-refresh";
}
return undefined;
}
function buildPendingNodePairingRequest(params: {
requestId?: string;
req: NodePairingRequestInput;
}): NodePairingPendingRequest {
const repairReason = params.req.repairReason;
return {
requestId: params.requestId ?? randomUUID(),
nodeId: params.req.nodeId,
@@ -110,8 +91,6 @@ function buildPendingNodePairingRequest(params: {
permissions: params.req.permissions,
remoteIp: params.req.remoteIp,
silent: params.req.silent,
repairReason,
isRepair: Boolean(repairReason),
ts: Date.now(),
};
}
@@ -120,7 +99,6 @@ function refreshPendingNodePairingRequest(
existing: NodePairingPendingRequest,
incoming: NodePairingRequestInput,
): NodePairingPendingRequest {
const repairReason = incoming.repairReason ?? existing.repairReason;
return {
...existing,
displayName: incoming.displayName ?? existing.displayName,
@@ -136,8 +114,6 @@ function refreshPendingNodePairingRequest(
remoteIp: incoming.remoteIp ?? existing.remoteIp,
// Preserve interactive visibility if either request needs attention.
silent: Boolean(existing.silent && incoming.silent),
repairReason,
isRepair: Boolean(repairReason),
ts: Date.now(),
};
}
@@ -218,10 +194,6 @@ export async function requestNodePairing(
if (!nodeId) {
throw new Error("nodeId required");
}
const repairReason = resolveNodePairingRepairReason({
existingPairedNode: Boolean(state.pairedByNodeId[nodeId]),
requestedRepairReason: req.repairReason,
});
const pendingForNode = Object.values(state.pendingById)
.filter((pending) => pending.nodeId === nodeId)
.toSorted((left, right) => right.ts - left.ts);
@@ -231,7 +203,6 @@ export async function requestNodePairing(
incoming: {
...req,
nodeId,
repairReason,
},
canRefreshSingle: () => true,
refreshSingle: (existing, incoming) => refreshPendingNodePairingRequest(existing, incoming),

View File

@@ -29,8 +29,6 @@ export type PendingRequest = {
coreVersion?: string;
uiVersion?: string;
remoteIp?: string;
isRepair?: boolean;
repairReason?: string;
ts: number;
};