fix: enforce device token scope containment

steipete · steipete · commit 8bbb143ab87e · 2026-04-26T07:28:21.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -76,6 +76,7 @@ Docs: https://docs.openclaw.ai
 
 - Update: ignore bundled plugin `.openclaw-install-stage` directories during global install verification and packaged dist pruning so leftover runtime-dep staging files do not turn successful updates into `unexpected packaged dist file` failures. Fixes #71752. Thanks @waynegault.
 - Gateway/plugins: stop persisted WhatsApp auth state from activating bundled channel runtime-dependency repair during startup when `channels.whatsapp` is absent, avoiding npm/git stalls on packaged Linux installs. Fixes #71994. Thanks @xiao398008.
+- Gateway/device tokens: enforce caller-scope containment inside token rotation and revocation so pairing-only sessions cannot mutate higher-scope operator tokens. Fixes #71990. Thanks @coygeek.
 - CLI/model runs: keep `openclaw infer model run` on explicit OpenRouter models from loading the full provider catalog or inheriting chat-agent silent-reply policy, restoring non-empty one-shot probe output. Fixes #68791. Thanks @limpredator.
 - Installer/macOS: rerun Homebrew install steps without the gum spinner when raw-mode ioctl failures occur, and avoid claiming `node@24` was installed when the Homebrew keg binary is missing. Fixes #70411. Thanks @1fanwang and @dad-io.
 - Installer: load nvm before Node.js detection so `curl | bash` installs respect nvm-managed Node instead of stale system Node. Fixes #49556. Thanks @heavenlxj.
diff --git a/docs/channels/pairing.md b/docs/channels/pairing.md
@@ -83,6 +83,8 @@ That bootstrap token carries the built-in pairing bootstrap profile:
 - bootstrap scope checks are role-prefixed, not one flat scope pool:
   operator scope entries only satisfy operator requests, and non-operator roles
   must still request scopes under their own role prefix
+- later token rotation/revocation remains bounded by both the device's approved
+  role contract and the caller session's operator scopes
 
 Treat the setup code like a password while it is valid.
 
diff --git a/docs/cli/devices.md b/docs/cli/devices.md
@@ -95,9 +95,9 @@ If you omit `--scope`, later reconnects with the stored rotated token reuse that
 token's cached approved scopes. If you pass explicit `--scope` values, those
 become the stored scope set for future cached-token reconnects.
 Non-admin paired-device callers can rotate only their **own** device token.
-Also, any explicit `--scope` values must stay within the caller session's own
-operator scopes; rotation cannot mint a broader operator token than the caller
-already has.
+The target token scope set must stay within the caller session's own operator
+scopes; rotation cannot mint or preserve a broader operator token than the
+caller already has.
 
 ```
 openclaw devices rotate --device <deviceId> --role operator --scope operator.read --scope operator.write
@@ -111,6 +111,8 @@ Revoke a device token for a specific role.
 
 Non-admin paired-device callers can revoke only their **own** device token.
 Revoking some other device's token requires `operator.admin`.
+The target token scope set must also fit within the caller session's own
+operator scopes; pairing-only callers cannot revoke admin/write operator tokens.
 
 ```
 openclaw devices revoke --device <deviceId> --role node
@@ -135,12 +137,15 @@ Pass `--token` or `--password` explicitly. Missing explicit credentials is an er
 - These commands require `operator.pairing` (or `operator.admin`) scope.
 - `gateway.nodes.pairing.autoApproveCidrs` is an opt-in Gateway policy for
   fresh node device pairing only; it does not change CLI approval authority.
-- Token rotation stays inside the approved pairing role set and approved scope
-  baseline for that device. A stray cached token entry does not grant a new
-  rotate target.
+- Token rotation and revocation stay inside the approved pairing role set and
+  approved scope baseline for that device. A stray cached token entry does not
+  grant a token-management target.
 - For paired-device token sessions, cross-device management is admin-only:
   `remove`, `rotate`, and `revoke` are self-only unless the caller has
   `operator.admin`.
+- Token mutation is also caller-scope contained: a pairing-only session cannot
+  rotate or revoke a token that currently carries `operator.admin` or
+  `operator.write`.
 - `devices clear` is intentionally gated by `--yes`.
 - If pairing scope is unavailable on local loopback (and no explicit `--url` is passed), list/approve can use a local pairing fallback.
 - `devices approve` requires an explicit request ID before minting tokens; omitting `requestId` or passing `--latest` only previews the newest pending request.
diff --git a/docs/gateway/protocol.md b/docs/gateway/protocol.md
@@ -360,8 +360,8 @@ enumeration of `src/gateway/server-methods/*.ts`.
   <Accordion title="Device pairing and device tokens">
     - `device.pair.list` returns pending and approved paired devices.
     - `device.pair.approve`, `device.pair.reject`, and `device.pair.remove` manage device-pairing records.
-    - `device.token.rotate` rotates a paired device token within its approved role and scope bounds.
-    - `device.token.revoke` revokes a paired device token.
+    - `device.token.rotate` rotates a paired device token within its approved role and caller scope bounds.
+    - `device.token.revoke` revokes a paired device token within its approved role and caller scope bounds.
   </Accordion>
 
   <Accordion title="Node pairing, invoke, and pending work">
@@ -549,15 +549,15 @@ rather than the pre-handshake defaults.
   reused when the client is reusing the stored per-device token.
 - Device tokens can be rotated/revoked via `device.token.rotate` and
   `device.token.revoke` (requires `operator.pairing` scope).
-- Token issuance/rotation stays bounded to the approved role set recorded in
-  that device's pairing entry; rotating a token cannot expand the device into a
-  role that pairing approval never granted.
+- Token issuance, rotation, and revocation stay bounded to the approved role set
+  recorded in that device's pairing entry; token mutation cannot expand or
+  target a device role that pairing approval never granted.
 - For paired-device token sessions, device management is self-scoped unless the
   caller also has `operator.admin`: non-admin callers can remove/revoke/rotate
   only their **own** device entry.
-- `device.token.rotate` also checks the requested operator scope set against the
-  caller's current session scopes. Non-admin callers cannot rotate a token into
-  a broader operator scope set than they already hold.
+- `device.token.rotate` and `device.token.revoke` also check the target operator
+  token scope set against the caller's current session scopes. Non-admin callers
+  cannot rotate or revoke a broader operator token than they already hold.
 - Auth failures include `error.details.code` plus recovery hints:
   - `error.details.canRetryWithDeviceToken` (boolean)
   - `error.details.recommendedNextStep` (`retry_with_device_token`, `update_auth_configuration`, `update_auth_credentials`, `wait_then_retry`, `review_auth_configuration`)
diff --git a/src/gateway/server-methods/devices.test.ts b/src/gateway/server-methods/devices.test.ts
@@ -181,7 +181,10 @@ describe("deviceHandlers", () => {
   });
 
   it("disconnects active clients after revoking a device token", async () => {
-    revokeDeviceTokenMock.mockResolvedValue({ role: "operator", revokedAtMs: 456 });
+    revokeDeviceTokenMock.mockResolvedValue({
+      ok: true,
+      entry: { role: "operator", revokedAtMs: 456 },
+    });
     const opts = createOptions("device.token.revoke", {
       deviceId: " device-1 ",
       role: " operator ",
@@ -193,6 +196,7 @@ describe("deviceHandlers", () => {
     expect(revokeDeviceTokenMock).toHaveBeenCalledWith({
       deviceId: " device-1 ",
       role: " operator ",
+      callerScopes: [],
     });
     expect(opts.context.disconnectClientsForDevice).toHaveBeenCalledWith("device-1", {
       role: "operator",
@@ -205,7 +209,10 @@ describe("deviceHandlers", () => {
   });
 
   it("allows admin-scoped callers to revoke another device's token", async () => {
-    revokeDeviceTokenMock.mockResolvedValue({ role: "operator", revokedAtMs: 456 });
+    revokeDeviceTokenMock.mockResolvedValue({
+      ok: true,
+      entry: { role: "operator", revokedAtMs: 456 },
+    });
     const opts = createOptions(
       "device.token.revoke",
       { deviceId: "device-2", role: "operator" },
@@ -217,6 +224,7 @@ describe("deviceHandlers", () => {
     expect(revokeDeviceTokenMock).toHaveBeenCalledWith({
       deviceId: "device-2",
       role: "operator",
+      callerScopes: ["operator.admin"],
     });
     expect(opts.respond).toHaveBeenCalledWith(
       true,
@@ -226,7 +234,10 @@ describe("deviceHandlers", () => {
   });
 
   it("treats normalized device ids as self-owned for token revocation", async () => {
-    revokeDeviceTokenMock.mockResolvedValue({ role: "operator", revokedAtMs: 456 });
+    revokeDeviceTokenMock.mockResolvedValue({
+      ok: true,
+      entry: { role: "operator", revokedAtMs: 456 },
+    });
     const opts = createOptions(
       "device.token.revoke",
       { deviceId: " device-1 ", role: "operator" },
@@ -238,6 +249,7 @@ describe("deviceHandlers", () => {
     expect(revokeDeviceTokenMock).toHaveBeenCalledWith({
       deviceId: " device-1 ",
       role: "operator",
+      callerScopes: ["operator.pairing"],
     });
     expect(opts.respond).toHaveBeenCalledWith(
       true,
@@ -272,6 +284,7 @@ describe("deviceHandlers", () => {
       deviceId: " device-1 ",
       role: " operator ",
       scopes: ["operator.pairing"],
+      callerScopes: ["operator.pairing"],
     });
     expect(opts.context.disconnectClientsForDevice).toHaveBeenCalledWith("device-1", {
       role: "operator",
@@ -308,6 +321,7 @@ describe("deviceHandlers", () => {
       deviceId: " device-1 ",
       role: "operator",
       scopes: ["operator.pairing"],
+      callerScopes: ["operator.pairing"],
     });
     expect(opts.respond).toHaveBeenCalledWith(
       true,
@@ -324,6 +338,7 @@ describe("deviceHandlers", () => {
 
   it("rejects rotating a token for a role that was never approved", async () => {
     mockPairedOperatorDevice();
+    rotateDeviceTokenMock.mockResolvedValue({ ok: false, reason: "unknown-device-or-role" });
     const opts = createOptions(
       "device.token.rotate",
       {
@@ -341,7 +356,12 @@ describe("deviceHandlers", () => {
 
     await deviceHandlers["device.token.rotate"](opts);
 
-    expect(rotateDeviceTokenMock).not.toHaveBeenCalled();
+    expect(rotateDeviceTokenMock).toHaveBeenCalledWith({
+      deviceId: "device-1",
+      role: "node",
+      scopes: undefined,
+      callerScopes: ["operator.pairing"],
+    });
     expect(opts.context.disconnectClientsForDevice).not.toHaveBeenCalled();
     expect(opts.respond).toHaveBeenCalledWith(
       false,
@@ -351,7 +371,7 @@ describe("deviceHandlers", () => {
   });
 
   it("does not disconnect clients when token revocation fails", async () => {
-    revokeDeviceTokenMock.mockResolvedValue(null);
+    revokeDeviceTokenMock.mockResolvedValue({ ok: false, reason: "unknown-device-or-role" });
     const opts = createOptions("device.token.revoke", {
       deviceId: "device-1",
       role: "operator",
@@ -363,7 +383,7 @@ describe("deviceHandlers", () => {
     expect(opts.respond).toHaveBeenCalledWith(
       false,
       undefined,
-      expect.objectContaining({ message: "unknown deviceId/role" }),
+      expect.objectContaining({ message: "device token revocation denied" }),
     );
   });
 
diff --git a/src/gateway/server-methods/devices.ts b/src/gateway/server-methods/devices.ts
@@ -1,20 +1,17 @@
 import {
   approveDevicePairing,
   formatDevicePairingForbiddenMessage,
-  getPairedDevice,
   getPendingDevicePairing,
-  listApprovedPairedDeviceRoles,
   listDevicePairing,
   removePairedDevice,
   type DeviceAuthToken,
+  type RevokeDeviceTokenDenyReason,
   type RotateDeviceTokenDenyReason,
   rejectDevicePairing,
   revokeDeviceToken,
   rotateDeviceToken,
   summarizeDeviceTokens,
 } from "../../infra/device-pairing.js";
-import { normalizeDeviceAuthScopes } from "../../shared/device-auth.js";
-import { resolveMissingRequestedScope } from "../../shared/operator-scope-compat.js";
 import {
   ErrorCodes,
   errorShape,
@@ -29,11 +26,7 @@ import {
 import type { GatewayClient, GatewayRequestHandlers } from "./types.js";
 
 const DEVICE_TOKEN_ROTATION_DENIED_MESSAGE = "device token rotation denied";
-
-type DeviceTokenRotateTarget = {
-  pairedDevice: NonNullable<Awaited<ReturnType<typeof getPairedDevice>>>;
-  normalizedRole: string;
-};
+const DEVICE_TOKEN_REVOCATION_DENIED_MESSAGE = "device token revocation denied";
 
 type DeviceSessionAuthz = {
   callerDeviceId: string | null;
@@ -62,11 +55,7 @@ function logDeviceTokenRotationDenied(params: {
   log: { warn: (message: string) => void };
   deviceId: string;
   role: string;
-  reason:
-    | RotateDeviceTokenDenyReason
-    | "caller-missing-scope"
-    | "unknown-device-or-role"
-    | "device-ownership-mismatch";
+  reason: RotateDeviceTokenDenyReason | "unknown-device-or-role" | "device-ownership-mismatch";
   scope?: string | null;
 }) {
   const suffix = params.scope ? ` scope=${params.scope}` : "";
@@ -75,23 +64,17 @@ function logDeviceTokenRotationDenied(params: {
   );
 }
 
-async function loadDeviceTokenRotateTarget(params: {
+function logDeviceTokenRevocationDenied(params: {
+  log: { warn: (message: string) => void };
   deviceId: string;
   role: string;
-  log: { warn: (message: string) => void };
-}): Promise<DeviceTokenRotateTarget | null> {
-  const normalizedRole = params.role.trim();
-  const pairedDevice = await getPairedDevice(params.deviceId);
-  if (!pairedDevice || !listApprovedPairedDeviceRoles(pairedDevice).includes(normalizedRole)) {
-    logDeviceTokenRotationDenied({
-      log: params.log,
-      deviceId: params.deviceId,
-      role: params.role,
-      reason: "unknown-device-or-role",
-    });
-    return null;
-  }
-  return { pairedDevice, normalizedRole };
+  reason: RevokeDeviceTokenDenyReason | "device-ownership-mismatch";
+  scope?: string | null;
+}) {
+  const suffix = params.scope ? ` scope=${params.scope}` : "";
+  params.log.warn(
+    `device token revocation denied device=${params.deviceId} role=${params.role} reason=${params.reason}${suffix}`,
+  );
 }
 
 function resolveDeviceManagementAuthz(
@@ -354,50 +337,19 @@ export const deviceHandlers: GatewayRequestHandlers = {
       );
       return;
     }
-    const rotateTarget = await loadDeviceTokenRotateTarget({
+    const rotated = await rotateDeviceToken({
       deviceId,
       role,
-      log: context.logGateway,
-    });
-    if (!rotateTarget) {
-      respond(
-        false,
-        undefined,
-        errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE),
-      );
-      return;
-    }
-    const { pairedDevice, normalizedRole } = rotateTarget;
-    const requestedScopes = normalizeDeviceAuthScopes(
-      scopes ?? pairedDevice.tokens?.[normalizedRole]?.scopes ?? pairedDevice.scopes,
-    );
-    const missingScope = resolveMissingRequestedScope({
-      role,
-      requestedScopes,
-      allowedScopes: authz.callerScopes,
+      scopes,
+      callerScopes: authz.callerScopes,
     });
-    if (missingScope) {
-      logDeviceTokenRotationDenied({
-        log: context.logGateway,
-        deviceId,
-        role,
-        reason: "caller-missing-scope",
-        scope: missingScope,
-      });
-      respond(
-        false,
-        undefined,
-        errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE),
-      );
-      return;
-    }
-    const rotated = await rotateDeviceToken({ deviceId, role, scopes });
     if (!rotated.ok) {
       logDeviceTokenRotationDenied({
         log: context.logGateway,
         deviceId,
         role,
         reason: rotated.reason,
+        scope: rotated.scope,
       });
       respond(
         false,
@@ -448,15 +400,27 @@ export const deviceHandlers: GatewayRequestHandlers = {
       respond(
         false,
         undefined,
-        errorShape(ErrorCodes.INVALID_REQUEST, "device token revocation denied"),
+        errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_REVOCATION_DENIED_MESSAGE),
       );
       return;
     }
-    const entry = await revokeDeviceToken({ deviceId, role });
-    if (!entry) {
-      respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId/role"));
+    const revoked = await revokeDeviceToken({ deviceId, role, callerScopes: authz.callerScopes });
+    if (!revoked.ok) {
+      logDeviceTokenRevocationDenied({
+        log: context.logGateway,
+        deviceId,
+        role,
+        reason: revoked.reason,
+        scope: revoked.scope,
+      });
+      respond(
+        false,
+        undefined,
+        errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_REVOCATION_DENIED_MESSAGE),
+      );
       return;
     }
+    const entry = revoked.entry;
     const normalizedDeviceId = deviceId.trim();
     context.logGateway.info(`device token revoked device=${normalizedDeviceId} role=${entry.role}`);
     respond(
diff --git a/src/gateway/server.device-token-rotate-authz.test.ts b/src/gateway/server.device-token-rotate-authz.test.ts
diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts
diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts
