🐛 fix Docker remediations for unpinned GHA dependencies (#4131)

* 🐛 fix Docker remediations for unpinned GHA dependencies

Previously, as both the check for unpinned dependencies in
GitHub Actions and the check for unpinned Docker dependencies
contribute to d.Dependencies, the loop that created remediations
for Docker dependencies would also create try to create Docker
remediations for the unpinned GitHub Actions dependencies.

This could get really slow, especially when scanning a repo
with many GitHub Actions such as https://github.com/apache/beam.

Signed-off-by: Arnout Engelen <arnout@bzzt.net>

* 🌱 Small refactor and test for remediations

Signed-off-by: Arnout Engelen <arnout@bzzt.net>

* 🌱 make test data more realistic

Signed-off-by: Arnout Engelen <arnout@bzzt.net>

---------

Signed-off-by: Arnout Engelen <arnout@bzzt.net>

checks/raw/pinned_dependencies.go

@@ -228,14 +228,18 @@ func collectDockerfilePinning(c *checker.CheckRequest, r *checker.PinningDepende
 		return err
 	}
 
-	for i := range r.Dependencies {
-		rr := &r.Dependencies[i]
-		if !*rr.Pinned {
+	applyDockerfilePinningRemediations(r.Dependencies)
+	return nil
+}
+
+func applyDockerfilePinningRemediations(d []checker.Dependency) {
+	for i := range d {
+		rr := &d[i]
+		if rr.Type == checker.DependencyUseTypeDockerfileContainerImage && !*rr.Pinned {
 			remediate := remediation.CreateDockerfilePinningRemediation(rr, remediation.CraneDigester{})
 			rr.Remediation = remediate
 		}
 	}
-	return nil
 }
 
 var validateDockerfilesPinning fileparser.DoWhileTrueOnFileContent = func(
@@ -483,14 +487,18 @@ func collectGitHubActionsWorkflowPinning(c *checker.CheckRequest, r *checker.Pin
 	//nolint:errcheck
 	remediationMetadata, _ := remediation.New(c)
 
-	for i := range r.Dependencies {
-		rr := &r.Dependencies[i]
-		if !*rr.Pinned {
-			remediate := remediationMetadata.CreateWorkflowPinningRemediation(rr.Location.Path)
+	applyWorkflowPinningRemediations(remediationMetadata, r.Dependencies)
+	return nil
+}
+
+func applyWorkflowPinningRemediations(rm *remediation.RemediationMetadata, d []checker.Dependency) {
+	for i := range d {
+		rr := &d[i]
+		if rr.Type == checker.DependencyUseTypeGHAction && !*rr.Pinned {
+			remediate := rm.CreateWorkflowPinningRemediation(rr.Location.Path)
 			rr.Remediation = remediate
 		}
 	}
-	return nil
 }
 
 // validateGitHubActionWorkflow checks if the workflow file contains unpinned actions. Returns true if the check

checks/raw/pinned_dependencies_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/ossf/scorecard/v5/checker"
 	mockrepo "github.com/ossf/scorecard/v5/clients/mockclients"
 	"github.com/ossf/scorecard/v5/finding"
+	"github.com/ossf/scorecard/v5/remediation"
 	scut "github.com/ossf/scorecard/v5/utests"
 )
 
@@ -415,6 +416,75 @@ func TestDockerfilePinning(t *testing.T) {
 	}
 }
 
+func TestMixedPinning(t *testing.T) {
+	t.Parallel()
+
+	workflowDependency := checker.Dependency{
+		Name:     stringAsPointer("github/codeql-action/analyze"),
+		PinnedAt: nil,
+		Location: &checker.File{
+			Path:      ".github/workflows/workflow-not-pinned.yaml",
+			Snippet:   "github/codeql-action/analyze@v1",
+			Offset:    79,
+			EndOffset: 79,
+			FileSize:  0,
+			Type:      1,
+		},
+		Pinned:      boolAsPointer(false),
+		Type:        "GitHubAction",
+		Remediation: nil,
+	}
+	dockerDependency := checker.Dependency{
+		Name:     stringAsPointer("python"),
+		PinnedAt: stringAsPointer("3.7"),
+		Location: &checker.File{
+			Path:      "./testdata/Dockerfile-not-pinned",
+			Snippet:   "FROM python:3.7",
+			Offset:    17,
+			EndOffset: 17,
+			Type:      0,
+		},
+		Pinned: boolAsPointer(false),
+		Type:   "containerImage",
+	}
+	dependencies := []checker.Dependency{
+		workflowDependency,
+		dockerDependency,
+	}
+
+	//nolint:errcheck
+	remediationMetadata, _ := remediation.New(nil)
+	remediationMetadata.Branch = "mybranch"
+	remediationMetadata.Repo = "myrepo"
+
+	applyWorkflowPinningRemediations(remediationMetadata, dependencies)
+	if dependencies[0].Remediation == nil {
+		t.Errorf("No remediation added to workflow dependency")
+		return
+	}
+	if !strings.Contains(dependencies[0].Remediation.Text, "update your workflow") {
+		t.Errorf("Unexpected workflow remediation text")
+	}
+	if dependencies[1].Remediation != nil {
+		t.Errorf("Unexpected docker remediation")
+		return
+	}
+	applyDockerfilePinningRemediations(dependencies)
+	if dependencies[0].Remediation == nil {
+		t.Errorf("Remediation disappeared from workflow dependency")
+	}
+	if !strings.Contains(dependencies[0].Remediation.Text, "update your workflow") {
+		t.Errorf("Unexpected workflow remediation text")
+	}
+	if dependencies[1].Remediation == nil {
+		t.Errorf("No remediation added to docker dependency")
+		return
+	}
+	if !strings.Contains(dependencies[1].Remediation.Text, "Docker") {
+		t.Errorf("Unexpected Docker remediation text")
+	}
+}
+
 func TestFileIsInVendorDir(t *testing.T) {
 	t.Parallel()
 	tests := []struct {