Skip to content

Conversation

@mrzepinski
Copy link
Contributor

@mrzepinski mrzepinski commented Oct 7, 2025

Pull Request Summary - Add PNPM minimumReleaseAge and minimumReleaseAgeExclude Support

PR Title

[rush-lib] Add support for PNPM's minimumReleaseAge setting

Summary

This PR adds support for PNPM's minimumReleaseAge and minimumReleaseAgeExclude settings in Rush's pnpm-config.json file. These settings help mitigate supply chain attacks by requiring a minimum age (in minutes) for package versions before they can be installed, with the ability to exclude trusted packages from this restriction.

Fixes #5372

Details

Changes Implemented (Updated based on code review feedback):

  1. Added minimumReleaseAge and minimumReleaseAgeExclude properties

    • Added to IPnpmOptionsJson interface and PnpmOptionsConfiguration class
    • Full JSDoc documentation with examples and version requirements
    • minimumReleaseAge: number (minutes to wait after package release)
    • minimumReleaseAgeExclude: string[] (package names/patterns to exclude from the check)
  2. Updated JSON schema (pnpm-config.schema.json)

    • Added validation for minimumReleaseAge (type: number)
    • Added validation for minimumReleaseAgeExclude (type: array of strings)
    • Includes comprehensive descriptions and examples
  3. Settings written to workspace package.json (refactored based on feedback)

    • Modified InstallHelpers.generateCommonPackageJson() to write both settings to common/temp/package.json in the pnpm section
    • This approach is more robust than passing CLI arguments
    • Follows the same pattern as other PNPM workspace settings
  4. Added PNPM version validation

    • Warns users if PNPM version < 10.16.0
    • Clear messaging guides users to either remove the settings or upgrade PNPM
    • Follows the same pattern as globalIgnoredOptionalDependencies
  5. Removed CLI argument approach

    • Removed command-line argument passing from BaseInstallManager.ts
    • Removed .npmrc duplicate detection (no longer needed with package.json approach)
  6. Updated API documentation in rush-lib.api.md

  7. Added comprehensive template documentation

    • Updated Rush initialization template with examples
    • Clear documentation about PNPM version requirements
    • Examples showing both settings working together
  8. Added test coverage

    • Updated PnpmOptionsConfiguration.test.ts to verify both settings
    • Added test fixture with both minimumReleaseAge and minimumReleaseAgeExclude

Design decisions:

  • Both settings are optional and default to undefined when not specified
  • Requires PNPM 10.16.0 or newer (documented and validated)
  • Settings are written to workspace package.json instead of passed as CLI args (more reliable)
  • minimumReleaseAgeExclude supports glob patterns (e.g., @myorg/*) per PNPM 10.17.0+

Backwards compatibility:

✅ This change is fully backwards compatible. Both settings are optional, and existing configurations will continue to work unchanged.

Performance impact:

  • No performance impact when the settings are not configured
  • When enabled, PNPM handles the filtering, with minimal impact during dependency resolution

Code Review Updates

This PR has been updated based on feedback from @D4N14L and @iclanton:

  • ✅ Added minimumReleaseAgeExclude support
  • ✅ Refactored to write settings to workspace package.json instead of CLI args
  • ✅ Added PNPM version validation (10.16.0+)
  • ✅ Removed rush-lib change file (keeping only template change file)
  • ✅ Fixed TSDoc warning for @ character escaping

@github-project-automation github-project-automation bot moved this to Needs triage in Bug Triage Oct 7, 2025
@mrzepinski mrzepinski force-pushed the feature/add-minimum-release-age-support branch from c760b75 to eaee160 Compare October 7, 2025 10:50
mrzepinski and others added 4 commits October 7, 2025 12:50
This change adds support for PNPM's minimumReleaseAge setting in Rush's pnpm-config.json file to help mitigate supply chain attacks by requiring a minimum age (in minutes) for package versions before installation.

Fixes microsoft#5372
@mrzepinski
Copy link
Contributor Author

@microsoft-github-policy-service agree

Copy link
Contributor

@D4N14L D4N14L left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General question, what is the value of doing this here? The NPMRC shipped with the project is equally capable, and you don't have the issue of specification in two separate places.

Though, this question isn't unique to your PR, as there are other settings that do this as well. CC @octogonz since he may have some insight/opinions here.

@iclanton iclanton moved this from Needs triage to In Progress in Bug Triage Oct 7, 2025
- Add minimumReleaseAgeExclude property to PnpmOptionsConfiguration
- Update pnpm-config.schema.json with new property definition
- Add documentation to template pnpm-config.json
- Write minimumReleaseAge and minimumReleaseAgeExclude to package.json instead of passing as CLI args
- Add PNPM version check (10.16.0+) in InstallHelpers
- Remove command-line argument passing from BaseInstallManager
- Update tests to verify minimumReleaseAgeExclude functionality
- Fix TSDoc warning for @ character escaping

Addresses code review feedback from PR microsoft#5405
@mrzepinski
Copy link
Contributor Author

@iclanton Let's review please ;) all the remarks were completed

@iclanton iclanton merged commit 569e2f4 into microsoft:main Oct 8, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Closed in Bug Triage Oct 8, 2025
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.

[rush] Support pnpm's minimumReleaseAge setting

3 participants