From d15d3e17ecb3f2b5d0ddb90adfea274bb8634b4b Mon Sep 17 00:00:00 2001 From: Yuval Dinodia <102706514+yetval@users.noreply.github.com> Date: Thu, 16 Jul 2026 06:58:11 -0400 Subject: [PATCH] fix(gateway): scope node pairing management to caller device (#104989) Apply the existing device-token ownership rule to node pairing list, approve, reject, and rename. Preserve shared-auth, admin, and self-service management while preventing cross-device enumeration and mutation. Co-authored-by: Peter Steinberger --- src/gateway/server-methods/nodes.ts | 114 +++++++- src/gateway/server.node-pairing-authz.test.ts | 264 ++++++++++++++++++ src/infra/node-pairing.ts | 16 ++ 3 files changed, 390 insertions(+), 4 deletions(-) diff --git a/src/gateway/server-methods/nodes.ts b/src/gateway/server-methods/nodes.ts index 95377c42f935..d97ffbfee17c 100644 --- a/src/gateway/server-methods/nodes.ts +++ b/src/gateway/server-methods/nodes.ts @@ -36,6 +36,7 @@ import { formatErrorMessage } from "../../infra/errors.js"; import { NODE_ADMIN_ONLY_INVOKE_COMMANDS } from "../../infra/node-commands.js"; import { approveNodePairing, + getPendingNodePairing, listNodePairing, rejectNodePairing, renamePairedNode, @@ -74,6 +75,7 @@ import { deniesCrossDeviceManagement, pairedDeviceHasNonOperatorRole, resolveDeviceManagementAuthz, + resolveDeviceSessionAuthz, type DeviceManagementAuthz, } from "./device-management-authz.js"; import { emitDeviceManagementSecurityEvent } from "./device-management-security.js"; @@ -348,6 +350,26 @@ function broadcastRemovedNodePairing(params: { ); } +function emitNodePairingDeniedSecurityEvent(params: { + authz: DeviceManagementAuthz; + nodeId: string; + controlId: "node.pair.approve" | "node.pair.reject" | "node.rename"; + reason: string; +}): void { + emitDeviceManagementSecurityEvent({ + action: "device.pairing.denied", + outcome: "denied", + severity: "medium", + authz: params.authz, + targetDeviceId: params.nodeId, + policyId: "gateway.device-pairing", + decision: "deny", + controlId: params.controlId, + reason: params.reason, + attributes: { role: "node" }, + }); +} + function emitNodeRoleRemovalSecurityEvent(params: { authz: DeviceManagementAuthz; deviceId: string; @@ -857,7 +879,7 @@ export async function waitForNodeReconnect(params: { } export const nodeHandlers: GatewayRequestHandlers = { - "node.pair.list": async ({ params, respond }) => { + "node.pair.list": async ({ params, respond, client }) => { if (!validateNodePairListParams(params)) { respondInvalidParams({ respond, @@ -868,7 +890,17 @@ export const nodeHandlers: GatewayRequestHandlers = { } await respondUnavailableOnThrow(respond, async () => { const list = await listNodePairing(); - respond(true, list, undefined); + const authz = resolveDeviceSessionAuthz(client); + const visibleList = + authz.callerDeviceId && !authz.isAdminCaller + ? { + pending: list.pending.filter( + (request) => request.nodeId.trim() === authz.callerDeviceId, + ), + paired: list.paired.filter((node) => node.nodeId.trim() === authz.callerDeviceId), + } + : list; + respond(true, visibleList, undefined); }); }, "node.pair.approve": async ({ params, respond, context, client }) => { @@ -884,6 +916,36 @@ export const nodeHandlers: GatewayRequestHandlers = { // Intentionally fail closed for RPC callers without an explicit scoped session. const callerScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : []; await respondUnavailableOnThrow(respond, async () => { + const pending = await getPendingNodePairing(requestId); + const sessionAuthz = resolveDeviceSessionAuthz(client); + if (!pending && sessionAuthz.callerDeviceId && !sessionAuthz.isAdminCaller) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "node pairing approval denied"), + ); + return; + } + if (pending) { + const authz = resolveDeviceManagementAuthz(client, pending.nodeId); + if (deniesCrossDeviceManagement(authz)) { + context.logGateway.warn( + `node pairing approval denied node=${pending.nodeId} reason=device-ownership-mismatch`, + ); + emitNodePairingDeniedSecurityEvent({ + authz, + nodeId: pending.nodeId, + controlId: "node.pair.approve", + reason: "device-ownership-mismatch", + }); + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "node pairing approval denied"), + ); + return; + } + } const approved = await approveNodePairing(requestId, { callerScopes }); if (!approved) { respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown requestId")); @@ -938,7 +1000,7 @@ export const nodeHandlers: GatewayRequestHandlers = { respond(true, approved, undefined); }); }, - "node.pair.reject": async ({ params, respond, context }) => { + "node.pair.reject": async ({ params, respond, context, client }) => { if (!validateNodePairRejectParams(params)) { respondInvalidParams({ respond, @@ -949,6 +1011,36 @@ export const nodeHandlers: GatewayRequestHandlers = { } const { requestId } = params as { requestId: string }; await respondUnavailableOnThrow(respond, async () => { + const pending = await getPendingNodePairing(requestId); + const sessionAuthz = resolveDeviceSessionAuthz(client); + if (!pending && sessionAuthz.callerDeviceId && !sessionAuthz.isAdminCaller) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "node pairing rejection denied"), + ); + return; + } + if (pending) { + const authz = resolveDeviceManagementAuthz(client, pending.nodeId); + if (deniesCrossDeviceManagement(authz)) { + context.logGateway.warn( + `node pairing rejection denied node=${pending.nodeId} reason=device-ownership-mismatch`, + ); + emitNodePairingDeniedSecurityEvent({ + authz, + nodeId: pending.nodeId, + controlId: "node.pair.reject", + reason: "device-ownership-mismatch", + }); + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "node pairing rejection denied"), + ); + return; + } + } const rejected = await rejectNodePairing(requestId); if (!rejected) { respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown requestId")); @@ -1010,7 +1102,7 @@ export const nodeHandlers: GatewayRequestHandlers = { } }); }, - "node.rename": async ({ params, respond }) => { + "node.rename": async ({ params, respond, context, client }) => { if (!validateNodeRenameParams(params)) { respondInvalidParams({ respond, @@ -1024,6 +1116,20 @@ export const nodeHandlers: GatewayRequestHandlers = { displayName: string; }; await respondUnavailableOnThrow(respond, async () => { + const authz = resolveDeviceManagementAuthz(client, nodeId); + if (deniesCrossDeviceManagement(authz)) { + context.logGateway.warn( + `node rename denied node=${authz.normalizedTargetDeviceId} reason=device-ownership-mismatch`, + ); + emitNodePairingDeniedSecurityEvent({ + authz, + nodeId: authz.normalizedTargetDeviceId, + controlId: "node.rename", + reason: "device-ownership-mismatch", + }); + respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "node rename denied")); + return; + } const trimmed = displayName.trim(); if (!trimmed) { respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "displayName required")); diff --git a/src/gateway/server.node-pairing-authz.test.ts b/src/gateway/server.node-pairing-authz.test.ts index de37c1014b7e..eeef7cfd0261 100644 --- a/src/gateway/server.node-pairing-authz.test.ts +++ b/src/gateway/server.node-pairing-authz.test.ts @@ -8,6 +8,7 @@ import { listDevicePairing, requestDevicePairing, } from "../infra/device-pairing.js"; +import { NODE_MCP_TOOLS_CALL_COMMAND } from "../infra/node-commands.js"; import { approveNodePairing, listNodePairing, requestNodePairing } from "../infra/node-pairing.js"; import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js"; import { @@ -17,6 +18,7 @@ import { } from "../utils/message-channel.js"; import { callGateway } from "./call.js"; import { + issueOperatorToken, loadDeviceIdentity, openTrackedWs, pairDeviceIdentity, @@ -384,6 +386,268 @@ describe("gateway node pairing authorization", () => { }); }); + describeWithGatewayServer("cross-device management guard", (getStarted) => { + async function requestVictimNodeSurface(nodeId: string) { + await seedNodeDevice(nodeId); + return await requestNodePairing({ + nodeId, + platform: "macos", + deviceFamily: "Mac", + commands: [NODE_MCP_TOOLS_CALL_COMMAND], + }); + } + + async function openDeviceTokenSession(params: { + name: string; + scopes: string[]; + }): Promise<{ ws: WebSocket; deviceId: string }> { + const operator = await issueOperatorToken({ + name: params.name, + approvedScopes: params.scopes, + tokenScopes: params.scopes, + }); + const ws = await openTrackedWs(getStarted().port); + await connectOk(ws, { + skipDefaultAuth: true, + deviceToken: operator.token.trim(), + deviceIdentityPath: operator.identityPath, + scopes: params.scopes, + }); + return { ws, deviceId: operator.deviceId }; + } + + test("denies non-admin cross-device list, approve, reject, and rename", async () => { + const approveVictimId = "node-cross-device-approve-victim"; + const approveRequest = await requestVictimNodeSurface(approveVictimId); + const rejectVictimId = "node-cross-device-reject-victim"; + const rejectRequest = await requestVictimNodeSurface(rejectVictimId); + const renameVictimId = "node-cross-device-rename-victim"; + const renameRequest = await requestVictimNodeSurface(renameVictimId); + requireApprovedPairing( + await approveNodePairing(renameRequest.request.requestId, { + callerScopes: ["operator.pairing", "operator.write"], + }), + ); + + const { ws } = await openDeviceTokenSession({ + name: "node-cross-device-attacker", + scopes: ["operator.pairing", "operator.write"], + }); + try { + const listed = await rpcReq>>( + ws, + "node.pair.list", + {}, + ); + expect(listed.ok).toBe(true); + expect(listed.payload).toEqual({ pending: [], paired: [] }); + + const approve = await rpcReq(ws, "node.pair.approve", { + requestId: approveRequest.request.requestId, + }); + expect(approve.ok).toBe(false); + expect(approve.error?.message).toBe("node pairing approval denied"); + await expect(findPairedNode(approveVictimId)).resolves.toBeNull(); + + const reject = await rpcReq(ws, "node.pair.reject", { + requestId: rejectRequest.request.requestId, + }); + expect(reject.ok).toBe(false); + expect(reject.error?.message).toBe("node pairing rejection denied"); + expect((await listNodePairing()).pending).toContainEqual( + expect.objectContaining({ nodeId: rejectVictimId }), + ); + + const unknownApprove = await rpcReq(ws, "node.pair.approve", { + requestId: "unknown-cross-device-approve", + }); + expect(unknownApprove.error?.message).toBe("node pairing approval denied"); + const unknownReject = await rpcReq(ws, "node.pair.reject", { + requestId: "unknown-cross-device-reject", + }); + expect(unknownReject.error?.message).toBe("node pairing rejection denied"); + + const rename = await rpcReq(ws, "node.rename", { + nodeId: renameVictimId, + displayName: "attacker rename", + }); + expect(rename.ok).toBe(false); + expect(rename.error?.message).toBe("node rename denied"); + await expect(findPairedNode(renameVictimId)).resolves.not.toMatchObject({ + displayName: "attacker rename", + }); + } finally { + ws.close(); + } + }); + + test("allows a non-admin device-token session to manage its own node surface", async () => { + const name = "node-self-device"; + const attacker = await issueOperatorToken({ + name, + approvedScopes: ["operator.pairing", "operator.write"], + tokenScopes: ["operator.pairing", "operator.write"], + }); + await pairDeviceIdentity({ + name, + role: "node", + scopes: [], + clientId: GATEWAY_CLIENT_NAMES.NODE_HOST, + clientMode: GATEWAY_CLIENT_MODES.NODE, + }); + const request = await requestNodePairing({ + nodeId: attacker.deviceId, + platform: "macos", + deviceFamily: "Mac", + commands: [NODE_MCP_TOOLS_CALL_COMMAND], + }); + + const ws = await openTrackedWs(getStarted().port); + try { + await connectOk(ws, { + skipDefaultAuth: true, + deviceToken: attacker.token.trim(), + deviceIdentityPath: attacker.identityPath, + scopes: ["operator.pairing", "operator.write"], + }); + + const listed = await rpcReq>>( + ws, + "node.pair.list", + {}, + ); + expect(listed.payload?.pending.map((entry) => entry.nodeId)).toEqual([attacker.deviceId]); + + const approve = await rpcReq(ws, "node.pair.approve", { + requestId: request.request.requestId, + }); + expect(approve.ok).toBe(true); + + const rename = await rpcReq(ws, "node.rename", { + nodeId: attacker.deviceId, + displayName: "self renamed", + }); + expect(rename.ok).toBe(true); + await expect(findPairedNode(attacker.deviceId)).resolves.toMatchObject({ + displayName: "self renamed", + }); + + const nextRequest = await requestNodePairing({ + nodeId: attacker.deviceId, + platform: "macos", + deviceFamily: "Mac", + commands: [], + }); + const reject = await rpcReq(ws, "node.pair.reject", { + requestId: nextRequest.request.requestId, + }); + expect(reject.ok).toBe(true); + expect((await listNodePairing()).pending).not.toContainEqual( + expect.objectContaining({ requestId: nextRequest.request.requestId }), + ); + } finally { + ws.close(); + } + }); + + test("allows a shared-auth operator session to manage another device's node surface", async () => { + const victimNodeId = "node-shared-auth-victim"; + const request = await requestVictimNodeSurface(victimNodeId); + const operator = await issueOperatorToken({ + name: "node-shared-auth-operator", + approvedScopes: ["operator.pairing", "operator.write"], + }); + + const ws = await openTrackedWs(getStarted().port); + try { + await connectOk(ws, { + token: "secret".trim(), + deviceIdentityPath: operator.identityPath, + scopes: ["operator.pairing", "operator.write"], + }); + + const listed = await rpcReq>>( + ws, + "node.pair.list", + {}, + ); + expect(listed.payload?.pending).toContainEqual( + expect.objectContaining({ nodeId: victimNodeId }), + ); + + const approve = await rpcReq(ws, "node.pair.approve", { + requestId: request.request.requestId, + }); + expect(approve.ok).toBe(true); + await expect(findPairedNode(victimNodeId)).resolves.toMatchObject({ + commands: [NODE_MCP_TOOLS_CALL_COMMAND], + }); + + const rename = await rpcReq(ws, "node.rename", { + nodeId: victimNodeId, + displayName: "shared renamed", + }); + expect(rename.ok).toBe(true); + + const nextRequest = await requestNodePairing({ + nodeId: victimNodeId, + platform: "macos", + deviceFamily: "Mac", + commands: [], + }); + const reject = await rpcReq(ws, "node.pair.reject", { + requestId: nextRequest.request.requestId, + }); + expect(reject.ok).toBe(true); + } finally { + ws.close(); + } + }); + + test("allows an admin device-token session to manage another device's node surface", async () => { + const victimNodeId = "node-admin-device-victim"; + const request = await requestVictimNodeSurface(victimNodeId); + const { ws } = await openDeviceTokenSession({ + name: "node-admin-device-operator", + scopes: ["operator.admin", "operator.pairing", "operator.write"], + }); + try { + const listed = await rpcReq>>( + ws, + "node.pair.list", + {}, + ); + expect(listed.payload?.pending).toContainEqual( + expect.objectContaining({ nodeId: victimNodeId }), + ); + + const approve = await rpcReq(ws, "node.pair.approve", { + requestId: request.request.requestId, + }); + expect(approve.ok).toBe(true); + + const rename = await rpcReq(ws, "node.rename", { + nodeId: victimNodeId, + displayName: "admin renamed", + }); + expect(rename.ok).toBe(true); + + const nextRequest = await requestNodePairing({ + nodeId: victimNodeId, + platform: "macos", + deviceFamily: "Mac", + commands: [], + }); + const reject = await rpcReq(ws, "node.pair.reject", { + requestId: nextRequest.request.requestId, + }); + expect(reject.ok).toBe(true); + } finally { + ws.close(); + } + }); + }); + describeWithGatewayServer("pending diagnostics scopes", (getStarted) => { test("shows pending pairing records to direct-local backend shared-auth callers", async () => { const pendingOnlyNodeId = "node-local-backend-pending"; diff --git a/src/infra/node-pairing.ts b/src/infra/node-pairing.ts index 4a315b7100cb..78bfbeca0ee8 100644 --- a/src/infra/node-pairing.ts +++ b/src/infra/node-pairing.ts @@ -559,6 +559,22 @@ export async function rejectNodePairing( }); } +/** Return the owning node id for a pending node pairing request, or null if none. */ +export async function getPendingNodePairing( + requestId: string, + baseDir?: string, +): Promise<{ requestId: string; nodeId: string } | null> { + return await withPairedDeviceRecords(baseDir, (pairedByDeviceId) => { + const device = Object.values(pairedByDeviceId).find( + (entry) => entry.pendingNodeSurface?.requestId === requestId, + ); + if (!device?.pendingNodeSurface) { + return { value: null, persist: false }; + } + return { value: { requestId, nodeId: device.deviceId }, persist: false }; + }); +} + /** Update runtime node-surface metadata (connect stamps, remote skill bins). */ export async function updatePairedNodeMetadata( nodeId: string,