From 1f1e85480767e3b065ef4eddaad30829cb069a90 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Fri, 17 Apr 2026 11:50:19 -0400 Subject: [PATCH] CLI: harden lazy channel setup prompts --- src/channels/plugins/setup-registry.ts | 6 ++ src/commands/configure.channels.test.ts | 74 +++++++++++++++- src/commands/configure.channels.ts | 10 ++- src/flows/channel-setup.status.test.ts | 22 +++++ src/flows/channel-setup.status.ts | 16 +++- src/flows/channel-setup.test.ts | 112 +++++++++++++++++++----- src/flows/channel-setup.ts | 46 ++++++---- 7 files changed, 241 insertions(+), 45 deletions(-) diff --git a/src/channels/plugins/setup-registry.ts b/src/channels/plugins/setup-registry.ts index fa8ed0fb6e8..3e7269a5a32 100644 --- a/src/channels/plugins/setup-registry.ts +++ b/src/channels/plugins/setup-registry.ts @@ -1,4 +1,5 @@ import { + getActivePluginChannelRegistry, getActivePluginRegistryVersion, requireActivePluginRegistry, } from "../../plugins/runtime.js"; @@ -82,6 +83,11 @@ export function listChannelSetupPlugins(): ChannelPlugin[] { return resolveCachedChannelSetupPlugins().sorted.slice(); } +export function listActiveChannelSetupPlugins(): ChannelPlugin[] { + const registry = getActivePluginChannelRegistry(); + return sortChannelSetupPlugins((registry?.channelSetups ?? []).map((entry) => entry.plugin)); +} + export function getChannelSetupPlugin(id: ChannelId): ChannelPlugin | undefined { const resolvedId = normalizeOptionalString(id) ?? ""; if (!resolvedId) { diff --git a/src/commands/configure.channels.test.ts b/src/commands/configure.channels.test.ts index 4245cc79f5e..fefdaf1caf7 100644 --- a/src/commands/configure.channels.test.ts +++ b/src/commands/configure.channels.test.ts @@ -3,12 +3,15 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const select = vi.hoisted(() => vi.fn()); const confirm = vi.hoisted(() => vi.fn()); const note = vi.hoisted(() => vi.fn()); - -vi.mock("../channels/chat-meta.js", () => ({ - listChatChannels: () => [ +const chatChannels = vi.hoisted(() => + vi.fn(() => [ { id: "telegram", label: "Telegram" }, { id: "twitch", label: "Twitch" }, - ], + ]), +); + +vi.mock("../channels/chat-meta.js", () => ({ + listChatChannels: () => chatChannels(), })); vi.mock("../terminal/note.js", () => ({ @@ -25,6 +28,10 @@ import { removeChannelConfigWizard } from "./configure.channels.js"; describe("removeChannelConfigWizard", () => { beforeEach(() => { vi.resetAllMocks(); + chatChannels.mockReturnValue([ + { id: "telegram", label: "Telegram" }, + { id: "twitch", label: "Twitch" }, + ]); confirm.mockResolvedValue(true); }); @@ -102,6 +109,65 @@ describe("removeChannelConfigWizard", () => { }); }); + it("does not list blocked object keys as removable channels", async () => { + select.mockResolvedValue("done"); + + await removeChannelConfigWizard( + { + channels: { + __proto__: { token: "secret" }, + constructor: { token: "secret" }, + prototype: { token: "secret" }, + telegram: { token: "secret" }, + }, + } as never, + {} as never, + ); + + expect(select).toHaveBeenCalledWith( + expect.objectContaining({ + options: [ + expect.objectContaining({ value: "telegram", label: "Telegram" }), + { value: "done", label: "Done" }, + ], + }), + ); + }); + + it("sanitizes known channel labels before rendering prompts", async () => { + chatChannels.mockReturnValue([ + { id: "telegram", label: "Telegram\u001B[31m\nBot\u0007" }, + { id: "twitch", label: "Twitch" }, + ]); + select.mockResolvedValueOnce("telegram").mockResolvedValueOnce("done"); + + await removeChannelConfigWizard( + { + channels: { + telegram: { token: "secret" }, + }, + } as never, + {} as never, + ); + + expect(select).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.arrayContaining([ + expect.objectContaining({ value: "telegram", label: "Telegram\\nBot" }), + ]), + }), + ); + expect(confirm).toHaveBeenCalledWith( + expect.objectContaining({ + message: "Delete Telegram\\nBot configuration from ~/.openclaw/openclaw.json?", + }), + ); + expect(note).toHaveBeenCalledWith( + "Telegram\\nBot removed from config.\nNote: credentials/sessions on disk are unchanged.", + "Channel removed", + ); + }); + it("sanitizes unknown channel keys before rendering prompts", async () => { const unsafeChannel = "bad\u001B[31m\nkey\u0007"; select.mockResolvedValueOnce(unsafeChannel).mockResolvedValueOnce("done"); diff --git a/src/commands/configure.channels.ts b/src/commands/configure.channels.ts index 15d36362b85..e4452978578 100644 --- a/src/commands/configure.channels.ts +++ b/src/commands/configure.channels.ts @@ -1,6 +1,7 @@ import { listChatChannels } from "../channels/chat-meta.js"; import { formatCliCommand } from "../cli/command-format.js"; import { CONFIG_PATH } from "../config/config.js"; +import { isBlockedObjectKey } from "../config/prototype-keys.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { RuntimeEnv } from "../runtime.js"; import { note } from "../terminal/note.js"; @@ -23,9 +24,12 @@ function listConfiguredChannelRemovalChoices( if (!channels) { return []; } - const labelsById = new Map(listChatChannels().map((meta) => [meta.id, meta.label])); + const labelsById = new Map( + listChatChannels().map((meta) => [meta.id, formatChannelRemovalLabel(meta.label, meta.id)]), + ); return Object.keys(channels) .filter((id) => !RESERVED_CHANNEL_CONFIG_KEYS.has(id)) + .filter((id) => !isBlockedObjectKey(id)) .map((id) => ({ id, label: labelsById.get(id) ?? formatUnknownChannelRemovalLabel(id), @@ -33,6 +37,10 @@ function listConfiguredChannelRemovalChoices( .toSorted(compareChannelRemovalChoices); } +function formatChannelRemovalLabel(label: string, fallback: string): string { + return sanitizeTerminalText(label) || formatUnknownChannelRemovalLabel(fallback); +} + function formatUnknownChannelRemovalLabel(id: string): string { return sanitizeTerminalText(id) || ""; } diff --git a/src/flows/channel-setup.status.test.ts b/src/flows/channel-setup.status.test.ts index 525b128ba54..d1e8c576be4 100644 --- a/src/flows/channel-setup.status.test.ts +++ b/src/flows/channel-setup.status.test.ts @@ -113,4 +113,26 @@ describe("resolveChannelSetupSelectionContributions", () => { hint: "configured · disabled", }); }); + + it("sanitizes picker labels and hints before terminal rendering", () => { + const contributions = resolveChannelSetupSelectionContributions({ + entries: [ + { + id: "zalo", + meta: { + id: "zalo", + label: "Zalo\u001B[31m\nBot\u0007", + }, + }, + ] as never, + statusByChannel: new Map([["zalo", { selectionHint: "configured\u001B[2K\nnow" }]]), + resolveDisabledHint: () => "disabled\u0007", + }); + + expect(contributions[0]?.option).toEqual({ + value: "zalo", + label: "Zalo\\nBot", + hint: "configured\\nnow · disabled", + }); + }); }); diff --git a/src/flows/channel-setup.status.ts b/src/flows/channel-setup.status.ts index f84ac0d1817..bdbbf6172cb 100644 --- a/src/flows/channel-setup.status.ts +++ b/src/flows/channel-setup.status.ts @@ -17,6 +17,7 @@ import type { ChannelChoice } from "../commands/onboard-types.js"; import { isChannelConfigured } from "../config/channel-configured.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { formatDocsLink } from "../terminal/links.js"; +import { sanitizeTerminalText } from "../terminal/safe-text.js"; import type { WizardPrompter } from "../wizard/prompts.js"; import type { FlowContribution } from "./types.js"; @@ -67,6 +68,17 @@ function buildChannelSetupSelectionContribution(params: { }; } +function formatSetupSelectionLabel(label: string, fallback: string): string { + return sanitizeTerminalText(label) || fallback; +} + +function formatSetupSelectionHint(hint: string | undefined): string | undefined { + if (!hint) { + return undefined; + } + return sanitizeTerminalText(hint) || undefined; +} + export async function collectChannelStatus(params: { cfg: OpenClawConfig; options?: SetupChannelsOptions; @@ -261,8 +273,8 @@ export function resolveChannelSetupSelectionContributions(params: { const hint = [statusHint, disabledHint].filter(Boolean).join(" · ") || undefined; return buildChannelSetupSelectionContribution({ channel: entry.id, - label: entry.meta.selectionLabel ?? entry.meta.label, - hint, + label: formatSetupSelectionLabel(entry.meta.selectionLabel ?? entry.meta.label, entry.id), + hint: formatSetupSelectionHint(hint), source: bundledChannelIds.has(entry.id) ? "core" : "plugin", }); }); diff --git a/src/flows/channel-setup.test.ts b/src/flows/channel-setup.test.ts index 928d9eca210..058e2641434 100644 --- a/src/flows/channel-setup.test.ts +++ b/src/flows/channel-setup.test.ts @@ -9,6 +9,7 @@ const listTrustedChannelPluginCatalogEntries = vi.hoisted(() => ); const getChannelSetupPlugin = vi.hoisted(() => vi.fn((_channel?: unknown) => undefined)); const listChannelSetupPlugins = vi.hoisted(() => vi.fn((): unknown[] => [])); +const listActiveChannelSetupPlugins = vi.hoisted(() => vi.fn((): unknown[] => [])); const loadChannelSetupPluginRegistrySnapshotForChannel = vi.hoisted(() => vi.fn((_params?: unknown) => ({ channels: [], channelSetups: [] })), ); @@ -50,6 +51,7 @@ vi.mock("../agents/agent-scope.js", () => ({ vi.mock("../channels/plugins/setup-registry.js", () => ({ getChannelSetupPlugin: (channel?: unknown) => getChannelSetupPlugin(channel), + listActiveChannelSetupPlugins: () => listActiveChannelSetupPlugins(), listChannelSetupPlugins: () => listChannelSetupPlugins(), })); @@ -116,6 +118,7 @@ describe("setupChannels workspace shadow exclusion", () => { }, ]); getChannelSetupPlugin.mockReturnValue(undefined); + listActiveChannelSetupPlugins.mockReturnValue([]); listChannelSetupPlugins.mockReturnValue([]); loadChannelSetupPluginRegistrySnapshotForChannel.mockReturnValue({ channels: [], @@ -228,6 +231,44 @@ describe("setupChannels workspace shadow exclusion", () => { expect(loadChannelSetupPluginRegistrySnapshotForChannel).not.toHaveBeenCalled(); }); + it("keeps already-active setup plugins in the deferred picker without registry fallback", async () => { + const activePlugin = { + id: "custom-chat", + meta: { id: "custom-chat", label: "Custom Chat", blurb: "" }, + }; + listActiveChannelSetupPlugins.mockReturnValue([activePlugin]); + resolveChannelSetupEntries.mockImplementation(() => ({ + entries: [], + installedCatalogEntries: [], + installableCatalogEntries: [], + installedCatalogById: new Map(), + installableCatalogById: new Map(), + })); + const select = vi.fn(async () => "__done__"); + + await setupChannels( + {} as never, + {} as never, + { + confirm: vi.fn(async () => true), + note: vi.fn(async () => undefined), + select, + } as never, + { + deferStatusUntilSelection: true, + skipConfirm: true, + }, + ); + + expect(resolveChannelSetupEntries).toHaveBeenCalledWith( + expect.objectContaining({ + installedPlugins: [activePlugin], + }), + ); + expect(listChannelSetupPlugins).not.toHaveBeenCalled(); + expect(collectChannelStatus).not.toHaveBeenCalled(); + }); + it("loads the selected bundled catalog plugin without writing explicit plugin enablement", async () => { const setupWizard = { channel: "telegram", @@ -315,7 +356,7 @@ describe("setupChannels workspace shadow exclusion", () => { }); }); - it("does not re-enable an explicitly disabled channel when selected lazily", async () => { + it("does not load or re-enable an explicitly disabled channel when selected lazily", async () => { const setupWizard = { channel: "telegram", getStatus: vi.fn(async () => ({ @@ -325,15 +366,6 @@ describe("setupChannels workspace shadow exclusion", () => { })), configure: vi.fn(), }; - const telegramPlugin = { - id: "telegram", - meta: { id: "telegram", label: "Telegram", blurb: "" }, - capabilities: {}, - config: { - resolveAccount: vi.fn(() => ({ enabled: false })), - }, - setupWizard, - }; resolveChannelSetupEntries.mockReturnValue({ entries: [ { @@ -346,11 +378,8 @@ describe("setupChannels workspace shadow exclusion", () => { installedCatalogById: new Map(), installableCatalogById: new Map(), }); - loadChannelSetupPluginRegistrySnapshotForChannel.mockReturnValue({ - channels: [{ plugin: telegramPlugin }], - channelSetups: [], - }); const select = vi.fn().mockResolvedValueOnce("telegram").mockResolvedValueOnce("__done__"); + const note = vi.fn(async () => undefined); const cfg = { channels: { telegram: { enabled: false, token: "secret" }, @@ -362,7 +391,7 @@ describe("setupChannels workspace shadow exclusion", () => { {} as never, { confirm: vi.fn(async () => true), - note: vi.fn(async () => undefined), + note, select, } as never, { @@ -372,11 +401,10 @@ describe("setupChannels workspace shadow exclusion", () => { }, ); - expect(loadChannelSetupPluginRegistrySnapshotForChannel).toHaveBeenCalledWith( - expect.objectContaining({ - channel: "telegram", - workspaceDir: "/tmp/openclaw-workspace", - }), + expect(loadChannelSetupPluginRegistrySnapshotForChannel).not.toHaveBeenCalled(); + expect(note).toHaveBeenCalledWith( + "telegram cannot be configured while disabled. Enable it before setup.", + "Channel setup", ); expect(setupWizard.configure).not.toHaveBeenCalled(); expect(next).toEqual({ @@ -385,4 +413,48 @@ describe("setupChannels workspace shadow exclusion", () => { }, }); }); + + it("honors global plugin disablement before lazy channel setup loads plugins", async () => { + resolveChannelSetupEntries.mockReturnValue({ + entries: [ + { + id: "telegram", + meta: { id: "telegram", label: "Telegram", blurb: "" }, + }, + ], + installedCatalogEntries: [], + installableCatalogEntries: [], + installedCatalogById: new Map(), + installableCatalogById: new Map(), + }); + const select = vi.fn().mockResolvedValueOnce("telegram").mockResolvedValueOnce("__done__"); + const note = vi.fn(async () => undefined); + const cfg = { + plugins: { enabled: false }, + channels: { + telegram: { enabled: true, token: "secret" }, + }, + }; + + await setupChannels( + cfg as never, + {} as never, + { + confirm: vi.fn(async () => true), + note, + select, + } as never, + { + deferStatusUntilSelection: true, + skipConfirm: true, + skipDmPolicyPrompt: true, + }, + ); + + expect(loadChannelSetupPluginRegistrySnapshotForChannel).not.toHaveBeenCalled(); + expect(note).toHaveBeenCalledWith( + "telegram cannot be configured while plugins disabled. Enable it before setup.", + "Channel setup", + ); + }); }); diff --git a/src/flows/channel-setup.ts b/src/flows/channel-setup.ts index 10d1e5ec062..e7cd7b69533 100644 --- a/src/flows/channel-setup.ts +++ b/src/flows/channel-setup.ts @@ -2,6 +2,7 @@ import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent import { resolveChannelDefaultAccountId } from "../channels/plugins/helpers.js"; import { getChannelSetupPlugin, + listActiveChannelSetupPlugins, listChannelSetupPlugins, } from "../channels/plugins/setup-registry.js"; import type { @@ -132,11 +133,12 @@ export async function setupChannels( }): ChannelSetupPlugin[] => { const includeRegistry = params?.includeRegistry ?? includeRegistryBeforeSelection; const merged = new Map(); - if (includeRegistry) { - for (const plugin of listChannelSetupPlugins()) { - if (shouldShowChannelInSetup(plugin.meta)) { - merged.set(plugin.id, plugin); - } + const registryPlugins = includeRegistry + ? listChannelSetupPlugins() + : listActiveChannelSetupPlugins(); + for (const plugin of registryPlugins) { + if (shouldShowChannelInSetup(plugin.meta)) { + merged.set(plugin.id, plugin); } } for (const plugin of scopedPluginsById.values()) { @@ -261,6 +263,12 @@ export async function setupChannels( }; const resolveConfigDisabledHint = (channel: ChannelChoice): string | undefined => { + if (next.plugins?.enabled === false) { + return "plugins disabled"; + } + if (next.plugins?.entries?.[channel]?.enabled === false) { + return "plugin disabled"; + } if ( typeof (next.channels as Record | undefined)?.[channel] ?.enabled === "boolean" @@ -269,12 +277,6 @@ export async function setupChannels( ? "disabled" : undefined; } - if (next.plugins?.entries?.[channel]?.enabled === false) { - return "plugin disabled"; - } - if (next.plugins?.enabled === false) { - return "plugins disabled"; - } return undefined; }; @@ -325,13 +327,11 @@ export async function setupChannels( } const disabledHint = resolveConfigDisabledHint(channel); if (disabledHint) { - const plugin = await loadScopedChannelPlugin(channel); - if (!plugin) { - await prompter.note(`${channel} plugin not available (${disabledHint}).`, "Channel setup"); - return false; - } - await refreshStatus(channel); - return true; + await prompter.note( + `${channel} cannot be configured while ${disabledHint}. Enable it before setup.`, + "Channel setup", + ); + return false; } const result = enablePluginInConfig(next, channel); next = result.config; @@ -507,6 +507,16 @@ export async function setupChannels( const { catalogById, installedCatalogById } = getChannelEntries(); const catalogEntry = catalogById.get(channel); const installedCatalogEntry = installedCatalogById.get(channel); + const deferredDisabledHint = deferStatusUntilSelection + ? resolveConfigDisabledHint(channel) + : undefined; + if (deferredDisabledHint) { + await prompter.note( + `${channel} cannot be configured while ${deferredDisabledHint}. Enable it before setup.`, + "Channel setup", + ); + return; + } if (catalogEntry) { const workspaceDir = resolveWorkspaceDir(); const result = await ensureChannelSetupPluginInstalled({