fix(gateway): require pairing for unpaired operator device auth

2026-02-26 00:51:34 +01:00
parent 2aa7842ade
commit 8d1481cb4a
3 changed files with 49 additions and 29 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai
 - Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
 - Gateway/Message media roots: thread `agentId` through gateway `send` RPC and prefer explicit `agentId` over session/default resolution so non-default agent workspace media sends no longer fail with `LocalMediaAccessError`; added regression coverage for agent precedence and blank-agent fallback. (#23249) Thanks @Sid-Qin.
 - Cron/Model overrides: when isolated `payload.model` is no longer allowlisted, fall back to default model selection instead of failing the job, while still returning explicit errors for invalid model strings. (#26717) Thanks @Youyou972.
+- Security/Gateway auth: require pairing for operator device-identity sessions authenticated with shared token auth so unpaired devices cannot self-assign operator scopes. Thanks @tdjackey for reporting.
 - Security/Nextcloud Talk: reject unsigned webhook traffic before full body reads, reducing unauthenticated request-body exposure, with auth-order regression coverage. (#26118) Thanks @bmendonca3.
 - Security/Nextcloud Talk: stop treating DM pairing-store entries as group allowlist senders, so group authorization remains bounded to configured group allowlists. (#26116) Thanks @bmendonca3.
 - Security/IRC: keep pairing-store approvals DM-only and out of IRC group allowlist authorization, with policy regression tests for allowlist resolution. (#26112) Thanks @bmendonca3.
--- a/src/gateway/server.auth.test.ts
+++ b/src/gateway/server.auth.test.ts
@@ -1065,7 +1065,7 @@ describe("gateway server auth/connect", () => {
    }
  });

-  test("skips pairing for operator scope upgrades when shared token auth is valid", async () => {
+  test("requires pairing for remote operator device identity with shared token auth", async () => {
    const { mkdtemp } = await import("node:fs/promises");
    const { tmpdir } = await import("node:os");
    const { join } = await import("node:path");
@@ -1102,21 +1102,29 @@ describe("gateway server auth/connect", () => {
        nonce,
      };
    };
-    const initialNonce = await readConnectChallengeNonce(ws);
-    const initial = await connectReq(ws, {
+    ws.close();
+
+    const wsRemoteRead = await openWs(port, { host: "gateway.example" });
+    const initialNonce = await readConnectChallengeNonce(wsRemoteRead);
+    const initial = await connectReq(wsRemoteRead, {
      token: "secret",
      scopes: ["operator.read"],
      client,
      device: buildDevice(["operator.read"], initialNonce),
    });
-    expect(initial.ok).toBe(true);
+    expect(initial.ok).toBe(false);
+    expect(initial.error?.message ?? "").toContain("pairing required");
    let pairing = await listDevicePairing();
-    expect(pairing.pending.filter((entry) => entry.deviceId === identity.deviceId)).toEqual([]);
+    const pendingAfterRead = pairing.pending.filter(
+      (entry) => entry.deviceId === identity.deviceId,
+    );
+    expect(pendingAfterRead).toHaveLength(1);
+    expect(pendingAfterRead[0]?.role).toBe("operator");
+    expect(pendingAfterRead[0]?.scopes ?? []).toContain("operator.read");
    expect(await getPairedDevice(identity.deviceId)).toBeNull();
+    wsRemoteRead.close();

-    ws.close();
-
-    const ws2 = await openWs(port);
+    const ws2 = await openWs(port, { host: "gateway.example" });
    const nonce2 = await readConnectChallengeNonce(ws2);
    const res = await connectReq(ws2, {
      token: "secret",
@@ -1124,9 +1132,16 @@ describe("gateway server auth/connect", () => {
      client,
      device: buildDevice(["operator.admin"], nonce2),
    });
-    expect(res.ok).toBe(true);
+    expect(res.ok).toBe(false);
+    expect(res.error?.message ?? "").toContain("pairing required");
    pairing = await listDevicePairing();
-    expect(pairing.pending.filter((entry) => entry.deviceId === identity.deviceId)).toEqual([]);
+    const pendingAfterAdmin = pairing.pending.filter(
+      (entry) => entry.deviceId === identity.deviceId,
+    );
+    expect(pendingAfterAdmin).toHaveLength(1);
+    expect(pendingAfterAdmin[0]?.scopes ?? []).toEqual(
+      expect.arrayContaining(["operator.read", "operator.admin"]),
+    );
    expect(await getPairedDevice(identity.deviceId)).toBeNull();
    ws2.close();
    await server.close();
@@ -1199,7 +1214,7 @@ describe("gateway server auth/connect", () => {
    restoreGatewayToken(prevToken);
  });

-  test("still requires node pairing while operator shared auth succeeds for the same device", async () => {
+  test("merges remote node/operator pairing requests for the same unpaired device", async () => {
    const { mkdtemp } = await import("node:fs/promises");
    const { tmpdir } = await import("node:os");
    const { join } = await import("node:path");
@@ -1266,23 +1281,25 @@ describe("gateway server auth/connect", () => {
    expect(nodeConnect.error?.message ?? "").toContain("pairing required");

    const operatorConnect = await connectWithNonce("operator", ["operator.read", "operator.write"]);
-    expect(operatorConnect.ok).toBe(true);
+    expect(operatorConnect.ok).toBe(false);
+    expect(operatorConnect.error?.message ?? "").toContain("pairing required");

    const pending = await listDevicePairing();
    const pendingForTestDevice = pending.pending.filter(
      (entry) => entry.deviceId === identity.deviceId,
    );
    expect(pendingForTestDevice).toHaveLength(1);
-    expect(pendingForTestDevice[0]?.roles).toEqual(expect.arrayContaining(["node"]));
-    expect(pendingForTestDevice[0]?.roles ?? []).not.toContain("operator");
+    expect(pendingForTestDevice[0]?.roles).toEqual(expect.arrayContaining(["node", "operator"]));
+    expect(pendingForTestDevice[0]?.scopes ?? []).toEqual(
+      expect.arrayContaining(["operator.read", "operator.write"]),
+    );
    if (!pendingForTestDevice[0]) {
      throw new Error("expected pending pairing request");
    }
    await approveDevicePairing(pendingForTestDevice[0].requestId);

    const paired = await getPairedDevice(identity.deviceId);
-    expect(paired?.roles).toEqual(expect.arrayContaining(["node"]));
-    expect(paired?.roles ?? []).not.toContain("operator");
+    expect(paired?.roles).toEqual(expect.arrayContaining(["node", "operator"]));

    const approvedOperatorConnect = await connectWithNonce("operator", ["operator.read"]);
    expect(approvedOperatorConnect.ok).toBe(true);
@@ -1438,8 +1455,8 @@ describe("gateway server auth/connect", () => {
      expect(reconnect.ok).toBe(true);

      const repaired = await getPairedDevice(deviceId);
-      expect(repaired?.roles).toBeUndefined();
-      expect(repaired?.scopes).toBeUndefined();
+      expect(repaired?.roles ?? []).toContain("operator");
+      expect(repaired?.scopes ?? []).toContain("operator.read");
      const list = await listDevicePairing();
      expect(list.pending.filter((entry) => entry.deviceId === deviceId)).toEqual([]);
    } finally {
@@ -1450,7 +1467,7 @@ describe("gateway server auth/connect", () => {
    }
  });

-  test("allows shared-auth scope escalation even when paired metadata is legacy-shaped", async () => {
+  test("auto-approves local scope upgrades even when paired metadata is legacy-shaped", async () => {
    const { mkdtemp } = await import("node:fs/promises");
    const { tmpdir } = await import("node:os");
    const { join } = await import("node:path");
@@ -1539,9 +1556,13 @@ describe("gateway server auth/connect", () => {
      expect(pendingUpgrade).toBeUndefined();
      const repaired = await getPairedDevice(identity.deviceId);
      expect(repaired?.role).toBe("operator");
-      expect(repaired?.roles).toBeUndefined();
-      expect(repaired?.scopes).toBeUndefined();
-      expect(repaired?.approvedScopes).not.toContain("operator.admin");
+      expect(repaired?.roles ?? []).toContain("operator");
+      expect(repaired?.scopes ?? []).toEqual(
+        expect.arrayContaining(["operator.read", "operator.admin"]),
+      );
+      expect(repaired?.approvedScopes ?? []).toEqual(
+        expect.arrayContaining(["operator.read", "operator.admin"]),
+      );
    } finally {
      ws.close();
      ws2?.close();
--- a/src/gateway/server/ws-connection/message-handler.ts
+++ b/src/gateway/server/ws-connection/message-handler.ts
@@ -565,18 +565,16 @@ export function attachGatewayWsMessageHandler(params: {
          return;
        }

-        // Shared token/password auth is already gateway-level trust for operator clients.
-        // In that case, don't force device pairing on first connect.
-        const skipPairingForOperatorSharedAuth =
-          role === "operator" && sharedAuthOk && !isControlUi && !isWebchat;
        const trustedProxyAuthOk =
          isControlUi &&
          resolvedAuth.mode === "trusted-proxy" &&
          authOk &&
          authMethod === "trusted-proxy";
-        const skipPairing =
-          shouldSkipControlUiPairing(controlUiAuthPolicy, sharedAuthOk, trustedProxyAuthOk) ||
-          skipPairingForOperatorSharedAuth;
+        const skipPairing = shouldSkipControlUiPairing(
+          controlUiAuthPolicy,
+          sharedAuthOk,
+          trustedProxyAuthOk,
+        );
        if (device && devicePublicKey && !skipPairing) {
          const formatAuditList = (items: string[] | undefined): string => {
            if (!items || items.length === 0) {