diff --git a/pkg/cmd/attestation/artifact/artifact.go b/pkg/cmd/attestation/artifact/artifact.go index dd39eacd5..04dc1642f 100644 --- a/pkg/cmd/attestation/artifact/artifact.go +++ b/pkg/cmd/attestation/artifact/artifact.go @@ -51,7 +51,7 @@ func normalizeReference(reference string, pathSeparator rune) (normalized string return filepath.Clean(reference), fileArtifactType, nil } -func NewDigestedArtifact(client oci.Client, reference, digestAlg string) (artifact *DigestedArtifact, err error) { +func NewDigestedArtifact(client *oci.Client, reference, digestAlg string) (artifact *DigestedArtifact, err error) { normalized, artifactType, err := normalizeReference(reference, os.PathSeparator) if err != nil { return nil, err diff --git a/pkg/cmd/attestation/artifact/image.go b/pkg/cmd/attestation/artifact/image.go index 9d69ac377..32c4f77ad 100644 --- a/pkg/cmd/attestation/artifact/image.go +++ b/pkg/cmd/attestation/artifact/image.go @@ -7,7 +7,10 @@ import ( "github.com/distribution/reference" ) -func digestContainerImageArtifact(url string, client oci.Client) (*DigestedArtifact, error) { +func digestContainerImageArtifact(url string, client *oci.Client) (*DigestedArtifact, error) { + if client == nil { + return nil, fmt.Errorf("missing OCI client") + } // try to parse the url as a valid registry reference named, err := reference.Parse(url) if err != nil { diff --git a/pkg/cmd/attestation/artifact/image_test.go b/pkg/cmd/attestation/artifact/image_test.go index cf02916e0..297cf9a39 100644 --- a/pkg/cmd/attestation/artifact/image_test.go +++ b/pkg/cmd/attestation/artifact/image_test.go @@ -19,23 +19,34 @@ func TestDigestContainerImageArtifact(t *testing.T) { assert.Equal(t, "sha256", digestedArtifact.digestAlg) } -func TestFetchImageFailure(t *testing.T) { +func TestParseImageRefFailure(t *testing.T) { client := oci.NewReferenceFailClient() url := "example.com/repo:tag" _, err := digestContainerImageArtifact(url, client) assert.Error(t, err) } -func TestRegistryAuthFailure(t *testing.T) { - client := oci.NewAuthFailClient() - url := "example.com/repo:tag" - _, err := digestContainerImageArtifact(url, client) - assert.ErrorIs(t, err, oci.ErrRegistryAuthz) -} +func TestFetchImageFailure(t *testing.T) { + testcase := []struct { + name string + client *oci.Client + expectedErr error + }{ + { + name: "Fail to authorize with registry", + client: oci.NewAuthFailClient(), + expectedErr: oci.ErrRegistryAuthz, + }, + { + name: "Fail to fetch image due to denial", + client: oci.NewDeniedClient(), + expectedErr: oci.ErrDenied, + }, + } -func TestDeniedFailure(t *testing.T) { - client := oci.NewDeniedClient() - url := "example.com/repo:tag" - _, err := digestContainerImageArtifact(url, client) - assert.ErrorIs(t, err, oci.ErrDenied) + for _, tc := range testcase { + url := "example.com/repo:tag" + _, err := digestContainerImageArtifact(url, tc.client) + assert.ErrorIs(t, err, tc.expectedErr) + } } diff --git a/pkg/cmd/attestation/artifact/oci/oci.go b/pkg/cmd/attestation/artifact/oci/oci.go index a12b70ab9..4d7a02586 100644 --- a/pkg/cmd/attestation/artifact/oci/oci.go +++ b/pkg/cmd/attestation/artifact/oci/oci.go @@ -52,15 +52,15 @@ func (c Client) GetImageDigest(imgName string) (*v1.Hash, error) { return &desc.Digest, nil } -func NewLiveClient() Client { - return Client{ +func NewLiveClient() *Client { + return &Client{ ParseReference: name.ParseReference, Get: remote.Get, } } -func NewMockClient() Client { - return Client{ +func NewMockClient() *Client { + return &Client{ ParseReference: func(string, ...name.Option) (name.Reference, error) { return name.Tag{}, nil }, @@ -76,8 +76,8 @@ func NewMockClient() Client { } } -func NewReferenceFailClient() Client { - return Client{ +func NewReferenceFailClient() *Client { + return &Client{ ParseReference: func(string, ...name.Option) (name.Reference, error) { return nil, fmt.Errorf("failed to parse reference") }, @@ -87,8 +87,8 @@ func NewReferenceFailClient() Client { } } -func NewAuthFailClient() Client { - return Client{ +func NewAuthFailClient() *Client { + return &Client{ ParseReference: func(string, ...name.Option) (name.Reference, error) { return name.Tag{}, nil }, @@ -98,8 +98,8 @@ func NewAuthFailClient() Client { } } -func NewDeniedClient() Client { - return Client{ +func NewDeniedClient() *Client { + return &Client{ ParseReference: func(string, ...name.Option) (name.Reference, error) { return name.Tag{}, nil }, diff --git a/pkg/cmd/attestation/attestation.go b/pkg/cmd/attestation/attestation.go index a6f8674fd..d3b5f57f9 100644 --- a/pkg/cmd/attestation/attestation.go +++ b/pkg/cmd/attestation/attestation.go @@ -16,6 +16,7 @@ func NewCmdAttestation(f *cmdutil.Factory) *cobra.Command { Use: "attestation [subcommand]", Short: "Work with attestations", Aliases: []string{"at"}, + Hidden: true, Long: heredoc.Docf(` Work with attestations that represent trusted metadata about artifacts and images. diff --git a/pkg/cmd/attestation/download/download.go b/pkg/cmd/attestation/download/download.go index 95d61f53a..4f3517b0f 100644 --- a/pkg/cmd/attestation/download/download.go +++ b/pkg/cmd/attestation/download/download.go @@ -102,6 +102,9 @@ func NewDownloadCmd(f *cmdutil.Factory) *cobra.Command { } func RunDownload(opts *Options) error { + if opts.APIClient == nil { + return fmt.Errorf("missing API client") + } artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm) if err != nil { return fmt.Errorf("failed to digest artifact: %w", err) diff --git a/pkg/cmd/attestation/download/download_test.go b/pkg/cmd/attestation/download/download_test.go index cdb1c079d..238376b10 100644 --- a/pkg/cmd/attestation/download/download_test.go +++ b/pkg/cmd/attestation/download/download_test.go @@ -108,6 +108,19 @@ func TestRunDownload(t *testing.T) { assert.Error(t, err) assert.ErrorContains(t, err, "failed to digest artifact") }) + + t.Run("with missing OCI client", func(t *testing.T) { + customOpts := baseOpts + customOpts.ArtifactPath = "oci://ghcr.io/github/test" + customOpts.OCIClient = nil + assert.Error(t, RunDownload(&customOpts)) + }) + + t.Run("with missing API client", func(t *testing.T) { + customOpts := baseOpts + customOpts.APIClient = nil + assert.Error(t, RunDownload(&customOpts)) + }) } func TestCreateJSONLinesFilePath(t *testing.T) { diff --git a/pkg/cmd/attestation/download/options.go b/pkg/cmd/attestation/download/options.go index 1e66d58d3..68f858534 100644 --- a/pkg/cmd/attestation/download/options.go +++ b/pkg/cmd/attestation/download/options.go @@ -10,12 +10,12 @@ import ( ) type Options struct { + APIClient api.Client ArtifactPath string DigestAlgorithm string - APIClient api.Client Logger *logging.Logger Limit int - OCIClient oci.Client + OCIClient *oci.Client OutputPath string Owner string Verbose bool diff --git a/pkg/cmd/attestation/inspect/inspect_test.go b/pkg/cmd/attestation/inspect/inspect_test.go index 6f480eb1e..fe5284dd0 100644 --- a/pkg/cmd/attestation/inspect/inspect_test.go +++ b/pkg/cmd/attestation/inspect/inspect_test.go @@ -3,6 +3,7 @@ package inspect import ( "testing" + "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" "github.com/cli/cli/v2/pkg/cmd/attestation/logging" "github.com/cli/cli/v2/pkg/cmd/attestation/test" @@ -30,6 +31,7 @@ func TestRunInspect(t *testing.T) { BundlePath: bundlePath, DigestAlgorithm: "sha512", Logger: logger, + OCIClient: oci.NewMockClient(), } t.Run("with valid artifact and bundle", func(t *testing.T) { @@ -63,4 +65,11 @@ func TestRunInspect(t *testing.T) { customOpts.BundlePath = "../test/data/sigstore-js-2.1.0_with_2_bundles.jsonl" assert.Nil(t, RunInspect(&customOpts)) }) + + t.Run("with missing OCI client", func(t *testing.T) { + customOpts := opts + customOpts.ArtifactPath = "oci://ghcr.io/github/test" + customOpts.OCIClient = nil + assert.Error(t, RunInspect(&customOpts)) + }) } diff --git a/pkg/cmd/attestation/inspect/options.go b/pkg/cmd/attestation/inspect/options.go index eaedd73f1..a8acbc77a 100644 --- a/pkg/cmd/attestation/inspect/options.go +++ b/pkg/cmd/attestation/inspect/options.go @@ -18,7 +18,7 @@ type Options struct { Verbose bool Quiet bool Logger *logging.Logger - OCIClient oci.Client + OCIClient *oci.Client } // Clean cleans the file path option values diff --git a/pkg/cmd/attestation/tufrootverify/tuf-root-verify.go b/pkg/cmd/attestation/tufrootverify/tuf-root-verify.go index 2ca57c340..22074c709 100644 --- a/pkg/cmd/attestation/tufrootverify/tuf-root-verify.go +++ b/pkg/cmd/attestation/tufrootverify/tuf-root-verify.go @@ -18,10 +18,9 @@ func NewTUFRootVerifyCmd(f *cmdutil.Factory) *cobra.Command { var mirror string var root string var cmd = cobra.Command{ - Use: "tuf-root-verify --mirror --root ", - Args: cobra.ExactArgs(0), - Hidden: true, - Short: "Verify the TUF repository from a provided TUF root", + Use: "tuf-root-verify --mirror --root ", + Args: cobra.ExactArgs(0), + Short: "Verify the TUF repository from a provided TUF root", Long: heredoc.Docf(` Verify a TUF repository with a local TUF root. diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index 2dc623998..e972a18ab 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -93,6 +93,9 @@ func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) { } func getRemoteAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) { + if c.APIClient == nil { + return nil, fmt.Errorf("api client must be provided") + } // check if Repo is set first because if Repo has been set, Owner will be set using the value of Repo. // If Repo is not set, the field will remain empty. It will not be populated using the value of Owner. if c.Repo != "" { diff --git a/pkg/cmd/attestation/verify/options.go b/pkg/cmd/attestation/verify/options.go index 19b1f8008..059891428 100644 --- a/pkg/cmd/attestation/verify/options.go +++ b/pkg/cmd/attestation/verify/options.go @@ -30,7 +30,7 @@ type Options struct { APIClient api.Client Logger *logging.Logger Limit int - OCIClient oci.Client + OCIClient *oci.Client } // Clean cleans the file path option values diff --git a/pkg/cmd/attestation/verify/verify_test.go b/pkg/cmd/attestation/verify/verify_test.go index efae0a823..9314e5f07 100644 --- a/pkg/cmd/attestation/verify/verify_test.go +++ b/pkg/cmd/attestation/verify/verify_test.go @@ -162,6 +162,20 @@ func TestRunVerify(t *testing.T) { opts.BundlePath = "../test/data/sigstore-js-2.1.0_with_2_bundles.jsonl" assert.Nil(t, RunVerify(&opts)) }) + + t.Run("with missing OCI client", func(t *testing.T) { + customOpts := publicGoodOpts + customOpts.ArtifactPath = "oci://ghcr.io/github/test" + customOpts.OCIClient = nil + assert.Error(t, RunVerify(&customOpts)) + }) + + t.Run("with missing API client", func(t *testing.T) { + customOpts := publicGoodOpts + customOpts.APIClient = nil + customOpts.BundlePath = "" + assert.Error(t, RunVerify(&customOpts)) + }) } func TestVerifySLSAPredicateType_InvalidPredicate(t *testing.T) {