From 50cda0df44f987c7c8803a4984eddcae7cdd4574 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Thu, 31 Oct 2024 16:56:49 -0600
Subject: [PATCH] add Valid method for EnforcementCriteria

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/verification/policy.go | 19 +++++++++++++++++++
 pkg/cmd/attestation/verify/policy.go       |  7 ++++---
 pkg/cmd/attestation/verify/policy_test.go  |  3 ++-
 pkg/cmd/attestation/verify/verify.go       | 17 +++++++++++------
 4 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go
index 9f8672628..ae21dae48 100644
--- a/pkg/cmd/attestation/verification/policy.go
+++ b/pkg/cmd/attestation/verification/policy.go
@@ -2,6 +2,7 @@ package verification
 
 import (
 	"encoding/hex"
+	"fmt"
 
 	"github.com/cli/cli/v2/pkg/cmd/attestation/artifact"
 
@@ -9,6 +10,8 @@ import (
 	"github.com/sigstore/sigstore-go/pkg/verify"
 )
 
+const GitHubRunner = "github-hosted"
+
 // BuildDigestPolicyOption builds a verify.ArtifactPolicyOption
 // from the given artifact digest and digest algorithm
 func BuildDigestPolicyOption(a artifact.DigestedArtifact) (verify.ArtifactPolicyOption, error) {
@@ -26,3 +29,19 @@ type EnforcementCriteria struct {
 	SANRegex      string
 	SAN           string
 }
+
+func (c EnforcementCriteria) Valid() error {
+	if c.Certificate.Issuer == "" {
+		return fmt.Errorf("Issuer must be set")
+	}
+	if c.Certificate.RunnerEnvironment != "" && c.Certificate.RunnerEnvironment != GitHubRunner {
+		return fmt.Errorf("RunnerEnvironment must be set to either \"\" or %s", GitHubRunner)
+	}
+	if c.Certificate.SourceRepositoryOwnerURI == "" {
+		return fmt.Errorf("SourceRepositoryOwnerURI must be set")
+	}
+	if c.PredicateType == "" {
+		return fmt.Errorf("PredicateType must be set")
+	}
+	return nil
+}
diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go
index f69c75c52..c68cdb452 100644
--- a/pkg/cmd/attestation/verify/policy.go
+++ b/pkg/cmd/attestation/verify/policy.go
@@ -14,8 +14,7 @@ import (
 
 const (
 	// represents the GitHub hosted runner in the certificate RunnerEnvironment extension
-	GitHubRunner = "github-hosted"
-	hostRegex    = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$`
+	hostRegex = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$`
 )
 
 func expandToGitHubURL(tenant, ownerOrRepo string) string {
@@ -44,7 +43,7 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er
 	}
 
 	if opts.DenySelfHostedRunner {
-		c.Certificate.RunnerEnvironment = GitHubRunner
+		c.Certificate.RunnerEnvironment = verification.GitHubRunner
 	} else {
 		// if Certificate.RunnerEnvironment value is set to the empty string
 		// through the second function argument,
@@ -74,6 +73,8 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er
 		c.Certificate.Issuer = opts.OIDCIssuer
 	}
 
+	c.PredicateType = opts.PredicateType
+
 	return c, nil
 }
 
diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go
index d4516e844..f5755e9d5 100644
--- a/pkg/cmd/attestation/verify/policy_test.go
+++ b/pkg/cmd/attestation/verify/policy_test.go
@@ -3,6 +3,7 @@ package verify
 import (
 	"testing"
 
+	"github.com/cli/cli/v2/pkg/cmd/attestation/verification"
 	"github.com/cli/cli/v2/pkg/cmd/factory"
 
 	"github.com/stretchr/testify/require"
@@ -65,7 +66,7 @@ func TestNewEnforcementCriteria(t *testing.T) {
 
 		c, err := newEnforcementCriteria(opts)
 		require.NoError(t, err)
-		require.Equal(t, GitHubRunner, c.Certificate.RunnerEnvironment)
+		require.Equal(t, verification.GitHubRunner, c.Certificate.RunnerEnvironment)
 	})
 
 	t.Run("sets Extensions.RunnerEnvironment to * value if opts.DenySelfHostedRunner is false", func(t *testing.T) {
diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go
index 5f1b1d6f3..188bde7b1 100644
--- a/pkg/cmd/attestation/verify/verify.go
+++ b/pkg/cmd/attestation/verify/verify.go
@@ -203,6 +203,17 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command
 }
 
 func runVerify(opts *Options) error {
+	ec, err := newEnforcementCriteria(opts)
+	if err != nil {
+		opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy"))
+		return err
+	}
+
+	if err := ec.Valid(); err != nil {
+		opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Invalid verification policy"))
+		return err
+	}
+
 	artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm)
 	if err != nil {
 		opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ Loading digest for %s failed\n"), opts.ArtifactPath)
@@ -258,12 +269,6 @@ func runVerify(opts *Options) error {
 
 	opts.Logger.VerbosePrintf("Verifying attestations with predicate type: %s\n", opts.PredicateType)
 
-	ec, err := newEnforcementCriteria(opts)
-	if err != nil {
-		opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy"))
-		return err
-	}
-
 	sp, err := buildSigstoreVerifyPolicy(ec, *artifact)
 	if err != nil {
 		opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build Sigstore verification policy"))