fix: harden device pairing scope approval

steipete · steipete · commit b660493e5466 · 2026-04-10T09:48:17.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -83,6 +83,7 @@ Docs: https://docs.openclaw.ai
 - Browser/plugin SDK: route browser auth, profile, host-inspection, and doctor readiness helpers through browser plugin public facades so core compatibility helpers stop carrying duplicate runtime implementations. (#63957) Thanks @joshavant.
 - Agents/failover: allow cooldown probes for `timeout` (including network outage classifications) so the primary model can recover after failover without a gateway restart. (#63996) Thanks @neeravmakwana.
 - iMessage (imsg): strip an accidental protobuf length-delimited UTF-8 field wrapper from inbound `text` and `reply_to_text` when it fully consumes the field, fixing leading garbage before the real message. (#63868) Thanks @neeravmakwana.
+- Gateway/pairing: fail closed for paired device records that have no device tokens, and reject pairing approvals whose requested scopes do not match the requested device roles.
 
 ## 2026.4.9
 
diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts
@@ -284,6 +284,53 @@ describe("device pairing tokens", () => {
     ).resolves.toEqual({ ok: true });
   });
 
+  test.each([
+    {
+      name: "node custom scope",
+      roles: ["node"],
+      scopes: ["vault.admin"],
+      missingScope: "vault.admin",
+      callerScopes: [],
+    },
+    {
+      name: "operator custom scope",
+      roles: ["operator"],
+      scopes: ["vault.admin"],
+      missingScope: "vault.admin",
+      callerScopes: ["operator.pairing"],
+    },
+    {
+      name: "node requesting operator scope",
+      roles: ["node"],
+      scopes: ["operator.read"],
+      missingScope: "operator.read",
+      callerScopes: ["operator.read"],
+    },
+  ])("rejects requested scopes outside requested roles: $name", async (params) => {
+    const baseDir = await makeDevicePairingDir();
+    const request = await requestDevicePairing(
+      {
+        deviceId: "device-1",
+        publicKey: "public-key-1",
+        roles: params.roles,
+        scopes: params.scopes,
+      },
+      baseDir,
+    );
+
+    await expect(
+      approveDevicePairing(
+        request.request.requestId,
+        { callerScopes: params.callerScopes },
+        baseDir,
+      ),
+    ).resolves.toEqual({
+      status: "forbidden",
+      missingScope: params.missingScope,
+    });
+    await expect(getPairedDevice("device-1", baseDir)).resolves.toBeNull();
+  });
+
   test("preserves existing non-operator scopes during operator-only mixed-role repairs", async () => {
     const baseDir = await makeDevicePairingDir();
     const initial = await requestDevicePairing(
@@ -831,7 +878,7 @@ describe("device pairing tokens", () => {
     expect(hasEffectivePairedDeviceRole(paired, "node")).toBe(false);
   });
 
-  test("falls back to legacy role fields when tokens map is empty", async () => {
+  test("fails closed for tokenless legacy role fields", async () => {
     const device: PairedDevice = {
       deviceId: "device-fallback",
       publicKey: "pk-fallback",
@@ -841,9 +888,9 @@ describe("device pairing tokens", () => {
       createdAtMs: Date.now(),
       approvedAtMs: Date.now(),
     };
-    expect(listEffectivePairedDeviceRoles(device)).toEqual(["node", "operator"]);
-    expect(hasEffectivePairedDeviceRole(device, "node")).toBe(true);
-    expect(hasEffectivePairedDeviceRole(device, "operator")).toBe(true);
+    expect(listEffectivePairedDeviceRoles(device)).toEqual([]);
+    expect(hasEffectivePairedDeviceRole(device, "node")).toBe(false);
+    expect(hasEffectivePairedDeviceRole(device, "operator")).toBe(false);
   });
 
   test("filters active token roles to the approved pairing role set", async () => {
diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts
@@ -189,16 +189,9 @@ export function listEffectivePairedDeviceRoles(
     const approvedRoles = new Set(listApprovedPairedDeviceRoles(device));
     return activeTokenRoles.filter((role) => approvedRoles.has(role));
   }
-  // Only fall back to legacy role fields when the tokens map is absent
-  // or has no entries at all (empty object from a fresh pairing record).
-  // When token entries exist but are all revoked, the revocation is
-  // authoritative — do not re-grant roles from sticky historical fields.
-  if (device.tokens && Object.keys(device.tokens).length > 0) {
-    return [];
-  }
-  // Legacy fallback: when no token map exists yet, treat the approved pairing
-  // roles as effective until token issuance has happened.
-  return listApprovedPairedDeviceRoles(device);
+  // Token entries are authoritative. Tokenless legacy records fail closed so
+  // sticky historical role fields cannot retain access after token migration.
+  return [];
 }
 
 export function hasEffectivePairedDeviceRole(
@@ -413,6 +406,25 @@ function scopesWithinApprovedDeviceBaseline(params: {
   });
 }
 
+function resolveScopeOutsideRequestedRoles(params: {
+  requestedRoles: readonly string[];
+  requestedScopes: readonly string[];
+}): string | null {
+  for (const scope of params.requestedScopes) {
+    const matchesRequestedRole = params.requestedRoles.some((role) =>
+      roleScopesAllow({
+        role,
+        requestedScopes: [scope],
+        allowedScopes: [scope],
+      }),
+    );
+    if (!matchesRequestedRole) {
+      return scope;
+    }
+  }
+  return null;
+}
+
 export async function listDevicePairing(baseDir?: string): Promise<DevicePairingList> {
   const state = await loadState(baseDir);
   const pending = Object.values(state.pendingById).toSorted((a, b) => b.ts - a.ts);
@@ -522,7 +534,15 @@ export async function approveDevicePairing(
       return null;
     }
     const requestedRoles = mergeRoles(pending.roles, pending.role) ?? [];
-    const requestedOperatorScopes = normalizeDeviceAuthScopes(pending.scopes).filter((scope) =>
+    const requestedScopes = normalizeDeviceAuthScopes(pending.scopes);
+    const roleMismatchScope = resolveScopeOutsideRequestedRoles({
+      requestedRoles,
+      requestedScopes,
+    });
+    if (roleMismatchScope) {
+      return { status: "forbidden", missingScope: roleMismatchScope };
+    }
+    const requestedOperatorScopes = requestedScopes.filter((scope) =>
       scope.startsWith(OPERATOR_SCOPE_PREFIX),
     );
     if (requestedOperatorScopes.length > 0) {
