From 20d39314279502ec3bc1bba40f4a4614f9d218c0 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Mon, 5 Aug 2024 09:11:25 -0700 Subject: [PATCH 01/12] tmp --- pkg/cmd/attestation/artifact/artifact.go | 13 ++- pkg/cmd/attestation/artifact/image.go | 9 +- pkg/cmd/attestation/artifact/oci/client.go | 130 +++++++++++++++++++-- pkg/cmd/attestation/verify/verify.go | 10 ++ 4 files changed, 146 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/attestation/artifact/artifact.go b/pkg/cmd/attestation/artifact/artifact.go index de354b947..f9302b439 100644 --- a/pkg/cmd/attestation/artifact/artifact.go +++ b/pkg/cmd/attestation/artifact/artifact.go @@ -7,6 +7,8 @@ import ( "path/filepath" "strings" + "github.com/cli/cli/v2/pkg/cmd/attestation/api" + "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" ) @@ -19,9 +21,10 @@ const ( // DigestedArtifact abstracts the software artifact being verified type DigestedArtifact struct { - URL string - digest string - digestAlg string + URL string + digest string + digestAlg string + attestations []*api.Attestation } func normalizeReference(reference string, pathSeparator rune) (normalized string, artifactType artifactType, err error) { @@ -77,3 +80,7 @@ func (a *DigestedArtifact) Algorithm() string { func (a *DigestedArtifact) DigestWithAlg() string { return fmt.Sprintf("%s:%s", a.digestAlg, a.digest) } + +func (a *DigestedArtifact) Attestations() []*api.Attestation { + return a.attestations +} diff --git a/pkg/cmd/attestation/artifact/image.go b/pkg/cmd/attestation/artifact/image.go index 2af13e723..45e3104c2 100644 --- a/pkg/cmd/attestation/artifact/image.go +++ b/pkg/cmd/attestation/artifact/image.go @@ -15,14 +15,15 @@ func digestContainerImageArtifact(url string, client oci.Client) (*DigestedArtif return nil, fmt.Errorf("artifact %s is not a valid registry reference: %v", url, err) } - digest, err := client.GetImageDigest(named.String()) + digest, attestations, err := client.GetImageDigest(named.String()) if err != nil { return nil, err } return &DigestedArtifact{ - URL: fmt.Sprintf("oci://%s", named.String()), - digest: digest.Hex, - digestAlg: digest.Algorithm, + URL: fmt.Sprintf("oci://%s", named.String()), + digest: digest.Hex, + digestAlg: digest.Algorithm, + attestations: attestations, }, nil } diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index 5b8d8cf7a..e0a497e40 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -1,21 +1,27 @@ package oci import ( + "bytes" "errors" "fmt" + "io" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/v1" + "github.com/sigstore/sigstore-go/pkg/bundle" + + "github.com/cli/cli/v2/pkg/cmd/attestation/api" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/remote/transport" + protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1" ) var ErrDenied = errors.New("the provided token was denied access to the requested resource, please check the token's expiration and repository access") var ErrRegistryAuthz = errors.New("remote registry authorization failed, please authenticate with the registry and try again") type Client interface { - GetImageDigest(imgName string) (*v1.Hash, error) + GetImageDigest(imgName string) (*v1.Hash, []*api.Attestation, error) } func checkForUnauthorizedOrDeniedErr(err transport.Error) error { @@ -36,27 +42,133 @@ type LiveClient struct { } // where name is formed like ghcr.io/github/my-image-repo -func (c LiveClient) GetImageDigest(imgName string) (*v1.Hash, error) { - name, err := c.parseReference(imgName) +func (c LiveClient) GetImageDigest(imgName string) (*v1.Hash, []*api.Attestation, error) { + nameFirst, err := c.parseReference(imgName) + var buf bytes.Buffer + attestations := []*api.Attestation{} + if err != nil { - return nil, fmt.Errorf("failed to create image tag: %v", err) + return nil, attestations, fmt.Errorf("failed to create image tag: %v", err) } // The user must already be authenticated with the container registry // The authn.DefaultKeychain argument indicates that Get should checks the // user's configuration for the registry credentials - desc, err := c.get(name, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + desc, err := c.get(nameFirst, remote.WithAuthFromKeychain(authn.DefaultKeychain)) if err != nil { var transportErr *transport.Error if errors.As(err, &transportErr) { if accessErr := checkForUnauthorizedOrDeniedErr(*transportErr); accessErr != nil { - return nil, accessErr + return nil, attestations, accessErr } } - return nil, fmt.Errorf("failed to fetch remote image: %v", err) + return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) } - return &desc.Digest, nil + dgst := nameFirst.Context().Digest(desc.Digest.String()) + + ref, err := remote.Referrers(dgst, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + indexManifest, err := ref.IndexManifest() + if err != nil { + return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) + } + manifests := indexManifest.Manifests + + for _, m := range manifests { + allowedArtifactTypes := []string{"application/vnd.dev.sigstore.bundle.v0.3+json"} + + for _, allowedType := range allowedArtifactTypes { + if allowedType == m.ArtifactType { + manifestDigest := m.Digest.String() + + manifestURL := fmt.Sprintf("%s/manifests/%s", imgName, manifestDigest) + fmt.Println(manifestURL) + + digest2 := nameFirst.Context().Digest(manifestDigest) + // replace to use GET for more correc type + img2, err := remote.Image(digest2, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + if err != nil { + return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) + } + // manifest2, err := ref2.Manifest() + // if err != nil { + // return nil, fmt.Errorf("failed to fetch remote image: %v", err) + // } + + // fmt.Println(manifest2.MediaType) + // Step 4: Get the layers + layers, err := img2.Layers() + if err != nil { + fmt.Println("Error getting layers: %v", err) + return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) + + } + + // For simplicity, we'll just fetch the first layer + if len(layers) > 0 { + fmt.Println("how many layers?", len(layers)) + layer := layers[0] + + // Step 5: Read the blob (layer) content + rc, err := layer.Compressed() + if err != nil { + fmt.Println("Error getting compressed layer: %v", err) + return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) + + } + defer rc.Close() + + layerBytes, err := io.ReadAll(rc) + + if err != nil { + // If creating a gzip reader fails, it might not be compressed + fmt.Println("Layer is not gzip-compressed. Reading raw content.") + // fmt.Println("Blob content:", buf.String()) + + var bundle bundle.ProtobufBundle + bundle.Bundle = new(protobundle.Bundle) + err = bundle.UnmarshalJSON(layerBytes) + fmt.Println("") + fmt.Println("JSON Content:", string(layerBytes)) + fmt.Println("") + + if err != nil { + fmt.Println("failed to unmarshal bundle from JSON: %v", err) + } else { + fmt.Println("Bundle content:", bundle.String()) + } + + a := api.Attestation{Bundle: &bundle} + attestations = append(attestations, &a) + + } else { + defer gz.Close() + + var decompressed bytes.Buffer + if _, err := io.Copy(&decompressed, gz); err != nil { + fmt.Println("Error decompressing layer content: %v", err) + } + + // Now you have the decompressed blob content in 'decompressed' buffer + fmt.Println("Decompressed blob content:", decompressed.String()) + } + + // Now you have the decompressed blob content in 'decompressed' buffer + // fmt.Println("Blob content:", decompressed.String()) + } else { + fmt.Println("No layers found in the image.") + } + } + } + } + + // msgName, err := c.parseReference(msg) + // fmt.Println() + // if err != nil { + // return nil, err + // } + + return &desc.Digest, attestations, nil } // Unlike other parts of this command set, we cannot pass a custom HTTP client diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index e1a5b1c50..990de3985 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -204,6 +204,16 @@ func runVerify(opts *Options) error { return err } + attestationsFromOCI := artifact.Attestations() + if len(attestationsFromOCI) > 0 { + attestations = append(attestations, attestationsFromOCI...) + + for _, attestation := range attestations { + // ja, err := attestation.Bundle.String() + opts.Logger.Printf("Loaded attestation from OCI registry: %s\n", attestation.Bundle.String()) + } + } + pluralAttestation := text.Pluralize(len(attestations), "attestation") if c.IsBundleProvided() { opts.Logger.Printf("Loaded %s from %s\n", pluralAttestation, opts.BundlePath) From 8d17896080cb6e0b117203be9c7014b7f3a533f0 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Mon, 5 Aug 2024 12:25:52 -0700 Subject: [PATCH 02/12] refactor the logic and logging --- pkg/cmd/attestation/artifact/artifact.go | 4 +- pkg/cmd/attestation/artifact/image.go | 35 +++++- pkg/cmd/attestation/artifact/oci/client.go | 114 ++++++------------ pkg/cmd/attestation/download/download.go | 2 +- pkg/cmd/attestation/inspect/inspect.go | 2 +- .../attestation/verification/attestation.go | 18 ++- pkg/cmd/attestation/verify/options.go | 43 +++---- pkg/cmd/attestation/verify/verify.go | 30 ++--- 8 files changed, 120 insertions(+), 128 deletions(-) diff --git a/pkg/cmd/attestation/artifact/artifact.go b/pkg/cmd/attestation/artifact/artifact.go index f9302b439..e8e0ea095 100644 --- a/pkg/cmd/attestation/artifact/artifact.go +++ b/pkg/cmd/attestation/artifact/artifact.go @@ -54,14 +54,14 @@ 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, useBundleFromRegistry bool) (artifact *DigestedArtifact, err error) { normalized, artifactType, err := normalizeReference(reference, os.PathSeparator) if err != nil { return nil, err } if artifactType == ociArtifactType { // TODO: should we allow custom digestAlg for OCI artifacts? - return digestContainerImageArtifact(normalized, client) + return digestContainerImageArtifact(normalized, client, useBundleFromRegistry) } return digestLocalFileArtifact(normalized, digestAlg) } diff --git a/pkg/cmd/attestation/artifact/image.go b/pkg/cmd/attestation/artifact/image.go index 45e3104c2..e1fba97bf 100644 --- a/pkg/cmd/attestation/artifact/image.go +++ b/pkg/cmd/attestation/artifact/image.go @@ -7,7 +7,7 @@ import ( "github.com/distribution/reference" ) -func digestContainerImageArtifact(url string, client oci.Client) (*DigestedArtifact, error) { +func digestContainerImageArtifact(url string, client oci.Client, useBundleFromRegistry bool) (*DigestedArtifact, error) { // try to parse the url as a valid registry reference named, err := reference.Parse(url) if err != nil { @@ -15,15 +15,38 @@ func digestContainerImageArtifact(url string, client oci.Client) (*DigestedArtif return nil, fmt.Errorf("artifact %s is not a valid registry reference: %v", url, err) } - digest, attestations, err := client.GetImageDigest(named.String()) + name, err := client.ParseReference(named.String()) + if err != nil { + return nil, err + } + + digest, err := client.GetImageDigest(name) + + if err != nil { + return nil, err + } + if useBundleFromRegistry { + attestations, err := client.GetAttestations(name, digest) + + if err != nil { + return nil, err + } + + return &DigestedArtifact{ + URL: fmt.Sprintf("oci://%s", named.String()), + digest: digest.Hex, + digestAlg: digest.Algorithm, + attestations: attestations, + }, nil + } + if err != nil { return nil, err } return &DigestedArtifact{ - URL: fmt.Sprintf("oci://%s", named.String()), - digest: digest.Hex, - digestAlg: digest.Algorithm, - attestations: attestations, + URL: fmt.Sprintf("oci://%s", named.String()), + digest: digest.Hex, + digestAlg: digest.Algorithm, }, nil } diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index e0a497e40..9e53c6f2b 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -1,7 +1,6 @@ package oci import ( - "bytes" "errors" "fmt" "io" @@ -21,7 +20,9 @@ var ErrDenied = errors.New("the provided token was denied access to the requeste var ErrRegistryAuthz = errors.New("remote registry authorization failed, please authenticate with the registry and try again") type Client interface { - GetImageDigest(imgName string) (*v1.Hash, []*api.Attestation, error) + GetImageDigest(name name.Reference) (*v1.Hash, error) + GetAttestations(name name.Reference, digest *v1.Hash) ([]*api.Attestation, error) + ParseReference(ref string) (name.Reference, error) } func checkForUnauthorizedOrDeniedErr(err transport.Error) error { @@ -41,36 +42,40 @@ type LiveClient struct { get func(name.Reference, ...remote.Option) (*remote.Descriptor, error) } +func (c LiveClient) ParseReference(ref string) (name.Reference, error) { + return c.parseReference(ref) +} + // where name is formed like ghcr.io/github/my-image-repo -func (c LiveClient) GetImageDigest(imgName string) (*v1.Hash, []*api.Attestation, error) { - nameFirst, err := c.parseReference(imgName) - var buf bytes.Buffer - attestations := []*api.Attestation{} - - if err != nil { - return nil, attestations, fmt.Errorf("failed to create image tag: %v", err) - } - +func (c LiveClient) GetImageDigest(name name.Reference) (*v1.Hash, error) { // The user must already be authenticated with the container registry // The authn.DefaultKeychain argument indicates that Get should checks the // user's configuration for the registry credentials - desc, err := c.get(nameFirst, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + desc, err := c.get(name, remote.WithAuthFromKeychain(authn.DefaultKeychain)) if err != nil { var transportErr *transport.Error if errors.As(err, &transportErr) { if accessErr := checkForUnauthorizedOrDeniedErr(*transportErr); accessErr != nil { - return nil, attestations, accessErr + return nil, accessErr } } - return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) + return nil, fmt.Errorf("failed to fetch remote image: %v", err) } - dgst := nameFirst.Context().Digest(desc.Digest.String()) + return &desc.Digest, nil +} - ref, err := remote.Referrers(dgst, remote.WithAuthFromKeychain(authn.DefaultKeychain)) +func (c LiveClient) GetAttestations(name name.Reference, digest *v1.Hash) ([]*api.Attestation, error) { + attestations := []*api.Attestation{} + nameDegist := name.Context().Digest(digest.String()) + + ref, err := remote.Referrers(nameDegist, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + if err != nil { + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + } indexManifest, err := ref.IndexManifest() if err != nil { - return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } manifests := indexManifest.Manifests @@ -81,94 +86,55 @@ func (c LiveClient) GetImageDigest(imgName string) (*v1.Hash, []*api.Attestation if allowedType == m.ArtifactType { manifestDigest := m.Digest.String() - manifestURL := fmt.Sprintf("%s/manifests/%s", imgName, manifestDigest) - fmt.Println(manifestURL) - - digest2 := nameFirst.Context().Digest(manifestDigest) + digest2 := nameDegist.Context().Digest(manifestDigest) // replace to use GET for more correc type img2, err := remote.Image(digest2, remote.WithAuthFromKeychain(authn.DefaultKeychain)) if err != nil { - return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } - // manifest2, err := ref2.Manifest() - // if err != nil { - // return nil, fmt.Errorf("failed to fetch remote image: %v", err) - // } - // fmt.Println(manifest2.MediaType) // Step 4: Get the layers layers, err := img2.Layers() if err != nil { - fmt.Println("Error getting layers: %v", err) - return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } // For simplicity, we'll just fetch the first layer if len(layers) > 0 { - fmt.Println("how many layers?", len(layers)) layer := layers[0] // Step 5: Read the blob (layer) content rc, err := layer.Compressed() if err != nil { - fmt.Println("Error getting compressed layer: %v", err) - return nil, attestations, fmt.Errorf("failed to fetch remote image: %v", err) - + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } defer rc.Close() layerBytes, err := io.ReadAll(rc) if err != nil { - // If creating a gzip reader fails, it might not be compressed - fmt.Println("Layer is not gzip-compressed. Reading raw content.") - // fmt.Println("Blob content:", buf.String()) - - var bundle bundle.ProtobufBundle - bundle.Bundle = new(protobundle.Bundle) - err = bundle.UnmarshalJSON(layerBytes) - fmt.Println("") - fmt.Println("JSON Content:", string(layerBytes)) - fmt.Println("") - - if err != nil { - fmt.Println("failed to unmarshal bundle from JSON: %v", err) - } else { - fmt.Println("Bundle content:", bundle.String()) - } - - a := api.Attestation{Bundle: &bundle} - attestations = append(attestations, &a) - - } else { - defer gz.Close() - - var decompressed bytes.Buffer - if _, err := io.Copy(&decompressed, gz); err != nil { - fmt.Println("Error decompressing layer content: %v", err) - } - - // Now you have the decompressed blob content in 'decompressed' buffer - fmt.Println("Decompressed blob content:", decompressed.String()) + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } - // Now you have the decompressed blob content in 'decompressed' buffer - // fmt.Println("Blob content:", decompressed.String()) + var bundle bundle.ProtobufBundle + bundle.Bundle = new(protobundle.Bundle) + err = bundle.UnmarshalJSON(layerBytes) + + if err != nil { + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + } + + a := api.Attestation{Bundle: &bundle} + attestations = append(attestations, &a) + } else { - fmt.Println("No layers found in the image.") + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } } } } - - // msgName, err := c.parseReference(msg) - // fmt.Println() - // if err != nil { - // return nil, err - // } - - return &desc.Digest, attestations, nil + return attestations, nil } // Unlike other parts of this command set, we cannot pass a custom HTTP client diff --git a/pkg/cmd/attestation/download/download.go b/pkg/cmd/attestation/download/download.go index 3af3b6200..6d74027ae 100644 --- a/pkg/cmd/attestation/download/download.go +++ b/pkg/cmd/attestation/download/download.go @@ -111,7 +111,7 @@ func NewDownloadCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Comman } func runDownload(opts *Options) error { - artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm) + artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm, false) if err != nil { return fmt.Errorf("failed to digest artifact: %v", err) } diff --git a/pkg/cmd/attestation/inspect/inspect.go b/pkg/cmd/attestation/inspect/inspect.go index 2731ee7a4..f38181b77 100644 --- a/pkg/cmd/attestation/inspect/inspect.go +++ b/pkg/cmd/attestation/inspect/inspect.go @@ -97,7 +97,7 @@ func NewInspectCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command } func runInspect(opts *Options) error { - artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm) + artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm, false) if err != nil { return fmt.Errorf("failed to digest artifact: %s", err) } diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index 52a8ff025..902657d41 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -16,12 +16,13 @@ import ( var ErrUnrecognisedBundleExtension = errors.New("bundle file extension not supported, must be json or jsonl") type FetchAttestationsConfig struct { - APIClient api.Client - BundlePath string - Digest string - Limit int - Owner string - Repo string + APIClient api.Client + BundlePath string + Digest string + Limit int + Owner string + Repo string + AttestationsFromOCI []*api.Attestation } func (c *FetchAttestationsConfig) IsBundleProvided() bool { @@ -32,6 +33,11 @@ func GetAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) { if c.IsBundleProvided() { return GetLocalAttestations(c.BundlePath) } + + if len(c.AttestationsFromOCI) > 0 { + return c.AttestationsFromOCI, nil + } + return GetRemoteAttestations(c) } diff --git a/pkg/cmd/attestation/verify/options.go b/pkg/cmd/attestation/verify/options.go index da2c7bb4e..902024b90 100644 --- a/pkg/cmd/attestation/verify/options.go +++ b/pkg/cmd/attestation/verify/options.go @@ -15,27 +15,28 @@ import ( // Options captures the options for the verify command type Options struct { - ArtifactPath string - BundlePath string - Config func() (gh.Config, error) - TrustedRoot string - DenySelfHostedRunner bool - DigestAlgorithm string - Limit int - NoPublicGood bool - OIDCIssuer string - Owner string - PredicateType string - Repo string - SAN string - SANRegex string - SignerRepo string - SignerWorkflow string - APIClient api.Client - Logger *io.Handler - OCIClient oci.Client - SigstoreVerifier verification.SigstoreVerifier - exporter cmdutil.Exporter + ArtifactPath string + BundlePath string + UseBundleFromRegistry bool + Config func() (gh.Config, error) + TrustedRoot string + DenySelfHostedRunner bool + DigestAlgorithm string + Limit int + NoPublicGood bool + OIDCIssuer string + Owner string + PredicateType string + Repo string + SAN string + SANRegex string + SignerRepo string + SignerWorkflow string + APIClient api.Client + Logger *io.Handler + OCIClient oci.Client + SigstoreVerifier verification.SigstoreVerifier + exporter cmdutil.Exporter } // Clean cleans the file path option values diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 990de3985..ffa87a383 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -150,6 +150,7 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command // general flags verifyCmd.Flags().StringVarP(&opts.BundlePath, "bundle", "b", "", "Path to bundle on disk, either a single bundle in a JSON file or a JSON lines file with multiple bundles") cmdutil.DisableAuthCheckFlag(verifyCmd.Flags().Lookup("bundle")) + verifyCmd.Flags().BoolVarP(&opts.UseBundleFromRegistry, "bundle-from-registry", "", false, "Use the bundle from the OCI registry") cmdutil.StringEnumFlag(verifyCmd, &opts.DigestAlgorithm, "digest-alg", "d", "sha256", []string{"sha256", "sha512"}, "The algorithm used to compute a digest of the artifact") verifyCmd.Flags().StringVarP(&opts.Owner, "owner", "o", "", "GitHub organization to scope attestation lookup by") verifyCmd.Flags().StringVarP(&opts.Repo, "repo", "R", "", "Repository name in the format /") @@ -173,7 +174,7 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command } func runVerify(opts *Options) error { - artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm) + artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm, opts.UseBundleFromRegistry) if err != nil { opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ Loading digest for %s failed\n"), opts.ArtifactPath) return err @@ -182,12 +183,13 @@ func runVerify(opts *Options) error { opts.Logger.Printf("Loaded digest %s for %s\n", artifact.DigestWithAlg(), artifact.URL) c := verification.FetchAttestationsConfig{ - APIClient: opts.APIClient, - BundlePath: opts.BundlePath, - Digest: artifact.DigestWithAlg(), - Limit: opts.Limit, - Owner: opts.Owner, - Repo: opts.Repo, + APIClient: opts.APIClient, + BundlePath: opts.BundlePath, + Digest: artifact.DigestWithAlg(), + Limit: opts.Limit, + Owner: opts.Owner, + Repo: opts.Repo, + AttestationsFromOCI: artifact.Attestations(), } attestations, err := verification.GetAttestations(c) if err != nil { @@ -198,25 +200,19 @@ func runVerify(opts *Options) error { if c.IsBundleProvided() { opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ Loading attestations from %s failed\n"), artifact.URL) + } else if opts.UseBundleFromRegistry { + opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Loading attestations from OCI registry failed")) } else { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Loading attestations from GitHub API failed")) } return err } - attestationsFromOCI := artifact.Attestations() - if len(attestationsFromOCI) > 0 { - attestations = append(attestations, attestationsFromOCI...) - - for _, attestation := range attestations { - // ja, err := attestation.Bundle.String() - opts.Logger.Printf("Loaded attestation from OCI registry: %s\n", attestation.Bundle.String()) - } - } - pluralAttestation := text.Pluralize(len(attestations), "attestation") if c.IsBundleProvided() { opts.Logger.Printf("Loaded %s from %s\n", pluralAttestation, opts.BundlePath) + } else if opts.UseBundleFromRegistry { + opts.Logger.Printf("Loaded %s from %s\n", pluralAttestation, opts.ArtifactPath) } else { opts.Logger.Printf("Loaded %s from GitHub API\n", pluralAttestation) } From 8ae4f1cfb9278530aa22e84ccb3dc789587af1ef Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Mon, 5 Aug 2024 12:53:43 -0700 Subject: [PATCH 03/12] add contain check --- pkg/cmd/attestation/artifact/oci/client.go | 94 ++++++++++++---------- 1 file changed, 52 insertions(+), 42 deletions(-) diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index 9e53c6f2b..0171b4187 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -65,78 +65,88 @@ func (c LiveClient) GetImageDigest(name name.Reference) (*v1.Hash, error) { return &desc.Digest, nil } +// Ref: https://github.com/github/package-security/blob/main/garden/retrieve-sigstore-bundle-from-oci-registry.md func (c LiveClient) GetAttestations(name name.Reference, digest *v1.Hash) ([]*api.Attestation, error) { attestations := []*api.Attestation{} nameDegist := name.Context().Digest(digest.String()) - ref, err := remote.Referrers(nameDegist, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + imageIndex, err := remote.Referrers(nameDegist, remote.WithAuthFromKeychain(authn.DefaultKeychain)) if err != nil { return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } - indexManifest, err := ref.IndexManifest() + indexManifest, err := imageIndex.IndexManifest() if err != nil { return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } manifests := indexManifest.Manifests for _, m := range manifests { - allowedArtifactTypes := []string{"application/vnd.dev.sigstore.bundle.v0.3+json"} - for _, allowedType := range allowedArtifactTypes { - if allowedType == m.ArtifactType { - manifestDigest := m.Digest.String() + if containAllowedArtifactTypes(m.ArtifactType) { + manifestDigest := m.Digest.String() + + digest2 := nameDegist.Context().Digest(manifestDigest) + // TODO: replace to use GET for more correct type + // OR IS IT CORRECT TO USE type IMAGE? + img2, err := remote.Image(digest2, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + if err != nil { + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + } + + // Step 4: Get the layers + layers, err := img2.Layers() + if err != nil { + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + + } + + // For simplicity, we'll just fetch the first layer + if len(layers) > 0 { + layer := layers[0] + + // Step 5: Read the blob (layer) content + rc, err := layer.Compressed() + if err != nil { + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + } + defer rc.Close() + + layerBytes, err := io.ReadAll(rc) - digest2 := nameDegist.Context().Digest(manifestDigest) - // replace to use GET for more correc type - img2, err := remote.Image(digest2, remote.WithAuthFromKeychain(authn.DefaultKeychain)) if err != nil { return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } - // Step 4: Get the layers - layers, err := img2.Layers() + var bundle bundle.ProtobufBundle + bundle.Bundle = new(protobundle.Bundle) + err = bundle.UnmarshalJSON(layerBytes) + if err != nil { return attestations, fmt.Errorf("failed to fetch remote image: %v", err) - } - // For simplicity, we'll just fetch the first layer - if len(layers) > 0 { - layer := layers[0] + a := api.Attestation{Bundle: &bundle} + attestations = append(attestations, &a) - // Step 5: Read the blob (layer) content - rc, err := layer.Compressed() - if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) - } - defer rc.Close() - - layerBytes, err := io.ReadAll(rc) - - if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) - } - - var bundle bundle.ProtobufBundle - bundle.Bundle = new(protobundle.Bundle) - err = bundle.UnmarshalJSON(layerBytes) - - if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) - } - - a := api.Attestation{Bundle: &bundle} - attestations = append(attestations, &a) - - } else { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) - } + } else { + return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } } } return attestations, nil } +func containAllowedArtifactTypes(artifactType string) bool { + allowedArtifactTypes := []string{"application/vnd.dev.sigstore.bundle.v0.3+json"} + + for _, allowedType := range allowedArtifactTypes { + if allowedType == artifactType { + return true + } + } + return false +} + // Unlike other parts of this command set, we cannot pass a custom HTTP client // to the go-containerregistry library. This means we have limited visibility // into the HTTP requests being made to container registries. From bad127c342060eadfaa24b50604a9b8b88476706 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Mon, 5 Aug 2024 12:56:35 -0700 Subject: [PATCH 04/12] clean naming --- pkg/cmd/attestation/artifact/oci/client.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index 0171b4187..8fd57af4c 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -81,30 +81,26 @@ func (c LiveClient) GetAttestations(name name.Reference, digest *v1.Hash) ([]*ap manifests := indexManifest.Manifests for _, m := range manifests { - if containAllowedArtifactTypes(m.ArtifactType) { - manifestDigest := m.Digest.String() - - digest2 := nameDegist.Context().Digest(manifestDigest) + artifactManifestNameDigest := nameDegist.Context().Digest(m.Digest.String()) // TODO: replace to use GET for more correct type // OR IS IT CORRECT TO USE type IMAGE? - img2, err := remote.Image(digest2, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + artifactManifest, err := remote.Image(artifactManifestNameDigest, remote.WithAuthFromKeychain(authn.DefaultKeychain)) if err != nil { return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } // Step 4: Get the layers - layers, err := img2.Layers() + layers, err := artifactManifest.Layers() if err != nil { return attestations, fmt.Errorf("failed to fetch remote image: %v", err) - } // For simplicity, we'll just fetch the first layer if len(layers) > 0 { layer := layers[0] - // Step 5: Read the blob (layer) content + // Step 5: Read the layer content rc, err := layer.Compressed() if err != nil { return attestations, fmt.Errorf("failed to fetch remote image: %v", err) @@ -127,7 +123,6 @@ func (c LiveClient) GetAttestations(name name.Reference, digest *v1.Hash) ([]*ap a := api.Attestation{Bundle: &bundle} attestations = append(attestations, &a) - } else { return attestations, fmt.Errorf("failed to fetch remote image: %v", err) } From 57aea664e5d95136c4b100adacb5eee75af000d3 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Wed, 7 Aug 2024 10:10:59 -0700 Subject: [PATCH 05/12] added test --- pkg/cmd/attestation/artifact/artifact.go | 18 +-- pkg/cmd/attestation/artifact/image.go | 28 +--- pkg/cmd/attestation/artifact/oci/client.go | 135 +++++++++--------- .../attestation/artifact/oci/client_test.go | 14 +- .../attestation/artifact/oci/mock_client.go | 56 ++++++-- pkg/cmd/attestation/download/download.go | 2 +- pkg/cmd/attestation/inspect/inspect.go | 2 +- .../attestation/verification/attestation.go | 34 +++-- pkg/cmd/attestation/verify/verify.go | 22 +-- pkg/cmd/attestation/verify/verify_test.go | 27 ++++ 10 files changed, 204 insertions(+), 134 deletions(-) diff --git a/pkg/cmd/attestation/artifact/artifact.go b/pkg/cmd/attestation/artifact/artifact.go index e8e0ea095..131785166 100644 --- a/pkg/cmd/attestation/artifact/artifact.go +++ b/pkg/cmd/attestation/artifact/artifact.go @@ -7,7 +7,7 @@ import ( "path/filepath" "strings" - "github.com/cli/cli/v2/pkg/cmd/attestation/api" + "github.com/google/go-containerregistry/pkg/name" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" ) @@ -21,10 +21,10 @@ const ( // DigestedArtifact abstracts the software artifact being verified type DigestedArtifact struct { - URL string - digest string - digestAlg string - attestations []*api.Attestation + URL string + digest string + digestAlg string + nameRef name.Reference } func normalizeReference(reference string, pathSeparator rune) (normalized string, artifactType artifactType, err error) { @@ -54,14 +54,14 @@ func normalizeReference(reference string, pathSeparator rune) (normalized string return filepath.Clean(reference), fileArtifactType, nil } -func NewDigestedArtifact(client oci.Client, reference, digestAlg string, useBundleFromRegistry bool) (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 } if artifactType == ociArtifactType { // TODO: should we allow custom digestAlg for OCI artifacts? - return digestContainerImageArtifact(normalized, client, useBundleFromRegistry) + return digestContainerImageArtifact(normalized, client) } return digestLocalFileArtifact(normalized, digestAlg) } @@ -81,6 +81,6 @@ func (a *DigestedArtifact) DigestWithAlg() string { return fmt.Sprintf("%s:%s", a.digestAlg, a.digest) } -func (a *DigestedArtifact) Attestations() []*api.Attestation { - return a.attestations +func (a *DigestedArtifact) NameRef() name.Reference { + return a.nameRef } diff --git a/pkg/cmd/attestation/artifact/image.go b/pkg/cmd/attestation/artifact/image.go index e1fba97bf..dda5f65db 100644 --- a/pkg/cmd/attestation/artifact/image.go +++ b/pkg/cmd/attestation/artifact/image.go @@ -7,7 +7,7 @@ import ( "github.com/distribution/reference" ) -func digestContainerImageArtifact(url string, client oci.Client, useBundleFromRegistry bool) (*DigestedArtifact, error) { +func digestContainerImageArtifact(url string, client oci.Client) (*DigestedArtifact, error) { // try to parse the url as a valid registry reference named, err := reference.Parse(url) if err != nil { @@ -15,30 +15,7 @@ func digestContainerImageArtifact(url string, client oci.Client, useBundleFromRe return nil, fmt.Errorf("artifact %s is not a valid registry reference: %v", url, err) } - name, err := client.ParseReference(named.String()) - if err != nil { - return nil, err - } - - digest, err := client.GetImageDigest(name) - - if err != nil { - return nil, err - } - if useBundleFromRegistry { - attestations, err := client.GetAttestations(name, digest) - - if err != nil { - return nil, err - } - - return &DigestedArtifact{ - URL: fmt.Sprintf("oci://%s", named.String()), - digest: digest.Hex, - digestAlg: digest.Algorithm, - attestations: attestations, - }, nil - } + digest, nameRef, err := client.GetImageDigest(named.String()) if err != nil { return nil, err @@ -48,5 +25,6 @@ func digestContainerImageArtifact(url string, client oci.Client, useBundleFromRe URL: fmt.Sprintf("oci://%s", named.String()), digest: digest.Hex, digestAlg: digest.Algorithm, + nameRef: nameRef, }, nil } diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index 8fd57af4c..55d74fac0 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" "io" + "net/http" + "strings" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" @@ -13,16 +15,14 @@ import ( v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/remote/transport" - protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1" ) var ErrDenied = errors.New("the provided token was denied access to the requested resource, please check the token's expiration and repository access") var ErrRegistryAuthz = errors.New("remote registry authorization failed, please authenticate with the registry and try again") type Client interface { - GetImageDigest(name name.Reference) (*v1.Hash, error) - GetAttestations(name name.Reference, digest *v1.Hash) ([]*api.Attestation, error) - ParseReference(ref string) (name.Reference, error) + GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) + GetAttestations(name name.Reference, digest string) ([]*api.Attestation, error) } func checkForUnauthorizedOrDeniedErr(err transport.Error) error { @@ -40,6 +40,7 @@ func checkForUnauthorizedOrDeniedErr(err transport.Error) error { type LiveClient struct { parseReference func(string, ...name.Option) (name.Reference, error) get func(name.Reference, ...remote.Option) (*remote.Descriptor, error) + referres func(d name.Digest, options ...remote.Option) (v1.ImageIndex, error) } func (c LiveClient) ParseReference(ref string) (name.Reference, error) { @@ -47,7 +48,11 @@ func (c LiveClient) ParseReference(ref string) (name.Reference, error) { } // where name is formed like ghcr.io/github/my-image-repo -func (c LiveClient) GetImageDigest(name name.Reference) (*v1.Hash, error) { +func (c LiveClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) { + name, err := c.parseReference(imgName) + if err != nil { + return nil, nil, fmt.Errorf("failed to create image tag: %v", err) + } // The user must already be authenticated with the container registry // The authn.DefaultKeychain argument indicates that Get should checks the // user's configuration for the registry credentials @@ -56,92 +61,92 @@ func (c LiveClient) GetImageDigest(name name.Reference) (*v1.Hash, error) { var transportErr *transport.Error if errors.As(err, &transportErr) { if accessErr := checkForUnauthorizedOrDeniedErr(*transportErr); accessErr != nil { - return nil, accessErr + return nil, nil, accessErr } } - return nil, fmt.Errorf("failed to fetch remote image: %v", err) + return nil, nil, fmt.Errorf("failed to fetch remote image: %v", err) } - return &desc.Digest, nil + return &desc.Digest, name, nil +} + +type noncompliantRegistryTransport struct{} + +// RoundTrip will check if a request and associated response fulfill the following: +// 1. The response returns a 406 status code +// 2. The request path contains /referrers/ +// If both conditions are met, the response's status code will be overwritten to 404 +// This is a temporary solution to handle non compliant registries that return +// an unexpected status code 406 when the go-containerregistry library used +// by this code attempts to make a request to the referrers API. +// The go-containerregistry library can handle 404 response but not a 406 response. +// See the related go-containerregistry issue: https://github.com/google/go-containerregistry/issues/1962 +func (a *noncompliantRegistryTransport) RoundTrip(req *http.Request) (*http.Response, error) { + resp, err := http.DefaultTransport.RoundTrip(req) + if resp.StatusCode == http.StatusNotAcceptable && strings.Contains(req.URL.Path, "/referrers/") { + resp.StatusCode = http.StatusNotFound + } + + return resp, err } // Ref: https://github.com/github/package-security/blob/main/garden/retrieve-sigstore-bundle-from-oci-registry.md -func (c LiveClient) GetAttestations(name name.Reference, digest *v1.Hash) ([]*api.Attestation, error) { - attestations := []*api.Attestation{} - nameDegist := name.Context().Digest(digest.String()) +func (c LiveClient) GetAttestations(ref name.Reference, digest string) ([]*api.Attestation, error) { + attestations := make([]*api.Attestation, 0) - imageIndex, err := remote.Referrers(nameDegist, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + transportOpts := []remote.Option{remote.WithTransport(&noncompliantRegistryTransport{}), remote.WithAuthFromKeychain(authn.DefaultKeychain)} + referrers, err := remote.Referrers(ref.Context().Digest(digest), transportOpts...) if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + return attestations, fmt.Errorf("error getting referrers: %w", err) } - indexManifest, err := imageIndex.IndexManifest() + refManifest, err := referrers.IndexManifest() if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + return attestations, fmt.Errorf("error getting referrers manifest: %w", err) } - manifests := indexManifest.Manifests - for _, m := range manifests { - if containAllowedArtifactTypes(m.ArtifactType) { - artifactManifestNameDigest := nameDegist.Context().Digest(m.Digest.String()) - // TODO: replace to use GET for more correct type - // OR IS IT CORRECT TO USE type IMAGE? - artifactManifest, err := remote.Image(artifactManifestNameDigest, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + for _, refDesc := range refManifest.Manifests { + if !strings.HasPrefix(refDesc.ArtifactType, "application/vnd.dev.sigstore.bundle") { + continue + } + + refImg, err := remote.Image(ref.Context().Digest(refDesc.Digest.String()), remote.WithAuthFromKeychain(authn.DefaultKeychain)) + if err != nil { + return attestations, fmt.Errorf("error getting referrer image: %w", err) + } + layers, err := refImg.Layers() + if err != nil { + return attestations, fmt.Errorf("error getting referrer image: %w", err) + } + + if len(layers) > 0 { + layer0, err := layers[0].Uncompressed() if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + return attestations, fmt.Errorf("error getting referrer image: %w", err) } + defer layer0.Close() + + bundleBytes, err := io.ReadAll(layer0) - // Step 4: Get the layers - layers, err := artifactManifest.Layers() if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + return attestations, fmt.Errorf("error getting referrer image: %w", err) } - // For simplicity, we'll just fetch the first layer - if len(layers) > 0 { - layer := layers[0] + b := &bundle.ProtobufBundle{} + err = b.UnmarshalJSON(bundleBytes) - // Step 5: Read the layer content - rc, err := layer.Compressed() - if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) - } - defer rc.Close() - - layerBytes, err := io.ReadAll(rc) - - if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) - } - - var bundle bundle.ProtobufBundle - bundle.Bundle = new(protobundle.Bundle) - err = bundle.UnmarshalJSON(layerBytes) - - if err != nil { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) - } - - a := api.Attestation{Bundle: &bundle} - attestations = append(attestations, &a) - } else { - return attestations, fmt.Errorf("failed to fetch remote image: %v", err) + if err != nil { + return attestations, fmt.Errorf("error unmarshalling bundle: %w", err) } + + a := api.Attestation{Bundle: b} + attestations = append(attestations, &a) + } else { + return attestations, fmt.Errorf("error getting referrer image: no layers found") } } return attestations, nil } -func containAllowedArtifactTypes(artifactType string) bool { - allowedArtifactTypes := []string{"application/vnd.dev.sigstore.bundle.v0.3+json"} - - for _, allowedType := range allowedArtifactTypes { - if allowedType == artifactType { - return true - } - } - return false -} - // Unlike other parts of this command set, we cannot pass a custom HTTP client // to the go-containerregistry library. This means we have limited visibility // into the HTTP requests being made to container registries. diff --git a/pkg/cmd/attestation/artifact/oci/client_test.go b/pkg/cmd/attestation/artifact/oci/client_test.go index 9aa415c47..a46533366 100644 --- a/pkg/cmd/attestation/artifact/oci/client_test.go +++ b/pkg/cmd/attestation/artifact/oci/client_test.go @@ -5,7 +5,7 @@ import ( "testing" "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/remote/transport" @@ -30,9 +30,10 @@ func TestGetImageDigest_Success(t *testing.T) { }, } - digest, err := c.GetImageDigest("test") + digest, nameRef, err := c.GetImageDigest("test") require.NoError(t, err) require.Equal(t, &expectedDigest, digest) + require.Equal(t, name.Tag{}, nameRef) } func TestGetImageDigest_ReferenceFail(t *testing.T) { @@ -45,9 +46,10 @@ func TestGetImageDigest_ReferenceFail(t *testing.T) { }, } - digest, err := c.GetImageDigest("test") + digest, nameRef, err := c.GetImageDigest("test") require.Error(t, err) require.Nil(t, digest) + require.Nil(t, nameRef) } func TestGetImageDigest_AuthFail(t *testing.T) { @@ -60,10 +62,11 @@ func TestGetImageDigest_AuthFail(t *testing.T) { }, } - digest, err := c.GetImageDigest("test") + digest, nameRef, err := c.GetImageDigest("test") require.Error(t, err) require.ErrorIs(t, err, ErrRegistryAuthz) require.Nil(t, digest) + require.Nil(t, nameRef) } func TestGetImageDigest_Denied(t *testing.T) { @@ -76,8 +79,9 @@ func TestGetImageDigest_Denied(t *testing.T) { }, } - digest, err := c.GetImageDigest("test") + digest, nameRef, err := c.GetImageDigest("test") require.Error(t, err) require.ErrorIs(t, err, ErrDenied) require.Nil(t, digest) + require.Nil(t, nameRef) } diff --git a/pkg/cmd/attestation/artifact/oci/mock_client.go b/pkg/cmd/attestation/artifact/oci/mock_client.go index 24368dec8..1c9bd7876 100644 --- a/pkg/cmd/attestation/artifact/oci/mock_client.go +++ b/pkg/cmd/attestation/artifact/oci/mock_client.go @@ -3,32 +3,70 @@ package oci import ( "fmt" - "github.com/google/go-containerregistry/pkg/v1" + "github.com/cli/cli/v2/pkg/cmd/attestation/api" + "github.com/cli/cli/v2/pkg/cmd/attestation/test/data" + "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" ) +func makeTestAttestation() api.Attestation { + return api.Attestation{Bundle: data.SigstoreBundle(nil)} +} + type MockClient struct{} -func (c MockClient) GetImageDigest(imgName string) (*v1.Hash, error) { +func (c MockClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) { return &v1.Hash{ Hex: "1234567890abcdef", Algorithm: "sha256", - }, nil + }, nil, nil +} + +func (c MockClient) GetAttestations(name name.Reference, digest string) ([]*api.Attestation, error) { + att1 := makeTestAttestation() + att2 := makeTestAttestation() + return []*api.Attestation{&att1, &att2}, nil } type ReferenceFailClient struct{} -func (c ReferenceFailClient) GetImageDigest(imgName string) (*v1.Hash, error) { - return nil, fmt.Errorf("failed to parse reference") +func (c ReferenceFailClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) { + return nil, nil, fmt.Errorf("failed to parse reference") +} + +func (c ReferenceFailClient) GetAttestations(name name.Reference, digest string) ([]*api.Attestation, error) { + return nil, nil } type AuthFailClient struct{} -func (c AuthFailClient) GetImageDigest(imgName string) (*v1.Hash, error) { - return nil, ErrRegistryAuthz +func (c AuthFailClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) { + return nil, nil, ErrRegistryAuthz +} + +func (c AuthFailClient) GetAttestations(name name.Reference, digest string) ([]*api.Attestation, error) { + return nil, nil } type DeniedClient struct{} -func (c DeniedClient) GetImageDigest(imgName string) (*v1.Hash, error) { - return nil, ErrDenied +func (c DeniedClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) { + return nil, nil, ErrDenied +} + +func (c DeniedClient) GetAttestations(name name.Reference, digest string) ([]*api.Attestation, error) { + return nil, nil +} + +type DeniedAttestationsClient struct{} + +func (c DeniedAttestationsClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) { + return &v1.Hash{ + Hex: "1234567890abcdef", + Algorithm: "sha256", + }, nil, nil +} + +func (c DeniedAttestationsClient) GetAttestations(name name.Reference, digest string) ([]*api.Attestation, error) { + return nil, nil } diff --git a/pkg/cmd/attestation/download/download.go b/pkg/cmd/attestation/download/download.go index 6d74027ae..3af3b6200 100644 --- a/pkg/cmd/attestation/download/download.go +++ b/pkg/cmd/attestation/download/download.go @@ -111,7 +111,7 @@ func NewDownloadCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Comman } func runDownload(opts *Options) error { - artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm, false) + artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm) if err != nil { return fmt.Errorf("failed to digest artifact: %v", err) } diff --git a/pkg/cmd/attestation/inspect/inspect.go b/pkg/cmd/attestation/inspect/inspect.go index f38181b77..2731ee7a4 100644 --- a/pkg/cmd/attestation/inspect/inspect.go +++ b/pkg/cmd/attestation/inspect/inspect.go @@ -97,7 +97,7 @@ func NewInspectCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command } func runInspect(opts *Options) error { - artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm, false) + artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm) if err != nil { return fmt.Errorf("failed to digest artifact: %s", err) } diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index 902657d41..20e6a01bf 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -9,6 +9,8 @@ import ( "path/filepath" "github.com/cli/cli/v2/pkg/cmd/attestation/api" + "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" + "github.com/google/go-containerregistry/pkg/name" protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1" "github.com/sigstore/sigstore-go/pkg/bundle" ) @@ -16,13 +18,15 @@ import ( var ErrUnrecognisedBundleExtension = errors.New("bundle file extension not supported, must be json or jsonl") type FetchAttestationsConfig struct { - APIClient api.Client - BundlePath string - Digest string - Limit int - Owner string - Repo string - AttestationsFromOCI []*api.Attestation + APIClient api.Client + BundlePath string + Digest string + Limit int + Owner string + Repo string + OCIClient oci.Client + UseBundleFromRegistry bool + NameRef name.Reference } func (c *FetchAttestationsConfig) IsBundleProvided() bool { @@ -34,8 +38,8 @@ func GetAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) { return GetLocalAttestations(c.BundlePath) } - if len(c.AttestationsFromOCI) > 0 { - return c.AttestationsFromOCI, nil + if c.UseBundleFromRegistry { + return GetOCIAttestations(c) } return GetRemoteAttestations(c) @@ -121,6 +125,18 @@ func GetRemoteAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error return nil, fmt.Errorf("owner or repo must be provided") } +func GetOCIAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) { + + attestations, err := c.OCIClient.GetAttestations(c.NameRef, c.Digest) + if err != nil { + return nil, fmt.Errorf("failed to fetch OCI attestations: %w", err) + } + if len(attestations) == 0 { + return nil, fmt.Errorf("no OCI attestations found") + } + return attestations, nil +} + type IntotoStatement struct { PredicateType string `json:"predicateType"` } diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index ffa87a383..5f4e70fbc 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -174,7 +174,7 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command } func runVerify(opts *Options) error { - artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm, opts.UseBundleFromRegistry) + artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm) if err != nil { opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ Loading digest for %s failed\n"), opts.ArtifactPath) return err @@ -183,13 +183,15 @@ func runVerify(opts *Options) error { opts.Logger.Printf("Loaded digest %s for %s\n", artifact.DigestWithAlg(), artifact.URL) c := verification.FetchAttestationsConfig{ - APIClient: opts.APIClient, - BundlePath: opts.BundlePath, - Digest: artifact.DigestWithAlg(), - Limit: opts.Limit, - Owner: opts.Owner, - Repo: opts.Repo, - AttestationsFromOCI: artifact.Attestations(), + APIClient: opts.APIClient, + BundlePath: opts.BundlePath, + Digest: artifact.DigestWithAlg(), + Limit: opts.Limit, + Owner: opts.Owner, + Repo: opts.Repo, + OCIClient: opts.OCIClient, + UseBundleFromRegistry: opts.UseBundleFromRegistry, + NameRef: artifact.NameRef(), } attestations, err := verification.GetAttestations(c) if err != nil { @@ -200,7 +202,7 @@ func runVerify(opts *Options) error { if c.IsBundleProvided() { opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ Loading attestations from %s failed\n"), artifact.URL) - } else if opts.UseBundleFromRegistry { + } else if c.UseBundleFromRegistry { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Loading attestations from OCI registry failed")) } else { opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Loading attestations from GitHub API failed")) @@ -211,7 +213,7 @@ func runVerify(opts *Options) error { pluralAttestation := text.Pluralize(len(attestations), "attestation") if c.IsBundleProvided() { opts.Logger.Printf("Loaded %s from %s\n", pluralAttestation, opts.BundlePath) - } else if opts.UseBundleFromRegistry { + } else if c.UseBundleFromRegistry { opts.Logger.Printf("Loaded %s from %s\n", pluralAttestation, opts.ArtifactPath) } else { opts.Logger.Printf("Loaded %s from GitHub API\n", pluralAttestation) diff --git a/pkg/cmd/attestation/verify/verify_test.go b/pkg/cmd/attestation/verify/verify_test.go index 8d06617f3..902fbc2d4 100644 --- a/pkg/cmd/attestation/verify/verify_test.go +++ b/pkg/cmd/attestation/verify/verify_test.go @@ -463,4 +463,31 @@ func TestRunVerify(t *testing.T) { customOpts.BundlePath = "" require.Error(t, runVerify(&customOpts)) }) + + t.Run("with valid OCI artifact", func(t *testing.T) { + customOpts := publicGoodOpts + customOpts.ArtifactPath = "oci://ghcr.io/github/test" + customOpts.BundlePath = "" + + require.Nil(t, runVerify(&customOpts)) + }) + + t.Run("with valid OCI artifact with UseBundleFromRegistry flag", func(t *testing.T) { + customOpts := publicGoodOpts + customOpts.ArtifactPath = "oci://ghcr.io/github/test" + customOpts.BundlePath = "" + customOpts.UseBundleFromRegistry = true + + require.Nil(t, runVerify(&customOpts)) + }) + + t.Run("with valid OCI artifact with UseBundleFromRegistry flag but fail on fetching bundle from registry", func(t *testing.T) { + customOpts := publicGoodOpts + customOpts.ArtifactPath = "oci://ghcr.io/github/test" + customOpts.BundlePath = "" + customOpts.UseBundleFromRegistry = true + customOpts.OCIClient = oci.DeniedAttestationsClient{} + + require.ErrorContains(t, runVerify(&customOpts), "no OCI attestations found") + }) } From d1cd69c81c10bedf6e955e934ab8f326a3535236 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Wed, 7 Aug 2024 10:16:30 -0700 Subject: [PATCH 06/12] minor fixed --- pkg/cmd/attestation/artifact/oci/client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index 55d74fac0..45c28cf7f 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -40,7 +40,6 @@ func checkForUnauthorizedOrDeniedErr(err transport.Error) error { type LiveClient struct { parseReference func(string, ...name.Option) (name.Reference, error) get func(name.Reference, ...remote.Option) (*remote.Descriptor, error) - referres func(d name.Digest, options ...remote.Option) (v1.ImageIndex, error) } func (c LiveClient) ParseReference(ref string) (name.Reference, error) { From 832a43072cf24aac5f8daaf118fc885527dda39b Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Wed, 7 Aug 2024 10:19:47 -0700 Subject: [PATCH 07/12] minor fixed --- pkg/cmd/attestation/artifact/oci/client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index 45c28cf7f..046601637 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -82,6 +82,9 @@ type noncompliantRegistryTransport struct{} // See the related go-containerregistry issue: https://github.com/google/go-containerregistry/issues/1962 func (a *noncompliantRegistryTransport) RoundTrip(req *http.Request) (*http.Response, error) { resp, err := http.DefaultTransport.RoundTrip(req) + if err != nil { + return resp, err + } if resp.StatusCode == http.StatusNotAcceptable && strings.Contains(req.URL.Path, "/referrers/") { resp.StatusCode = http.StatusNotFound } From 5ae03d6e878fcce9743307e5b7a70f1f877e5e3d Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Mon, 12 Aug 2024 07:10:19 -0700 Subject: [PATCH 08/12] addded more test --- pkg/cmd/attestation/artifact/oci/client.go | 1 - .../attestation/artifact/oci/mock_client.go | 19 ++++++++++++++++--- .../attestation/verification/attestation.go | 1 - pkg/cmd/attestation/verify/verify_test.go | 12 +++++++++++- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index 046601637..5428fff2f 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -92,7 +92,6 @@ func (a *noncompliantRegistryTransport) RoundTrip(req *http.Request) (*http.Resp return resp, err } -// Ref: https://github.com/github/package-security/blob/main/garden/retrieve-sigstore-bundle-from-oci-registry.md func (c LiveClient) GetAttestations(ref name.Reference, digest string) ([]*api.Attestation, error) { attestations := make([]*api.Attestation, 0) diff --git a/pkg/cmd/attestation/artifact/oci/mock_client.go b/pkg/cmd/attestation/artifact/oci/mock_client.go index 1c9bd7876..b869c60a9 100644 --- a/pkg/cmd/attestation/artifact/oci/mock_client.go +++ b/pkg/cmd/attestation/artifact/oci/mock_client.go @@ -58,15 +58,28 @@ func (c DeniedClient) GetAttestations(name name.Reference, digest string) ([]*ap return nil, nil } -type DeniedAttestationsClient struct{} +type NoAttestationsClient struct{} -func (c DeniedAttestationsClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) { +func (c NoAttestationsClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) { return &v1.Hash{ Hex: "1234567890abcdef", Algorithm: "sha256", }, nil, nil } -func (c DeniedAttestationsClient) GetAttestations(name name.Reference, digest string) ([]*api.Attestation, error) { +func (c NoAttestationsClient) GetAttestations(name name.Reference, digest string) ([]*api.Attestation, error) { return nil, nil } + +type FailedToFetchAttestationsClient struct{} + +func (c FailedToFetchAttestationsClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, error) { + return &v1.Hash{ + Hex: "1234567890abcdef", + Algorithm: "sha256", + }, nil, nil +} + +func (c FailedToFetchAttestationsClient) GetAttestations(name name.Reference, digest string) ([]*api.Attestation, error) { + return nil, fmt.Errorf("failed to fetch attestations") +} diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index 20e6a01bf..c1fb25b12 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -126,7 +126,6 @@ func GetRemoteAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error } func GetOCIAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) { - attestations, err := c.OCIClient.GetAttestations(c.NameRef, c.Digest) if err != nil { return nil, fmt.Errorf("failed to fetch OCI attestations: %w", err) diff --git a/pkg/cmd/attestation/verify/verify_test.go b/pkg/cmd/attestation/verify/verify_test.go index 902fbc2d4..0f31d900c 100644 --- a/pkg/cmd/attestation/verify/verify_test.go +++ b/pkg/cmd/attestation/verify/verify_test.go @@ -481,12 +481,22 @@ func TestRunVerify(t *testing.T) { require.Nil(t, runVerify(&customOpts)) }) + t.Run("with valid OCI artifact with UseBundleFromRegistry flag but no bundle return from registry", func(t *testing.T) { + customOpts := publicGoodOpts + customOpts.ArtifactPath = "oci://ghcr.io/github/test" + customOpts.BundlePath = "" + customOpts.UseBundleFromRegistry = true + customOpts.OCIClient = oci.NoAttestationsClient{} + + require.ErrorContains(t, runVerify(&customOpts), "no OCI attestations found") + }) + t.Run("with valid OCI artifact with UseBundleFromRegistry flag but fail on fetching bundle from registry", func(t *testing.T) { customOpts := publicGoodOpts customOpts.ArtifactPath = "oci://ghcr.io/github/test" customOpts.BundlePath = "" customOpts.UseBundleFromRegistry = true - customOpts.OCIClient = oci.DeniedAttestationsClient{} + customOpts.OCIClient = oci.NoAttestationsClient{} require.ErrorContains(t, runVerify(&customOpts), "no OCI attestations found") }) From 05891965d0c8827fcee6019bf28258c64a77a3fd Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Thu, 15 Aug 2024 11:56:28 -0400 Subject: [PATCH 09/12] udpate the options --- pkg/cmd/attestation/verify/options.go | 10 +++++ pkg/cmd/attestation/verify/options_test.go | 43 ++++++++++++++++++++++ pkg/cmd/attestation/verify/verify.go | 2 +- 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verify/options.go b/pkg/cmd/attestation/verify/options.go index 902024b90..a21e27c58 100644 --- a/pkg/cmd/attestation/verify/options.go +++ b/pkg/cmd/attestation/verify/options.go @@ -84,6 +84,16 @@ func (opts *Options) AreFlagsValid() error { return fmt.Errorf("limit %d not allowed, must be between 1 and 1000", opts.Limit) } + // Check that the bundle-from-registry flag is only used with OCI artifact paths + if opts.UseBundleFromRegistry && !strings.HasPrefix(opts.ArtifactPath, "oci://") { + return fmt.Errorf("bundle-from-registry flag can only be used with OCI artifact paths") + } + + // Check that both the bundle-from-registry and bundle-path flags are not used together + if opts.UseBundleFromRegistry && opts.BundlePath != "" { + return fmt.Errorf("bundle-from-registry flag cannot be used with bundle-path flag") + } + return nil } diff --git a/pkg/cmd/attestation/verify/options_test.go b/pkg/cmd/attestation/verify/options_test.go index aea131b27..9fd319041 100644 --- a/pkg/cmd/attestation/verify/options_test.go +++ b/pkg/cmd/attestation/verify/options_test.go @@ -116,4 +116,47 @@ func TestSetPolicyFlags(t *testing.T) { require.Equal(t, "sigstore", opts.Owner) require.Equal(t, "^https://github/foo", opts.SANRegex) }) + + t.Run("returns error when UseBundleFromRegistry is true and ArtifactPath is not an OCI path", func(t *testing.T) { + opts := Options{ + ArtifactPath: publicGoodArtifactPath, + DigestAlgorithm: "sha512", + Owner: "sigstore", + UseBundleFromRegistry: true, + Limit: 1, + } + + err := opts.AreFlagsValid() + require.Error(t, err) + require.ErrorContains(t, err, "bundle-from-registry flag can only be used with OCI artifact paths") + }) + + t.Run("does not return error when UseBundleFromRegistry is true and ArtifactPath is an OCI path", func(t *testing.T) { + opts := Options{ + ArtifactPath: "oci://sigstore/sigstore-js:2.1.0", + DigestAlgorithm: "sha512", + OIDCIssuer: "some issuer", + Owner: "sigstore", + UseBundleFromRegistry: true, + Limit: 1, + } + + err := opts.AreFlagsValid() + require.NoError(t, err) + }) + + t.Run("returns error when UseBundleFromRegistry is true and BundlePath is provided", func(t *testing.T) { + opts := Options{ + ArtifactPath: "oci://sigstore/sigstore-js:2.1.0", + BundlePath: publicGoodBundlePath, + DigestAlgorithm: "sha512", + Owner: "sigstore", + UseBundleFromRegistry: true, + Limit: 1, + } + + err := opts.AreFlagsValid() + require.Error(t, err) + require.ErrorContains(t, err, "bundle-from-registry flag cannot be used with bundle-path flag") + }) } diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 5f4e70fbc..f77f5f822 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -150,7 +150,7 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command // general flags verifyCmd.Flags().StringVarP(&opts.BundlePath, "bundle", "b", "", "Path to bundle on disk, either a single bundle in a JSON file or a JSON lines file with multiple bundles") cmdutil.DisableAuthCheckFlag(verifyCmd.Flags().Lookup("bundle")) - verifyCmd.Flags().BoolVarP(&opts.UseBundleFromRegistry, "bundle-from-registry", "", false, "Use the bundle from the OCI registry") + verifyCmd.Flags().BoolVarP(&opts.UseBundleFromRegistry, "bundle-from-registry", "", false, "Use the bundle from the OCI registry when artifact is an OCI image") cmdutil.StringEnumFlag(verifyCmd, &opts.DigestAlgorithm, "digest-alg", "d", "sha256", []string{"sha256", "sha512"}, "The algorithm used to compute a digest of the artifact") verifyCmd.Flags().StringVarP(&opts.Owner, "owner", "o", "", "GitHub organization to scope attestation lookup by") verifyCmd.Flags().StringVarP(&opts.Repo, "repo", "R", "", "Repository name in the format /") From 3fd309bdde0242dc46ff9a5a6473a5592361bce0 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Mon, 19 Aug 2024 10:29:01 -0400 Subject: [PATCH 10/12] rename flag to bundle-from-oci --- pkg/cmd/attestation/verify/options.go | 8 ++++---- pkg/cmd/attestation/verify/options_test.go | 4 ++-- pkg/cmd/attestation/verify/verify.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/attestation/verify/options.go b/pkg/cmd/attestation/verify/options.go index a21e27c58..3ec2d49f1 100644 --- a/pkg/cmd/attestation/verify/options.go +++ b/pkg/cmd/attestation/verify/options.go @@ -84,14 +84,14 @@ func (opts *Options) AreFlagsValid() error { return fmt.Errorf("limit %d not allowed, must be between 1 and 1000", opts.Limit) } - // Check that the bundle-from-registry flag is only used with OCI artifact paths + // Check that the bundle-from-oci flag is only used with OCI artifact paths if opts.UseBundleFromRegistry && !strings.HasPrefix(opts.ArtifactPath, "oci://") { - return fmt.Errorf("bundle-from-registry flag can only be used with OCI artifact paths") + return fmt.Errorf("bundle-from-oci flag can only be used with OCI artifact paths") } - // Check that both the bundle-from-registry and bundle-path flags are not used together + // Check that both the bundle-from-oci and bundle-path flags are not used together if opts.UseBundleFromRegistry && opts.BundlePath != "" { - return fmt.Errorf("bundle-from-registry flag cannot be used with bundle-path flag") + return fmt.Errorf("bundle-from-oci flag cannot be used with bundle-path flag") } return nil diff --git a/pkg/cmd/attestation/verify/options_test.go b/pkg/cmd/attestation/verify/options_test.go index 9fd319041..b9be054a5 100644 --- a/pkg/cmd/attestation/verify/options_test.go +++ b/pkg/cmd/attestation/verify/options_test.go @@ -128,7 +128,7 @@ func TestSetPolicyFlags(t *testing.T) { err := opts.AreFlagsValid() require.Error(t, err) - require.ErrorContains(t, err, "bundle-from-registry flag can only be used with OCI artifact paths") + require.ErrorContains(t, err, "bundle-from-oci flag can only be used with OCI artifact paths") }) t.Run("does not return error when UseBundleFromRegistry is true and ArtifactPath is an OCI path", func(t *testing.T) { @@ -157,6 +157,6 @@ func TestSetPolicyFlags(t *testing.T) { err := opts.AreFlagsValid() require.Error(t, err) - require.ErrorContains(t, err, "bundle-from-registry flag cannot be used with bundle-path flag") + require.ErrorContains(t, err, "bundle-from-oci flag cannot be used with bundle-path flag") }) } diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index f77f5f822..f053240de 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -150,7 +150,7 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command // general flags verifyCmd.Flags().StringVarP(&opts.BundlePath, "bundle", "b", "", "Path to bundle on disk, either a single bundle in a JSON file or a JSON lines file with multiple bundles") cmdutil.DisableAuthCheckFlag(verifyCmd.Flags().Lookup("bundle")) - verifyCmd.Flags().BoolVarP(&opts.UseBundleFromRegistry, "bundle-from-registry", "", false, "Use the bundle from the OCI registry when artifact is an OCI image") + verifyCmd.Flags().BoolVarP(&opts.UseBundleFromRegistry, "bundle-from-oci", "", false, "When verifying an OCI image, fetch the attestation bundle from the OCI registry instead of from GitHub") cmdutil.StringEnumFlag(verifyCmd, &opts.DigestAlgorithm, "digest-alg", "d", "sha256", []string{"sha256", "sha512"}, "The algorithm used to compute a digest of the artifact") verifyCmd.Flags().StringVarP(&opts.Owner, "owner", "o", "", "GitHub organization to scope attestation lookup by") verifyCmd.Flags().StringVarP(&opts.Repo, "repo", "R", "", "Repository name in the format /") From 47a8f4bbdd36e755dc5eba5bb548921329fc70a9 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Tue, 20 Aug 2024 16:14:39 -0400 Subject: [PATCH 11/12] update error message --- pkg/cmd/attestation/verification/attestation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index c1fb25b12..5feca47ea 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -131,7 +131,7 @@ func GetOCIAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) { return nil, fmt.Errorf("failed to fetch OCI attestations: %w", err) } if len(attestations) == 0 { - return nil, fmt.Errorf("no OCI attestations found") + return nil, fmt.Errorf("no attestations found in the OCI registry. Retry the command without the --bundle-from-oci flag to check GitHub for the attestation") } return attestations, nil } From 0d38a2fd8e3a828deb8f92b7d5042463fff43709 Mon Sep 17 00:00:00 2001 From: ejahnGithub Date: Wed, 21 Aug 2024 10:52:42 -0400 Subject: [PATCH 12/12] fixed the test --- pkg/cmd/attestation/verify/verify_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/attestation/verify/verify_test.go b/pkg/cmd/attestation/verify/verify_test.go index 0f31d900c..89c5ae7c1 100644 --- a/pkg/cmd/attestation/verify/verify_test.go +++ b/pkg/cmd/attestation/verify/verify_test.go @@ -488,7 +488,7 @@ func TestRunVerify(t *testing.T) { customOpts.UseBundleFromRegistry = true customOpts.OCIClient = oci.NoAttestationsClient{} - require.ErrorContains(t, runVerify(&customOpts), "no OCI attestations found") + require.ErrorContains(t, runVerify(&customOpts), "no attestations found in the OCI registry. Retry the command without the --bundle-from-oci flag to check GitHub for the attestation") }) t.Run("with valid OCI artifact with UseBundleFromRegistry flag but fail on fetching bundle from registry", func(t *testing.T) { @@ -498,6 +498,6 @@ func TestRunVerify(t *testing.T) { customOpts.UseBundleFromRegistry = true customOpts.OCIClient = oci.NoAttestationsClient{} - require.ErrorContains(t, runVerify(&customOpts), "no OCI attestations found") + require.ErrorContains(t, runVerify(&customOpts), "no attestations found in the OCI registry. Retry the command without the --bundle-from-oci flag to check GitHub for the attestation") }) }