From 7e3ea6dc86a42254544fd97c9bcd079dfe405206 Mon Sep 17 00:00:00 2001 From: Chengjie Wang Date: Tue, 26 May 2026 06:50:00 +0800 Subject: [PATCH 1/2] fix(onboard): reject invalid stored messaging tokens Signed-off-by: Chengjie Wang --- src/lib/onboard.ts | 29 +++++++++++-------- .../onboard/messaging-channel-setup.test.ts | 24 +++++++++++++++ src/lib/onboard/messaging-channel-setup.ts | 25 ++++++++++++---- src/lib/onboard/messaging-token.ts | 28 ++++++++++++++++++ 4 files changed, 88 insertions(+), 18 deletions(-) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index bbf426ccc6..6f32c4fdf0 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -500,7 +500,7 @@ import { resolveMessagingChannelSeed, resolveQrSelectedChannels, } from "./onboard/messaging-state"; -import { getMessagingToken } from "./onboard/messaging-token"; +import { getMessagingToken, getValidatedMessagingToken } from "./onboard/messaging-token"; import type { DockerDriverBinaryOverrides, OpenShellInstallDeps, @@ -2996,11 +2996,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 []; @@ -3068,27 +3068,27 @@ async function createSandbox( { name: `${sandboxName}-discord-bridge`, envKey: "DISCORD_BOT_TOKEN", - token: getMessagingToken("DISCORD_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey("DISCORD_BOT_TOKEN"), }, { name: `${sandboxName}-slack-bridge`, envKey: "SLACK_BOT_TOKEN", - token: getMessagingToken("SLACK_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey("SLACK_BOT_TOKEN"), }, { name: `${sandboxName}-slack-app`, envKey: "SLACK_APP_TOKEN", - token: getMessagingToken("SLACK_APP_TOKEN"), + token: getValidatedMessagingTokenByEnvKey("SLACK_APP_TOKEN"), }, { name: `${sandboxName}-telegram-bridge`, envKey: "TELEGRAM_BOT_TOKEN", - token: getMessagingToken("TELEGRAM_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey("TELEGRAM_BOT_TOKEN"), }, { name: `${sandboxName}-wechat-bridge`, envKey: "WECHAT_BOT_TOKEN", - token: getMessagingToken("WECHAT_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey("WECHAT_BOT_TOKEN"), }, ] .filter(({ envKey }) => !enabledEnvKeys || enabledEnvKeys.has(envKey)) @@ -5860,6 +5860,11 @@ async function setupInference( const MESSAGING_CHANNELS = listChannels(); +function getValidatedMessagingTokenByEnvKey(envKey: string): string | null { + const channel = MESSAGING_CHANNELS.find((ch) => getChannelTokenKeys(ch).includes(envKey)); + return channel ? getValidatedMessagingToken(channel, envKey) : getMessagingToken(envKey); +} + function getRecordedMessagingChannelsForResume( resume: boolean, session: Session | null, @@ -5946,7 +5951,7 @@ async function setupMessagingChannels( resolveMessagingChannelSeed( availableChannels, existingChannels, - (channel) => Boolean(getMessagingToken(channel.envKey)), + (channel) => Boolean(getValidatedMessagingToken(channel, channel.envKey)), { includeAllExisting }, ); @@ -5956,7 +5961,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("TELEGRAM_BOT_TOKEN"); if (telegramToken) { await checkTelegramReachability(telegramToken); } @@ -5986,7 +5991,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"); @@ -6085,7 +6090,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("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 30012b4b55..4fe15cb1d8 100644 --- a/src/lib/onboard/messaging-channel-setup.test.ts +++ b/src/lib/onboard/messaging-channel-setup.test.ts @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { prompt, saveCredential } from "../credentials/store"; import { KNOWN_CHANNELS } from "../sandbox/channels"; import { setupSelectedMessagingChannels } from "./messaging-channel-setup"; @@ -52,4 +53,27 @@ describe("setupSelectedMessagingChannels", () => { expect(output).toContain("remove and re-add the bot to each group"); 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"); + }); }); diff --git a/src/lib/onboard/messaging-channel-setup.ts b/src/lib/onboard/messaging-channel-setup.ts index 0ebe3fac52..bfb9ab5d70 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.ts b/src/lib/onboard/messaging-token.ts index 1d22c58f76..4a38a058b7 100644 --- a/src/lib/onboard/messaging-token.ts +++ b/src/lib/onboard/messaging-token.ts @@ -2,8 +2,36 @@ // SPDX-License-Identifier: Apache-2.0 import { getCredential, normalizeCredentialValue } from "../credentials/store"; +import 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) 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; +} From e23ffad0d4f136c1f69b11b1a4b29b6c281542ed Mon Sep 17 00:00:00 2001 From: Chengjie Wang Date: Tue, 26 May 2026 07:19:59 +0800 Subject: [PATCH 2/2] fix(onboard): keep token validation out of entrypoint Signed-off-by: Chengjie Wang --- src/lib/onboard.ts | 21 +++++++----------- src/lib/onboard/messaging-token.test.ts | 29 +++++++++++++++++++++++++ src/lib/onboard/messaging-token.ts | 10 ++++++++- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 6f32c4fdf0..ff917b7a84 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -500,7 +500,7 @@ import { resolveMessagingChannelSeed, resolveQrSelectedChannels, } from "./onboard/messaging-state"; -import { getMessagingToken, getValidatedMessagingToken } from "./onboard/messaging-token"; +import { getValidatedMessagingToken, getValidatedMessagingTokenByEnvKey } from "./onboard/messaging-token"; import type { DockerDriverBinaryOverrides, OpenShellInstallDeps, @@ -3068,27 +3068,27 @@ async function createSandbox( { name: `${sandboxName}-discord-bridge`, envKey: "DISCORD_BOT_TOKEN", - token: getValidatedMessagingTokenByEnvKey("DISCORD_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "DISCORD_BOT_TOKEN"), }, { name: `${sandboxName}-slack-bridge`, envKey: "SLACK_BOT_TOKEN", - token: getValidatedMessagingTokenByEnvKey("SLACK_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "SLACK_BOT_TOKEN"), }, { name: `${sandboxName}-slack-app`, envKey: "SLACK_APP_TOKEN", - token: getValidatedMessagingTokenByEnvKey("SLACK_APP_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "SLACK_APP_TOKEN"), }, { name: `${sandboxName}-telegram-bridge`, envKey: "TELEGRAM_BOT_TOKEN", - token: getValidatedMessagingTokenByEnvKey("TELEGRAM_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "TELEGRAM_BOT_TOKEN"), }, { name: `${sandboxName}-wechat-bridge`, envKey: "WECHAT_BOT_TOKEN", - token: getValidatedMessagingTokenByEnvKey("WECHAT_BOT_TOKEN"), + token: getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "WECHAT_BOT_TOKEN"), }, ] .filter(({ envKey }) => !enabledEnvKeys || enabledEnvKeys.has(envKey)) @@ -5860,11 +5860,6 @@ async function setupInference( const MESSAGING_CHANNELS = listChannels(); -function getValidatedMessagingTokenByEnvKey(envKey: string): string | null { - const channel = MESSAGING_CHANNELS.find((ch) => getChannelTokenKeys(ch).includes(envKey)); - return channel ? getValidatedMessagingToken(channel, envKey) : getMessagingToken(envKey); -} - function getRecordedMessagingChannelsForResume( resume: boolean, session: Session | null, @@ -5961,7 +5956,7 @@ async function setupMessagingChannels( if (found.length > 0) { note(` [non-interactive] Messaging tokens detected: ${found.join(", ")}`); if (found.includes("telegram")) { - const telegramToken = getValidatedMessagingTokenByEnvKey("TELEGRAM_BOT_TOKEN"); + const telegramToken = getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "TELEGRAM_BOT_TOKEN"); if (telegramToken) { await checkTelegramReachability(telegramToken); } @@ -6090,7 +6085,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 = getValidatedMessagingTokenByEnvKey("TELEGRAM_BOT_TOKEN"); + const telegramToken = getValidatedMessagingTokenByEnvKey(MESSAGING_CHANNELS, "TELEGRAM_BOT_TOKEN"); if (telegramToken) { await checkTelegramReachability(telegramToken); } diff --git a/src/lib/onboard/messaging-token.test.ts b/src/lib/onboard/messaging-token.test.ts index 039d6c303e..92d9f1f71b 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,31 @@ 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 { getValidatedMessagingTokenByEnvKey } = await import("./messaging-token"); + + 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 4a38a058b7..6db39793de 100644 --- a/src/lib/onboard/messaging-token.ts +++ b/src/lib/onboard/messaging-token.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { getCredential, normalizeCredentialValue } from "../credentials/store"; -import type { ChannelDef } from "../sandbox/channels"; +import { getChannelTokenKeys, type ChannelDef } from "../sandbox/channels"; export function getMessagingToken(envKey: string | undefined): string | null { if (!envKey) return null; @@ -35,3 +35,11 @@ export function getValidatedMessagingToken( 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; +}