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

[Poison] Add detection of reference assemblies #2817

Closed
crummel opened this issue Apr 7, 2022 · 7 comments · Fixed by dotnet/installer#17339
Closed

[Poison] Add detection of reference assemblies #2817

crummel opened this issue Apr 7, 2022 · 7 comments · Fixed by dotnet/installer#17339
Assignees
Labels
area-poison Poison leaks and the leak detection infrastructure

Comments

@crummel
Copy link
Contributor

crummel commented Apr 7, 2022

Currently, the leak detection infrastructure only poisons the previously-source-built nupkgs. We've talked with the runtime team and we also shouldn't ever be shipping reference assemblies - we can augment the poison reporting to detect these to help with this goal:

  • MarkAndCatalog task should include the ref packages to record the hashes of those assemblies as well.
  • CheckForPoison should include a new PoisonType, ReferenceAttribute or similar, that checks for the ReferenceAssembly attribute on shipped assemblies.
@ellahathaway
Copy link
Member

ellahathaway commented Aug 31, 2023

In discussing this issue with @NikolaMilosavljevic, it seems that adding in this feature might require some refactoring of the poison steps. Currently, there are reference assemblies are coming from the SBRP repo which is built after we poison the files, and there are reference assemblies in the previously-source-built artifacts package.

We've never done poisoning steps during build, which addressing this issue would require us to do should we use the reference assemblies from the SBRP repo. On the other hand, there are also the reference assemblies coming from the PSB artifacts package. Do we know which ones are used during the build?

Should we decide to use the assemblies from the SBRP repo, this issue might warrant some brainstorming.

@ellahathaway
Copy link
Member

ellahathaway commented Sep 15, 2023

In trying to address this, I'm having trouble finding some of the .dll files that are present in the SDK.

One reference assembly in particular is Microsoft.Win32.SystemEvents.dll. I expected this assembly to be in a nuget package or as a plain binary in the dotnet repo, but no such luck. The best I can trace it to is ./installer/src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/bin/Debug/net8.0/.dotnet/sdk/9.0.100-alpha.1.23431.1/Microsoft.Win32.SystemEvents.dll and ./dotnet/src/installer/artifacts/source-build/self/src/artifacts/bin/redist/Release/dotnet/sdk/9.0.100-alpha.1.23422.1/Microsoft.Win32.SystemEvents.dll. Is there an explanation for why this assembly is not present in PSB or SBRP?

@mthalman
Copy link
Member

It is in SBRP: https://github.com/dotnet/source-build-reference-packages/tree/main/src/referencePackages/src/microsoft.win32.systemevents/7.0.0. It would be in /vmr/prereqs/packages/reference during the build.

@ellahathaway
Copy link
Member

ellahathaway commented Sep 15, 2023

I should've been more specific. When poisoning SBRP (which included /vmr/prereqs/packages/reference and /vmr/artifacts/obj/x64/Release/blob-feed/packages/SourceBuildReferencePackages, done in different builds), I found that the reference assembly would be in the poison catalog, but the assembly was still in the resulting sdk and was not in the poison usage report. This led me to realize that the sha256 hashes of the ref assemblies in the catalog, all with the same name as the assembly in the sdk, did not match the hash of the assembly in the sdk. Additionally, the assembly in the sdk was missing the poison marker, indicating that it had not been poisoned and cataloged.

I've found multiple instances of the reference assembly with the same name in the repo, but none have the same sha256 hash and binary code as the reference assembly in the sdk. Here is a report of the sha256 hash of the reference assembly in the sdk and the other hashes of matching names in /vmr/prereqs/

Tldr: The reference assembly is not being identified as a leak when I poison SBRP, leading me to wonder where this reference assembly is being pulled from if it's not in SBRP.

@ellahathaway
Copy link
Member

@MichaelSimons MichaelSimons added area-testing Improvements in CI and testing area-prebuilts Reducing the number of prebuilt packages in the tarball area-poison Poison leaks and the leak detection infrastructure and removed area-testing Improvements in CI and testing area-prebuilts Reducing the number of prebuilt packages in the tarball area-infra Source-build infrastructure and reporting labels Oct 19, 2023
@ellahathaway
Copy link
Member

@ellahathaway
Copy link
Member

Closing with merge of dotnet/installer#17339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-poison Poison leaks and the leak detection infrastructure
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants