Improve pairing diagnostics without unsafe formatting (#73933)

Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com>

src/pairing/allow-from-store-file.test.ts

@@ -0,0 +1,67 @@
+import { describe, expect, it } from "vitest";
+import {
+  resolveAllowFromAccountId,
+  resolveAllowFromFilePath,
+  safeChannelKey,
+} from "./allow-from-store-file.js";
+import type { PairingChannel } from "./pairing-store.types.js";
+
+function expectInvalidPairingKey(params: {
+  run: () => unknown;
+  message: string;
+  leaked?: string;
+}): void {
+  try {
+    params.run();
+  } catch (err) {
+    expect(err).toBeInstanceOf(Error);
+    const message = (err as Error).message;
+    expect(message).toBe(params.message);
+    if (params.leaked) {
+      expect(message).not.toContain(params.leaked);
+    }
+    return;
+  }
+  throw new Error("expected invalid pairing key error");
+}
+
+describe("allow-from store file keys", () => {
+  it("formats invalid channel diagnostics without stringifying unsafe values", () => {
+    const circular: Record<string, unknown> = { label: "private-channel-value" };
+    circular.self = circular;
+
+    expectInvalidPairingKey({
+      run: () => safeChannelKey(circular as unknown as PairingChannel),
+      message: "invalid pairing channel: expected non-empty string; got object",
+      leaked: "private-channel-value",
+    });
+  });
+
+  it("formats invalid account diagnostics without stringifying unsafe values", () => {
+    expectInvalidPairingKey({
+      run: () => resolveAllowFromFilePath("telegram", process.env, 10n as unknown as string),
+      message: "invalid pairing account id: expected non-empty string; got bigint",
+      leaked: "10",
+    });
+
+    expectInvalidPairingKey({
+      run: () => resolveAllowFromAccountId(10n as unknown as string),
+      message: "invalid pairing account id: expected non-empty string; got bigint",
+      leaked: "10",
+    });
+  });
+
+  it("reports sanitized-empty filename keys without exposing the raw key", () => {
+    expectInvalidPairingKey({
+      run: () => safeChannelKey(".." as PairingChannel),
+      message: "invalid pairing channel: sanitized filename key is empty; got string length 2",
+      leaked: "..",
+    });
+
+    expectInvalidPairingKey({
+      run: () => resolveAllowFromFilePath("telegram", process.env, "/" as string),
+      message: "invalid pairing account id: sanitized filename key is empty; got string length 1",
+      leaked: "/",
+    });
+  });
+});

src/pairing/allow-from-store-file.ts

@@ -34,29 +34,68 @@ export function resolvePairingCredentialsDir(env: NodeJS.ProcessEnv = process.en
   return resolveOAuthDir(env, stateDir);
 }
 
-/** Sanitize channel ID for use in filenames (prevent path traversal). */
-export function safeChannelKey(channel: PairingChannel): string {
-  const raw = normalizeLowercaseStringOrEmpty(String(channel));
+type PairingFilenameKeyKind = "channel" | "account id";
+
+function describePairingFilenameKeyInput(value: unknown): string {
+  if (value === null) {
+    return "null";
+  }
+  if (Array.isArray(value)) {
+    return "array";
+  }
+  if (typeof value === "string") {
+    const trimmed = value.trim();
+    return trimmed ? `string length ${trimmed.length}` : "empty string";
+  }
+  if (typeof value === "number" && !Number.isFinite(value)) {
+    return "non-finite number";
+  }
+  return typeof value;
+}
+
+function invalidPairingFilenameKeyError(
+  kind: PairingFilenameKeyKind,
+  reason: string,
+  value: unknown,
+): Error {
+  return new Error(
+    `invalid pairing ${kind}: ${reason}; got ${describePairingFilenameKeyInput(value)}`,
+  );
+}
+
+function normalizePairingFilenameKey(value: unknown, kind: PairingFilenameKeyKind): string {
+  if (typeof value !== "string") {
+    throw invalidPairingFilenameKeyError(kind, "expected non-empty string", value);
+  }
+  const raw = normalizeLowercaseStringOrEmpty(value);
   if (!raw) {
-    throw new Error("invalid pairing channel");
+    throw invalidPairingFilenameKeyError(kind, "expected non-empty string", value);
   }
   const safe = raw.replace(/[\\/:*?"<>|]/g, "_").replace(/\.\./g, "_");
   if (!safe || safe === "_") {
-    throw new Error("invalid pairing channel");
+    throw invalidPairingFilenameKeyError(kind, "sanitized filename key is empty", value);
   }
   return safe;
 }
 
+/** Sanitize channel ID for use in filenames (prevent path traversal). */
+export function safeChannelKey(channel: PairingChannel): string {
+  return normalizePairingFilenameKey(channel, "channel");
+}
+
 function safeAccountKey(accountId: string): string {
-  const raw = normalizeLowercaseStringOrEmpty(accountId);
-  if (!raw) {
-    throw new Error("invalid pairing account id");
+  return normalizePairingFilenameKey(accountId, "account id");
+}
+
+function resolveOptionalAccountFilenameKey(accountId: unknown): string | null {
+  if (accountId == null) {
+    return null;
   }
-  const safe = raw.replace(/[\\/:*?"<>|]/g, "_").replace(/\.\./g, "_");
-  if (!safe || safe === "_") {
-    throw new Error("invalid pairing account id");
+  if (typeof accountId !== "string") {
+    throw invalidPairingFilenameKeyError("account id", "expected non-empty string", accountId);
   }
-  return safe;
+  const normalizedAccountId = normalizeOptionalString(accountId) ?? "";
+  return normalizedAccountId ? safeAccountKey(normalizedAccountId) : null;
 }
 
 export function resolveAllowFromFilePath(
@@ -65,14 +104,11 @@ export function resolveAllowFromFilePath(
   accountId?: string,
 ): string {
   const base = safeChannelKey(channel);
-  const normalizedAccountId = normalizeOptionalString(accountId) ?? "";
-  if (!normalizedAccountId) {
+  const accountKey = resolveOptionalAccountFilenameKey(accountId);
+  if (!accountKey) {
     return path.join(resolvePairingCredentialsDir(env), `${base}-allowFrom.json`);
   }
-  return path.join(
-    resolvePairingCredentialsDir(env),
-    `${base}-${safeAccountKey(normalizedAccountId)}-allowFrom.json`,
-  );
+  return path.join(resolvePairingCredentialsDir(env), `${base}-${accountKey}-allowFrom.json`);
 }
 
 export function dedupePreserveOrder(entries: string[]): string[] {
@@ -94,6 +130,9 @@ export function shouldIncludeLegacyAllowFromEntries(normalizedAccountId: string)
 }
 
 export function resolveAllowFromAccountId(accountId?: string): string {
+  if (accountId != null && typeof accountId !== "string") {
+    throw invalidPairingFilenameKeyError("account id", "expected non-empty string", accountId);
+  }
   return normalizeLowercaseStringOrEmpty(accountId) || DEFAULT_ACCOUNT_ID;
 }
 

src/pairing/pairing-store.test.ts

@@ -424,6 +424,32 @@ describe("pairing store", () => {
     });
   });
 
+  it("reports unique code exhaustion without exposing reserved codes", async () => {
+    await withTempStateDir(async () => {
+      await withMockRandomInt({
+        initialValue: 0,
+        run: async () => {
+          const first = await upsertChannelPairingRequest({
+            channel: "telegram",
+            id: "123",
+            accountId: DEFAULT_ACCOUNT_ID,
+          });
+          expect(first.code).toBe("AAAAAAAA");
+
+          await expect(
+            upsertChannelPairingRequest({
+              channel: "telegram",
+              id: "456",
+              accountId: DEFAULT_ACCOUNT_ID,
+            }),
+          ).rejects.toThrow(
+            "failed to generate unique pairing code after 500 attempts; existing code count: 1",
+          );
+        },
+      });
+    });
+  });
+
   it("keeps allowFrom account-scoped across manual and pairing-code approvals", async () => {
     await withTempStateDir(async () => {
       await addChannelAllowFromStoreEntry({

src/pairing/pairing-store.ts

@@ -30,6 +30,7 @@ export type { PairingChannel } from "./pairing-store.types.js";
 
 const PAIRING_CODE_LENGTH = 8;
 const PAIRING_CODE_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789";
+const PAIRING_CODE_MAX_ATTEMPTS = 500;
 const PAIRING_PENDING_TTL_MS = 60 * 60 * 1000;
 const PAIRING_PENDING_MAX = 3;
 const PAIRING_STORE_LOCK_OPTIONS = {
@@ -201,13 +202,15 @@ function randomCode(): string {
 }
 
 function generateUniqueCode(existing: Set<string>): string {
-  for (let attempt = 0; attempt < 500; attempt += 1) {
+  for (let attempt = 0; attempt < PAIRING_CODE_MAX_ATTEMPTS; attempt += 1) {
     const code = randomCode();
     if (!existing.has(code)) {
       return code;
     }
   }
-  throw new Error("failed to generate unique pairing code");
+  throw new Error(
+    `failed to generate unique pairing code after ${PAIRING_CODE_MAX_ATTEMPTS} attempts; existing code count: ${existing.size}`,
+  );
 }
 
 function normalizePairingAccountId(accountId?: string): string {