fix: harden plugin commands (#1558) (thanks @Glucksberg)
This commit is contained in:
parent
051c92651f
commit
bfc0fb742e
@ -7,6 +7,7 @@ Docs: https://docs.clawd.bot
|
||||
### Changes
|
||||
- Browser: add node-host proxy auto-routing for remote gateways (configurable per gateway/node).
|
||||
- Plugins: add optional llm-task JSON-only tool for workflows. (#1498) Thanks @vignesh07.
|
||||
- Plugins: add LLM-free plugin slash commands and include them in `/commands`. (#1558) Thanks @Glucksberg.
|
||||
- CLI: restart the gateway by default after `clawdbot update`; add `--no-restart` to skip it.
|
||||
- CLI: add live auth probes to `clawdbot models status` for per-profile verification.
|
||||
- CLI: add `clawdbot system` for system events + heartbeat controls; remove standalone `wake`.
|
||||
|
||||
53
src/auto-reply/reply/commands-plugin.test.ts
Normal file
53
src/auto-reply/reply/commands-plugin.test.ts
Normal file
@ -0,0 +1,53 @@
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
import type { ClawdbotConfig } from "../../config/config.js";
|
||||
import { clearPluginCommands, registerPluginCommand } from "../../plugins/commands.js";
|
||||
import type { HandleCommandsParams } from "./commands-types.js";
|
||||
import { handlePluginCommand } from "./commands-plugin.js";
|
||||
|
||||
describe("handlePluginCommand", () => {
|
||||
beforeEach(() => {
|
||||
clearPluginCommands();
|
||||
});
|
||||
|
||||
it("skips plugin commands when text commands are disabled", async () => {
|
||||
registerPluginCommand("plugin-core", {
|
||||
name: "ping",
|
||||
description: "Ping",
|
||||
handler: () => ({ text: "pong" }),
|
||||
});
|
||||
|
||||
const params = {
|
||||
command: {
|
||||
commandBodyNormalized: "/ping",
|
||||
senderId: "user-1",
|
||||
channel: "test",
|
||||
isAuthorizedSender: true,
|
||||
},
|
||||
cfg: {} as ClawdbotConfig,
|
||||
} as HandleCommandsParams;
|
||||
|
||||
const result = await handlePluginCommand(params, false);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("executes plugin commands when text commands are enabled", async () => {
|
||||
registerPluginCommand("plugin-core", {
|
||||
name: "ping",
|
||||
description: "Ping",
|
||||
handler: () => ({ text: "pong" }),
|
||||
});
|
||||
|
||||
const params = {
|
||||
command: {
|
||||
commandBodyNormalized: "/ping",
|
||||
senderId: "user-1",
|
||||
channel: "test",
|
||||
isAuthorizedSender: true,
|
||||
},
|
||||
cfg: {} as ClawdbotConfig,
|
||||
} as HandleCommandsParams;
|
||||
|
||||
const result = await handlePluginCommand(params, true);
|
||||
expect(result?.reply?.text).toBe("pong");
|
||||
});
|
||||
});
|
||||
@ -15,8 +15,9 @@ import type { CommandHandler, CommandHandlerResult } from "./commands-types.js";
|
||||
*/
|
||||
export const handlePluginCommand: CommandHandler = async (
|
||||
params,
|
||||
_allowTextCommands,
|
||||
allowTextCommands,
|
||||
): Promise<CommandHandlerResult | null> => {
|
||||
if (!allowTextCommands) return null;
|
||||
const { command, cfg } = params;
|
||||
|
||||
// Try to match a plugin command
|
||||
|
||||
@ -4,9 +4,11 @@ import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { normalizeTestText } from "../../test/helpers/normalize-text.js";
|
||||
import { withTempHome } from "../../test/helpers/temp-home.js";
|
||||
import type { ClawdbotConfig } from "../config/config.js";
|
||||
import { clearPluginCommands, registerPluginCommand } from "../plugins/commands.js";
|
||||
import { buildCommandsMessage, buildHelpMessage, buildStatusMessage } from "./status.js";
|
||||
|
||||
afterEach(() => {
|
||||
clearPluginCommands();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
@ -423,6 +425,19 @@ describe("buildCommandsMessage", () => {
|
||||
);
|
||||
expect(text).toContain("/demo_skill - Demo skill");
|
||||
});
|
||||
|
||||
it("includes plugin commands when registered", () => {
|
||||
registerPluginCommand("plugin-core", {
|
||||
name: "plugstatus",
|
||||
description: "Plugin status",
|
||||
handler: () => ({ text: "ok" }),
|
||||
});
|
||||
const text = buildCommandsMessage({
|
||||
commands: { config: false, debug: false },
|
||||
} as ClawdbotConfig);
|
||||
expect(text).toContain("🔌 Plugin commands");
|
||||
expect(text).toContain("/plugstatus - Plugin status");
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildHelpMessage", () => {
|
||||
|
||||
@ -22,6 +22,7 @@ import {
|
||||
} from "../utils/usage-format.js";
|
||||
import { VERSION } from "../version.js";
|
||||
import { listChatCommands, listChatCommandsForConfig } from "./commands-registry.js";
|
||||
import { listPluginCommands } from "../plugins/commands.js";
|
||||
import type { SkillCommandSpec } from "../agents/skills.js";
|
||||
import type { ElevatedLevel, ReasoningLevel, ThinkLevel, VerboseLevel } from "./thinking.js";
|
||||
import type { MediaUnderstandingDecision } from "../media-understanding/types.js";
|
||||
@ -442,5 +443,12 @@ export function buildCommandsMessage(
|
||||
const scopeLabel = command.scope === "text" ? " (text-only)" : "";
|
||||
lines.push(`${primary}${aliasLabel}${scopeLabel} - ${command.description}`);
|
||||
}
|
||||
const pluginCommands = listPluginCommands();
|
||||
if (pluginCommands.length > 0) {
|
||||
lines.push("🔌 Plugin commands");
|
||||
for (const command of pluginCommands) {
|
||||
lines.push(`/${command.name} - ${command.description}`);
|
||||
}
|
||||
}
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
63
src/plugins/commands.test.ts
Normal file
63
src/plugins/commands.test.ts
Normal file
@ -0,0 +1,63 @@
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
import type { ClawdbotConfig } from "../config/config.js";
|
||||
import {
|
||||
clearPluginCommands,
|
||||
executePluginCommand,
|
||||
matchPluginCommand,
|
||||
registerPluginCommand,
|
||||
validateCommandName,
|
||||
} from "./commands.js";
|
||||
|
||||
describe("validateCommandName", () => {
|
||||
it("rejects reserved aliases from built-in commands", () => {
|
||||
const error = validateCommandName("id");
|
||||
expect(error).toContain("reserved");
|
||||
});
|
||||
});
|
||||
|
||||
describe("plugin command registry", () => {
|
||||
beforeEach(() => {
|
||||
clearPluginCommands();
|
||||
});
|
||||
|
||||
it("normalizes command names for registration and matching", () => {
|
||||
const result = registerPluginCommand("plugin-core", {
|
||||
name: " ping ",
|
||||
description: "Ping",
|
||||
handler: () => ({ text: "pong" }),
|
||||
});
|
||||
expect(result.ok).toBe(true);
|
||||
|
||||
const match = matchPluginCommand("/ping");
|
||||
expect(match?.command.name).toBe("ping");
|
||||
});
|
||||
|
||||
it("blocks registration while a command is executing", async () => {
|
||||
let nestedResult: { ok: boolean; error?: string } | undefined;
|
||||
|
||||
registerPluginCommand("plugin-core", {
|
||||
name: "outer",
|
||||
description: "Outer",
|
||||
handler: () => {
|
||||
nestedResult = registerPluginCommand("plugin-inner", {
|
||||
name: "inner",
|
||||
description: "Inner",
|
||||
handler: () => ({ text: "ok" }),
|
||||
});
|
||||
return { text: "done" };
|
||||
},
|
||||
});
|
||||
|
||||
await executePluginCommand({
|
||||
command: matchPluginCommand("/outer")!.command,
|
||||
senderId: "user-1",
|
||||
channel: "test",
|
||||
isAuthorizedSender: true,
|
||||
commandBody: "/outer",
|
||||
config: {} as ClawdbotConfig,
|
||||
});
|
||||
|
||||
expect(nestedResult?.ok).toBe(false);
|
||||
expect(nestedResult?.error).toContain("processing is in progress");
|
||||
});
|
||||
});
|
||||
@ -6,6 +6,7 @@
|
||||
*/
|
||||
|
||||
import type { ClawdbotConfig } from "../config/config.js";
|
||||
import { listChatCommands } from "../auto-reply/commands-registry.js";
|
||||
import type { ClawdbotPluginCommandDefinition, PluginCommandContext } from "./types.js";
|
||||
import { logVerbose } from "../globals.js";
|
||||
|
||||
@ -16,53 +17,33 @@ type RegisteredPluginCommand = ClawdbotPluginCommandDefinition & {
|
||||
// Registry of plugin commands
|
||||
const pluginCommands: Map<string, RegisteredPluginCommand> = new Map();
|
||||
|
||||
// Lock to prevent modifications during command execution
|
||||
let registryLocked = false;
|
||||
// Lock counter to prevent modifications during command execution
|
||||
let registryLockCount = 0;
|
||||
|
||||
// Maximum allowed length for command arguments (defense in depth)
|
||||
const MAX_ARGS_LENGTH = 4096;
|
||||
|
||||
/**
|
||||
* Reserved command names that plugins cannot override.
|
||||
* These are built-in commands from commands-registry.data.ts.
|
||||
*/
|
||||
const RESERVED_COMMANDS = new Set([
|
||||
// Core commands
|
||||
"help",
|
||||
"commands",
|
||||
"status",
|
||||
"whoami",
|
||||
"context",
|
||||
// Session management
|
||||
"stop",
|
||||
"restart",
|
||||
"reset",
|
||||
"new",
|
||||
"compact",
|
||||
// Configuration
|
||||
"config",
|
||||
"debug",
|
||||
"allowlist",
|
||||
"activation",
|
||||
// Agent control
|
||||
"skill",
|
||||
"subagents",
|
||||
"model",
|
||||
"models",
|
||||
"queue",
|
||||
// Messaging
|
||||
"send",
|
||||
// Execution
|
||||
"bash",
|
||||
"exec",
|
||||
// Mode toggles
|
||||
"think",
|
||||
"verbose",
|
||||
"reasoning",
|
||||
"elevated",
|
||||
// Billing
|
||||
"usage",
|
||||
]);
|
||||
let cachedReservedCommands: Set<string> | null = null;
|
||||
|
||||
function getReservedCommands(): Set<string> {
|
||||
if (cachedReservedCommands) return cachedReservedCommands;
|
||||
const reserved = new Set<string>();
|
||||
for (const command of listChatCommands()) {
|
||||
if (command.nativeName) {
|
||||
const normalized = command.nativeName.trim().toLowerCase();
|
||||
if (normalized) reserved.add(normalized);
|
||||
}
|
||||
for (const alias of command.textAliases ?? []) {
|
||||
const trimmed = alias.trim();
|
||||
if (!trimmed) continue;
|
||||
const withoutSlash = trimmed.startsWith("/") ? trimmed.slice(1) : trimmed;
|
||||
const normalized = withoutSlash.trim().toLowerCase();
|
||||
if (normalized) reserved.add(normalized);
|
||||
}
|
||||
}
|
||||
cachedReservedCommands = reserved;
|
||||
return reserved;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a command name.
|
||||
@ -82,7 +63,7 @@ export function validateCommandName(name: string): string | null {
|
||||
}
|
||||
|
||||
// Check reserved commands
|
||||
if (RESERVED_COMMANDS.has(trimmed)) {
|
||||
if (getReservedCommands().has(trimmed)) {
|
||||
return `Command name "${trimmed}" is reserved by a built-in command`;
|
||||
}
|
||||
|
||||
@ -103,7 +84,7 @@ export function registerPluginCommand(
|
||||
command: ClawdbotPluginCommandDefinition,
|
||||
): CommandRegistrationResult {
|
||||
// Prevent registration while commands are being processed
|
||||
if (registryLocked) {
|
||||
if (registryLockCount > 0) {
|
||||
return { ok: false, error: "Cannot register commands while processing is in progress" };
|
||||
}
|
||||
|
||||
@ -117,7 +98,8 @@ export function registerPluginCommand(
|
||||
return { ok: false, error: validationError };
|
||||
}
|
||||
|
||||
const key = `/${command.name.toLowerCase()}`;
|
||||
const normalizedName = command.name.trim();
|
||||
const key = `/${normalizedName.toLowerCase()}`;
|
||||
|
||||
// Check for duplicate registration
|
||||
if (pluginCommands.has(key)) {
|
||||
@ -128,7 +110,7 @@ export function registerPluginCommand(
|
||||
};
|
||||
}
|
||||
|
||||
pluginCommands.set(key, { ...command, pluginId });
|
||||
pluginCommands.set(key, { ...command, name: normalizedName, pluginId });
|
||||
logVerbose(`Registered plugin command: ${key} (plugin: ${pluginId})`);
|
||||
return { ok: true };
|
||||
}
|
||||
@ -139,6 +121,7 @@ export function registerPluginCommand(
|
||||
*/
|
||||
export function clearPluginCommands(): void {
|
||||
pluginCommands.clear();
|
||||
registryLockCount = 0;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -190,12 +173,28 @@ function sanitizeArgs(args: string | undefined): string | undefined {
|
||||
if (!args) return undefined;
|
||||
|
||||
// Enforce length limit
|
||||
if (args.length > MAX_ARGS_LENGTH) {
|
||||
return args.slice(0, MAX_ARGS_LENGTH);
|
||||
}
|
||||
const trimmed = args.length > MAX_ARGS_LENGTH ? args.slice(0, MAX_ARGS_LENGTH) : args;
|
||||
|
||||
// Remove control characters (except newlines and tabs which may be intentional)
|
||||
return args.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, "");
|
||||
let needsSanitize = false;
|
||||
for (let i = 0; i < trimmed.length; i += 1) {
|
||||
const code = trimmed.charCodeAt(i);
|
||||
if (code === 0x09 || code === 0x0a) continue;
|
||||
if (code < 0x20 || code === 0x7f) {
|
||||
needsSanitize = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!needsSanitize) return trimmed;
|
||||
|
||||
let sanitized = "";
|
||||
for (let i = 0; i < trimmed.length; i += 1) {
|
||||
const code = trimmed.charCodeAt(i);
|
||||
if (code === 0x09 || code === 0x0a || (code >= 0x20 && code !== 0x7f)) {
|
||||
sanitized += trimmed[i];
|
||||
}
|
||||
}
|
||||
return sanitized;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -237,7 +236,7 @@ export async function executePluginCommand(params: {
|
||||
};
|
||||
|
||||
// Lock registry during execution to prevent concurrent modifications
|
||||
registryLocked = true;
|
||||
registryLockCount += 1;
|
||||
try {
|
||||
const result = await command.handler(ctx);
|
||||
logVerbose(
|
||||
@ -250,7 +249,7 @@ export async function executePluginCommand(params: {
|
||||
// Don't leak internal error details - return a safe generic message
|
||||
return { text: "⚠️ Command failed. Please try again later." };
|
||||
} finally {
|
||||
registryLocked = false;
|
||||
registryLockCount = Math.max(0, registryLockCount - 1);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -376,7 +376,8 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
|
||||
}
|
||||
|
||||
// Register with the plugin command system (validates name and checks for duplicates)
|
||||
const result = registerPluginCommand(record.id, command);
|
||||
const normalizedCommand = { ...command, name };
|
||||
const result = registerPluginCommand(record.id, normalizedCommand);
|
||||
if (!result.ok) {
|
||||
pushDiagnostic({
|
||||
level: "error",
|
||||
@ -390,7 +391,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
|
||||
record.commands.push(name);
|
||||
registry.commands.push({
|
||||
pluginId: record.id,
|
||||
command,
|
||||
command: normalizedCommand,
|
||||
source: record.source,
|
||||
});
|
||||
};
|
||||
|
||||
Loading…
Reference in New Issue
Block a user