mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-29 02:11:12 +00:00
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 <steipete@gmail.com>
This commit is contained in:
@@ -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"));
|
||||
|
||||
@@ -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<Awaited<ReturnType<typeof listNodePairing>>>(
|
||||
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<Awaited<ReturnType<typeof listNodePairing>>>(
|
||||
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<Awaited<ReturnType<typeof listNodePairing>>>(
|
||||
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<Awaited<ReturnType<typeof listNodePairing>>>(
|
||||
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";
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user