From c608dd4084d5cce030bcc7b4dc217ae509b3ca0b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 17 May 2026 04:23:46 +0100 Subject: [PATCH] fix: preserve legacy pairing allowlists --- src/commands/doctor/legacy/channel-pairing.ts | 7 ++- src/pairing/pairing-store.test.ts | 51 ++++++++++++++++++ src/pairing/pairing-store.ts | 52 +++++++++++++++---- 3 files changed, 99 insertions(+), 11 deletions(-) diff --git a/src/commands/doctor/legacy/channel-pairing.ts b/src/commands/doctor/legacy/channel-pairing.ts index 09d93a7d59a..6f6ebe873db 100644 --- a/src/commands/doctor/legacy/channel-pairing.ts +++ b/src/commands/doctor/legacy/channel-pairing.ts @@ -267,8 +267,13 @@ export async function importLegacyChannelPairingFilesToSqlite( accountId = configuredAccountIds.get(accountId) ?? accountId; } const state = readChannelPairingState(allowFromTarget.channel, env); + const resolvedAccountId = resolveAllowFromAccountId(accountId); + const existingEntries = state.allowFrom?.[resolvedAccountId] ?? []; state.allowFrom ??= {}; - state.allowFrom[resolveAllowFromAccountId(accountId)] = entries; + state.allowFrom[resolvedAccountId] = normalizeAllowFromList(allowFromTarget.channel, { + version: 1, + allowFrom: [...existingEntries, ...entries], + }); writeChannelPairingState(allowFromTarget.channel, state, env); allowFrom += entries.length; await fs.rm(filePath, { force: true }).catch(() => undefined); diff --git a/src/pairing/pairing-store.test.ts b/src/pairing/pairing-store.test.ts index a5cda6f92b1..9fff229d15c 100644 --- a/src/pairing/pairing-store.test.ts +++ b/src/pairing/pairing-store.test.ts @@ -356,6 +356,34 @@ describe("pairing store", () => { }); }); + it("merges legacy allowFrom import with newer SQLite entries", async () => { + await withTempStateDir(async (stateDir) => { + await writeAllowFromFixture({ + stateDir, + channel: "telegram", + accountId: "yy", + allowFrom: ["sqlite-new"], + }); + const allowFromPath = resolveLegacyChannelAllowFromPath("telegram", process.env, "yy"); + fsSync.mkdirSync(path.dirname(allowFromPath), { recursive: true }); + fsSync.writeFileSync( + allowFromPath, + `${JSON.stringify({ version: 1, allowFrom: ["legacy-old", "sqlite-new"] })}\n`, + "utf8", + ); + + await expect(importLegacyChannelPairingFilesToSqlite(process.env)).resolves.toMatchObject({ + allowFrom: 2, + }); + + await expect(readChannelAllowFromStore("telegram", process.env, "yy")).resolves.toEqual([ + "sqlite-new", + "legacy-old", + ]); + expect(fsSync.existsSync(allowFromPath)).toBe(false); + }); + }); + it("handles pending pairing request lifecycle and limits", async () => { await withTempStateDir(async (stateDir) => { const first = await upsertChannelPairingRequest({ @@ -636,6 +664,29 @@ describe("pairing store", () => { }); }); + it("can remove a legacy-only allowFrom fallback before doctor migration", async () => { + await withTempStateDir(async (stateDir) => { + clearOAuthFixtures(stateDir); + const scopedPath = resolveAllowFromFilePath(stateDir, "telegram", "yy"); + fsSync.mkdirSync(path.dirname(scopedPath), { recursive: true }); + fsSync.writeFileSync( + scopedPath, + `${JSON.stringify({ version: 1, allowFrom: ["legacy-scoped"] })}\n`, + "utf8", + ); + + const removed = await removeChannelAllowFromStoreEntry({ + channel: "telegram", + accountId: "yy", + entry: "legacy-scoped", + }); + + expect(removed).toEqual({ changed: true, allowFrom: [] }); + await expect(readChannelAllowFromStore("telegram", process.env, "yy")).resolves.toEqual([]); + expect(fsSync.existsSync(scopedPath)).toBe(false); + }); + }); + it("keeps pending pairing requests isolated by account", async () => { await withTempStateDir(async (stateDir) => { await expectPendingPairingRequestsIsolatedByAccount({ diff --git a/src/pairing/pairing-store.ts b/src/pairing/pairing-store.ts index 30fae268d42..90db1604674 100644 --- a/src/pairing/pairing-store.ts +++ b/src/pairing/pairing-store.ts @@ -215,7 +215,7 @@ function resolveLegacyAllowFromPath( ); } -function readLegacyAllowFromEntries(filePath: string): string[] { +function readLegacyAllowFromEntries(channel: PairingChannel, filePath: string): string[] { let raw = ""; try { raw = fs.readFileSync(filePath, "utf8"); @@ -226,7 +226,7 @@ function readLegacyAllowFromEntries(filePath: string): string[] { const parsed = JSON.parse(raw) as { allowFrom?: unknown }; const list = Array.isArray(parsed.allowFrom) ? parsed.allowFrom : []; return dedupePreserveOrder( - list.map((entry) => normalizeOptionalString(entry) ?? "").filter(Boolean), + list.map((entry) => normalizeAllowEntry(channel, String(entry))).filter(Boolean), ); } catch { return []; @@ -376,14 +376,41 @@ function readAllowFromState(channel: PairingChannel, env: NodeJS.ProcessEnv, acc const resolvedAccountId = resolveAllowFromAccountId(accountId); const state = readChannelPairingState(channel, env); const sqliteEntries = (state.allowFrom?.[resolvedAccountId] ?? []).slice(); - const scopedLegacyEntries = readLegacyAllowFromEntries( - resolveLegacyAllowFromPath(channel, env, resolvedAccountId), + const legacyEntries = readLegacyAllowFromState(channel, env, resolvedAccountId); + return dedupePreserveOrder([...sqliteEntries, ...legacyEntries]); +} + +function resolveLegacyAllowFromFallbackPaths( + channel: PairingChannel, + env: NodeJS.ProcessEnv, + resolvedAccountId: string, +): string[] { + if (resolvedAccountId === DEFAULT_ACCOUNT_ID) { + return [resolveLegacyAllowFromPath(channel, env)]; + } + return [resolveLegacyAllowFromPath(channel, env, resolvedAccountId)]; +} + +function readLegacyAllowFromState( + channel: PairingChannel, + env: NodeJS.ProcessEnv, + resolvedAccountId: string, +): string[] { + return dedupePreserveOrder( + resolveLegacyAllowFromFallbackPaths(channel, env, resolvedAccountId).flatMap((filePath) => + readLegacyAllowFromEntries(channel, filePath), + ), ); - const defaultLegacyEntries = - resolvedAccountId === DEFAULT_ACCOUNT_ID - ? readLegacyAllowFromEntries(resolveLegacyAllowFromPath(channel, env)) - : []; - return dedupePreserveOrder([...sqliteEntries, ...scopedLegacyEntries, ...defaultLegacyEntries]); +} + +function removeLegacyAllowFromFallbackFiles( + channel: PairingChannel, + env: NodeJS.ProcessEnv, + resolvedAccountId: string, +): void { + for (const filePath of resolveLegacyAllowFromFallbackPaths(channel, env, resolvedAccountId)) { + fs.rmSync(filePath, { force: true }); + } } async function updateAllowFromStoreEntry(params: { @@ -398,7 +425,9 @@ async function updateAllowFromStoreEntry(params: { const normalized = normalizeAllowFromInput(params.channel, params.entry); return runOpenClawStateWriteTransaction((database) => { const state = readChannelPairingStateFromDatabase(database, params.channel); - const current = (state.allowFrom?.[normalizedAccountId] ?? []).slice(); + const sqliteEntries = (state.allowFrom?.[normalizedAccountId] ?? []).slice(); + const legacyEntries = readLegacyAllowFromState(params.channel, env, normalizedAccountId); + const current = dedupePreserveOrder([...sqliteEntries, ...legacyEntries]); if (!normalized) { return { changed: false, allowFrom: current }; } @@ -409,6 +438,9 @@ async function updateAllowFromStoreEntry(params: { state.allowFrom ??= {}; state.allowFrom[normalizedAccountId] = next; writeChannelPairingStateToDatabase(database, params.channel, state); + if (legacyEntries.length > 0) { + removeLegacyAllowFromFallbackFiles(params.channel, env, normalizedAccountId); + } return { changed: true, allowFrom: next }; }, sqliteOptionsForEnv(env)); }