nyrn
/
openclaw
mirror of https://github.com/openclaw/openclaw.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: harden paired-device management authz (#50627) (thanks @coygeek)

This commit is contained in:
Peter Steinberger 2026-04-04 23:19:34 +09:00
parent 9ac9edff43
commit fb8e20ddb6
4 changed files with 349 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -112,6 +112,7 @@ Docs: https://docs.openclaw.ai
- Mattermost/config schema: accept `groups.*.requireMention` again so existing Mattermost configs no longer fail strict validation after upgrade. (#58271) Thanks @MoerAI.
- Providers/OpenRouter failover: classify `403 "Key limit exceeded"` spending-limit responses as billing so model fallback continues instead of stopping on generic auth. (#59892) Thanks @rockcent.
- Device pairing/security: keep non-operator device scope checks bound to the requested role prefix so bootstrap verification cannot redeem `operator.*` scopes through `node` auth. (#57258) Thanks @jlapenna.
- Gateway/device pairing: require non-admin paired-device sessions to manage only their own device for token rotate/revoke and paired-device removal, blocking cross-device token theft inside pairing-scoped sessions. (#50627) Thanks @coygeek.
## 2026.4.2

141
src/gateway/server-methods/devices.test.ts
View File

@ -27,6 +27,15 @@ vi.mock("../../infra/device-pairing.js", async () => {
};
});
function createClient(scopes: string[], deviceId?: string) {
return {
connect: {
scopes,
...(deviceId ? { device: { id: deviceId } } : {}),
},
} as never;
}
function createOptions(
method: string,
params: Record<string, unknown>,
@ -86,6 +95,41 @@ describe("deviceHandlers", () => {
);
});
it("rejects removing another device from a non-admin device session", async () => {
const opts = createOptions(
"device.pair.remove",
{ deviceId: "device-2" },
{ client: createClient(["operator.pairing"], "device-1") },
);
await deviceHandlers["device.pair.remove"](opts);
expect(removePairedDeviceMock).not.toHaveBeenCalled();
expect(opts.respond).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({ message: "device pairing removal denied" }),
);
});
it("treats normalized device ids as self-owned for paired device removal", async () => {
removePairedDeviceMock.mockResolvedValue({ deviceId: "device-1", removedAtMs: 123 });
const opts = createOptions(
"device.pair.remove",
{ deviceId: " device-1 " },
{ client: createClient(["operator.pairing"], "device-1") },
);
await deviceHandlers["device.pair.remove"](opts);
expect(removePairedDeviceMock).toHaveBeenCalledWith(" device-1 ");
expect(opts.respond).toHaveBeenCalledWith(
true,
{ deviceId: "device-1", removedAtMs: 123 },
undefined,
);
});
it("disconnects active clients after revoking a device token", async () => {
revokeDeviceTokenMock.mockResolvedValue({ role: "operator", revokedAtMs: 456 });
const opts = createOptions("device.token.revoke", {
@ -110,6 +154,48 @@ describe("deviceHandlers", () => {
);
});
it("allows admin-scoped callers to revoke another device's token", async () => {
revokeDeviceTokenMock.mockResolvedValue({ role: "operator", revokedAtMs: 456 });
const opts = createOptions(
"device.token.revoke",
{ deviceId: "device-2", role: "operator" },
{ client: createClient(["operator.admin"], "device-1") },
);
await deviceHandlers["device.token.revoke"](opts);
expect(revokeDeviceTokenMock).toHaveBeenCalledWith({
deviceId: "device-2",
role: "operator",
});
expect(opts.respond).toHaveBeenCalledWith(
true,
{ deviceId: "device-2", role: "operator", revokedAtMs: 456 },
undefined,
);
});
it("treats normalized device ids as self-owned for token revocation", async () => {
revokeDeviceTokenMock.mockResolvedValue({ role: "operator", revokedAtMs: 456 });
const opts = createOptions(
"device.token.revoke",
{ deviceId: " device-1 ", role: "operator" },
{ client: createClient(["operator.pairing"], "device-1") },
);
await deviceHandlers["device.token.revoke"](opts);
expect(revokeDeviceTokenMock).toHaveBeenCalledWith({
deviceId: " device-1 ",
role: "operator",
});
expect(opts.respond).toHaveBeenCalledWith(
true,
{ deviceId: "device-1", role: "operator", revokedAtMs: 456 },
undefined,
);
});
it("disconnects active clients after rotating a device token", async () => {
getPairedDeviceMock.mockResolvedValue({
deviceId: "device-1",
@ -175,6 +261,61 @@ describe("deviceHandlers", () => {
);
});
it("treats normalized device ids as self-owned for token rotation", async () => {
getPairedDeviceMock.mockResolvedValue({
deviceId: "device-1",
role: "operator",
roles: ["operator"],
scopes: ["operator.pairing"],
tokens: {
operator: {
token: "old-token",
role: "operator",
scopes: ["operator.pairing"],
createdAtMs: 123,
},
},
});
rotateDeviceTokenMock.mockResolvedValue({
ok: true,
entry: {
token: "new-token",
role: "operator",
scopes: ["operator.pairing"],
createdAtMs: 456,
rotatedAtMs: 789,
},
});
const opts = createOptions(
"device.token.rotate",
{
deviceId: " device-1 ",
role: "operator",
scopes: ["operator.pairing"],
},
{ client: createClient(["operator.pairing"], "device-1") },
);
await deviceHandlers["device.token.rotate"](opts);
expect(rotateDeviceTokenMock).toHaveBeenCalledWith({
deviceId: " device-1 ",
role: "operator",
scopes: ["operator.pairing"],
});
expect(opts.respond).toHaveBeenCalledWith(
true,
{
deviceId: " device-1 ",
role: "operator",
token: "new-token",
scopes: ["operator.pairing"],
rotatedAtMs: 789,
},
undefined,
);
});
it("rejects rotating a token for a role that was never approved", async () => {
getPairedDeviceMock.mockResolvedValue({
deviceId: "device-1",

87
src/gateway/server-methods/devices.ts
View File

@ -24,7 +24,7 @@ import {
validateDeviceTokenRevokeParams,
validateDeviceTokenRotateParams,
} from "../protocol/index.js";
import type { GatewayRequestHandlers } from "./types.js";
import type { GatewayClient, GatewayRequestHandlers } from "./types.js";
const DEVICE_TOKEN_ROTATION_DENIED_MESSAGE = "device token rotation denied";
@ -33,6 +33,13 @@ type DeviceTokenRotateTarget = {
normalizedRole: string;
};
type DeviceManagementAuthz = {
callerDeviceId: string | null;
callerScopes: string[];
isAdminCaller: boolean;
normalizedTargetDeviceId: string;
};
function redactPairedDevice(
device: { tokens?: Record<string, DeviceAuthToken> } & Record<string, unknown>,
) {
@ -47,7 +54,11 @@ function logDeviceTokenRotationDenied(params: {
log: { warn: (message: string) => void };
deviceId: string;
role: string;
reason: RotateDeviceTokenDenyReason | "caller-missing-scope" | "unknown-device-or-role";
reason:
| RotateDeviceTokenDenyReason
| "caller-missing-scope"
| "unknown-device-or-role"
| "device-ownership-mismatch";
scope?: string | null;
}) {
const suffix = params.scope ? ` scope=${params.scope}` : "";
@ -75,6 +86,32 @@ async function loadDeviceTokenRotateTarget(params: {
return { pairedDevice, normalizedRole };
}
function resolveDeviceManagementAuthz(
client: GatewayClient | null,
targetDeviceId: string,
): DeviceManagementAuthz {
const callerScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : [];
const rawCallerDeviceId = client?.connect?.device?.id;
const callerDeviceId =
typeof rawCallerDeviceId === "string" && rawCallerDeviceId.trim()
? rawCallerDeviceId.trim()
: null;
return {
callerDeviceId,
callerScopes,
isAdminCaller: callerScopes.includes("operator.admin"),
normalizedTargetDeviceId: targetDeviceId.trim(),
};
}
function deniesCrossDeviceManagement(authz: DeviceManagementAuthz): boolean {
return Boolean(
authz.callerDeviceId &&
authz.callerDeviceId !== authz.normalizedTargetDeviceId &&
!authz.isAdminCaller,
);
}
export const deviceHandlers: GatewayRequestHandlers = {
"device.pair.list": async ({ params, respond }) => {
if (!validateDevicePairListParams(params)) {
@ -176,7 +213,7 @@ export const deviceHandlers: GatewayRequestHandlers = {
);
respond(true, rejected, undefined);
},
"device.pair.remove": async ({ params, respond, context }) => {
"device.pair.remove": async ({ params, respond, context, client }) => {
if (!validateDevicePairRemoveParams(params)) {
respond(
false,
@ -191,6 +228,18 @@ export const deviceHandlers: GatewayRequestHandlers = {
return;
}
const { deviceId } = params as { deviceId: string };
const authz = resolveDeviceManagementAuthz(client, deviceId);
if (deniesCrossDeviceManagement(authz)) {
context.logGateway.warn(
`device pairing removal denied device=${deviceId} reason=device-ownership-mismatch`,
);
respond(
false,
undefined,
errorShape(ErrorCodes.INVALID_REQUEST, "device pairing removal denied"),
);
return;
}
const removed = await removePairedDevice(deviceId);
if (!removed) {
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId"));
@ -221,6 +270,21 @@ export const deviceHandlers: GatewayRequestHandlers = {
role: string;
scopes?: string[];
};
const authz = resolveDeviceManagementAuthz(client, deviceId);
if (deniesCrossDeviceManagement(authz)) {
logDeviceTokenRotationDenied({
log: context.logGateway,
deviceId,
role,
reason: "device-ownership-mismatch",
});
respond(
false,
undefined,
errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE),
);
return;
}
const rotateTarget = await loadDeviceTokenRotateTarget({
deviceId,
role,
@ -235,14 +299,13 @@ export const deviceHandlers: GatewayRequestHandlers = {
return;
}
const { pairedDevice, normalizedRole } = rotateTarget;
const callerScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : [];
const requestedScopes = normalizeDeviceAuthScopes(
scopes ?? pairedDevice.tokens?.[normalizedRole]?.scopes ?? pairedDevice.scopes,
);
const missingScope = resolveMissingRequestedScope({
role,
requestedScopes,
allowedScopes: callerScopes,
allowedScopes: authz.callerScopes,
});
if (missingScope) {
logDeviceTokenRotationDenied({
@ -293,7 +356,7 @@ export const deviceHandlers: GatewayRequestHandlers = {
context.disconnectClientsForDevice?.(deviceId.trim(), { role: entry.role });
});
},
"device.token.revoke": async ({ params, respond, context }) => {
"device.token.revoke": async ({ params, respond, context, client }) => {
if (!validateDeviceTokenRevokeParams(params)) {
respond(
false,
@ -308,6 +371,18 @@ export const deviceHandlers: GatewayRequestHandlers = {
return;
}
const { deviceId, role } = params as { deviceId: string; role: string };
const authz = resolveDeviceManagementAuthz(client, deviceId);
if (deniesCrossDeviceManagement(authz)) {
context.logGateway.warn(
`device token revocation denied device=${deviceId} role=${role} reason=device-ownership-mismatch`,
);
respond(
false,
undefined,
errorShape(ErrorCodes.INVALID_REQUEST, "device token revocation denied"),
);
return;
}
const entry = await revokeDeviceToken({ deviceId, role });
if (!entry) {
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId/role"));

126
src/gateway/server.device-token-rotate-authz.test.ts
View File

@ -107,6 +107,132 @@ async function waitForMacrotasks(): Promise<void> {
await new Promise<void>((resolve) => setImmediate(resolve));
}
async function issuePairingScopedTokenForAdminApprovedDevice(name: string): Promise<{
deviceId: string;
identityPath: string;
pairingToken: string;
}> {
const issued = await issueOperatorToken({
name,
approvedScopes: ["operator.admin"],
tokenScopes: ["operator.pairing"],
clientId: GATEWAY_CLIENT_NAMES.TEST,
clientMode: GATEWAY_CLIENT_MODES.TEST,
});
return {
deviceId: issued.deviceId,
identityPath: issued.identityPath,
pairingToken: issued.token,
};
}
describe("gateway device.token.rotate/revoke ownership guard (IDOR)", () => {
test("rejects a device-token caller rotating another device's token", async () => {
const started = await startServerWithClient("secret");
const deviceA = await issuePairingScopedTokenForAdminApprovedDevice("idor-device-a");
const deviceB = await issuePairingScopedTokenForAdminApprovedDevice("idor-device-b");
let pairingWs: WebSocket | undefined;
try {
pairingWs = await connectPairingScopedOperator({
port: started.port,
identityPath: deviceA.identityPath,
deviceToken: deviceA.pairingToken,
});
const rotate = await rpcReq(pairingWs, "device.token.rotate", {
deviceId: deviceB.deviceId,
role: "operator",
scopes: ["operator.pairing"],
});
expect(rotate.ok).toBe(false);
expect(rotate.error?.message).toBe("device token rotation denied");
const pairedB = await getPairedDevice(deviceB.deviceId);
expect(pairedB?.tokens?.operator?.token).toBe(deviceB.pairingToken);
} finally {
pairingWs?.close();
started.ws.close();
await started.server.close();
started.envSnapshot.restore();
}
});
test("allows an admin-scoped caller to rotate another device's token", async () => {
const started = await startServerWithClient("secret");
const device = await issuePairingScopedTokenForAdminApprovedDevice("idor-admin-rotate");
try {
await connectOk(started.ws);
const rotate = await rpcReq<{ token?: string }>(started.ws, "device.token.rotate", {
deviceId: device.deviceId,
role: "operator",
scopes: ["operator.pairing"],
});
expect(rotate.ok).toBe(true);
expect(rotate.payload?.token).toBeTruthy();
} finally {
started.ws.close();
await started.server.close();
started.envSnapshot.restore();
}
});
test("rejects a device-token caller revoking another device's token", async () => {
const started = await startServerWithClient("secret");
const deviceA = await issuePairingScopedTokenForAdminApprovedDevice("idor-revoke-a");
const deviceB = await issuePairingScopedTokenForAdminApprovedDevice("idor-revoke-b");
let pairingWs: WebSocket | undefined;
try {
pairingWs = await connectPairingScopedOperator({
port: started.port,
identityPath: deviceA.identityPath,
deviceToken: deviceA.pairingToken,
});
const revoke = await rpcReq(pairingWs, "device.token.revoke", {
deviceId: deviceB.deviceId,
role: "operator",
});
expect(revoke.ok).toBe(false);
expect(revoke.error?.message).toBe("device token revocation denied");
const pairedB = await getPairedDevice(deviceB.deviceId);
expect(pairedB?.tokens?.operator?.revokedAtMs).toBeUndefined();
} finally {
pairingWs?.close();
started.ws.close();
await started.server.close();
started.envSnapshot.restore();
}
});
test("allows an admin-scoped caller to revoke another device's token", async () => {
const started = await startServerWithClient("secret");
const device = await issuePairingScopedTokenForAdminApprovedDevice("idor-admin-revoke");
try {
await connectOk(started.ws);
const revoke = await rpcReq<{ revokedAtMs?: number }>(started.ws, "device.token.revoke", {
deviceId: device.deviceId,
role: "operator",
});
expect(revoke.ok).toBe(true);
expect(revoke.payload?.revokedAtMs).toBeTypeOf("number");
const paired = await getPairedDevice(device.deviceId);
expect(paired?.tokens?.operator?.revokedAtMs).toBeTypeOf("number");
} finally {
started.ws.close();
await started.server.close();
started.envSnapshot.restore();
}
});
});
describe("gateway device.token.rotate caller scope guard", () => {
test("rejects rotating an admin-approved device token above the caller session scopes", async () => {
const started = await startServerWithClient("secret");