From 71ad22437f9f1b484fefced26095d35cfd58c3b5 Mon Sep 17 00:00:00 2001 From: Rohit Date: Sun, 24 May 2026 12:36:36 +0530 Subject: [PATCH] fix(mapper): quote validation command arguments --- src/mapper.test.ts | 195 +++++++++++++++++++++++++++++++++++++++- src/mappers/elixir.ts | 3 +- src/mappers/projects.ts | 25 ++++-- src/mappers/rust.ts | 3 +- src/mappers/shared.ts | 17 ++-- src/mappers/swift.ts | 8 +- src/mappers/turbo.ts | 11 ++- src/shell.test.ts | 12 +++ 8 files changed, 250 insertions(+), 24 deletions(-) diff --git a/src/mapper.test.ts b/src/mapper.test.ts index 0b0889f..eafe511 100644 --- a/src/mapper.test.ts +++ b/src/mapper.test.ts @@ -3,13 +3,26 @@ import { basename, join } from "node:path"; import { describe, expect, it } from "vitest"; import { detectProject } from "./detect.js"; import { mapFeatures } from "./mapper.js"; -import { discoverNodeProjects } from "./mappers/projects.js"; +import { discoverNodeProjects, scriptCommand } from "./mappers/projects.js"; +import { nodeScriptCommand } from "./mappers/shared.js"; import { turboTaskGraph } from "./mappers/turbo.js"; import { fixtureRoot, writeFixture } from "./test-helpers.js"; const symlinkIt = process.platform === "win32" ? it.skip : it; describe("mapFeatures", () => { + it("quotes dynamic Node validation command parts", () => { + expect(scriptCommand("pnpm", "packages/app; touch INJECTED", "test")).toBe( + 'pnpm --dir "packages/app; touch INJECTED" test', + ); + expect(scriptCommand("npm", ".", "test:unit; touch INJECTED")).toBe( + 'npm run "test:unit; touch INJECTED"', + ); + expect(nodeScriptCommand("npm", "apps/site $(touch INJECTED)", "test")).toBe( + 'npm --prefix "apps/site \\$(touch INJECTED)" run test', + ); + }); + it("applies configured path excludes to heuristic feature mapping", async () => { const root = await fixtureRoot("clawpatch-map-exclude-"); await writeFixture(root, "requirements.txt", "pytest\n"); @@ -852,6 +865,51 @@ describe("mapFeatures", () => { ]); }); + it("quotes workspace package roots with shell metacharacters in mapped validation commands", async () => { + const root = await fixtureRoot("clawpatch-task-graph-fallback-quoted-"); + const packageRoot = "apps/web; touch INJECTED"; + await writeFixture( + root, + "package.json", + JSON.stringify( + { name: "workspace-root", workspaces: ["apps/*"], dependencies: { next: "1.0.0" } }, + null, + 2, + ), + ); + await writeFixture(root, "pnpm-lock.yaml", ""); + await writeFixture( + root, + `${packageRoot}/package.json`, + JSON.stringify( + { + name: "web", + scripts: { test: "vitest run", build: "next build" }, + dependencies: { next: "1.0.0" }, + }, + null, + 2, + ), + ); + await writeFixture( + root, + `${packageRoot}/app/page.tsx`, + "export default function Page() { return null; }\n", + ); + await writeFixture(root, `${packageRoot}/app/page.test.tsx`, "test('page', () => {});\n"); + + const project = await detectProject(root); + const result = await mapFeatures(root, project, []); + const route = result.features.find((feature) => feature.title === "web route /"); + + expect(route?.tests).toEqual([ + { + path: `${packageRoot}/app/page.test.tsx`, + command: 'pnpm --dir "apps/web; touch INJECTED" test', + }, + ]); + }); + it("uses bun workspace commands when the root has a text bun lockfile", async () => { const root = await fixtureRoot("clawpatch-task-graph-bun-lock-"); await writeFixture( @@ -2426,6 +2484,55 @@ describe("mapFeatures", () => { ]); }); + it("quotes Turbo task filters with shell metacharacters", async () => { + const root = await fixtureRoot("clawpatch-turbo-quoted-filter-"); + await writeFixture( + root, + "package.json", + JSON.stringify( + { + name: "workspace-root", + packageManager: "pnpm@10.0.0", + workspaces: ["apps/*"], + }, + null, + 2, + ), + ); + await writeFixture(root, "pnpm-lock.yaml", ""); + await writeFixture(root, "turbo.json", JSON.stringify({ tasks: { test: {} } }, null, 2)); + await writeFixture( + root, + "apps/web/package.json", + JSON.stringify( + { + name: "web; touch INJECTED", + scripts: { test: "vitest run" }, + dependencies: { next: "1.0.0" }, + }, + null, + 2, + ), + ); + await writeFixture( + root, + "apps/web/app/page.tsx", + "export default function Page() { return null; }\n", + ); + await writeFixture(root, "apps/web/app/page.test.tsx", "test('page', () => {});\n"); + + const project = await detectProject(root); + const result = await mapFeatures(root, project, []); + const mappedTest = result.features + .flatMap((feature) => feature.tests) + .find((test) => test.path === "apps/web/app/page.test.tsx"); + + expect(mappedTest).toEqual({ + path: "apps/web/app/page.test.tsx", + command: 'pnpm turbo run test --filter "web; touch INJECTED"', + }); + }); + it("keeps package-local validation for fallback packages outside the workspace graph", async () => { const root = await fixtureRoot("clawpatch-turbo-non-workspace-package-"); await writeFixture( @@ -4973,6 +5080,42 @@ describe("mapFeatures", () => { ]); }); + it("quotes nested Swift package roots with shell metacharacters", async () => { + const root = await fixtureRoot("clawpatch-swift-quoted-package-path-"); + const packageRoot = "apps/macos; touch INJECTED"; + await writeFixture( + root, + `${packageRoot}/Package.swift`, + [ + "// swift-tools-version: 6.0", + "import PackageDescription", + "let package = Package(", + ' name: "MacApp",', + ' targets: [.executableTarget(name: "MacApp"), .testTarget(name: "MacAppTests", dependencies: ["MacApp"])]', + ")", + ].join("\n"), + ); + await writeFixture(root, `${packageRoot}/Sources/MacApp/main.swift`, "@main struct App {}\n"); + await writeFixture( + root, + `${packageRoot}/Tests/MacAppTests/MacAppTests.swift`, + "import Testing\n", + ); + + const project = await detectProject(root); + const result = await mapFeatures(root, project, []); + const mac = result.features.find((feature) => + feature.title.startsWith("Swift executable MacApp"), + ); + + expect(mac?.tests).toEqual([ + { + path: `${packageRoot}/Tests/MacAppTests/MacAppTests.swift`, + command: 'swift test --package-path "apps/macos; touch INJECTED"', + }, + ]); + }); + it("maps Kotlin Android semantic roles from framework evidence", async () => { const root = await fixtureRoot("clawpatch-kotlin-android-role-map-"); await writeFixture(root, "settings.gradle.kts", 'pluginManagement {}\ninclude(":app")\n'); @@ -10797,6 +10940,26 @@ let package = Package(name: "HybridApp", targets: [.target(name: "HybridApp")]) ]); }); + it("quotes conventional Rust crate manifest paths with shell metacharacters", async () => { + const root = await fixtureRoot("clawpatch-rust-quoted-manifest-path-"); + const memberRoot = "crates/member; touch INJECTED"; + await writeFixture(root, "Cargo.toml", "[workspace]\n"); + await writeFixture(root, `${memberRoot}/Cargo.toml`, '[package]\nname = "member"\n'); + await writeFixture(root, `${memberRoot}/src/lib.rs`, "pub fn run() {}\n"); + await writeFixture(root, `${memberRoot}/tests/member_test.rs`, "#[test]\nfn works() {}\n"); + + const project = await detectProject(root); + const result = await mapFeatures(root, project, []); + const library = result.features.find((feature) => feature.title === "Rust library member"); + + expect(library?.tests).toEqual([ + { + path: `${memberRoot}/tests/member_test.rs`, + command: 'cargo test --manifest-path "crates/member; touch INJECTED/Cargo.toml"', + }, + ]); + }); + it("bounds Rust integration tests attached to entrypoint features", async () => { const root = await fixtureRoot("clawpatch-rust-test-bound-"); await writeFixture(root, "Cargo.toml", '[package]\nname = "rust-test-bound"\n'); @@ -15507,6 +15670,36 @@ end ]); }); + it("quotes Elixir test paths with shell metacharacters", async () => { + const root = await fixtureRoot("clawpatch-elixir-quoted-test-path-"); + await writeFixture( + root, + "mix.exs", + 'defmodule SampleApp.MixProject do\n use Mix.Project\n def project, do: [app: :sample_app, version: "0.1.0"]\nend\n', + ); + await writeFixture( + root, + "lib/sample_app/accounts.ex", + "defmodule SampleApp.Accounts do\nend\n", + ); + await writeFixture( + root, + "test/sample_app/accounts/injected; touch INJECTED_test.exs", + "defmodule SampleApp.AccountsTest do\nuse ExUnit.Case\nend\n", + ); + + const project = await detectProject(root); + const result = await mapFeatures(root, project, []); + const accounts = result.features.find((feature) => feature.title === "Elixir context accounts"); + + expect(accounts?.tests).toEqual([ + { + path: "test/sample_app/accounts/injected; touch INJECTED_test.exs", + command: 'mix test "test/sample_app/accounts/injected; touch INJECTED_test.exs"', + }, + ]); + }); + it("does not map generated Mix dependency C files", async () => { const root = await fixtureRoot("clawpatch-elixir-deps-skip-"); await writeFixture( 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..84857d1 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 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}`; } + const quotedRoot = shellQuotePath(packageRoot); 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 { @@ -995,11 +1000,13 @@ async function detectNodePackageManager(root: string): Promise { } function nxCommand(packageManager: string, target: string, projectName: string): string { + const quotedTarget = shellQuotePath(target); + const quotedProjectName = shellQuotePath(projectName); if (packageManager === "npm") { - return `npx nx ${target} ${projectName}`; + return `npx nx ${quotedTarget} ${quotedProjectName}`; } if (packageManager === "bun") { - return `bunx nx ${target} ${projectName}`; + return `bunx nx ${quotedTarget} ${quotedProjectName}`; } - return `${packageManager} nx ${target} ${projectName}`; + return `${packageManager} nx ${quotedTarget} ${quotedProjectName}`; } diff --git a/src/mappers/rust.ts b/src/mappers/rust.ts index 61e5b7f..de3ae64 100644 --- a/src/mappers/rust.ts +++ b/src/mappers/rust.ts @@ -1,6 +1,7 @@ import { readFile, readdir } from "node:fs/promises"; import { join } from "node:path"; import { pathExists } from "../fs.js"; +import { shellQuotePath } from "../shell.js"; import { isSafeDirectory, isSafeFile, @@ -89,7 +90,7 @@ async function rustMemberDirs(root: string): Promise { for (const member of await conventionalCrateDirs(root, workspace.excluded)) { dirs.set(member, { dir: member, - testCommand: `cargo test --manifest-path ${member}/Cargo.toml`, + testCommand: `cargo test --manifest-path ${shellQuotePath(`${member}/Cargo.toml`)}`, }); } } diff --git a/src/mappers/shared.ts b/src/mappers/shared.ts index 07a71e1..72dd93e 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 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}`; } + const quotedRoot = shellQuotePath(packageRoot); 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..7c851d7 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,9 @@ function prefixSwiftSeed(seed: FeatureSeed, packageRoot: string): FeatureSeed { if (packageRoot === ".") { return seed; } - const testCommand = seed.testCommand === null ? null : `swift test --package-path ${packageRoot}`; + const quotedPackageRoot = shellQuotePath(packageRoot); + const testCommand = + seed.testCommand === null ? null : `swift test --package-path ${quotedPackageRoot}`; const prefixed: FeatureSeed = { ...seed, title: `${seed.title} (${packageRoot})`, @@ -211,10 +214,11 @@ function prefixTestRefs( packageRoot: string, refs: SeedTestRef[] | undefined, ): SeedTestRef[] | undefined { + const quotedPackageRoot = shellQuotePath(packageRoot); 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 ${quotedPackageRoot}`, })); } diff --git a/src/mappers/turbo.ts b/src/mappers/turbo.ts index c99d072..884f398 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 { @@ -125,16 +126,18 @@ function stringArray(value: unknown): string[] { } function turboCommand(packageManager: string, task: string, filter: string): string { + const quotedTask = shellQuotePath(task); + const quotedFilter = shellQuotePath(filter); if (packageManager === "pnpm") { - return `pnpm turbo run ${task} --filter ${filter}`; + return `pnpm turbo run ${quotedTask} --filter ${quotedFilter}`; } if (packageManager === "yarn") { - return `yarn turbo run ${task} --filter ${filter}`; + return `yarn turbo run ${quotedTask} --filter ${quotedFilter}`; } if (packageManager === "bun") { - return `bunx turbo run ${task} --filter ${filter}`; + return `bunx turbo run ${quotedTask} --filter ${quotedFilter}`; } - return `npx turbo run ${task} --filter ${filter}`; + return `npx turbo run ${quotedTask} --filter ${quotedFilter}`; } function turboPackageName(project: NodeProjectInfo): string | null { diff --git a/src/shell.test.ts b/src/shell.test.ts index 05567b2..716aac7 100644 --- a/src/shell.test.ts +++ b/src/shell.test.ts @@ -21,4 +21,16 @@ describe("shellQuotePath", () => { '"src/\\$App/\\"Project\\".csproj"', ); }); + + it("quotes POSIX command separators", () => { + expect(shellQuotePath("packages/app; touch INJECTED", "linux")).toBe( + '"packages/app; touch INJECTED"', + ); + }); + + it("escapes POSIX command substitutions inside quoted paths", () => { + expect(shellQuotePath("packages/$(touch INJECTED)", "linux")).toBe( + '"packages/\\$(touch INJECTED)"', + ); + }); });