From 67d2026e22e9dbae208e3bc4a4c004ef5ca39d96 Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Mon, 20 Apr 2026 13:05:51 +0530 Subject: [PATCH] feat(cli): show pairing access upgrades --- docs/cli/devices.md | 11 +- src/cli/devices-cli.test.ts | 159 ++++++++++++++++++++++++++++- src/cli/devices-cli.ts | 194 ++++++++++++++++++++++++++---------- 3 files changed, 304 insertions(+), 60 deletions(-) diff --git a/docs/cli/devices.md b/docs/cli/devices.md index f2387f047f8..83d295c7bc1 100644 --- a/docs/cli/devices.md +++ b/docs/cli/devices.md @@ -21,8 +21,9 @@ openclaw devices list openclaw devices list --json ``` -Pending request output includes the requested role and scopes so approvals can -be reviewed before you approve. +Pending request output shows the requested access next to the device's current +approved access when the device is already paired. This makes scope/role +upgrades explicit instead of looking like the pairing was lost. ### `openclaw devices remove ` @@ -59,6 +60,12 @@ key), OpenClaw supersedes the previous pending entry and issues a new `requestId`. Run `openclaw devices list` right before approval to use the current ID. +If the device is already paired and asks for broader scopes or a broader role, +OpenClaw keeps the existing approval in place and creates a new pending upgrade +request. Review the `Requested` vs `Approved` columns in `openclaw devices list` +or use `openclaw devices approve --latest` to preview the exact upgrade before +approving it. + ``` openclaw devices approve openclaw devices approve diff --git a/src/cli/devices-cli.test.ts b/src/cli/devices-cli.test.ts index 8e32aea94ca..f70cc638539 100644 --- a/src/cli/devices-cli.test.ts +++ b/src/cli/devices-cli.test.ts @@ -97,6 +97,14 @@ describe("devices cli approve", () => { ts: 1000, }, ], + paired: [ + { + deviceId: "device-9", + displayName: "Device Nine", + roles: ["operator"], + scopes: ["operator.read"], + }, + ], }); await runDevicesApprove([]); @@ -108,6 +116,8 @@ describe("devices cli approve", () => { const logOutput = runtime.log.mock.calls.map((c) => readRuntimeCallText(c)).join("\n"); expect(logOutput).toContain("req-abc"); expect(logOutput).toContain("Device Nine"); + expect(logOutput).toContain("Approved: roles: operator; scopes: operator.read"); + expect(logOutput).toContain("Requested scopes exceed the current approval"); expect(runtime.error).toHaveBeenCalledWith( expect.stringContaining("openclaw devices approve req-abc"), ); @@ -117,6 +127,36 @@ describe("devices cli approve", () => { ); }); + it("sanitizes preview ip output for implicit approval", async () => { + callGateway.mockResolvedValueOnce({ + pending: [ + { + requestId: "req-abc", + deviceId: "device-9", + displayName: "Device Nine", + role: "operator", + scopes: ["operator.admin"], + remoteIp: "10.0.0.9\rspoof", + ts: 1000, + }, + ], + paired: [ + { + deviceId: "device-9", + displayName: "Device Nine", + roles: ["operator"], + scopes: ["operator.read"], + }, + ], + }); + + await runDevicesApprove([]); + + const logOutput = runtime.log.mock.calls.map((c) => readRuntimeCallText(c)).join("\n"); + expect(logOutput).not.toContain("\r"); + expect(logOutput).toContain("IP: 10.0.0.9spoof"); + }); + it.each([ { name: "id is omitted", @@ -208,6 +248,7 @@ describe("devices cli approve", () => { it("returns JSON for implicit approval preview in JSON mode", async () => { callGateway.mockResolvedValueOnce({ pending: [{ requestId: "req-json", deviceId: "device-json", ts: 1000 }], + paired: [], }); await runDevicesApprove(["--latest", "--json", "--url", "ws://gateway.example:18789"]); @@ -216,6 +257,11 @@ describe("devices cli approve", () => { expect(runtime.error).not.toHaveBeenCalled(); expect(runtime.writeJson).toHaveBeenCalledWith({ selected: { requestId: "req-json", deviceId: "device-json", ts: 1000 }, + approvalState: { + kind: "new-pairing", + requested: { roles: [], scopes: [] }, + approved: null, + }, approveCommand: "openclaw devices approve req-json --url ws://gateway.example:18789 --json", requiresAuthFlags: { token: false, @@ -404,7 +450,7 @@ describe("devices cli local fallback", () => { }); describe("devices cli list", () => { - it("renders pending scopes when present", async () => { + it("renders requested versus approved access for pending upgrades", async () => { callGateway.mockResolvedValueOnce({ pending: [ { @@ -416,14 +462,119 @@ describe("devices cli list", () => { ts: 1, }, ], - paired: [], + paired: [ + { + deviceId: "device-1", + displayName: "Device One", + roles: ["operator"], + scopes: ["operator.read"], + }, + ], }); await runDevicesCommand(["list"]); const output = runtime.log.mock.calls.map((entry) => readRuntimeCallText(entry)).join("\n"); - expect(output).toContain("Scopes"); - expect(output).toContain("operator.admin, operator.read"); + expect(output).toContain("Requested"); + expect(output).toContain("Approved"); + expect(output).toContain("operator.write"); + expect(output).toContain("operator.read"); + expect(output).toContain("scope upgrade"); + }); + + it("normalizes pending device ids before matching paired approvals", async () => { + callGateway.mockResolvedValueOnce({ + pending: [ + { + requestId: "req-1", + deviceId: " device-1 ", + displayName: "Device One", + role: "operator", + scopes: ["operator.admin"], + ts: 1, + }, + ], + paired: [ + { + deviceId: "device-1", + displayName: "Device One", + roles: ["operator"], + scopes: ["operator.read"], + }, + ], + }); + + await runDevicesCommand(["list"]); + + const output = runtime.log.mock.calls.map((entry) => readRuntimeCallText(entry)).join("\n"); + expect(output).toContain("scope upgrade"); + expect(output).toContain("operator.read"); + }); + + it("does not show upgrade context for key-mismatched pending requests", async () => { + callGateway.mockResolvedValueOnce({ + pending: [ + { + requestId: "req-1", + deviceId: "device-1", + publicKey: "new-key", + displayName: "Device One", + role: "operator", + scopes: ["operator.admin"], + ts: 1, + }, + ], + paired: [ + { + deviceId: "device-1", + publicKey: "old-key", + displayName: "Device One", + roles: ["operator"], + scopes: ["operator.read"], + }, + ], + }); + + await runDevicesCommand(["list"]); + + const output = runtime.log.mock.calls.map((entry) => readRuntimeCallText(entry)).join("\n"); + expect(output).toContain("new pairing"); + expect(output).not.toContain("scope upgrade"); + expect(output).not.toContain("roles: operator; scopes: operator.read"); + }); + + it("sanitizes device-controlled terminal output", async () => { + callGateway.mockResolvedValueOnce({ + pending: [ + { + requestId: "req-1", + deviceId: "device-1", + displayName: "Bad\u001b[2J\nName", + role: "operator", + scopes: ["operator.admin"], + remoteIp: "10.0.0.9\rspoof", + ts: 1, + }, + ], + paired: [ + { + deviceId: "device-1", + displayName: "Pair\u001b]8;;https://evil.example\u001b\\ed", + roles: ["operator"], + scopes: ["operator.read"], + remoteIp: "10.0.0.1\u007f", + }, + ], + }); + + await runDevicesCommand(["list"]); + + const output = runtime.log.mock.calls.map((entry) => readRuntimeCallText(entry)).join("\n"); + expect(output).not.toContain("\u001b"); + expect(output).not.toContain("\r"); + expect(output).toContain("BadName"); + expect(output).toContain("spoof"); + expect(output).toContain("Paired"); }); }); diff --git a/src/cli/devices-cli.ts b/src/cli/devices-cli.ts index ced85580483..6419f9948c2 100644 --- a/src/cli/devices-cli.ts +++ b/src/cli/devices-cli.ts @@ -11,11 +11,17 @@ import { } from "../infra/device-pairing.js"; import { formatTimeAgo } from "../infra/format-time/format-relative.ts"; import { defaultRuntime } from "../runtime.js"; +import { + resolvePendingDeviceApprovalState, + type DevicePairingAccessSummary, + type PendingDeviceApprovalKind, +} from "../shared/device-pairing-access.js"; import { normalizeLowercaseStringOrEmpty, normalizeOptionalString, normalizeStringifiedOptionalString, } from "../shared/string-coerce.js"; +import { sanitizeForLog } from "../terminal/ansi.js"; import { getTerminalTableWidth, renderTable } from "../terminal/table.js"; import { theme } from "../terminal/theme.js"; import { withProgress } from "./progress.js"; @@ -43,6 +49,7 @@ type DeviceTokenSummary = { type PendingDevice = { requestId: string; deviceId: string; + publicKey?: string; displayName?: string; role?: string; roles?: string[]; @@ -54,6 +61,7 @@ type PendingDevice = { type PairedDevice = { deviceId: string; + publicKey?: string; displayName?: string; roles?: string[]; scopes?: string[]; @@ -212,37 +220,77 @@ function formatTokenSummary(tokens: DeviceTokenSummary[] | undefined) { return "none"; } const parts = tokens - .map((t) => `${t.role}${t.revokedAtMs ? " (revoked)" : ""}`) + .map((t) => `${sanitizeForLog(t.role)}${t.revokedAtMs ? " (revoked)" : ""}`) .toSorted((a, b) => a.localeCompare(b)); return parts.join(", "); } -function formatPendingRoles(request: PendingDevice): string { - const role = normalizeOptionalString(request.role) ?? ""; - if (role) { - return role; - } - const roles = Array.isArray(request.roles) - ? request.roles.map((item) => item.trim()).filter((item) => item.length > 0) - : []; - if (roles.length === 0) { - return ""; - } - return roles.join(", "); -} - -function formatPendingScopes(request: PendingDevice): string { - const scopes = Array.isArray(request.scopes) - ? request.scopes.map((item) => item.trim()).filter((item) => item.length > 0) - : []; - if (scopes.length === 0) { - return ""; - } - return scopes.join(", "); -} - function formatPendingDeviceIdentity(request: PendingDevice): string { - return normalizeOptionalString(request.displayName) ?? request.deviceId; + const displayName = normalizeOptionalString(request.displayName); + if (displayName) { + return sanitizeForLog(displayName); + } + return sanitizeForLog(normalizeOptionalString(request.deviceId) ?? ""); +} + +function formatAccessSummary(access: DevicePairingAccessSummary | null): string { + if (!access) { + return "none"; + } + const roles = + access.roles.length > 0 ? access.roles.map((role) => sanitizeForLog(role)).join(", ") : "none"; + const scopes = + access.scopes.length > 0 + ? access.scopes.map((scope) => sanitizeForLog(scope)).join(", ") + : "none"; + return `roles: ${roles}; scopes: ${scopes}`; +} + +function formatPendingApprovalKind(kind: PendingDeviceApprovalKind): string { + switch (kind) { + case "new-pairing": + return "new pairing"; + case "role-upgrade": + return "role upgrade"; + case "scope-upgrade": + return "scope upgrade"; + case "re-approval": + return "re-approval"; + } + const exhaustiveKind: never = kind; + void exhaustiveKind; + throw new Error("unsupported pending approval kind"); +} + +function indexPairedDevices(paired: PairedDevice[] | undefined): Map { + const out = new Map(); + for (const device of paired ?? []) { + const deviceId = normalizeOptionalString(device.deviceId); + if (deviceId) { + out.set(deviceId, device); + } + } + return out; +} + +function lookupPairedDevice( + pairedByDeviceId: ReadonlyMap, + request: Pick, +): PairedDevice | undefined { + const normalizedDeviceId = normalizeOptionalString(request.deviceId); + if (!normalizedDeviceId) { + return undefined; + } + const paired = pairedByDeviceId.get(normalizedDeviceId); + if (!paired) { + return undefined; + } + const requestPublicKey = normalizeOptionalString(request.publicKey); + const pairedPublicKey = normalizeOptionalString(paired.publicKey); + if (requestPublicKey && pairedPublicKey && requestPublicKey !== pairedPublicKey) { + return undefined; + } + return paired; } function quoteCliArg(value: string): string { @@ -304,6 +352,7 @@ export function registerDevicesCli(program: Command) { .description("List pending and paired devices") .action(async (opts: DevicesRpcOpts) => { const list = await listPairingWithFallback(opts); + const pairedByDeviceId = indexPairedDevices(list.paired); if (opts.json) { defaultRuntime.writeJson(list); return; @@ -319,21 +368,29 @@ export function registerDevicesCli(program: Command) { columns: [ { key: "Request", header: "Request", minWidth: 10 }, { key: "Device", header: "Device", minWidth: 16, flex: true }, - { key: "Role", header: "Role", minWidth: 8 }, - { key: "Scopes", header: "Scopes", minWidth: 14, flex: true }, - { key: "IP", header: "IP", minWidth: 12 }, + { key: "Requested", header: "Requested", minWidth: 20, flex: true }, + { key: "Approved", header: "Approved", minWidth: 20, flex: true }, { key: "Age", header: "Age", minWidth: 8 }, - { key: "Flags", header: "Flags", minWidth: 8 }, + { key: "Status", header: "Status", minWidth: 12 }, ], - rows: list.pending.map((req) => ({ - Request: req.requestId, - Device: req.displayName || req.deviceId, - Role: formatPendingRoles(req), - Scopes: formatPendingScopes(req), - IP: req.remoteIp ?? "", - Age: typeof req.ts === "number" ? formatTimeAgo(Date.now() - req.ts) : "", - Flags: req.isRepair ? "repair" : "", - })), + rows: list.pending.map((req) => { + const approval = resolvePendingDeviceApprovalState( + req, + lookupPairedDevice(pairedByDeviceId, req), + ); + const statusParts = [formatPendingApprovalKind(approval.kind)]; + if (req.isRepair) { + statusParts.push("repair"); + } + return { + Request: req.requestId, + Device: `${formatPendingDeviceIdentity(req)}${req.remoteIp ? ` ยท ${sanitizeForLog(req.remoteIp)}` : ""}`, + Requested: formatAccessSummary(approval.requested), + Approved: formatAccessSummary(approval.approved), + Age: typeof req.ts === "number" ? formatTimeAgo(Date.now() - req.ts) : "", + Status: statusParts.join(", "), + }; + }), }).trimEnd(), ); } @@ -353,11 +410,15 @@ export function registerDevicesCli(program: Command) { { key: "IP", header: "IP", minWidth: 12 }, ], rows: list.paired.map((device) => ({ - Device: device.displayName || device.deviceId, - Roles: device.roles?.length ? device.roles.join(", ") : "", - Scopes: device.scopes?.length ? device.scopes.join(", ") : "", + Device: sanitizeForLog(device.displayName || device.deviceId), + Roles: device.roles?.length + ? device.roles.map((role) => sanitizeForLog(role)).join(", ") + : "", + Scopes: device.scopes?.length + ? device.scopes.map((scope) => sanitizeForLog(scope)).join(", ") + : "", Tokens: formatTokenSummary(device.tokens), - IP: device.remoteIp ?? "", + IP: device.remoteIp ? sanitizeForLog(device.remoteIp) : "", })), }).trimEnd(), ); @@ -449,13 +510,13 @@ export function registerDevicesCli(program: Command) { .argument("[requestId]", "Pending request id") .option("--latest", "Show the most recent pending request to approve explicitly", false) .action(async (requestId: string | undefined, opts: DevicesRpcOpts) => { + let pairingList: DevicePairingList | null = null; let resolvedRequestId = requestId?.trim(); const usingImplicitSelection = !resolvedRequestId || Boolean(opts.latest); let selectedRequest: PendingDevice | null = null; if (usingImplicitSelection) { - selectedRequest = selectLatestPendingRequest( - (await listPairingWithFallback(opts)).pending, - ); + pairingList = await listPairingWithFallback(opts); + selectedRequest = selectLatestPendingRequest(pairingList.pending); resolvedRequestId = selectedRequest?.requestId?.trim(); } if (!resolvedRequestId) { @@ -467,11 +528,20 @@ export function registerDevicesCli(program: Command) { // Keep implicit selection preview-only. A second command with the exact // requestId binds the approval to the request the operator inspected. const req = selectedRequest!; + const approval = resolvePendingDeviceApprovalState( + req, + lookupPairedDevice(indexPairedDevices(pairingList?.paired), req), + ); const approveCommand = buildExplicitApproveCommand(opts, req.requestId); const authReminder = formatAuthFlagReminder(opts); if (opts.json) { defaultRuntime.writeJson({ selected: req, + approvalState: { + kind: approval.kind, + requested: approval.requested, + approved: approval.approved, + }, approveCommand, requiresAuthFlags: { token: Boolean(normalizeOptionalString(opts.token)), @@ -485,16 +555,32 @@ export function registerDevicesCli(program: Command) { `${theme.warn("Selected pending device request")} ${theme.command(req.requestId)}`, ); defaultRuntime.log(` Device: ${formatPendingDeviceIdentity(req)}`); - const role = formatPendingRoles(req); - if (role) { - defaultRuntime.log(` Role: ${role}`); - } - const scopes = formatPendingScopes(req); - if (scopes) { - defaultRuntime.log(` Scopes: ${scopes}`); + defaultRuntime.log(` Requested: ${formatAccessSummary(approval.requested)}`); + if (approval.approved) { + defaultRuntime.log(` Approved: ${formatAccessSummary(approval.approved)}`); } if (req.remoteIp) { - defaultRuntime.log(` IP: ${req.remoteIp}`); + defaultRuntime.log(` IP: ${sanitizeForLog(req.remoteIp)}`); + } + switch (approval.kind) { + case "scope-upgrade": + defaultRuntime.log( + " Note: Already paired. Requested scopes exceed the current approval, so reconnect stays blocked until you approve this upgrade.", + ); + break; + case "role-upgrade": + defaultRuntime.log( + " Note: Already paired. Requested role exceeds the current approval, so reconnect stays blocked until you approve this upgrade.", + ); + break; + case "re-approval": + defaultRuntime.log( + " Note: Already paired. Approval-bound device details changed, so OpenClaw created a fresh request instead of silently reusing the old approval.", + ); + break; + case "new-pairing": + defaultRuntime.log(" Note: First-time device pairing request."); + break; } defaultRuntime.error(`Approve this exact request with: ${approveCommand}`); if (authReminder) {