fix: prevent command injection via shell metacharacters on Windows

[VenkatKwest]

Description

This PR fixes command injection vulnerabilities on Windows in two files:

1. executor.ts (git spawn)

• Removed shell: true — git is a native .exe and doesn't need it
• Switched from spawn(\git ${args.join(' ')}`)tospawn('git', args)`
• Separated -m flag from commit message: ['commit', '-m', message] instead of ['commit', \-m "${message}"`]`

2. host.ts (package manager spawn)

• Package managers (npm/yarn/pnpm) are .cmd scripts on Windows and need a shell
• Instead of using shell: true with unsanitized argument concatenation, we now invoke cmd.exe /d /s /c directly with properly escaped arguments
• Escape logic is based on cross-spawn's battle-tested approach, inlined to avoid adding a dependency
• Uses windowsVerbatimArguments: true to prevent Node.js from re-escaping

Testing

Verified on Windows 11 (Node v25.6.0):

• npm/git commands work correctly with the new approach
• Injection payloads (& echo INJECTED, & echo PWNED > file) are blocked
• Old shell: true pattern confirmed vulnerable (control test)

Fixes #32509

[gemini-code-assist]

Code Review

This pull request addresses command injection vulnerabilities on Windows by removing shell: true where it's not needed and by properly escaping arguments when a shell is required.

The changes in packages/angular_devkit/schematics/tasks/repo-init/executor.ts for git commands are correct, switching to spawn with an argument array instead of a concatenated string.

The changes in packages/angular/cli/src/package-managers/host.ts for package manager commands correctly identify the need for a shell on Windows but replace the insecure shell: true with a direct call to cmd.exe. However, I've found an issue in the custom argument escaping logic. The regular expressions for escaping backslashes are different from the cross-spawn library they are based on, and one of them appears to be buggy, which could lead to incorrect escaping. I've left a comment with a suggested fix to align the implementation with cross-spawn's battle-tested approach.

_{Note: Security Review is unavailable for this PR.}

[alan-agius4]

I had a chat with @clydin and actually this is not a vulnerability at all as the commit message cannot be passed via the command line and thus command injection cannot be triggered in this case.

Hence, kindly revert all the changes host.ts, can leave the cleanup in the executor.ts

[VenkatKwest]

Thanks for the review, @alan-agius4! I've addressed all your feedback:

1. **Removed the redundant arg = \${arg}`** — since the parameter is already typed as string`.
2. Added proper attribution with source links — the docblock now clearly states it's adapted from cross-spawn's lib/util/escape.js and links to the authoritative Microsoft documentation for the escaping algorithm it uses.
3. Refactored to avoid multiple re-assignments — using a chained .replace() approach as you suggested.

[VenkatKwest]

bandicam.2026-03-05.11-17-49-918.mp4

bandicam.2026-03-05.11-23-12-736.mp4

Thanks @alan-agius4 for the review and for checking with @clydin. I appreciate the time you both took.

You're correct about executor.ts — I verified that --commit.message is rejected by the CLI's yargs parser (Unknown argument: commit.message), and --commit is only registered as a boolean. The commit message in executor.ts is not user-controlled via the CLI. I'm happy to keep that as a defense-in-depth cleanup only.

However, I respectfully disagree about host.ts. The host.ts vulnerability is real and exploitable. I have two confirmed PoCs where calc.exe opens on Windows:

---

PoC 1: Package specifier injection via ng add

ng add "lodash@1.0.0 & calc" --skip-confirmation

Result: calc.exe opens. The injected command executes silently alongside npm install.

Why it works — full stack trace:

IMPORTANT NOTE: STACK TRACE MIGHT BE WRONG BUT ABLE TO REPRODUCE THE VULN

1. User runs: ng add "lodash@1.0.0 & calc" --skip-confirmation

2. add/cli.ts:164 → npa("lodash@1.0.0 & calc")
   └─ npa splits at '@' → name="lodash", spec="1.0.0 & calc"
   └─ npa.js:462 → semver.validRange("1.0.0 & calc") returns truthy
   └─ npa classifies as type: "range"
      (BYPASSES the encodeURIComponent check on line 468,
       which only runs for type "tag")
   └─ Returns: { name: "lodash", type: "range", fetchSpec: "1.0.0 & calc" }

3. add/cli.ts → loadPackageInfoTask()
   └─ packageManager.getManifest(packageIdentifier)
   └─ package-manager.ts:494 → case 'range': getRegistryMetadata("lodash")
   └─ Fetches metadata, finds version "1.0.0" satisfies range
   └─ Returns manifest for lodash@1.0.0 ✓

4. add/cli.ts → installPackageTask()
   └─ packageManager.add("lodash@1.0.0 & calc", ...)
   └─ package-manager.ts:326 → args = ["add", "lodash@1.0.0 & calc", "--save-exact"]
   └─ package-manager.ts:327 → this.#run(args, { registry })

5. package-manager.ts:196 → host.runCommand("npm", ["add", "lodash@1.0.0 & calc", ...])

The key bypass is in npm-package-arg (npa). When a user provides lodash@1.0.0 & calc, npa splits at @ and passes 1.0.0 & calc to semver.validRange(). Semver is permissive — it ignores the trailing garbage & calc and returns a valid range. Because npa classifies this as type "range" (not "tag"), the encodeURIComponent validation on npa.js line 468 is never executed. The unsanitized & calc flows all the way to host.ts.

---

PoC 2: Registry flag injection via ng add --registry

ng add @angular/localize --registry "http://registry.npmjs.org & calc" --skip-confirmation

Result: calc.exe opens 3 times (once per npm command invoked during install).

Why it works — full stack trace:

1. User runs: ng add @angular/localize --registry "http://registry.npmjs.org & calc"

2. add/cli.ts:118 → yargs parses --registry as a string option (no validation)
   └─ options.registry = "http://registry.npmjs.org & calc"

3. add/cli.ts:555 → installPackageTask() destructures { registry } from options
   └─ add/cli.ts:583-590 → packageManager.add(packageIdentifier, ..., { registry })
   └─ package-manager.ts:327 → this.#run(args, options)  (options includes { registry })
   └─ package-manager.ts:161 → #run(args, { registry: "http://registry.npmjs.org & calc" })

4. package-manager.ts:172 → descriptor.getRegistryOptions(registry)
   └─ Returns: { args: ["--registry", "http://registry.npmjs.org & calc"] }
   └─ package-manager.ts:180 → finalArgs.push("--registry", "http://registry.npmjs.org & calc")

5. package-manager.ts:196 → host.runCommand("npm", [..., "--registry", "http://registry.npmjs.org & calc"])

6. host.ts:182 → shellCommand = "npm view ... --registry http://registry.npmjs.org & calc"

---

The host.ts fix is a critical security fix, not just a cleanup. I agree that executor.ts is defense-in-depth only.

Tested on: Angular CLI 21.1.4, Node.js v25.6.0, Windows 11, npm 11.3.0

Happy to adjust the PR scope based on this. Let me know how you'd like to proceed.

[alan-agius4]

This PR was merged into the repository. The changes were merged into the following branches:

• main: f1ed025
• 21.2.x: 7d3844a