From 05da802e1cdcffe59acd3700d9f86d2c05e3372f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Apr 2026 19:54:50 +0900 Subject: [PATCH] refactor: split device-pair command helpers --- extensions/device-pair/index.test.ts | 214 +++++++----------- extensions/device-pair/index.ts | 111 ++------- .../device-pair/pair-command-approve.ts | 69 ++++++ .../device-pair/pair-command-auth.test.ts | 53 +++++ extensions/device-pair/pair-command-auth.ts | 46 ++++ 5 files changed, 277 insertions(+), 216 deletions(-) create mode 100644 extensions/device-pair/pair-command-approve.ts create mode 100644 extensions/device-pair/pair-command-auth.test.ts create mode 100644 extensions/device-pair/pair-command-auth.ts diff --git a/extensions/device-pair/index.test.ts b/extensions/device-pair/index.test.ts index 045c35d0a5f..2aab6f13182 100644 --- a/extensions/device-pair/index.test.ts +++ b/extensions/device-pair/index.test.ts @@ -53,6 +53,14 @@ vi.mock("./notify.js", () => ({ import { approveDevicePairing, listDevicePairing } from "./api.js"; import registerDevicePair from "./index.js"; +type ListedPendingPairingRequest = Awaited>["pending"][number]; +type ApproveDevicePairingResolved = Awaited>; +type ApprovedPairingResult = Extract< + NonNullable, + { status: "approved" } +>; +type ApprovedPairingDevice = ApprovedPairingResult["device"]; + function createApi(params?: { runtime?: OpenClawPluginApi["runtime"]; pluginConfig?: Record; @@ -144,6 +152,60 @@ function createCommandContext(params?: Partial): PluginCom }; } +function makePendingPairingRequest( + overrides: Partial = {}, +): ListedPendingPairingRequest { + return { + requestId: "req-1", + deviceId: "victim-phone", + publicKey: "victim-public-key", + displayName: "Victim Phone", + platform: "ios", + ts: Date.now(), + ...overrides, + }; +} + +function makeApprovedPairingDevice( + overrides: Partial = {}, +): ApprovedPairingDevice { + return { + deviceId: "victim-phone", + publicKey: "victim-public-key", + displayName: "Victim Phone", + platform: "ios", + role: "operator", + roles: ["operator"], + scopes: ["operator.pairing"], + approvedScopes: ["operator.pairing"], + tokens: { + operator: { + token: "token-1", + role: "operator", + scopes: ["operator.pairing"], + createdAtMs: Date.now(), + }, + }, + createdAtMs: Date.now(), + approvedAtMs: Date.now(), + ...overrides, + }; +} + +function makeApprovedPairingResult( + overrides: Omit, "device"> & { + device?: Partial; + } = {}, +): ApprovedPairingResult { + const { device, ...resultOverrides } = overrides; + return { + status: "approved", + requestId: "req-1", + device: makeApprovedPairingDevice(device), + ...resultOverrides, + }; +} + describe("device-pair /pair qr", () => { beforeEach(async () => { vi.clearAllMocks(); @@ -567,16 +629,7 @@ describe("device-pair /pair approve", () => { it("rejects internal gateway callers without operator.pairing", async () => { vi.mocked(listDevicePairing).mockResolvedValueOnce({ - pending: [ - { - requestId: "req-1", - deviceId: "victim-phone", - publicKey: "victim-public-key", - displayName: "Victim Phone", - platform: "ios", - ts: Date.now(), - }, - ], + pending: [makePendingPairingRequest()], paired: [], }); @@ -598,42 +651,10 @@ describe("device-pair /pair approve", () => { it("allows internal gateway callers with operator.pairing", async () => { vi.mocked(listDevicePairing).mockResolvedValueOnce({ - pending: [ - { - requestId: "req-1", - deviceId: "victim-phone", - publicKey: "victim-public-key", - displayName: "Victim Phone", - platform: "ios", - ts: Date.now(), - }, - ], + pending: [makePendingPairingRequest()], paired: [], }); - vi.mocked(approveDevicePairing).mockResolvedValueOnce({ - status: "approved", - requestId: "req-1", - device: { - deviceId: "victim-phone", - publicKey: "victim-public-key", - displayName: "Victim Phone", - platform: "ios", - role: "operator", - roles: ["operator"], - scopes: ["operator.pairing"], - approvedScopes: ["operator.pairing"], - tokens: { - operator: { - token: "token-1", - role: "operator", - scopes: ["operator.pairing"], - createdAtMs: Date.now(), - }, - }, - createdAtMs: Date.now(), - approvedAtMs: Date.now(), - }, - }); + vi.mocked(approveDevicePairing).mockResolvedValueOnce(makeApprovedPairingResult()); const command = registerPairCommand(); const result = await command.handler( @@ -653,42 +674,10 @@ describe("device-pair /pair approve", () => { it("does not force an empty caller scope context for external approvals", async () => { vi.mocked(listDevicePairing).mockResolvedValueOnce({ - pending: [ - { - requestId: "req-1", - deviceId: "victim-phone", - publicKey: "victim-public-key", - displayName: "Victim Phone", - platform: "ios", - ts: Date.now(), - }, - ], + pending: [makePendingPairingRequest()], paired: [], }); - vi.mocked(approveDevicePairing).mockResolvedValueOnce({ - status: "approved", - requestId: "req-1", - device: { - deviceId: "victim-phone", - publicKey: "victim-public-key", - displayName: "Victim Phone", - platform: "ios", - role: "operator", - roles: ["operator"], - scopes: ["operator.pairing"], - approvedScopes: ["operator.pairing"], - tokens: { - operator: { - token: "token-1", - role: "operator", - scopes: ["operator.pairing"], - createdAtMs: Date.now(), - }, - }, - createdAtMs: Date.now(), - approvedAtMs: Date.now(), - }, - }); + vi.mocked(approveDevicePairing).mockResolvedValueOnce(makeApprovedPairingResult()); const command = registerPairCommand(); const result = await command.handler( @@ -706,16 +695,7 @@ describe("device-pair /pair approve", () => { it("fails closed for approvals when internal gateway scopes are absent", async () => { vi.mocked(listDevicePairing).mockResolvedValueOnce({ - pending: [ - { - requestId: "req-1", - deviceId: "victim-phone", - publicKey: "victim-public-key", - displayName: "Victim Phone", - platform: "ios", - ts: Date.now(), - }, - ], + pending: [makePendingPairingRequest()], paired: [], }); @@ -737,16 +717,7 @@ describe("device-pair /pair approve", () => { it("rejects approvals that request scopes above the caller session", async () => { vi.mocked(listDevicePairing).mockResolvedValueOnce({ - pending: [ - { - requestId: "req-1", - deviceId: "victim-phone", - publicKey: "victim-public-key", - displayName: "Victim Phone", - platform: "ios", - ts: Date.now(), - }, - ], + pending: [makePendingPairingRequest()], paired: [], }); vi.mocked(approveDevicePairing).mockResolvedValueOnce({ @@ -774,42 +745,25 @@ describe("device-pair /pair approve", () => { it("preserves approvals for non-gateway command surfaces", async () => { vi.mocked(listDevicePairing).mockResolvedValueOnce({ - pending: [ - { - requestId: "req-1", - deviceId: "victim-phone", - publicKey: "victim-public-key", - displayName: "Victim Phone", - platform: "ios", - ts: Date.now(), - }, - ], + pending: [makePendingPairingRequest()], paired: [], }); - vi.mocked(approveDevicePairing).mockResolvedValueOnce({ - status: "approved", - requestId: "req-1", - device: { - deviceId: "victim-phone", - publicKey: "victim-public-key", - displayName: "Victim Phone", - platform: "ios", - role: "operator", - roles: ["operator"], - scopes: ["operator.admin"], - approvedScopes: ["operator.admin"], - tokens: { - operator: { - token: "token-1", - role: "operator", - scopes: ["operator.admin"], - createdAtMs: Date.now(), + vi.mocked(approveDevicePairing).mockResolvedValueOnce( + makeApprovedPairingResult({ + device: { + scopes: ["operator.admin"], + approvedScopes: ["operator.admin"], + tokens: { + operator: { + token: "token-1", + role: "operator", + scopes: ["operator.admin"], + createdAtMs: Date.now(), + }, }, }, - createdAtMs: Date.now(), - approvedAtMs: Date.now(), - }, - }); + }), + ); const command = registerPairCommand(); const result = await command.handler( diff --git a/extensions/device-pair/index.ts b/extensions/device-pair/index.ts index 7d9b30dabb4..eccfa8dc8ee 100644 --- a/extensions/device-pair/index.ts +++ b/extensions/device-pair/index.ts @@ -2,7 +2,6 @@ import { mkdtemp, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { - approveDevicePairing, clearDeviceBootstrapTokens, definePluginEntry, issueDeviceBootstrapToken, @@ -23,6 +22,14 @@ import { handleNotifyCommand, registerPairingNotifierService, } from "./notify.js"; +import { + approvePendingPairingRequest, + selectPendingApprovalRequest, +} from "./pair-command-approve.js"; +import { + buildMissingPairingScopeReply, + resolvePairingCommandAuthState, +} from "./pair-command-auth.js"; async function renderQrDataUrl(data: string): Promise { const pngBase64 = await renderQrPngBase64(data); @@ -481,43 +488,6 @@ function resolveQrReplyTarget(ctx: QrCommandContext): string { return ctx.senderId?.trim() || ctx.from?.trim() || ctx.to?.trim() || ""; } -function buildMissingPairingScopeReply(): { text: string } { - return { - text: "⚠️ This command requires operator.pairing for internal gateway callers.", - }; -} - -function isInternalGatewayPairingCaller(params: { - channel: string; - gatewayClientScopes?: readonly string[] | null; -}): boolean { - return params.channel === "webchat" || Array.isArray(params.gatewayClientScopes); -} - -function isMissingPairingScope(params: { - channel: string; - gatewayClientScopes?: readonly string[] | null; -}): boolean { - if (!isInternalGatewayPairingCaller(params)) { - return false; - } - const gatewayClientScopes = params.gatewayClientScopes; - return !Array.isArray(gatewayClientScopes) - ? true - : !gatewayClientScopes.includes("operator.pairing") && - !gatewayClientScopes.includes("operator.admin"); -} - -function resolveApprovalCallerScopes(params: { - channel: string; - gatewayClientScopes?: readonly string[] | null; -}): readonly string[] | undefined { - if (!isInternalGatewayPairingCaller(params)) { - return undefined; - } - return Array.isArray(params.gatewayClientScopes) ? params.gatewayClientScopes : []; -} - const PAIR_SETUP_NON_ISSUING_ACTIONS = new Set([ "approve", "cleanup", @@ -593,6 +563,10 @@ export default definePluginEntry({ const gatewayClientScopes = Array.isArray(ctx.gatewayClientScopes) ? ctx.gatewayClientScopes : undefined; + const authState = resolvePairingCommandAuthState({ + channel: ctx.channel, + gatewayClientScopes, + }); api.logger.info?.( `device-pair: /pair invoked channel=${ctx.channel} sender=${ctx.senderId ?? "unknown"} action=${ action || "new" @@ -614,61 +588,29 @@ export default definePluginEntry({ } if (action === "approve") { - if (isMissingPairingScope({ channel: ctx.channel, gatewayClientScopes })) { + if (authState.isMissingInternalPairingPrivilege) { return buildMissingPairingScopeReply(); } - const requested = tokens[1]?.trim(); const list = await listDevicePairing(); - if (list.pending.length === 0) { - return { text: "No pending device pairing requests." }; - } - - let pending: (typeof list.pending)[number] | undefined; - if (requested) { - if (requested.toLowerCase() === "latest") { - pending = [...list.pending].toSorted((a, b) => (b.ts ?? 0) - (a.ts ?? 0))[0]; - } else { - pending = list.pending.find((entry) => entry.requestId === requested); - } - } else if (list.pending.length === 1) { - pending = list.pending[0]; - } else { - return { - text: - `${formatPendingRequests(list.pending)}\n\n` + - "Multiple pending requests found. Approve one explicitly:\n" + - "/pair approve \n" + - "Or approve the most recent:\n" + - "/pair approve latest", - }; + const selected = selectPendingApprovalRequest({ + pending: list.pending, + requested: tokens[1]?.trim(), + }); + if (selected.reply) { + return selected.reply; } + const pending = selected.pending; if (!pending) { return { text: "Pairing request not found." }; } - const callerScopes = resolveApprovalCallerScopes({ - channel: ctx.channel, - gatewayClientScopes, + return await approvePendingPairingRequest({ + requestId: pending.requestId, + callerScopes: authState.approvalCallerScopes, }); - const approved = - callerScopes === undefined - ? await approveDevicePairing(pending.requestId) - : await approveDevicePairing(pending.requestId, { callerScopes }); - if (!approved) { - return { text: "Pairing request not found." }; - } - if (approved.status === "forbidden") { - return { - text: `⚠️ This command requires ${approved.missingScope} to approve this pairing request.`, - }; - } - const label = approved.device.displayName?.trim() || approved.device.deviceId; - const platform = approved.device.platform?.trim(); - const platformLabel = platform ? ` (${platform})` : ""; - return { text: `✅ Paired ${label}${platformLabel}.` }; } if (action === "cleanup" || action === "clear" || action === "revoke") { - if (isMissingPairingScope({ channel: ctx.channel, gatewayClientScopes })) { + if (authState.isMissingInternalPairingPrivilege) { return buildMissingPairingScopeReply(); } const cleared = await clearDeviceBootstrapTokens(); @@ -684,10 +626,7 @@ export default definePluginEntry({ if (authLabelResult.error) { return { text: `Error: ${authLabelResult.error}` }; } - if ( - issuesPairSetupCode(action) && - isMissingPairingScope({ channel: ctx.channel, gatewayClientScopes }) - ) { + if (issuesPairSetupCode(action) && authState.isMissingInternalPairingPrivilege) { return buildMissingPairingScopeReply(); } diff --git a/extensions/device-pair/pair-command-approve.ts b/extensions/device-pair/pair-command-approve.ts new file mode 100644 index 00000000000..e703a75a032 --- /dev/null +++ b/extensions/device-pair/pair-command-approve.ts @@ -0,0 +1,69 @@ +import { approveDevicePairing, listDevicePairing } from "./api.js"; +import { formatPendingRequests } from "./notify.js"; + +type PendingPairingEntry = Awaited>["pending"][number]; +type ApprovePairingResult = Awaited>; +type ApprovedPairingEntry = Exclude; + +function buildMultiplePendingApprovalReply(pending: PendingPairingEntry[]): { text: string } { + return { + text: + `${formatPendingRequests(pending)}\n\n` + + "Multiple pending requests found. Approve one explicitly:\n" + + "/pair approve \n" + + "Or approve the most recent:\n" + + "/pair approve latest", + }; +} + +export function selectPendingApprovalRequest(params: { + pending: PendingPairingEntry[]; + requested?: string; +}): { pending?: PendingPairingEntry; reply?: { text: string } } { + if (params.pending.length === 0) { + return { reply: { text: "No pending device pairing requests." } }; + } + + if (!params.requested) { + return params.pending.length === 1 + ? { pending: params.pending[0] } + : { reply: buildMultiplePendingApprovalReply(params.pending) }; + } + + if (params.requested.toLowerCase() === "latest") { + return { + pending: [...params.pending].toSorted((a, b) => (b.ts ?? 0) - (a.ts ?? 0))[0], + }; + } + + return { + pending: params.pending.find((entry) => entry.requestId === params.requested), + reply: undefined, + }; +} + +function formatApprovedPairingReply(approved: ApprovedPairingEntry): { text: string } { + const label = approved.device.displayName?.trim() || approved.device.deviceId; + const platform = approved.device.platform?.trim(); + const platformLabel = platform ? ` (${platform})` : ""; + return { text: `✅ Paired ${label}${platformLabel}.` }; +} + +export async function approvePendingPairingRequest(params: { + requestId: string; + callerScopes?: readonly string[]; +}): Promise<{ text: string }> { + const approved = + params.callerScopes === undefined + ? await approveDevicePairing(params.requestId) + : await approveDevicePairing(params.requestId, { callerScopes: params.callerScopes }); + if (!approved) { + return { text: "Pairing request not found." }; + } + if (approved.status === "forbidden") { + return { + text: `⚠️ This command requires ${approved.missingScope} to approve this pairing request.`, + }; + } + return formatApprovedPairingReply(approved); +} diff --git a/extensions/device-pair/pair-command-auth.test.ts b/extensions/device-pair/pair-command-auth.test.ts new file mode 100644 index 00000000000..36f209c51da --- /dev/null +++ b/extensions/device-pair/pair-command-auth.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from "vitest"; +import { resolvePairingCommandAuthState } from "./pair-command-auth.js"; + +describe("device-pair pairing command auth", () => { + it("treats non-gateway channels as external approvals", () => { + expect( + resolvePairingCommandAuthState({ + channel: "telegram", + gatewayClientScopes: undefined, + }), + ).toEqual({ + isInternalGatewayCaller: false, + isMissingInternalPairingPrivilege: false, + approvalCallerScopes: undefined, + }); + }); + + it("fails closed for webchat when scopes are absent", () => { + expect( + resolvePairingCommandAuthState({ + channel: "webchat", + gatewayClientScopes: undefined, + }), + ).toEqual({ + isInternalGatewayCaller: true, + isMissingInternalPairingPrivilege: true, + approvalCallerScopes: [], + }); + }); + + it("accepts pairing and admin scopes for internal callers", () => { + expect( + resolvePairingCommandAuthState({ + channel: "webchat", + gatewayClientScopes: ["operator.write", "operator.pairing"], + }), + ).toEqual({ + isInternalGatewayCaller: true, + isMissingInternalPairingPrivilege: false, + approvalCallerScopes: ["operator.write", "operator.pairing"], + }); + expect( + resolvePairingCommandAuthState({ + channel: "webchat", + gatewayClientScopes: ["operator.admin"], + }), + ).toEqual({ + isInternalGatewayCaller: true, + isMissingInternalPairingPrivilege: false, + approvalCallerScopes: ["operator.admin"], + }); + }); +}); diff --git a/extensions/device-pair/pair-command-auth.ts b/extensions/device-pair/pair-command-auth.ts new file mode 100644 index 00000000000..60bc4501b4b --- /dev/null +++ b/extensions/device-pair/pair-command-auth.ts @@ -0,0 +1,46 @@ +type PairingCommandAuthParams = { + channel: string; + gatewayClientScopes?: readonly string[] | null; +}; + +export type PairingCommandAuthState = { + isInternalGatewayCaller: boolean; + isMissingInternalPairingPrivilege: boolean; + approvalCallerScopes?: readonly string[]; +}; + +function isInternalGatewayPairingCaller(params: PairingCommandAuthParams): boolean { + return params.channel === "webchat" || Array.isArray(params.gatewayClientScopes); +} + +export function resolvePairingCommandAuthState( + params: PairingCommandAuthParams, +): PairingCommandAuthState { + const isInternalGatewayCaller = isInternalGatewayPairingCaller(params); + if (!isInternalGatewayCaller) { + return { + isInternalGatewayCaller, + isMissingInternalPairingPrivilege: false, + approvalCallerScopes: undefined, + }; + } + + const approvalCallerScopes = Array.isArray(params.gatewayClientScopes) + ? params.gatewayClientScopes + : []; + const isMissingInternalPairingPrivilege = + !approvalCallerScopes.includes("operator.pairing") && + !approvalCallerScopes.includes("operator.admin"); + + return { + isInternalGatewayCaller, + isMissingInternalPairingPrivilege, + approvalCallerScopes, + }; +} + +export function buildMissingPairingScopeReply(): { text: string } { + return { + text: "⚠️ This command requires operator.pairing for internal gateway callers.", + }; +}