Port NuGet Audit back to 9.0 #108854
Merged
Port NuGet Audit back to 9.0 #108854
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Enable NuGet Audit and fix issues Microsoft.NET.HostModel can reference the live builds of the packages it depends on. These will be deployed by the SDK.� Most other audit alerts were due to tasks pulling in old dependencies that aren't even used by the task. Avoid these by cherry-picking just the assemblies needed by the tasks and provided by MSBuild / SDK. This prevents NuGet from downloading the package closure with the vulnerable packages. We don't need those packages since the tasks aren't responsible for deploying them. A better solution in the future would be a targeting pack for MSBuild and the .NET SDK - so that components that contribute to these hosts have a surface area they can target without taking on responsibility for servicing. There is once case where we have a test that references NuGet.* packages which also bring in stale dependencies that overlap with framework assemblies. Avoid these by cherry-picking the NuGet packages in the same way. * Fix package path on linux * Only use live JSON from HostModel SDK pins S.R.M and a few others, so don't make them upgrade yet. * Add a couple missing assembly references * Refactor tasks dependencies Consolidate representation of msbuild-provided task dependencies * Fix audit warnings in tests * Remove MetadataLoadContext from WasmAppBuilder package * Update Analyzer.Testing packages * Reduce exposure of Microsoft.Build.Tasks.Core * Fix audit warnings that only occur on browser * Update Asn1 used by linker analyzer tests * React to breaking change in analyzer test SDK * Enable working DryIoc tests * Fix double-write when LibrariesConfiguration differs from Configuration * Fix LibrariesConfiguration update target * Clean up references and add comments. * Make HostModel references private This ensures projects referenced will not be rebuilt by tests. This also means the HostModel package will not list these as references, but that's OK since the SDK provides them and this is not a shipping package. * Use ProjectReferenceExclusion to avoid framework project references On .NETCore we want to use the targeting pack and avoid rebuilding libs. * Update src/libraries/System.Runtime.InteropServices.JavaScript/tests/JSImportGenerator.UnitTest/JSImportGenerator.Unit.Tests.csproj Co-authored-by: Jeremy Koritzinsky <jkoritzinsky@gmail.com> --------- Co-authored-by: Jeremy Koritzinsky <jkoritzinsky@gmail.com>
Since we're no longer trying to reference live S.T.J we don't need these.
ericstj
requested review from
marek-safar,
sbomer,
maraf,
akoeplinger,
lewing and
pavelsavara
as code owners
dotnet-issue-labeler
bot
added
the
needs-area-label
steveisok
approved these changes
Oct 14, 2024
jkoritzinsky
approved these changes
Oct 14, 2024
|
@rainersigwald - can we count on System.Text.Json 8.0.5 being present in all VS versions that support using the 9.0 GA SDK? I see the SDK is still on 8.0.4 in the 9.0.1xx branch https://github.com/dotnet/sdk/blob/0009da1479d71f7c69ba5ae8e4d0e91ef2f7e6eb/eng/Versions.props#L142-L144 |
|
No, it's not in 17.12-preview3 and I'm not aware of plans to push it to preview4+. It's in 17.13-preview1. Should we change that? |
|
I wanted to check before we pushed this into 9.0, I was triple checking this work for potential product fallout and this stood out.
I see - I think this means we need to keep the tasks at 8.0.4 and suppress the NuGet Audit warning. |
|
We just discussed this in Tactics but to preserve it: the current 17.11 patch release is relevant to the 9.0.100 SDK requirement to be buildable on day 1 of release, and it has 8.0.4, so the SDK must ship with 8.0.4 (for the references in build tasks and their dependencies). |
We cannot count on VS and MSBuild updating by the time 9.0 ships GA. Fix WASM projects which only target .NET by referencing the LKG and dropping all assets. For Microsoft.NET.HostModel and other build tasks, keep them on the version we can garuntee is present in VS. NoWarn the Audit warnings here. This is safe because we can ensure one of two things. 1. The package is non-shipping and customers won't see the warning and the referencing repo in the product will ensure an update or exclusion of the dependency. (HostModel) 2. The project excludes the reference entirely as making it PrivateAssets (not in package) and ExcludeAssets=runtime (no possibility of using runtime).
|
I made a change here. I think I also made it so we won't need to chase our tail here if we have future CVE's in 8.0 STJ. Can I get a new review? |
This was referenced Oct 15, 2024
Open
ericstj
added
Servicing-approved
|
Tell-mode infrastructure change, also discussed in tactics. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enables NuGet audit in 9.0 and fix the references of build tasks to avoid update churn.