[rush] Preserve pnpm credential environment variables on POSIX - #6040
Open
Mike DelGaudio (mikedelgaudio) wants to merge 2 commits into
Open
Mike DelGaudio (mikedelgaudio) wants to merge 2 commits into
Mike DelGaudio (mikedelgaudio) wants to merge 2 commits into
Conversation
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
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88399e18-c0da-4cfc-af47-6673b1cc1545
| "changes": [ | ||
| { | ||
| "packageName": "@microsoft/rush", | ||
| "comment": "Fix pnpm registry credentials being dropped by POSIX shells when provideNpmrcCredentialsViaEnvironment is enabled.", |
There was a problem hiding this comment.
Suggested change
| "comment": "Fix pnpm registry credentials being dropped by POSIX shells when provideNpmrcCredentialsViaEnvironment is enabled.", | |
| "comment": "Fix pnpm registry credentials being dropped by POSIX shells when `provideNpmrcCredentialsViaEnvironment` is enabled.", |
| let environment: NodeJS.ProcessEnv; | ||
|
|
||
| beforeAll(async () => { | ||
| directory = await fs.promises.mkdtemp(`${os.tmpdir()}/rush credentials `); |
There was a problem hiding this comment.
Don't use os.tempdir. We want to keep all writes in deterministic places.
| // 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'; |
There was a problem hiding this comment.
Use FileSystem from node-core-library.
Comment on lines
+43
to
+48
| 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' | ||
| ); |
There was a problem hiding this comment.
FileSystem.writeFile has an option to ensure the folder exists.
| ); | ||
| }); | ||
|
|
||
| (IS_WINDOWS ? it.skip : it)( |
There was a problem hiding this comment.
Ensure the cache key for this project's tests takes the OS into account.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix missing pnpm registry authentication on POSIX when
provideNpmrcCredentialsViaEnvironmentis enabled.Follow-up to #5995: Rush creates URL-scoped
npm_config_*credential variables, but passing them through/bin/shcan discard them before pnpm starts. On Linux with dash, a clean install using pnpm 10.34.5 fails withERR_PNPM_FETCH_401and no Authorization header.Details
useShellexecution option, defaulting to existing shell behavior.rush-pnpm..cmdshims, default behavior for other callers, output handling, retries, and exit-code capture.How it was tested
Revalidated the unchanged patch on macOS with Node 22.21.1:
rush --quiet test --only @microsoft/rush-lib --include-phase-deps --parallelism 4: 931 tests passed, zero failures.rush change --verify --target-branch origin/main, Prettier checking all changed files, andgit diff --check origin/main...HEADpassed.Prior Linux validation of this exact patch, recorded in the implementation handoff:
rush-pnpm outdatedand autoinstaller update authenticated successfully.The manual Linux registry reproduction was not rerun on macOS. Windows was not executed locally; its existing shell path is unchanged and the POSIX-only argument test is skipped on Windows. Full legacy-install and cold autoinstaller-install integrations were not separately exercised.