DO NOT MERGE: Test Sigstore/Tuf 1.0.0 packages - #19335
DO NOT MERGE: Test Sigstore/Tuf 1.0.0 packages#19335Mitch Denny (mitchdenny) wants to merge 5 commits into
Conversation
Bump Sigstore and Tuf to 1.0.0-alpha.100.1.dd7cd33 to validate the tightened verification implementation before the packages are mirrored to the dnceng feeds. Temporarily points at nuget.org to acquire the alpha packages. This requires three changes because simply adding the source is not sufficient: - Add the nuget.org package source. - Add a packageSourceMapping entry. Source mapping is enabled repo-wide, so an unmapped source is never consulted. The exact-name Sigstore/Tuf patterns beat the "*" patterns on the dnceng feeds via longest-prefix-match, so only these two packages come from nuget.org. - Add <clear /> to disabledPackageSources. User/machine-level NuGet configs commonly disable nuget.org, and the existing <clear /> in <packageSources> does not affect that section. DO NOT MERGE: external feeds break the internal build pipeline. Revert the NuGet.config changes once Sigstore/Tuf are mirrored to dotnet-public. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4bd5a7f6-724d-4043-acb5-4b792184658b
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19335Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19335" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Temporarily validates tightened Sigstore/Tuf verification using prerelease packages.
Changes:
- Bumps Sigstore and Tuf to alpha builds.
- Temporarily enables nuget.org with restricted source mappings.
- Clears inherited disabled-package sources.
Show a summary per file
| File | Description |
|---|---|
NuGet.config |
Temporarily enables and maps nuget.org. |
Directory.Packages.props |
Updates Sigstore and Tuf versions. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| packages before they are mirrored into the dnceng internal feeds. External feeds break the | ||
| internal build pipeline, so this entry and its package source mapping below must be reverted | ||
| (and the packages mirrored to dotnet-public) before this change ships. --> | ||
| <add key="nuget.org" value="https://api.nuget.org/v3/index.json" /> |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
CI failed at restore on every job with NU1902/NU1903 for packages that have nothing to do with this change: Microsoft.OpenApi 3.5.3, SSH.NET 2024.2.0, MessagePack 2.5.198, SharpCompress 0.30.1 and Snappier 1.0.0. None are Sigstore or Tuf, none are in Directory.Packages.props, and all are pre-existing transitive dependencies untouched by this branch. Root cause is a side effect of adding nuget.org, not the alpha packages. NuGet audit is on by default, but it silently no-ops unless a configured source advertises a VulnerabilityInfo resource. The dnceng feeds expose none; nuget.org exposes VulnerabilityInfo/6.7.0. Adding nuget.org gave the restore a vulnerability database for the first time, so audit started flagging long-standing transitive packages, and TreatWarningsAsErrors promoted those warnings into restore-breaking errors repo-wide. Verified both directions locally on Aspire.Milvus.Client.Tests, a project with no Sigstore/Tuf reference: with NuGetAudit=true it reproduces the CI error exactly (NU1903 SSH.NET, GHSA-q939-rpr3-3284), and with the property below it restores cleanly. Aspire.Cli builds with 0 warnings and the full Aspire.Cli.Tests suite passes 4919/4919, including the SdkDumpCommandTests case that previously tripped on Microsoft.OpenApi. Revert this together with the nuget.org source in NuGet.config once the packages are mirrored to dotnet-public. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4bd5a7f6-724d-4043-acb5-4b792184658b
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review details
Suppressed comments (2)
NuGet.config:28
- This adds an unapproved external package source, which the repository's internal build cannot consume. This block must not merge; mirror Sigstore/Tuf to an approved dnceng feed, then remove the nuget.org source, mapping, and inherited-disable override together.
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
Directory.Packages.props:152
- The package upgrade changes the production cryptographic verification implementation, but the existing SigstoreNpmProvenanceChecker tests inject a handler that bypasses real Sigstore verification (tests/Aspire.Cli.Tests/Agents/SigstoreNpmProvenanceCheckerTests.cs:1210-1213). Before shipping this bump, add deterministic automated coverage that invokes the real verifier for a genuine bundle and rejects identity/digest tampering; the out-of-band harness cannot catch future regressions in CI.
<PackageVersion Include="Sigstore" Version="1.0.0-alpha.100.1.dd7cd33" />
<PackageVersion Include="Tuf" Version="1.0.0-alpha.100.1.dd7cd33" />
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
| vulnerability database for the first time, so audit begins flagging pre-existing transitive | ||
| packages, and TreatWarningsAsErrors above promotes NU1901-NU1904 to restore-breaking errors | ||
| across the whole repo. None of these advisories are related to the Sigstore/Tuf bump. --> | ||
| <NuGetAudit>false</NuGetAudit> |
There was a problem hiding this comment.
Review details
Suppressed comments (2)
NuGet.config:28
- This external source cannot ship: repository policy permits only the dnceng feed domains because public feeds break the internal build pipeline (
AGENTS.md:53-67). Mirror Sigstore/Tuf to an approved feed, then remove this source together with its mapping and inherited-disable override before approval.
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
Directory.Build.props:26
- This disables NuGet vulnerability auditing for every project in the repository, so restores will stop reporting NU1901-NU1904 advisories globally. It must be removed together with the temporary nuget.org source before this can be merged.
<NuGetAudit>false</NuGetAudit>
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Balanced
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Moves from the alpha to the newly published beta. Dependency shape is unchanged (BouncyCastle.Cryptography 2.7.0), and the CLI builds with no API breaks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4bd5a7f6-724d-4043-acb5-4b792184658b
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review details
Suppressed comments (5)
NuGet.config:28
- The root source list otherwise uses only approved dnceng feeds (NuGet.config:18-23); adding nuget.org breaks the internal build path. Before the version bump can merge, mirror these packages to dotnet-public and remove this source together with its mapping, disabled-source override, and audit suppression.
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
Directory.Build.props:26
- This disables NuGet vulnerability auditing for every project in the repository, so vulnerable dependencies can restore without NU1901-NU1904 diagnostics. Remove this suppression before merge and resolve the temporary feed/advisory behavior instead of globally bypassing the audit.
<NuGetAudit>false</NuGetAudit>
NuGet.config:63
- This second temporary note also refers to the alpha even though the configured versions are beta builds.
drops inherited disable entries so the Sigstore/Tuf alpha can be restored from nuget.org.
Directory.Packages.props:152
- This changes the security-sensitive cryptographic verification dependency, but the existing SigstoreNpmProvenanceChecker tests inject a handler that returns a prebuilt result (tests/Aspire.Cli.Tests/Agents/SigstoreNpmProvenanceCheckerTests.cs:1210-1213), so they cannot detect verifier regressions. Add checked-in coverage that exercises the real verifier with a known genuine bundle and rejection cases before shipping the bump.
<PackageVersion Include="Sigstore" Version="1.0.0-beta.101.1.dd7cd33" />
<PackageVersion Include="Tuf" Version="1.0.0-beta.101.1.dd7cd33" />
NuGet.config:24
- This comment still says the validation targets the alpha, but the package versions are now beta builds. Update it so the temporary configuration accurately describes what is being restored.
This issue also appears on line 63 of the same file.
<!-- TEMPORARY - DO NOT MERGE: nuget.org is added solely to test the Sigstore/Tuf 1.0.0-alpha
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Dependency shape is unchanged from the prereleases (BouncyCastle.Cryptography 2.7.0), and the CLI builds with no API breaks. The temporary NuGet.config and NuGetAudit changes are still required: dnceng dotnet-public still only carries up to 0.5.0 and does not upstream-fetch 1.0.0. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4bd5a7f6-724d-4043-acb5-4b792184658b
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. Runs the full test matrix + all jobs (ALL) — a rule matching 'Directory.Build.props' selects ALL Selection computed for commit |
There was a problem hiding this comment.
Review details
Suppressed comments (4)
Directory.Build.props:26
- This disables vulnerability auditing for every package restored anywhere in the repository, hiding NU1901–NU1904 advisories unrelated to this experiment. Remove this temporary global opt-out before merge; validation-only invocations should suppress audit locally rather than weakening the repository default.
<NuGetAudit>false</NuGetAudit>
NuGet.config:28
- This adds an unapproved external package source. Repository policy permits only dnceng feed domains because public feeds break the internal build pipeline. Before this can merge, mirror Sigstore/Tuf 1.0.0 to
dotnet-publicand remove this source together with its mapping anddisabledPackageSourcesoverride.
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
NuGet.config:63
- This still describes restoring an alpha, but the changed dependency is stable 1.0.0. Keep the temporary configuration comment aligned with the version under test.
drops inherited disable entries so the Sigstore/Tuf alpha can be restored from nuget.org.
NuGet.config:27
- This comment is stale: the package versions in
Directory.Packages.propsare stable 1.0.0, not 1.0.0-alpha. Update it so the temporary source's purpose matches the actual dependency being validated.
This issue also appears on line 63 of the same file.
<!-- TEMPORARY - DO NOT MERGE: nuget.org is added solely to test the Sigstore/Tuf 1.0.0-alpha
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Description
Bumps
SigstoreandTufto stable1.0.0to validate the tightened verification implementation ahead of the packages being mirrored into the dnceng internal feeds. Opened purely to share validation results.Why
NuGet.configchanges are still neededStable
1.0.0is on nuget.org, but not on the internal feeds —dnceng/dotnet-publicstill carries only up to0.5.0, and it does not upstream-fetch1.0.0on demand:Sigstore/Tuf1.0.0dnceng/dotnet-public404, saved versions stop at0.5.0So this still temporarily points at nuget.org. Simply adding the source is not sufficient — three changes were required:
nuget.orgsource.packageSourceMappingentry. Source mapping is enabled repo-wide, so an unmapped source is never consulted. The exact-nameSigstore/Tufpatterns beat the*patterns on the dnceng feeds via longest-prefix-match, so only these two packages resolve from nuget.org — everything else still comes from dnceng.<clear />todisabledPackageSources. User/machine-level NuGet configs commonly disable nuget.org, and the existing<clear />in<packageSources>does not affect that section.All three are annotated inline with
TEMPORARY - DO NOT MERGEcomments explaining what to revert.Validation
Aspire.Clibuilds with 0 warnings, 0 errors — no API breaks from the tightening. FullAspire.Cli.Testsrun on stable1.0.0: 4957 passed, 0 failed (34 skipped, all Windows-only).Worth calling out: the 85
SigstoreNpmProvenanceCheckerTestsstub verification via an injectedSigstoreBundleVerificationHandler, so they never execute real cryptography. Those passing is not by itself evidence the tightening works.To get genuine coverage I ran a throwaway harness against the live npm attestation for
@playwright/cli@0.1.18(not included in this PR):Verified, signed by.../publish.yml@refs/tags/v0.1.18PackageDigestMismatch— signature still valid, subject binding caught itThe tightened implementation accepts genuine provenance and correctly rejects both identity and digest tampering.
This harness was re-run against the beta and again against stable
1.0.0, producing identical results across all three scenarios each time.CI failure root cause (not a Sigstore regression)
The first CI run failed on every job, including many with no Sigstore/Tuf reference at all (Milvus, Kafka, MongoDB, MySqlConnector, Dashboard). All failed at restore in ~40s–1m30s with
NU1902/NU1903on pre-existing transitive packages:Microsoft.OpenApi 3.5.3,SSH.NET 2024.2.0,MessagePack 2.5.198,SharpCompress 0.30.1,Snappier 1.0.0None are Sigstore or Tuf, none are in
Directory.Packages.props, and none were touched by this diff.Mechanism. NuGet audit is on by default but silently no-ops unless a configured source advertises a
VulnerabilityInforesource. The dnceng feeds expose none; nuget.org exposesVulnerabilityInfo/6.7.0. Adding nuget.org handed restore a vulnerability database for the first time, so audit began flagging long-standing transitive packages — andTreatWarningsAsErrors(Directory.Build.props) promoted those warnings into restore-breaking errors repo-wide.Fix. Added
<NuGetAudit>false</NuGetAudit>toDirectory.Build.props, annotated as part of the sameTEMPORARY - DO NOT MERGEblock. Verified both directions onAspire.Milvus.Client.Tests(a project with no Sigstore/Tuf reference):NuGetAudit=trueNU1903SSH.NET, GHSA-q939-rpr3-3284Aspire.Clibuilds 0 warnings / 0 errors and the fullAspire.Cli.Testssuite passes — includingSdkDumpCommandTests.SdkDumpCi_ForHostingProject_DoesNotEmitWarnings, which previously tripped onMicrosoft.OpenApiand now passes.CI status after the fix
Merged latest
mainand re-ran. 355 checks pass (previously 0 — every job died at restore). The 4 remaining failures are pre-existing or infrastructure, none Sigstore/Tuf related:Hosting.Sdk (windows-latest)mainstate locally and got the identical failures (ComputeRunArgumentsNormalizesRelativePathEntries,BuildFallsBackToDnxWhenPathAspireCliSetupTimesOut) — 100 ms-timeout tests from #19251Hosting.Azure (ubuntu-latest)failed to bind host port ... 1433/tcp: address already in useAspire CLI Starter Validation (Windows)aspire startexit 2. Theaspire runpath has no Sigstore involvement — the only consumers arePlaywrightCliInstallerandGitHubArtifactAttestationVerifier. Correlates with freshly-merged #15634 (TS AppHost dev cert)VS Code extension E2E (Windows)Error: spawnSync dotnet ETIMEDOUTNote: 1.0.0 swapped its crypto dependency
Sigstore 0.5.0depended onNSec.Cryptography 25.4.0;1.0.0depends onBouncyCastle.Cryptography 2.7.0instead. The dependency shape has been stable across alpha, beta, and stable (Sigstore→Tuf+BouncyCastle.Cryptography 2.7.0). Not implicated in any failure here, but worth being aware of for the real bump.Before this can ship
Mirror
Sigstore/Tuf1.0.0todotnet-public, then revert both temporary changes — the entireNuGet.configblock and<NuGetAudit>false</NuGetAudit>inDirectory.Build.props— keeping only theDirectory.Packages.propsversion bump.Checklist