Skip to content

Require exactly one npm ESRP owner in release pipeline (release/13.4)#18219

Open
adamint wants to merge 1 commit into
microsoft:release/13.4from
adamint:copilot/npm-single-esrp-owner-release-13.4
Open

Require exactly one npm ESRP owner in release pipeline (release/13.4)#18219
adamint wants to merge 1 commit into
microsoft:release/13.4from
adamint:copilot/npm-single-esrp-owner-release-13.4

Conversation

@adamint

@adamint adamint commented Jun 15, 2026

Copy link
Copy Markdown
Member

Description

This PR targets release/13.4 and mirrors the same change made against main in #18218.

The npm ESRP publishing step in the release pipeline previously accepted multiple owner aliases as long as at least one matched a configured required owner. This left ownership of the @microsoft/aspire-cli package ambiguous when several aliases were supplied. This change requires the owner to be a single alias, matching the rule that already applied to the approver, so package ownership maps to one accountable alias.

Operator-facing behavior:

  • The NpmPublishOwners pipeline parameter must now resolve to exactly one alias (after normalization), and that alias must still be one of the required ESRP owners (joperezr or ankj).
  • Supplying more than one distinct owner now fails fast with: NpmPublishOwners must contain exactly one Microsoft alias or @microsoft.com email address.
  • The parameter default changed from joperezr,ankj to joperezr, because the old two-owner default would now fail the single-owner check. The NPM_PUBLISH_REQUIRED_OWNERS set stays joperezr,ankj — it is the set of allowed owners, and the single configured owner must be one of them.
  • Parameter displayName, the script doc comments, and docs/specs/npm-cli-package.md were updated from plural "owners" to singular "owner".

Implementation details:

  • Added Assert-SingleNpmReleaseAlias $normalizedOwners 'NpmPublishOwners' in the owners validation path, before the required-owner check. This is mirrored in both eng/scripts/validate-npm-release-aliases.ps1 and the inline helper region in eng/pipelines/release-publish-nuget.yml (the release job runs with checkout: none, so the helpers are duplicated and kept in sync by ReleasePublishNugetPipelineTests.NpmAliasValidationHelpersMatchScript).
  • Validation order is now: owners non-empty → approvers non-empty → single owner → required owner → single approver → owner/approver non-overlap.

Validation

  • Added FailsWhenOwnersHasMultipleAliases and updated the dedup test to feed duplicate spellings of a single owner.
  • Updated pipeline tests to assert the new single-owner default and displayName wording.
  • All filtered Infrastructure.Tests pass locally (59/59), including the helper-sync test.

Fixes # (issue)

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

The npm ESRP publish validation previously allowed multiple owner aliases
as long as at least one was a required release owner. Restrict owners to a
single alias (matching the existing single-approver rule) so ownership of
the @microsoft/aspire-cli package maps to one accountable alias.

- Add Assert-SingleNpmReleaseAlias for owners before the required-owner
  check, mirrored in both validate-npm-release-aliases.ps1 and the inline
  helpers in release-publish-nuget.yml.
- Change NpmPublishOwners default from 'joperezr,ankj' to 'joperezr' (the
  old multi-owner default would now fail validation) and update the param
  displayName/spec doc to describe the single-owner rule.
- Update Infrastructure.Tests to cover multi-owner rejection and the new
  default.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 15, 2026 17:37
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18219

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18219"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR enforces that the NpmPublishOwners pipeline parameter must resolve to exactly one alias (previously multiple were allowed), ensuring single-accountable ownership for the @microsoft/aspire-cli npm package in the ESRP release flow. This mirrors the same change already made against main in PR #18218 but targets the release/13.4 branch.

Changes:

  • Added Assert-SingleNpmReleaseAlias call for owners in both the validation script and the mirrored inline pipeline YAML, changing the validation order to reject multiple owners before checking the required-owner set.
  • Updated the NpmPublishOwners parameter default from 'joperezr,ankj' to 'joperezr' and changed display names/docs from plural "owners" to singular "owner".
  • Added/updated tests to cover the new single-owner enforcement and adjusted the dedup test to use multiple spellings of one alias.
Show a summary per file
File Description
eng/scripts/validate-npm-release-aliases.ps1 Added single-owner assertion call and updated doc comments to reflect singular owner semantics
eng/pipelines/release-publish-nuget.yml Changed parameter default/displayName to singular, added mirrored single-owner assertion in inline script
docs/specs/npm-cli-package.md Updated spec paragraph to describe single-owner requirement instead of multi-owner
tests/Infrastructure.Tests/PowerShellScripts/ValidateNpmReleaseAliasesTests.cs Added FailsWhenOwnersHasMultipleAliases test, updated dedup test for single-owner scenario
tests/Infrastructure.Tests/Pipelines/ReleasePublishNugetPipelineTests.cs Updated assertions for new displayName, default value, and single-owner assertion presence
tests/Infrastructure.Tests/Pipelines/NpmCliPackageTests.cs Updated default value assertion from multi-owner to single-owner

Copilot's findings

  • Files reviewed: 6/6 changed files
  • Comments generated: 0

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants