mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 23:58:25 +00:00
fix: Device Token Scope Escalation via Rotate Endpoint (#20703)
Merged via /review-pr -> /prepare-pr -> /merge-pr.
Prepared head SHA: 4f2c2ecef4
Co-authored-by: coygeek <65363919+coygeek@users.noreply.github.com>
Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com>
Reviewed-by: @mbelinky
This commit is contained in:
@@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
|
|
||||||
|
- Gateway/Pairing: prevent device-token rotate scope escalation by enforcing an approved-scope baseline, preserving approved scopes across metadata updates, and rejecting rotate requests that exceed approved role scope implications. (#20703) thanks @coygeek.
|
||||||
- Gateway/Security: require secure context and paired-device checks for Control UI auth even when `gateway.controlUi.allowInsecureAuth` is set, and align audit messaging with the hardened behavior. (#20684) thanks @coygeek.
|
- Gateway/Security: require secure context and paired-device checks for Control UI auth even when `gateway.controlUi.allowInsecureAuth` is set, and align audit messaging with the hardened behavior. (#20684) thanks @coygeek.
|
||||||
- macOS/Build: default release packaging to `BUNDLE_ID=ai.openclaw.mac` in `scripts/package-mac-dist.sh`, so Sparkle feed URL is retained and auto-update no longer fails with an empty appcast feed. (#19750) thanks @loganprit.
|
- macOS/Build: default release packaging to `BUNDLE_ID=ai.openclaw.mac` in `scripts/package-mac-dist.sh`, so Sparkle feed URL is retained and auto-update no longer fails with an empty appcast feed. (#19750) thanks @loganprit.
|
||||||
|
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ import type { GatewayRequestHandlers } from "./types.js";
|
|||||||
function redactPairedDevice(
|
function redactPairedDevice(
|
||||||
device: { tokens?: Record<string, DeviceAuthToken> } & Record<string, unknown>,
|
device: { tokens?: Record<string, DeviceAuthToken> } & Record<string, unknown>,
|
||||||
) {
|
) {
|
||||||
const { tokens, ...rest } = device;
|
const { tokens, approvedScopes: _approvedScopes, ...rest } = device;
|
||||||
return {
|
return {
|
||||||
...rest,
|
...rest,
|
||||||
tokens: summarizeDeviceTokens(tokens),
|
tokens: summarizeDeviceTokens(tokens),
|
||||||
|
|||||||
@@ -97,7 +97,7 @@ describe("device pairing tokens", () => {
|
|||||||
expect(Buffer.from(token, "base64url")).toHaveLength(32);
|
expect(Buffer.from(token, "base64url")).toHaveLength(32);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("preserves existing token scopes when rotating without scopes", async () => {
|
test("allows down-scoping from admin and preserves approved scope baseline", async () => {
|
||||||
const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-"));
|
const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-"));
|
||||||
await setupPairedOperatorDevice(baseDir, ["operator.admin"]);
|
await setupPairedOperatorDevice(baseDir, ["operator.admin"]);
|
||||||
|
|
||||||
@@ -109,7 +109,8 @@ describe("device pairing tokens", () => {
|
|||||||
});
|
});
|
||||||
let paired = await getPairedDevice("device-1", baseDir);
|
let paired = await getPairedDevice("device-1", baseDir);
|
||||||
expect(paired?.tokens?.operator?.scopes).toEqual(["operator.read"]);
|
expect(paired?.tokens?.operator?.scopes).toEqual(["operator.read"]);
|
||||||
expect(paired?.scopes).toEqual(["operator.read"]);
|
expect(paired?.scopes).toEqual(["operator.admin"]);
|
||||||
|
expect(paired?.approvedScopes).toEqual(["operator.admin"]);
|
||||||
|
|
||||||
await rotateDeviceToken({
|
await rotateDeviceToken({
|
||||||
deviceId: "device-1",
|
deviceId: "device-1",
|
||||||
@@ -120,6 +121,26 @@ describe("device pairing tokens", () => {
|
|||||||
expect(paired?.tokens?.operator?.scopes).toEqual(["operator.read"]);
|
expect(paired?.tokens?.operator?.scopes).toEqual(["operator.read"]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("rejects scope escalation when rotating 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 rotated = await rotateDeviceToken({
|
||||||
|
deviceId: "device-1",
|
||||||
|
role: "operator",
|
||||||
|
scopes: ["operator.admin"],
|
||||||
|
baseDir,
|
||||||
|
});
|
||||||
|
expect(rotated).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 () => {
|
test("verifies token and rejects mismatches", async () => {
|
||||||
const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-"));
|
const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-"));
|
||||||
await setupPairedOperatorDevice(baseDir, ["operator.read"]);
|
await setupPairedOperatorDevice(baseDir, ["operator.read"]);
|
||||||
|
|||||||
@@ -56,6 +56,7 @@ export type PairedDevice = {
|
|||||||
role?: string;
|
role?: string;
|
||||||
roles?: string[];
|
roles?: string[];
|
||||||
scopes?: string[];
|
scopes?: string[];
|
||||||
|
approvedScopes?: string[];
|
||||||
remoteIp?: string;
|
remoteIp?: string;
|
||||||
tokens?: Record<string, DeviceAuthToken>;
|
tokens?: Record<string, DeviceAuthToken>;
|
||||||
createdAtMs: number;
|
createdAtMs: number;
|
||||||
@@ -176,6 +177,44 @@ function mergePendingDevicePairingRequest(
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function scopesAllow(requested: string[], allowed: string[]): boolean {
|
||||||
|
if (requested.length === 0) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (allowed.length === 0) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const allowedSet = new Set(allowed);
|
||||||
|
return requested.every((scope) => allowedSet.has(scope));
|
||||||
|
}
|
||||||
|
|
||||||
|
const DEVICE_SCOPE_IMPLICATIONS: Readonly<Record<string, readonly string[]>> = {
|
||||||
|
"operator.admin": ["operator.read", "operator.write", "operator.approvals", "operator.pairing"],
|
||||||
|
"operator.write": ["operator.read"],
|
||||||
|
};
|
||||||
|
|
||||||
|
function expandScopeImplications(scopes: string[]): string[] {
|
||||||
|
const expanded = new Set(scopes);
|
||||||
|
const queue = [...scopes];
|
||||||
|
while (queue.length > 0) {
|
||||||
|
const scope = queue.pop();
|
||||||
|
if (!scope) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
for (const impliedScope of DEVICE_SCOPE_IMPLICATIONS[scope] ?? []) {
|
||||||
|
if (!expanded.has(impliedScope)) {
|
||||||
|
expanded.add(impliedScope);
|
||||||
|
queue.push(impliedScope);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return [...expanded];
|
||||||
|
}
|
||||||
|
|
||||||
|
function scopesAllowWithImplications(requested: string[], allowed: string[]): boolean {
|
||||||
|
return scopesAllow(expandScopeImplications(requested), expandScopeImplications(allowed));
|
||||||
|
}
|
||||||
|
|
||||||
function newToken() {
|
function newToken() {
|
||||||
return generatePairingToken();
|
return generatePairingToken();
|
||||||
}
|
}
|
||||||
@@ -286,7 +325,10 @@ export async function approveDevicePairing(
|
|||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const existing = state.pairedByDeviceId[pending.deviceId];
|
const existing = state.pairedByDeviceId[pending.deviceId];
|
||||||
const roles = mergeRoles(existing?.roles, existing?.role, pending.roles, pending.role);
|
const roles = mergeRoles(existing?.roles, existing?.role, pending.roles, pending.role);
|
||||||
const scopes = mergeScopes(existing?.scopes, pending.scopes);
|
const approvedScopes = mergeScopes(
|
||||||
|
existing?.approvedScopes ?? existing?.scopes,
|
||||||
|
pending.scopes,
|
||||||
|
);
|
||||||
const tokens = existing?.tokens ? { ...existing.tokens } : {};
|
const tokens = existing?.tokens ? { ...existing.tokens } : {};
|
||||||
const roleForToken = normalizeRole(pending.role);
|
const roleForToken = normalizeRole(pending.role);
|
||||||
if (roleForToken) {
|
if (roleForToken) {
|
||||||
@@ -312,7 +354,8 @@ export async function approveDevicePairing(
|
|||||||
clientMode: pending.clientMode,
|
clientMode: pending.clientMode,
|
||||||
role: pending.role,
|
role: pending.role,
|
||||||
roles,
|
roles,
|
||||||
scopes,
|
scopes: approvedScopes,
|
||||||
|
approvedScopes,
|
||||||
remoteIp: pending.remoteIp,
|
remoteIp: pending.remoteIp,
|
||||||
tokens,
|
tokens,
|
||||||
createdAtMs: existing?.createdAtMs ?? now,
|
createdAtMs: existing?.createdAtMs ?? now,
|
||||||
@@ -359,7 +402,9 @@ export async function removePairedDevice(
|
|||||||
|
|
||||||
export async function updatePairedDeviceMetadata(
|
export async function updatePairedDeviceMetadata(
|
||||||
deviceId: string,
|
deviceId: string,
|
||||||
patch: Partial<Omit<PairedDevice, "deviceId" | "createdAtMs" | "approvedAtMs">>,
|
patch: Partial<
|
||||||
|
Omit<PairedDevice, "deviceId" | "createdAtMs" | "approvedAtMs" | "approvedScopes">
|
||||||
|
>,
|
||||||
baseDir?: string,
|
baseDir?: string,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
return await withLock(async () => {
|
return await withLock(async () => {
|
||||||
@@ -376,6 +421,7 @@ export async function updatePairedDeviceMetadata(
|
|||||||
deviceId: existing.deviceId,
|
deviceId: existing.deviceId,
|
||||||
createdAtMs: existing.createdAtMs,
|
createdAtMs: existing.createdAtMs,
|
||||||
approvedAtMs: existing.approvedAtMs,
|
approvedAtMs: existing.approvedAtMs,
|
||||||
|
approvedScopes: existing.approvedScopes,
|
||||||
role: patch.role ?? existing.role,
|
role: patch.role ?? existing.role,
|
||||||
roles,
|
roles,
|
||||||
scopes,
|
scopes,
|
||||||
@@ -525,6 +571,12 @@ export async function rotateDeviceToken(params: {
|
|||||||
const requestedScopes = normalizeDeviceAuthScopes(
|
const requestedScopes = normalizeDeviceAuthScopes(
|
||||||
params.scopes ?? existing?.scopes ?? device.scopes,
|
params.scopes ?? existing?.scopes ?? device.scopes,
|
||||||
);
|
);
|
||||||
|
const approvedScopes = normalizeDeviceAuthScopes(
|
||||||
|
device.approvedScopes ?? device.scopes ?? existing?.scopes,
|
||||||
|
);
|
||||||
|
if (!scopesAllowWithImplications(requestedScopes, approvedScopes)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const next = buildDeviceAuthToken({
|
const next = buildDeviceAuthToken({
|
||||||
role,
|
role,
|
||||||
@@ -535,9 +587,6 @@ export async function rotateDeviceToken(params: {
|
|||||||
});
|
});
|
||||||
tokens[role] = next;
|
tokens[role] = next;
|
||||||
device.tokens = tokens;
|
device.tokens = tokens;
|
||||||
if (params.scopes !== undefined) {
|
|
||||||
device.scopes = requestedScopes;
|
|
||||||
}
|
|
||||||
state.pairedByDeviceId[device.deviceId] = device;
|
state.pairedByDeviceId[device.deviceId] = device;
|
||||||
await persistState(state, params.baseDir);
|
await persistState(state, params.baseDir);
|
||||||
return next;
|
return next;
|
||||||
|
|||||||
Reference in New Issue
Block a user