diff --git a/CHANGELOG.md b/CHANGELOG.md index d5cbd2384..c2df44e39 100644 --- 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. diff --git a/src/gateway/server.auth.test.ts b/src/gateway/server.auth.test.ts index 8da0e18ef..83a97644d 100644 --- 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(); diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index 191278275..970832500 100644 --- 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) {