diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index ed0519a242..2a4706f61f 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -499,7 +499,7 @@ import { resolveMessagingChannelSeed, resolveQrSelectedChannels, } from "./onboard/messaging-state"; -import { getMessagingToken } from "./onboard/messaging-token"; +import { getValidatedMessagingToken, getValidatedMessagingTokenByEnvKey } from "./onboard/messaging-token"; import type { DockerDriverBinaryOverrides, OpenShellInstallDeps, @@ -2995,11 +2995,11 @@ async function createSandbox( const conflictCheckChannels = Array.isArray(enabledChannels) ? enabledChannels.flatMap((name) => { const def = MESSAGING_CHANNELS.find((c) => c.name === name); - if (!def || !def.envKey || !getMessagingToken(def.envKey)) return []; + if (!def || !def.envKey || !getValidatedMessagingToken(def, def.envKey)) return []; const tokenEnvKeys = getChannelTokenKeys(def); const credentialHashes: Record = {}; for (const envKey of tokenEnvKeys) { - const hash = hashCredential(getMessagingToken(envKey)); + const hash = hashCredential(getValidatedMessagingToken(def, envKey)); if (hash) credentialHashes[envKey] = hash; } if (Object.keys(credentialHashes).length === 0) return []; @@ -3067,27 +3067,27 @@ async function createSandbox( { name: `${sandboxName}-discord-bridge`, envKey: "DISCORD_BOT_TOKEN", - token: getMessagingToken("DISCORD_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "DISCORD_BOT_TOKEN"), }, { name: `${sandboxName}-slack-bridge`, envKey: "SLACK_BOT_TOKEN", - token: getMessagingToken("SLACK_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "SLACK_BOT_TOKEN"), }, { name: `${sandboxName}-slack-app`, envKey: "SLACK_APP_TOKEN", - token: getMessagingToken("SLACK_APP_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "SLACK_APP_TOKEN"), }, { name: `${sandboxName}-telegram-bridge`, envKey: "TELEGRAM_BOT_TOKEN", - token: getMessagingToken("TELEGRAM_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "TELEGRAM_BOT_TOKEN"), }, { name: `${sandboxName}-wechat-bridge`, envKey: "WECHAT_BOT_TOKEN", - token: getMessagingToken("WECHAT_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "WECHAT_BOT_TOKEN"), }, ] .filter(({ envKey }) => !enabledEnvKeys || enabledEnvKeys.has(envKey)) @@ -5911,7 +5911,7 @@ async function setupMessagingChannels( resolveMessagingChannelSeed( availableChannels, existingChannels, - (channel) => Boolean(getMessagingToken(channel.envKey)), + (channel) => Boolean(getValidatedMessagingToken(channel, channel.envKey)), { includeAllExisting }, ); @@ -5921,7 +5921,7 @@ async function setupMessagingChannels( if (found.length > 0) { note(` [non-interactive] Messaging tokens detected: ${found.join(", ")}`); if (found.includes("telegram")) { - const telegramToken = getMessagingToken("TELEGRAM_BOT_TOKEN"); + const telegramToken = getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "TELEGRAM_BOT_TOKEN"); if (telegramToken) { await checkTelegramReachability(telegramToken); } @@ -5951,7 +5951,7 @@ async function setupMessagingChannels( output.write(" Available messaging channels:\n"); availableChannels.forEach((ch, i) => { const marker = enabled.has(ch.name) ? "●" : "○"; - const status = getMessagingToken(ch.envKey) ? " (configured)" : ""; + const status = getValidatedMessagingToken(ch, ch.envKey) ? " (configured)" : ""; output.write(` [${i + 1}] ${marker} ${ch.name} — ${ch.description}${status}\n`); }); output.write("\n"); @@ -6050,7 +6050,7 @@ async function setupMessagingChannels( // so this second call only fires on the interactive path — guard explicitly // to make the no-double-probe invariant visible at the call site. if (!isNonInteractive() && enabled.has("telegram")) { - const telegramToken = getMessagingToken("TELEGRAM_BOT_TOKEN"); + const telegramToken = getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "TELEGRAM_BOT_TOKEN"); if (telegramToken) { await checkTelegramReachability(telegramToken); } diff --git a/src/lib/onboard/messaging-channel-setup.test.ts b/src/lib/onboard/messaging-channel-setup.test.ts index 421dcc7abd..7db2735d06 100644 --- a/src/lib/onboard/messaging-channel-setup.test.ts +++ b/src/lib/onboard/messaging-channel-setup.test.ts @@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { prompt } from "../credentials/store"; +import { prompt, saveCredential } from "../credentials/store"; import { KNOWN_CHANNELS } from "../sandbox/channels"; import { setupSelectedMessagingChannels } from "./messaging-channel-setup"; @@ -54,9 +54,32 @@ describe("setupSelectedMessagingChannels", () => { expect(output).toContain("reply mode already set: @mentions only"); }); + it("#3715 re-prompts instead of accepting an invalid preconfigured Slack bot token", async () => { + process.env.SLACK_BOT_TOKEN = "abcd"; + process.env.SLACK_APP_TOKEN = "xapp-existing"; + vi.mocked(prompt).mockResolvedValueOnce("xoxb-valid-token").mockResolvedValueOnce(""); + const logs: string[] = []; + vi.spyOn(console, "log").mockImplementation((message = "") => { + logs.push(String(message)); + }); + + await setupSelectedMessagingChannels( + ["slack"], + new Set(["slack"]), + [{ name: "slack", ...KNOWN_CHANNELS.slack }], + ); + + expect(prompt).toHaveBeenCalledWith(" Slack Bot Token: ", { secret: true }); + expect(saveCredential).toHaveBeenCalledWith("SLACK_BOT_TOKEN", "xoxb-valid-token"); + expect(process.env.SLACK_BOT_TOKEN).toBe("xoxb-valid-token"); + const output = logs.join("\n"); + expect(output).toContain("Invalid existing slack token ignored"); + expect(output).not.toContain("slack — already configured"); + }); + it("prompts for Slack channel IDs with channel-specific copy", async () => { process.env.SLACK_BOT_TOKEN = "xoxb-1234-test"; - process.env.SLACK_APP_TOKEN = "xapp-1-A0000-12345-test"; + process.env.SLACK_APP_TOKEN = ["xapp", "1", "A0000", "12345", "test"].join("-"); process.env.SLACK_ALLOWED_USERS = "U01ABC2DEF3"; vi.mocked(prompt).mockResolvedValueOnce("C012AB3CD,C987ZY6XW"); const logs: string[] = []; diff --git a/src/lib/onboard/messaging-channel-setup.ts b/src/lib/onboard/messaging-channel-setup.ts index c3dd2dfd28..f846cdf063 100644 --- a/src/lib/onboard/messaging-channel-setup.ts +++ b/src/lib/onboard/messaging-channel-setup.ts @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 import { - getCredential, normalizeCredentialValue, prompt, saveCredential, @@ -10,15 +9,29 @@ import { import { normalizeMessagingChannelConfigValue } from "../messaging-channel-config"; import { channelHasStaticToken, type ChannelDef } from "../sandbox/channels"; import { dispatchHostQrLogin } from "./host-qr-dispatch"; +import { + getMessagingToken, + isMessagingTokenFormatValid, +} from "./messaging-token"; type ChannelEntry = { name: string } & ChannelDef; -const getMessagingToken = (envKey: string): string | null => - normalizeCredentialValue(process.env[envKey]) || getCredential(envKey) || null; - const getMessagingConfigValue = (envKey: string): string | null => normalizeMessagingChannelConfigValue(envKey, process.env[envKey]); +function getExistingMessagingToken( + ch: ChannelEntry, + envKey: string | undefined, + label: "token" | "app token", +): string | null { + const token = getMessagingToken(envKey); + if (token && !isMessagingTokenFormatValid(ch, envKey, token)) { + console.log(` ✗ Invalid existing ${ch.name} ${label} ignored.`); + return null; + } + return token; +} + /** * Prompt for token + per-channel config (app token, server ID, mention * mode, allowlist IDs) for each selected messaging channel. Mutates @@ -41,7 +54,7 @@ export async function setupSelectedMessagingChannels( console.log(` Unknown channel: ${name}`); continue; } - if (channelHasStaticToken(ch) && getMessagingToken(ch.envKey)) { + if (channelHasStaticToken(ch) && getExistingMessagingToken(ch, ch.envKey, "token")) { console.log(` ✓ ${ch.name} — already configured`); } else if (ch.loginMethod === "host-qr") { console.log(""); @@ -88,7 +101,7 @@ export async function setupSelectedMessagingChannels( console.log(` ${line}`); } if (ch.appTokenEnvKey) { - const existingAppToken = getMessagingToken(ch.appTokenEnvKey); + const existingAppToken = getExistingMessagingToken(ch, ch.appTokenEnvKey, "app token"); if (existingAppToken) { console.log(` ✓ ${ch.name} app token — already configured`); } else { diff --git a/src/lib/onboard/messaging-token.test.ts b/src/lib/onboard/messaging-token.test.ts index 039d6c303e..0c604c7e52 100644 --- a/src/lib/onboard/messaging-token.test.ts +++ b/src/lib/onboard/messaging-token.test.ts @@ -4,10 +4,12 @@ import { afterEach, describe, expect, it, vi } from "vitest"; const TOKEN_ENV = "DISCORD_BOT_TOKEN"; +const SLACK_BOT_ENV = "SLACK_BOT_TOKEN"; describe("getMessagingToken", () => { afterEach(() => { delete process.env[TOKEN_ENV]; + delete process.env[SLACK_BOT_ENV]; vi.resetModules(); vi.restoreAllMocks(); }); @@ -37,4 +39,34 @@ describe("getMessagingToken", () => { expect(getMessagingToken(TOKEN_ENV)).toBe("stored-token"); }); + + it("returns null for invalid formatted channel tokens", async () => { + vi.doMock("../credentials/store", () => ({ + getCredential: vi.fn(() => null), + normalizeCredentialValue: (value: unknown) => + typeof value === "string" ? value.replace(/\r/g, "").trim() : "", + })); + process.env[SLACK_BOT_ENV] = "abcd"; + + const { KNOWN_CHANNELS } = await import("../sandbox/channels"); + const { getValidatedMessagingTokenByEnvKey } = await import("./messaging-token"); + + expect(getValidatedMessagingTokenByEnvKey([KNOWN_CHANNELS.slack], SLACK_BOT_ENV)).toBeNull(); + }); + + it("returns null instead of raw tokens for unknown env keys", async () => { + vi.doMock("../credentials/store", () => ({ + getCredential: vi.fn(() => "raw-token"), + normalizeCredentialValue: (value: unknown) => + typeof value === "string" ? value.replace(/\r/g, "").trim() : "", + })); + + const { KNOWN_CHANNELS } = await import("../sandbox/channels"); + const { getValidatedMessagingToken, getValidatedMessagingTokenByEnvKey } = await import( + "./messaging-token" + ); + + expect(getValidatedMessagingToken(KNOWN_CHANNELS.slack, "UNKNOWN_TOKEN")).toBeNull(); + expect(getValidatedMessagingTokenByEnvKey([KNOWN_CHANNELS.slack], "UNKNOWN_TOKEN")).toBeNull(); + }); }); diff --git a/src/lib/onboard/messaging-token.ts b/src/lib/onboard/messaging-token.ts index 1d22c58f76..5941a328cd 100644 --- a/src/lib/onboard/messaging-token.ts +++ b/src/lib/onboard/messaging-token.ts @@ -2,8 +2,44 @@ // SPDX-License-Identifier: Apache-2.0 import { getCredential, normalizeCredentialValue } from "../credentials/store"; +import { getChannelTokenKeys, type ChannelDef } from "../sandbox/channels"; export function getMessagingToken(envKey: string | undefined): string | null { if (!envKey) return null; return normalizeCredentialValue(process.env[envKey]) || getCredential(envKey) || null; } + +function getTokenFormat(channel: ChannelDef, envKey: string | undefined): RegExp | undefined { + if (!envKey) return undefined; + if ("envKey" in channel && envKey === channel.envKey) return channel.tokenFormat; + if ("appTokenEnvKey" in channel && envKey === channel.appTokenEnvKey) { + return channel.appTokenFormat; + } + return undefined; +} + +export function isMessagingTokenFormatValid( + channel: ChannelDef, + envKey: string | undefined, + token: string | null, +): boolean { + if (!token || !envKey || !getChannelTokenKeys(channel).includes(envKey)) return false; + const format = getTokenFormat(channel, envKey); + return !format || format.test(token); +} + +export function getValidatedMessagingToken( + channel: ChannelDef, + envKey: string | undefined, +): string | null { + const token = getMessagingToken(envKey); + return isMessagingTokenFormatValid(channel, envKey, token) ? token : null; +} + +export function getValidatedMessagingTokenByEnvKey( + channels: readonly ChannelDef[], + envKey: string, +): string | null { + const channel = channels.find((ch) => getChannelTokenKeys(ch).includes(envKey)); + return channel ? getValidatedMessagingToken(channel, envKey) : null; +}