Hotfix: only verify authentication to registry if creds exist (#1893)

## Description

Fixes what I broke.

This does re-architect some portions of OrasRemote and locks it down
more. Everything in `*remote.Registry` is no longer exposed to outside
usage and users of this remote client are restricted to the public
receiver methods written in `pkg/oci`.

The context is now private as it really should not be edited outside of
private receivers within OrasRemote.

During the writing of this PR I found out that ORAs already handles
scopes at the request level and there is zero need to handle scopes
yourself. I have not checked if I never had to do this, or if ORAs
updated.

## Related Issue

Fixes #1881 
Fixes #1795 
Fixes #1821 

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed

---------

Signed-off-by: razzle <[email protected]>

src/pkg/oci/common.go

@@ -6,7 +6,6 @@ package oci
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -31,8 +30,8 @@ const (
 
 // OrasRemote is a wrapper around the Oras remote repository that includes a progress bar for interactive feedback.
 type OrasRemote struct {
-	*remote.Repository
-	context.Context
+	repo      *remote.Repository
+	ctx       context.Context
 	Transport *utils.Transport
 	CopyOpts  oras.CopyOptions
 	client    *auth.Client
@@ -47,18 +46,13 @@ func NewOrasRemote(url string) (*OrasRemote, error) {
 		return nil, fmt.Errorf("failed to parse OCI reference: %w", err)
 	}
 	o := &OrasRemote{}
-	o.Context = context.TODO()
+	o.ctx = context.TODO()
 
 	err = o.WithRepository(ref)
 	if err != nil {
 		return nil, err
 	}
 
-	err = o.CheckAuth()
-	if err != nil {
-		return nil, fmt.Errorf("unable to authenticate to %s: %s", ref.Registry, err.Error())
-	}
-
 	copyOpts := oras.DefaultCopyOptions
 	copyOpts.OnCopySkipped = o.printLayerSuccess
 	copyOpts.PostCopy = o.printLayerSuccess
@@ -86,32 +80,35 @@ func (o *OrasRemote) WithRepository(ref registry.Reference) error {
 	}
 	repo.PlainHTTP = zarfconfig.CommonOptions.Insecure
 	repo.Client = o.client
-	o.Repository = repo
+	o.repo = repo
 	return nil
 }
 
-// withScopes returns a context with the given scopes.
-//
-// This is needed for pushing to Docker Hub.
-func withScopes(ref registry.Reference) context.Context {
-	// For pushing to Docker Hub, we need to set the scope to the repository with pull+push actions, otherwise a 401 is returned
-	scopes := []string{
-		fmt.Sprintf("repository:%s:pull,push", ref.Repository),
-	}
-	return auth.WithScopes(context.TODO(), scopes...)
-}
-
 // withAuthClient returns an auth client for the given reference.
 //
 // The credentials are pulled using Docker's default credential store.
 func (o *OrasRemote) withAuthClient(ref registry.Reference) (*auth.Client, error) {
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.TLSClientConfig.InsecureSkipVerify = zarfconfig.CommonOptions.Insecure
+
+	o.Transport = utils.NewTransport(transport, nil)
+
+	client := &auth.Client{
+		Cache: auth.DefaultCache,
+		Client: &http.Client{
+			Transport: o.Transport,
+		},
+	}
+	client.SetUserAgent("zarf/" + zarfconfig.CLIVersion)
+
 	message.Debugf("Loading docker config file from default config location: %s", config.Dir())
 	cfg, err := config.Load(config.Dir())
 	if err != nil {
 		return nil, err
 	}
 	if !cfg.ContainsAuth() {
-		return nil, errors.New("no docker config file found, run 'zarf tools registry login --help'")
+		message.Debug("no docker config file found, run 'zarf tools registry login --help'")
+		return client, nil
 	}
 
 	configs := []*configfile.ConfigFile{cfg}
@@ -127,41 +124,14 @@ func (o *OrasRemote) withAuthClient(ref registry.Reference) (*auth.Client, error
 		return nil, fmt.Errorf("unable to get credentials for %s: %w", key, err)
 	}
 
-	if authConf.ServerAddress != "" {
-		o.Context = withScopes(ref)
-	}
-
 	cred := auth.Credential{
 		Username:     authConf.Username,
 		Password:     authConf.Password,
 		AccessToken:  authConf.RegistryToken,
 		RefreshToken: authConf.IdentityToken,
 	}
 
-	transport := http.DefaultTransport.(*http.Transport).Clone()
-	transport.TLSClientConfig.InsecureSkipVerify = zarfconfig.CommonOptions.Insecure
-
-	o.Transport = utils.NewTransport(transport, nil)
-
-	client := &auth.Client{
-		Credential: auth.StaticCredential(ref.Registry, cred),
-		Cache:      auth.NewCache(),
-		Client: &http.Client{
-			Transport: o.Transport,
-		},
-	}
-	client.SetUserAgent("zarf/" + zarfconfig.CLIVersion)
+	client.Credential = auth.StaticCredential(ref.Registry, cred)
 
 	return client, nil
 }
-
-// CheckAuth checks if the user is authenticated to the remote registry.
-func (o *OrasRemote) CheckAuth() error {
-	reg, err := remote.NewRegistry(o.Repository.Reference.Registry)
-	if err != nil {
-		return err
-	}
-	reg.PlainHTTP = zarfconfig.CommonOptions.Insecure
-	reg.Client = o.client
-	return reg.Ping(o.Context)
-}

src/pkg/oci/pull.go

@@ -123,7 +123,7 @@ func (o *OrasRemote) LayersFromRequestedComponents(requestedComponents []string)
 //   - zarf.yaml.sig
 func (o *OrasRemote) PullPackage(destinationDir string, concurrency int, layersToPull ...ocispec.Descriptor) (partialPaths []string, err error) {
 	isPartialPull := len(layersToPull) > 0
-	ref := o.Reference
+	ref := o.repo.Reference
 
 	pterm.Println()
 	message.Debugf("Pulling %s", ref.String())
@@ -181,7 +181,7 @@ func (o *OrasRemote) PullPackage(destinationDir string, concurrency int, layersT
 	var wg sync.WaitGroup
 	wg.Add(1)
 	go utils.RenderProgressBarForLocalDirWrite(destinationDir, estimatedBytes, &wg, doneSaving, "Pulling Zarf package data")
-	_, err = oras.Copy(o.Context, o.Repository, ref.String(), dst, ref.String(), copyOpts)
+	_, err = oras.Copy(o.ctx, o.repo, ref.String(), dst, ref.String(), copyOpts)
 	if err != nil {
 		return partialPaths, err
 	}

src/pkg/oci/push.go

@@ -36,19 +36,10 @@ type ConfigPartial struct {
 	Annotations  map[string]string `json:"annotations,omitempty"`
 }
 
-// PushFile pushes the file at the given path to the remote repository.
-func (o *OrasRemote) PushFile(path string) (*ocispec.Descriptor, error) {
-	b, err := os.ReadFile(path)
-	if err != nil {
-		return nil, err
-	}
-	return o.PushBytes(b, ZarfLayerMediaTypeBlob)
-}
-
-// PushBytes pushes the given bytes to the remote repository.
-func (o *OrasRemote) PushBytes(b []byte, mediaType string) (*ocispec.Descriptor, error) {
+// PushLayer pushes the given layer (bytes) to the remote repository.
+func (o *OrasRemote) PushLayer(b []byte, mediaType string) (*ocispec.Descriptor, error) {
 	desc := content.NewDescriptorFromBytes(mediaType, b)
-	return &desc, o.Push(o.Context, desc, bytes.NewReader(b))
+	return &desc, o.repo.Push(o.ctx, desc, bytes.NewReader(b))
 }
 
 func (o *OrasRemote) pushManifestConfigFromMetadata(metadata *types.ZarfMetadata, build *types.ZarfBuildData) (*ocispec.Descriptor, error) {
@@ -65,7 +56,7 @@ func (o *OrasRemote) pushManifestConfigFromMetadata(metadata *types.ZarfMetadata
 	if err != nil {
 		return nil, err
 	}
-	return o.PushBytes(manifestConfigBytes, ocispec.MediaTypeImageConfig)
+	return o.PushLayer(manifestConfigBytes, ocispec.MediaTypeImageConfig)
 }
 
 func (o *OrasRemote) manifestAnnotationsFromMetadata(metadata *types.ZarfMetadata) map[string]string {
@@ -98,11 +89,11 @@ func (o *OrasRemote) generatePackManifest(src *file.Store, descs []ocispec.Descr
 	packOpts.PackImageManifest = true
 	packOpts.ManifestAnnotations = o.manifestAnnotationsFromMetadata(metadata)
 
-	root, err := oras.Pack(o.Context, src, ocispec.MediaTypeImageManifest, descs, packOpts)
+	root, err := oras.Pack(o.ctx, src, ocispec.MediaTypeImageManifest, descs, packOpts)
 	if err != nil {
 		return ocispec.Descriptor{}, err
 	}
-	if err = src.Tag(o.Context, root, root.Digest.String()); err != nil {
+	if err = src.Tag(o.ctx, root, root.Digest.String()); err != nil {
 		return ocispec.Descriptor{}, err
 	}
 
@@ -111,15 +102,15 @@ func (o *OrasRemote) generatePackManifest(src *file.Store, descs []ocispec.Descr
 
 // PublishPackage publishes the package to the remote repository.
 func (o *OrasRemote) PublishPackage(pkg *types.ZarfPackage, sourceDir string, concurrency int) error {
-	ctx := o.Context
+	ctx := o.ctx
 	// source file store
 	src, err := file.New(sourceDir)
 	if err != nil {
 		return err
 	}
 	defer src.Close()
 
-	message.Infof("Publishing package to %s", o.Reference.String())
+	message.Infof("Publishing package to %s", o.repo.Reference)
 	spinner := message.NewProgressSpinner("")
 	defer spinner.Stop()
 
@@ -167,7 +158,7 @@ func (o *OrasRemote) PublishPackage(pkg *types.ZarfPackage, sourceDir string, co
 	}
 	// assumes referrers API is not supported since OCI artifact
 	// media type is not supported
-	o.SetReferrersCapability(false)
+	o.repo.SetReferrersCapability(false)
 
 	// push the manifest config
 	// since this config is so tiny, and the content is not used again
@@ -182,25 +173,25 @@ func (o *OrasRemote) PublishPackage(pkg *types.ZarfPackage, sourceDir string, co
 	}
 	total += root.Size + manifestConfigDesc.Size
 
-	o.Transport.ProgressBar = message.NewProgressBar(total, fmt.Sprintf("Publishing %s:%s", o.Reference.Repository, o.Reference.Reference))
+	o.Transport.ProgressBar = message.NewProgressBar(total, fmt.Sprintf("Publishing %s:%s", o.repo.Reference.Repository, o.repo.Reference.Reference))
 	defer o.Transport.ProgressBar.Stop()
 	// attempt to push the image manifest
-	_, err = oras.Copy(ctx, src, root.Digest.String(), o, o.Reference.Reference, copyOpts)
+	_, err = oras.Copy(ctx, src, root.Digest.String(), o.repo, o.repo.Reference.Reference, copyOpts)
 	if err != nil {
 		return err
 	}
 
-	o.Transport.ProgressBar.Successf("Published %s [%s]", o.Reference, root.MediaType)
+	o.Transport.ProgressBar.Successf("Published %s [%s]", o.repo.Reference, root.MediaType)
 	message.HorizontalRule()
-	if strings.HasSuffix(o.Reference.String(), SkeletonSuffix) {
+	if strings.HasSuffix(o.repo.Reference.String(), SkeletonSuffix) {
 		message.Title("How to import components from this skeleton:", "")
 		ex := []types.ZarfComponent{}
 		for _, c := range pkg.Components {
 			ex = append(ex, types.ZarfComponent{
 				Name: fmt.Sprintf("import-%s", c.Name),
 				Import: types.ZarfComponentImport{
 					ComponentName: c.Name,
-					URL:           fmt.Sprintf("oci://%s", o.Reference),
+					URL:           fmt.Sprintf("oci://%s", o.repo.Reference),
 				},
 			})
 		}
@@ -211,9 +202,9 @@ func (o *OrasRemote) PublishPackage(pkg *types.ZarfPackage, sourceDir string, co
 			flags = "--insecure"
 		}
 		message.Title("To inspect/deploy/pull:", "")
-		message.ZarfCommand("package inspect oci://%s %s", o.Reference, flags)
-		message.ZarfCommand("package deploy oci://%s %s", o.Reference, flags)
-		message.ZarfCommand("package pull oci://%s %s", o.Reference, flags)
+		message.ZarfCommand("package inspect oci://%s %s", o.repo.Reference, flags)
+		message.ZarfCommand("package deploy oci://%s %s", o.repo.Reference, flags)
+		message.ZarfCommand("package pull oci://%s %s", o.repo.Reference, flags)
 	}
 
 	return nil

src/pkg/oci/utils.go

@@ -55,7 +55,7 @@ func ReferenceFromMetadata(registryLocation string, metadata *types.ZarfMetadata
 // FetchRoot fetches the root manifest from the remote repository.
 func (o *OrasRemote) FetchRoot() (*ZarfOCIManifest, error) {
 	// get the manifest descriptor
-	descriptor, err := o.Resolve(o.Context, o.Reference.Reference)
+	descriptor, err := o.repo.Resolve(o.ctx, o.repo.Reference.Reference)
 	if err != nil {
 		return nil, err
 	}
@@ -88,7 +88,7 @@ func (o *OrasRemote) FetchManifest(desc ocispec.Descriptor) (manifest *ZarfOCIMa
 
 // FetchLayer fetches the layer with the given descriptor from the remote repository.
 func (o *OrasRemote) FetchLayer(desc ocispec.Descriptor) (bytes []byte, err error) {
-	return content.FetchAll(o.Context, o, desc)
+	return content.FetchAll(o.ctx, o.repo, desc)
 }
 
 // FetchZarfYAML fetches the zarf.yaml file from the remote repository.

src/pkg/packager/publish.go

@@ -66,7 +66,7 @@ func (p *Packager) Publish() error {
 		}
 	}
 
-	message.HeaderInfof("📦 PACKAGE PUBLISH %s:%s", p.cfg.Pkg.Metadata.Name, p.remote.Reference.Reference)
+	message.HeaderInfof("📦 PACKAGE PUBLISH %s:%s", p.cfg.Pkg.Metadata.Name, ref)
 
 	// Publish the package/skeleton to the registry
 	return p.remote.PublishPackage(&p.cfg.Pkg, p.tmp.Base, config.CommonOptions.OCIConcurrency)

src/test/common.go

@@ -15,8 +15,6 @@ import (
 
 	"github.com/defenseunicorns/zarf/src/pkg/utils/exec"
 	"github.com/defenseunicorns/zarf/src/pkg/utils/helpers"
-	dconfig "github.com/docker/cli/cli/config"
-	"github.com/docker/cli/cli/config/configfile"
 	"github.com/stretchr/testify/require"
 )
 
@@ -101,22 +99,11 @@ func (e2e *ZarfE2ETest) GetLogFileContents(t *testing.T, stdErr string) string {
 }
 
 // SetupDockerRegistry uses the host machine's docker daemon to spin up a local registry for testing purposes.
-func (e2e *ZarfE2ETest) SetupDockerRegistry(t *testing.T, port int) *configfile.ConfigFile {
+func (e2e *ZarfE2ETest) SetupDockerRegistry(t *testing.T, port int) {
 	// spin up a local registry
 	registryImage := "registry:2.8.2"
 	err := exec.CmdWithPrint("docker", "run", "-d", "--restart=always", "-p", fmt.Sprintf("%d:5000", port), "--name", "registry", registryImage)
 	require.NoError(t, err)
-
-	// docker config folder
-	cfg, err := dconfig.Load(dconfig.Dir())
-	require.NoError(t, err)
-	if !cfg.ContainsAuth() {
-		// make a docker config file w/ some blank creds
-		_, _, err := e2e.Zarf("tools", "registry", "login", "--username", "zarf", "-p", "zarf", "localhost:6000")
-		require.NoError(t, err)
-	}
-
-	return cfg
 }
 
 // GetZarfVersion returns the current build/zarf version

src/test/e2e/50_oci_package_test.go

@@ -122,6 +122,11 @@ func (suite *RegistryClientTestSuite) Test_3_Inspect() {
 	// Test inspect w/ bad ref.
 	_, stdErr, err = e2e.Zarf("package", "inspect", "oci://"+badRef.String(), "--insecure")
 	suite.Error(err, stdErr)
+
+	// Test inspect on a public package.
+	// NOTE: This also makes sure that Zarf does not attempt auth when inspecting a public package.
+	_, stdErr, err = e2e.Zarf("package", "inspect", "oci://ghcr.io/defenseunicorns/packages/dubbd-k3d:0.3.0-amd64")
+	suite.NoError(err, stdErr)
 }
 
 func (suite *RegistryClientTestSuite) Test_4_Pull_And_Deploy() {

src/test/e2e/52_oci_compose_differential_test.go

@@ -41,7 +41,7 @@ func (suite *OCIDifferentialSuite) SetupSuite() {
 	differentialPackageName = fmt.Sprintf("zarf-package-podinfo-with-oci-flux-%s-v0.24.0-differential-v0.25.0.tar.zst", e2e.Arch)
 	normalPackageName = fmt.Sprintf("zarf-package-podinfo-with-oci-flux-%s-v0.24.0.tar.zst", e2e.Arch)
 
-	_ = e2e.SetupDockerRegistry(suite.T(), 555)
+	e2e.SetupDockerRegistry(suite.T(), 555)
 	suite.Reference.Registry = "localhost:555"
 }