diff --git a/src/commands/doctor-state-migrations.test.ts b/src/commands/doctor-state-migrations.test.ts index 467e68d9e0dd..b44f18a7da42 100644 --- a/src/commands/doctor-state-migrations.test.ts +++ b/src/commands/doctor-state-migrations.test.ts @@ -1437,6 +1437,32 @@ describe("doctor legacy state migrations", () => { expect(fs.existsSync(path.join(oauthDir, "telegram-allowFrom.json"))).toBe(false); }); + it("migrates a case-preserved Telegram account filename through Doctor", async () => { + const root = await makeTempRoot(); + const cfg: OpenClawConfig = { + channels: { + telegram: { + accounts: { + HY_RIN_Bot: {}, + }, + }, + }, + }; + const oauthDir = ensureCredentialsDir(root); + const sourcePath = path.join(oauthDir, "telegram-HY_RIN_Bot-allowFrom.json"); + fs.writeFileSync(sourcePath, '["1008"]\n', "utf8"); + const env = { OPENCLAW_STATE_DIR: root } as NodeJS.ProcessEnv; + + const detected = await detectLegacyStateMigrations({ cfg, env }); + const result = await runLegacyStateMigrations({ detected, config: cfg, env, now: () => 123 }); + + expect(result.warnings).toStrictEqual([]); + expect(readChannelPairingStateSnapshot("telegram", env).allowFrom).toEqual({ + hy_rin_bot: ["1008"], + }); + expect(fs.existsSync(sourcePath)).toBe(false); + }); + it("no-ops when nothing detected", async () => { const root = await makeTempRoot(); const cfg: OpenClawConfig = {}; diff --git a/src/infra/state-migrations.channel-pairing.test.ts b/src/infra/state-migrations.channel-pairing.test.ts index ba33d2a7d11a..fc38260ac585 100644 --- a/src/infra/state-migrations.channel-pairing.test.ts +++ b/src/infra/state-migrations.channel-pairing.test.ts @@ -32,6 +32,14 @@ function writeJson(filePath: string, value: unknown): void { fs.writeFileSync(filePath, `${JSON.stringify(value, null, 2)}\n`, "utf8"); } +function isCaseSensitiveDirectory(directory: string): boolean { + const marker = path.join(directory, "case-check"); + fs.writeFileSync(marker, "case", "utf8"); + const caseSensitive = !fs.existsSync(path.join(directory, "CASE-CHECK")); + fs.rmSync(marker); + return caseSensitive; +} + describe("legacy channel pairing state migration", () => { it("imports pairing requests and scoped allowFrom entries into SQLite", async () => { const { env, sourceDir } = await createFixture(); @@ -57,7 +65,7 @@ describe("legacy channel pairing state migration", () => { const detected = detectLegacyChannelPairingState({ sourceDir, - configuredAccountIds: { telegram: ["alerts", "ops/bot"] }, + configuredAccountIds: { telegram: ["alerts", "ops_bot"] }, }); expect(detected.hasLegacy).toBe(true); const result = migrateLegacyChannelPairingState({ detected, env }); @@ -76,7 +84,7 @@ describe("legacy channel pairing state migration", () => { meta: { accountId: "alerts" }, }, ], - allowFrom: { default: ["1001"], alerts: ["1002"], "ops/bot": ["1003"] }, + allowFrom: { default: ["1001"], alerts: ["1002"], ops_bot: ["1003"] }, }); expect(fs.existsSync(path.join(path.dirname(sourceDir), "state", "openclaw.sqlite"))).toBe( true, @@ -159,9 +167,9 @@ describe("legacy channel pairing state migration", () => { }); }); - it("leaves ambiguous sanitized account filenames in place", async () => { + it("matches the raw account key instead of a punctuation-normalized sibling", async () => { const { env, sourceDir } = await createFixture(); - const filePath = path.join(sourceDir, "telegram-ops_bot-allowFrom.json"); + const filePath = path.join(sourceDir, "telegram-Ops_Bot-allowFrom.json"); writeJson(filePath, { version: 1, allowFrom: ["1003"] }); const detected = detectLegacyChannelPairingState({ @@ -170,16 +178,81 @@ describe("legacy channel pairing state migration", () => { }); const result = migrateLegacyChannelPairingState({ detected, env }); - expect(result.changes).toEqual([]); - expect(result.warnings).toEqual([ - expect.stringContaining( - "Legacy channel allowFrom channel/account is ambiguous; left in place", - ), + expect(result.warnings).toEqual([]); + expect(result.changes).toEqual([ + "Migrated 1 telegram/ops_bot allowFrom entry → shared SQLite state", ]); - expect(fs.existsSync(filePath)).toBe(true); + expect(fs.existsSync(filePath)).toBe(false); + expect(readChannelPairingStateSnapshot("telegram", env).allowFrom).toEqual({ + ops_bot: ["1003"], + }); + }); + + it("leaves case-folded filename collision sets in place", async () => { + const { env, sourceDir } = await createFixture(); + const caseSensitive = isCaseSensitiveDirectory(sourceDir); + if (!caseSensitive) { + expect(caseSensitive).toBe(false); + return; + } + const filenames = [ + "telegram-AmbiguousAcct-allowFrom.json", + "telegram-AMBIGUOUSACCT-allowFrom.json", + "telegram-exactacct-allowFrom.json", + "telegram-ExactAcct-allowFrom.json", + ]; + for (const [index, filename] of filenames.entries()) { + writeJson(path.join(sourceDir, filename), { version: 1, allowFrom: [`user-${index}`] }); + } + + const detected = detectLegacyChannelPairingState({ + sourceDir, + configuredAccountIds: { telegram: ["ambiguousacct", "exactacct"] }, + }); + const result = migrateLegacyChannelPairingState({ detected, env }); + + expect(result.changes).toEqual([]); + expect(result.warnings).toHaveLength(filenames.length); + expect(result.warnings).toEqual( + expect.arrayContaining( + filenames.map((filename) => + expect.stringContaining( + `Legacy channel allowFrom channel/account is ambiguous; left in place at ${path.join(sourceDir, filename)}`, + ), + ), + ), + ); + expect(filenames.every((filename) => fs.existsSync(path.join(sourceDir, filename)))).toBe(true); expect(readChannelPairingStateSnapshot("telegram", env).allowFrom).toEqual({}); }); + it.each([ + { filenameAccountKey: "ops..bot", configuredAccountId: "ops_bot" }, + { filenameAccountKey: "ops_bot", configuredAccountId: "ops.bot" }, + ])( + "does not re-encode $filenameAccountKey into $configuredAccountId", + async ({ filenameAccountKey, configuredAccountId }) => { + const { env, sourceDir } = await createFixture(); + const filePath = path.join(sourceDir, `telegram-${filenameAccountKey}-allowFrom.json`); + writeJson(filePath, { version: 1, allowFrom: ["1003"] }); + + const detected = detectLegacyChannelPairingState({ + sourceDir, + configuredAccountIds: { telegram: [configuredAccountId] }, + }); + const result = migrateLegacyChannelPairingState({ detected, env }); + + expect(result.changes).toEqual([]); + expect(result.warnings).toEqual([ + expect.stringContaining( + "Legacy channel allowFrom channel/account is unresolved; left in place", + ), + ]); + expect(fs.existsSync(filePath)).toBe(true); + expect(readChannelPairingStateSnapshot("telegram", env).allowFrom).toEqual({}); + }, + ); + it("ignores invalid account candidates while resolving scoped filenames", async () => { const { env, sourceDir } = await createFixture(); const filePath = path.join(sourceDir, "telegram-alerts-allowFrom.json"); @@ -201,6 +274,27 @@ describe("legacy channel pairing state migration", () => { }); }); + it("leaves nonliteral default filename suffixes unresolved", async () => { + const { env, sourceDir } = await createFixture(); + const filePath = path.join(sourceDir, "telegram-DEFAULT-allowFrom.json"); + writeJson(filePath, { version: 1, allowFrom: ["1003"] }); + + const detected = detectLegacyChannelPairingState({ + sourceDir, + configuredAccountIds: { telegram: ["default"] }, + }); + const result = migrateLegacyChannelPairingState({ detected, env }); + + expect(result.changes).toEqual([]); + expect(result.warnings).toEqual([ + expect.stringContaining( + "Legacy channel allowFrom channel/account is unresolved; left in place", + ), + ]); + expect(fs.existsSync(filePath)).toBe(true); + expect(readChannelPairingStateSnapshot("telegram", env).allowFrom).toEqual({}); + }); + it("does not infer default accounts for external channels", async () => { const { env, sourceDir } = await createFixture(); const filePath = path.join(sourceDir, "custom-channel-default-allowFrom.json"); diff --git a/src/infra/state-migrations.channel-pairing.ts b/src/infra/state-migrations.channel-pairing.ts index d7f2c24e421c..5da17fa298f0 100644 --- a/src/infra/state-migrations.channel-pairing.ts +++ b/src/infra/state-migrations.channel-pairing.ts @@ -78,6 +78,35 @@ function parsePairingFilename(filename: string): PairingChannel | null { : null; } +function findCaseFoldedAllowFromCollisions( + filenames: readonly string[], + knownChannelIds: readonly string[], +): Set { + const firstFilenameByKey = new Map(); + const collisions = new Set(); + for (const filename of filenames) { + if (!filename.endsWith(ALLOW_FROM_SUFFIX)) { + continue; + } + const stem = filename.slice(0, -ALLOW_FROM_SUFFIX.length); + for (const channel of knownChannelIds) { + if (!stem.startsWith(`${channel}-`)) { + continue; + } + const accountKey = stem.slice(channel.length + 1).toLowerCase(); + const collisionKey = `${channel}\0${accountKey}`; + const firstFilename = firstFilenameByKey.get(collisionKey); + if (firstFilename) { + collisions.add(firstFilename); + collisions.add(filename); + } else { + firstFilenameByKey.set(collisionKey, filename); + } + } + } + return collisions; +} + function parseAllowFromFilename( filename: string, knownChannelIds: readonly string[], @@ -105,9 +134,14 @@ function parseAllowFromFilename( continue; } const accountKey = stem.slice(channel.length + 1); + // Fold case only: either side may contain punctuation that safe-key encoding would conflate. const matchingAccountIds = (accountIds[channel] ?? []).filter((accountId) => { try { - return safeAccountKey(accountId) === accountKey; + safeAccountKey(accountId); + if (accountId === DEFAULT_ACCOUNT_ID && accountKey !== DEFAULT_ACCOUNT_ID) { + return false; + } + return accountId.toLowerCase() === accountKey.toLowerCase(); } catch { // One invalid configured candidate must not abort every legacy migration. // With no valid match, the source remains in place as unresolved below. @@ -241,6 +275,10 @@ export function migrateLegacyChannelPairingState(params: { }): { changes: string[]; warnings: string[] } { const changes: string[] = []; const warnings: string[] = []; + const caseFoldedCollisions = findCaseFoldedAllowFromCollisions( + params.detected.files, + params.detected.knownChannelIds, + ); for (const filename of params.detected.files) { const filePath = path.join(params.detected.sourceDir, filename); const pairingChannel = parsePairingFilename(filename); @@ -269,8 +307,10 @@ export function migrateLegacyChannelPairingState(params: { if (!allowTarget) { continue; } - if (!allowTarget.target) { - const reason = allowTarget.reason === "ambiguous" ? "ambiguous" : "unresolved"; + const hasCaseFoldedCollision = caseFoldedCollisions.has(filename); + if (hasCaseFoldedCollision || !allowTarget.target) { + const reason = + hasCaseFoldedCollision || allowTarget.reason === "ambiguous" ? "ambiguous" : "unresolved"; warnings.push( `Legacy channel allowFrom channel/account is ${reason}; left in place at ${filePath}`, );