test/integration: make every pull client actually read a blob

Two of the three matrix clients never fetched one. orasClient.Pull stopped at
repo.Resolve (a HEAD on the manifest) and regclient's stopped at ManifestHead,
so their pull rows — including anonymous_pull_denied, stranger_pull and
crew_read_only_pull on the private-hold matrix — were passing without ever
exercising blob authorization. They passed on the manifest denial alone.

That is invisible from the outside because it produces the right verdicts for
the wrong reason. The full suite is still green after the fix, so no
authorization bug was hiding behind it; what was hiding was the coverage.

craneClient.Pull was already correct: 5aa13ab added its layer-materialization
loop precisely because crane is lazy and ATCR serves manifests from the user's
PDS, where they are world-readable by design. The oras comment still carried
the pre-5aa13ab rationale — "We don't need to fetch blobs; that mirrors
crane.Pull followed by .Digest(), which is also manifest-only" — which that
commit had already invalidated. Both clients now match crane, and the stale
comment is gone.

TestPullClientsReadBlobs guards all three against regressing to manifest-only.
It runs each client against a registry that serves the manifest happily and
403s the layer, and asserts both that a blob was requested and that the refusal
surfaces as an error. Written first and run before the fix, where it passed for
crane and failed for oras and regclient — which is how the gap was found.

This also closes the hole the plan flagged for craneClient.Pull alone: nothing
protected that loop, and deleting it left the entire suite green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SeaUS5AFPX9gqCahoLRMRh

test/integration/clients.go

@@ -4,6 +4,7 @@ package integration
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -22,6 +23,7 @@ import (
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/regclient/regclient"
 	"github.com/regclient/regclient/config"
+	"github.com/regclient/regclient/types/manifest"
 	"github.com/regclient/regclient/types/ref"
 	"oras.land/oras-go/v2"
 	orasoci "oras.land/oras-go/v2/content/oci"
@@ -195,12 +197,19 @@ func (orasClient) Pull(ctx context.Context, ref string, a testharness.Auth) (v1.
 	}
 	// Resolve performs HEAD /v2/<name>/manifests/<tag> and returns the
 	// manifest descriptor — its Digest matches ggcr's img.Digest() for the
-	// same content. We don't need to fetch blobs; that mirrors crane.Pull
-	// followed by .Digest(), which is also manifest-only.
+	// same content.
 	desc, err := repo.Resolve(ctx, tag.TagStr())
 	if err != nil {
 		return v1.Hash{}, normalizeErr(err)
 	}
+	// Then read the layer bytes, for the same reason craneClient.Pull does:
+	// resolving the manifest alone never asks the registry for a blob, and
+	// ATCR serves manifests from the user's PDS where they are world-readable.
+	// A manifest-only "pull" cannot fail an authorization check on blobs, so
+	// every pull row in the matrix would pass without testing what it claims.
+	if err := orasReadLayers(ctx, repo, desc); err != nil {
+		return v1.Hash{}, normalizeErr(err)
+	}
 	return v1.Hash{Algorithm: desc.Digest.Algorithm().String(), Hex: desc.Digest.Encoded()}, nil
 }
 
@@ -240,6 +249,44 @@ func (orasClient) PushIndex(ctx context.Context, t *testing.T, refStr string, id
 	return nil
 }
 
+// orasReadLayers fetches every layer blob named by a manifest and discards the
+// bytes. Pulling is the only matrix operation that can exercise blob
+// authorization, and it only does so if something actually reads a blob.
+func orasReadLayers(ctx context.Context, repo *orasremote.Repository, desc ocispec.Descriptor) error {
+	rc, err := repo.Manifests().Fetch(ctx, desc)
+	if err != nil {
+		return err
+	}
+	body, err := io.ReadAll(rc)
+	if closeErr := rc.Close(); err == nil {
+		err = closeErr
+	}
+	if err != nil {
+		return err
+	}
+
+	var m ocispec.Manifest
+	if err := json.Unmarshal(body, &m); err != nil {
+		// An index rather than an image manifest: PullIndex covers that shape.
+		return nil //nolint:nilerr
+	}
+	for _, layer := range m.Layers {
+		lr, err := repo.Blobs().Fetch(ctx, layer)
+		if err != nil {
+			return err
+		}
+		_, copyErr := io.Copy(io.Discard, lr)
+		closeErr := lr.Close()
+		if copyErr != nil {
+			return copyErr
+		}
+		if closeErr != nil {
+			return closeErr
+		}
+	}
+	return nil
+}
+
 func (orasClient) PullIndex(ctx context.Context, refStr string, a testharness.Auth) (v1.Hash, error) {
 	// Same head-only resolve as Pull — manifest digest is the same value for
 	// images and indexes (it's just the manifest body's sha256).
@@ -337,13 +384,34 @@ func (regclientClient) Pull(ctx context.Context, refStr string, a testharness.Au
 	rc := newRegclient(parsed.Context().RegistryStr(), a)
 	defer rc.Close(ctx, r)
 
-	// ManifestHead matches crane's lazy-pull semantics: HEAD /manifests/<tag>
-	// returns the descriptor with the registry-computed digest.
-	mh, err := rc.ManifestHead(ctx, r)
+	// ManifestGet rather than ManifestHead: the head gives the digest, but the
+	// body is what names the layers, and the layers are the only part of a
+	// pull that exercises blob authorization. See craneClient.Pull.
+	m, err := rc.ManifestGet(ctx, r)
 	if err != nil {
 		return v1.Hash{}, normalizeErr(err)
 	}
-	desc := mh.GetDescriptor()
+	desc := m.GetDescriptor()
+	if imager, ok := m.(manifest.Imager); ok {
+		layers, err := imager.GetLayers()
+		if err != nil {
+			return v1.Hash{}, normalizeErr(err)
+		}
+		for _, l := range layers {
+			rdr, err := rc.BlobGet(ctx, r, l)
+			if err != nil {
+				return v1.Hash{}, normalizeErr(err)
+			}
+			_, copyErr := io.Copy(io.Discard, rdr)
+			closeErr := rdr.Close()
+			if copyErr != nil {
+				return v1.Hash{}, normalizeErr(copyErr)
+			}
+			if closeErr != nil {
+				return v1.Hash{}, normalizeErr(closeErr)
+			}
+		}
+	}
 	return v1.Hash{Algorithm: desc.Digest.Algorithm().String(), Hex: desc.Digest.Encoded()}, nil
 }
 

test/integration/clients_guard_test.go

@@ -0,0 +1,121 @@
+//go:build integration
+
+package integration
+
+import (
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync/atomic"
+	"testing"
+
+	"atcr.io/internal/testharness"
+)
+
+// TestPullClientsReadBlobs guards the guard.
+//
+// craneClient.Pull deliberately materializes the layer bytes after crane.Pull,
+// because crane is lazy: crane.Pull plus img.Digest() fetches only the
+// manifest, and ATCR serves manifests from the user's PDS where they are
+// world-readable by design. A "pull" that stops at the manifest never asks the
+// hold for a blob, so it never exercises blob authorization at all.
+//
+// That loop is the only reason the pull rows of the auth matrix test anything
+// about who may read blobs — and nothing was protecting it. Deleting it leaves
+// the entire integration suite green, including TestAuthMatrixPrivateHold,
+// which still passes because the manifest is refused too. The blob half simply
+// stops being covered, silently.
+//
+// This asserts the property directly, against a registry that serves the
+// manifest happily and refuses the blob. A client that never reads blobs
+// returns success here and fails this test.
+func TestPullClientsReadBlobs(t *testing.T) {
+	for _, c := range Clients {
+		t.Run(c.Name(), func(t *testing.T) {
+			var blobRequested atomic.Bool
+
+			// Minimal registry: /v2/ ping, a real manifest, and a blob endpoint
+			// that always refuses. The config blob is served so nothing fails
+			// for an unrelated reason — only the layer is forbidden.
+			layerBody := []byte("layer-bytes-that-must-be-fetched")
+			layerDigest := digestOf(layerBody)
+			configBody := []byte(`{"architecture":"amd64","os":"linux","rootfs":{"type":"layers","diff_ids":[]},"config":{}}`)
+			configDigest := digestOf(configBody)
+
+			manifest := map[string]any{
+				"schemaVersion": 2,
+				"mediaType":     "application/vnd.oci.image.manifest.v1+json",
+				"config": map[string]any{
+					"mediaType": "application/vnd.oci.image.config.v1+json",
+					"digest":    configDigest,
+					"size":      len(configBody),
+				},
+				"layers": []map[string]any{{
+					"mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
+					"digest":    layerDigest,
+					"size":      len(layerBody),
+				}},
+			}
+			manifestJSON, err := json.Marshal(manifest)
+			if err != nil {
+				t.Fatalf("marshal manifest: %v", err)
+			}
+
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				switch {
+				case r.URL.Path == "/v2/":
+					w.WriteHeader(http.StatusOK)
+
+				case strings.Contains(r.URL.Path, "/manifests/"):
+					w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json")
+					w.Header().Set("Docker-Content-Digest", digestOf(manifestJSON))
+					if r.Method == http.MethodHead {
+						w.Header().Set("Content-Length", fmt.Sprint(len(manifestJSON)))
+						w.WriteHeader(http.StatusOK)
+						return
+					}
+					_, _ = w.Write(manifestJSON)
+
+				case strings.HasSuffix(r.URL.Path, "/blobs/"+configDigest):
+					_, _ = w.Write(configBody)
+
+				case strings.Contains(r.URL.Path, "/blobs/"):
+					// The layer, and only the layer, is refused. This is the
+					// hold saying no to an unauthorized reader.
+					blobRequested.Store(true)
+					w.WriteHeader(http.StatusForbidden)
+					_, _ = w.Write([]byte(`{"errors":[{"code":"DENIED","message":"blob read denied"}]}`))
+
+				default:
+					w.WriteHeader(http.StatusNotFound)
+				}
+			}))
+			defer srv.Close()
+
+			ref := strings.TrimPrefix(srv.URL, "http://") + "/alice.test/guard:v1"
+			_, err = c.Pull(context.Background(), ref, testharness.Auth{})
+
+			t.Logf("%s.Pull returned err=%v (blobRequested=%v)", c.Name(), err, blobRequested.Load())
+
+			if !blobRequested.Load() {
+				t.Fatalf("%s.Pull never requested a layer blob — it stops at the manifest, "+
+					"so every pull row in the auth matrix is passing without exercising blob "+
+					"authorization. Restore the layer materialization in clients.go.", c.Name())
+			}
+			if err == nil {
+				t.Errorf("%s.Pull succeeded against a registry that refuses the layer blob; "+
+					"the refusal must surface as an error", c.Name())
+			}
+		})
+	}
+}
+
+func digestOf(b []byte) string {
+	sum := sha256.Sum256(b)
+	return "sha256:" + hex.EncodeToString(sum[:])
+}