Skip to content

Commit 71ad224

Browse files
rohitjavvadirohitjavvadi
committed
fix(mapper): quote validation command arguments
1 parent 857d854 commit 71ad224

8 files changed

Lines changed: 250 additions & 24 deletions

File tree

‎src/mapper.test.ts‎

Lines changed: 194 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,26 @@ import { basename, join } from "node:path";
33
import { describe, expect, it } from "vitest";
44
import { detectProject } from "./detect.js";
55
import { mapFeatures } from "./mapper.js";
6-
import { discoverNodeProjects } from "./mappers/projects.js";
6+
import { discoverNodeProjects, scriptCommand } from "./mappers/projects.js";
7+
import { nodeScriptCommand } from "./mappers/shared.js";
78
import { turboTaskGraph } from "./mappers/turbo.js";
89
import { fixtureRoot, writeFixture } from "./test-helpers.js";
910

1011
const symlinkIt = process.platform === "win32" ? it.skip : it;
1112

1213
describe("mapFeatures", () => {
14+
it("quotes dynamic Node validation command parts", () => {
15+
expect(scriptCommand("pnpm", "packages/app; touch INJECTED", "test")).toBe(
16+
'pnpm --dir "packages/app; touch INJECTED" test',
17+
);
18+
expect(scriptCommand("npm", ".", "test:unit; touch INJECTED")).toBe(
19+
'npm run "test:unit; touch INJECTED"',
20+
);
21+
expect(nodeScriptCommand("npm", "apps/site $(touch INJECTED)", "test")).toBe(
22+
'npm --prefix "apps/site \\$(touch INJECTED)" run test',
23+
);
24+
});
25+
1326
it("applies configured path excludes to heuristic feature mapping", async () => {
1427
const root = await fixtureRoot("clawpatch-map-exclude-");
1528
await writeFixture(root, "requirements.txt", "pytest\n");
@@ -852,6 +865,51 @@ describe("mapFeatures", () => {
852865
]);
853866
});
854867

868+
it("quotes workspace package roots with shell metacharacters in mapped validation commands", async () => {
869+
const root = await fixtureRoot("clawpatch-task-graph-fallback-quoted-");
870+
const packageRoot = "apps/web; touch INJECTED";
871+
await writeFixture(
872+
root,
873+
"package.json",
874+
JSON.stringify(
875+
{ name: "workspace-root", workspaces: ["apps/*"], dependencies: { next: "1.0.0" } },
876+
null,
877+
2,
878+
),
879+
);
880+
await writeFixture(root, "pnpm-lock.yaml", "");
881+
await writeFixture(
882+
root,
883+
`${packageRoot}/package.json`,
884+
JSON.stringify(
885+
{
886+
name: "web",
887+
scripts: { test: "vitest run", build: "next build" },
888+
dependencies: { next: "1.0.0" },
889+
},
890+
null,
891+
2,
892+
),
893+
);
894+
await writeFixture(
895+
root,
896+
`${packageRoot}/app/page.tsx`,
897+
"export default function Page() { return null; }\n",
898+
);
899+
await writeFixture(root, `${packageRoot}/app/page.test.tsx`, "test('page', () => {});\n");
900+
901+
const project = await detectProject(root);
902+
const result = await mapFeatures(root, project, []);
903+
const route = result.features.find((feature) => feature.title === "web route /");
904+
905+
expect(route?.tests).toEqual([
906+
{
907+
path: `${packageRoot}/app/page.test.tsx`,
908+
command: 'pnpm --dir "apps/web; touch INJECTED" test',
909+
},
910+
]);
911+
});
912+
855913
it("uses bun workspace commands when the root has a text bun lockfile", async () => {
856914
const root = await fixtureRoot("clawpatch-task-graph-bun-lock-");
857915
await writeFixture(
@@ -2426,6 +2484,55 @@ describe("mapFeatures", () => {
24262484
]);
24272485
});
24282486

2487+
it("quotes Turbo task filters with shell metacharacters", async () => {
2488+
const root = await fixtureRoot("clawpatch-turbo-quoted-filter-");
2489+
await writeFixture(
2490+
root,
2491+
"package.json",
2492+
JSON.stringify(
2493+
{
2494+
name: "workspace-root",
2495+
packageManager: "pnpm@10.0.0",
2496+
workspaces: ["apps/*"],
2497+
},
2498+
null,
2499+
2,
2500+
),
2501+
);
2502+
await writeFixture(root, "pnpm-lock.yaml", "");
2503+
await writeFixture(root, "turbo.json", JSON.stringify({ tasks: { test: {} } }, null, 2));
2504+
await writeFixture(
2505+
root,
2506+
"apps/web/package.json",
2507+
JSON.stringify(
2508+
{
2509+
name: "web; touch INJECTED",
2510+
scripts: { test: "vitest run" },
2511+
dependencies: { next: "1.0.0" },
2512+
},
2513+
null,
2514+
2,
2515+
),
2516+
);
2517+
await writeFixture(
2518+
root,
2519+
"apps/web/app/page.tsx",
2520+
"export default function Page() { return null; }\n",
2521+
);
2522+
await writeFixture(root, "apps/web/app/page.test.tsx", "test('page', () => {});\n");
2523+
2524+
const project = await detectProject(root);
2525+
const result = await mapFeatures(root, project, []);
2526+
const mappedTest = result.features
2527+
.flatMap((feature) => feature.tests)
2528+
.find((test) => test.path === "apps/web/app/page.test.tsx");
2529+
2530+
expect(mappedTest).toEqual({
2531+
path: "apps/web/app/page.test.tsx",
2532+
command: 'pnpm turbo run test --filter "web; touch INJECTED"',
2533+
});
2534+
});
2535+
24292536
it("keeps package-local validation for fallback packages outside the workspace graph", async () => {
24302537
const root = await fixtureRoot("clawpatch-turbo-non-workspace-package-");
24312538
await writeFixture(
@@ -4973,6 +5080,42 @@ describe("mapFeatures", () => {
49735080
]);
49745081
});
49755082

5083+
it("quotes nested Swift package roots with shell metacharacters", async () => {
5084+
const root = await fixtureRoot("clawpatch-swift-quoted-package-path-");
5085+
const packageRoot = "apps/macos; touch INJECTED";
5086+
await writeFixture(
5087+
root,
5088+
`${packageRoot}/Package.swift`,
5089+
[
5090+
"// swift-tools-version: 6.0",
5091+
"import PackageDescription",
5092+
"let package = Package(",
5093+
' name: "MacApp",',
5094+
' targets: [.executableTarget(name: "MacApp"), .testTarget(name: "MacAppTests", dependencies: ["MacApp"])]',
5095+
")",
5096+
].join("\n"),
5097+
);
5098+
await writeFixture(root, `${packageRoot}/Sources/MacApp/main.swift`, "@main struct App {}\n");
5099+
await writeFixture(
5100+
root,
5101+
`${packageRoot}/Tests/MacAppTests/MacAppTests.swift`,
5102+
"import Testing\n",
5103+
);
5104+
5105+
const project = await detectProject(root);
5106+
const result = await mapFeatures(root, project, []);
5107+
const mac = result.features.find((feature) =>
5108+
feature.title.startsWith("Swift executable MacApp"),
5109+
);
5110+
5111+
expect(mac?.tests).toEqual([
5112+
{
5113+
path: `${packageRoot}/Tests/MacAppTests/MacAppTests.swift`,
5114+
command: 'swift test --package-path "apps/macos; touch INJECTED"',
5115+
},
5116+
]);
5117+
});
5118+
49765119
it("maps Kotlin Android semantic roles from framework evidence", async () => {
49775120
const root = await fixtureRoot("clawpatch-kotlin-android-role-map-");
49785121
await writeFixture(root, "settings.gradle.kts", 'pluginManagement {}\ninclude(":app")\n');
@@ -10797,6 +10940,26 @@ let package = Package(name: "HybridApp", targets: [.target(name: "HybridApp")])
1079710940
]);
1079810941
});
1079910942

10943+
it("quotes conventional Rust crate manifest paths with shell metacharacters", async () => {
10944+
const root = await fixtureRoot("clawpatch-rust-quoted-manifest-path-");
10945+
const memberRoot = "crates/member; touch INJECTED";
10946+
await writeFixture(root, "Cargo.toml", "[workspace]\n");
10947+
await writeFixture(root, `${memberRoot}/Cargo.toml`, '[package]\nname = "member"\n');
10948+
await writeFixture(root, `${memberRoot}/src/lib.rs`, "pub fn run() {}\n");
10949+
await writeFixture(root, `${memberRoot}/tests/member_test.rs`, "#[test]\nfn works() {}\n");
10950+
10951+
const project = await detectProject(root);
10952+
const result = await mapFeatures(root, project, []);
10953+
const library = result.features.find((feature) => feature.title === "Rust library member");
10954+
10955+
expect(library?.tests).toEqual([
10956+
{
10957+
path: `${memberRoot}/tests/member_test.rs`,
10958+
command: 'cargo test --manifest-path "crates/member; touch INJECTED/Cargo.toml"',
10959+
},
10960+
]);
10961+
});
10962+
1080010963
it("bounds Rust integration tests attached to entrypoint features", async () => {
1080110964
const root = await fixtureRoot("clawpatch-rust-test-bound-");
1080210965
await writeFixture(root, "Cargo.toml", '[package]\nname = "rust-test-bound"\n');
@@ -15507,6 +15670,36 @@ end
1550715670
]);
1550815671
});
1550915672

15673+
it("quotes Elixir test paths with shell metacharacters", async () => {
15674+
const root = await fixtureRoot("clawpatch-elixir-quoted-test-path-");
15675+
await writeFixture(
15676+
root,
15677+
"mix.exs",
15678+
'defmodule SampleApp.MixProject do\n use Mix.Project\n def project, do: [app: :sample_app, version: "0.1.0"]\nend\n',
15679+
);
15680+
await writeFixture(
15681+
root,
15682+
"lib/sample_app/accounts.ex",
15683+
"defmodule SampleApp.Accounts do\nend\n",
15684+
);
15685+
await writeFixture(
15686+
root,
15687+
"test/sample_app/accounts/injected; touch INJECTED_test.exs",
15688+
"defmodule SampleApp.AccountsTest do\nuse ExUnit.Case\nend\n",
15689+
);
15690+
15691+
const project = await detectProject(root);
15692+
const result = await mapFeatures(root, project, []);
15693+
const accounts = result.features.find((feature) => feature.title === "Elixir context accounts");
15694+
15695+
expect(accounts?.tests).toEqual([
15696+
{
15697+
path: "test/sample_app/accounts/injected; touch INJECTED_test.exs",
15698+
command: 'mix test "test/sample_app/accounts/injected; touch INJECTED_test.exs"',
15699+
},
15700+
]);
15701+
});
15702+
1551015703
it("does not map generated Mix dependency C files", async () => {
1551115704
const root = await fixtureRoot("clawpatch-elixir-deps-skip-");
1551215705
await writeFixture(

‎src/mappers/elixir.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { readFile, readdir } from "node:fs/promises";
22
import { basename, join } from "node:path";
33
import { pathExists } from "../fs.js";
4+
import { shellQuotePath } from "../shell.js";
45
import { packageKind, pathMatchesPrefix, shouldSkip, stripLineComments, walk } from "./shared.js";
56
import { FeatureSeed, SeedTestRef } from "./types.js";
67

@@ -301,7 +302,7 @@ function associatedTests(files: string[], testFiles: string[]): SeedTestRef[] {
301302
return testFiles
302303
.filter((path) => prefixes.some((prefix) => pathMatchesPrefix(path, prefix)))
303304
.slice(0, elixirTestGroupMaxFiles)
304-
.map((path) => ({ path, command: `mix test ${path}` }));
305+
.map((path) => ({ path, command: `mix test ${shellQuotePath(path)}` }));
305306
}
306307

307308
function testPrefixesForSource(path: string): string[] {

‎src/mappers/projects.ts‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { lstat, readFile, readdir, realpath } from "node:fs/promises";
22
import { basename, dirname, join } from "node:path";
33
import { packageScripts, readPackageJson } from "../detect.js";
44
import { pathExists } from "../fs.js";
5+
import { shellQuotePath } from "../shell.js";
56
import { isSafeDirectory, normalize, pathMatchesPrefix, shouldSkip } from "./shared.js";
67
import { taskGraphCommand, type WorkspaceTaskGraph } from "./task-graph.js";
78
import type { SeedFileRef } from "./types.js";
@@ -194,22 +195,26 @@ export function packageRelativePath(packageRoot: string, path: string): string {
194195
}
195196

196197
export function scriptCommand(packageManager: string, packageRoot: string, script: string): string {
198+
const quotedScript = shellQuotePath(script);
197199
if (packageRoot === ".") {
198200
if (packageManager === "bun") {
199-
return `bun run ${script}`;
201+
return `bun run ${quotedScript}`;
200202
}
201-
return packageManager === "npm" ? `npm run ${script}` : `${packageManager} ${script}`;
203+
return packageManager === "npm"
204+
? `npm run ${quotedScript}`
205+
: `${packageManager} ${quotedScript}`;
202206
}
207+
const quotedRoot = shellQuotePath(packageRoot);
203208
if (packageManager === "pnpm") {
204-
return `pnpm --dir ${packageRoot} ${script}`;
209+
return `pnpm --dir ${quotedRoot} ${quotedScript}`;
205210
}
206211
if (packageManager === "yarn") {
207-
return `yarn --cwd ${packageRoot} ${script}`;
212+
return `yarn --cwd ${quotedRoot} ${quotedScript}`;
208213
}
209214
if (packageManager === "bun") {
210-
return `bun --cwd ${packageRoot} run ${script}`;
215+
return `bun --cwd ${quotedRoot} run ${quotedScript}`;
211216
}
212-
return `npm --prefix ${packageRoot} run ${script}`;
217+
return `npm --prefix ${quotedRoot} run ${quotedScript}`;
213218
}
214219

215220
export function projectDisplayName(info: NodeProjectInfo): string {
@@ -995,11 +1000,13 @@ async function detectNodePackageManager(root: string): Promise<string> {
9951000
}
9961001

9971002
function nxCommand(packageManager: string, target: string, projectName: string): string {
1003+
const quotedTarget = shellQuotePath(target);
1004+
const quotedProjectName = shellQuotePath(projectName);
9981005
if (packageManager === "npm") {
999-
return `npx nx ${target} ${projectName}`;
1006+
return `npx nx ${quotedTarget} ${quotedProjectName}`;
10001007
}
10011008
if (packageManager === "bun") {
1002-
return `bunx nx ${target} ${projectName}`;
1009+
return `bunx nx ${quotedTarget} ${quotedProjectName}`;
10031010
}
1004-
return `${packageManager} nx ${target} ${projectName}`;
1011+
return `${packageManager} nx ${quotedTarget} ${quotedProjectName}`;
10051012
}

‎src/mappers/rust.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { readFile, readdir } from "node:fs/promises";
22
import { join } from "node:path";
33
import { pathExists } from "../fs.js";
4+
import { shellQuotePath } from "../shell.js";
45
import {
56
isSafeDirectory,
67
isSafeFile,
@@ -89,7 +90,7 @@ async function rustMemberDirs(root: string): Promise<RustMemberDir[]> {
8990
for (const member of await conventionalCrateDirs(root, workspace.excluded)) {
9091
dirs.set(member, {
9192
dir: member,
92-
testCommand: `cargo test --manifest-path ${member}/Cargo.toml`,
93+
testCommand: `cargo test --manifest-path ${shellQuotePath(`${member}/Cargo.toml`)}`,
9394
});
9495
}
9596
}

‎src/mappers/shared.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { lstat, readdir, realpath } from "node:fs/promises";
22
import { dirname, isAbsolute, join, relative, sep } from "node:path";
33
import { pathExists } from "../fs.js";
4+
import { shellQuotePath } from "../shell.js";
45
import { TrustBoundary } from "../types.js";
56
import { FeatureSeed } from "./types.js";
67

@@ -359,22 +360,26 @@ export function nodeScriptCommand(
359360
packageRoot: string,
360361
script: string,
361362
): string {
363+
const quotedScript = shellQuotePath(script);
362364
if (packageRoot === ".") {
363365
if (packageManager === "bun") {
364-
return `bun run ${script}`;
366+
return `bun run ${quotedScript}`;
365367
}
366-
return packageManager === "npm" ? `npm run ${script}` : `${packageManager} ${script}`;
368+
return packageManager === "npm"
369+
? `npm run ${quotedScript}`
370+
: `${packageManager} ${quotedScript}`;
367371
}
372+
const quotedRoot = shellQuotePath(packageRoot);
368373
if (packageManager === "pnpm") {
369-
return `pnpm --dir ${packageRoot} ${script}`;
374+
return `pnpm --dir ${quotedRoot} ${quotedScript}`;
370375
}
371376
if (packageManager === "yarn") {
372-
return `yarn --cwd ${packageRoot} ${script}`;
377+
return `yarn --cwd ${quotedRoot} ${quotedScript}`;
373378
}
374379
if (packageManager === "bun") {
375-
return `bun --cwd ${packageRoot} run ${script}`;
380+
return `bun --cwd ${quotedRoot} run ${quotedScript}`;
376381
}
377-
return `npm --prefix ${packageRoot} run ${script}`;
382+
return `npm --prefix ${quotedRoot} run ${quotedScript}`;
378383
}
379384

380385
function isTestPath(path: string): boolean {

0 commit comments

Comments
 (0)