build: declare CPM in Directory.Build.props for Scorecard detection#96
Merged
Merged
Conversation
Enabling Central Package Management in Directory.Packages.props (#94, #95) did not move Scorecard's Pinned-Dependencies check off 7: the v5.3.0 run still reported "0/2 nugetCommand dependencies pinned" with only the base remediation text, i.e. its NuGet check evaluated CPM as disabled. Root cause (internal/dotnet/properties/properties.go): the check reads every Directory.*.props file and keeps only the LAST one's config when deciding whether CPM is enabled. This repo also has Directory.Build.props, which carries no ManagePackageVersionsCentrally element; read after Directory.Packages.props, it overwrites the "enabled" verdict with "disabled". Declare ManagePackageVersionsCentrally=true in Directory.Build.props as well. Since it has no <PackageVersion>, the empty version set trivially satisfies the "all versions are fixed" rule, so the restore is credited as pinned. MSBuild behaviour is unchanged (redundant true; FloorCheck still opts out in its csproj); restore + build + 598 tests stay green. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_014W1zrcBNy3jEs8ngAQKwZg
This file contains hidden or 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
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.
Summary
Third and (source-backed) fix for OpenSSF Scorecard's Pinned-Dependencies check. Enabling Central Package Management in
Directory.Packages.props(#94, #95) never moved the check off 7 — the v5.3.0 run kept reporting0/2 nugetCommand dependencies pinnedwith only the base remediation text, i.e. its NuGet check evaluated CPM as disabled.Root cause (
internal/dotnet/properties/properties.go): the check reads everyDirectory.*.propsfile and keeps only the last one's config when deciding whether CPM is enabled (it overwrites its config per file). This repo also hasDirectory.Build.props, which has noManagePackageVersionsCentrallyelement; read afterDirectory.Packages.props, it overwrites the "enabled" verdict with "disabled". (This also explains why the earlier comment-rewording had no effect — Scorecard usesxml.Unmarshal, which ignores comments.)Fix: declare
ManagePackageVersionsCentrally=trueinDirectory.Build.propstoo. It carries no<PackageVersion>, so the empty version set trivially satisfies Scorecard's "all versions are fixed" rule and the restore is credited as pinned.Note: confirmation can only come from the next
scorecardrun onmain(Scorecard isn't runnable locally here). Given the current run provesDirectory.Build.propsis the last-scanned props file, this is well-grounded — but it's the fix, not a guarantee.Type of change
Changes
Directory.Build.props: added a<PropertyGroup>with<ManagePackageVersionsCentrally>true</...>(redundant for MSBuild — same value asDirectory.Packages.props— with a comment explaining the Scorecard tooling reason). FloorCheck still overrides it tofalsein its own project file.Testing
dotnet build FirstClassErrors.sln(Release, with the CI warning ratchet — 0 warnings)dotnet test FirstClassErrors.sln(Release — 598 passed, 0 failed)FirstClassErrors.Analyzers.UnitTests)Documentation
doc/updateddoc/README.fr.md) updated if user-facing behavior changedRelated issues
None.
🤖 Generated with Claude Code
https://claude.ai/code/session_014W1zrcBNy3jEs8ngAQKwZg
Generated by Claude Code