Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/local-actions/branch-manager/main.js

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion ng-dev/release/publish/external-commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,12 @@ export abstract class ExternalCommands {
}

try {
// We must source nvm.sh so the shell recognizes the 'nvm' command since nvm is not a binary but a shell script.
// We must source nvm.sh so the shell recognizes the 'nvm' command since nvm is not a binary
// but a shell function. The dot (.) built-in and && operator require shell: true here.
await ChildProcess.spawn('. ~/.nvm/nvm.sh && nvm install', [], {
cwd: projectDir,
mode: 'on-error',
shell: true,
});

if (!quiet) {
Expand Down
22 changes: 15 additions & 7 deletions ng-dev/utils/child-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@ export interface CommonCmdOpts {

/** Interface describing the options for spawning a process synchronously. */
export interface SpawnSyncOptions
extends CommonCmdOpts, Omit<_SpawnSyncOptions, 'shell' | 'stdio' | 'input'> {}
extends CommonCmdOpts, Omit<_SpawnSyncOptions, 'stdio' | 'input'> {}

/** Interface describing the options for spawning a process. */
export interface SpawnOptions extends CommonCmdOpts, Omit<_SpawnOptions, 'shell' | 'stdio'> {}
export interface SpawnOptions extends CommonCmdOpts, Omit<_SpawnOptions, 'stdio'> {}

/** Interface describing the options for exec-ing a process. */
export interface ExecOptions extends CommonCmdOpts, Omit<_ExecOptions, 'shell' | 'stdio'> {}
export interface ExecOptions extends CommonCmdOpts, Omit<_ExecOptions, 'stdio'> {}

/** Interface describing the options for spawning an interactive process. */
export interface SpawnInteractiveCommandOptions extends Omit<_SpawnOptions, 'shell' | 'stdio'> {}
export interface SpawnInteractiveCommandOptions extends Omit<_SpawnOptions, 'stdio'> {}

/** Interface describing the result of a spawned process. */
export interface SpawnResult {
Expand Down Expand Up @@ -71,7 +71,7 @@ export abstract class ChildProcess {
return new Promise<void>((resolve, reject) => {
const commandText = `${command} ${args.join(' ')}`;
Log.debug(`Executing command: ${commandText}`);
const childProcess = _spawn(command, args, {...options, shell: true, stdio: 'inherit'});
const childProcess = _spawn(command, args, {...options, stdio: 'inherit'});
// The `close` event is used because the process is guaranteed to have completed writing to
// stdout and stderr, using the `exit` event can cause inconsistent information in stdout and
// stderr due to a race condition around exiting.
Expand All @@ -85,6 +85,10 @@ export abstract class ChildProcess {
* @returns The command's stdout and stderr.
*/
static spawnSync(command: string, args: string[], options: SpawnSyncOptions = {}): SpawnResult {
// Default shell to false to prevent OS command injection: with shell: true, Node.js
// internally joins command + args into a single string evaluated by /bin/sh, making
// shell metacharacters in args exploitable. Callers that genuinely require shell
// features (e.g. sourcing shell scripts) may explicitly pass shell: true.
const commandText = `${command} ${args.join(' ')}`;
const env = getEnvironmentForNonInteractiveCommand(options.env);

Expand All @@ -95,7 +99,7 @@ export abstract class ChildProcess {
signal,
stdout,
stderr,
} = _spawnSync(command, args, {...options, env, encoding: 'utf8', shell: true, stdio: 'pipe'});
} = _spawnSync(command, args, {...options, env, encoding: 'utf8', stdio: 'pipe'});

/** The status of the spawn result. */
const status = statusFromExitCodeAndSignal(exitCode, signal);
Expand All @@ -116,13 +120,17 @@ export abstract class ChildProcess {
* rejects on command failure.
*/
static spawn(command: string, args: string[], options: SpawnOptions = {}): Promise<SpawnResult> {
// Default shell to false to prevent OS command injection: with shell: true, Node.js
// internally joins command + args into a single string evaluated by /bin/sh, making
// shell metacharacters in args exploitable. Callers that genuinely require shell
// features (e.g. sourcing shell scripts) may explicitly pass shell: true.
const commandText = `${command} ${args.join(' ')}`;
const env = getEnvironmentForNonInteractiveCommand(options.env);

return processAsyncCmd(
commandText,
options,
_spawn(command, args, {...options, env, shell: true, stdio: 'pipe'}),
_spawn(command, args, {...options, env, stdio: 'pipe'}),
);
}

Expand Down
2 changes: 1 addition & 1 deletion ng-dev/utils/repo-directory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {ChildProcess} from './child-process.js';

/** Determines the repository base directory from the current working directory. */
export function determineRepoBaseDirFromCwd() {
const {stdout, stderr, status} = ChildProcess.spawnSync('git', ['rev-parse --show-toplevel']);
const {stdout, stderr, status} = ChildProcess.spawnSync('git', ['rev-parse', '--show-toplevel']);
if (status !== 0) {
throw Error(
`Unable to find the path to the base directory of the repository.\n` +
Expand Down
Loading