From 0f4a8baec62b0f2918986ef0a94563c6d46281b0 Mon Sep 17 00:00:00 2001 From: Adam Martin Date: Tue, 21 Jul 2026 11:55:22 -0400 Subject: [PATCH] fix for copying digest artifacts (#673) Signed-off-by: Adam Martin --- cmd/hauler/cli/store/copy.go | 30 +++++++--- cmd/hauler/cli/store/copy_test.go | 99 +++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 8 deletions(-) diff --git a/cmd/hauler/cli/store/copy.go b/cmd/hauler/cli/store/copy.go index 6f2f2d1..14b8f64 100644 --- a/cmd/hauler/cli/store/copy.go +++ b/cmd/hauler/cli/store/copy.go @@ -229,10 +229,7 @@ func CopyCmd(ctx context.Context, o *flags.CopyOpts, s *store.Layout, targetRef if ext, isSigKind := sigExts[kind]; isSigKind { if imgDigest, ok := refDigest[baseRef]; ok { digestTag := strings.ReplaceAll(imgDigest, ":", "-") - repo := baseRef - if colon := strings.LastIndex(baseRef, ":"); colon != -1 { - repo = baseRef[:colon] - } + repo := repoFromBaseRef(baseRef) destRef = repo + ":" + digestTag + ext } } else if strings.HasPrefix(kind, consts.KindAnnotationReferrers) { @@ -240,15 +237,16 @@ func CopyCmd(ctx context.Context, o *flags.CopyOpts, s *store.Layout, targetRef // the target registry wires it up via the OCI Referrers API (subject field). // For registries that don't support the Referrers API natively, the manifest // is still pushed intact... the subject linkage depends on registry support. - repo := baseRef - if colon := strings.LastIndex(baseRef, ":"); colon != -1 { - repo = baseRef[:colon] - } + repo := repoFromBaseRef(baseRef) destRef = repo + "@" + desc.Digest.String() } toRef, err := content.RewriteRefToRegistry(destRef, components[1]) if err != nil { + if !ro.IgnoreErrors { + fatalErr = fmt.Errorf("rewriting ref [%s]: %w", baseRef, err) + return nil + } l.Warnf("failed to rewrite ref [%s]: %v", baseRef, err) return nil } @@ -288,6 +286,22 @@ func CopyCmd(ctx context.Context, o *flags.CopyOpts, s *store.Layout, targetRef return nil } +// repoFromBaseRef strips any digest and/or tag from a stored ref name, yielding +// just the repository path. AnnotationRefName never contains a registry host, so +// the only colons come from a tag or the digest algorithm separator. A digest-only +// ref (myorg/myimage@sha256:) must strip the "@sha256:" suffix rather +// than the last colon, which would otherwise land inside the digest (#667). +func repoFromBaseRef(baseRef string) string { + repo := baseRef + if at := strings.Index(repo, "@"); at != -1 { + repo = repo[:at] + } + if colon := strings.LastIndex(repo, ":"); colon != -1 { + repo = repo[:colon] + } + return repo +} + // extractManifestContent extracts a manifest's layers through a mapper target // This is used for child manifests in indexes that aren't in the store's nameMap func extractManifestContent(ctx context.Context, s *store.Layout, desc ocispec.Descriptor, m ocispec.Manifest, target content.Target) error { diff --git a/cmd/hauler/cli/store/copy_test.go b/cmd/hauler/cli/store/copy_test.go index 2bb80bf..431fd75 100644 --- a/cmd/hauler/cli/store/copy_test.go +++ b/cmd/hauler/cli/store/copy_test.go @@ -12,10 +12,15 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" + digest "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/rs/zerolog" "hauler.dev/go/hauler/v2/internal/flags" v1 "hauler.dev/go/hauler/v2/pkg/apis/hauler.cattle.io/v1" + "hauler.dev/go/hauler/v2/pkg/consts" + "hauler.dev/go/hauler/v2/pkg/content" + "hauler.dev/go/hauler/v2/pkg/store" ) // -------------------------------------------------------------------------- @@ -78,6 +83,47 @@ func TestCopyCmd_UnknownProtocol(t *testing.T) { } } +// -------------------------------------------------------------------------- +// repoFromBaseRef / destination ref derivation tests (#667) +// -------------------------------------------------------------------------- + +func TestRepoFromBaseRef(t *testing.T) { + cases := map[string]string{ + "myorg/myimage@sha256:" + strings.Repeat("a", 64): "myorg/myimage", + "myorg/myimage:v1.0.2": "myorg/myimage", + "myorg/myimage": "myorg/myimage", + "nested/path/img@sha256:" + strings.Repeat("b", 64): "nested/path/img", + } + for in, want := range cases { + if got := repoFromBaseRef(in); got != want { + t.Errorf("repoFromBaseRef(%q) = %q, want %q", in, got, want) + } + } +} + +// TestDestRef_DigestOnly_Parses is a regression test for #667: destination refs +// derived for artifacts of a digest-only image must be parseable by the real +// RewriteRefToRegistry. +func TestDestRef_DigestOnly_Parses(t *testing.T) { + imgDigest := "sha256:" + strings.Repeat("a", 64) + refDigestHex := strings.Repeat("c", 64) + base := "myorg/myimage@" + imgDigest // tag@digest ingests to digest-only + + repo := repoFromBaseRef(base) + + // sig/att/sbom cosign tag + sigDest := repo + ":" + strings.ReplaceAll(imgDigest, ":", "-") + ".sig" + if _, err := content.RewriteRefToRegistry(sigDest, "target.example.com"); err != nil { + t.Errorf("sig destRef %q failed to rewrite: %v", sigDest, err) + } + + // referrer by manifest digest + refDest := repo + "@sha256:" + refDigestHex + if _, err := content.RewriteRefToRegistry(refDest, "target.example.com"); err != nil { + t.Errorf("referrer destRef %q failed to rewrite: %v", refDest, err) + } +} + // -------------------------------------------------------------------------- // Registry copy tests // -------------------------------------------------------------------------- @@ -232,6 +278,59 @@ func TestCopyCmd_Registry_IgnoreErrors(t *testing.T) { } } +// TestCopy_UndeliverableArtifact_RespectsIgnoreErrors verifies that when +// CopyCmd's registry branch derives an unparseable destination ref for an +// artifact (RewriteRefToRegistry failure), the walk fails by default and +// only swallows the error when --ignore-errors is set (#667). +// +// AnnotationRefName is validated on the way into the store's index (AddIndex +// parses it), so a malformed ref name can never reach CopyCmd's walk. The +// referrer destRef, however, is derived from the descriptor's raw Digest +// field ("@"), which is never validated as a reference. Seeding +// a referrer descriptor with a Digest containing a space reproduces the +// derivation failure without needing an actually malformed AnnotationRefName. +func TestCopy_UndeliverableArtifact_RespectsIgnoreErrors(t *testing.T) { + ctx := newTestContext(t) + + buildStore := func(t *testing.T) *store.Layout { + s := newTestStore(t) + desc := ocispec.Descriptor{ + MediaType: ocispec.MediaTypeImageManifest, + Digest: digest.Digest("sha256:not a valid digest"), + Size: 1, + Annotations: map[string]string{ + ocispec.AnnotationRefName: "myorg/myimage", + consts.ContainerdImageNameKey: "myorg/myimage", + consts.KindAnnotationName: consts.KindAnnotationReferrers + "/" + strings.Repeat("a", 64), + }, + } + if err := s.OCI.AddIndex(desc); err != nil { + t.Fatalf("AddIndex: %v", err) + } + return s + } + + dstHost, _ := newTestRegistry(t) + + t.Run("default returns error", func(t *testing.T) { + s := buildStore(t) + o := &flags.CopyOpts{StoreRootOpts: defaultRootOpts(s.Root), PlainHTTP: true} + if err := CopyCmd(ctx, o, s, "registry://"+dstHost, defaultCliOpts()); err == nil { + t.Fatal("expected error for undeliverable artifact, got nil") + } + }) + + t.Run("ignore errors returns nil", func(t *testing.T) { + s := buildStore(t) + o := &flags.CopyOpts{StoreRootOpts: defaultRootOpts(s.Root), PlainHTTP: true} + ro := defaultCliOpts() + ro.IgnoreErrors = true + if err := CopyCmd(ctx, o, s, "registry://"+dstHost, ro); err != nil { + t.Errorf("expected no error with IgnoreErrors=true, got: %v", err) + } + }) +} + // TestCopyCmd_Registry_InvalidFilenameSkipTest verifies that CopyCmd emits a // warning and skips file artifacts whose names begin with characters invalid // as OCI tag starts, rather than attempting to push them to the registry.