Skip to content
13 changes: 10 additions & 3 deletions packages/angular/cli/src/package-managers/host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[],
Expand All @@ -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,
Expand All @@ -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 = '';
Expand Down Expand Up @@ -173,4 +180,4 @@ export const NodeJS_HOST: Host = {
});
});
},
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<number> {
Expand All @@ -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() }));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -113,7 +114,7 @@ export default function (
}

if (factoryOptions.registry) {
args.push(`--registry="${factoryOptions.registry}"`);
args.push('--registry', factoryOptions.registry);
}

if (factoryOptions.force) {
Expand All @@ -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) {
Expand Down
8 changes: 8 additions & 0 deletions packages/schematics/angular/workspace/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`, {
Expand Down
Loading