fix(security): shell-quote filesystem-derived paths in validation commands

[aaronjmars]

Summary

Several mappers interpolate filesystem-derived paths and package.json script keys directly into the command strings that the validation pipeline executes (clawpatch fix and clawpatch ci). Those command strings are run through runCommand (src/exec.ts), which uses spawn(..., { shell: true }) — i.e. /bin/sh -c "<command>". An attacker-controlled name flowed unquoted into that string, so shell metacharacters in a workspace package directory, a Swift package directory, an Elixir test filename, or a package.json script key were parsed as shell syntax.

Impact

When an operator runs clawpatch fix --finding <id> or clawpatch ci against a project they did not author, any of the following attacker-controlled names execute arbitrary shell commands on the operator's machine with their privileges:

• pnpm / yarn / bun / npm workspace package directories. scriptCommand and nodeScriptCommand produced pnpm --dir packages/$(id)-pkg test, yarn --cwd …, bun --cwd …, npm --prefix …. A workspace member whose directory name contained ;, $( … ), backticks, or | ran arbitrary commands during validation.
• package.json script keys. Same builders took script unquoted. A repo declaring a script named $(curl …|sh) ran that script when the validation pipeline mapped its target.
• Swift packages. prefixSwiftSeed and prefixTestRefs produced swift test --package-path $(id)-pkg. A SwiftPM package directory with shell metacharacters in its name landed on the shell.
• Elixir test filenames. associatedTests produced mix test test/$(id)_test.exs. A repo with a _test.exs file whose name contained shell metacharacters ran them at validation time.

Severity: high. Reachability is the operator-runs-clawpatch-against-untrusted-repo path that SECURITY.md already names ("repository feature mapping", "patch workflow state").

Location

• src/mappers/projects.ts — scriptCommand
• src/mappers/shared.ts — nodeScriptCommand
• src/mappers/elixir.ts — associatedTests (mix test <path>)
• src/mappers/swift.ts — prefixSwiftSeed and prefixTestRefs (swift test --package-path <pkg>)

Fix

Route each filesystem-derived or config-derived value through the existing shellQuotePath (src/shell.ts) before splicing it into the command string. shellQuotePath returns ordinary path-like values verbatim (whitelist of [A-Za-z0-9_./:@+-]) and wraps anything else in double quotes with $, backtick, and backslash escaped — so the value reaches the shell as a literal even under shell: true.

This is intentionally a local fix; it keeps runCommand as the validation execution path (where freeform pipelines are valid) and only hardens the few sites where the command string is built from attacker-reachable strings.

Verification

• pnpm typecheck — clean
• pnpm lint — clean (0 warnings, 0 errors)
• pnpm format:check — clean
• pnpm test — 720 passed, 1 skipped (up from 714 + 1 baseline; the new tests live in src/cmd-injection-regression.test.ts)

The regression suite covers two angles per builder:

1. With an attacker-controlled name ($(id)-pkg, $(id)), the produced command parses safely under /bin/sh -c (no unquoted $( … ), no unquoted ;, no && / || outside quotes).
2. With ordinary names (packages/ui, test), the produced command is byte-identical to the previous behavior — no over-quoting.

There is also a Swift end-to-end test that walks the full detectProject → mapFeatures → validationCommandsForFeature pipeline against a project whose package directory name contains $(id), and asserts the produced commands are shell-safe.

Detected by

Aeon — manual review against the maintainer's own SECURITY.md scope statement (the in-scope surface lists "repository feature mapping" and "patch workflow state"; the validation execution path falls under both). Class: CWE-78 (OS command injection via filesystem-derived strings) and CWE-88 (argument injection in --dir / --cwd / --prefix / --package-path flags).

Scanner pass (semgrep p/security-audit + p/owasp-top-ten + p/secrets + p/javascript + p/typescript + p/nodejs + p/rust + p/python, trufflehog filesystem + git, osv-scanner --recursive) returned 0 findings; the issue was reached entirely by reading the mappers against the operator-runs-validation trust boundary.

[clawsweeper]

Codex review: needs real behavior proof before merge.

Latest ClawSweeper review: 2026-05-24 07:56 UTC / May 24, 2026, 3:56 AM ET.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The PR shell-quotes selected Node workspace, Elixir test, and Swift package path fragments in mapper-produced validation commands and adds command-injection regression tests.

Reproducibility: yes. source-reproducible: current main runs mapper-produced validation command strings through shell: true and several builders interpolate repo-controlled strings. I did not execute an exploit in this read-only review.

PR rating
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Summary: The idea is important, but the patch is not quality-ready because same-class command-injection paths remain and real behavior proof is missing.

Rank-up moves:

• Quote Turbo filters and Nx project names, with regression coverage for both command paths.
• Add redacted terminal or log proof from a real throwaway project showing the generated validation commands do not execute metacharacters.

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

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
Needs real behavior proof before merge: Only check/test summaries are provided; before merge, add redacted terminal output or logs from a real throwaway repo showing injected names are kept literal, then update the PR body for re-review.

Risk before merge

• Turbo task-graph commands bypass scriptCommand and still interpolate attacker-controlled package-name or project-root filters into shell command strings.
• Nx target commands still interpolate project names from project.json or package.json into shell command strings.
• The PR body provides check/test summaries, but not real after-fix terminal or log proof from a throwaway project showing metacharacters remain literal.

Maintainer options:

1. Decide the mitigation before merge
   Quote or otherwise avoid shell interpretation for every mapper-built validation command fragment derived from repo data, especially Turbo filters and Nx project names, then merge with focused regression tests and redacted real terminal proof.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
Needs human/security review because the patch has blocking same-class gaps and missing real behavior proof; this is not a safe repair-lane candidate while contributor proof is required.

Security
Needs attention: The patch reduces shell injection exposure but leaves same-class Turbo and Nx validation command paths unquoted.

Review findings

• [P1] Quote Turbo filters before returning graph commands — src/mappers/turbo.ts:129-137
• [P1] Quote Nx project names in validation commands — src/mappers/projects.ts:1004-1009

Review details

Best possible solution:

Quote or otherwise avoid shell interpretation for every mapper-built validation command fragment derived from repo data, especially Turbo filters and Nx project names, then merge with focused regression tests and redacted real terminal proof.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: current main runs mapper-produced validation command strings through shell: true and several builders interpolate repo-controlled strings. I did not execute an exploit in this read-only review.

Is this the best way to solve the issue?

No, not yet: using shellQuotePath is a narrow fit for the touched paths, but the PR is incomplete while Turbo task-graph filters and Nx project names remain unquoted.

Label changes:

• add P0: This PR addresses a reachable command-injection path in validation commands run against untrusted repositories.
• add rating: 🧂 unranked krab: Current PR rating is 🧂 unranked krab because proof is 🧂 unranked krab, patch quality is 🧂 unranked krab, and The idea is important, but the patch is not quality-ready because same-class command-injection paths remain and real behavior proof is missing.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only check/test summaries are provided; before merge, add redacted terminal output or logs from a real throwaway repo showing injected names are kept literal, then update the PR body for re-review.

Label justifications:

• P0: This PR addresses a reachable command-injection path in validation commands run against untrusted repositories.
• rating: 🧂 unranked krab: Current PR rating is 🧂 unranked krab because proof is 🧂 unranked krab, patch quality is 🧂 unranked krab, and The idea is important, but the patch is not quality-ready because same-class command-injection paths remain and real behavior proof is missing.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only check/test summaries are provided; before merge, add redacted terminal output or logs from a real throwaway repo showing injected names are kept literal, then update the PR body for re-review.

Full review comments:

• [P1] Quote Turbo filters before returning graph commands — src/mappers/turbo.ts:129-137
projectTargetCommand returns task-graph commands before it reaches the newly quoted scriptCommand, but Turbo still builds --filter ${filter} from a package name or project root. A workspace using Turbo with $(...) in that repo-controlled value would still produce a shell-interpreted validation command, so this fix needs to quote the Turbo filter path/name too.
  Confidence: 0.9
• [P1] Quote Nx project names in validation commands — src/mappers/projects.ts:1004-1009
  Nx targets still go through nxCommand, which interpolates projectName from project.json or package.json directly into the shell command. A malicious project name can therefore keep the same command-injection path when validation selects an Nx-backed target, so quote that argument as part of this security fix.
  Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.86

Security concerns:

• [high] Turbo validation filters remain shell-interpreted — src/mappers/turbo.ts:129
  Turbo task-graph commands are selected before the newly quoted package script builder and still interpolate filters from package names or project roots into shell: true command strings.
  Confidence: 0.9
• [high] Nx project names remain shell-interpreted — src/mappers/projects.ts:1004
  Nx validation commands still interpolate repo-configured project names directly into command strings, preserving a command-injection path for Nx-backed targets.
  Confidence: 0.86

Acceptance criteria:

• pnpm test src/cmd-injection-regression.test.ts
• pnpm typecheck
• pnpm lint
• pnpm test

What I checked:

• Validation commands run through a shell: runCommandRaw executes command strings with spawn(command, { shell: true }), and applyPatchWithProvider runs each validation command string through runCommand. (src/exec.ts:26, 857d854ac8d0)
• Current main has unquoted mapper-built validation commands: On current main, scriptCommand directly interpolates package roots and script names; the same pattern is present in nodeScriptCommand, Elixir associated tests, and Swift package-prefix commands. (src/mappers/projects.ts:196, 857d854ac8d0)
• PR hardens selected paths: The PR commit imports shellQuotePath and applies it to the selected Node workspace, Elixir test path, and Swift package-root command fragments. (src/mappers/projects.ts:197, b2acb67122d5)
• Regression tests cover selected builders: The new test file checks scriptCommand and nodeScriptCommand with $(id) inputs, preserves ordinary command strings, and exercises a Swift detect-to-validation path with a malicious package directory. (src/cmd-injection-regression.test.ts:36, b2acb67122d5)
• Turbo task graph still bypasses the quoted script builder: At the PR head, projectTargetCommand returns task-graph commands before scriptCommand, and Turbo commands still interpolate --filter ${filter} where the filter comes from package name or project root. (src/mappers/turbo.ts:67, b2acb67122d5)
• Nx target commands still interpolate project names: At the PR head, Nx targets still call nxCommand, which interpolates projectName from repo configuration into shell command strings without quoting. (src/mappers/projects.ts:1004, b2acb67122d5)

Likely related people:

• Peter Steinberger: Current checkout blame for the affected mapper command builders points to the v0.4.0 release commit authored by Peter Steinberger. (role: introduced current mapper behavior; confidence: medium; commits: cdd58ac59213; files: src/mappers/projects.ts, src/mappers/shared.ts, src/mappers/elixir.ts)
• openclaw/openclaw-secops: CODEOWNERS assigns the security-sensitive mapper and execution paths to this team. (role: CODEOWNERS reviewer; confidence: high; commits: 857d854ac8d0; files: .github/CODEOWNERS, src/mappers/, src/exec.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 857d854ac8d0.

[clawsweeper]

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?

• The egg game starts only after the PR passes the real-behavior proof check.
• Before that, no creature or rarity is rolled. The treat waits for real proof.
• This is still just collectible flavor: proof affects review readiness, not creature quality.