fix: resolve layer URLs at fetch time

cmd/clair-action/report.go

@@ -133,13 +133,16 @@ func report(c *cli.Context) error {
 	)
 	switch {
 	case imgRef != "":
-		cl := http.DefaultClient
-		fa = libindex.NewRemoteFetchArena(cl, os.TempDir())
 		err := os.Setenv("DOCKER_CONFIG", dockerConfigDir)
 		if err != nil {
 			return fmt.Errorf("error setting DOCKER_CONFIG env var")
 		}
-		mf, err = image.ManifestFromRemote(ctx, imgRef)
+		cl, err := image.NewRegistryClient(ctx, imgRef)
+		if err != nil {
+			return fmt.Errorf("error creating registry client: %v", err)
+		}
+		fa = libindex.NewRemoteFetchArena(cl, os.TempDir())
+		mf, err = image.ManifestFromRemote(ctx, cl, imgRef)
 		if err != nil {
 			return fmt.Errorf("error getting image information: %v", err)
 		}

image/manifest.go

@@ -1,5 +1,3 @@
-// This is lifted from Clairctl
-
 package image
 
 import (
@@ -15,7 +13,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"strings"
 
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
@@ -52,17 +49,24 @@ func rt(ctx context.Context, ref string) (http.RoundTripper, error) {
 	return rt, nil
 }
 
-func ManifestFromRemote(ctx context.Context, r string) (*claircore.Manifest, error) {
-	rt, err := rt(ctx, r)
+// NewRegistryClient returns an [http.Client] that is able to authenticate
+// against authenticates against the registry hosting ref, refreshing
+// credentials as needed. The Authorization header is restricted to the
+// registry host, i.e., it won't be sent to blob storage.
+func NewRegistryClient(ctx context.Context, ref string) (*http.Client, error) {
+	rt, err := rt(ctx, ref)
 	if err != nil {
 		return nil, err
 	}
+	return &http.Client{Transport: rt}, nil
+}
 
+func ManifestFromRemote(ctx context.Context, cl *http.Client, r string) (*claircore.Manifest, error) {
 	ref, err := name.ParseReference(r)
 	if err != nil {
 		return nil, err
 	}
-	desc, err := remote.Get(ref, remote.WithTransport(rt))
+	desc, err := remote.Get(ref, remote.WithTransport(cl.Transport))
 	if err != nil {
 		return nil, err
 	}
@@ -98,10 +102,11 @@ func ManifestFromRemote(ctx context.Context, r string) (*claircore.Manifest, err
 		Scheme: repo.Scheme(),
 		Host:   repo.RegistryStr(),
 	}
-	c := http.Client{
-		Transport: rt,
-	}
 
+	// Layer URIs point at the registry's stable blob endpoints. Any
+	// redirect to short-lived presigned storage URLs is followed at fetch
+	// time, inside the fetcher's GET, so the URLs cannot expire between
+	// manifest construction and layer download (quay/clair-action#275).
 	for _, l := range ls {
 		d, err := l.Digest()
 		if err != nil {
@@ -111,27 +116,13 @@ func ManifestFromRemote(ctx context.Context, r string) (*claircore.Manifest, err
 		if err != nil {
 			return nil, err
 		}
-		u, err := rURL.Parse(path.Join("/", "v2", strings.TrimPrefix(repo.RepositoryStr(), repo.RegistryStr()), "blobs", d.String()))
-		if err != nil {
-			return nil, err
-		}
-		req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
+		u, err := rURL.Parse(path.Join("/", "v2", repo.RepositoryStr(), "blobs", d.String()))
 		if err != nil {
 			return nil, err
 		}
-		req.Header.Add("Range", "bytes=0-0")
-		res, err := c.Do(req)
-		if err != nil {
-			return nil, err
-		}
-		res.Body.Close()
-
-		res.Request.Header.Del("User-Agent")
-		res.Request.Header.Del("Range")
 		out.Layers = append(out.Layers, &claircore.Layer{
-			Hash:    ccd,
-			URI:     res.Request.URL.String(),
-			Headers: res.Request.Header,
+			Hash: ccd,
+			URI:  u.String(),
 		})
 	}
 

image/manifest_test.go

@@ -6,11 +6,19 @@ import (
 	"context"
 	"encoding/base64"
 	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/google/go-containerregistry/pkg/registry"
+	"github.com/google/go-containerregistry/pkg/v1/random"
+	"github.com/google/go-containerregistry/pkg/v1/remote"
+	"github.com/quay/claircore/libindex"
 	"golang.org/x/tools/txtar"
 )
 
@@ -95,6 +103,13 @@ func TestLocalManifest(t *testing.T) {
 			if err != nil {
 				t.Fatalf("InspectLocal error: %v", err)
 			}
+			defer func() {
+				for _, l := range m.Layers {
+					if l != nil {
+						l.Close()
+					}
+				}
+			}()
 			if m.Hash.String() != tt.wantManifestHash {
 				t.Fatalf("manifest hash = %s, want %s", m.Hash.String(), tt.wantManifestHash)
 			}
@@ -114,3 +129,128 @@ func TestLocalManifest(t *testing.T) {
 		})
 	}
 }
+
+func TestRemoteManifestStableURIs(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+
+	srv := httptest.NewServer(registry.New())
+	defer srv.Close()
+	u, err := url.Parse(srv.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	img, err := random.Image(1024, 3)
+	if err != nil {
+		t.Fatal(err)
+	}
+	refStr := u.Host + "/test/repo:latest"
+	ref, err := name.ParseReference(refStr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := remote.Write(ref, img); err != nil {
+		t.Fatal(err)
+	}
+
+	cl, err := NewRegistryClient(ctx, refStr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	mf, err := ManifestFromRemote(ctx, cl, refStr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(mf.Layers) != 3 {
+		t.Fatalf("got %d layers, want 3", len(mf.Layers))
+	}
+	for _, l := range mf.Layers {
+		lu, err := url.Parse(l.URI)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if lu.Host != u.Host {
+			t.Errorf("layer URI host = %q, want %q", lu.Host, u.Host)
+		}
+		wantPath := "/v2/test/repo/blobs/" + l.Hash.String()
+		if lu.Path != wantPath {
+			t.Errorf("layer URI path = %q, want %q", lu.Path, wantPath)
+		}
+		if lu.RawQuery != "" {
+			t.Errorf("layer URI has query %q, want none", lu.RawQuery)
+		}
+		if len(l.Headers) != 0 {
+			t.Errorf("layer has captured headers: %v", l.Headers)
+		}
+	}
+}
+
+// TestRealizeThroughRedirect exercises the registry 302s blob GETs to a
+// separate "object storage" server, mimicking registries backed by S3-style
+// storage. Layer URLs must be resolved at fetch time, and registry
+// credentials must not follow the redirect.
+// See also: quay/clair-action#275
+func TestRealizeThroughRedirect(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+
+	reg := registry.New()
+
+	storage := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("Authorization") != "" {
+			http.Error(w, "unexpected Authorization header", http.StatusBadRequest)
+			return
+		}
+		reg.ServeHTTP(w, r)
+	}))
+	defer storage.Close()
+
+	regSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/blobs/sha256:") {
+			http.Redirect(w, r, storage.URL+r.URL.Path, http.StatusTemporaryRedirect)
+			return
+		}
+		reg.ServeHTTP(w, r)
+	}))
+	defer regSrv.Close()
+
+	u, err := url.Parse(regSrv.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	img, err := random.Image(1024, 2)
+	if err != nil {
+		t.Fatal(err)
+	}
+	refStr := u.Host + "/test/redirect:latest"
+	ref, err := name.ParseReference(refStr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := remote.Write(ref, img); err != nil {
+		t.Fatal(err)
+	}
+
+	cl, err := NewRegistryClient(ctx, refStr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	mf, err := ManifestFromRemote(ctx, cl, refStr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	fa := libindex.NewRemoteFetchArena(cl, t.TempDir())
+	defer fa.Close(ctx)
+	rz := fa.Realizer(ctx)
+	defer rz.Close()
+	if err := rz.Realize(ctx, mf.Layers); err != nil {
+		t.Fatalf("realize: %v", err)
+	}
+	for _, l := range mf.Layers {
+		if _, err := l.Reader(); err != nil {
+			t.Errorf("layer %v not realized: %v", l.Hash, err)
+		}
+	}
+}