From 8e6ed6eb38b2c51213e45605b82e7fc0738beb62 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Fri, 30 May 2025 09:30:05 -0700 Subject: [PATCH] improve test --- .../test/data/github_release_artifact.zip | 2 +- .../data/github_release_artifact_invalid.zip | 14 ++++ pkg/cmd/release/attestation/options.go | 11 --- pkg/cmd/release/attestation/options_test.go | 5 +- pkg/cmd/release/verify-asset/verify-asset.go | 4 +- .../release/verify-asset/verify-asset_test.go | 77 ++++++++++++++++++- pkg/cmd/release/verify/verify.go | 7 ++ pkg/cmd/release/verify/verify_test.go | 40 +++++++++- 8 files changed, 139 insertions(+), 21 deletions(-) create mode 100644 pkg/cmd/attestation/test/data/github_release_artifact_invalid.zip diff --git a/pkg/cmd/attestation/test/data/github_release_artifact.zip b/pkg/cmd/attestation/test/data/github_release_artifact.zip index f4595ef44..934302cd9 100644 --- a/pkg/cmd/attestation/test/data/github_release_artifact.zip +++ b/pkg/cmd/attestation/test/data/github_release_artifact.zip @@ -1,4 +1,4 @@ -a # frozen_string_literal: true +# frozen_string_literal: true source "https://rubygems.org" diff --git a/pkg/cmd/attestation/test/data/github_release_artifact_invalid.zip b/pkg/cmd/attestation/test/data/github_release_artifact_invalid.zip new file mode 100644 index 000000000..26b414dbc --- /dev/null +++ b/pkg/cmd/attestation/test/data/github_release_artifact_invalid.zip @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +source "https://rubygems.org" + +source "https://rubygems.pkg.github.com/github" do + gem "entitlements-aad-plugin", "~> 1.0" + gem "entitlements-app", "~> 1.2" + gem "entitlements-github-plugin", "~> 1.2" + gem "entitlements-gitrepo-auditor-plugin", "~> 1.0" + gem "entitlements-jit-github-plugin", "~> 1.0" + gem "entitlements-lib", "~> 0.2" + gem "entitlements-stafftools-plugin", "~> 1.0" +end + diff --git a/pkg/cmd/release/attestation/options.go b/pkg/cmd/release/attestation/options.go index 9dd84647e..7140c4f33 100644 --- a/pkg/cmd/release/attestation/options.go +++ b/pkg/cmd/release/attestation/options.go @@ -10,7 +10,6 @@ import ( "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/attestation/api" - "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmdutil" @@ -27,22 +26,12 @@ type AttestOptions struct { Exporter cmdutil.Exporter TagName string TrustedRoot string - DigestAlgorithm string Limit int - OIDCIssuer string Owner string PredicateType string Repo string - SAN string - SANRegex string - SignerDigest string - SignerRepo string - SignerWorkflow string - SourceDigest string - SourceRef string APIClient api.Client Logger *io.Handler - OCIClient oci.Client SigstoreVerifier verification.SigstoreVerifier Hostname string EC verification.EnforcementCriteria diff --git a/pkg/cmd/release/attestation/options_test.go b/pkg/cmd/release/attestation/options_test.go index 89d260199..125723b17 100644 --- a/pkg/cmd/release/attestation/options_test.go +++ b/pkg/cmd/release/attestation/options_test.go @@ -7,9 +7,8 @@ import ( func TestAttestOptions_AreFlagsValid_Valid(t *testing.T) { opts := &AttestOptions{ - Repo: "owner/repo", - SignerRepo: "signer/repo", - Limit: 10, + Repo: "owner/repo", + Limit: 10, } if err := opts.AreFlagsValid(); err != nil { t.Errorf("expected no error, got %v", err) diff --git a/pkg/cmd/release/verify-asset/verify-asset.go b/pkg/cmd/release/verify-asset/verify-asset.go index c87fc8e65..3845ada9b 100644 --- a/pkg/cmd/release/verify-asset/verify-asset.go +++ b/pkg/cmd/release/verify-asset/verify-asset.go @@ -3,6 +3,7 @@ package verifyasset import ( "context" "errors" + "fmt" "path/filepath" "github.com/cli/cli/v2/pkg/cmd/attestation/auth" @@ -95,6 +96,7 @@ func NewCmdVerifyAsset(f *cmdutil.Factory, runF func(*attestation.AttestOptions) } config := verification.SigstoreConfig{ + HttpClient: opts.HttpClient, Logger: opts.Logger, NoPublicGood: true, TrustDomain: td, @@ -172,7 +174,7 @@ func verifyAssetRun(opts *attestation.AttestOptions) error { if len(filteredAttestations) == 0 { opts.Logger.Printf(opts.Logger.ColorScheme.Red("Release %s does not contain %s (%s)\n"), opts.TagName, opts.AssetFilePath, fileDigest.DigestWithAlg()) - return nil + return fmt.Errorf("no attestations found for %s in release %s", fileName, opts.TagName) } opts.Logger.Printf("Loaded %s from GitHub API\n", text.Pluralize(len(filteredAttestations), "attestation")) diff --git a/pkg/cmd/release/verify-asset/verify-asset_test.go b/pkg/cmd/release/verify-asset/verify-asset_test.go index 784c43e1e..2a26dc6d3 100644 --- a/pkg/cmd/release/verify-asset/verify-asset_test.go +++ b/pkg/cmd/release/verify-asset/verify-asset_test.go @@ -95,7 +95,7 @@ func TestNewCmdVerifyAsset_Args(t *testing.T) { func Test_verifyAssetRun_Success(t *testing.T) { ios, _, _, _ := iostreams.Test() - tagName := "v1.2.3" + tagName := "v5" fakeHTTP := &httpmock.Registry{} defer fakeHTTP.Verify(t) @@ -114,12 +114,81 @@ func Test_verifyAssetRun_Success(t *testing.T) { Logger: io.NewHandler(ios), APIClient: api.NewTestClient(), SigstoreVerifier: verification.NewMockSigstoreVerifier(t), + PredicateType: attestation.ReleasePredicateType, + HttpClient: &http.Client{Transport: fakeHTTP}, + BaseRepo: baseRepo, + } + + ec, err := attestation.NewEnforcementCriteria(opts) + require.NoError(t, err) + opts.EC = ec + + err = verifyAssetRun(opts) + require.NoError(t, err) +} + +func Test_verifyAssetRun_Failed_With_Wrong_tag(t *testing.T) { + ios, _, _, _ := iostreams.Test() + tagName := "v1" + + fakeHTTP := &httpmock.Registry{} + defer fakeHTTP.Verify(t) + fakeSHA := "1234567890abcdef1234567890abcdef12345678" + shared.StubFetchRefSHA(t, fakeHTTP, "owner", "repo", tagName, fakeSHA) + + baseRepo, err := ghrepo.FromFullName("owner/repo") + require.NoError(t, err) + + opts := &attestation.AttestOptions{ + TagName: tagName, + AssetFilePath: "../../attestation/test/data/github_release_artifact.zip", + Repo: "owner/repo", + Owner: "owner", + Limit: 10, + Logger: io.NewHandler(ios), + APIClient: api.NewTestClient(), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), + PredicateType: attestation.ReleasePredicateType, + HttpClient: &http.Client{Transport: fakeHTTP}, + BaseRepo: baseRepo, + } + + ec, err := attestation.NewEnforcementCriteria(opts) + require.NoError(t, err) + opts.EC = ec + + err = verifyAssetRun(opts) + require.Error(t, err, "no attestations found for github_release_artifact.zip in release v1") +} + +func Test_verifyAssetRun_Failed_With_Invalid_Artifact(t *testing.T) { + ios, _, _, _ := iostreams.Test() + tagName := "v1.2.3" + + fakeHTTP := &httpmock.Registry{} + defer fakeHTTP.Verify(t) + fakeSHA := "1234567890abcdef1234567890abcdef12345678" + shared.StubFetchRefSHA(t, fakeHTTP, "owner", "repo", tagName, fakeSHA) + + baseRepo, err := ghrepo.FromFullName("owner/repo") + require.NoError(t, err) + + opts := &attestation.AttestOptions{ + TagName: tagName, + AssetFilePath: "../../attestation/test/data/github_release_artifact_invalid.zip", + Repo: "owner/repo", + Owner: "owner", + Limit: 10, + Logger: io.NewHandler(ios), + APIClient: api.NewTestClient(), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), + PredicateType: attestation.ReleasePredicateType, HttpClient: &http.Client{Transport: fakeHTTP}, BaseRepo: baseRepo, } err = verifyAssetRun(opts) - require.NoError(t, err) + require.Error(t, err, "no attestations found for github_release_artifact_invalid.zip in release v1.2.3") } func Test_verifyAssetRun_NoAttestation(t *testing.T) { @@ -133,7 +202,9 @@ func Test_verifyAssetRun_NoAttestation(t *testing.T) { IO: ios, APIClient: api.NewTestClient(), SigstoreVerifier: verification.NewMockSigstoreVerifier(t), - EC: verification.EnforcementCriteria{}, + PredicateType: attestation.ReleasePredicateType, + + EC: verification.EnforcementCriteria{}, } err := verifyAssetRun(opts) diff --git a/pkg/cmd/release/verify/verify.go b/pkg/cmd/release/verify/verify.go index d58628725..c6579f825 100644 --- a/pkg/cmd/release/verify/verify.go +++ b/pkg/cmd/release/verify/verify.go @@ -3,6 +3,7 @@ package verify import ( "context" "errors" + "fmt" v1 "github.com/in-toto/attestation/go/v1" "google.golang.org/protobuf/encoding/protojson" @@ -89,6 +90,7 @@ func NewCmdVerify(f *cmdutil.Factory, runF func(*attestation.AttestOptions) erro } config := verification.SigstoreConfig{ + HttpClient: opts.HttpClient, Logger: opts.Logger, NoPublicGood: true, TrustDomain: td, @@ -148,6 +150,11 @@ func verifyRun(opts *attestation.AttestOptions) error { return err } + if len(filteredAttestations) == 0 { + opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ No attestations found for release %s in %s\n"), opts.TagName, opts.Repo) + return fmt.Errorf("no attestations found for release %s in %s", opts.TagName, opts.Repo) + } + opts.Logger.Printf("Loaded %s from GitHub API\n", text.Pluralize(len(filteredAttestations), "attestation")) // Verify attestations diff --git a/pkg/cmd/release/verify/verify_test.go b/pkg/cmd/release/verify/verify_test.go index 71a282aa2..53078f450 100644 --- a/pkg/cmd/release/verify/verify_test.go +++ b/pkg/cmd/release/verify/verify_test.go @@ -79,7 +79,7 @@ func TestNewCmdVerify_Args(t *testing.T) { func Test_verifyRun_Success(t *testing.T) { ios, _, _, _ := iostreams.Test() - tagName := "v1.2.3" + tagName := "v5" fakeHTTP := &httpmock.Registry{} defer fakeHTTP.Verify(t) @@ -99,6 +99,7 @@ func Test_verifyRun_Success(t *testing.T) { SigstoreVerifier: verification.NewMockSigstoreVerifier(t), HttpClient: &http.Client{Transport: fakeHTTP}, BaseRepo: baseRepo, + PredicateType: attestation.ReleasePredicateType, } ec, err := attestation.NewEnforcementCriteria(opts) @@ -109,7 +110,41 @@ func Test_verifyRun_Success(t *testing.T) { require.NoError(t, err) } -func Test_verifyRun_NoAttestation(t *testing.T) { +func Test_verifyRun_Failed_With_Invalid_Tag(t *testing.T) { + ios, _, _, _ := iostreams.Test() + tagName := "v1.2.3" + + fakeHTTP := &httpmock.Registry{} + defer fakeHTTP.Verify(t) + fakeSHA := "1234567890abcdef1234567890abcdef12345678" + shared.StubFetchRefSHA(t, fakeHTTP, "owner", "repo", tagName, fakeSHA) + + baseRepo, err := ghrepo.FromFullName("owner/repo") + require.NoError(t, err) + + opts := &attestation.AttestOptions{ + TagName: tagName, + Repo: "owner/repo", + Owner: "owner", + Limit: 10, + Logger: io.NewHandler(ios), + APIClient: api.NewFailTestClient(), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), + PredicateType: attestation.ReleasePredicateType, + + HttpClient: &http.Client{Transport: fakeHTTP}, + BaseRepo: baseRepo, + } + + ec, err := attestation.NewEnforcementCriteria(opts) + require.NoError(t, err) + opts.EC = ec + + err = verifyRun(opts) + require.Error(t, err, "failed to fetch attestations from owner/repo") +} + +func Test_verifyRun_Failed_NoAttestation(t *testing.T) { ios, _, _, _ := iostreams.Test() tagName := "v1.2.3" @@ -131,6 +166,7 @@ func Test_verifyRun_NoAttestation(t *testing.T) { SigstoreVerifier: verification.NewMockSigstoreVerifier(t), HttpClient: &http.Client{Transport: fakeHTTP}, BaseRepo: baseRepo, + PredicateType: attestation.ReleasePredicateType, } ec, err := attestation.NewEnforcementCriteria(opts)