Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NuGetPackageDownloader: Stop verifying the package signature on Mac and queue off of the environment on Linux #47463

Conversation

edvilme
Copy link
Member

@edvilme edvilme commented Mar 11, 2025

Backport of #47321

…tnet#47321)

Co-authored-by: Noah Gilson <OTAKUPENGUINOP@GMAIL.COM>
Co-authored-by: Forgind <12969783+Forgind@users.noreply.github.com>
@Copilot Copilot bot review requested due to automatic review settings March 11, 2025 17:54
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Workloads untriaged Request triage from a team member labels Mar 11, 2025

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 updates the NuGet package downloader to verify package signatures only on Windows by default while providing an override via an environment variable, and ensures that packages failing verification are deleted.

  • Added XML documentation for the _verifySignatures field.
  • Updated the constructor logic to conditionally verify signatures based on the operating system and an environment variable.
  • Wrapped the signature verification in a try-catch to delete packages that fail verification.
@edvilme edvilme requested a review from a team March 11, 2025 17:55
@edvilme edvilme changed the title NuGetPackageDownloader: Only verify signing on windows by default NuGetPackageDownloader: Stop verifying the package signature on Mac and queue off of the environment on Linux Mar 11, 2025
Copy link
Member

@nagilson nagilson left a comment

Choose a reason for hiding this comment

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

@mmitche @marcpopMSFT Does this change mean we will need to take a new p2 build again? Dotnet tool install on mac for tools with old certificates will fail without this change (was considered important to service 9.0.200.)

@marcpopMSFT
Copy link
Member

@nagilson They are taking another change from Aracade so decided to include this as well.

@edvilme edvilme enabled auto-merge (squash) March 11, 2025 18:39
@Forgind
Copy link
Member

Forgind commented Mar 11, 2025

I would say this doesn't need tactics approval because it was approved for 9.0.2xx, which should be a higher bar than 10.0.1xx-p2. Is that correct?

@marcpopMSFT
Copy link
Member

I would say this doesn't need tactics approval because it was approved for 9.0.2xx, which should be a higher bar than 10.0.1xx-p2. Is that correct?

It got tactics approval. It still needs that even for these because preview 2 is on a build by build slip so we can't just check in anything. This is only allowed because another change was going in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Workloads untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants