From 3108d99208273dad1b91813f7edffee409a00cd6 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Fri, 23 May 2025 15:31:33 -0400 Subject: [PATCH] added the unit test --- .../test/data/release-attestation.json | 24 +++++++ pkg/cmd/release/attestation/attestation.go | 7 -- pkg/cmd/release/attestation/options.go | 7 -- pkg/cmd/release/attestation/options_test.go | 72 +++++++++++++++++++ pkg/cmd/release/attestation/policy.go | 6 +- pkg/cmd/release/attestation/policy_test.go | 71 ++++++++++++++++++ pkg/cmd/release/shared/fetch.go | 2 +- pkg/cmd/release/verify-asset/verify-asset.go | 30 +++++--- pkg/cmd/release/verify/verify.go | 29 +++++--- 9 files changed, 213 insertions(+), 35 deletions(-) create mode 100644 pkg/cmd/attestation/test/data/release-attestation.json create mode 100644 pkg/cmd/release/attestation/options_test.go create mode 100644 pkg/cmd/release/attestation/policy_test.go diff --git a/pkg/cmd/attestation/test/data/release-attestation.json b/pkg/cmd/attestation/test/data/release-attestation.json new file mode 100644 index 000000000..ae8dd1b56 --- /dev/null +++ b/pkg/cmd/attestation/test/data/release-attestation.json @@ -0,0 +1,24 @@ +{ + "mediaType": "application/vnd.dev.sigstore.bundle.v0.3+json", + "verificationMaterial": { + "timestampVerificationData": { + "rfc3161Timestamps": [ + { + "signedTimestamp": "MIIC0TADAgEAMIICyAYJKoZIhvcNAQcCoIICuTCCArUCAQMxDTALBglghkgBZQMEAgIwgbwGCyqGSIb3DQEJEAEEoIGsBIGpMIGmAgEBBgkrBgEEAYO/MAIwMTANBglghkgBZQMEAgEFAAQgGvFc6nUuLhnXfhM9p0DV91c5kHvafP1hs9BX8KYeeSYCFQDhjGrIIiaH/jkMdN6HUsErnUfrlRgPMjAyNTA1MTMyMzAzNTFaMAMCAQGgNqQ0MDIxFTATBgNVBAoTDEdpdEh1YiwgSW5jLjEZMBcGA1UEAxMQVFNBIFRpbWVzdGFtcGluZ6AAMYIB3jCCAdoCAQEwSjAyMRUwEwYDVQQKEwxHaXRIdWIsIEluYy4xGTAXBgNVBAMTEFRTQSBpbnRlcm1lZGlhdGUCFB+7MIjE5/rL4XA4fNDnmXHA04+wMAsGCWCGSAFlAwQCAqCCAQUwGgYJKoZIhvcNAQkDMQ0GCyqGSIb3DQEJEAEEMBwGCSqGSIb3DQEJBTEPFw0yNTA1MTMyMzAzNTFaMD8GCSqGSIb3DQEJBDEyBDDVh2oDCJy7ustugLKfVcUSNjo5M2MFMNKIU11sIQDCNOo5gbj9R97sCWXNnfmUztMwgYcGCyqGSIb3DQEJEAIvMXgwdjB0MHIEIHuISsKSyiJtlhGjT+RyS+tYQ7iwCMsMCTGmz2NK3D7DME4wNqQ0MDIxFTATBgNVBAoTDEdpdEh1YiwgSW5jLjEZMBcGA1UEAxMQVFNBIGludGVybWVkaWF0ZQIUH7swiMTn+svhcDh80OeZccDTj7AwCgYIKoZIzj0EAwMEZzBlAjAqp/fYVfQcU9aMcmTIZvb0cxk00OaVBYLzuiIvcRqkMdAJiz/gSxOWU0AQjEPskHUCMQCrUKlZR4shPZuMvY6CCUOhxxKq/6LUoccWNHyL6sGkHRXE7j9HETh4uLKzRwNDVVA=" + } + ] + }, + "certificate": { + "rawBytes": "MIICKjCCAbCgAwIBAgIUaa62dj98DUB+TpyvKtVaR4vGSM0wCgYIKoZIzj0EAwMwODEVMBMGA1UEChMMR2l0SHViLCBJbmMuMR8wHQYDVQQDExZGdWxjaW8gSW50ZXJtZWRpYXRlIGwxMB4XDTI1MDMxMDE1MDMwMloXDTI2MDMxMDE1MDMwMlowKjEVMBMGA1UEChMMR2l0SHViLCBJbmMuMREwDwYDVQQDEwhBdHRlc3RlcjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABIMB7plPnZvBRlC2lvAocKTAqAPMJqstEqYk26e9vDJDC1yqoiHxZfPV4W/1RqUMZD1dFKm9t4RiSmm73/QnQKajgaUwgaIwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoGCCsGAQUFBwMDMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFOqaGpr5SbdYk5CQXsmmDZCBHR+XMB8GA1UdIwQYMBaAFMDhuFKkS08+3no4EQbPSY6hRZszMC0GA1UdEQQmMCSGImh0dHBzOi8vZG90Y29tLnJlbGVhc2VzLmdpdGh1Yi5jb20wCgYIKoZIzj0EAwMDaAAwZQIwWFdF6xcXazHVPHEAtd1SeaizLdY1erRl5hK+XlwhfpnasQHHZ9bdu4Zj8ARhW/AhAjEArujhmJGo7Fi4/Ek1RN8bufs6UhIQneQd/pxE8QdorwZkj2C8nf2EzrUYzlxKfktC" + } + }, + "dsseEnvelope": { + "payload": "eyJfdHlwZSI6Imh0dHBzOi8vaW4tdG90by5pby9TdGF0ZW1lbnQvdjEiLCJzdWJqZWN0IjpbeyJ1cmkiOiJwa2c6Z2l0aHViL2JkZWhhbWVyL2RlbG1lQHY1IiwiZGlnZXN0Ijp7InNoYTEiOiJjNWUxN2E2MmUwNmExZDIwMTU3MDI0OWM2MWZhZTUzMWU5MjQ0ZTFiIn19LHsibmFtZSI6ImEuemlwIiwiZGlnZXN0Ijp7InNoYTI1NiI6ImY3MTY1ODQ4ZjlmNWRkYzU3OGQ3YWRiZDFmNTY2YTM5NDE2OTM4NWM3M2JkODhiZjYwZGY3ZTc1OWRiOGUwOGQifX0seyJuYW1lIjoiYi56aXAiLCJkaWdlc3QiOnsic2hhMjU2IjoiOGI3ZWIxNTcyMzQ2NjkyZmZkM2FlMDEyNDhjNzBhMzQxYWUzYWE4YmUxZGY4YjEyMzQ2YjUwYWNiOTAwMjI4MiJ9fV0sInByZWRpY2F0ZVR5cGUiOiJodHRwczovL2luLXRvdG8uaW8vYXR0ZXN0YXRpb24vcmVsZWFzZS92MC4xIiwicHJlZGljYXRlIjp7Im93bmVySWQiOiIzOTgwMjciLCJwdXJsIjoicGtnOmdpdGh1Yi9iZGVoYW1lci9kZWxtZUB2NSIsInJlbGVhc2VJZCI6IjIxODQxOTIxNyIsInJlcG9zaXRvcnkiOiJiZGVoYW1lci9kZWxtZSIsInJlcG9zaXRvcnlJZCI6IjkwNTk4ODA0NCIsInRhZyI6InY1In19", + "payloadType": "application/vnd.in-toto+json", + "signatures": [ + { + "sig": "MEQCIH6LDUanQYOCPovZlIqI1cE49SiGJdexR65qsAZHohsZAiA9w3usgPWtgn5voB8bRvpJQtjEVqC5eMDh3mJEdyMcXw==" + } + ] + } +} \ No newline at end of file diff --git a/pkg/cmd/release/attestation/attestation.go b/pkg/cmd/release/attestation/attestation.go index 08e1398b8..bf2f39a7c 100644 --- a/pkg/cmd/release/attestation/attestation.go +++ b/pkg/cmd/release/attestation/attestation.go @@ -50,13 +50,6 @@ func VerifyAttestations(art artifact.DigestedArtifact, att []*api.Attestation, s return nil, logMsg, err } - // Verify extensions - // certExtVerified, err := verification.VerifyCertExtensions(sigstoreVerified, ec) - // if err != nil { - // logMsg := "✗ Policy verification failed" - // return nil, logMsg, err - // } - return sigstoreVerified, "", nil } diff --git a/pkg/cmd/release/attestation/options.go b/pkg/cmd/release/attestation/options.go index f0957c04b..9dd84647e 100644 --- a/pkg/cmd/release/attestation/options.go +++ b/pkg/cmd/release/attestation/options.go @@ -19,7 +19,6 @@ import ( const ReleasePredicateType = "https://in-toto.io/attestation/release/v0.1" -// AttestOptions captures the options for the verify command type AttestOptions struct { Config func() (gh.Config, error) HttpClient *http.Client @@ -67,17 +66,11 @@ func (opts *AttestOptions) AreFlagsValid() error { return fmt.Errorf("invalid value provided for repo: %s", opts.Repo) } - // If provided, check that the SignerRepo option is in the expected format / - if opts.SignerRepo != "" && !isProvidedRepoValid(opts.SignerRepo) { - return fmt.Errorf("invalid value provided for signer-repo: %s", opts.SignerRepo) - } - // Check that limit is between 1 and 1000 if opts.Limit < 1 || opts.Limit > 1000 { return fmt.Errorf("limit %d not allowed, must be between 1 and 1000", opts.Limit) } - // Verify provided hostname if opts.Hostname != "" { if err := ghinstance.HostnameValidator(opts.Hostname); err != nil { return fmt.Errorf("error parsing hostname: %w", err) diff --git a/pkg/cmd/release/attestation/options_test.go b/pkg/cmd/release/attestation/options_test.go new file mode 100644 index 000000000..00bba29a5 --- /dev/null +++ b/pkg/cmd/release/attestation/options_test.go @@ -0,0 +1,72 @@ +package attestation + +import ( + "errors" + "testing" +) + +func TestAttestOptions_Clean(t *testing.T) { + opts := &AttestOptions{ + AssetFilePath: "foo/bar/../baz.txt", + } + opts.Clean() + expected := "foo/baz.txt" + if opts.AssetFilePath != expected && opts.AssetFilePath != "./foo/baz.txt" { // OS differences + t.Errorf("expected AssetFilePath to be cleaned to %q, got %q", expected, opts.AssetFilePath) + } +} + +func TestAttestOptions_AreFlagsValid_Valid(t *testing.T) { + opts := &AttestOptions{ + Repo: "owner/repo", + SignerRepo: "signer/repo", + Limit: 10, + } + if err := opts.AreFlagsValid(); err != nil { + t.Errorf("expected no error, got %v", err) + } +} + +func TestAttestOptions_AreFlagsValid_InvalidRepo(t *testing.T) { + opts := &AttestOptions{ + Repo: "invalidrepo", + } + err := opts.AreFlagsValid() + if err == nil || !errors.Is(err, err) { + t.Errorf("expected error for invalid repo, got %v", err) + } +} + +func TestAttestOptions_AreFlagsValid_LimitTooLow(t *testing.T) { + opts := &AttestOptions{ + Repo: "owner/repo", + Limit: 0, + } + err := opts.AreFlagsValid() + if err == nil || !errors.Is(err, err) { + t.Errorf("expected error for limit too low, got %v", err) + } +} + +func TestAttestOptions_AreFlagsValid_LimitTooHigh(t *testing.T) { + opts := &AttestOptions{ + Repo: "owner/repo", + Limit: 1001, + } + err := opts.AreFlagsValid() + if err == nil || !errors.Is(err, err) { + t.Errorf("expected error for limit too high, got %v", err) + } +} + +func TestAttestOptions_AreFlagsValid_ValidHostname(t *testing.T) { + opts := &AttestOptions{ + Repo: "owner/repo", + Limit: 10, + Hostname: "github.com", + } + err := opts.AreFlagsValid() + if err != nil { + t.Errorf("expected no error for valid hostname, got %v", err) + } +} diff --git a/pkg/cmd/release/attestation/policy.go b/pkg/cmd/release/attestation/policy.go index 7dfb88cfe..d7bf0f096 100644 --- a/pkg/cmd/release/attestation/policy.go +++ b/pkg/cmd/release/attestation/policy.go @@ -3,7 +3,6 @@ package attestation import ( "fmt" - att_io "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "github.com/sigstore/sigstore-go/pkg/verify" @@ -18,12 +17,11 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string { return fmt.Sprintf("https://%s.ghe.com/%s", tenant, ownerOrRepo) } -// TODO: revist this policy -func NewEnforcementCriteria(opts *AttestOptions, logger *att_io.Handler) (verification.EnforcementCriteria, error) { +func NewEnforcementCriteria(opts *AttestOptions) (verification.EnforcementCriteria, error) { // initialize the enforcement criteria with the provided PredicateType and SAN c := verification.EnforcementCriteria{ PredicateType: opts.PredicateType, - // if the proxima is provided, the default uses the proxima-specific SAN + // TODO: if the proxima is provided, the default uses the proxima-specific SAN SAN: "https://dotcom.releases.github.com", } diff --git a/pkg/cmd/release/attestation/policy_test.go b/pkg/cmd/release/attestation/policy_test.go new file mode 100644 index 000000000..57eab86b2 --- /dev/null +++ b/pkg/cmd/release/attestation/policy_test.go @@ -0,0 +1,71 @@ +package attestation + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNewEnforcementCriteria(t *testing.T) { + t.Run("check SAN", func(t *testing.T) { + opts := &AttestOptions{ + Owner: "foo", + Repo: "foo/bar", + PredicateType: "https://in-toto.io/attestation/release/v0.1", + } + + c, err := NewEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://dotcom.releases.github.com", c.SAN) + require.Equal(t, "https://in-toto.io/attestation/release/v0.1", c.PredicateType) + }) + + t.Run("sets Extensions.SourceRepositoryURI using opts.Repo and opts.Tenant", func(t *testing.T) { + opts := &AttestOptions{ + Owner: "foo", + Repo: "foo/bar", + Tenant: "baz", + } + + c, err := NewEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://baz.ghe.com/foo/bar", c.Certificate.SourceRepositoryURI) + }) + + t.Run("sets Extensions.SourceRepositoryURI using opts.Repo", func(t *testing.T) { + opts := &AttestOptions{ + Owner: "foo", + Repo: "foo/bar", + } + + c, err := NewEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://github.com/foo/bar", c.Certificate.SourceRepositoryURI) + }) + + t.Run("sets Extensions.SourceRepositoryOwnerURI using opts.Owner and opts.Tenant", func(t *testing.T) { + opts := &AttestOptions{ + + Owner: "foo", + Repo: "foo/bar", + Tenant: "baz", + } + + c, err := NewEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://baz.ghe.com/foo", c.Certificate.SourceRepositoryOwnerURI) + }) + + t.Run("sets Extensions.SourceRepositoryOwnerURI using opts.Owner", func(t *testing.T) { + opts := &AttestOptions{ + + Owner: "foo", + Repo: "foo/bar", + } + + c, err := NewEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "https://github.com/foo", c.Certificate.SourceRepositoryOwnerURI) + }) + +} diff --git a/pkg/cmd/release/shared/fetch.go b/pkg/cmd/release/shared/fetch.go index 5fea30b7c..3daa1d3fc 100644 --- a/pkg/cmd/release/shared/fetch.go +++ b/pkg/cmd/release/shared/fetch.go @@ -132,7 +132,7 @@ type fetchResult struct { } func FetchRefSHA(ctx context.Context, httpClient *http.Client, repo ghrepo.Interface, tagName string) (string, error) { - path := fmt.Sprintf("repos/%s/%s/git/refs/tags/%s", repo.RepoOwner(), repo.RepoName(), tagName) + path := fmt.Sprintf("repos/%s/git/refs/tags/%s", repo.RepoOwner(), repo.RepoName(), tagName) req, err := http.NewRequestWithContext(ctx, "GET", ghinstance.RESTPrefix(repo.RepoHost())+path, nil) if err != nil { return "", err diff --git a/pkg/cmd/release/verify-asset/verify-asset.go b/pkg/cmd/release/verify-asset/verify-asset.go index 585643c2d..ddefdf5be 100644 --- a/pkg/cmd/release/verify-asset/verify-asset.go +++ b/pkg/cmd/release/verify-asset/verify-asset.go @@ -5,6 +5,7 @@ import ( "errors" "path/filepath" + "github.com/cli/cli/v2/pkg/cmd/attestation/auth" ghauth "github.com/cli/go-gh/v2/pkg/auth" "github.com/cli/cli/v2/internal/text" @@ -23,9 +24,10 @@ func NewCmdVerifyAsset(f *cmdutil.Factory, runF func(*attestation.AttestOptions) opts := &attestation.AttestOptions{} cmd := &cobra.Command{ - Use: "verify-asset ", - Short: "Verify that a given asset originated from a specific GitHub Release.", - Args: cobra.ExactArgs(2), + Use: "verify-asset ", + Short: "Verify that a given asset originated from a specific GitHub Release.", + Hidden: true, + Args: cobra.ExactArgs(2), PreRunE: func(cmd *cobra.Command, args []string) error { if len(args) < 2 { return cmdutil.FlagErrorf("You must specify a tag and a file path") @@ -45,6 +47,11 @@ func NewCmdVerifyAsset(f *cmdutil.Factory, runF func(*attestation.AttestOptions) logger := att_io.NewHandler(f.IOStreams) hostname, _ := ghauth.DefaultHost() + err = auth.IsHostSupported(hostname) + if err != nil { + return err + } + *opts = attestation.AttestOptions{ TagName: tagName, AssetFilePath: assetFilePath, @@ -56,21 +63,24 @@ func NewCmdVerifyAsset(f *cmdutil.Factory, runF func(*attestation.AttestOptions) Logger: logger, HttpClient: httpClient, BaseRepo: baseRepo, + Hostname: hostname, } + + // Check that the given flag combination is valid + if err := opts.AreFlagsValid(); err != nil { + return err + } + return nil }, RunE: func(cmd *cobra.Command, args []string) error { - if runF != nil { - return runF(opts) - } - td, err := opts.APIClient.GetTrustDomain() if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to get trust domain")) return err } - ec, err := attestation.NewEnforcementCriteria(opts, opts.Logger) + ec, err := attestation.NewEnforcementCriteria(opts) if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build policy information")) return err @@ -90,6 +100,10 @@ func NewCmdVerifyAsset(f *cmdutil.Factory, runF func(*attestation.AttestOptions) opts.SigstoreVerifier = sigstoreVerifier opts.EC = ec + if runF != nil { + return runF(opts) + } + return verifyAssetRun(opts) }, } diff --git a/pkg/cmd/release/verify/verify.go b/pkg/cmd/release/verify/verify.go index e8c6621e5..96c33c50b 100644 --- a/pkg/cmd/release/verify/verify.go +++ b/pkg/cmd/release/verify/verify.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" + "github.com/cli/cli/v2/pkg/cmd/attestation/auth" att_io "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmd/release/attestation" @@ -24,9 +25,10 @@ func NewCmdVerify(f *cmdutil.Factory, runF func(*attestation.AttestOptions) erro opts := &attestation.AttestOptions{} cmd := &cobra.Command{ - Use: "verify []", - Short: "Verify the attestation for a GitHub Release.", - Args: cobra.ExactArgs(1), + Use: "verify []", + Short: "Verify the attestation for a GitHub Release.", + Hidden: true, + Args: cobra.ExactArgs(1), PreRunE: func(cmd *cobra.Command, args []string) error { if len(args) < 1 { return cmdutil.FlagErrorf("You must specify a tag") @@ -46,6 +48,11 @@ func NewCmdVerify(f *cmdutil.Factory, runF func(*attestation.AttestOptions) erro logger := att_io.NewHandler(f.IOStreams) hostname, _ := ghauth.DefaultHost() + err = auth.IsHostSupported(hostname) + if err != nil { + return err + } + *opts = attestation.AttestOptions{ TagName: opts.TagName, Repo: baseRepo.RepoOwner() + "/" + baseRepo.RepoName(), @@ -56,21 +63,23 @@ func NewCmdVerify(f *cmdutil.Factory, runF func(*attestation.AttestOptions) erro Logger: logger, HttpClient: httpClient, BaseRepo: baseRepo, + Hostname: hostname, + } + + // Check that the given flag combination is valid + if err := opts.AreFlagsValid(); err != nil { + return err } return nil }, RunE: func(cmd *cobra.Command, args []string) error { - if runF != nil { - return runF(opts) - } - td, err := opts.APIClient.GetTrustDomain() if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to get trust domain")) return err } - ec, err := attestation.NewEnforcementCriteria(opts, opts.Logger) + ec, err := attestation.NewEnforcementCriteria(opts) if err != nil { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build policy information")) return err @@ -91,6 +100,10 @@ func NewCmdVerify(f *cmdutil.Factory, runF func(*attestation.AttestOptions) erro opts.SigstoreVerifier = sigstoreVerifier opts.EC = ec + if runF != nil { + return runF(opts) + } + return verifyRun(opts) }, }