-
Notifications
You must be signed in to change notification settings - Fork 503
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
✨ Support Nuget Central Package Management #4369
base: main
Are you sure you want to change the base?
Changes from 4 commits
abbfe84
20eb987
d839200
dcc47a4
04fc62f
deb1350
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,8 @@ import ( | |
"github.com/ossf/scorecard/v5/checks/fileparser" | ||
sce "github.com/ossf/scorecard/v5/errors" | ||
"github.com/ossf/scorecard/v5/finding" | ||
"github.com/ossf/scorecard/v5/internal/csproj" | ||
"github.com/ossf/scorecard/v5/internal/dotnet/csproj" | ||
"github.com/ossf/scorecard/v5/internal/dotnet/properties" | ||
"github.com/ossf/scorecard/v5/remediation" | ||
) | ||
|
||
|
@@ -38,6 +39,11 @@ type dotnetCsprojLockedData struct { | |
LockedModeSet bool | ||
} | ||
|
||
type NugetPostProcessData struct { | ||
CsprojConfigs []dotnetCsprojLockedData | ||
CpmConfig properties.CentralPackageManagementConfig | ||
} | ||
|
||
// PinningDependencies checks for (un)pinned dependencies. | ||
func PinningDependencies(c *checker.CheckRequest) (checker.PinningDependenciesData, error) { | ||
var results checker.PinningDependenciesData | ||
|
@@ -67,14 +73,87 @@ func PinningDependencies(c *checker.CheckRequest) (checker.PinningDependenciesDa | |
return checker.PinningDependenciesData{}, err | ||
} | ||
|
||
if unpinnedNugetDependencies := getUnpinnedNugetDependencies(&results); len(unpinnedNugetDependencies) > 0 { | ||
if err := processCsprojLockedMode(c, unpinnedNugetDependencies); err != nil { | ||
return checker.PinningDependenciesData{}, err | ||
} | ||
// Nuget Post Processing | ||
if err := postProcessNugetDependencies(c, &results); err != nil { | ||
return checker.PinningDependenciesData{}, err | ||
} | ||
|
||
return results, nil | ||
} | ||
|
||
func postProcessNugetDependencies(c *checker.CheckRequest, | ||
pinningDependenciesData *checker.PinningDependenciesData, | ||
) error { | ||
if unpinnedDependencies := getUnpinnedNugetDependencies(pinningDependenciesData); len(unpinnedDependencies) > 0 { | ||
var nugetPostProcessData NugetPostProcessData | ||
if err := retrieveNugetCentralPackageManagement(c, &nugetPostProcessData); err != nil { | ||
return err | ||
} | ||
if err := retrieveCsprojConfig(c, &nugetPostProcessData); err != nil { | ||
return err | ||
} | ||
if nugetPostProcessData.CpmConfig.IsCPMEnabled { | ||
collectPostProcessNugetCPMDependencies(unpinnedDependencies, &nugetPostProcessData) | ||
} else { | ||
collectPostProcessNugetCsprojDependencies(unpinnedDependencies, &nugetPostProcessData) | ||
} | ||
} | ||
return nil | ||
Comment on lines
+87
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. optional: in go it's common to return early to reduce nesting: https://danp.net/posts/reducing-go-nesting/ so you could do something like this if you want (but I won't block on it): unpinnedDependencies := getUnpinnedNugetDependencies(pinningDependenciesData)
if len(unpinnedDependencies) == 0 {
return nil
}
// the rest of the function |
||
} | ||
|
||
func collectPostProcessNugetCPMDependencies(unpinnedNugetDependencies []*checker.Dependency, | ||
postProcessingData *NugetPostProcessData, | ||
) { | ||
packageVersions := postProcessingData.CpmConfig.PackageVersions | ||
|
||
numFixedVersions, unfixedVersions := countFixedVersions(packageVersions) | ||
// if all dependencies are fixed to specific versions, pin all dependencies | ||
if numFixedVersions == len(packageVersions) { | ||
Comment on lines
+109
to
+111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the variable is called |
||
pinAllNugetDependencies(unpinnedNugetDependencies) | ||
Comment on lines
+104
to
+112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is CPM all-or-nothing for managing (direct) dependencies, or can you manage only a subset centrally? For example, can you pin I want to make sure I understand the feature. CPM is just a configuration feature so you don't need to specify versions in more than one place? The reason I'm asking is to see if any of the rules (pinned to specific version) should also apply to all project files? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CPM is all or nothing for an individual project. By enabling it in |
||
} else { | ||
// if some or all dependencies are not fixed to specific versions, update the remediation | ||
for i := range unpinnedNugetDependencies { | ||
(unpinnedNugetDependencies)[i].Remediation.Text = (unpinnedNugetDependencies)[i].Remediation.Text + | ||
": some of dependency versions are not fixes to specific versions: " + unfixedVersions | ||
} | ||
} | ||
} | ||
|
||
func retrieveNugetCentralPackageManagement(c *checker.CheckRequest, nugetPostProcessData *NugetPostProcessData) error { | ||
if err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ | ||
Pattern: "Directory.*.props", | ||
Comment on lines
+123
to
+124
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does Jeff's comment matter here? If you have the pattern broad for the purposes of the test harness, I can help with that.
|
||
CaseSensitive: false, | ||
}, processDirectoryPropsFile, nugetPostProcessData, c.Dlogger); err != nil { | ||
return err | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func processDirectoryPropsFile(path string, content []byte, args ...interface{}) (bool, error) { | ||
pdata, ok := args[0].(*NugetPostProcessData) | ||
if !ok { | ||
// panic if it is not correct type | ||
panic(fmt.Sprintf("expected type NugetPostProcessData, got %v", reflect.TypeOf(args[0]))) | ||
} | ||
|
||
cpmConfig, err := properties.GetCentralPackageManagementConfig(path, content) | ||
if err != nil { | ||
dl, ok := args[1].(checker.DetailLogger) | ||
if !ok { | ||
// panic if it is not correct type | ||
panic(fmt.Sprintf("expected type checker.DetailLogger, got %v", reflect.TypeOf(args[1]))) | ||
} | ||
|
||
dl.Warn(&checker.LogMessage{ | ||
Text: fmt.Sprintf("malformed properties file: %v", err), | ||
}) | ||
return true, nil | ||
} | ||
pdata.CpmConfig = cpmConfig | ||
return false, nil | ||
} | ||
|
||
func getUnpinnedNugetDependencies(pinningDependenciesData *checker.PinningDependenciesData) []*checker.Dependency { | ||
var unpinnedNugetDependencies []*checker.Dependency | ||
nugetDependencies := getDependenciesByType(pinningDependenciesData, checker.DependencyUseTypeNugetCommand) | ||
|
@@ -98,30 +177,25 @@ func getDependenciesByType(p *checker.PinningDependenciesData, | |
return deps | ||
} | ||
|
||
func processCsprojLockedMode(c *checker.CheckRequest, dependencies []*checker.Dependency) error { | ||
csprojDeps, err := collectCsprojLockedModeData(c) | ||
if err != nil { | ||
return err | ||
} | ||
unlockedCsprojDeps, unlockedPath := countUnlocked(csprojDeps) | ||
|
||
// none of the csproject files set RestoreLockedMode. Keep the same status of the nuget dependencies | ||
if unlockedCsprojDeps == len(csprojDeps) { | ||
return nil | ||
} | ||
|
||
// all csproj files set RestoreLockedMode, update the dependency pinning status of all nuget dependencies to pinned | ||
if unlockedCsprojDeps == 0 { | ||
pinAllNugetDependencies(dependencies) | ||
} else { | ||
func collectPostProcessNugetCsprojDependencies(unpinnedNugetDependencies []*checker.Dependency, | ||
postProcessingData *NugetPostProcessData, | ||
) { | ||
unlockedCsprojDeps, unlockedPath := countUnlocked(postProcessingData.CsprojConfigs) | ||
switch unlockedCsprojDeps { | ||
case len(postProcessingData.CsprojConfigs): | ||
// none of the csproject files set RestoreLockedMode. Keep the same status of the nuget dependencies | ||
return | ||
case 0: | ||
// all csproj files set RestoreLockedMode, update the dependency pinning status of all nuget dependencies to pinned | ||
pinAllNugetDependencies(unpinnedNugetDependencies) | ||
default: | ||
// only some csproj files are locked, keep the same status of the nuget dependencies but create a remediation | ||
for i := range dependencies { | ||
(dependencies)[i].Remediation.Text = (dependencies)[i].Remediation.Text + | ||
for i := range unpinnedNugetDependencies { | ||
(unpinnedNugetDependencies)[i].Remediation.Text = (unpinnedNugetDependencies)[i].Remediation.Text + | ||
": some of your csproj files set the RestoreLockedMode property to true, " + | ||
"while other do not set it: " + unlockedPath | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func pinAllNugetDependencies(dependencies []*checker.Dependency) { | ||
|
@@ -133,16 +207,15 @@ func pinAllNugetDependencies(dependencies []*checker.Dependency) { | |
} | ||
} | ||
|
||
func collectCsprojLockedModeData(c *checker.CheckRequest) ([]dotnetCsprojLockedData, error) { | ||
var csprojDeps []dotnetCsprojLockedData | ||
func retrieveCsprojConfig(c *checker.CheckRequest, nugetPostProcessData *NugetPostProcessData) error { | ||
if err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ | ||
Pattern: "*.csproj", | ||
CaseSensitive: false, | ||
}, analyseCsprojLockedMode, &csprojDeps, c.Dlogger); err != nil { | ||
return nil, err | ||
}, analyseCsprojLockedMode, &nugetPostProcessData.CsprojConfigs, c.Dlogger); err != nil { | ||
return err | ||
} | ||
|
||
return csprojDeps, nil | ||
return nil | ||
} | ||
|
||
func analyseCsprojLockedMode(path string, content []byte, args ...interface{}) (bool, error) { | ||
|
@@ -186,6 +259,17 @@ func countUnlocked(csprojFiles []dotnetCsprojLockedData) (int, string) { | |
return len(unlockedPaths), strings.Join(unlockedPaths, ", ") | ||
} | ||
|
||
func countFixedVersions(packages []properties.NugetPackage) (int, string) { | ||
var unfixedVersions []string | ||
Comment on lines
+262
to
+263
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this counts unfixed versions |
||
|
||
for i := range packages { | ||
if !packages[i].IsFixed { | ||
unfixedVersions = append(unfixedVersions, packages[i].Version) | ||
} | ||
} | ||
return len(unfixedVersions), strings.Join(unfixedVersions, ", ") | ||
} | ||
|
||
func dataAsPinnedDependenciesPointer(data interface{}) *checker.PinningDependenciesData { | ||
pdata, ok := data.(*checker.PinningDependenciesData) | ||
if !ok { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not export this type