fix(cli): mitigate CWE-78 command injection on Windows

Author: g0w6y

packages/angular/cli/src/package-managers/host.ts

@@ -131,7 +131,7 @@ export const NodeJS_HOST: Host = {
 
     return new Promise((resolve, reject) => {
       const spawnOptions = {
-        shell: isWin32,
+        shell: false,
         stdio: options.stdio ?? 'pipe',
         signal,
         cwd: options.cwd,
@@ -144,10 +144,14 @@ export const NodeJS_HOST: Host = {
         },
       } satisfies SpawnOptions;
 
-      // FIXED: Use safe array form to prevent OS Command Injection (CWE-78)
-      // Previously used unsafe string concatenation on Windows (`shell: true` + args.join)
-      // Now always uses safe array form while preserving shell behavior
-      const childProcess = spawn(command, args, spawnOptions);
+      // FIXED (CWE-78): On Windows npm/yarn/pnpm are .cmd scripts that need a shell.
+      // spawn(cmd, args, {shell:true}) has Node.js join the args internally — the
+      // previous PR only removed the explicit join; injection still existed.
+      // Correct fix: invoke cmd.exe directly with shell:false. cmd.exe is a real
+      // .exe so no shell wrapper is needed. Args as array = no metachar injection.
+      const childProcess = isWin32
+        ? spawn('cmd.exe', ['/d', '/s', '/c', command, ...args], spawnOptions)
+        : spawn(command, args, spawnOptions);
 
       let stdout = '';
       childProcess.stdout?.on('data', (data) => (stdout += data.toString()));

packages/angular_devkit/build_angular/src/builders/ssr-dev-server/index.ts

@@ -238,7 +238,8 @@ function startNodeServer(
 
   return of(null).pipe(
     delay(0), // Avoid EADDRINUSE error since it will cause the kill event to be finish.
-    switchMap(() => spawnAsObservable('node', args, { env, shell: true })),
+    // shell:true removed — spawnAsObservable handles Windows safely via cmd.exe
+    switchMap(() => spawnAsObservable('node', args, { env })),
     tap((res) => log({ stderr: res.stderr, stdout: res.stdout }, logger)),
     ignoreElements(),
     // Emit a signal after the process has been started

packages/angular_devkit/build_angular/src/builders/ssr-dev-server/utils.ts

@@ -7,6 +7,7 @@
  */
 
 import { SpawnOptions, spawn } from 'node:child_process';
+import { platform } from 'node:os';
 import { AddressInfo, createConnection, createServer } from 'node:net';
 import { Observable, mergeMap, retryWhen, throwError, timer } from 'rxjs';
 
@@ -29,7 +30,13 @@ export function spawnAsObservable(
   options: SpawnOptions = {},
 ): Observable<{ stdout?: string; stderr?: string }> {
   return new Observable((obs) => {
-    const proc = spawn(`${command} ${args.join(' ')}`, options);
+    // FIXED (CWE-78): raw args.join + shell:true allows injection via
+    // a crafted outputPath/serverScript value in angular.json.
+    // Invoke cmd.exe directly on Windows (shell:false) — no escaping needed.
+    const isWin32 = platform() === 'win32';
+    const proc = isWin32
+      ? spawn('cmd.exe', ['/d', '/s', '/c', command, ...args], { ...options, shell: false })
+      : spawn(command, args, { ...options, shell: false });
     if (proc.stdout) {
       proc.stdout.on('data', (data) => obs.next({ stdout: data.toString() }));
     }

packages/angular_devkit/schematics/tasks/package-manager/executor.ts

@@ -17,10 +17,19 @@ import { SpawnOptions, spawn } from 'node:child_process';
  * Algorithm: https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/
  */
 function escapeArgForWindowsShell(arg: string): string {
-  // Fast path: only safe characters present
-  if (arg === '' || /^[a-zA-Z0-9_\-./\\:@]+$/.test(arg)) {
+  // FIX A: empty string must produce a quoted empty token.
+  // Returning bare '' causes cmd.exe to drop the arg silently,
+  // shifting all subsequent args by one (argument confusion).
+  if (!arg) return '""';
+  // Fast path: only shell-safe chars, no quoting needed
+  if (/^[a-zA-Z0-9_\-./\\:@]+$/.test(arg)) {
     return arg;
   }
+  // FIX B: escape % BEFORE wrapping in double-quotes.
+  // cmd.exe expands %VAR% before evaluating quote context, so
+  // %COMSPEC% inside "..." still expands (BatBadBut / CVE-2024-3566).
+  // %% disables expansion and becomes a literal percent sign.
+  arg = arg.replace(/%/g, '%%');
   let result = '"';
   let slashes = 0;
   for (const char of arg) {

packages/schematics/angular/workspace/index.ts

@@ -25,7 +25,15 @@ export default function (options: WorkspaceOptions): Rule {
     const packageManager = options.packageManager;
     let packageManagerWithVersion: string | undefined;
 
+    const ALLOWED_PKG_MANAGERS = new Set(['npm', 'yarn', 'pnpm', 'bun', 'cnpm']);
     if (packageManager) {
+      // FIXED (CWE-78): packageManager is user-supplied with no runtime enum
+      // validation (SCAN C = zero hits). Enforce an allowlist before execSync.
+      if (!ALLOWED_PKG_MANAGERS.has(packageManager)) {
+        throw new Error(
+          `Invalid packageManager: "${packageManager}". Allowed: npm, yarn, pnpm, bun, cnpm`,
+        );
+      }
       let packageManagerVersion: string | undefined;
       try {
         packageManagerVersion = execSync(`${packageManager} --version`, {