diff --git a/packages/angular/cli/src/package-managers/host.ts b/packages/angular/cli/src/package-managers/host.ts index cee68015f677..e2cb6a51cbf8 100644 --- a/packages/angular/cli/src/package-managers/host.ts +++ b/packages/angular/cli/src/package-managers/host.ts @@ -115,6 +115,7 @@ export const NodeJS_HOST: Host = { createTempDirectory: (baseDir?: string) => mkdtemp(join(baseDir ?? tmpdir(), 'angular-cli-tmp-packages-')), deleteDirectory: (path: string) => rm(path, { recursive: true, force: true }), + runCommand: async ( command: string, args: readonly string[], @@ -130,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, @@ -142,8 +143,14 @@ export const NodeJS_HOST: Host = { NPM_CONFIG_UPDATE_NOTIFIER: 'false', }, } satisfies 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(`${command} ${args.join(' ')}`, spawnOptions) + ? spawn('cmd.exe', ['/d', '/s', '/c', command, ...args], spawnOptions) : spawn(command, args, spawnOptions); let stdout = ''; @@ -173,4 +180,4 @@ export const NodeJS_HOST: Host = { }); }); }, -}; +}; \ No newline at end of file diff --git a/packages/angular_devkit/build_angular/src/builders/ssr-dev-server/index.ts b/packages/angular_devkit/build_angular/src/builders/ssr-dev-server/index.ts index e919f188e0bf..eb20ae99a932 100644 --- a/packages/angular_devkit/build_angular/src/builders/ssr-dev-server/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/ssr-dev-server/index.ts @@ -231,14 +231,15 @@ function startNodeServer( const path = join(outputPath, 'main.js'); const env = { ...process.env, PORT: '' + port, NG_ALLOWED_HOSTS: host ?? 'localhost' }; - const args = ['--enable-source-maps', `"${path}"`]; + const args = ['--enable-source-maps', path]; if (inspectMode) { args.unshift('--inspect-brk'); } 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 diff --git a/packages/angular_devkit/build_angular/src/builders/ssr-dev-server/utils.ts b/packages/angular_devkit/build_angular/src/builders/ssr-dev-server/utils.ts index 059c0e0a89e9..40b84a5a1bfd 100644 --- a/packages/angular_devkit/build_angular/src/builders/ssr-dev-server/utils.ts +++ b/packages/angular_devkit/build_angular/src/builders/ssr-dev-server/utils.ts @@ -8,6 +8,7 @@ import { SpawnOptions, spawn } from 'node:child_process'; import { AddressInfo, createConnection, createServer } from 'node:net'; +import { platform } from 'node:os'; import { Observable, mergeMap, retryWhen, throwError, timer } from 'rxjs'; export function getAvailablePort(): Promise { @@ -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() })); } diff --git a/packages/angular_devkit/schematics/tasks/package-manager/executor.ts b/packages/angular_devkit/schematics/tasks/package-manager/executor.ts index dc97a4e78277..27bf3511c57e 100644 --- a/packages/angular_devkit/schematics/tasks/package-manager/executor.ts +++ b/packages/angular_devkit/schematics/tasks/package-manager/executor.ts @@ -8,6 +8,7 @@ import { BaseException } from '@angular-devkit/core'; import { SpawnOptions, spawn } from 'node:child_process'; + import * as path from 'node:path'; import ora from 'ora'; import { TaskExecutor, UnsuccessfulWorkflowExecution } from '../../src'; @@ -77,7 +78,7 @@ export default function ( const bufferedOutput: { stream: NodeJS.WriteStream; data: Buffer }[] = []; const spawnOptions: SpawnOptions = { - shell: true, + shell: false, cwd: path.join(rootDirectory, options.workingDirectory || ''), }; if (options.hideOutput) { @@ -113,7 +114,7 @@ export default function ( } if (factoryOptions.registry) { - args.push(`--registry="${factoryOptions.registry}"`); + args.push('--registry', factoryOptions.registry); } if (factoryOptions.force) { @@ -126,7 +127,22 @@ export default function ( // Workaround for https://github.com/sindresorhus/ora/issues/136. discardStdin: process.platform != 'win32', }).start(); - const childProcess = spawn(`${taskPackageManagerName} ${args.join(' ')}`, spawnOptions).on( + // SECURITY FIX (CWE-78): never concatenate args as a raw shell string. + // On Windows, package managers are .cmd scripts requiring a shell, but + // instead of shell:true + string concat (injection vector), we invoke + // cmd.exe directly with shell:false and pass each arg as an array element. + // Node.js then controls quoting — metacharacters in args are never + // interpreted by cmd.exe as shell operators. + const isWin32 = process.platform === 'win32'; + const childProcess = ( + isWin32 + ? spawn( + 'cmd.exe', + ['/d', '/s', '/c', taskPackageManagerName, ...args], + { ...spawnOptions, shell: false }, + ) + : spawn(taskPackageManagerName, args, { ...spawnOptions, shell: false }) + ).on( 'close', (code: number) => { if (code === 0) { diff --git a/packages/schematics/angular/workspace/index.ts b/packages/schematics/angular/workspace/index.ts index 85205042f3e3..6a11a228f58a 100644 --- a/packages/schematics/angular/workspace/index.ts +++ b/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`, {