Warn when a vulnerable or EOL SDK version is resolved#53557
Conversation
There was a problem hiding this comment.
Pull request overview
Adds SDK vulnerability/EOL warnings to improve security visibility when a potentially vulnerable or out-of-support .NET SDK is resolved, using the same release metadata source as dotnet sdk check and a local on-disk cache to avoid introducing startup latency or network calls during MSBuild resolution.
Changes:
- Add CLI startup notifier that reads cached vulnerability/EOL info and triggers background cache refresh (opt-out + configurable refresh interval via env vars).
- Add MSBuild SDK resolver warnings sourced only from the local cache (no network calls).
- Add shared cache-reader + new unit tests and release-metadata test assets to validate the vulnerability/EOL evaluation logic.
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/dotnet.Tests/SdkVulnerabilityTests/GivenSdkVulnerabilityChecker.cs | Unit tests for vulnerability/EOL evaluation logic against test release metadata. |
| test/dotnet.Tests/SdkVulnerabilityTests/GivenSdkVulnerabilityCacheReader.cs | Unit tests for reading cached summary JSON (incl. disabled/no-cache cases). |
| test/dotnet.Tests/SdkVulnerabilityTests/GivenSdkReleaseMetadataCache.cs | Unit tests for cache sentinel interval behavior and cache summary reading. |
| test/TestAssets/TestReleases/VulnerabilityTestRelease/releases-index.json | Test releases-index metadata including an active and an EOL channel. |
| test/TestAssets/TestReleases/VulnerabilityTestRelease/9.0/releases.json | Test channel release metadata with CVEs used by unit tests. |
| test/TestAssets/TestReleases/VulnerabilityTestRelease/7.0/releases.json | Test EOL channel metadata with CVEs used by unit tests. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.zh-Hant.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.zh-Hans.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.tr.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.ru.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.pt-BR.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.pl.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.ko.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.ja.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.it.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.fr.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.es.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.de.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/xlf/Strings.cs.xlf | New localizable MSBuild resolver warning strings. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Strings.resx | Adds MSBuild resolver warning resources for vulnerable/EOL SDKs. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj | Links shared cache-reader into MSBuild resolver build. |
| src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs | Emits MSBuild warnings based on cached vulnerability/EOL summary data. |
| src/Common/SdkVulnerabilityCacheReader.cs | Shared cache-summary reader DTO + source-gen JSON context for CLI + resolver. |
| src/Common/EnvironmentVariableNames.cs | Adds env vars to disable the check and configure refresh interval. |
| src/Cli/dotnet/xlf/CliStrings.zh-Hant.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.zh-Hans.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.tr.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.ru.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.pt-BR.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.pl.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.ko.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.ja.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.it.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.fr.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.es.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.de.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/xlf/CliStrings.cs.xlf | New localizable CLI warning strings for vulnerable/EOL SDKs. |
| src/Cli/dotnet/dotnet.csproj | Links shared cache-reader into CLI build. |
| src/Cli/dotnet/SdkVulnerability/SdkVulnerabilityNotifier.cs | CLI startup hook to read cache, print warnings, and refresh cache in background. |
| src/Cli/dotnet/SdkVulnerability/SdkVulnerabilityInfo.cs | DTOs representing SDK vulnerability/EOL status and CVE entries. |
| src/Cli/dotnet/SdkVulnerability/SdkVulnerabilityChecker.cs | Pure logic that computes vulnerabilities/EOL status from release metadata. |
| src/Cli/dotnet/SdkVulnerability/SdkReleaseMetadataCache.cs | Cache manager: refresh interval/sentinel, background metadata fetch, and summary persistence. |
| src/Cli/dotnet/Program.cs | Invokes the notifier during CLI startup. |
| src/Cli/dotnet/CliStrings.resx | Adds CLI warning resources for vulnerable/EOL SDKs (incl. no-upgrade variants). |
|
Waiting for @baronfel's comment. We were discussing adding an experience for this once we had a way to update the SDK rather than going to the download page. We were hesitant as we didn't want to have to download releases.json on every CLI action so have not fully designed this feature yet. |
|
Summarizing offline conversation:
Overall, we like the direction of not hitting the network and using the cache instead and it makes sense to warn the user when their out of date. VS and VSCode will have more experiences like this in the future but they're going to have ways of upgrading the user for them which greatly improves the user experience. |
|
@marcpopMSFT Thanks for the feedback. I think the right move is to get warnings out of both the CLI startup and the SDK resolver, and put them in a .targets file instead. That one change actually addresses most of your points at once. Right now warnings fire from The cache refresh moves to For source-build, I'll gate the cache refresh with For detecting other install sources (VS-managed, install script, etc.), there's no runtime mechanism for that today and adding one means touching MSI, PKG, DEB, RPM, install scripts, and VS bundling. I'd rather defer that to a follow-up. The |
|
More offline conversation: Your suggestions about excluding from source build make sense though with this opt-in, becomes less critical. Another concern we had would be release day. We don't want there to be a timing problem that ends up breaking people's CI and local builds because the releases.json has updated but their SDK hasn't updated. For example, our own repos usually do a deliberate central rollout with new SDK versions which isn't immediate so this would flag during that validation window. I think we're still ok having it as an opt-in and we do plan on plugging dotnetup into this experience in the future. |
fc8aa27 to
b0eb48d
Compare
📋 SDK Diagnostic Documentation ReminderThis PR introduces 3 new SDK diagnostic codes:
Action RequiredPlease ensure that documentation for these diagnostics is added or updated in the dotnet/docs repository at:
Each diagnostic should have:
Thank you for helping keep our documentation up to date! 🙏 |
|
@marcpopMSFT Updated the PR. It's opt-in now. Warnings moved out of CLI startup and the SDK resolver entirely. They come from a .targets file that runs an MSBuild task during build. Cache refresh moved to On the release-day timing: since it's opt-in the blast radius is already small, but worth noting that CVE warnings only fire when there's a newer release with security fixes in the same channel. So the window only matters if someone opts in, their SDK version is in releases.json, and a newer patch exists they haven't rolled to yet. Repos doing a deliberate rollout could suppress the code in Directory.Build.props during that window. |
b0eb48d to
3730cbc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
test/dotnet.Tests/SdkVulnerabilityTests/GivenSdkReleaseMetadataCache.cs:140
- This test sets the sentinel timestamp via
File.SetLastAccessTimeandDateTime.Now, but the implementation checksGetLastWriteTimeUtcagainstDateTime.UtcNow. To make the interval logic test meaningful and non-flaky, setLastWriteTimeUtcinstead (and useUtcNow) so the 24h vs 1h assertions reflect the actual behavior.
string sentinelPath = Path.Combine(cacheDir, ".sentinel");
File.Create(sentinelPath).Close();
// Set sentinel to 2 hours ago
File.SetLastAccessTime(sentinelPath, DateTime.Now.AddHours(-2));
// With default 24h interval, should not be due
var cache24h = new SdkReleaseMetadataCache(cacheDir, _ => null);
cache24h.IsDueForUpdate().Should().BeFalse();
// With 1h interval, should be due
var cache1h = new SdkReleaseMetadataCache(
cacheDir,
name => name == EnvironmentVariableNames.SDK_VULNERABILITY_CHECK_INTERVAL_HOURS ? "1" : null);
cache1h.IsDueForUpdate().Should().BeTrue();
3730cbc to
fe95e84
Compare
fe95e84 to
49b724c
Compare
49b724c to
3c8028c
Compare
|
/azp run dotnet-sdk-public-ci |
|
Commenter does not have sufficient privileges for PR 53557 in repo dotnet/sdk |
|
/azp run dotnet-sdk-public-ci |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Marked as do-not-merge because this hasn't been reviewed by the SDK team yet. |
| /// <param name="productCollection">The product collection from releases-index.json.</param> | ||
| /// <param name="getProductReleases">A function to get releases for a product (may read from cache).</param> | ||
| /// <returns>Vulnerability info, or null if the SDK version cannot be matched to release data.</returns> | ||
| public static SdkVulnerabilityInfo? Check( |
There was a problem hiding this comment.
Does this check for discontinued feature band, for example, at some point we typically stop shipping 2xx or 3xx and only 1xx and 4xx would be in the latest release. So, even though the product (e.g. 8.0) is not EOL, we may have stopped producing 3xx. Essentially you'd want to check if the SDKs collection in the latest release contains the current feature band, if not, it's vulnerable, but technically only if there are at least one newer release that contains security fixes. Depends on how strict you want to be. The runtime of a newer release may contain CVEs, but those may not necessarily impact the SDK.
There was a problem hiding this comment.
Right now if someone's on a discontinued band, the checker still pulls in CVEs from newer releases (since those releases are newer than theirs) but FindLatestSdkInFeatureBand returns null because it only looks within the same band. So they get a "has known vulnerabilities, no upgrade available" warning pointing at the download page, which is misleading because there is an upgrade, they just need to switch bands.
I'll add a discontinued-band path: if there's no newer SDK in the user's band but product.LatestSdkVersion is greater than theirs, recommend that instead and use a new warning string saying the band is no longer serviced. Should just be a bool + string on SdkVulnerabilityInfo.
On runtime vs SDK CVE attribution, I looked and don't see a clean way to do it. The cves collection is per-release, not per-component, and there's no flag for runtime-only fixes. The SDK ships the runtime anyway, so a runtime CVE in your SDK's bundled runtime is genuinely a CVE in your install. I'd rather over-warn than try to parse CVE descriptions to guess scope. Let me know if you have a signal I'm missing though.
On strictness I'm leaning towards warn whenever a same-channel newer release with CVEs exists. Stricter than what you described but matches what I'd expect from a "vulnerable SDK" warning. Happy to dial it back if that's too aggressive.
There was a problem hiding this comment.
No, that sounds good to me. Starting off more aggressive feels like the correct approach given that we're dealing with vulnerabilities.
|
Do the checks handle newer SDKs that are not published? There are a couple of edge cases.
For early releases, I think it's fine to consider it in support, e.g. if the Major.Minor is newer than the latest product version, e.g. you installed .NET 12.0, but the last official product we shipped was 11.0. Same for SDKs, if the SDK version is newer than any known SDK and the product is not EOL, we should consider the SDK to be valid. |
It's doable, we have code that can do this on Windows, and do it extremely accurately, but it's expensive. If you want that to have value, you have to be accurate. |
5a89341 to
c4d64b9
Compare
All three are already handled. The
Case 1, 12.0 alpha when latest released is 11.0: channel
Case 2, daily build or new in-preview band on a known channel: channel found, SDK not in any release on it,
Case 3 is the same path as case 2. Channel can be EOL, but if the SDK isn't in releases.json we return null and emit nothing. For the 5.0.4xx scenario that seems right — those SDKs were deliberately shipped to let people target EOL frameworks, so warning about EOL and pointing at the download page would be actively wrong. I'll add a regression test for case 3 (EOL channel + unknown SDK -> null) so the behaviour doesn't drift. |
a1ec289 to
f6d729f
Compare
|
@baronfel do we have a tracking issue for docs? |
|
@joeloff No tracking issue, but I opened dotnet/docs#53870 with the three error pages and supporting cross-references. |
NETSDK1236 warns if the SDK has known CVEs. NETSDK1237 warns if it's end-of-life. Both are opt-in: set <CheckSdkVulnerabilities>true</> in your project or Directory.Build.props. <NoWarn> works too. Release metadata comes from the same releases-index.json that dotnet sdk check uses. The CLI fetches it in the background during restore and writes it to ~/.dotnet/sdk-vulnerability-cache/ with a 24h refresh sentinel. The MSBuild task reads from cache only, never hits the network. No cache file means no warnings, so air-gapped and source-build environments are fine. DOTNET_SDK_VULNERABILITY_CHECK_DISABLE and DOTNET_SDK_VULNERABILITY_CHECK_INTERVAL_HOURS control the behavior. Closes dotnet#49552
f6d729f to
1aecafb
Compare
Closes #49552
NuGet warns about vulnerable packages (NU1901-NU1904) but nothing warns about the SDK itself. This adds opt-in build warnings when the resolved SDK has known CVEs or is end of life.
Set
<CheckSdkVulnerabilities>true</CheckSdkVulnerabilities>in your project or Directory.Build.props. Off by default. Two warning codes: NETSDK1237 for vulnerabilities, NETSDK1238 for EOL. Suppressible with<NoWarn>if you want one but not the other.During restore, the CLI fetches release metadata in the background (same releases-index.json that
dotnet sdk checkuses) and caches it to~/.dotnet/sdk-vulnerability-cache/. The MSBuild task reads from that cache during build. It never hits the network itself. No cache file means no warnings, so air-gapped builds work fine.Cache refresh runs alongside the workload manifest updater in
RestoringCommand, 24h sentinel. Skipped in source-build.DOTNET_SDK_VULNERABILITY_CHECK_DISABLEandDOTNET_SDK_VULNERABILITY_CHECK_INTERVAL_HOURSare there if you need them.@baronfel would appreciate your input.