From b8372a714ccc5d1d8a26a12d3e8d941687d664b7 Mon Sep 17 00:00:00 2001 From: Agustin Rivera <31522568+eleqtrizit@users.noreply.github.com> Date: Wed, 29 Apr 2026 13:11:16 -0700 Subject: [PATCH] fix(auth): bound bootstrap handoff scopes (#72919) * fix(auth): bound bootstrap handoff scopes Co-authored-by: zsx * fix(auth): log stripped bootstrap scopes * docs: add changelog entry for bootstrap handoff scope bounds --------- Co-authored-by: zsx Co-authored-by: Devin Robison --- CHANGELOG.md | 1 + src/infra/device-bootstrap.test.ts | 92 ++++++++++++++++ src/infra/device-bootstrap.ts | 71 ++++++++++--- src/infra/device-pairing.test.ts | 112 ++++++++++++++++++++ src/infra/device-pairing.ts | 14 ++- src/shared/device-bootstrap-profile.test.ts | 34 ++++++ src/shared/device-bootstrap-profile.ts | 20 ++++ 7 files changed, 324 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 724627e9f8d..52c90ee26fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -230,6 +230,7 @@ Docs: https://docs.openclaw.ai - Gateway/startup: skip inherited workspace startup memory for sandboxed spawned sessions without real-workspace write access, so `/new` no longer preloads host workspace memory into isolated child runs. (#73611) Thanks @drobison00. - Agents/tool policy: validate caller group IDs against session or spawned context before applying group-scoped tool policies or persisting gateway group metadata, so forged group IDs cannot unlock more permissive tools. (#73720) Thanks @mmaps. - Commands: keep channel-prefixed owner allowlist entries scoped to matching providers so webchat command contexts cannot inherit external channel owners. Thanks @zsxsoft. +- Auth/device pairing: bound bootstrap handoff token issuance, redemption, and approved pairing baselines to the documented per-role scope allowlist, so bootstrap approvals cannot persistently grant `operator.admin`, `operator.pairing`, or `node.exec` scopes. Thanks @eleqtrizit. ## 2026.4.27 diff --git a/src/infra/device-bootstrap.test.ts b/src/infra/device-bootstrap.test.ts index 9f8b379e8b2..ccba69c1518 100644 --- a/src/infra/device-bootstrap.test.ts +++ b/src/infra/device-bootstrap.test.ts @@ -1,6 +1,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { resetLogger, setLoggerOverride } from "../logging.js"; import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js"; import { clearDeviceBootstrapTokens, @@ -40,6 +41,8 @@ async function verifyBootstrapToken( afterEach(async () => { vi.useRealTimers(); + resetLogger(); + setLoggerOverride(null); await tempDirs.cleanup(); }); @@ -329,6 +332,95 @@ describe("device bootstrap tokens", () => { ).resolves.toEqual({ ok: true }); }); + it("bounds explicitly issued bootstrap profiles to handoff scopes", async () => { + const baseDir = await createTempDir(); + const issued = await issueDeviceBootstrapToken({ + baseDir, + profile: { + roles: ["node", "operator"], + scopes: [ + "node.exec", + "operator.admin", + "operator.approvals", + "operator.pairing", + "operator.read", + "operator.talk.secrets", + "operator.write", + ], + }, + }); + + await expect(getDeviceBootstrapTokenProfile({ baseDir, token: issued.token })).resolves.toEqual( + { + roles: ["node", "operator"], + scopes: ["operator.approvals", "operator.read", "operator.talk.secrets", "operator.write"], + }, + ); + await expect( + verifyBootstrapToken(baseDir, issued.token, { + role: "operator", + scopes: ["operator.admin"], + }), + ).resolves.toEqual({ ok: false, reason: "bootstrap_token_invalid" }); + }); + + it("logs when issued bootstrap profiles strip overbroad scopes", async () => { + const baseDir = await createTempDir(); + const logPath = path.join(baseDir, "bootstrap.log"); + setLoggerOverride({ level: "warn", consoleLevel: "silent", file: logPath }); + + await issueDeviceBootstrapToken({ + baseDir, + profile: { + roles: ["node", "operator"], + scopes: ["node.exec", "operator.admin", "operator.read"], + }, + }); + + const content = await fs.readFile(logPath, "utf8"); + expect(content).toContain("bootstrap_token_scopes_stripped"); + expect(content).toContain("node.exec"); + expect(content).toContain("operator.admin"); + expect(content).toContain("operator.read"); + }); + + it("bounds redeemed bootstrap profiles to handoff scopes", async () => { + const baseDir = await createTempDir(); + const issued = await issueDeviceBootstrapToken({ + baseDir, + profile: { + roles: ["operator"], + scopes: ["operator.approvals", "operator.read", "operator.talk.secrets", "operator.write"], + }, + }); + + await expect( + redeemDeviceBootstrapTokenProfile({ + baseDir, + token: issued.token, + role: "operator", + scopes: [ + "operator.admin", + "operator.approvals", + "operator.pairing", + "operator.read", + "operator.talk.secrets", + "operator.write", + ], + }), + ).resolves.toEqual({ recorded: true, fullyRedeemed: true }); + + const raw = await fs.readFile(resolveBootstrapPath(baseDir), "utf8"); + const parsed = JSON.parse(raw) as Record< + string, + { redeemedProfile?: { roles?: string[]; scopes?: string[] } } + >; + expect(parsed[issued.token]?.redeemedProfile).toEqual({ + roles: ["operator"], + scopes: ["operator.approvals", "operator.read", "operator.talk.secrets", "operator.write"], + }); + }); + it("accepts trimmed bootstrap tokens and binds them", async () => { const baseDir = await createTempDir(); const issued = await issueDeviceBootstrapToken({ baseDir }); diff --git a/src/infra/device-bootstrap.ts b/src/infra/device-bootstrap.ts index e2357a1b42f..df589280cb7 100644 --- a/src/infra/device-bootstrap.ts +++ b/src/infra/device-bootstrap.ts @@ -1,7 +1,10 @@ import path from "node:path"; +import { createSubsystemLogger } from "../logging/subsystem.js"; import { + normalizeDeviceBootstrapHandoffProfile, normalizeDeviceBootstrapProfile, PAIRING_SETUP_BOOTSTRAP_PROFILE, + resolveBootstrapProfileScopesForRole, type DeviceBootstrapProfile, type DeviceBootstrapProfileInput, } from "../shared/device-bootstrap-profile.js"; @@ -34,11 +37,29 @@ export type DeviceBootstrapTokenRecord = { type DeviceBootstrapStateFile = Record; const withLock = createAsyncLock(); +const log = createSubsystemLogger("device-bootstrap"); function resolveBootstrapPath(baseDir?: string): string { return path.join(resolvePairingPaths(baseDir, "devices").dir, "bootstrap.json"); } +function resolveIssuedBootstrapProfileInput(params: { + profile?: DeviceBootstrapProfileInput; + roles?: readonly string[]; + scopes?: readonly string[]; +}): DeviceBootstrapProfileInput | undefined { + if (params.profile) { + return params.profile; + } + if (params.roles || params.scopes) { + return { + roles: params.roles, + scopes: params.scopes, + }; + } + return undefined; +} + function resolvePersistedBootstrapProfile( record: Partial, ): DeviceBootstrapProfile { @@ -56,18 +77,39 @@ function resolveIssuedBootstrapProfile(params: { roles?: readonly string[]; scopes?: readonly string[]; }): DeviceBootstrapProfile { - if (params.profile) { - return normalizeDeviceBootstrapProfile(params.profile); - } - if (params.roles || params.scopes) { - return normalizeDeviceBootstrapProfile({ - roles: params.roles, - scopes: params.scopes, - }); + const input = resolveIssuedBootstrapProfileInput(params); + if (input) { + return normalizeDeviceBootstrapHandoffProfile(input); } return PAIRING_SETUP_BOOTSTRAP_PROFILE; } +function warnIfIssuedBootstrapScopesWereStripped(params: { + input: DeviceBootstrapProfileInput | undefined; + profile: DeviceBootstrapProfile; +}): void { + if (!params.input) { + return; + } + const requestedProfile = normalizeDeviceBootstrapProfile(params.input); + const requestedScopes = requestedProfile.scopes; + if (requestedScopes.length === 0) { + return; + } + const retainedScopeSet = new Set(params.profile.scopes); + const strippedScopes = requestedScopes.filter((scope) => !retainedScopeSet.has(scope)); + if (strippedScopes.length === 0) { + return; + } + log.warn("bootstrap_token_scopes_stripped", { + roles: requestedProfile.roles, + requestedScopes, + retainedScopes: params.profile.scopes, + strippedScopes, + consoleMessage: "bootstrap token scopes stripped to bootstrap handoff allowlist", + }); +} + function bootstrapProfileAllowsRequest(params: { allowedProfile: DeviceBootstrapProfile; requestedRole: string; @@ -83,13 +125,6 @@ function bootstrapProfileAllowsRequest(params: { ); } -function resolveBootstrapProfileScopes(role: string, scopes: readonly string[]): string[] { - if (role === "operator") { - return scopes.filter((scope) => scope.startsWith("operator.")); - } - return scopes.filter((scope) => !scope.startsWith("operator.")); -} - function bootstrapProfileSatisfiesProfile(params: { actualProfile: DeviceBootstrapProfile; requiredProfile: DeviceBootstrapProfile; @@ -98,7 +133,7 @@ function bootstrapProfileSatisfiesProfile(params: { if (!params.actualProfile.roles.includes(requiredRole)) { return false; } - const requiredScopes = resolveBootstrapProfileScopes( + const requiredScopes = resolveBootstrapProfileScopesForRole( requiredRole, params.requiredProfile.scopes, ); @@ -175,7 +210,9 @@ export async function issueDeviceBootstrapToken( const state = await loadState(params.baseDir); const token = generatePairingToken(); const issuedAtMs = Date.now(); + const profileInput = resolveIssuedBootstrapProfileInput(params); const profile = resolveIssuedBootstrapProfile(params); + warnIfIssuedBootstrapScopesWereStripped({ input: profileInput, profile }); state[token] = { token, ts: issuedAtMs, @@ -276,7 +313,7 @@ export async function redeemDeviceBootstrapTokenProfile(params: { roles: [...resolvePersistedRedeemedProfile(record).roles, params.role], scopes: [ ...resolvePersistedRedeemedProfile(record).scopes, - ...resolveBootstrapProfileScopes(params.role, params.scopes), + ...resolveBootstrapProfileScopesForRole(params.role, params.scopes), ], }); state[tokenKey] = { diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index b9c9f6bf6ab..c98d439606d 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -948,6 +948,118 @@ describe("device pairing tokens", () => { expect(paired?.tokens?.node?.scopes).toEqual([]); }); + test("bootstrap pairing bounds approved baseline to handoff scopes", async () => { + const baseDir = await makeDevicePairingDir(); + const request = await requestDevicePairing( + { + deviceId: "bootstrap-device-bounded-baseline", + publicKey: "bootstrap-public-key-bounded-baseline", + role: "node", + roles: ["node", "operator"], + scopes: [], + silent: true, + }, + baseDir, + ); + + await expect( + approveBootstrapDevicePairing( + request.request.requestId, + { + roles: ["node", "operator"], + scopes: [ + "node.exec", + "operator.admin", + "operator.approvals", + "operator.pairing", + "operator.read", + "operator.talk.secrets", + "operator.write", + ], + }, + baseDir, + ), + ).resolves.toEqual(expect.objectContaining({ status: "approved" })); + + const paired = await getPairedDevice("bootstrap-device-bounded-baseline", baseDir); + expect(paired?.approvedScopes).toEqual([ + "operator.approvals", + "operator.read", + "operator.talk.secrets", + "operator.write", + ]); + expect(paired?.tokens?.operator?.scopes).toEqual([ + "operator.approvals", + "operator.read", + "operator.talk.secrets", + "operator.write", + ]); + expect(paired?.tokens?.node?.scopes).toEqual([]); + await expect( + ensureDeviceToken({ + deviceId: "bootstrap-device-bounded-baseline", + role: "operator", + scopes: ["operator.admin"], + baseDir, + }), + ).resolves.toBeNull(); + }); + + test("bootstrap pairing sanitizes merged legacy baseline scopes", async () => { + const baseDir = await makeDevicePairingDir(); + const first = await requestDevicePairing( + { + deviceId: "bootstrap-device-legacy-baseline", + publicKey: "bootstrap-public-key-legacy-baseline", + role: "node", + roles: ["node", "operator"], + scopes: [], + silent: true, + }, + baseDir, + ); + + await approveBootstrapDevicePairing( + first.request.requestId, + PAIRING_SETUP_BOOTSTRAP_PROFILE, + baseDir, + ); + await mutatePairedDevice(baseDir, "bootstrap-device-legacy-baseline", (device) => { + device.approvedScopes = ["operator.admin"]; + device.scopes = ["operator.admin"]; + }); + + const repair = await requestDevicePairing( + { + deviceId: "bootstrap-device-legacy-baseline", + publicKey: "bootstrap-public-key-legacy-baseline-rotated", + role: "node", + roles: ["node", "operator"], + scopes: [], + silent: true, + }, + baseDir, + ); + await expect( + approveBootstrapDevicePairing( + repair.request.requestId, + PAIRING_SETUP_BOOTSTRAP_PROFILE, + baseDir, + ), + ).resolves.toEqual(expect.objectContaining({ status: "approved" })); + + const paired = await getPairedDevice("bootstrap-device-legacy-baseline", baseDir); + expect(paired?.approvedScopes).toEqual(PAIRING_SETUP_BOOTSTRAP_PROFILE.scopes); + await expect( + ensureDeviceToken({ + deviceId: "bootstrap-device-legacy-baseline", + role: "operator", + scopes: ["operator.admin"], + baseDir, + }), + ).resolves.toBeNull(); + }); + test("verifies token and rejects mismatches", async () => { const { baseDir, token } = await setupOperatorToken(["operator.read"]); diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index 2a44e3deb5f..26d53909f48 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"; import { normalizeDeviceAuthScopes } from "../shared/device-auth.js"; import { resolveBootstrapProfileScopesForRole, + resolveBootstrapProfileScopesForRoles, type DeviceBootstrapProfile, } from "../shared/device-bootstrap-profile.js"; import { @@ -676,7 +677,10 @@ export async function approveBootstrapDevicePairing( // node/operator baseline from the verified bootstrap profile without routing // operator scope approval through the generic interactive approval checker. const approvedRoles = mergeRoles(bootstrapProfile.roles) ?? []; - const approvedScopes = normalizeDeviceAuthScopes([...bootstrapProfile.scopes]); + const approvedScopes = resolveBootstrapProfileScopesForRoles( + approvedRoles, + bootstrapProfile.scopes, + ); return await withLock(async () => { const state = await loadState(baseDir); const pending = state.pendingById[requestId]; @@ -714,6 +718,10 @@ export async function approveBootstrapDevicePairing( pending.scopes, approvedScopes, ); + const sanitizedApprovedScopes = resolveBootstrapProfileScopesForRoles( + approvedRoles, + nextApprovedScopes ?? [], + ); const tokens = existing?.tokens ? { ...existing.tokens } : {}; for (const roleForToken of approvedRoles) { const existingToken = tokens[roleForToken]; @@ -740,8 +748,8 @@ export async function approveBootstrapDevicePairing( clientMode: pending.clientMode, role: pending.role, roles, - scopes: nextApprovedScopes, - approvedScopes: nextApprovedScopes, + scopes: sanitizedApprovedScopes, + approvedScopes: sanitizedApprovedScopes, remoteIp: pending.remoteIp, tokens, createdAtMs: existing?.createdAtMs ?? now, diff --git a/src/shared/device-bootstrap-profile.test.ts b/src/shared/device-bootstrap-profile.test.ts index 8d8e2345fc5..b2890aa8d19 100644 --- a/src/shared/device-bootstrap-profile.test.ts +++ b/src/shared/device-bootstrap-profile.test.ts @@ -1,7 +1,9 @@ import { describe, expect, test } from "vitest"; import { BOOTSTRAP_HANDOFF_OPERATOR_SCOPES, + normalizeDeviceBootstrapHandoffProfile, resolveBootstrapProfileScopesForRole, + resolveBootstrapProfileScopesForRoles, } from "./device-bootstrap-profile.js"; describe("device bootstrap profile", () => { @@ -22,6 +24,38 @@ describe("device bootstrap profile", () => { ).toEqual([]); }); + test("bounds bootstrap handoff scopes across profile roles", () => { + expect( + resolveBootstrapProfileScopesForRoles( + ["node", "operator"], + ["node.exec", "operator.admin", "operator.approvals", "operator.read", "operator.write"], + ), + ).toEqual(["operator.approvals", "operator.read", "operator.write"]); + + expect( + resolveBootstrapProfileScopesForRoles(["node"], ["node.exec", "operator.admin"]), + ).toEqual([]); + }); + + test("normalizes issued handoff profiles to the bootstrap allowlist", () => { + expect( + normalizeDeviceBootstrapHandoffProfile({ + roles: ["node", "operator"], + scopes: [ + "node.exec", + "operator.admin", + "operator.approvals", + "operator.pairing", + "operator.read", + "operator.write", + ], + }), + ).toEqual({ + roles: ["node", "operator"], + scopes: ["operator.approvals", "operator.read", "operator.write"], + }); + }); + test("bootstrap handoff operator allowlist stays aligned with pairing setup profile", () => { expect([...BOOTSTRAP_HANDOFF_OPERATOR_SCOPES]).toEqual([ "operator.approvals", diff --git a/src/shared/device-bootstrap-profile.ts b/src/shared/device-bootstrap-profile.ts index f1bd7721611..48c713dacd3 100644 --- a/src/shared/device-bootstrap-profile.ts +++ b/src/shared/device-bootstrap-profile.ts @@ -36,6 +36,26 @@ export function resolveBootstrapProfileScopesForRole( return []; } +export function resolveBootstrapProfileScopesForRoles( + roles: readonly string[], + scopes: readonly string[], +): string[] { + return normalizeDeviceAuthScopes( + roles.flatMap((role) => resolveBootstrapProfileScopesForRole(role, scopes)), + ); +} + +export function normalizeDeviceBootstrapHandoffProfile( + input: DeviceBootstrapProfileInput | undefined, +): DeviceBootstrapProfile { + const profile = normalizeDeviceBootstrapProfile(input); + // Bootstrap handoff profiles can only carry the documented handoff allowlist. + return { + roles: profile.roles, + scopes: resolveBootstrapProfileScopesForRoles(profile.roles, profile.scopes), + }; +} + function normalizeBootstrapRoles(roles: readonly string[] | undefined): string[] { if (!Array.isArray(roles)) { return [];