From c6f7b9dc9b1a1ad9b6ad3e963c2040a2d3170831 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Mon, 15 Jun 2026 13:22:01 -0400 Subject: [PATCH] Require exactly one npm ESRP owner in release pipeline 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> --- docs/specs/npm-cli-package.md | 2 +- eng/pipelines/release-publish-nuget.yml | 9 ++++++--- eng/scripts/validate-npm-release-aliases.ps1 | 15 ++++++++------ .../Pipelines/NpmCliPackageTests.cs | 2 +- .../ReleasePublishNugetPipelineTests.cs | 13 ++++++------ .../ValidateNpmReleaseAliasesTests.cs | 20 ++++++++++++++++--- 6 files changed, 41 insertions(+), 20 deletions(-) diff --git a/docs/specs/npm-cli-package.md b/docs/specs/npm-cli-package.md index 9b936bc0609..64881117ed0 100644 --- a/docs/specs/npm-cli-package.md +++ b/docs/specs/npm-cli-package.md @@ -188,7 +188,7 @@ The release pipeline prepares two npm artifact folders and one validation artifa The package split is intentional. The release job submits RID packages first, waits for the ESRP submission to complete, waits an additional npm registry propagation delay, and then submits the pointer package. Publishing the pointer package last avoids optional dependency resolution races when a user installs the top-level package immediately after release. -Before publishing, the release pipeline validates that exactly one pointer tarball and exactly one tarball for each supported RID are present, that every tarball has a detached `.tgz.sig` sidecar, that all tarballs have one version, and that the `NpmValidationSummary` artifact reports `validatedByPreparePipeline: true` with every required check `passed` for Windows, Linux, and macOS install validation. npm publishing reads its ESRP identities from the `NpmPublishOwners` and `NpmPublishApprovers` pipeline parameters, which default to working values in `eng/pipelines/release-publish-nuget.yml` so an unattended queue submission does not fail and can be overridden per run: owners must include at least one required release owner alias configured in the pipeline, approvers must contain exactly one Microsoft alias or `@microsoft.com` email address, and the owner and approver sets must not overlap. The pipeline forwards both parameters to the validation step as environment variables (rather than interpolating them into the inline script) so the operator-supplied values are treated as data. The validation logic lives in `eng/scripts/validate-npm-release-aliases.ps1`; the release job runs with `checkout: none`, so the same helpers are mirrored inline in the pipeline and kept in sync by a unit test. +Before publishing, the release pipeline validates that exactly one pointer tarball and exactly one tarball for each supported RID are present, that every tarball has a detached `.tgz.sig` sidecar, that all tarballs have one version, and that the `NpmValidationSummary` artifact reports `validatedByPreparePipeline: true` with every required check `passed` for Windows, Linux, and macOS install validation. npm publishing reads its ESRP identities from the `NpmPublishOwners` and `NpmPublishApprovers` pipeline parameters, which default to working values in `eng/pipelines/release-publish-nuget.yml` so an unattended queue submission does not fail and can be overridden per run: the owner must be a single alias that matches one of the required release owner aliases configured in the pipeline, approvers must contain exactly one Microsoft alias or `@microsoft.com` email address, and the owner and approver must not be the same alias. The pipeline forwards both parameters to the validation step as environment variables (rather than interpolating them into the inline script) so the operator-supplied values are treated as data. The validation logic lives in `eng/scripts/validate-npm-release-aliases.ps1`; the release job runs with `checkout: none`, so the same helpers are mirrored inline in the pipeline and kept in sync by a unit test. The release pipeline checks only the package groups scheduled for publishing before invoking MicroBuild. If `SkipNpmRidPublish=false`, every staged RID tarball is checked with `npm view @ version`; if `SkipNpmPointerPublish=false`, the pointer tarball is checked the same way. Any scheduled package version that already exists on npm fails before ESRP submission, because npm versions are immutable and a duplicate publish would otherwise fail later in MicroBuild. Re-runs after partial success should use `SkipNpmRidPublish=true` only when every RID package for the selected version is already live, `SkipNpmPointerPublish=true` only when the pointer package is already live, and both flags together only after the entire npm publish path has completed. diff --git a/eng/pipelines/release-publish-nuget.yml b/eng/pipelines/release-publish-nuget.yml index fc4c4552ea8..472a289b4c5 100644 --- a/eng/pipelines/release-publish-nuget.yml +++ b/eng/pipelines/release-publish-nuget.yml @@ -63,12 +63,12 @@ parameters: default: false - name: NpmPublishOwners - displayName: '[Advanced] npm ESRP owners (comma-separated Microsoft aliases or emails; must include joperezr or ankj)' + displayName: '[Advanced] npm ESRP owner (single Microsoft alias or email; must be joperezr or ankj)' type: string - default: 'joperezr,ankj' + default: 'joperezr' - name: NpmPublishApprovers - displayName: '[Advanced] npm ESRP approver (single Microsoft alias or email; must differ from the owners)' + displayName: '[Advanced] npm ESRP approver (single Microsoft alias or email; must differ from the owner)' type: string default: 'adamratzman' @@ -829,6 +829,9 @@ extends: exit 1 } + # ESRP accepts multiple owners, but the release process requires exactly one so that + # ownership of the @microsoft/aspire-cli package maps to a single accountable alias. + Assert-SingleNpmReleaseAlias $normalizedOwners 'NpmPublishOwners' Assert-ContainsAnyRequiredNpmOwnerAlias $normalizedOwners $requiredNpmOwners 'NpmPublishOwners' Assert-SingleNpmReleaseAlias $normalizedApprovers 'NpmPublishApprovers' diff --git a/eng/scripts/validate-npm-release-aliases.ps1 b/eng/scripts/validate-npm-release-aliases.ps1 index 8f20c034fda..2ec9d096921 100644 --- a/eng/scripts/validate-npm-release-aliases.ps1 +++ b/eng/scripts/validate-npm-release-aliases.ps1 @@ -9,10 +9,10 @@ Validates the npm ESRP owner and approver aliases used by the release pipeline. The Aspire release pipeline (eng/pipelines/release-publish-nuget.yml) submits the @microsoft/aspire-cli npm packages through MicroBuild's ESRP publish template. ESRP requires a set of owner aliases and a single approver alias. This script normalizes -those values, enforces the release rules (owners must include at least one required -owner alias, approvers must be a single alias, and the two sets must not overlap), and -emits the normalized ("effective") sets so the pipeline can forward them to the publish -template. +those values, enforces the release rules (owners must be a single alias that is one of +the required owner aliases, approvers must be a single alias, and the two must not be +the same alias), and emits the normalized ("effective") sets so the pipeline can forward +them to the publish template. The release job runs with `checkout: none`, so the pipeline cannot dot-source this file at runtime. The helper functions are mirrored inline in the pipeline YAML and the @@ -25,7 +25,7 @@ Dot-source the script to import only the helper functions without running valida . ./validate-npm-release-aliases.ps1 .PARAMETER Owners -Comma-separated owner aliases or @microsoft.com email addresses. Defaults to the +A single owner alias or @microsoft.com email address. Defaults to the NPM_PUBLISH_OWNERS environment variable. .PARAMETER Approvers @@ -33,7 +33,7 @@ A single approver alias or @microsoft.com email address. Defaults to the NPM_PUBLISH_APPROVERS environment variable. .PARAMETER RequiredOwners -Comma-separated list of owner aliases, at least one of which must appear in Owners. +Comma-separated list of owner aliases; the single owner must be one of these. Defaults to the NPM_PUBLISH_REQUIRED_OWNERS environment variable. #> [CmdletBinding()] @@ -120,6 +120,9 @@ function Invoke-NpmReleaseAliasValidation( exit 1 } + # ESRP accepts multiple owners, but the release process requires exactly one so that + # ownership of the @microsoft/aspire-cli package maps to a single accountable alias. + Assert-SingleNpmReleaseAlias $normalizedOwners 'NpmPublishOwners' Assert-ContainsAnyRequiredNpmOwnerAlias $normalizedOwners $requiredNpmOwners 'NpmPublishOwners' Assert-SingleNpmReleaseAlias $normalizedApprovers 'NpmPublishApprovers' diff --git a/tests/Infrastructure.Tests/Pipelines/NpmCliPackageTests.cs b/tests/Infrastructure.Tests/Pipelines/NpmCliPackageTests.cs index 3d4adcd3dc3..b17c5f92de3 100644 --- a/tests/Infrastructure.Tests/Pipelines/NpmCliPackageTests.cs +++ b/tests/Infrastructure.Tests/Pipelines/NpmCliPackageTests.cs @@ -327,7 +327,7 @@ public async Task ReleasePipelineUsesEffectiveNpmOwnersAndApproversFromSingleSou Assert.DoesNotContain("NPM_PUBLISH_REQUIRED_APPROVERS", commonVariables); Assert.DoesNotContain("NPM_PUBLISH_REQUIRED_APPROVERS", releasePipeline); Assert.DoesNotContain("requiredNpmApprovers", releasePipeline); - Assert.Contains("default: 'joperezr,ankj'", releasePipeline); + Assert.Contains("default: 'joperezr'", releasePipeline); Assert.Contains("default: 'adamratzman'", releasePipeline); Assert.Contains("NpmPublishOwnersEffective", releasePipeline); Assert.Contains("NpmPublishApproversEffective", releasePipeline); diff --git a/tests/Infrastructure.Tests/Pipelines/ReleasePublishNugetPipelineTests.cs b/tests/Infrastructure.Tests/Pipelines/ReleasePublishNugetPipelineTests.cs index cd73840daa4..59c43cc90eb 100644 --- a/tests/Infrastructure.Tests/Pipelines/ReleasePublishNugetPipelineTests.cs +++ b/tests/Infrastructure.Tests/Pipelines/ReleasePublishNugetPipelineTests.cs @@ -141,8 +141,8 @@ public async Task UsesRequiredNpmEsrpOwnersAndApprover() Assert.Contains("value: joperezr,ankj", pipeline); Assert.DoesNotContain("NPM_PUBLISH_DEFAULT_APPROVER", commonVariables); Assert.DoesNotContain("NPM_PUBLISH_REQUIRED_APPROVERS", commonVariables); - Assert.Contains("displayName: '[Advanced] npm ESRP owners (comma-separated Microsoft aliases or emails; must include joperezr or ankj)'", pipeline); - Assert.Contains("displayName: '[Advanced] npm ESRP approver (single Microsoft alias or email; must differ from the owners)'", pipeline); + Assert.Contains("displayName: '[Advanced] npm ESRP owner (single Microsoft alias or email; must be joperezr or ankj)'", pipeline); + Assert.Contains("displayName: '[Advanced] npm ESRP approver (single Microsoft alias or email; must differ from the owner)'", pipeline); Assert.Contains("$requiredNpmOwnersValue = $env:NPM_PUBLISH_REQUIRED_OWNERS", pipeline); Assert.DoesNotContain("NPM_PUBLISH_DEFAULT_APPROVER", pipeline); Assert.DoesNotContain("NPM_PUBLISH_REQUIRED_APPROVERS", pipeline); @@ -157,6 +157,7 @@ public async Task NpmEsrpOwnersRequireAnyConfiguredOwnerAlias() { var pipeline = await ReadRepoFileAsync("eng/pipelines/release-publish-nuget.yml"); + Assert.Contains("Assert-SingleNpmReleaseAlias $normalizedOwners 'NpmPublishOwners'", pipeline); Assert.Contains("Assert-ContainsAnyRequiredNpmOwnerAlias $normalizedOwners $requiredNpmOwners 'NpmPublishOwners'", pipeline); Assert.DoesNotContain("Assert-ContainsRequiredNpmAliases $normalizedOwners $requiredNpmOwners 'NpmPublishOwners'", pipeline); Assert.Contains("Assert-SingleNpmReleaseAlias $normalizedApprovers 'NpmPublishApprovers'", pipeline); @@ -215,13 +216,13 @@ public async Task NpmPublishOwnerAndApproverParametersHaveWorkingDefaults() var pipeline = await ReadRepoFileAsync("eng/pipelines/release-publish-nuget.yml"); // Defaults let an unattended queue submission pass validation without operator input: - // owners include a required owner alias, the approver is a single distinct alias, and the - // per-run override parameters are marked advanced. + // the owner is a single required owner alias, the approver is a single distinct alias, and + // the per-run override parameters are marked advanced. Assert.Contains("- name: NpmPublishOwners", pipeline); - Assert.Contains("default: 'joperezr,ankj'", pipeline); + Assert.Contains("default: 'joperezr'", pipeline); Assert.Contains("- name: NpmPublishApprovers", pipeline); Assert.Contains("default: 'adamratzman'", pipeline); - Assert.Contains("[Advanced] npm ESRP owners", pipeline); + Assert.Contains("[Advanced] npm ESRP owner", pipeline); Assert.Contains("[Advanced] npm ESRP approver", pipeline); Assert.Contains("[Advanced] Minutes to wait between npm RID and pointer package submissions", pipeline); } diff --git a/tests/Infrastructure.Tests/PowerShellScripts/ValidateNpmReleaseAliasesTests.cs b/tests/Infrastructure.Tests/PowerShellScripts/ValidateNpmReleaseAliasesTests.cs index a7f6d5d6ead..cf7c7558691 100644 --- a/tests/Infrastructure.Tests/PowerShellScripts/ValidateNpmReleaseAliasesTests.cs +++ b/tests/Infrastructure.Tests/PowerShellScripts/ValidateNpmReleaseAliasesTests.cs @@ -62,6 +62,18 @@ public async Task FailsWhenApproversHasMultipleAliases() Flatten(result.Output)); } + [Fact] + [RequiresTools(["pwsh"])] + public async Task FailsWhenOwnersHasMultipleAliases() + { + var result = await RunValidation(owners: "joperezr,ankj", approvers: "adamratzman"); + + Assert.NotEqual(0, result.ExitCode); + Assert.Contains( + "NpmPublishOwners must contain exactly one Microsoft alias or @microsoft.com email address.", + Flatten(result.Output)); + } + [Fact] [RequiresTools(["pwsh"])] public async Task FailsWhenOwnersMissingEveryRequiredAlias() @@ -151,12 +163,14 @@ public async Task StripsMicrosoftEmailSuffixFromOwnerAliases() [Fact] [RequiresTools(["pwsh"])] - public async Task EmitsSortedDeduplicatedEffectiveAliasesOnSuccess() + public async Task EmitsDeduplicatedEffectiveAliasesOnSuccess() { - var result = await RunValidation(owners: "ankj,joperezr,ANKJ", approvers: "adamratzman"); + // Duplicate spellings of the same single owner alias (different casing and the + // @microsoft.com suffix) collapse to one entry, satisfying the single-owner rule. + var result = await RunValidation(owners: "joperezr,JOPEREZR,joperezr@microsoft.com", approvers: "adamratzman"); result.EnsureSuccessful(); - Assert.Contains("variable=NpmPublishOwnersEffective]ankj,joperezr", result.Output); + Assert.Contains("variable=NpmPublishOwnersEffective]joperezr", result.Output); Assert.Contains("variable=NpmPublishApproversEffective]adamratzman", result.Output); }