fix: require admin for node device approvals

steipete · steipete · commit 517ce3df75a9 · 2026-05-27T13:18:50.000+01:00
diff --git a/docs/cli/devices.md b/docs/cli/devices.md
@@ -71,6 +71,10 @@ matching client IPs can be approved before they appear in this list. That policy
 is disabled by default and never applies to operator/browser clients or upgrade
 requests.
 
+Approving a non-operator device role, such as `role: node`, requires
+`operator.admin`. `operator.pairing` is enough for operator-device approvals
+only when the requested operator scopes stay within the caller's own scopes.
+
 ```
 openclaw devices approve
 openclaw devices approve <requestId>
@@ -169,7 +173,8 @@ When you set `--url`, the CLI does not fall back to config or environment creden
 - Token rotation returns a new token (sensitive). Treat it like a secret.
 - These commands require `operator.pairing` (or `operator.admin`) scope. Some
   approvals also require the caller to hold the operator scopes that the target
-  device would mint or inherit; see [Operator scopes](/gateway/operator-scopes).
+  device would mint or inherit. Non-operator device roles require
+  `operator.admin`; see [Operator scopes](/gateway/operator-scopes).
 - `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 and revocation stay inside the approved pairing role set and
diff --git a/docs/gateway/operator-scopes.md b/docs/gateway/operator-scopes.md
@@ -69,6 +69,8 @@ for a broader role or broader scopes create a new pending upgrade request.
 When approving a device request:
 
 - A request with no operator role does not need operator token scope approval.
+- A request for a non-operator device role, such as `node`, requires
+  `operator.admin`.
 - A request for `operator.read`, `operator.write`, `operator.approvals`,
   `operator.pairing`, or `operator.talk.secrets` requires the caller to hold
   those scopes, or `operator.admin`.
@@ -77,10 +79,15 @@ When approving a device request:
   token scopes. If that existing token is admin-scoped, approval still requires
   `operator.admin`.
 
-For paired-device token sessions, management is self-scoped unless the caller
-also has `operator.admin`: non-admin callers see only their own pairing entries,
-can approve or reject only their own pending request, and can rotate, revoke, or
-remove only their own device entry.
+Non-admin sessions can approve operator-device requests only inside their own
+operator scopes. Approving non-operator roles is admin-only even for
+shared-secret or trusted-proxy sessions that can otherwise use
+`operator.pairing`.
+
+For paired-device token sessions, management is also self-scoped unless the
+caller has `operator.admin`: non-admin callers see only their own pairing
+entries, can approve or reject only their own pending request, and can rotate,
+revoke, or remove only their own device entry.
 
 ## Node pairing approvals
 
diff --git a/src/gateway/device-authz.test-helpers.ts b/src/gateway/device-authz.test-helpers.ts
@@ -109,8 +109,11 @@ export async function issueOperatorToken(params: {
   };
 }
 
-export async function openTrackedWs(port: number): Promise<WebSocket> {
-  const ws = new WebSocket(`ws://127.0.0.1:${port}`);
+export async function openTrackedWs(
+  port: number,
+  headers?: Record<string, string>,
+): Promise<WebSocket> {
+  const ws = new WebSocket(`ws://127.0.0.1:${port}`, headers ? { headers } : undefined);
   trackConnectChallengeNonce(ws);
   await new Promise<void>((resolve, reject) => {
     const timer = setTimeout(() => reject(new Error("timeout waiting for ws open")), 5_000);
diff --git a/src/gateway/server-methods/devices.test.ts b/src/gateway/server-methods/devices.test.ts
@@ -888,6 +888,116 @@ describe("deviceHandlers", () => {
     );
   });
 
+  it("allows non-device operator sessions to approve operator roles within caller scopes", async () => {
+    getPendingDevicePairingMock.mockResolvedValue({
+      requestId: "req-1",
+      deviceId: "device-2",
+      publicKey: "pk-2",
+      role: "operator",
+      roles: ["operator"],
+      scopes: ["operator.pairing"],
+      ts: 100,
+    });
+    approveDevicePairingMock.mockResolvedValue({
+      status: "approved",
+      requestId: "req-1",
+      device: {
+        deviceId: "device-2",
+        publicKey: "pk-2",
+        role: "operator",
+        roles: ["operator"],
+        approvedScopes: ["operator.pairing"],
+        approvedAtMs: 100,
+        createdAtMs: 50,
+      },
+    });
+    const opts = createOptions(
+      "device.pair.approve",
+      { requestId: "req-1" },
+      { client: createClient(["operator.pairing"]) },
+    );
+
+    await deviceHandlers["device.pair.approve"](opts);
+
+    expect(getPendingDevicePairingMock).toHaveBeenCalledWith("req-1");
+    expect(approveDevicePairingMock).toHaveBeenCalledWith("req-1", {
+      callerScopes: ["operator.pairing"],
+    });
+    expect(opts.respond).toHaveBeenCalledWith(
+      true,
+      {
+        requestId: "req-1",
+        device: {
+          deviceId: "device-2",
+          publicKey: "pk-2",
+          role: "operator",
+          roles: ["operator"],
+          approvedAtMs: 100,
+          createdAtMs: 50,
+          tokens: undefined,
+        },
+      },
+      undefined,
+    );
+  });
+
+  it("rejects approving node roles from non-admin shared-auth sessions", async () => {
+    getPendingDevicePairingMock.mockResolvedValue({
+      requestId: "req-1",
+      deviceId: "device-2",
+      publicKey: "pk-2",
+      role: "node",
+      roles: ["node"],
+      ts: 100,
+    });
+    const opts = createOptions(
+      "device.pair.approve",
+      { requestId: "req-1" },
+      { client: createClient(["operator.pairing"]) },
+    );
+
+    await deviceHandlers["device.pair.approve"](opts);
+
+    expect(approveDevicePairingMock).not.toHaveBeenCalled();
+    expectRespondedErrorMessage(opts, "device pairing approval denied");
+  });
+
+  it("rejects approving mixed operator and node roles from non-admin sessions", async () => {
+    getPendingDevicePairingMock.mockResolvedValue({
+      requestId: "req-1",
+      deviceId: "device-2",
+      publicKey: "pk-2",
+      role: "operator",
+      roles: [" operator ", " node "],
+      scopes: ["operator.pairing"],
+      ts: 100,
+    });
+    const opts = createOptions(
+      "device.pair.approve",
+      { requestId: "req-1" },
+      { client: createClient(["operator.pairing"]) },
+    );
+
+    await deviceHandlers["device.pair.approve"](opts);
+
+    expect(approveDevicePairingMock).not.toHaveBeenCalled();
+    expectRespondedErrorMessage(opts, "device pairing approval denied");
+  });
+
+  it("denies unknown approvals from non-admin non-device sessions", async () => {
+    getPendingDevicePairingMock.mockResolvedValue(null);
+    const opts = createOptions(
+      "device.pair.approve",
+      { requestId: "missing" },
+      { client: createClient(["operator.pairing"]) },
+    );
+
+    await deviceHandlers["device.pair.approve"](opts);
+
+    expect(approveDevicePairingMock).not.toHaveBeenCalled();
+    expectRespondedErrorMessage(opts, "device pairing approval denied");
+  });
+
   it("rejects approving node roles for the caller device without admin scope", async () => {
     getPendingDevicePairingMock.mockResolvedValue({
       requestId: "req-1",
diff --git a/src/gateway/server-methods/devices.ts b/src/gateway/server-methods/devices.ts
@@ -222,7 +222,7 @@ export const deviceHandlers: GatewayRequestHandlers = {
     }
     const { requestId } = params as { requestId: string };
     const authz = resolveDeviceSessionAuthz(client);
-    if (authz.callerDeviceId && !authz.isAdminCaller) {
+    if (!authz.isAdminCaller) {
       const pending = await getPendingDevicePairing(requestId);
       if (!pending) {
         respond(
@@ -232,7 +232,7 @@ export const deviceHandlers: GatewayRequestHandlers = {
         );
         return;
       }
-      if (pending.deviceId.trim() !== authz.callerDeviceId) {
+      if (authz.callerDeviceId && pending.deviceId.trim() !== authz.callerDeviceId) {
         context.logGateway.warn(
           `device pairing approval denied request=${requestId} reason=device-ownership-mismatch`,
         );
diff --git a/src/gateway/server.device-pair-approve-authz.test.ts b/src/gateway/server.device-pair-approve-authz.test.ts
