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

Evaluate usefulness and correctness of ValidateBundledManifestSigning target #41081

Open
mmitche opened this issue Jan 10, 2024 · 3 comments
Open
Assignees
Labels
Area-NetSDK untriaged Request triage from a team member

Comments

@mmitche
Copy link
Member

mmitche commented Jan 10, 2024

This target is supposed to validate the signing of bundled manifests. But, it is disabled for workloads that are post-build signed. I think this is probably largely a holdover from 6.0. Now nothing is post-build signed, but we don't sign some workloads out of the main branch, so removing the exclusions for emscripten in the target would simply cause the build to fail.

https://github.com/dotnet/installer/blob/0b82a5aa64ad639184b1da058a7f11323d89a031/src/redist/targets/BundledManifests.targets#L54-L89

On the other hand if the exclusion is always there, and signing is done in release branches, then we're never checking emscripten's signing. We do end up checking Maui (they always sign), but Aspire does not.

Is this target still valuable? Should it be changed somehow?

/cc @joeloff

@mmitche
Copy link
Member Author

mmitche commented Jan 10, 2024

@joperezr

@marcpopMSFT
Copy link
Member

I believe that the exclusion list is leftover from the post-build signing days but as you noted, still applies to main since we don't sign main builds of runtime/emsdk.

I guess the question is, should we have any signing validation here? I believe Jacques added it after we had issues in VS with signed workloads in the early days that weren't caught with the existing arcade sign checking. We haven't had those issues recently.

Is the check bothersome or slow such that we'd want to get rid of it? I can update the comment if we want to keep it.

@joeloff
Copy link
Member

joeloff commented Feb 20, 2024

Correct. We had manifests from runtime flow through in 6.0 to installer, then when we staged and inserted into Visual Studio we'd trip PR checks because the runtime manifest wasn't signed, which then requires resetting the whole build of 6.0.

@ViktorHofer ViktorHofer transferred this issue from dotnet/installer May 22, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-NetSDK untriaged Request triage from a team member labels May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-NetSDK untriaged Request triage from a team member
Projects
None yet
Development

No branches or pull requests

3 participants