Skip to content

[rush] Preserve pnpm credential environment variables on POSIX - #6040

Merged
Ian Clanton-Thuon (iclanton) merged 3 commits into
microsoft:mainfrom
mikedelgaudio:fix/pnpm-posix-credential-environment
Sep 22, 2026
Merged

Ian Clanton-Thuon (iclanton) merged 3 commits into
microsoft:mainfrom
mikedelgaudio:fix/pnpm-posix-credential-environment

Conversation

@mikedelgaudio

@mikedelgaudio Mike DelGaudio (mikedelgaudio) commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix missing pnpm registry authentication on POSIX when provideNpmrcCredentialsViaEnvironment is enabled.

Follow-up to #5995: Rush creates URL-scoped npm_config_* credential variables, but passing them through /bin/sh can discard them before pnpm starts. On Linux with dash, a clean install using pnpm 10.34.5 fails with ERR_PNPM_FETCH_401 and no Authorization header.

Details

  • Add an internal useShell execution option, defaulting to existing shell behavior.
  • Opt package-manager launches out of the POSIX shell only when the existing credential-environment experiment/version gate applies. Pass the executable and original argument array directly to the child process.
  • Cover workspace installs, legacy installs, autoinstaller installation/update, and rush-pnpm.
  • Preserve Windows shell handling for .cmd shims, default behavior for other callers, output handling, retries, and exit-code capture.
  • Keep credentials in the child environment; do not introduce credential files or shell overrides.
  • Include the Rush patch release note.
  • Use a deterministic, suite-specific fixture under the Rush-library project's temp/ directory with node-core-library FileSystem helpers, retaining the space-containing script path.
  • Include [os] in the repository's cache-entry naming pattern so Linux, macOS, and Windows cannot reuse one another's test results. This repository-wide policy also partitions other phases' caches and prevents reuse of entries under the old naming pattern; it does not change shipped Rush cache defaults.

How it was tested

Revalidated the review follow-up on September 21, 2026, on macOS with Node 22.21.1:

  • rush --quiet test --only @microsoft/rush-lib --include-phase-deps --parallelism 4: 931 tests passed, zero failures.
  • Six new real-subprocess regressions cover generated credential preservation, the install retry execution path, literal POSIX arguments, exit-code capture, failure rejection, and unchanged default shell execution.
  • The tests actually executed with the new cache pattern, rather than restoring a cached test result.
  • Whole-suite Rush-library coverage: 63.89% lines and 77.91% branches (not changed-lines coverage).
  • rush change --verify --target-branch origin/main, Prettier checking all changed files, and git diff --check origin/main...HEAD passed.
  • Validated the configured pattern through the existing CacheEntryId.parsePattern implementation with simulated process.platform values: identical project/phase/input hashes produce distinct linux, darwin, and win32 keys. Verified the suite fixture was removed after execution.

Prior Linux validation of the original implementation, recorded in the implementation handoff (production launcher unchanged by the review follow-up):

  • Linux/dash with Node 22.16.0: the same Rush-library command passed all 931 tests.
  • Manual clean authenticated-registry comparison with pnpm 10.34.5, isolated HOME/package stores, an empty user npm config, and synthetic credentials: released Rush 5.179.0 failed with 401; patched Rush installed successfully without writing the resolved token to generated config.
  • Patched rush-pnpm outdated and autoinstaller update authenticated successfully.

The original PR head passed all six CI jobs, including Windows on Node 24 and 26; those results predate this follow-up. 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.

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
Comment thread libraries/rush-lib/src/utilities/test/Utilities.test.ts Outdated
Comment thread libraries/rush-lib/src/utilities/test/Utilities.test.ts Outdated
Comment thread libraries/rush-lib/src/utilities/test/Utilities.test.ts Outdated
Comment thread libraries/rush-lib/src/utilities/test/Utilities.test.ts
@mikedelgaudio

Copy link
Copy Markdown
Contributor Author

Validation update for 8866cb5: all six CI jobs passed (Linux: Node 20/22/24/26; Windows: Node 24/26), and the CLA check passed. CI run: https://github.com/microsoft/rushstack/actions/runs/35648488938

Confirmed the first Windows Node 26 Rush retest step actually executed Utilities.test.js (12 passed, zero failures), not a cross-OS cache restoration. The Rush-library suite reported 930 successes out of 931 total, with the POSIX-only argument test skipped as intended. Local macOS validation passed all 931 tests.

All five review comments have implementation replies. The repository-wide OS cache partitioning tradeoff remains documented for review; no threads were marked resolved on behalf of the reviewer.

@iclanton
Ian Clanton-Thuon (iclanton) merged commit 21689f7 into microsoft:main Sep 22, 2026
7 checks passed
@github-project-automation github-project-automation Bot moved this from Needs triage to Closed in Bug Triage Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

3 participants