Quote mapper-generated validation command arguments#111
Conversation
|
Codex review: needs maintainer review before merge. Latest ClawSweeper review: 2026-05-24 07:11 UTC / May 24, 2026, 3:11 AM ET. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. Source inspection shows current main can generate commands like an unquoted PR rating What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land the focused quoting hardening after normal maintainer review and CI, keeping the existing shellQuotePath helper as the shared command-argument escaping path. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main can generate commands like an unquoted Is this the best way to solve the issue? Yes. Quoting dynamic arguments where mapper validation commands are assembled is the narrow maintainable fix, and the PR uses the existing helper rather than adding a parallel escaping implementation. Label changes:
Label justifications:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 857d854ac8d0. |
|
ClawSweeper PR egg ✨ Hatched: 🌱 uncommon Velvet Patch Peep Hatch commandComment Hatchability rules:
Rarity: 🌱 uncommon. What is this egg doing here?
|
Summary
Why
Clawpatch later executes validation command strings through the shell. If a mapped package path or package name contains shell syntax, a command like
pnpm --dir apps/web; touch INJECTED testcan be interpreted as two shell commands instead of one literal package path.Proof
I tested a fixture with a workspace package under
apps/web; touch INJECTED. After this change,clawpatch mapgenerated:Running that generated command did not create the
INJECTEDsentinel file, which confirms the semicolon stayed part of the package path instead of becoming shell syntax.Testing
./node_modules/.bin/oxfmt src/mapper.test.ts src/mappers/elixir.ts src/mappers/projects.ts src/mappers/rust.ts src/mappers/shared.ts src/mappers/swift.ts src/mappers/turbo.ts src/shell.test.ts --check./node_modules/.bin/oxlint . --config oxlint.json./node_modules/.bin/tsc -p tsconfig.json --noEmit./node_modules/.bin/tsc -p tsconfig.build.json./node_modules/.bin/vitest run src/shell.test.ts src/mapper.test.ts./node_modules/.bin/vitest runapps/web; touch INJECTEDFixes #110