refactor: split device-pair command helpers

2026-04-05 22:32:12 +00:00 · 2026-04-04 19:54:50 +09:00
parent fdb1be0079
commit 05da802e1c
5 changed files with 277 additions and 216 deletions
--- 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<ReturnType<typeof listDevicePairing>>["pending"][number];
+type ApproveDevicePairingResolved = Awaited<ReturnType<typeof approveDevicePairing>>;
+type ApprovedPairingResult = Extract<
+  NonNullable<ApproveDevicePairingResolved>,
+  { status: "approved" }
+>;
+type ApprovedPairingDevice = ApprovedPairingResult["device"];
+
 function createApi(params?: {
  runtime?: OpenClawPluginApi["runtime"];
  pluginConfig?: Record<string, unknown>;
@@ -144,6 +152,60 @@ function createCommandContext(params?: Partial<PluginCommandContext>): PluginCom
  };
 }

+function makePendingPairingRequest(
+  overrides: Partial<ListedPendingPairingRequest> = {},
+): 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> = {},
+): 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<Partial<ApprovedPairingResult>, "device"> & {
+    device?: Partial<ApprovedPairingDevice>;
+  } = {},
+): 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(
--- 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<string> {
  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 <requestId>\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();
        }

--- a/extensions/device-pair/pair-command-approve.ts
+++ 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<ReturnType<typeof listDevicePairing>>["pending"][number];
+type ApprovePairingResult = Awaited<ReturnType<typeof approveDevicePairing>>;
+type ApprovedPairingEntry = Exclude<ApprovePairingResult, null | { status: "forbidden" }>;
+
+function buildMultiplePendingApprovalReply(pending: PendingPairingEntry[]): { text: string } {
+  return {
+    text:
+      `${formatPendingRequests(pending)}\n\n` +
+      "Multiple pending requests found. Approve one explicitly:\n" +
+      "/pair approve <requestId>\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);
+}
--- a/extensions/device-pair/pair-command-auth.test.ts
+++ 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"],
+    });
+  });
+});
--- a/extensions/device-pair/pair-command-auth.ts
+++ 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.",
+  };
+}