From 995f8d0f59a3ff9a95c878ee48410e27dda4b77a Mon Sep 17 00:00:00 2001 From: Mike DelGaudio <134981291+mdelgaudio_microsoft@users.noreply.github.com> Date: Fri, 18 Sep 2026 20:44:31 +0000 Subject: [PATCH 1/2] Fix pnpm credential environment on POSIX Bypass the shell for package-manager invocations when the npmrc credential environment experiment is active. Preserve Windows command shims and default shell behavior for other callers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88399e18-c0da-4cfc-af47-6673b1cc1545 --- .../src/cli/RushPnpmCommandLineParser.ts | 1 + libraries/rush-lib/src/logic/Autoinstaller.ts | 6 +- .../installManager/RushInstallManager.ts | 9 +- .../installManager/WorkspaceInstallManager.ts | 6 +- libraries/rush-lib/src/utilities/Utilities.ts | 48 ++++-- .../src/utilities/test/Utilities.test.ts | 151 ++++++++++++++++++ 6 files changed, 196 insertions(+), 25 deletions(-) diff --git a/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts b/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts index 30dda7645a7..74a3cdaaa13 100644 --- a/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts +++ b/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts @@ -513,6 +513,7 @@ export class RushPnpmCommandLineParser { workingDirectory: process.cwd(), environment: pnpmEnvironmentMap.toObject(), keepEnvironment: true, + useShell: !InstallHelpers.shouldProvideNpmrcCredentialsViaEnvironment(rushConfiguration), onStdoutStreamChunk, captureExitCodeAndSignal: true }); diff --git a/libraries/rush-lib/src/logic/Autoinstaller.ts b/libraries/rush-lib/src/logic/Autoinstaller.ts index 9a38c7edeeb..6db745b78c8 100644 --- a/libraries/rush-lib/src/logic/Autoinstaller.ts +++ b/libraries/rush-lib/src/logic/Autoinstaller.ts @@ -159,7 +159,8 @@ export class Autoinstaller { args: ['install', '--frozen-lockfile'], workingDirectory: autoinstallerFullPath, environment: this.#getPackageManagerEnvironment(autoinstallerFullPath), - keepEnvironment: true + keepEnvironment: true, + useShell: !InstallHelpers.shouldProvideNpmrcCredentialsViaEnvironment(this.#rushConfiguration) }); // Create file: ../common/autoinstallers/my-task/.rush/temp/last-install.flag @@ -245,7 +246,8 @@ export class Autoinstaller { args: ['install'], workingDirectory: this.folderFullPath, environment: this.#getPackageManagerEnvironment(this.folderFullPath), - keepEnvironment: true + keepEnvironment: true, + useShell: !InstallHelpers.shouldProvideNpmrcCredentialsViaEnvironment(this.#rushConfiguration) }); this.#logIfConsoleOutputIsNotRestricted(); diff --git a/libraries/rush-lib/src/logic/installManager/RushInstallManager.ts b/libraries/rush-lib/src/logic/installManager/RushInstallManager.ts index 69bc81e703e..032d6d8fc36 100644 --- a/libraries/rush-lib/src/logic/installManager/RushInstallManager.ts +++ b/libraries/rush-lib/src/logic/installManager/RushInstallManager.ts @@ -504,8 +504,9 @@ export class RushInstallManager extends BaseInstallManager { this.rushConfiguration, { ...this.options, npmrcFolder: subspace.getSubspaceTempFolderPath() } ); - const keepEnvironment: boolean = - InstallHelpers.shouldProvideNpmrcCredentialsViaEnvironment(this.rushConfiguration); + const keepEnvironment: boolean = InstallHelpers.shouldProvideNpmrcCredentialsViaEnvironment( + this.rushConfiguration + ); const commonNodeModulesFolder: string = path.join( this.rushConfiguration.commonTempFolder, @@ -560,8 +561,7 @@ export class RushInstallManager extends BaseInstallManager { // eslint-disable-next-line no-console console.log(`Deleting ${pathToDeleteWithoutStar}\\*`); // Glob can't handle Windows paths - const normalizedPathToDeleteWithoutStar: string = - Path.convertToSlashes(pathToDeleteWithoutStar); + const normalizedPathToDeleteWithoutStar: string = Path.convertToSlashes(pathToDeleteWithoutStar); const { default: glob } = await import('fast-glob'); const tempModulePaths: string[] = await glob( @@ -625,6 +625,7 @@ export class RushInstallManager extends BaseInstallManager { workingDirectory: this.rushConfiguration.commonTempFolder, environment: packageManagerEnv, keepEnvironment, + useShell: !keepEnvironment, suppressOutput: false }, this.options.maxInstallAttempts, diff --git a/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts b/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts index b58bff78507..b3bb82a1f65 100644 --- a/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts +++ b/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts @@ -496,8 +496,9 @@ export class WorkspaceInstallManager extends BaseInstallManager { this.rushConfiguration, { ...this.options, npmrcFolder: subspace.getSubspaceTempFolderPath() } ); - const keepEnvironment: boolean = - InstallHelpers.shouldProvideNpmrcCredentialsViaEnvironment(this.rushConfiguration); + const keepEnvironment: boolean = InstallHelpers.shouldProvideNpmrcCredentialsViaEnvironment( + this.rushConfiguration + ); if (ConsoleTerminalProvider.supportsColor) { packageManagerEnv.FORCE_COLOR = '1'; } @@ -599,6 +600,7 @@ export class WorkspaceInstallManager extends BaseInstallManager { workingDirectory: subspace.getSubspaceTempFolderPath(), environment: packageManagerEnv, keepEnvironment, + useShell: !keepEnvironment, suppressOutput: false, onStdoutStreamChunk: onPnpmStdoutChunk }, diff --git a/libraries/rush-lib/src/utilities/Utilities.ts b/libraries/rush-lib/src/utilities/Utilities.ts index 2a5746672a7..4efdefd5d79 100644 --- a/libraries/rush-lib/src/utilities/Utilities.ts +++ b/libraries/rush-lib/src/utilities/Utilities.ts @@ -47,6 +47,11 @@ export interface IExecuteCommandOptions { environment?: IEnvironment; suppressOutput?: boolean; keepEnvironment?: boolean; + /** + * Whether to use a shell on POSIX. Defaults to true. + * Windows always uses a shell to support package manager .cmd shims. + */ + useShell?: boolean; /** * Note that this takes precedence over {@link IExecuteCommandOptions.suppressOutput} */ @@ -369,6 +374,7 @@ export class Utilities { onStdoutStreamChunk, environment, keepEnvironment, + useShell, captureExitCodeAndSignal } = options; const { exitCode, signal } = await _executeCommandInternalAsync({ @@ -389,6 +395,7 @@ export class Utilities { ['inherit', 'inherit', 'inherit'], environment, keepEnvironment, + useShell, onStdoutStreamChunk, captureOutput: false, captureExitCodeAndSignal @@ -851,13 +858,14 @@ async function _executeCommandInternalAsync({ stdio, environment, keepEnvironment, + useShell = true, onStdoutStreamChunk, captureOutput, captureExitCodeAndSignal }: IExecuteCommandInternalOptions): Promise | IWaitForExitResultWithoutOutput> { const spawnOptions: child_process.SpawnSyncOptions = { cwd: workingDirectory, - shell: true, + shell: IS_WINDOWS || useShell, stdio: stdio, env: keepEnvironment ? environment @@ -865,25 +873,31 @@ async function _executeCommandInternalAsync({ maxBuffer: 10 * 1024 * 1024 // Set default max buffer size to 10MB }; - // This is needed since we specify shell=true below. - // NOTE: On Windows if we escape "NPM", the spawnSync() function runs something like this: - // [ 'C:\\Windows\\system32\\cmd.exe', '/s', '/c', '""NPM" "install""' ] - // - // Due to a bug with Windows cmd.exe, the npm.cmd batch file's "%~dp0" variable will - // return the current working directory instead of the batch file's directory. - // The workaround is to not escape, npm, i.e. do this instead: - // [ 'C:\\Windows\\system32\\cmd.exe', '/s', '/c', '"npm "install""' ] - // - // We will come up with a better solution for this when we promote executeCommand() - // into node-core-library, but for now this hack will unblock people: + let childProcess: child_process.ChildProcess; + if (!spawnOptions.shell) { + // POSIX shells can discard URL-scoped npm_config_* credential variables. + childProcess = child_process.spawn(command, args, spawnOptions); + } else { + // This is needed since we specify shell=true below. + // NOTE: On Windows if we escape "NPM", the spawnSync() function runs something like this: + // [ 'C:\\Windows\\system32\\cmd.exe', '/s', '/c', '""NPM" "install""' ] + // + // Due to a bug with Windows cmd.exe, the npm.cmd batch file's "%~dp0" variable will + // return the current working directory instead of the batch file's directory. + // The workaround is to not escape, npm, i.e. do this instead: + // [ 'C:\\Windows\\system32\\cmd.exe', '/s', '/c', '"npm "install""' ] + // + // We will come up with a better solution for this when we promote executeCommand() + // into node-core-library, but for now this hack will unblock people: - // Only escape the command if it actually contains spaces: - const escapedCommand: string = escapeArgumentIfNeeded(command); + // Only escape the command if it actually contains spaces: + const escapedCommand: string = escapeArgumentIfNeeded(command); - const escapedArgs: string[] = args.map((x) => escapeArgumentIfNeeded(x)); - const shellCommand: string = [escapedCommand, ...escapedArgs].join(' '); + const escapedArgs: string[] = args.map((x) => escapeArgumentIfNeeded(x)); + const shellCommand: string = [escapedCommand, ...escapedArgs].join(' '); - const childProcess: child_process.ChildProcess = child_process.spawn(shellCommand, spawnOptions); + childProcess = child_process.spawn(shellCommand, spawnOptions); + } if (onStdoutStreamChunk) { const inspectStream: Transform = new Transform({ diff --git a/libraries/rush-lib/src/utilities/test/Utilities.test.ts b/libraries/rush-lib/src/utilities/test/Utilities.test.ts index cd3e79c85d0..a7910010338 100644 --- a/libraries/rush-lib/src/utilities/test/Utilities.test.ts +++ b/libraries/rush-lib/src/utilities/test/Utilities.test.ts @@ -1,7 +1,12 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import * as fs from 'node:fs'; +import * as os from 'node:os'; + import { type IDisposable, Utilities } from '../Utilities'; +import { getNpmrcEnvironmentVariables, syncNpmrc } from '../npmrcUtilities'; +import { IS_WINDOWS } from '../executionUtilities'; function withComSpec(value: string | undefined, callback: () => T): T { const originalValue: string | undefined = process.env.comspec; @@ -23,6 +28,152 @@ function withComSpec(value: string | undefined, callback: () => T): T { } describe(Utilities.name, () => { + describe('package manager credential environment', () => { + const credentialKey: string = 'npm_config_//registry.example.test/npm/:_authToken'; + const credentialValue: string = 'non-secret-test-token'; + let directory: string; + let scriptPath: string; + let environment: NodeJS.ProcessEnv; + + beforeAll(async () => { + directory = await fs.promises.mkdtemp(`${os.tmpdir()}/rush credentials `); + scriptPath = `${directory}/check credentials.cjs`; + const sourceFolder: string = `${directory}/source`; + const targetFolder: string = `${directory}/target`; + await fs.promises.mkdir(sourceFolder); + await fs.promises.mkdir(targetFolder); + await fs.promises.writeFile( + `${sourceFolder}/.npmrc`, + '//registry.example.test/npm/:_authToken=${RUSH_TEST_TOKEN}\n' + ); + const sourceEnvironment: NodeJS.ProcessEnv = { RUSH_TEST_TOKEN: credentialValue }; + syncNpmrc({ + sourceNpmrcFolder: sourceFolder, + targetNpmrcFolder: targetFolder, + supportEnvVarFallbackSyntax: true, + moveSensitiveSettingsToEnvironment: true, + env: sourceEnvironment + }); + environment = { + ...process.env, + ...getNpmrcEnvironmentVariables({ + npmrcFolder: targetFolder, + supportEnvVarFallbackSyntax: true, + env: sourceEnvironment + }) + }; + await fs.promises.writeFile( + scriptPath, + [ + `if (process.env[${JSON.stringify(credentialKey)}] !== ${JSON.stringify(credentialValue)}) {`, + ' process.exit(42);', + '}', + 'process.stdout.write(JSON.stringify(process.argv.slice(2)));' + ].join('\n') + ); + expect(await fs.promises.readFile(`${targetFolder}/.npmrc`, 'utf8')).not.toContain(credentialValue); + }); + + afterAll(async () => { + if (directory) { + await fs.promises.rm(directory, { recursive: true, force: true }); + } + }); + + it('preserves generated credentials through the captured subprocess path', async () => { + const output: string = await Utilities.executeCommandAndCaptureOutputAsync({ + command: process.execPath, + args: [scriptPath, 'space argument'], + workingDirectory: directory, + environment, + keepEnvironment: true, + useShell: false + }); + expect(JSON.parse(output)).toEqual(['space argument']); + }); + + it('preserves generated credentials through the install retry path', async () => { + await Utilities.executeCommandWithRetryAsync( + { + command: process.execPath, + args: [scriptPath], + workingDirectory: directory, + environment, + keepEnvironment: true, + useShell: false, + suppressOutput: true + }, + 1 + ); + }); + + (IS_WINDOWS ? it.skip : it)( + 'passes POSIX arguments without shell expansion or pre-escaping', + async () => { + const args: string[] = [ + '', + 'two words', + '"quoted"', + "single'quote", + '$HOME', + '$(echo expanded)', + '*' + ]; + const output: string = await Utilities.executeCommandAndCaptureOutputAsync({ + command: process.execPath, + args: [scriptPath, ...args], + workingDirectory: directory, + environment, + keepEnvironment: true, + useShell: false + }); + expect(JSON.parse(output)).toEqual(args); + } + ); + + it('retains exit-code capture for failed direct subprocesses', async () => { + const { exitCode } = await Utilities.executeCommandAsync({ + command: process.execPath, + args: [scriptPath], + workingDirectory: directory, + environment: { ...environment, [credentialKey]: 'wrong-test-token' }, + keepEnvironment: true, + useShell: false, + captureExitCodeAndSignal: true, + suppressOutput: true + }); + expect(exitCode).toBe(42); + }); + + it('still rejects failed direct subprocesses by default', async () => { + await expect( + Utilities.executeCommandAsync({ + command: process.execPath, + args: [scriptPath], + workingDirectory: directory, + environment: { ...environment, [credentialKey]: 'wrong-test-token' }, + keepEnvironment: true, + useShell: false, + suppressOutput: true + }) + ).rejects.toThrow(); + }); + + it('retains shell execution by default', async () => { + const output: string = await Utilities.executeCommandAndCaptureOutputAsync({ + command: 'echo', + args: ['first', '&&', 'echo', 'second'], + workingDirectory: directory + }); + expect( + output + .trim() + .split(/\r?\n/) + .map((line) => line.trim()) + ).toEqual(['first', 'second']); + }); + }); + describe(Utilities.usingAsync.name, () => { let disposed: boolean; From 8cd1436d8d1bde074fbe6b0308d01e9cd1671de0 Mon Sep 17 00:00:00 2001 From: Mike DelGaudio <134981291+mdelgaudio_microsoft@users.noreply.github.com> Date: Fri, 18 Sep 2026 20:46:15 +0000 Subject: [PATCH 2/2] Add Rush credential fix release note Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88399e18-c0da-4cfc-af47-6673b1cc1545 --- ...x-pnpm-posix-credential-environment_2026-09-18.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@microsoft/rush/fix-pnpm-posix-credential-environment_2026-09-18.json diff --git a/common/changes/@microsoft/rush/fix-pnpm-posix-credential-environment_2026-09-18.json b/common/changes/@microsoft/rush/fix-pnpm-posix-credential-environment_2026-09-18.json new file mode 100644 index 00000000000..72faa350429 --- /dev/null +++ b/common/changes/@microsoft/rush/fix-pnpm-posix-credential-environment_2026-09-18.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Fix pnpm registry credentials being dropped by POSIX shells when provideNpmrcCredentialsViaEnvironment is enabled.", + "type": "patch" + } + ], + "packageName": "@microsoft/rush" +}