From db823c18b8b8cee3387b253bf22e9e14be43763f Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 20 Feb 2025 16:55:57 +0100 Subject: [PATCH] Allow injection of TUFMetadataDir in tests This avoids multiple tests using the same dir for metadata, which was causing flakes --- .../attestation/trustedroot/trustedroot.go | 5 ++-- pkg/cmd/attestation/verification/sigstore.go | 27 +++++++++++-------- .../verification/sigstore_integration_test.go | 18 ++++++++----- pkg/cmd/attestation/verification/tuf.go | 11 ++++---- pkg/cmd/attestation/verification/tuf_test.go | 10 +++++-- .../verify/attestation_integration_test.go | 4 ++- .../verify/verify_integration_test.go | 13 ++++++--- 7 files changed, 57 insertions(+), 31 deletions(-) diff --git a/pkg/cmd/attestation/trustedroot/trustedroot.go b/pkg/cmd/attestation/trustedroot/trustedroot.go index 1a40aae3f..571fb0eda 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot.go @@ -11,6 +11,7 @@ import ( "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" + o "github.com/cli/cli/v2/pkg/option" ghauth "github.com/cli/go-gh/v2/pkg/auth" "github.com/MakeNowJust/heredoc" @@ -121,7 +122,7 @@ func getTrustedRoot(makeTUF tufClientInstantiator, opts *Options) error { var tufOptions []tufConfig var defaultTR = "trusted_root.json" - tufOpt := verification.DefaultOptionsWithCacheSetting() + tufOpt := verification.DefaultOptionsWithCacheSetting(o.None[string]()) // Disable local caching, so we get up-to-date response from TUF repository tufOpt.CacheValidity = 0 @@ -150,7 +151,7 @@ func getTrustedRoot(makeTUF tufClientInstantiator, opts *Options) error { targets: []string{defaultTR}, }) - tufOpt = verification.GitHubTUFOptions() + tufOpt = verification.GitHubTUFOptions(o.None[string]()) tufOpt.CacheValidity = 0 tufOptions = append(tufOptions, tufConfig{ tufOptions: tufOpt, diff --git a/pkg/cmd/attestation/verification/sigstore.go b/pkg/cmd/attestation/verification/sigstore.go index c71c600c7..6dd31dac0 100644 --- a/pkg/cmd/attestation/verification/sigstore.go +++ b/pkg/cmd/attestation/verification/sigstore.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/io" + o "github.com/cli/cli/v2/pkg/option" "github.com/sigstore/sigstore-go/pkg/bundle" "github.com/sigstore/sigstore-go/pkg/root" @@ -34,6 +35,8 @@ type SigstoreConfig struct { NoPublicGood bool // If tenancy mode is not used, trust domain is empty TrustDomain string + // TUFMetadataDir + TUFMetadataDir o.Option[string] } type SigstoreVerifier interface { @@ -45,7 +48,8 @@ type LiveSigstoreVerifier struct { Logger *io.Handler NoPublicGood bool // If tenancy mode is not used, trust domain is empty - TrustDomain string + TrustDomain string + TUFMetadataDir o.Option[string] } var ErrNoAttestationsVerified = errors.New("no attestations were verified") @@ -55,10 +59,11 @@ var ErrNoAttestationsVerified = errors.New("no attestations were verified") // Public Good, GitHub, or a custom trusted root. func NewLiveSigstoreVerifier(config SigstoreConfig) *LiveSigstoreVerifier { return &LiveSigstoreVerifier{ - TrustedRoot: config.TrustedRoot, - Logger: config.Logger, - NoPublicGood: config.NoPublicGood, - TrustDomain: config.TrustDomain, + TrustedRoot: config.TrustedRoot, + Logger: config.Logger, + NoPublicGood: config.NoPublicGood, + TrustDomain: config.TrustDomain, + TUFMetadataDir: config.TUFMetadataDir, } } @@ -89,9 +94,9 @@ func (v *LiveSigstoreVerifier) chooseVerifier(issuer string) (*verify.SignedEnti if v.NoPublicGood { return nil, fmt.Errorf("detected public good instance but requested verification without public good instance") } - return newPublicGoodVerifier() + return newPublicGoodVerifier(v.TUFMetadataDir) case GitHubIssuerOrg: - return newGitHubVerifier(v.TrustDomain) + return newGitHubVerifier(v.TrustDomain, v.TUFMetadataDir) default: return nil, fmt.Errorf("leaf certificate issuer is not recognized") } @@ -255,10 +260,10 @@ func newCustomVerifier(trustedRoot *root.TrustedRoot) (*verify.SignedEntityVerif return gv, nil } -func newGitHubVerifier(trustDomain string) (*verify.SignedEntityVerifier, error) { +func newGitHubVerifier(trustDomain string, tufMetadataDir o.Option[string]) (*verify.SignedEntityVerifier, error) { var tr string - opts := GitHubTUFOptions() + opts := GitHubTUFOptions(tufMetadataDir) client, err := tuf.New(opts) if err != nil { return nil, fmt.Errorf("failed to create TUF client: %v", err) @@ -289,8 +294,8 @@ func newGitHubVerifierWithTrustedRoot(trustedRoot *root.TrustedRoot) (*verify.Si return gv, nil } -func newPublicGoodVerifier() (*verify.SignedEntityVerifier, error) { - opts := DefaultOptionsWithCacheSetting() +func newPublicGoodVerifier(tufMetadataDir o.Option[string]) (*verify.SignedEntityVerifier, error) { + opts := DefaultOptionsWithCacheSetting(tufMetadataDir) client, err := tuf.New(opts) if err != nil { return nil, fmt.Errorf("failed to create TUF client: %v", err) diff --git a/pkg/cmd/attestation/verification/sigstore_integration_test.go b/pkg/cmd/attestation/verification/sigstore_integration_test.go index c37dd30c2..987fb9caa 100644 --- a/pkg/cmd/attestation/verification/sigstore_integration_test.go +++ b/pkg/cmd/attestation/verification/sigstore_integration_test.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/test" + o "github.com/cli/cli/v2/pkg/option" "github.com/sigstore/sigstore-go/pkg/verify" "github.com/stretchr/testify/require" @@ -50,7 +51,8 @@ func TestLiveSigstoreVerifier(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), }) results, err := verifier.Verify(tc.attestations, publicGoodPolicy(t)) @@ -68,7 +70,8 @@ func TestLiveSigstoreVerifier(t *testing.T) { t.Run("with 2/3 verified attestations", func(t *testing.T) { verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), }) invalidBundle := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0-bundle-v0.1.json") @@ -84,7 +87,8 @@ func TestLiveSigstoreVerifier(t *testing.T) { t.Run("fail with 0/2 verified attestations", func(t *testing.T) { verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), }) invalidBundle := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0-bundle-v0.1.json") @@ -107,7 +111,8 @@ func TestLiveSigstoreVerifier(t *testing.T) { attestations := getAttestationsFor(t, "../test/data/github_provenance_demo-0.0.12-py3-none-any-bundle.jsonl") verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), }) results, err := verifier.Verify(attestations, githubPolicy) @@ -119,8 +124,9 @@ func TestLiveSigstoreVerifier(t *testing.T) { attestations := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0_with_2_bundles.jsonl") verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), - TrustedRoot: test.NormalizeRelativePath("../test/data/trusted_root.json"), + Logger: io.NewTestHandler(), + TrustedRoot: test.NormalizeRelativePath("../test/data/trusted_root.json"), + TUFMetadataDir: o.Some(t.TempDir()), }) results, err := verifier.Verify(attestations, publicGoodPolicy(t)) diff --git a/pkg/cmd/attestation/verification/tuf.go b/pkg/cmd/attestation/verification/tuf.go index f46a483d8..dcfdd0b32 100644 --- a/pkg/cmd/attestation/verification/tuf.go +++ b/pkg/cmd/attestation/verification/tuf.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" + o "github.com/cli/cli/v2/pkg/option" "github.com/cli/go-gh/v2/pkg/config" "github.com/sigstore/sigstore-go/pkg/tuf" ) @@ -14,7 +15,7 @@ var githubRoot []byte const GitHubTUFMirror = "https://tuf-repo.github.com" -func DefaultOptionsWithCacheSetting() *tuf.Options { +func DefaultOptionsWithCacheSetting(tufMetadataDir o.Option[string]) *tuf.Options { opts := tuf.DefaultOptions() // The CODESPACES environment variable will be set to true in a Codespaces workspace @@ -25,8 +26,8 @@ func DefaultOptionsWithCacheSetting() *tuf.Options { opts.DisableLocalCache = true } - // Set the cache path to a directory owned by the CLI - opts.CachePath = filepath.Join(config.CacheDir(), ".sigstore", "root") + // Set the cache path to the provided dir, or a directory owned by the CLI + opts.CachePath = tufMetadataDir.UnwrapOr(filepath.Join(config.CacheDir(), ".sigstore", "root")) // Allow TUF cache for 1 day opts.CacheValidity = 1 @@ -34,8 +35,8 @@ func DefaultOptionsWithCacheSetting() *tuf.Options { return opts } -func GitHubTUFOptions() *tuf.Options { - opts := DefaultOptionsWithCacheSetting() +func GitHubTUFOptions(tufMetadataDir o.Option[string]) *tuf.Options { + opts := DefaultOptionsWithCacheSetting(tufMetadataDir) opts.Root = githubRoot opts.RepositoryBaseURL = GitHubTUFMirror diff --git a/pkg/cmd/attestation/verification/tuf_test.go b/pkg/cmd/attestation/verification/tuf_test.go index 7d816bf82..e8b6ecf98 100644 --- a/pkg/cmd/attestation/verification/tuf_test.go +++ b/pkg/cmd/attestation/verification/tuf_test.go @@ -5,16 +5,22 @@ import ( "path/filepath" "testing" + o "github.com/cli/cli/v2/pkg/option" "github.com/cli/go-gh/v2/pkg/config" "github.com/stretchr/testify/require" ) -func TestGitHubTUFOptions(t *testing.T) { +func TestGitHubTUFOptionsNoMetadataDir(t *testing.T) { os.Setenv("CODESPACES", "true") - opts := GitHubTUFOptions() + opts := GitHubTUFOptions(o.None[string]()) require.Equal(t, GitHubTUFMirror, opts.RepositoryBaseURL) require.NotNil(t, opts.Root) require.True(t, opts.DisableLocalCache) require.Equal(t, filepath.Join(config.CacheDir(), ".sigstore", "root"), opts.CachePath) } + +func TestGitHubTUFOptionsWithMetadataDir(t *testing.T) { + opts := GitHubTUFOptions(o.Some("anything")) + require.Equal(t, "anything", opts.CachePath) +} diff --git a/pkg/cmd/attestation/verify/attestation_integration_test.go b/pkg/cmd/attestation/verify/attestation_integration_test.go index 630eb1dc5..9ff174141 100644 --- a/pkg/cmd/attestation/verify/attestation_integration_test.go +++ b/pkg/cmd/attestation/verify/attestation_integration_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/test" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" + o "github.com/cli/cli/v2/pkg/option" "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "github.com/stretchr/testify/require" ) @@ -25,7 +26,8 @@ func getAttestationsFor(t *testing.T, bundlePath string) []*api.Attestation { func TestVerifyAttestations(t *testing.T) { sgVerifier := verification.NewLiveSigstoreVerifier(verification.SigstoreConfig{ - Logger: io.NewTestHandler(), + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), }) certSummary := certificate.Summary{} diff --git a/pkg/cmd/attestation/verify/verify_integration_test.go b/pkg/cmd/attestation/verify/verify_integration_test.go index f25055d22..09479995c 100644 --- a/pkg/cmd/attestation/verify/verify_integration_test.go +++ b/pkg/cmd/attestation/verify/verify_integration_test.go @@ -11,6 +11,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/test" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmd/factory" + o "github.com/cli/cli/v2/pkg/option" "github.com/cli/go-gh/v2/pkg/auth" "github.com/stretchr/testify/require" ) @@ -19,7 +20,8 @@ func TestVerifyIntegration(t *testing.T) { logger := io.NewTestHandler() sigstoreConfig := verification.SigstoreConfig{ - Logger: logger, + Logger: logger, + TUFMetadataDir: o.Some(t.TempDir()), } cmdFactory := factory.New("test") @@ -130,7 +132,8 @@ func TestVerifyIntegrationCustomIssuer(t *testing.T) { logger := io.NewTestHandler() sigstoreConfig := verification.SigstoreConfig{ - Logger: logger, + Logger: logger, + TUFMetadataDir: o.Some(t.TempDir()), } cmdFactory := factory.New("test") @@ -200,7 +203,8 @@ func TestVerifyIntegrationReusableWorkflow(t *testing.T) { logger := io.NewTestHandler() sigstoreConfig := verification.SigstoreConfig{ - Logger: logger, + Logger: logger, + TUFMetadataDir: o.Some(t.TempDir()), } cmdFactory := factory.New("test") @@ -289,7 +293,8 @@ func TestVerifyIntegrationReusableWorkflowSignerWorkflow(t *testing.T) { logger := io.NewTestHandler() sigstoreConfig := verification.SigstoreConfig{ - Logger: logger, + Logger: logger, + TUFMetadataDir: o.Some(t.TempDir()), } cmdFactory := factory.New("test")