From 4810fc2a74142408eb173222a91a4346a47d5562 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 4 Dec 2024 14:30:32 -0700 Subject: [PATCH] move content of veriy policy options function into enforcement criteria Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/options.go | 25 --------- pkg/cmd/attestation/verify/options_test.go | 60 ---------------------- pkg/cmd/attestation/verify/policy.go | 33 +++++++++--- pkg/cmd/attestation/verify/policy_test.go | 23 +++++++++ pkg/cmd/attestation/verify/verify.go | 3 -- 5 files changed, 50 insertions(+), 94 deletions(-) diff --git a/pkg/cmd/attestation/verify/options.go b/pkg/cmd/attestation/verify/options.go index 126159023..4296cb8ec 100644 --- a/pkg/cmd/attestation/verify/options.go +++ b/pkg/cmd/attestation/verify/options.go @@ -50,26 +50,6 @@ func (opts *Options) Clean() { } } -func (opts *Options) SetPolicyFlags() { - // check that Repo is in the expected format if provided - if opts.Repo != "" { - // we expect the repo argument to be in the format / - splitRepo := strings.Split(opts.Repo, "/") - - // if Repo is provided but owner is not, set the OWNER portion of the Repo value - // to Owner - opts.Owner = splitRepo[0] - - if !isSignerIdentityProvided(opts) { - opts.SANRegex = expandToGitHubURL(opts.Tenant, opts.Repo) - } - return - } - if !isSignerIdentityProvided(opts) { - opts.SANRegex = expandToGitHubURL(opts.Tenant, opts.Owner) - } -} - // AreFlagsValid checks that the provided flag combination is valid // and returns an error otherwise func (opts *Options) AreFlagsValid() error { @@ -108,11 +88,6 @@ func (opts *Options) AreFlagsValid() error { return nil } -// check if any of the signer identity flags have been provided -func isSignerIdentityProvided(opts *Options) bool { - return opts.SAN != "" || opts.SANRegex != "" || opts.SignerRepo != "" || opts.SignerWorkflow != "" -} - func isProvidedRepoValid(repo string) bool { // we expect a provided repository argument be in the format / splitRepo := strings.Split(repo, "/") diff --git a/pkg/cmd/attestation/verify/options_test.go b/pkg/cmd/attestation/verify/options_test.go index 77c0e3b23..bdb851e7b 100644 --- a/pkg/cmd/attestation/verify/options_test.go +++ b/pkg/cmd/attestation/verify/options_test.go @@ -80,63 +80,3 @@ func TestAreFlagsValid(t *testing.T) { require.ErrorContains(t, err, "bundle-from-oci flag cannot be used with bundle-path flag") }) } - -func TestSetPolicyFlags(t *testing.T) { - t.Run("sets Owner and SANRegex when Repo is provided", func(t *testing.T) { - opts := Options{ - ArtifactPath: publicGoodArtifactPath, - DigestAlgorithm: "sha512", - OIDCIssuer: "some issuer", - Repo: "sigstore/sigstore-js", - } - - opts.SetPolicyFlags() - require.Equal(t, "sigstore", opts.Owner) - require.Equal(t, "sigstore/sigstore-js", opts.Repo) - require.Equal(t, "(?i)^https://github.com/sigstore/sigstore-js/", opts.SANRegex) - }) - - t.Run("does not set SANRegex when SANRegex and Repo are provided", func(t *testing.T) { - opts := Options{ - ArtifactPath: publicGoodArtifactPath, - DigestAlgorithm: "sha512", - OIDCIssuer: "some issuer", - Repo: "sigstore/sigstore-js", - SANRegex: "^https://github/foo", - } - - opts.SetPolicyFlags() - require.Equal(t, "sigstore", opts.Owner) - require.Equal(t, "sigstore/sigstore-js", opts.Repo) - require.Equal(t, "^https://github/foo", opts.SANRegex) - }) - - t.Run("sets SANRegex when Owner is provided", func(t *testing.T) { - opts := Options{ - ArtifactPath: publicGoodArtifactPath, - BundlePath: publicGoodBundlePath, - DigestAlgorithm: "sha512", - OIDCIssuer: "some issuer", - Owner: "sigstore", - } - - opts.SetPolicyFlags() - require.Equal(t, "sigstore", opts.Owner) - require.Equal(t, "(?i)^https://github.com/sigstore/", opts.SANRegex) - }) - - t.Run("does not set SANRegex when SANRegex and Owner are provided", func(t *testing.T) { - opts := Options{ - ArtifactPath: publicGoodArtifactPath, - BundlePath: publicGoodBundlePath, - DigestAlgorithm: "sha512", - OIDCIssuer: "some issuer", - Owner: "sigstore", - SANRegex: "^https://github/foo", - } - - opts.SetPolicyFlags() - require.Equal(t, "sigstore", opts.Owner) - require.Equal(t, "^https://github/foo", opts.SANRegex) - }) -} diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index c2e154fe2..019ae9bbb 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "regexp" + "strings" "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "github.com/sigstore/sigstore-go/pkg/verify" @@ -22,7 +23,23 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { } func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, error) { - var c verification.EnforcementCriteria + // initialize the enforcement criteria with the provided PredicateType + c := verification.EnforcementCriteria{ + PredicateType: opts.PredicateType, + } + + // set the owner value by checking the repo and owner options + var owner string + if opts.Repo != "" { + // we expect the repo argument to be in the format / + splitRepo := strings.Split(opts.Repo, "/") + // if Repo is provided but owner is not, set the OWNER portion of the Repo value + // to Owner + owner = splitRepo[0] + } else { + // otherwise use the user provided owner value + owner = opts.Owner + } // Set SANRegex using either the opts.SignerRepo or opts.SignerWorkflow values if opts.SignerRepo != "" { @@ -35,10 +52,16 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er } c.SANRegex = validatedWorkflowRegex - } else { + } else if opts.SANRegex != "" || opts.SAN != "" { // If neither of those values were set, default to the provided SANRegex and SAN values c.SANRegex = opts.SANRegex c.SAN = opts.SAN + } else if opts.Repo != "" { + // if the user has not provided the SAN, SANRegex, SignerRepo, or SignerWorkflow options + // then we default to the repo and owner options + c.SANRegex = expandToGitHubURL(opts.Tenant, opts.Repo) + } else { + c.SANRegex = expandToGitHubURL(opts.Tenant, owner) } // if the DenySelfHostedRunner option is set to true, set the @@ -66,9 +89,9 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er // If the tenant option is provided, set the SourceRepositoryOwnerURI extension // using the specific URI format if opts.Tenant != "" { - c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner) + c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, owner) } else { - c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner) + c.Certificate.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", owner) } // if the tenant is provided and OIDC issuer provided matches the default @@ -80,8 +103,6 @@ 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 420c57f3a..9290b4a9c 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -56,6 +56,29 @@ func TestNewEnforcementCriteria(t *testing.T) { require.Equal(t, "(?i)^https://github/foo", c.SANRegex) }) + t.Run("sets SANRegex using opts.Repo", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + Repo: "foo/bar", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "(?i)^https://github.com/foo/bar/", c.SANRegex) + }) + + t.Run("sets SANRegex using opts.Owner", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "foo", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "(?i)^https://github.com/foo/", c.SANRegex) + }) + t.Run("sets Extensions.RunnerEnvironment to GitHubRunner value if opts.DenySelfHostedRunner is true", func(t *testing.T) { opts := &Options{ ArtifactPath: artifactPath, diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 5b31371ff..f77b7cc91 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -157,9 +157,6 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command opts.Tenant = tenant } - // set policy flags based on what has been provided - opts.SetPolicyFlags() - if runF != nil { return runF(opts) }