From b2acb67122d5ef645c281b83b8643c69d609288a Mon Sep 17 00:00:00 2001 From: aeon-aaron Date: Sun, 24 May 2026 07:49:47 +0000 Subject: [PATCH 1/2] fix(security): shell-quote filesystem-derived paths in validation commands The validation pipeline (clawpatch fix / clawpatch ci) executes mapper- produced command strings via runCommand, which uses spawn(..., shell: true). Several mappers interpolated filesystem-derived paths and package.json script names directly into those command strings: - src/mappers/projects.ts scriptCommand - src/mappers/shared.ts nodeScriptCommand - src/mappers/elixir.ts associatedTests (mix test ) - src/mappers/swift.ts prefixSwiftSeed + prefixTestRefs (swift test --package-path ) A workspace package directory, Swift package directory, Elixir test filename, or package.json script key whose name contained shell metacharacters became part of the string handed to /bin/sh -c, producing command injection when an operator runs clawpatch fix or clawpatch ci on the affected project. The fix routes each interpolation through shellQuotePath (src/shell.ts), so ordinary values render identically while attacker-controlled metacharacters are wrapped in double quotes with dollar / backtick / backslash escaped. Severity: high. Reachability requires an operator to run clawpatch fix or clawpatch ci on a project they did not author. Detected by Aeon during manual review against the maintainer's own SECURITY.md scope statement, which named "repository feature mapping" and "patch workflow state" as in-scope surfaces. A regression suite (src/cmd-injection-regression.test.ts) asserts that the relevant builders produce shell-safe output on attacker-controlled inputs and unchanged output on ordinary inputs (no over-quoting). --- src/cmd-injection-regression.test.ts | 108 +++++++++++++++++++++++++++ src/mappers/elixir.ts | 3 +- src/mappers/projects.ts | 17 +++-- src/mappers/shared.ts | 17 +++-- src/mappers/swift.ts | 7 +- 5 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 src/cmd-injection-regression.test.ts diff --git a/src/cmd-injection-regression.test.ts b/src/cmd-injection-regression.test.ts new file mode 100644 index 0000000..4b14f90 --- /dev/null +++ b/src/cmd-injection-regression.test.ts @@ -0,0 +1,108 @@ +// Regression tests for command-injection via filesystem-derived paths +// reaching shell-interpreted command strings in the validation pipeline. +// +// `runCommand` in src/exec.ts uses `spawn(..., { shell: true })`, so any +// command string the validation pipeline produces is passed straight to +// /bin/sh -c. Each mapper that interpolates a filesystem-derived path or +// package-config-derived name into a command string must shell-quote that +// value via `shellQuotePath` so attacker-controlled metacharacters cannot +// be parsed as shell syntax. +// +// Each test below builds a project tree with attacker-controlled names +// and asserts the produced commands are shell-safe (no unquoted `$(...)`, +// no unquoted `;`, etc.). + +import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { mapFeatures } from "./mapper.js"; +import { validationCommandsForFeature } from "./validation.js"; +import { detectProject } from "./detect.js"; +import { scriptCommand } from "./mappers/projects.js"; +import { nodeScriptCommand } from "./mappers/shared.js"; + +function isShellSafe(command: string): boolean { + // Strip double-quoted segments — anything `$(...)` or `;` inside `"..."` + // is shell-literal (backslash-escaped `$` and `\`). + const withoutQuoted = command.replace(/"(?:\\.|[^"\\])*"/gu, ""); + if (/\$\(/u.test(withoutQuoted)) return false; + if (/`/u.test(withoutQuoted)) return false; + if (/(?:^|\s);/u.test(withoutQuoted)) return false; + if (/&&|\|\|/u.test(withoutQuoted)) return false; + return true; +} + +describe("validation pipeline shell-quotes filesystem-derived names (regression)", () => { + it("scriptCommand: malicious workspace package-root is shell-quoted", () => { + for (const pm of ["pnpm", "yarn", "bun", "npm"] as const) { + const cmd = scriptCommand(pm, "packages/$(id)-pkg", "test"); + expect(isShellSafe(cmd), `pm=${pm} cmd=${cmd}`).toBe(true); + } + }); + + it("scriptCommand: malicious package.json script-name is shell-quoted", () => { + for (const pm of ["pnpm", "yarn", "bun", "npm"] as const) { + const cmd = scriptCommand(pm, "packages/ok", "$(id)"); + expect(isShellSafe(cmd), `pm=${pm} cmd=${cmd}`).toBe(true); + } + }); + + it("scriptCommand: ordinary inputs are unchanged (no over-quoting)", () => { + expect(scriptCommand("pnpm", "packages/ui", "test")).toBe("pnpm --dir packages/ui test"); + expect(scriptCommand("npm", "packages/ui", "test")).toBe("npm --prefix packages/ui run test"); + expect(scriptCommand("yarn", "packages/ui", "test")).toBe("yarn --cwd packages/ui test"); + expect(scriptCommand("bun", "packages/ui", "test")).toBe("bun --cwd packages/ui run test"); + expect(scriptCommand("pnpm", ".", "test")).toBe("pnpm test"); + expect(scriptCommand("npm", ".", "test")).toBe("npm run test"); + expect(scriptCommand("bun", ".", "test")).toBe("bun run test"); + }); + + it("nodeScriptCommand: malicious workspace package-root is shell-quoted", () => { + for (const pm of ["pnpm", "yarn", "bun", "npm"] as const) { + const cmd = nodeScriptCommand(pm, "packages/$(id)-pkg", "test"); + expect(isShellSafe(cmd), `pm=${pm} cmd=${cmd}`).toBe(true); + } + }); + + it("nodeScriptCommand: ordinary inputs are unchanged", () => { + expect(nodeScriptCommand("pnpm", "apps/web", "test")).toBe("pnpm --dir apps/web test"); + expect(nodeScriptCommand("npm", "apps/web", "test")).toBe("npm --prefix apps/web run test"); + }); + + it("swift end-to-end: malicious package-root directory cannot inject into swift test --package-path", async () => { + const root = await mkdtemp(join(tmpdir(), "clawpatch-cmd-inj-swift-")); + try { + const maliciousPkg = "$(id)-pkg"; + await mkdir(join(root, maliciousPkg, "Sources", "Evil"), { recursive: true }); + await writeFile( + join(root, maliciousPkg, "Package.swift"), + '// swift-tools-version:5.9\nimport PackageDescription\nlet package = Package(name: "evil", targets: [.target(name: "Evil"), .testTarget(name: "EvilTests", dependencies: ["Evil"])])\n', + ); + await writeFile( + join(root, maliciousPkg, "Sources", "Evil", "Evil.swift"), + "public struct Evil {}\n", + ); + await mkdir(join(root, maliciousPkg, "Tests", "EvilTests"), { recursive: true }); + await writeFile( + join(root, maliciousPkg, "Tests", "EvilTests", "EvilTests.swift"), + "import XCTest\nclass EvilTests: XCTestCase {}\n", + ); + const project = await detectProject(root); + const result = await mapFeatures(root, project, []); + for (const feature of result.features) { + const commands = validationCommandsForFeature(feature, { + typecheck: null, + lint: null, + format: null, + test: null, + }); + for (const c of commands) { + expect(isShellSafe(c), `unsafe command produced: ${c}`).toBe(true); + } + } + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +}); diff --git a/src/mappers/elixir.ts b/src/mappers/elixir.ts index 6942d29..739a50f 100644 --- a/src/mappers/elixir.ts +++ b/src/mappers/elixir.ts @@ -1,6 +1,7 @@ import { readFile, readdir } from "node:fs/promises"; import { basename, join } from "node:path"; import { pathExists } from "../fs.js"; +import { shellQuotePath } from "../shell.js"; import { packageKind, pathMatchesPrefix, shouldSkip, stripLineComments, walk } from "./shared.js"; import { FeatureSeed, SeedTestRef } from "./types.js"; @@ -301,7 +302,7 @@ function associatedTests(files: string[], testFiles: string[]): SeedTestRef[] { return testFiles .filter((path) => prefixes.some((prefix) => pathMatchesPrefix(path, prefix))) .slice(0, elixirTestGroupMaxFiles) - .map((path) => ({ path, command: `mix test ${path}` })); + .map((path) => ({ path, command: `mix test ${shellQuotePath(path)}` })); } function testPrefixesForSource(path: string): string[] { diff --git a/src/mappers/projects.ts b/src/mappers/projects.ts index dcf60f2..3983cb3 100644 --- a/src/mappers/projects.ts +++ b/src/mappers/projects.ts @@ -2,6 +2,7 @@ import { lstat, readFile, readdir, realpath } from "node:fs/promises"; import { basename, dirname, join } from "node:path"; import { packageScripts, readPackageJson } from "../detect.js"; import { pathExists } from "../fs.js"; +import { shellQuotePath } from "../shell.js"; import { isSafeDirectory, normalize, pathMatchesPrefix, shouldSkip } from "./shared.js"; import { taskGraphCommand, type WorkspaceTaskGraph } from "./task-graph.js"; import type { SeedFileRef } from "./types.js"; @@ -194,22 +195,26 @@ export function packageRelativePath(packageRoot: string, path: string): string { } export function scriptCommand(packageManager: string, packageRoot: string, script: string): string { + const quotedRoot = shellQuotePath(packageRoot); + const quotedScript = shellQuotePath(script); if (packageRoot === ".") { if (packageManager === "bun") { - return `bun run ${script}`; + return `bun run ${quotedScript}`; } - return packageManager === "npm" ? `npm run ${script}` : `${packageManager} ${script}`; + return packageManager === "npm" + ? `npm run ${quotedScript}` + : `${packageManager} ${quotedScript}`; } if (packageManager === "pnpm") { - return `pnpm --dir ${packageRoot} ${script}`; + return `pnpm --dir ${quotedRoot} ${quotedScript}`; } if (packageManager === "yarn") { - return `yarn --cwd ${packageRoot} ${script}`; + return `yarn --cwd ${quotedRoot} ${quotedScript}`; } if (packageManager === "bun") { - return `bun --cwd ${packageRoot} run ${script}`; + return `bun --cwd ${quotedRoot} run ${quotedScript}`; } - return `npm --prefix ${packageRoot} run ${script}`; + return `npm --prefix ${quotedRoot} run ${quotedScript}`; } export function projectDisplayName(info: NodeProjectInfo): string { diff --git a/src/mappers/shared.ts b/src/mappers/shared.ts index 07a71e1..45656b7 100644 --- a/src/mappers/shared.ts +++ b/src/mappers/shared.ts @@ -1,6 +1,7 @@ import { lstat, readdir, realpath } from "node:fs/promises"; import { dirname, isAbsolute, join, relative, sep } from "node:path"; import { pathExists } from "../fs.js"; +import { shellQuotePath } from "../shell.js"; import { TrustBoundary } from "../types.js"; import { FeatureSeed } from "./types.js"; @@ -359,22 +360,26 @@ export function nodeScriptCommand( packageRoot: string, script: string, ): string { + const quotedRoot = shellQuotePath(packageRoot); + const quotedScript = shellQuotePath(script); if (packageRoot === ".") { if (packageManager === "bun") { - return `bun run ${script}`; + return `bun run ${quotedScript}`; } - return packageManager === "npm" ? `npm run ${script}` : `${packageManager} ${script}`; + return packageManager === "npm" + ? `npm run ${quotedScript}` + : `${packageManager} ${quotedScript}`; } if (packageManager === "pnpm") { - return `pnpm --dir ${packageRoot} ${script}`; + return `pnpm --dir ${quotedRoot} ${quotedScript}`; } if (packageManager === "yarn") { - return `yarn --cwd ${packageRoot} ${script}`; + return `yarn --cwd ${quotedRoot} ${quotedScript}`; } if (packageManager === "bun") { - return `bun --cwd ${packageRoot} run ${script}`; + return `bun --cwd ${quotedRoot} run ${quotedScript}`; } - return `npm --prefix ${packageRoot} run ${script}`; + return `npm --prefix ${quotedRoot} run ${quotedScript}`; } function isTestPath(path: string): boolean { diff --git a/src/mappers/swift.ts b/src/mappers/swift.ts index b2d20d3..2377a70 100644 --- a/src/mappers/swift.ts +++ b/src/mappers/swift.ts @@ -1,6 +1,7 @@ import { lstat, readFile, readdir } from "node:fs/promises"; import { join } from "node:path"; import { pathExists } from "../fs.js"; +import { shellQuotePath } from "../shell.js"; import { normalize, isSampleProjectPath, @@ -174,7 +175,8 @@ function prefixSwiftSeed(seed: FeatureSeed, packageRoot: string): FeatureSeed { if (packageRoot === ".") { return seed; } - const testCommand = seed.testCommand === null ? null : `swift test --package-path ${packageRoot}`; + const testCommand = + seed.testCommand === null ? null : `swift test --package-path ${shellQuotePath(packageRoot)}`; const prefixed: FeatureSeed = { ...seed, title: `${seed.title} (${packageRoot})`, @@ -214,7 +216,8 @@ function prefixTestRefs( return refs?.map((ref) => ({ ...ref, path: prefixSwiftPath(packageRoot, ref.path), - command: ref.command === null ? null : `swift test --package-path ${packageRoot}`, + command: + ref.command === null ? null : `swift test --package-path ${shellQuotePath(packageRoot)}`, })); } From a9022877bd702c301c53b6f1bcbb8996470cd0a5 Mon Sep 17 00:00:00 2001 From: Aaron Elijah Mars Date: Mon, 25 May 2026 09:28:32 -0400 Subject: [PATCH 2/2] fix(security): shell-quote Turbo --filter and Nx project name too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the validation-command quoting: turboCommand's `--filter` (repo-derived package.json name or ./root) and nxCommand's projectName (Nx project.json / package.json name / dir basename) were interpolated unquoted into command strings that reach spawn(..., { shell: true }) — the same injection class this PR already fixes for scriptCommand / nodeScriptCommand / Swift. Both now go through shellQuotePath; task/target remain unquoted (validationTaskNames allowlist). Added regression tests covering malicious + ordinary Turbo filters and Nx names. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cmd-injection-regression.test.ts | 37 +++++++++++++++++++++++++++- src/mappers/projects.ts | 12 ++++++--- src/mappers/turbo.ts | 15 +++++++---- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/cmd-injection-regression.test.ts b/src/cmd-injection-regression.test.ts index 4b14f90..a92b9db 100644 --- a/src/cmd-injection-regression.test.ts +++ b/src/cmd-injection-regression.test.ts @@ -19,8 +19,9 @@ import { describe, expect, it } from "vitest"; import { mapFeatures } from "./mapper.js"; import { validationCommandsForFeature } from "./validation.js"; import { detectProject } from "./detect.js"; -import { scriptCommand } from "./mappers/projects.js"; +import { scriptCommand, nxCommand } from "./mappers/projects.js"; import { nodeScriptCommand } from "./mappers/shared.js"; +import { turboCommand } from "./mappers/turbo.js"; function isShellSafe(command: string): boolean { // Strip double-quoted segments — anything `$(...)` or `;` inside `"..."` @@ -70,6 +71,40 @@ describe("validation pipeline shell-quotes filesystem-derived names (regression) expect(nodeScriptCommand("npm", "apps/web", "test")).toBe("npm --prefix apps/web run test"); }); + it("turboCommand: malicious turbo --filter (package name or root) is shell-quoted", () => { + for (const pm of ["pnpm", "yarn", "bun", "npm"] as const) { + // package.json `name` + expect( + isShellSafe(turboCommand(pm, "test", "$(id)")), + `pm=${pm}`, + ).toBe(true); + // root fallback `./${project.root}` + expect( + isShellSafe(turboCommand(pm, "test", "./packages/$(id)-pkg")), + `pm=${pm}`, + ).toBe(true); + } + }); + + it("turboCommand: ordinary inputs are unchanged (no over-quoting)", () => { + expect(turboCommand("pnpm", "test", "my-pkg")).toBe("pnpm turbo run test --filter my-pkg"); + expect(turboCommand("npm", "test", "./packages/ui")).toBe( + "npx turbo run test --filter ./packages/ui", + ); + }); + + it("nxCommand: malicious Nx project name is shell-quoted", () => { + for (const pm of ["npm", "bun", "pnpm", "yarn"] as const) { + const cmd = nxCommand(pm, "test", "$(id)"); + expect(isShellSafe(cmd), `pm=${pm} cmd=${cmd}`).toBe(true); + } + }); + + it("nxCommand: ordinary inputs are unchanged", () => { + expect(nxCommand("npm", "test", "my-app")).toBe("npx nx test my-app"); + expect(nxCommand("pnpm", "build", "my-app")).toBe("pnpm nx build my-app"); + }); + it("swift end-to-end: malicious package-root directory cannot inject into swift test --package-path", async () => { const root = await mkdtemp(join(tmpdir(), "clawpatch-cmd-inj-swift-")); try { diff --git a/src/mappers/projects.ts b/src/mappers/projects.ts index 3983cb3..c742e9a 100644 --- a/src/mappers/projects.ts +++ b/src/mappers/projects.ts @@ -999,12 +999,16 @@ async function detectNodePackageManager(root: string): Promise { return "npm"; } -function nxCommand(packageManager: string, target: string, projectName: string): string { +export function nxCommand(packageManager: string, target: string, projectName: string): string { + // `projectName` is repo-derived (Nx project.json / package.json name / dir + // basename), so it must be shell-quoted before reaching the `shell: true` + // runner. `target` comes from the validationTaskNames allowlist. + const quotedName = shellQuotePath(projectName); if (packageManager === "npm") { - return `npx nx ${target} ${projectName}`; + return `npx nx ${target} ${quotedName}`; } if (packageManager === "bun") { - return `bunx nx ${target} ${projectName}`; + return `bunx nx ${target} ${quotedName}`; } - return `${packageManager} nx ${target} ${projectName}`; + return `${packageManager} nx ${target} ${quotedName}`; } diff --git a/src/mappers/turbo.ts b/src/mappers/turbo.ts index c99d072..ace5b1b 100644 --- a/src/mappers/turbo.ts +++ b/src/mappers/turbo.ts @@ -2,6 +2,7 @@ import { readFile } from "node:fs/promises"; import { join } from "node:path"; import { packageScripts } from "../detect.js"; import { pathExists } from "../fs.js"; +import { shellQuotePath } from "../shell.js"; import type { NodeProjectInfo } from "./projects.js"; import { detectNodePackageManager } from "./shared.js"; import { @@ -124,17 +125,21 @@ function stringArray(value: unknown): string[] { : []; } -function turboCommand(packageManager: string, task: string, filter: string): string { +export function turboCommand(packageManager: string, task: string, filter: string): string { + // `filter` is repo-derived (package.json name or on-disk root), so it must be + // shell-quoted before reaching the `shell: true` runner — same as the other + // validation commands. `task` comes from the validationTaskNames allowlist. + const quotedFilter = shellQuotePath(filter); if (packageManager === "pnpm") { - return `pnpm turbo run ${task} --filter ${filter}`; + return `pnpm turbo run ${task} --filter ${quotedFilter}`; } if (packageManager === "yarn") { - return `yarn turbo run ${task} --filter ${filter}`; + return `yarn turbo run ${task} --filter ${quotedFilter}`; } if (packageManager === "bun") { - return `bunx turbo run ${task} --filter ${filter}`; + return `bunx turbo run ${task} --filter ${quotedFilter}`; } - return `npx turbo run ${task} --filter ${filter}`; + return `npx turbo run ${task} --filter ${quotedFilter}`; } function turboPackageName(project: NodeProjectInfo): string | null {