From 282c32db7caea2520a80b4993ceb96dc6d447e97 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Fri, 24 Apr 2026 22:22:57 -0400 Subject: [PATCH] fix(cli): sanitize plugin command descriptors --- docs/plugins/sdk-entrypoints.md | 4 ++ .../program/command-descriptor-utils.test.ts | 22 ++++++++ src/cli/program/command-descriptor-utils.ts | 27 ++++++++-- src/plugins/loader.cli-metadata.test.ts | 54 +++++++++++++++++++ src/plugins/registry.ts | 40 +++++++++++--- src/terminal/ansi.test.ts | 1 + src/terminal/ansi.ts | 5 +- 7 files changed, 140 insertions(+), 13 deletions(-) diff --git a/docs/plugins/sdk-entrypoints.md b/docs/plugins/sdk-entrypoints.md index 510c4a4e27b..f87257a0b96 100644 --- a/docs/plugins/sdk-entrypoints.md +++ b/docs/plugins/sdk-entrypoints.md @@ -240,6 +240,10 @@ For CLI registrars specifically: want OpenClaw to lazy-load the real CLI module on first invocation - make sure those descriptors cover every top-level command root exposed by the registrar +- keep descriptor command names to letters, numbers, hyphen, and underscore, + starting with a letter or number; OpenClaw rejects descriptor names outside + that shape and strips terminal control sequences from descriptions before + rendering help - use `commands` alone only for eager compatibility paths ## Plugin shapes diff --git a/src/cli/program/command-descriptor-utils.test.ts b/src/cli/program/command-descriptor-utils.test.ts index 4eab242c07c..706f52d2f1f 100644 --- a/src/cli/program/command-descriptor-utils.test.ts +++ b/src/cli/program/command-descriptor-utils.test.ts @@ -71,4 +71,26 @@ describe("command-descriptor-utils", () => { "delta", ]); }); + + it("strips terminal escapes from rendered descriptor descriptions", () => { + const program = new Command(); + + addCommandDescriptorsToProgram(program, [ + { + name: "safe-command", + description: "Open \u001B]8;;https://example.test\u0007link\u001B]8;;\u0007 now\u001B[2J", + }, + ]); + + expect(program.commands[0]?.description()).toBe("Open link now"); + }); + + it("rejects unsafe descriptor command names before rendering", () => { + const program = new Command(); + + expect(() => + addCommandDescriptorsToProgram(program, [{ name: "bad\nname", description: "Bad" }]), + ).toThrow('Invalid CLI command name: "bad\\nname"'); + expect(program.commands).toEqual([]); + }); }); diff --git a/src/cli/program/command-descriptor-utils.ts b/src/cli/program/command-descriptor-utils.ts index 5cb247f035b..9101e55d45f 100644 --- a/src/cli/program/command-descriptor-utils.ts +++ b/src/cli/program/command-descriptor-utils.ts @@ -1,8 +1,11 @@ import type { Command } from "commander"; +import { sanitizeForLog } from "../../terminal/ansi.js"; import type { NamedCommandDescriptor } from "./command-group-descriptors.js"; export type CommandDescriptorLike = Pick; +const SAFE_COMMAND_NAME_PATTERN = /^[A-Za-z0-9][A-Za-z0-9_-]*$/; + export type CommandDescriptorCatalog = { descriptors: readonly TDescriptor[]; getDescriptors: () => readonly TDescriptor[]; @@ -10,6 +13,23 @@ export type CommandDescriptorCatalog getCommandsWithSubcommands: () => string[]; }; +export function normalizeCommandDescriptorName(name: string): string | null { + const normalized = name.trim(); + return SAFE_COMMAND_NAME_PATTERN.test(normalized) ? normalized : null; +} + +export function assertSafeCommandDescriptorName(name: string): string { + const normalized = normalizeCommandDescriptorName(name); + if (!normalized) { + throw new Error(`Invalid CLI command name: ${JSON.stringify(name.trim())}`); + } + return normalized; +} + +export function sanitizeCommandDescriptorDescription(description: string): string { + return sanitizeForLog(description).trim(); +} + export function getCommandDescriptorNames(descriptors: readonly CommandDescriptorLike[]): string[] { return descriptors.map((descriptor) => descriptor.name); } @@ -56,11 +76,12 @@ export function addCommandDescriptorsToProgram( existingCommands: Set = new Set(), ): Set { for (const descriptor of descriptors) { - if (existingCommands.has(descriptor.name)) { + const name = assertSafeCommandDescriptorName(descriptor.name); + if (existingCommands.has(name)) { continue; } - program.command(descriptor.name).description(descriptor.description); - existingCommands.add(descriptor.name); + program.command(name).description(sanitizeCommandDescriptorDescription(descriptor.description)); + existingCommands.add(name); } return existingCommands; } diff --git a/src/plugins/loader.cli-metadata.test.ts b/src/plugins/loader.cli-metadata.test.ts index b2b0086f639..96bc2e7e20a 100644 --- a/src/plugins/loader.cli-metadata.test.ts +++ b/src/plugins/loader.cli-metadata.test.ts @@ -656,6 +656,60 @@ module.exports = { ); }); + it("sanitizes plugin CLI descriptor descriptions and rejects unsafe command names", async () => { + useNoBundledPlugins(); + const unsafeDescription = + "Open \u001B]8;;https://example.test\u0007link\u001B]8;;\u0007 now\u001B[2J"; + const plugin = writePlugin({ + id: "unsafe-cli-descriptors", + filename: "unsafe-cli-descriptors.cjs", + body: `module.exports = { + id: "unsafe-cli-descriptors", + register(api) { + api.registerCli(() => {}, { + commands: ["bad\\ncommand"], + descriptors: [ + { + name: "safe-command", + description: ${JSON.stringify(unsafeDescription)}, + hasSubcommands: false, + }, + { + name: "bad\\nname", + description: "Bad descriptor", + hasSubcommands: false, + }, + ], + }); + }, +};`, + }); + + const registry = await loadOpenClawPluginCliRegistry({ + cache: false, + config: { + plugins: { + load: { paths: [plugin.dir] }, + allow: ["unsafe-cli-descriptors"], + }, + }, + }); + + expect(registry.cliRegistrars).toHaveLength(1); + expect(registry.cliRegistrars[0]?.commands).toEqual(["safe-command"]); + expect(registry.cliRegistrars[0]?.descriptors).toEqual([ + { + name: "safe-command", + description: "Open link now", + hasSubcommands: false, + }, + ]); + expect(registry.diagnostics.map((diag) => diag.message)).toEqual([ + 'invalid cli descriptor name: "bad\\nname"', + 'invalid cli command name: "bad\\ncommand"', + ]); + }); + it("rejects async plugin registration when collecting CLI metadata", async () => { useNoBundledPlugins(); const plugin = writePlugin({ diff --git a/src/plugins/registry.ts b/src/plugins/registry.ts index 54e0ad6519d..4249463a62b 100644 --- a/src/plugins/registry.ts +++ b/src/plugins/registry.ts @@ -6,6 +6,10 @@ import { import type { AgentHarness } from "../agents/harness/types.js"; import type { AnyAgentTool } from "../agents/tools/common.js"; import type { ChannelPlugin } from "../channels/plugins/types.plugin.js"; +import { + normalizeCommandDescriptorName, + sanitizeCommandDescriptorDescription, +} from "../cli/program/command-descriptor-utils.js"; import { clearContextEnginesForOwner, registerContextEngineForOwner, @@ -1003,19 +1007,39 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { registrar: OpenClawPluginCliRegistrar, opts?: { commands?: string[]; descriptors?: OpenClawPluginCliCommandDescriptor[] }, ) => { + const normalizeCommandRoot = (raw: string, source: "command" | "descriptor") => { + const normalized = normalizeCommandDescriptorName(raw); + if (!normalized) { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: `invalid cli ${source} name: ${JSON.stringify(raw.trim())}`, + }); + } + return normalized; + }; const descriptors = (opts?.descriptors ?? []) - .map((descriptor) => ({ - name: descriptor.name.trim(), - description: descriptor.description.trim(), - hasSubcommands: descriptor.hasSubcommands, - })) - .filter((descriptor) => descriptor.name && descriptor.description); + .map((descriptor) => { + const name = normalizeCommandRoot(descriptor.name, "descriptor"); + const description = sanitizeCommandDescriptorDescription(descriptor.description); + return name && description + ? { + name, + description, + hasSubcommands: descriptor.hasSubcommands, + } + : null; + }) + .filter( + (descriptor): descriptor is OpenClawPluginCliCommandDescriptor => descriptor !== null, + ); const commands = [ ...(opts?.commands ?? []), ...descriptors.map((descriptor) => descriptor.name), ] - .map((cmd) => cmd.trim()) - .filter(Boolean); + .map((cmd) => normalizeCommandRoot(cmd, "command")) + .filter((command): command is string => command !== null); if (commands.length === 0) { pushDiagnostic({ level: "error", diff --git a/src/terminal/ansi.test.ts b/src/terminal/ansi.test.ts index 04745cc835d..71ba3d8121f 100644 --- a/src/terminal/ansi.test.ts +++ b/src/terminal/ansi.test.ts @@ -6,6 +6,7 @@ describe("terminal ansi helpers", () => { expect(stripAnsi("\u001B[31mred\u001B[0m")).toBe("red"); expect(stripAnsi("\u001B[2K\u001B[1Ared")).toBe("red"); expect(stripAnsi("\u001B]8;;https://openclaw.ai\u001B\\link\u001B]8;;\u001B\\")).toBe("link"); + expect(stripAnsi("\u001B]8;;https://openclaw.ai\u0007link\u001B]8;;\u0007")).toBe("link"); }); it("sanitizes control characters for log-safe interpolation", () => { diff --git a/src/terminal/ansi.ts b/src/terminal/ansi.ts index 3c6ffc57144..acc87ff62c1 100644 --- a/src/terminal/ansi.ts +++ b/src/terminal/ansi.ts @@ -1,7 +1,8 @@ // Full CSI: ESC [ covers cursor movement, erase, and SGR. const ANSI_CSI_PATTERN = "\\x1b\\[[\\x20-\\x3f]*[\\x40-\\x7e]"; -// OSC-8 hyperlinks: ESC ] 8 ; ; url ST ... ESC ] 8 ; ; ST -const OSC8_PATTERN = "\\x1b\\]8;;.*?\\x1b\\\\|\\x1b\\]8;;\\x1b\\\\"; +// OSC-8 hyperlinks: ESC ] 8 ; ; url ST ... ESC ] 8 ; ; ST. +// ST can be either ESC \ or BEL. +const OSC8_PATTERN = "\\x1b\\]8;;.*?(?:\\x1b\\\\|\\x07)|\\x1b\\]8;;(?:\\x1b\\\\|\\x07)"; const ANSI_CSI_REGEX = new RegExp(ANSI_CSI_PATTERN, "g"); const OSC8_REGEX = new RegExp(OSC8_PATTERN, "g");