mirror of
https://fastgit.cc/github.com/openclaw/openclaw
synced 2026-04-30 22:12:32 +08:00
fix(cli): sanitize plugin command descriptors
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<NamedCommandDescriptor, "name" | "description">;
|
||||
|
||||
const SAFE_COMMAND_NAME_PATTERN = /^[A-Za-z0-9][A-Za-z0-9_-]*$/;
|
||||
|
||||
export type CommandDescriptorCatalog<TDescriptor extends NamedCommandDescriptor> = {
|
||||
descriptors: readonly TDescriptor[];
|
||||
getDescriptors: () => readonly TDescriptor[];
|
||||
@@ -10,6 +13,23 @@ export type CommandDescriptorCatalog<TDescriptor extends NamedCommandDescriptor>
|
||||
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<string> = new Set(),
|
||||
): Set<string> {
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
// Full CSI: ESC [ <params> <final byte> 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");
|
||||
|
||||
Reference in New Issue
Block a user