diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bc48b65841..85d61df698d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai ### Security - Security/exec approvals: escape invisible Unicode format characters in approval prompts so zero-width command text renders as visible `\u{...}` escapes instead of spoofing the reviewed command. (#43687) Thanks @EkiXu and @vincentkoc. +- Security/device pairing: cap issued and verified device-token scopes to each paired device's approved scope baseline so stale or overbroad tokens cannot exceed approved access. (#43686) Thanks @tdjackey and @vincentkoc. ### Changes diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index c76b44b323d..915e06bb9c6 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -1,16 +1,19 @@ -import { mkdtemp } from "node:fs/promises"; +import { mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, test } from "vitest"; import { approveDevicePairing, clearDevicePairing, + ensureDeviceToken, getPairedDevice, removePairedDevice, requestDevicePairing, rotateDeviceToken, verifyDeviceToken, + type PairedDevice, } from "./device-pairing.js"; +import { resolvePairingPaths } from "./pairing-files.js"; async function setupPairedOperatorDevice(baseDir: string, scopes: string[]) { const request = await requestDevicePairing( @@ -51,6 +54,21 @@ function requireToken(token: string | undefined): string { return token; } +async function overwritePairedOperatorTokenScopes(baseDir: string, scopes: string[]) { + const { pairedPath } = resolvePairingPaths(baseDir, "devices"); + const pairedByDeviceId = JSON.parse(await readFile(pairedPath, "utf8")) as Record< + string, + PairedDevice + >; + const device = pairedByDeviceId["device-1"]; + expect(device?.tokens?.operator).toBeDefined(); + if (!device?.tokens?.operator) { + throw new Error("expected paired operator token"); + } + device.tokens.operator.scopes = scopes; + await writeFile(pairedPath, JSON.stringify(pairedByDeviceId, null, 2)); +} + describe("device pairing tokens", () => { test("reuses existing pending requests for the same device", async () => { const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); @@ -180,6 +198,26 @@ describe("device pairing tokens", () => { expect(after?.approvedScopes).toEqual(["operator.read"]); }); + test("rejects scope escalation when ensuring a token and leaves state unchanged", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); + await setupPairedOperatorDevice(baseDir, ["operator.read"]); + const before = await getPairedDevice("device-1", baseDir); + + const ensured = await ensureDeviceToken({ + deviceId: "device-1", + role: "operator", + scopes: ["operator.admin"], + baseDir, + }); + expect(ensured).toBeNull(); + + const after = await getPairedDevice("device-1", baseDir); + expect(after?.tokens?.operator?.token).toEqual(before?.tokens?.operator?.token); + expect(after?.tokens?.operator?.scopes).toEqual(["operator.read"]); + expect(after?.scopes).toEqual(["operator.read"]); + expect(after?.approvedScopes).toEqual(["operator.read"]); + }); + test("verifies token and rejects mismatches", async () => { const { baseDir, token } = await setupOperatorToken(["operator.read"]); @@ -199,6 +237,19 @@ describe("device pairing tokens", () => { expect(mismatch.reason).toBe("token-mismatch"); }); + test("rejects persisted tokens whose scopes exceed the approved scope baseline", async () => { + const { baseDir, token } = await setupOperatorToken(["operator.read"]); + await overwritePairedOperatorTokenScopes(baseDir, ["operator.admin"]); + + await expect( + verifyOperatorToken({ + baseDir, + token, + scopes: ["operator.admin"], + }), + ).resolves.toEqual({ ok: false, reason: "scope-mismatch" }); + }); + test("accepts operator.read/operator.write requests with an operator.admin token scope", async () => { const { baseDir, token } = await setupOperatorToken(["operator.admin"]); diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index 591a9d70888..9d994a308f2 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -494,6 +494,12 @@ export async function verifyDeviceToken(params: { if (!verifyPairingToken(params.token, entry.token)) { return { ok: false, reason: "token-mismatch" }; } + const approvedScopes = normalizeDeviceAuthScopes( + device.approvedScopes ?? device.scopes ?? entry.scopes, + ); + if (!scopesAllowWithImplications(entry.scopes, approvedScopes)) { + return { ok: false, reason: "scope-mismatch" }; + } const requestedScopes = normalizeDeviceAuthScopes(params.scopes); if (!roleScopesAllow({ role, requestedScopes, allowedScopes: entry.scopes })) { return { ok: false, reason: "scope-mismatch" }; @@ -525,8 +531,18 @@ export async function ensureDeviceToken(params: { return null; } const { device, role, tokens, existing } = context; + const approvedScopes = normalizeDeviceAuthScopes( + device.approvedScopes ?? device.scopes ?? existing?.scopes, + ); + if (!scopesAllowWithImplications(requestedScopes, approvedScopes)) { + return null; + } if (existing && !existing.revokedAtMs) { - if (roleScopesAllow({ role, requestedScopes, allowedScopes: existing.scopes })) { + const existingWithinApproved = scopesAllowWithImplications(existing.scopes, approvedScopes); + if ( + existingWithinApproved && + roleScopesAllow({ role, requestedScopes, allowedScopes: existing.scopes }) + ) { return existing; } }