Skip to content

Commit c272b53

Browse files
aaronjmarsclaude
andcommitted
fix(security): shell-quote Rust conventional-crate manifest path
The Rust mapper's conventional-crate discovery reads `crates/*` directly via readdir and interpolated each directory name into `cargo test --manifest-path <dir>/Cargo.toml` unquoted. Because the validation pipeline runs commands through `spawn(_, { shell: true })`, a crate directory literally named e.g. `$(id)` executed arbitrary commands — the same command-injection class this PR closes for the Node/Nx/Turbo/ Elixir/Swift mappers, but this one filesystem-derived sink was missed. Route the manifest path through `shellQuotePath`, matching every other mapper. Ordinary crate names stay unquoted (no over-quoting); names with shell metacharacters are quoted and escaped. Adds an end-to-end regression test that builds a workspace containing a `crates/$(id)-crate` directory and asserts the produced `--manifest-path` command is shell-safe. The test fails against the prior code and passes with the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a902287 commit c272b53

2 files changed

Lines changed: 48 additions & 9 deletions

File tree

src/cmd-injection-regression.test.ts

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,15 +74,9 @@ describe("validation pipeline shell-quotes filesystem-derived names (regression)
7474
it("turboCommand: malicious turbo --filter (package name or root) is shell-quoted", () => {
7575
for (const pm of ["pnpm", "yarn", "bun", "npm"] as const) {
7676
// package.json `name`
77-
expect(
78-
isShellSafe(turboCommand(pm, "test", "$(id)")),
79-
`pm=${pm}`,
80-
).toBe(true);
77+
expect(isShellSafe(turboCommand(pm, "test", "$(id)")), `pm=${pm}`).toBe(true);
8178
// root fallback `./${project.root}`
82-
expect(
83-
isShellSafe(turboCommand(pm, "test", "./packages/$(id)-pkg")),
84-
`pm=${pm}`,
85-
).toBe(true);
79+
expect(isShellSafe(turboCommand(pm, "test", "./packages/$(id)-pkg")), `pm=${pm}`).toBe(true);
8680
}
8781
});
8882

@@ -140,4 +134,45 @@ describe("validation pipeline shell-quotes filesystem-derived names (regression)
140134
await rm(root, { recursive: true, force: true });
141135
}
142136
});
137+
138+
it("rust end-to-end: malicious conventional crate directory cannot inject into cargo --manifest-path", async () => {
139+
const root = await mkdtemp(join(tmpdir(), "clawpatch-cmd-inj-rust-"));
140+
try {
141+
// A workspace with no declared members triggers conventional crate
142+
// discovery, which reads `crates/*` straight off disk — so the directory
143+
// name flows into `cargo test --manifest-path <dir>/Cargo.toml`.
144+
await writeFile(join(root, "Cargo.toml"), '[workspace]\nresolver = "2"\n');
145+
const maliciousCrate = join("crates", "$(id)-crate");
146+
await mkdir(join(root, maliciousCrate, "src"), { recursive: true });
147+
await writeFile(
148+
join(root, maliciousCrate, "Cargo.toml"),
149+
'[package]\nname = "evil"\nversion = "0.1.0"\nedition = "2021"\n',
150+
);
151+
await writeFile(join(root, maliciousCrate, "src", "lib.rs"), "pub fn evil() {}\n");
152+
await mkdir(join(root, maliciousCrate, "tests"), { recursive: true });
153+
await writeFile(join(root, maliciousCrate, "tests", "it.rs"), "#[test]\nfn it() {}\n");
154+
155+
const project = await detectProject(root);
156+
const result = await mapFeatures(root, project, []);
157+
const allCommands = result.features.flatMap((feature) =>
158+
validationCommandsForFeature(feature, {
159+
typecheck: null,
160+
lint: null,
161+
format: null,
162+
test: null,
163+
}),
164+
);
165+
// Guard against a silently-empty assertion: the malicious crate must
166+
// actually produce the manifest-path command we're checking.
167+
expect(
168+
allCommands.some((c) => c.includes("--manifest-path")),
169+
`expected a --manifest-path command, got: ${JSON.stringify(allCommands)}`,
170+
).toBe(true);
171+
for (const c of allCommands) {
172+
expect(isShellSafe(c), `unsafe command produced: ${c}`).toBe(true);
173+
}
174+
} finally {
175+
await rm(root, { recursive: true, force: true });
176+
}
177+
});
143178
});

src/mappers/rust.ts

Lines changed: 5 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,10 @@ 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+
// `member` is a directory name discovered via readdir, so a crate dir
94+
// literally named e.g. `$(id)` would inject under `spawn(_, {shell:true})`.
95+
// Quote it like every other filesystem-derived validation-command fragment.
96+
testCommand: `cargo test --manifest-path ${shellQuotePath(`${member}/Cargo.toml`)}`,
9397
});
9498
}
9599
}

0 commit comments

Comments
 (0)