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<string, string> = {}` 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 <noreply@anthropic.com>
2026-03-01 14:35:31 -08:00
parent a3701e220d
commit 4b60bcaf6c
3 changed files with 233 additions and 24 deletions
--- 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
 }
--- 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<string, string> = {};

 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/")
 }

--- a/tests/openclaw-converter.test.ts
+++ 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<string, unknown>)
+    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<string, unknown>)
+    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<string, string> = {}")
+  })
+
+  test("plugin without MCP servers has no openclawConfig", () => {
+    const plugin: ClaudePlugin = { ...fixturePlugin, mcpServers: undefined }
+    const bundle = convertClaudeToOpenClaw(plugin, defaultOptions)
+    expect(bundle.openclawConfig).toBeUndefined()
+  })
+})