diff --git a/checks/raw/pinned_dependencies.go b/checks/raw/pinned_dependencies.go index f3d2d077be7..0e6896d8fcb 100644 --- a/checks/raw/pinned_dependencies.go +++ b/checks/raw/pinned_dependencies.go @@ -29,9 +29,15 @@ 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/remediation" ) +type dotnetCsprojLockedData struct { + Path string + LockedModeSet bool +} + // PinningDependencies checks for (un)pinned dependencies. func PinningDependencies(c *checker.CheckRequest) (checker.PinningDependenciesData, error) { var results checker.PinningDependenciesData @@ -61,9 +67,125 @@ 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 + } + } return results, nil } +func getUnpinnedNugetDependencies(pinningDependenciesData *checker.PinningDependenciesData) []*checker.Dependency { + var unpinnedNugetDependencies []*checker.Dependency + nugetDependencies := getDependenciesByType(pinningDependenciesData, checker.DependencyUseTypeNugetCommand) + for i := range nugetDependencies { + if !*nugetDependencies[i].Pinned { + unpinnedNugetDependencies = append(unpinnedNugetDependencies, nugetDependencies[i]) + } + } + return unpinnedNugetDependencies +} + +func getDependenciesByType(p *checker.PinningDependenciesData, + useType checker.DependencyUseType, +) []*checker.Dependency { + var deps []*checker.Dependency + for i := range p.Dependencies { + if p.Dependencies[i].Type == useType { + deps = append(deps, &p.Dependencies[i]) + } + } + 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 { + // 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 + + ": 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) { + for i := range dependencies { + if dependencies[i].Type == checker.DependencyUseTypeNugetCommand { + dependencies[i].Pinned = asBoolPointer(true) + dependencies[i].Remediation = nil + } + } +} + +func collectCsprojLockedModeData(c *checker.CheckRequest) ([]dotnetCsprojLockedData, error) { + var csprojDeps []dotnetCsprojLockedData + if err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: "*.csproj", + CaseSensitive: false, + }, analyseCsprojLockedMode, &csprojDeps, c.Dlogger); err != nil { + return nil, err + } + + return csprojDeps, nil +} + +func analyseCsprojLockedMode(path string, content []byte, args ...interface{}) (bool, error) { + pdata, ok := args[0].(*[]dotnetCsprojLockedData) + if !ok { + // panic if it is not correct type + panic(fmt.Sprintf("expected type *[]dotnetCsprojLockedData, got %v", reflect.TypeOf(args[0]))) + } + + pinned, err := csproj.IsRestoreLockedModeEnabled(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 csproj file: %v", err), + }) + return true, nil + } + + csprojData := dotnetCsprojLockedData{ + Path: path, + LockedModeSet: pinned, + } + + *pdata = append(*pdata, csprojData) + return true, nil +} + +func countUnlocked(csprojFiles []dotnetCsprojLockedData) (int, string) { + var unlockedPaths []string + + for i := range csprojFiles { + if !csprojFiles[i].LockedModeSet { + unlockedPaths = append(unlockedPaths, csprojFiles[i].Path) + } + } + return len(unlockedPaths), strings.Join(unlockedPaths, ", ") +} + func dataAsPinnedDependenciesPointer(data interface{}) *checker.PinningDependenciesData { pdata, ok := data.(*checker.PinningDependenciesData) if !ok { diff --git a/checks/raw/pinned_dependencies_test.go b/checks/raw/pinned_dependencies_test.go index a399cfb5634..53c3962d753 100644 --- a/checks/raw/pinned_dependencies_test.go +++ b/checks/raw/pinned_dependencies_test.go @@ -2084,3 +2084,326 @@ func TestCollectGitHubActionsWorkflowPinning(t *testing.T) { }) } } + +func TestCsProjAnalysis(t *testing.T) { + t.Parallel() + + //nolint:govet + tests := []struct { + unlocked int + expectErrorLogs bool + name string + filename string + }{ + { + name: "empty file", + filename: "./testdata/dotnet-empty.csproj", + expectErrorLogs: true, + }, + { + name: "locked mode enabled", + filename: "./testdata/dotnet-locked-mode-enabled.csproj", + unlocked: 0, + }, + { + name: "locked mode disabled", + filename: "./testdata/dotnet-locked-mode-disabled.csproj", + unlocked: 1, + }, + { + name: "locked mode disabled implicitly", + filename: "./testdata/dotnet-locked-mode-disabled-implicitly.csproj", + unlocked: 1, + }, + { + name: "invalid file", + filename: "./testdata/dotnet-invalid.csproj", + expectErrorLogs: true, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var content []byte + var err error + + content, err = os.ReadFile(tt.filename) + if err != nil { + t.Fatalf("cannot read file: %v", err) + } + + p := strings.Replace(tt.filename, "./testdata/", "", 1) + p = strings.Replace(p, "../testdata/", "", 1) + + var r []dotnetCsprojLockedData + dl := scut.TestDetailLogger{} + + _, err = analyseCsprojLockedMode(p, content, &r, &dl) + if err != nil { + t.Fatalf("unexpected error %v", err) + return + } + if tt.expectErrorLogs { + messages := dl.Flush() + if len(messages) != 0 && messages[0].Type != checker.DetailWarn { + t.Errorf("expected logged warning, got none") + } + } + + unlocked := 0 + for _, d := range r { + if !d.LockedModeSet { + unlocked++ + } + } + + if tt.unlocked != unlocked { + t.Errorf("expected %v. Got %v", tt.unlocked, unlocked) + } + }) + } +} + +func TestCollectInsecureNugetCsproj(t *testing.T) { + t.Parallel() + tests := []struct { + name string + filenames []string + stagedDependencies []*checker.Dependency + outcomeDependencies []*checker.Dependency + expectError bool + }{ + { + name: "pinned by command and 'locked mode' disabled implicitly", + filenames: []string{"./dotnet-locked-mode-disabled-implicitly.csproj"}, + expectError: false, + stagedDependencies: []*checker.Dependency{ + { + Type: checker.DependencyUseTypeNugetCommand, + Pinned: boolAsPointer(true), + Remediation: nil, + }, + }, + outcomeDependencies: []*checker.Dependency{ + { + Type: checker.DependencyUseTypeNugetCommand, + Pinned: boolAsPointer(true), + Remediation: nil, + }, + }, + }, + { + name: "unpinned by command and 'locked mode' disabled implicitly", + filenames: []string{"./dotnet-locked-mode-disabled-implicitly.csproj"}, + expectError: false, + stagedDependencies: []*checker.Dependency{ + { + Type: checker.DependencyUseTypeNugetCommand, + Pinned: boolAsPointer(false), + Remediation: &finding.Remediation{ + Text: "pin your dependecies by either using a lockfile (https://learn.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#locking-dependencies) or by enabling central package management (https://learn.microsoft.com/en-us/nuget/consume-packages/Central-Package-Management)", + }, + }, + }, + outcomeDependencies: []*checker.Dependency{ + { + Type: checker.DependencyUseTypeNugetCommand, + Pinned: boolAsPointer(false), + Remediation: &finding.Remediation{ + Text: "pin your dependecies by either using a lockfile (https://learn.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#locking-dependencies) or by enabling central package management (https://learn.microsoft.com/en-us/nuget/consume-packages/Central-Package-Management)", + }, + }, + }, + }, + { + name: "unpinned by command and 'locked mode' enabled", + filenames: []string{"./dotnet-locked-mode-enabled.csproj"}, + expectError: false, + stagedDependencies: []*checker.Dependency{ + { + Type: checker.DependencyUseTypeNugetCommand, + Pinned: boolAsPointer(false), + Remediation: &finding.Remediation{ + Text: "pin your dependecies by either using a lockfile (https://learn.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#locking-dependencies) or by enabling central package management (https://learn.microsoft.com/en-us/nuget/consume-packages/Central-Package-Management)", + }, + }, + }, + outcomeDependencies: []*checker.Dependency{ + { + Type: checker.DependencyUseTypeNugetCommand, + Pinned: boolAsPointer(true), + Remediation: nil, + }, + }, + }, + { + name: "unpinned by command and 'locked mode' enabled and disabled in different csproj files", + filenames: []string{"./dotnet-locked-mode-enabled.csproj", "./dotnet-locked-mode-disabled.csproj"}, + expectError: false, + stagedDependencies: []*checker.Dependency{ + { + Type: checker.DependencyUseTypeNugetCommand, + Pinned: boolAsPointer(false), + Remediation: &finding.Remediation{Text: "remediate"}, + }, + }, + outcomeDependencies: []*checker.Dependency{ + { + Type: checker.DependencyUseTypeNugetCommand, + Pinned: boolAsPointer(false), + Remediation: &finding.Remediation{Text: "remediate: some of your csproj files set the RestoreLockedMode property to true, while other do not set it: ./dotnet-locked-mode-disabled.csproj"}, + }, + }, + }, + } + + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockRepoClient := mockrepo.NewMockRepoClient(ctrl) + mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(tt.filenames, nil).AnyTimes() + mockRepoClient.EXPECT().GetDefaultBranchName().Return("main", nil).AnyTimes() + mockRepoClient.EXPECT().URI().Return("github.com/ossf/scorecard").AnyTimes() + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).AnyTimes().DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open(filepath.Join("testdata", file)) + }) + + req := checker.CheckRequest{ + RepoClient: mockRepoClient, + } + + err := processCsprojLockedMode(&req, tt.stagedDependencies) + if err != nil { + if !tt.expectError { + t.Error(err.Error()) + } + } + t.Log(tt.stagedDependencies) + if diff := cmp.Diff(tt.outcomeDependencies, tt.stagedDependencies); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestPinningDependenciesData_GetDependenciesByType(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + data checker.PinningDependenciesData + useType checker.DependencyUseType + expected []checker.Dependency + }{ + { + name: "No staged dependencies", + data: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{}, + }, + useType: checker.DependencyUseTypeGHAction, + expected: []checker.Dependency{}, + }, + { + name: "Single matching dependency", + data: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Name: newString("dep1"), + Type: checker.DependencyUseTypeGHAction, + }, + }, + }, + useType: checker.DependencyUseTypeGHAction, + expected: []checker.Dependency{ + { + Name: newString("dep1"), + Type: checker.DependencyUseTypeGHAction, + }, + }, + }, + { + name: "Multiple dependencies with one match", + data: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Name: newString("dep1"), + Type: checker.DependencyUseTypeGHAction, + }, + { + Name: newString("dep2"), + Type: checker.DependencyUseTypeDockerfileContainerImage, + }, + }, + }, + useType: checker.DependencyUseTypeGHAction, + expected: []checker.Dependency{ + { + Name: newString("dep1"), + Type: checker.DependencyUseTypeGHAction, + }, + }, + }, + { + name: "Multiple dependencies with multiple matches", + data: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Name: newString("dep1"), + Type: checker.DependencyUseTypeGHAction, + }, + { + Name: newString("dep2"), + Type: checker.DependencyUseTypeGHAction, + }, + }, + }, + useType: checker.DependencyUseTypeGHAction, + expected: []checker.Dependency{ + { + Name: newString("dep1"), + Type: checker.DependencyUseTypeGHAction, + }, + { + Name: newString("dep2"), + Type: checker.DependencyUseTypeGHAction, + }, + }, + }, + { + name: "No matching dependencies", + data: checker.PinningDependenciesData{ + Dependencies: []checker.Dependency{ + { + Name: newString("dep1"), + Type: checker.DependencyUseTypeDockerfileContainerImage, + }, + }, + }, + useType: checker.DependencyUseTypeGHAction, + expected: []checker.Dependency{}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := getDependenciesByType(&tt.data, tt.useType) + if len(result) != len(tt.expected) { + t.Errorf("Expected %d dependencies, got %d", len(tt.expected), len(result)) + } + for i, dep := range result { + if *dep.Name != *tt.expected[i].Name || dep.Type != tt.expected[i].Type { + t.Errorf("Expected dependency %v, got %v", tt.expected[i], dep) + } + } + }) + } +} + +func newString(s string) *string { + return &s +} diff --git a/checks/raw/shell_download_validate.go b/checks/raw/shell_download_validate.go index 488dbe88b0f..a5906b8af6f 100644 --- a/checks/raw/shell_download_validate.go +++ b/checks/raw/shell_download_validate.go @@ -994,6 +994,16 @@ func collectUnpinnedPackageManagerDownload(startLine, endLine uint, node syntax. // Nuget install and restore if isNuget(c) { + pinned := !isNugetUnpinned(c) + var remediation *finding.Remediation + if !pinned { + remediation = &finding.Remediation{ + Text: "pin your dependecies by either enabling central package management " + + "(https://learn.microsoft.com/nuget/consume-packages/Central-Package-Management) " + + "or using a lockfile (https://learn.microsoft.com/nuget/consume-packages/" + + "package-references-in-project-files#locking-dependencies)", + } + } r.Dependencies = append(r.Dependencies, checker.Dependency{ Location: &checker.File{ @@ -1003,11 +1013,10 @@ func collectUnpinnedPackageManagerDownload(startLine, endLine uint, node syntax. EndOffset: endLine, Snippet: cmd, }, - Pinned: asBoolPointer(!isNugetUnpinned(c)), - Type: checker.DependencyUseTypeNugetCommand, - }, - ) - + Pinned: &pinned, + Type: checker.DependencyUseTypeNugetCommand, + Remediation: remediation, + }) return } diff --git a/checks/raw/testdata/dotnet-empty.csproj b/checks/raw/testdata/dotnet-empty.csproj new file mode 100644 index 00000000000..e69de29bb2d diff --git a/checks/raw/testdata/dotnet-invalid.csproj b/checks/raw/testdata/dotnet-invalid.csproj new file mode 100644 index 00000000000..7b8af361b2b --- /dev/null +++ b/checks/raw/testdata/dotnet-invalid.csproj @@ -0,0 +1 @@ + + + + netstandard2.0 + + \ No newline at end of file diff --git a/checks/raw/testdata/dotnet-locked-mode-disabled.csproj b/checks/raw/testdata/dotnet-locked-mode-disabled.csproj new file mode 100644 index 00000000000..27e2e78c5a6 --- /dev/null +++ b/checks/raw/testdata/dotnet-locked-mode-disabled.csproj @@ -0,0 +1,10 @@ + + + + netstandard2.0 + + + + false + + \ No newline at end of file diff --git a/checks/raw/testdata/dotnet-locked-mode-enabled.csproj b/checks/raw/testdata/dotnet-locked-mode-enabled.csproj new file mode 100644 index 00000000000..13f9649e54a --- /dev/null +++ b/checks/raw/testdata/dotnet-locked-mode-enabled.csproj @@ -0,0 +1,10 @@ + + + + netstandard2.0 + + + + true + + \ No newline at end of file diff --git a/internal/csproj/csproj.go b/internal/csproj/csproj.go new file mode 100644 index 00000000000..c6aa3ad3852 --- /dev/null +++ b/internal/csproj/csproj.go @@ -0,0 +1,49 @@ +// Copyright 2024 OpenSSF Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package csproj + +import ( + "encoding/xml" + "errors" +) + +var errInvalidCsProjFile = errors.New("error parsing csproj file") + +type PropertyGroup struct { + XMLName xml.Name `xml:"PropertyGroup"` + RestoreLockedMode bool `xml:"RestoreLockedMode"` +} + +type Project struct { + XMLName xml.Name `xml:"Project"` + PropertyGroups []PropertyGroup `xml:"PropertyGroup"` +} + +func IsRestoreLockedModeEnabled(content []byte) (bool, error) { + var project Project + + err := xml.Unmarshal(content, &project) + if err != nil { + return false, errInvalidCsProjFile + } + + for _, propertyGroup := range project.PropertyGroups { + if propertyGroup.RestoreLockedMode { + return true, nil + } + } + + return false, nil +}