Repository: crazy-max/diun
Pull request #968 (2023-09-16 16:42:21)

handle analysis of image with tag and digest
follow-up #915 


Patch:
diff --git a/docs/faq.md b/docs/faq.md
index ae887ab5..1f54e98b 100644
--- a/docs/faq.md
+++ b/docs/faq.md
@@ -333,3 +333,17 @@ Following profilers are available:
 * `mutex` enables mutex profiling
 * `threads` enables thread creation profiling
 * `block` enables block (contention) profiling
+
+## Image with digest and `image:tag@digest` format
+
+Analysis of an image with a digest but without tag will be done using `latest`
+as a tag which could lead to false positives.
+
+For example `crazymax/diun@sha256:fa80af32a7c61128ffda667344547805b3c5e7721ecbbafd70e35bb7bb7c989f`
+is referring to `crazymax/diun:4.24.0` tag, so it's not correct to assume that
+we want to analyze `crazymax/diun:latest`.
+
+You can still pin an image to a specific digest and analyze the image if the
+tag is specified using the `image:tag@digest` format. Taking the previous
+example if we specify `crazymax/diun:4.24.0@sha256:fa80af32a7c61128ffda667344547805b3c5e7721ecbbafd70e35bb7bb7c989f`,
+then `crazymax/diun:4.24.0` will be analyzed.
diff --git a/internal/app/job.go b/internal/app/job.go
index 956a9b03..f186b17d 100644
--- a/internal/app/job.go
+++ b/internal/app/job.go
@@ -22,7 +22,7 @@ func (di *Diun) createJob(job model.Job) {
 		Str("image", job.Image.Name).
 		Logger()
 
-	// Validate image
+	// Parse image
 	prvImage, err = registry.ParseImage(registry.ParseImageOptions{
 		Name:   job.Image.Name,
 		HubTpl: job.Image.HubTpl,
diff --git a/internal/provider/common.go b/internal/provider/common.go
index 31687061..27830b4d 100644
--- a/internal/provider/common.go
+++ b/internal/provider/common.go
@@ -19,10 +19,6 @@ var (
 
 // ValidateImage returns a standard image through Docker labels
 func ValidateImage(image string, metadata, labels map[string]string, watchByDef bool, imageDefaults model.Image) (img model.Image, err error) {
-	if i := strings.Index(image, "@sha256:"); i > 0 {
-		image = image[:i]
-	}
-
 	img = model.Image{
 		Name: image,
 	}
diff --git a/pkg/registry/manifest.go b/pkg/registry/manifest.go
index 4c6b1b8c..a8314e20 100644
--- a/pkg/registry/manifest.go
+++ b/pkg/registry/manifest.go
@@ -35,15 +35,10 @@ func (c *Client) Manifest(image Image, dbManifest Manifest) (Manifest, bool, err
 		return Manifest{}, false, errors.Wrap(err, "cannot parse reference")
 	}
 
-	// Retrieve remote digest through HEAD request or get one from image reference
-	var rmDigest digest.Digest
-	if len(image.Digest) > 0 {
-		rmDigest = image.Digest
-	} else {
-		rmDigest, err = docker.GetDigest(ctx, c.sysCtx, rmRef)
-		if err != nil {
-			return Manifest{}, false, errors.Wrap(err, "cannot get image digest from HEAD request")
-		}
+	// Retrieve remote digest through HEAD request
+	rmDigest, err := docker.GetDigest(ctx, c.sysCtx, rmRef)
+	if err != nil {
+		return Manifest{}, false, errors.Wrap(err, "cannot get image digest from HEAD request")
 	}
 
 	// Digest match, returns db manifest
diff --git a/pkg/registry/ref.go b/pkg/registry/ref.go
index 00883bdc..194ecb68 100644
--- a/pkg/registry/ref.go
+++ b/pkg/registry/ref.go
@@ -39,31 +39,28 @@ func namedReference(name string) (reference.Named, error) {
 		if err != nil {
 			return nil, errors.Wrapf(err, "normalizing tagged digested name %q", name)
 		}
-		return ref, nil
-	}
-	if _, hasDigest := ref.(reference.Digested); hasDigest {
-		return ref, nil
+	} else if _, hasDigest := ref.(reference.Digested); hasDigest {
+		ref = reference.TrimNamed(ref)
 	}
 
 	return reference.TagNameOnly(ref), nil
 }
 
-// normalizeTaggedDigestedNamed strips the tag off the specified named
-// reference if it is tagged and digested. Note that the tag is entirely
-// ignored.
+// normalizeTaggedDigestedNamed strips the digest off the specified named
+// reference if it is tagged and digested.
 func normalizeTaggedDigestedNamed(named reference.Named) (reference.Named, error) {
-	_, isTagged := named.(reference.NamedTagged)
-	if !isTagged {
+	_, isDigested := named.(reference.Digested)
+	if !isDigested {
 		return named, nil
 	}
-	digested, isDigested := named.(reference.Digested)
-	if !isDigested {
+	tag, isTagged := named.(reference.NamedTagged)
+	if !isTagged {
 		return named, nil
 	}
-	// strip off the tag
+	// strip off the tag and digest
 	newNamed := reference.TrimNamed(named)
-	// re-add the digest
-	newNamed, err := reference.WithDigest(newNamed, digested.Digest())
+	// re-add the tag
+	newNamed, err := reference.WithTag(newNamed, tag.Tag())
 	if err != nil {
 		return named, err
 	}


Test patch:
diff --git a/internal/provider/common_test.go b/internal/provider/common_test.go
index d6d145e6..abaf59f7 100644
--- a/internal/provider/common_test.go
+++ b/internal/provider/common_test.go
@@ -21,11 +21,11 @@ func TestValidateImage(t *testing.T) {
 		expectedErr   interface{}
 	}{
 		{
-			name:       "Test strip digest",
+			name:       "Test with digest",
 			image:      "myimg@sha256:1234567890abcdef",
 			watchByDef: true,
 			expectedImage: model.Image{
-				Name: "myimg",
+				Name: "myimg@sha256:1234567890abcdef",
 			},
 			expectedErr: nil,
 		},
diff --git a/pkg/registry/manifest_test.go b/pkg/registry/manifest_test.go
index 180fbf54..06107763 100644
--- a/pkg/registry/manifest_test.go
+++ b/pkg/registry/manifest_test.go
@@ -339,7 +339,7 @@ func TestManifestTaggedDigest(t *testing.T) {
 	assert.Equal(t, "linux/amd64", manifest.Platform)
 }
 
-func TestManifestTaggedDigestDummyTag(t *testing.T) {
+func TestManifestTaggedDigestUnknownTag(t *testing.T) {
 	rc, err := New(Options{
 		CompareDigest: true,
 		ImageOs:       "linux",
@@ -356,19 +356,8 @@ func TestManifestTaggedDigestDummyTag(t *testing.T) {
 		t.Error(err)
 	}
 
-	// download manifest
 	_, _, err = rc.Manifest(img, Manifest{})
-	assert.NoError(t, err)
-
-	// check manifest
-	manifest, updated, err := rc.Manifest(img, manifestCrazymaxDiun4250)
-	assert.NoError(t, err)
-	assert.Equal(t, false, updated)
-	assert.Equal(t, "docker.io/crazymax/diun", manifest.Name)
-	assert.Equal(t, "latest", manifest.Tag)
-	assert.Equal(t, "application/vnd.oci.image.index.v1+json", manifest.MIMEType)
-	assert.Equal(t, "sha256:3fca3dd86c2710586208b0f92d1ec4ce25382f4cad4ae76a2275db8e8bb24031", manifest.Digest.String())
-	assert.Equal(t, "linux/amd64", manifest.Platform)
+	assert.Error(t, err)
 }
 
 var manifestCrazymaxDiun4250 = Manifest{
diff --git a/pkg/registry/ref_test.go b/pkg/registry/ref_test.go
index f70942b9..6588b8cd 100644
--- a/pkg/registry/ref_test.go
+++ b/pkg/registry/ref_test.go
@@ -36,11 +36,15 @@ func TestImageReference(t *testing.T) {
 		},
 		{
 			input:    "busybox" + sha256digest,
-			expected: "docker.io/library/busybox" + sha256digest,
+			expected: "docker.io/library/busybox:latest",
 		},
 		{
 			input:    "busybox:latest" + sha256digest,
-			expected: "docker.io/library/busybox" + sha256digest,
+			expected: "docker.io/library/busybox:latest",
+		},
+		{
+			input:    "busybox:v1.0.0" + sha256digest,
+			expected: "docker.io/library/busybox:v1.0.0",
 		},
 		{
 			input:    "UPPERCASEISINVALID",
diff --git a/test/dockerfile2/mount/Dockerfile b/test/dockerfile2/mount/Dockerfile
index 0dadae18..ac66e9d4 100644
--- a/test/dockerfile2/mount/Dockerfile
+++ b/test/dockerfile2/mount/Dockerfile
@@ -6,7 +6,7 @@ FROM alpine:latest
 # diun.platform=linux/amd64
 # diun.metadata.foo=bar
 RUN --mount=type=bind,target=.,rw \
-  --mount=type=bind,from=crazymax/undock:0.5.0@sha256:736fdfde1268b93c2f733c53a7c45ece24e275318628fbb790bee7f89459961f,source=/usr/local/bin/undock,target=/usr/local/bin/undock \
+  --mount=type=bind,from=crazymax/undock@sha256:736fdfde1268b93c2f733c53a7c45ece24e275318628fbb790bee7f89459961f,source=/usr/local/bin/undock,target=/usr/local/bin/undock \
   undock --version
 
 # diun.platform=linux/amd64