🐛 Retain tag when remediating unpinned docker images. (ossf#2595)

Signed-off-by: Spencer Schrock <sschrock@google.com> Signed-off-by: Spencer Schrock <sschrock@google.com>
raghavkaul · Feb 9, 2023 · 7a28da6 · 7a28da6
1 parent ffe0187
commit 7a28da6
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 6 deletions.
diff --git a/checks/evaluation/pinned_dependencies.go b/checks/evaluation/pinned_dependencies.go
@@ -141,7 +141,7 @@ func generateRemediation(remediaitonMd remediation.RemediationMetadata, rr *chec
 	case checker.DependencyUseTypeGHAction:
 		return remediaitonMd.CreateWorkflowPinningRemediation(rr.Location.Path)
 	case checker.DependencyUseTypeDockerfileContainerImage:
-		return remediation.CreateDockerfilePinningRemediation(rr.Name)
+		return remediation.CreateDockerfilePinningRemediation(rr, remediation.CraneDigester{})
 	default:
 		return nil
 	}
@@ -183,7 +183,6 @@ func generateOwnerToDisplay(gitHubOwned bool) string {
 }
 
 // TODO(laurent): need to support GCB pinning.
-//nolint
 func maxScore(s1, s2 int) int {
 	if s1 > s2 {
 		return s1

diff --git a/remediation/remediations.go b/remediation/remediations.go
@@ -85,17 +85,38 @@ func (r *RemediationMetadata) createWorkflowRemediation(path, t string) *checker
 	}
 }
 
+func dockerImageName(d *checker.Dependency) (name string, ok bool) {
+	if d.Name == nil || *d.Name == "" {
+		return "", false
+	}
+	if d.PinnedAt != nil && *d.PinnedAt != "" {
+		return fmt.Sprintf("%s:%s", *d.Name, *d.PinnedAt), true
+	}
+	return *d.Name, true
+}
+
+type Digester interface{ Digest(string) (string, error) }
+
+type CraneDigester struct{}
+
+func (c CraneDigester) Digest(name string) (string, error) {
+	//nolint:wrapcheck // error value not used
+	return crane.Digest(name)
+}
+
 // CreateDockerfilePinningRemediation create remediaiton for pinning Dockerfile images.
-func CreateDockerfilePinningRemediation(name *string) *checker.Remediation {
-	if name == nil {
+func CreateDockerfilePinningRemediation(dep *checker.Dependency, digester Digester) *checker.Remediation {
+	name, ok := dockerImageName(dep)
+	if !ok {
 		return nil
 	}
-	hash, err := crane.Digest(*name)
+
+	hash, err := digester.Digest(name)
 	if err != nil {
 		return nil
 	}
 
-	text := fmt.Sprintf(dockerfilePinText, *name, hash)
+	text := fmt.Sprintf(dockerfilePinText, name, hash)
 	markdown := text
 
 	return &checker.Remediation{

diff --git a/remediation/remediations_test.go b/remediation/remediations_test.go
@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/google/go-cmp/cmp"
 
 	"github.com/ossf/scorecard/v4/checker"
 	mockrepo "github.com/ossf/scorecard/v4/clients/mockclients"
@@ -49,3 +50,92 @@ func TestRepeatedSetup(t *testing.T) {
 		}
 	}
 }
+
+func asPointer(s string) *string {
+	return &s
+}
+
+type stubDigester struct{}
+
+func (s stubDigester) Digest(name string) (string, error) {
+	m := map[string]string{
+		"foo":               "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae",
+		"baz":               "fcde2b2edba56bf408601fb721fe9b5c338d10ee429ea04fae5511b68fbf8fb9",
+		"amazoncorretto:11": "b1a711069b801a325a30885f08f5067b2b102232379750dda4d25a016afd9a88",
+	}
+	hash, ok := m[name]
+	if !ok {
+		//nolint:goerr113
+		return "", fmt.Errorf("no hash for image: %q", name)
+	}
+	return fmt.Sprintf("sha256:%s", hash), nil
+}
+
+func TestCreateDockerfilePinningRemediation(t *testing.T) {
+	t.Parallel()
+
+	//nolint:govet,lll
+	tests := []struct {
+		name     string
+		dep      checker.Dependency
+		expected *checker.Remediation
+	}{
+		{
+			name:     "no depdendency",
+			dep:      checker.Dependency{},
+			expected: nil,
+		},
+		{
+			name: "image name no tag",
+			dep: checker.Dependency{
+				Name: asPointer("foo"),
+				Type: checker.DependencyUseTypeDockerfileContainerImage,
+			},
+			expected: &checker.Remediation{
+				HelpText:     "pin your Docker image by updating foo to foo@sha256:2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae",
+				HelpMarkdown: "pin your Docker image by updating foo to foo@sha256:2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae",
+			},
+		},
+		{
+			// github.com/ossf/scorecard/issues/2581
+			name: "image name with tag",
+			dep: checker.Dependency{
+				Name:     asPointer("amazoncorretto"),
+				PinnedAt: asPointer("11"),
+				Type:     checker.DependencyUseTypeDockerfileContainerImage,
+			},
+			expected: &checker.Remediation{
+				HelpText:     "pin your Docker image by updating amazoncorretto:11 to amazoncorretto:11@sha256:b1a711069b801a325a30885f08f5067b2b102232379750dda4d25a016afd9a88",
+				HelpMarkdown: "pin your Docker image by updating amazoncorretto:11 to amazoncorretto:11@sha256:b1a711069b801a325a30885f08f5067b2b102232379750dda4d25a016afd9a88",
+			},
+		},
+		{
+			name: "unknown image",
+			dep: checker.Dependency{
+				Name: asPointer("not-found"),
+				Type: checker.DependencyUseTypeDockerfileContainerImage,
+			},
+			expected: nil,
+		},
+		{
+			name: "unknown tag",
+			dep: checker.Dependency{
+				Name:     asPointer("foo"),
+				PinnedAt: asPointer("not-found"),
+				Type:     checker.DependencyUseTypeDockerfileContainerImage,
+			},
+			expected: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := CreateDockerfilePinningRemediation(&tt.dep, stubDigester{})
+			if !cmp.Equal(got, tt.expected) {
+				t.Errorf(cmp.Diff(got, tt.expected))
+			}
+		})
+	}
+}