From 4b60bcaf6cb322d9efcd7c658356852c48257c95 Mon Sep 17 00:00:00 2001 From: Kieran Klaassen Date: Sun, 1 Mar 2026 14:35:31 -0800 Subject: [PATCH] fix: Address review findings in OpenClaw converter - Fix P1: Replace incomplete string escaping in generateEntryPoint with JSON.stringify() to prevent code injection via command names/descriptions with backslashes, newlines, or other special characters - Fix P1: Remove hardcoded 'compound-engineering' output path; resolve from plugin.manifest.name via new openclawHome + pluginName params - Fix P2: Add --openclaw-home CLI flag (default: ~/.openclaw/extensions) consistent with --codex-home and --pi-home patterns - Fix P2: Emit typed `const skills: Record = {}` in generated TypeScript to prevent downstream type errors - Fix P3: Add lookbehind guards to rewritePaths() matching kiro pattern - Fix P3: Extract duplicated disableModelInvocation filter to variable - Fix P3: Build manifest skills list before constructing manifest object (no post-construction mutation) - Export ClaudeToOpenClawOptions type alias for interface clarity - Add openclaw-converter.test.ts with 13 tests covering all scenarios Co-Authored-By: Claude --- src/commands/install.ts | 14 +- src/converters/claude-to-openclaw.ts | 43 +++--- tests/openclaw-converter.test.ts | 200 +++++++++++++++++++++++++++ 3 files changed, 233 insertions(+), 24 deletions(-) create mode 100644 tests/openclaw-converter.test.ts diff --git a/src/commands/install.ts b/src/commands/install.ts index b5522b8..58a471c 100644 --- a/src/commands/install.ts +++ b/src/commands/install.ts @@ -42,6 +42,11 @@ export default defineCommand({ alias: "pi-home", description: "Write Pi output to this Pi root (ex: ~/.pi/agent or ./.pi)", }, + openclawHome: { + type: "string", + alias: "openclaw-home", + description: "Write OpenClaw output to this extensions root (ex: ~/.openclaw/extensions)", + }, also: { type: "string", description: "Comma-separated extra targets to generate (ex: codex)", @@ -84,6 +89,7 @@ export default defineCommand({ const outputRoot = resolveOutputRoot(args.output) const codexHome = resolveTargetHome(args.codexHome, path.join(os.homedir(), ".codex")) const piHome = resolveTargetHome(args.piHome, path.join(os.homedir(), ".pi", "agent")) + const openclawHome = resolveTargetHome(args.openclawHome, path.join(os.homedir(), ".openclaw", "extensions")) const options = { agentMode: String(args.agentMode) === "primary" ? "primary" : "subagent", @@ -96,7 +102,7 @@ export default defineCommand({ throw new Error(`Target ${targetName} did not return a bundle.`) } const hasExplicitOutput = Boolean(args.output && String(args.output).trim()) - const primaryOutputRoot = resolveTargetOutputRoot(targetName, outputRoot, codexHome, piHome, hasExplicitOutput) + const primaryOutputRoot = resolveTargetOutputRoot(targetName, outputRoot, codexHome, piHome, openclawHome, plugin.manifest.name, hasExplicitOutput) await target.write(primaryOutputRoot, bundle) console.log(`Installed ${plugin.manifest.name} to ${primaryOutputRoot}`) @@ -117,7 +123,7 @@ export default defineCommand({ console.warn(`Skipping ${extra}: no output returned.`) continue } - const extraRoot = resolveTargetOutputRoot(extra, path.join(outputRoot, extra), codexHome, piHome, hasExplicitOutput) + const extraRoot = resolveTargetOutputRoot(extra, path.join(outputRoot, extra), codexHome, piHome, openclawHome, plugin.manifest.name, hasExplicitOutput) await handler.write(extraRoot, extraBundle) console.log(`Installed ${plugin.manifest.name} to ${extraRoot}`) } @@ -174,6 +180,8 @@ function resolveTargetOutputRoot( outputRoot: string, codexHome: string, piHome: string, + openclawHome: string, + pluginName: string, hasExplicitOutput: boolean, ): string { if (targetName === "codex") return codexHome @@ -196,7 +204,7 @@ function resolveTargetOutputRoot( return path.join(base, ".kiro") } if (targetName === "openclaw") { - return path.join(os.homedir(), ".openclaw", "extensions", "compound-engineering") + return path.join(openclawHome, pluginName) } return outputRoot } diff --git a/src/converters/claude-to-openclaw.ts b/src/converters/claude-to-openclaw.ts index 71fdce3..83a0192 100644 --- a/src/converters/claude-to-openclaw.ts +++ b/src/converters/claude-to-openclaw.ts @@ -13,21 +13,17 @@ import type { } from "../types/openclaw" import type { ClaudeToOpenCodeOptions } from "./claude-to-opencode" +export type ClaudeToOpenClawOptions = ClaudeToOpenCodeOptions + export function convertClaudeToOpenClaw( plugin: ClaudePlugin, - _options: ClaudeToOpenCodeOptions, + _options: ClaudeToOpenClawOptions, ): OpenClawBundle { - const manifest = buildManifest(plugin) - const packageJson = buildPackageJson(plugin) + const enabledCommands = plugin.commands.filter((cmd) => !cmd.disableModelInvocation) const agentSkills = plugin.agents.map(convertAgentToSkill) - const commandSkills = plugin.commands - .filter((cmd) => !cmd.disableModelInvocation) - .map(convertCommandToSkill) - - const commands = plugin.commands - .filter((cmd) => !cmd.disableModelInvocation) - .map(convertCommand) + const commandSkills = enabledCommands.map(convertCommandToSkill) + const commands = enabledCommands.map(convertCommand) const skills: OpenClawSkillFile[] = [...agentSkills, ...commandSkills] @@ -36,13 +32,15 @@ export function convertClaudeToOpenClaw( name: skill.name, })) - // Add original skill names to manifest.skills const allSkillDirs = [ ...agentSkills.map((s) => s.dir), ...commandSkills.map((s) => s.dir), ...plugin.skills.map((s) => s.name), ] - manifest.skills = allSkillDirs.map((dir) => `skills/${dir}`) + + const manifest = buildManifest(plugin, allSkillDirs) + + const packageJson = buildPackageJson(plugin) const openclawConfig = plugin.mcpServers ? buildOpenClawConfig(plugin.mcpServers) @@ -61,11 +59,12 @@ export function convertClaudeToOpenClaw( } } -function buildManifest(plugin: ClaudePlugin): OpenClawPluginManifest { +function buildManifest(plugin: ClaudePlugin, skillDirs: string[]): OpenClawPluginManifest { return { id: plugin.manifest.name, name: formatDisplayName(plugin.manifest.name), kind: "tool", + skills: skillDirs.map((dir) => `skills/${dir}`), } } @@ -170,15 +169,17 @@ function buildOpenClawConfig( function generateEntryPoint(commands: OpenClawCommandRegistration[]): string { const commandRegistrations = commands .map((cmd) => { - const escapedName = cmd.name.replace(/"/g, '\\"') - const escapedDesc = (cmd.description ?? "").replace(/"/g, '\\"') + // JSON.stringify produces a fully-escaped string literal safe for JS/TS source embedding + const safeName = JSON.stringify(cmd.name) + const safeDesc = JSON.stringify(cmd.description ?? "") + const safeNotFound = JSON.stringify(`Command ${cmd.name} not found. Check skills directory.`) return ` api.registerCommand({ - name: "${escapedName}", - description: "${escapedDesc}", + name: ${safeName}, + description: ${safeDesc}, acceptsArgs: ${cmd.acceptsArgs}, requireAuth: false, handler: (ctx) => ({ - text: skills["${escapedName}"] ?? "Command ${escapedName} not found. Check skills directory.", + text: skills[${safeName}] ?? ${safeNotFound}, }), });` }) @@ -193,7 +194,7 @@ import { fileURLToPath } from "url"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); // Pre-load skill bodies for command responses -const skills = {}; +const skills: Record = {}; async function loadSkills() { const skillsDir = path.join(__dirname, "skills"); @@ -226,8 +227,8 @@ ${commandRegistrations} function rewritePaths(body: string): string { return body - .replace(/~\/\.claude\//g, "~/.openclaw/") - .replace(/\.claude\//g, ".openclaw/") + .replace(/(?<=^|\s|["'`])~\/\.claude\//gm, "~/.openclaw/") + .replace(/(?<=^|\s|["'`])\.claude\//gm, ".openclaw/") .replace(/\.claude-plugin\//g, "openclaw-plugin/") } diff --git a/tests/openclaw-converter.test.ts b/tests/openclaw-converter.test.ts new file mode 100644 index 0000000..7cde0ae --- /dev/null +++ b/tests/openclaw-converter.test.ts @@ -0,0 +1,200 @@ +import { describe, expect, test } from "bun:test" +import { convertClaudeToOpenClaw } from "../src/converters/claude-to-openclaw" +import { parseFrontmatter } from "../src/utils/frontmatter" +import type { ClaudePlugin } from "../src/types/claude" + +const fixturePlugin: ClaudePlugin = { + root: "/tmp/plugin", + manifest: { name: "compound-engineering", version: "1.0.0", description: "A plugin" }, + agents: [ + { + name: "security-reviewer", + description: "Security-focused agent", + capabilities: ["Threat modeling", "OWASP"], + model: "claude-sonnet-4-20250514", + body: "Focus on vulnerabilities in ~/.claude/settings.", + sourcePath: "/tmp/plugin/agents/security-reviewer.md", + }, + ], + commands: [ + { + name: "workflows:plan", + description: "Planning command", + argumentHint: "[FOCUS]", + model: "inherit", + allowedTools: ["Read"], + body: "Plan the work. See ~/.claude/settings for config.", + sourcePath: "/tmp/plugin/commands/workflows/plan.md", + }, + { + name: "disabled-cmd", + description: "Disabled command", + model: "inherit", + allowedTools: [], + body: "Should be excluded.", + disableModelInvocation: true, + sourcePath: "/tmp/plugin/commands/disabled-cmd.md", + }, + ], + skills: [ + { + name: "existing-skill", + description: "Existing skill", + sourceDir: "/tmp/plugin/skills/existing-skill", + skillPath: "/tmp/plugin/skills/existing-skill/SKILL.md", + }, + ], + hooks: undefined, + mcpServers: { + local: { command: "npx", args: ["-y", "some-mcp-server"] }, + remote: { url: "https://mcp.example.com/api", headers: { Authorization: "Bearer token" } }, + }, +} + +const defaultOptions = { + agentMode: "subagent" as const, + inferTemperature: false, + permissions: "none" as const, +} + +describe("convertClaudeToOpenClaw", () => { + test("converts agents to skill files with SKILL.md content", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + const skill = bundle.skills.find((s) => s.name === "security-reviewer") + expect(skill).toBeDefined() + expect(skill!.dir).toBe("agent-security-reviewer") + const parsed = parseFrontmatter(skill!.content) + expect(parsed.data.name).toBe("security-reviewer") + expect(parsed.data.description).toBe("Security-focused agent") + expect(parsed.data.model).toBe("claude-sonnet-4-20250514") + expect(parsed.body).toContain("Focus on vulnerabilities") + }) + + test("converts commands to skill files (excluding disableModelInvocation)", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + const cmdSkill = bundle.skills.find((s) => s.name === "workflows:plan") + expect(cmdSkill).toBeDefined() + expect(cmdSkill!.dir).toBe("cmd-workflows:plan") + + const disabledSkill = bundle.skills.find((s) => s.name === "disabled-cmd") + expect(disabledSkill).toBeUndefined() + }) + + test("commands list excludes disableModelInvocation commands", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + const cmd = bundle.commands.find((c) => c.name === "workflows-plan") + expect(cmd).toBeDefined() + expect(cmd!.description).toBe("Planning command") + expect(cmd!.acceptsArgs).toBe(true) + + const disabled = bundle.commands.find((c) => c.name === "disabled-cmd") + expect(disabled).toBeUndefined() + }) + + test("command colons are replaced with dashes in command registrations", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + const cmd = bundle.commands.find((c) => c.name === "workflows-plan") + expect(cmd).toBeDefined() + expect(cmd!.name).not.toContain(":") + }) + + test("manifest includes plugin id, display name, and skills list", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + expect(bundle.manifest.id).toBe("compound-engineering") + expect(bundle.manifest.name).toBe("Compound Engineering") + expect(bundle.manifest.kind).toBe("tool") + expect(bundle.manifest.skills).toContain("skills/agent-security-reviewer") + expect(bundle.manifest.skills).toContain("skills/cmd-workflows:plan") + expect(bundle.manifest.skills).toContain("skills/existing-skill") + }) + + test("package.json uses plugin name and version", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + expect(bundle.packageJson.name).toBe("openclaw-compound-engineering") + expect(bundle.packageJson.version).toBe("1.0.0") + expect(bundle.packageJson.type).toBe("module") + }) + + test("skillDirCopies includes original skill directories", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + const copy = bundle.skillDirCopies.find((s) => s.name === "existing-skill") + expect(copy).toBeDefined() + expect(copy!.sourceDir).toBe("/tmp/plugin/skills/existing-skill") + }) + + test("stdio MCP servers included in openclaw config", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + expect(bundle.openclawConfig).toBeDefined() + const mcp = (bundle.openclawConfig!.mcpServers as Record) + expect(mcp.local).toBeDefined() + expect((mcp.local as any).type).toBe("stdio") + expect((mcp.local as any).command).toBe("npx") + }) + + test("HTTP MCP servers included as http type in openclaw config", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + const mcp = (bundle.openclawConfig!.mcpServers as Record) + expect(mcp.remote).toBeDefined() + expect((mcp.remote as any).type).toBe("http") + expect((mcp.remote as any).url).toBe("https://mcp.example.com/api") + }) + + test("paths are rewritten from .claude/ to .openclaw/ in skill content", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + + const agentSkill = bundle.skills.find((s) => s.name === "security-reviewer") + expect(agentSkill!.content).toContain("~/.openclaw/settings") + expect(agentSkill!.content).not.toContain("~/.claude/settings") + + const cmdSkill = bundle.skills.find((s) => s.name === "workflows:plan") + expect(cmdSkill!.content).toContain("~/.openclaw/settings") + expect(cmdSkill!.content).not.toContain("~/.claude/settings") + }) + + test("generateEntryPoint uses JSON.stringify for safe string escaping", () => { + const plugin: ClaudePlugin = { + ...fixturePlugin, + commands: [ + { + name: "tricky-cmd", + description: 'Has "quotes" and \\backslashes\\ and\nnewlines', + model: "inherit", + allowedTools: [], + body: "body", + sourcePath: "/tmp/cmd.md", + }, + ], + } + const bundle = convertClaudeToOpenClaw(plugin, defaultOptions) + + // Entry point must be valid JS/TS — JSON.stringify handles all special chars + expect(bundle.entryPoint).toContain('"tricky-cmd"') + expect(bundle.entryPoint).toContain('\\"quotes\\"') + expect(bundle.entryPoint).toContain("\\\\backslashes\\\\") + expect(bundle.entryPoint).toContain("\\n") + // No raw unescaped newline inside a string literal + const lines = bundle.entryPoint.split("\n") + const nameLine = lines.find((l) => l.includes("tricky-cmd") && l.includes("name:")) + expect(nameLine).toBeDefined() + }) + + test("generateEntryPoint emits typed skills record", () => { + const bundle = convertClaudeToOpenClaw(fixturePlugin, defaultOptions) + expect(bundle.entryPoint).toContain("const skills: Record = {}") + }) + + test("plugin without MCP servers has no openclawConfig", () => { + const plugin: ClaudePlugin = { ...fixturePlugin, mcpServers: undefined } + const bundle = convertClaudeToOpenClaw(plugin, defaultOptions) + expect(bundle.openclawConfig).toBeUndefined() + }) +})