From ee44a6adec0066b8f23139bc02dcf5dd2a0234a9 Mon Sep 17 00:00:00 2001 From: Tomasz Knura Date: Fri, 10 Jul 2026 10:20:20 +0200 Subject: [PATCH] fix(materials): do not record container image digest as tag Signed-off-by: Tomasz Knura --- .../crafter/materials/oci_image.go | 18 ++- .../crafter/materials/oci_image_test.go | 119 ++++++++++++++++++ .../oci-layouts/containerd/index.json | 31 +++++ .../oci-layouts/containerd/oci-layout | 1 + 4 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 pkg/attestation/crafter/materials/testdata/oci-layouts/containerd/index.json create mode 100644 pkg/attestation/crafter/materials/testdata/oci-layouts/containerd/oci-layout diff --git a/pkg/attestation/crafter/materials/oci_image.go b/pkg/attestation/crafter/materials/oci_image.go index dc61f9e75..7548800df 100644 --- a/pkg/attestation/crafter/materials/oci_image.go +++ b/pkg/attestation/crafter/materials/oci_image.go @@ -129,13 +129,19 @@ func (i *OCIImageCrafter) Craft(ctx context.Context, imageRef string) (*api.Atte // Check if the image is tagged as "latest" hasLatestTag := i.isLatestTag(ref, remoteRef.DigestStr()) + // Digest references carry no tag; ref.Identifier() would return the digest itself + var tag string + if t, ok := ref.(name.Tag); ok { + tag = t.TagStr() + } + containerImage := &api.Attestation_Material_ContainerImage{ // TODO: remove once we know servers are not running server-side validation Id: i.input.Name, Name: repoName, Digest: remoteRef.DigestStr(), IsSubject: i.input.Output, - Tag: ref.Identifier(), + Tag: tag, HasLatestTag: wrapperspb.Bool(hasLatestTag), } @@ -414,10 +420,12 @@ func (i *OCIImageCrafter) buildMaterialFromManifest(layoutPath string, manifest tag := "" if manifest.Annotations != nil { if t, ok := manifest.Annotations["io.containerd.image.name"]; ok { - // Extract tag from full reference (e.g., "registry/repo:tag" -> "tag") - parts := strings.Split(t, ":") - if len(parts) > 1 { - tag = parts[len(parts)-1] + // Parse the full reference (e.g. "registry:5000/repo:tag"); naive splitting on ":" + // would mistake registry ports or digests for tags + if parsed, err := name.ParseReference(t); err == nil { + if tagged, ok := parsed.(name.Tag); ok { + tag = tagged.TagStr() + } } } } diff --git a/pkg/attestation/crafter/materials/oci_image_test.go b/pkg/attestation/crafter/materials/oci_image_test.go index cef29b411..bc0660eae 100644 --- a/pkg/attestation/crafter/materials/oci_image_test.go +++ b/pkg/attestation/crafter/materials/oci_image_test.go @@ -17,10 +17,20 @@ package materials_test import ( "context" + "fmt" + "io" + "log" + "net/http/httptest" + "net/url" "testing" contractAPI "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials" + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -215,3 +225,112 @@ func TestOCIImageCraft_LayoutWithDigestSelector(t *testing.T) { }) } } + +func TestOCIImageCraft_RemoteTag(t *testing.T) { + server := httptest.NewServer(registry.New(registry.Logger(log.New(io.Discard, "", 0)))) + defer server.Close() + + u, err := url.Parse(server.URL) + require.NoError(t, err) + repo := fmt.Sprintf("%s/test/image", u.Host) + + for _, tag := range []string{"v1.0.0", "latest"} { + ref, err := name.ParseReference(fmt.Sprintf("%s:%s", repo, tag)) + require.NoError(t, err) + require.NoError(t, remote.Write(ref, empty.Image)) + } + + taggedRef, err := name.ParseReference(fmt.Sprintf("%s:v1.0.0", repo)) + require.NoError(t, err) + desc, err := remote.Get(taggedRef) + require.NoError(t, err) + digest := desc.Digest.String() + + testCases := []struct { + name string + imageRef string + wantTag string + }{ + { + name: "tagged reference", + imageRef: fmt.Sprintf("%s:v1.0.0", repo), + wantTag: "v1.0.0", + }, + { + // a digest reference carries no tag, the digest must not leak into the tag field + name: "digest reference", + imageRef: fmt.Sprintf("%s@%s", repo, digest), + wantTag: "", + }, + { + name: "untagged reference defaults to latest", + imageRef: repo, + wantTag: "latest", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + schema := &contractAPI.CraftingSchema_Material{ + Name: "test", + Type: contractAPI.CraftingSchema_Material_CONTAINER_IMAGE, + } + l := zerolog.Nop() + crafter, err := materials.NewOCIImageCrafter(schema, authn.DefaultKeychain, &l) + require.NoError(t, err) + + got, err := crafter.Craft(context.TODO(), tc.imageRef) + require.NoError(t, err) + + containerImage := got.GetContainerImage() + require.NotNil(t, containerImage) + assert.Equal(t, tc.wantTag, containerImage.Tag) + assert.Equal(t, digest, containerImage.Digest) + }) + } +} + +func TestOCIImageCraft_LayoutTagExtraction(t *testing.T) { + testCases := []struct { + name string + digestSelector string + wantTag string + }{ + { + name: "tagged containerd reference", + digestSelector: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + wantTag: "v2.0.0", + }, + { + // a digest reference carries no tag, the digest must not leak into the tag field + name: "digest containerd reference", + digestSelector: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + wantTag: "", + }, + { + // the registry port must not be mistaken for a tag + name: "untagged reference on a registry with port", + digestSelector: "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + wantTag: "latest", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + schema := &contractAPI.CraftingSchema_Material{ + Name: "test", + Type: contractAPI.CraftingSchema_Material_CONTAINER_IMAGE, + } + l := zerolog.Nop() + crafter, err := materials.NewOCIImageCrafter(schema, nil, &l) + require.NoError(t, err) + + got, err := crafter.Craft(context.TODO(), "testdata/oci-layouts/containerd@"+tc.digestSelector) + require.NoError(t, err) + + containerImage := got.GetContainerImage() + require.NotNil(t, containerImage) + assert.Equal(t, tc.wantTag, containerImage.Tag) + }) + } +} diff --git a/pkg/attestation/crafter/materials/testdata/oci-layouts/containerd/index.json b/pkg/attestation/crafter/materials/testdata/oci-layouts/containerd/index.json new file mode 100644 index 000000000..40b44f310 --- /dev/null +++ b/pkg/attestation/crafter/materials/testdata/oci-layouts/containerd/index.json @@ -0,0 +1,31 @@ +{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.index.v1+json", + "manifests": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "size": 743, + "annotations": { + "io.containerd.image.name": "ghcr.io/org/image:v2.0.0", + "org.opencontainers.image.ref.name": "v2.0.0" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "size": 743, + "annotations": { + "io.containerd.image.name": "ghcr.io/org/image@sha256:3ac173d9b28419abf3ac44a6bf26ed1959da2bdf7ab29c634a3944e9f0f7d34" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + "size": 743, + "annotations": { + "io.containerd.image.name": "localhost:5000/org/image" + } + } + ] +} diff --git a/pkg/attestation/crafter/materials/testdata/oci-layouts/containerd/oci-layout b/pkg/attestation/crafter/materials/testdata/oci-layouts/containerd/oci-layout new file mode 100644 index 000000000..d5e10fd65 --- /dev/null +++ b/pkg/attestation/crafter/materials/testdata/oci-layouts/containerd/oci-layout @@ -0,0 +1 @@ +{"imageLayoutVersion":"1.0.0"}