updated comments

Signed-off-by: Asra Ali <[email protected]>

Author: asraa

cli/slsa-verifier/main.go

@@ -9,28 +9,28 @@ import (
 	"io"
 	"os"
 
-	"github.com/slsa-framework/slsa-verifier/container"
 	"github.com/slsa-framework/slsa-verifier/options"
 	"github.com/slsa-framework/slsa-verifier/verifiers"
+	"github.com/slsa-framework/slsa-verifier/verifiers/container"
 )
 
 var (
-	provenancePath    string
-	builderID         string
-	artifactPath      string
-	artifactReference string
-	source            string
-	branch            string
-	tag               string
-	versiontag        string
-	printProvenance   bool
+	provenancePath  string
+	builderID       string
+	artifactPath    string
+	artifactImage   string
+	source          string
+	branch          string
+	tag             string
+	versiontag      string
+	printProvenance bool
 )
 
 func main() {
 	flag.StringVar(&builderID, "builder-id", "", "EXPERIMENTAL: the unique builder ID who created the provenance")
 	flag.StringVar(&provenancePath, "provenance", "", "path to a provenance file")
 	flag.StringVar(&artifactPath, "artifact-path", "", "path to an artifact to verify")
-	flag.StringVar(&artifactReference, "artifact-reference", "", "reference to an OCI image to verify")
+	flag.StringVar(&artifactImage, "artifact-image", "", "name of the OCI image to verify")
 	flag.StringVar(&source, "source", "",
 		"expected source repository that should have produced the binary, e.g. github.com/some/repo")
 	flag.StringVar(&branch, "branch", "", "[optional] expected branch the binary was compiled from")
@@ -41,8 +41,14 @@ func main() {
 		"print the verified provenance to std out")
 	flag.Parse()
 
-	if (provenancePath == "" || artifactPath == "") && artifactReference == "" {
-		fmt.Fprintf(os.Stderr, "either 'provenance' and 'artifact-path' or '' must be specified\n")
+	if (provenancePath == "" || artifactPath == "") && artifactImage == "" {
+		fmt.Fprintf(os.Stderr, "either 'provenance' and 'artifact-path' or 'artifact-image' must be specified\n")
+		flag.Usage()
+		os.Exit(1)
+	}
+
+	if artifactImage != "" && (provenancePath != "" || artifactPath != "") {
+		fmt.Fprintf(os.Stderr, "'provenance' and 'artifact-path' should not be specified when 'artifact-image' is provided\n")
 		flag.Usage()
 		os.Exit(1)
 	}
@@ -73,7 +79,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	verifiedProvenance, _, err := runVerify(artifactReference, artifactPath, provenancePath,
+	verifiedProvenance, _, err := runVerify(artifactImage, artifactPath, provenancePath,
 		source, pbranch, pbuilderID, ptag, pversiontag)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "FAILED: SLSA verification failed: %v\n", err)
@@ -98,13 +104,13 @@ func isFlagPassed(name string) bool {
 	return found
 }
 
-func runVerify(artifactReference, artifactPath, provenancePath, source string,
+func runVerify(artifactImage, artifactPath, provenancePath, source string,
 	branch, builderID, ptag, pversiontag *string,
 ) ([]byte, string, error) {
 	ctx := context.Background()
 
 	// Artifact hash retrieval depends on the artifact type.
-	artifactHash, err := getArtifactHash(artifactReference, artifactPath)
+	artifactHash, err := getArtifactHash(artifactImage, artifactPath)
 	if err != nil {
 		return nil, "", err
 	}
@@ -129,10 +135,10 @@ func runVerify(artifactReference, artifactPath, provenancePath, source string,
 		}
 	}
 
-	return verifiers.Verify(ctx, artifactReference, provenance, artifactHash, provenanceOpts, builderOpts)
+	return verifiers.Verify(ctx, artifactImage, provenance, artifactHash, provenanceOpts, builderOpts)
 }
 
-func getArtifactHash(artifactReference, artifactPath string) (string, error) {
+func getArtifactHash(artifactImage, artifactPath string) (string, error) {
 	if artifactPath != "" {
 		f, err := os.Open(artifactPath)
 		if err != nil {
@@ -146,5 +152,5 @@ func getArtifactHash(artifactReference, artifactPath string) (string, error) {
 		return hex.EncodeToString(h.Sum(nil)), nil
 	}
 	// Retrieve image digest
-	return container.GetImageDigest(artifactReference)
+	return container.GetImageDigest(artifactImage)
 }

cli/slsa-verifier/main_test.go

@@ -18,8 +18,8 @@ import (
 	"github.com/sigstore/cosign/pkg/oci"
 	"github.com/sigstore/cosign/pkg/oci/layout"
 
-	"github.com/slsa-framework/slsa-verifier/container"
 	serrors "github.com/slsa-framework/slsa-verifier/errors"
+	"github.com/slsa-framework/slsa-verifier/verifiers/container"
 )
 
 func errCmp(e1, e2 error) bool {
@@ -32,18 +32,48 @@ func pString(s string) *string {
 
 const TEST_DIR = "./testdata"
 
-func Test_runVerifyGoAndGeneric(t *testing.T) {
-	// t.Parallel()
+var ARTIFACT_PATH_BUILDERS = []string{"go", "generic"}
+var ARTIFACT_IMAGE_BUILDERS = []string{"generic_container"}
+
+func getBuildersAndVersions(t *testing.T,
+	optionalMinVersion string, specifiedBuilders []string,
+	defaultBuilders []string) []string {
+	res := []string{}
+	builders := specifiedBuilders
+	if len(builders) == 0 {
+		builders = defaultBuilders
+	}
+	// Get versions for each builder.
+	for _, builder := range builders {
+		builderDir, err := ioutil.ReadDir(filepath.Join(TEST_DIR, builder))
+		if err != nil {
+			t.Error(err)
+		}
+		for _, f := range builderDir {
+			// Builder subfolders are semantic version strings.
+			// Compare if a min version is given.
+			if f.IsDir() && (optionalMinVersion == "" ||
+				semver.Compare(optionalMinVersion, f.Name()) <= 0) {
+				// These are the supported versions of the builder
+				res = append(res, filepath.Join(builder, f.Name()))
+			}
+		}
+	}
+	return res
+}
+
+func Test_runVerifyArtifactPath(t *testing.T) {
+	t.Parallel()
 	tests := []struct {
-		name        string
-		artifact    string
-		source      string
-		pbranch     *string
-		ptag        *string
-		pversiontag *string
-		pbuilderID  *string
-		builderID   string
-		err         error
+		name         string
+		artifact     string
+		source       string
+		pbranch      *string
+		ptag         *string
+		pversiontag  *string
+		pbuilderID   *string
+		outBuilderId string
+		err          error
 		// noversion is a special case where we are not testing all builder versions
 		// for example, testdata for the builder at head in trusted repo workflows
 		// or testdata from malicious untrusted builders.
@@ -338,22 +368,22 @@ func Test_runVerifyGoAndGeneric(t *testing.T) {
 			builders:   []string{"generic"},
 		},
 		{
-			name:       "multiple subject second match - builderID",
-			artifact:   "binary-linux-amd64-multi-subject-second",
-			source:     "github.com/slsa-framework/example-package",
-			minversion: "v1.2.0",
-			builders:   []string{"generic"},
-			pbuilderID: pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml"),
-			builderID:  "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml",
+			name:         "multiple subject second match - builderID",
+			artifact:     "binary-linux-amd64-multi-subject-second",
+			source:       "github.com/slsa-framework/example-package",
+			minversion:   "v1.2.0",
+			builders:     []string{"generic"},
+			pbuilderID:   pString("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml"),
+			outBuilderId: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml",
 		},
 		// Special case of the e2e test repository building builder from head.
 		{
-			name:      "e2e test repository verified with builder at head",
-			artifact:  "binary-linux-amd64-e2e-builder-repo",
-			source:    "github.com/slsa-framework/example-package",
-			pbranch:   pString("main"),
-			noversion: true,
-			builderID: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_go_slsa3.yml",
+			name:         "e2e test repository verified with builder at head",
+			artifact:     "binary-linux-amd64-e2e-builder-repo",
+			source:       "github.com/slsa-framework/example-package",
+			pbranch:      pString("main"),
+			noversion:    true,
+			outBuilderId: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_go_slsa3.yml",
 		},
 		// Malicious builders and workflows.
 		{
@@ -407,32 +437,8 @@ func Test_runVerifyGoAndGeneric(t *testing.T) {
 		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()
-			getBuildersAndVersions := func(minversion string, ttBuilders []string) []string {
-				res := []string{}
-				builders := tt.builders
-				if len(builders) == 0 {
-					// This tests both generic and go
-					builders = []string{"go", "generic"}
-				}
-				// Get versions for each builder.
-				for _, builder := range builders {
-					builderDir, err := ioutil.ReadDir(filepath.Join(TEST_DIR, builder))
-					if err != nil {
-						t.Error(err)
-					}
-					for _, f := range builderDir {
-						// Builder subfolders are semantic version strings.
-						// Compare if a min version is given.
-						if f.IsDir() && semver.Compare(minversion, f.Name()) <= 0 {
-							// These are the supported versions of the builder
-							res = append(res, filepath.Join(builder, f.Name()))
-						}
-					}
-				}
-				return res
-			}
 
-			checkVersions := getBuildersAndVersions(tt.minversion, tt.builders)
+			checkVersions := getBuildersAndVersions(t, tt.minversion, tt.builders, ARTIFACT_PATH_BUILDERS)
 			if tt.noversion {
 				checkVersions = []string{""}
 			}
@@ -442,7 +448,7 @@ func Test_runVerifyGoAndGeneric(t *testing.T) {
 				artifactPath := filepath.Clean(filepath.Join(TEST_DIR, v, tt.artifact))
 				provenancePath := fmt.Sprintf("%s.intoto.jsonl", artifactPath)
 
-				_, builderID, err := runVerify("", artifactPath,
+				_, outBuilderId, err := runVerify("", artifactPath,
 					provenancePath,
 					tt.source, tt.pbranch, tt.pbuilderID,
 					tt.ptag, tt.pversiontag)
@@ -455,27 +461,29 @@ func Test_runVerifyGoAndGeneric(t *testing.T) {
 					return
 				}
 
-				if tt.builderID != "" && builderID != tt.builderID {
-					t.Errorf(cmp.Diff(builderID, tt.builderID))
+				if tt.outBuilderId != "" && outBuilderId != tt.outBuilderId {
+					t.Errorf(cmp.Diff(outBuilderId, tt.outBuilderId))
 				}
 			}
 		})
 	}
 }
 
-func TestContainerVerification(t *testing.T) {
+func Test_runVerifyArtifactImage(t *testing.T) {
+	t.Parallel()
+
 	// Override cosign image verification function for local image testing.
 	container.RunCosignImageVerification = func(ctx context.Context,
-		imageReference string, co *cosign.CheckOpts) ([]oci.Signature, bool, error) {
-		return cosign.VerifyLocalImageAttestations(ctx, imageReference, co)
+		image string, co *cosign.CheckOpts) ([]oci.Signature, bool, error) {
+		return cosign.VerifyLocalImageAttestations(ctx, image, co)
 	}
 
 	// TODO: Is there a more uniform way of handling getting image digest for both
 	// remote and local images?
-	container.GetImageDigest = func(imageReference string) (string, error) {
+	container.GetImageDigest = func(image string) (string, error) {
 		// This is copied from cosign's VerifyLocalImageAttestation code:
 		// https://github.com/sigstore/cosign/blob/fdceee4825dc5d56b130f3f431aab93137359e79/pkg/cosign/verify.go#L654
-		se, err := layout.SignedImageIndex(imageReference)
+		se, err := layout.SignedImageIndex(image)
 		if err != nil {
 			return "", err
 		}
@@ -507,17 +515,16 @@ func TestContainerVerification(t *testing.T) {
 		return strings.TrimPrefix(h.String(), "sha256:"), nil
 	}
 
-	// t.Parallel()
 	tests := []struct {
-		name        string
-		artifact    string
-		source      string
-		pbranch     *string
-		ptag        *string
-		pversiontag *string
-		pbuilderID  *string
-		builderID   string
-		err         error
+		name         string
+		artifact     string
+		source       string
+		pbranch      *string
+		ptag         *string
+		pversiontag  *string
+		pbuilderID   *string
+		outBuilderID string
+		err          error
 		// noversion is a special case where we are not testing all builder versions
 		// for example, testdata for the builder at head in trusted repo workflows
 		// or testdata from malicious untrusted builders.
@@ -587,34 +594,17 @@ func TestContainerVerification(t *testing.T) {
 	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()
-			getVersions := func() []string {
-				res := []string{}
-				builder := "container"
-
-				builderDir, err := ioutil.ReadDir(filepath.Join(TEST_DIR, builder))
-				if err != nil {
-					t.Error(err)
-				}
-				for _, f := range builderDir {
-					if f.IsDir() {
-						// These are the supported versions of the builder
-						res = append(res, filepath.Join(builder, f.Name()))
-					}
-				}
-
-				return res
-			}
+			t.Parallel()
 
-			checkVersions := getVersions()
+			checkVersions := getBuildersAndVersions(t, "", nil, ARTIFACT_IMAGE_BUILDERS)
 			if tt.noversion {
 				checkVersions = []string{""}
 			}
 
 			for _, v := range checkVersions {
-				artifactReference := filepath.Clean(filepath.Join(TEST_DIR, v, tt.artifact))
+				image := filepath.Clean(filepath.Join(TEST_DIR, v, tt.artifact))
 
-				_, builderID, err := runVerify(artifactReference, "", "",
+				_, outBuilderID, err := runVerify(image, "", "",
 					tt.source, tt.pbranch, tt.pbuilderID,
 					tt.ptag, tt.pversiontag)
 
@@ -626,8 +616,8 @@ func TestContainerVerification(t *testing.T) {
 					return
 				}
 
-				if tt.builderID != "" && builderID != tt.builderID {
-					t.Errorf(cmp.Diff(builderID, tt.builderID))
+				if tt.outBuilderID != "" && outBuilderID != tt.outBuilderID {
+					t.Errorf(cmp.Diff(outBuilderID, tt.outBuilderID))
 				}
 			}
 		})