diff --git a/cmd/hauler/cli/store/add_test.go b/cmd/hauler/cli/store/add_test.go index 8840802..3af0060 100644 --- a/cmd/hauler/cli/store/add_test.go +++ b/cmd/hauler/cli/store/add_test.go @@ -251,6 +251,36 @@ func TestApplyDefaultRegistry(t *testing.T) { } } +func TestEncodeOriginalChartRef(t *testing.T) { + tests := []struct { + name string + repoURL string + total string + want string + }{ + { + name: "repoURL and ref joined with pipe", + repoURL: "https://charts.example.com", + total: "mychart:1.0.0", + want: "https://charts.example.com|mychart:1.0.0", + }, + { + name: "empty repoURL still joined with pipe", + repoURL: "", + total: "mychart:1.0.0", + want: "|mychart:1.0.0", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := encodeOriginalChartRef(tc.repoURL, tc.total) + if got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} + func TestRewriteReference(t *testing.T) { ctx := newTestContext(t) @@ -499,6 +529,8 @@ func TestStoreFile(t *testing.T) { t.Fatalf("storeFile: %v", err) } assertArtifactInStore(t, s, filepath.Base(tmp.Name())) + // OriginalRefAnnotation must capture the resolved absolute local path. + assertOriginalRefInStore(t, s, filepath.Base(tmp.Name()), tmp.Name()) }) t.Run("HTTP URL stored under basename", func(t *testing.T) { @@ -508,6 +540,8 @@ func TestStoreFile(t *testing.T) { t.Fatalf("storeFile: %v", err) } assertArtifactInStore(t, s, "script.sh") + // OriginalRefAnnotation must capture the original URL, not the local basename. + assertOriginalRefInStore(t, s, "script.sh", url) }) t.Run("name override changes stored ref", func(t *testing.T) { @@ -638,6 +672,9 @@ func TestStoreImage_Rewrite(t *testing.T) { t.Fatalf("storeImage with rewrite: %v", err) } assertArtifactInStore(t, s, "newrepo/img:v2") + // OriginalRefAnnotation must retain the pre-rewrite source reference, + // even though AnnotationRefName/ContainerdImageNameKey were rewritten. + assertOriginalRefInStore(t, s, "newrepo/img:v2", host+"/src/repo:v1") }) t.Run("rewrite without tag inherits source tag", func(t *testing.T) { @@ -920,6 +957,29 @@ func TestStoreChart_Rewrite(t *testing.T) { t.Fatalf("AddChartCmd with rewrite: %v", err) } assertArtifactInStore(t, s, "myorg/custom-chart") + + // OriginalRefAnnotation must retain the pre-rewrite "repoURL|repo:tag" encoding + // (see encodeOriginalChartRef) regardless of the --rewrite applied above. + wantPrefix := chartTestdataDir + "|" + found := false + if err := s.OCI.Walk(func(_ string, desc ocispec.Descriptor) error { + if strings.Contains(desc.Annotations[ocispec.AnnotationRefName], "myorg/custom-chart") { + original := desc.Annotations[consts.OriginalRefAnnotation] + if !strings.HasPrefix(original, wantPrefix) { + t.Errorf("OriginalRefAnnotation = %q, want prefix %q", original, wantPrefix) + } + if strings.Contains(original, "myorg/custom-chart") { + t.Errorf("OriginalRefAnnotation = %q must not contain the rewritten ref", original) + } + found = true + } + return nil + }); err != nil { + t.Fatalf("walk: %v", err) + } + if !found { + t.Fatal("expected to find rewritten chart artifact in store") + } } // seedChartWithImages builds a minimal Helm chart whose helm.sh/images diff --git a/cmd/hauler/cli/store/testhelpers_test.go b/cmd/hauler/cli/store/testhelpers_test.go index d73317d..2e6055d 100644 --- a/cmd/hauler/cli/store/testhelpers_test.go +++ b/cmd/hauler/cli/store/testhelpers_test.go @@ -532,6 +532,25 @@ func assertAnnotationsInStore(t *testing.T, s *store.Layout, refName, containerd } } +// assertOriginalRefInStore walks the store and fails if no descriptor has both +// AnnotationRefName containing refSubstring AND OriginalRefAnnotation == wantOriginalRef. +func assertOriginalRefInStore(t *testing.T, s *store.Layout, refSubstring, wantOriginalRef string) { + t.Helper() + found := false + if err := s.OCI.Walk(func(_ string, desc ocispec.Descriptor) error { + if strings.Contains(desc.Annotations[ocispec.AnnotationRefName], refSubstring) && + desc.Annotations[consts.OriginalRefAnnotation] == wantOriginalRef { + found = true + } + return nil + }); err != nil { + t.Fatalf("assertOriginalRefInStore walk: %v", err) + } + if !found { + t.Errorf("no artifact with ref containing %q and OriginalRefAnnotation=%q found in store", refSubstring, wantOriginalRef) + } +} + // assertReferrerInStore walks the store and fails if no descriptor has a kind // annotation with the KindAnnotationReferrers prefix and a ref containing refSubstring. func assertReferrerInStore(t *testing.T, s *store.Layout, refSubstring string) { diff --git a/pkg/store/store_test.go b/pkg/store/store_test.go index 75cd17c..80b6b57 100644 --- a/pkg/store/store_test.go +++ b/pkg/store/store_test.go @@ -787,97 +787,3 @@ func TestAddImage_OCI11Referrers(t *testing.T) { } t.Logf("captured %d OCI referrer(s) for %s", referrerCount, baseTag.Name()) } - -// newTestRegistry starts an in-process registry and returns its host and the -// remote.Option needed to talk to it over plain HTTP. -func newTestRegistry(t *testing.T) (string, []remote.Option) { - t.Helper() - srv := httptest.NewServer(registry.New()) - t.Cleanup(srv.Close) - host := strings.TrimPrefix(srv.URL, "http://") - return host, []remote.Option{remote.WithTransport(srv.Client().Transport)} -} - -// newTestStore creates a fresh OCI layout store rooted in a temp directory. -func newTestStore(t *testing.T) *store.Layout { - t.Helper() - s, err := store.NewLayout(t.TempDir()) - if err != nil { - t.Fatalf("new layout: %v", err) - } - return s -} - -// seedImage pushes a random image to host/repo:tag and returns it, so a test -// can later assert on the exact bytes/digest that were pushed. -func seedImage(t *testing.T, host, repo, tag string, opts ...remote.Option) v1.Image { - t.Helper() - img, err := random.Image(1024, 3) - if err != nil { - t.Fatalf("random.Image: %v", err) - } - ref, err := gname.NewTag(host+"/"+repo+":"+tag, gname.Insecure) - if err != nil { - t.Fatalf("new tag: %v", err) - } - if err := remote.Write(ref, img, opts...); err != nil { - t.Fatalf("remote.Write: %v", err) - } - return img -} - -// TestAddImagePinnedDigestIgnoresMovedTag proves the TOCTOU fix: once a caller -// pins the digest it verified, a tag that moves to different content between -// verification and the fetch cannot substitute its bytes into the store. -func TestAddImagePinnedDigestIgnoresMovedTag(t *testing.T) { - host, remoteOpts := newTestRegistry(t) - original := seedImage(t, host, "test/pinned", "v1", remoteOpts...) - originalDigest, err := original.Digest() - if err != nil { - t.Fatalf("original digest: %v", err) - } - - // Move the tag to different content, exactly as a mutable tag could be - // re-pushed between verification and the pull. - replacement, err := random.Image(1024, 3) - if err != nil { - t.Fatalf("random.Image: %v", err) - } - ref, err := gname.ParseReference(host + "/test/pinned:v1") - if err != nil { - t.Fatalf("parse: %v", err) - } - if err := remote.Write(ref, replacement, remoteOpts...); err != nil { - t.Fatalf("remote.Write replacement: %v", err) - } - - s := newTestStore(t) - got, err := s.AddImage(context.Background(), host+"/test/pinned:v1", "", true, - originalDigest.String(), false, "", remoteOpts...) - if err != nil { - t.Fatalf("AddImage: %v", err) - } - if got != originalDigest.String() { - t.Fatalf("stored digest = %s, want the pinned %s (the moved tag won)", got, originalDigest.String()) - } -} - -// TestAddImageEmptyPinResolvesTag confirms the unpinned path is untouched: -// an empty pinnedDigest still resolves the tag normally. -func TestAddImageEmptyPinResolvesTag(t *testing.T) { - host, remoteOpts := newTestRegistry(t) - img := seedImage(t, host, "test/unpinned", "v1", remoteOpts...) - want, err := img.Digest() - if err != nil { - t.Fatalf("digest: %v", err) - } - - s := newTestStore(t) - got, err := s.AddImage(context.Background(), host+"/test/unpinned:v1", "", true, "", false, "", remoteOpts...) - if err != nil { - t.Fatalf("AddImage: %v", err) - } - if got != want.String() { - t.Fatalf("stored digest = %s, want %s", got, want.String()) - } -}