Address artifact cache review

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/BurntSushi/toml v1.6.0
 	github.com/CycloneDX/cyclonedx-go v0.11.0
 	github.com/git-pkgs/archives v0.5.1
+	github.com/git-pkgs/artifacts v0.1.0
 	github.com/git-pkgs/cooldown v0.1.1
 	github.com/git-pkgs/enrichment v0.6.5
 	github.com/git-pkgs/magic v0.2.0
@@ -17,6 +18,7 @@ require (
 	github.com/go-chi/chi/v5 v5.3.1
 	github.com/jmoiron/sqlx v1.4.0
 	github.com/lib/pq v1.12.3
+	github.com/opencontainers/go-digest v1.0.0
 	github.com/prometheus/client_golang v1.24.1
 	github.com/prometheus/client_model v0.6.2
 	github.com/spdx/tools-golang v0.5.7

go.sum

@@ -246,6 +246,8 @@ github.com/ghostiam/protogetter v0.3.20 h1:oW7OPFit2FxZOpmMRPP9FffU4uUpfeE/rEdE1
 github.com/ghostiam/protogetter v0.3.20/go.mod h1:FjIu5Yfs6FT391m+Fjp3fbAYJ6rkL/J6ySpZBfnODuI=
 github.com/git-pkgs/archives v0.5.1 h1:qwu/vsoerQZF1iysRtfcxpy1KIUSJJSpXJ5JNxzNoQw=
 github.com/git-pkgs/archives v0.5.1/go.mod h1:AKpkxnts49R9uAt1mL2ULYcHrmYujCDVu24IsFvW9so=
+github.com/git-pkgs/artifacts v0.1.0 h1:es73XxrsJefxO/DqtergTt+IaL+xPhzE0gSVrb6AFwk=
+github.com/git-pkgs/artifacts v0.1.0/go.mod h1:zWZ0mrFi2M6ajGwpbo1nBGQTSb7qptP+zNG/EwAemBE=
 github.com/git-pkgs/cooldown v0.1.1 h1:9OqqzCB8gANz/y44SmqGD0Jp8Qtu81D1sCbKl6Ehg7w=
 github.com/git-pkgs/cooldown v0.1.1/go.mod h1:v7APuK/UouTiu8mWQZbdDmj7DfxxkGUeuhjaRB5gv9E=
 github.com/git-pkgs/enrichment v0.6.5 h1:U0SPzWVGoK4R8TwojCTASBRTEV+QSs0IitdLmzI/g/k=
@@ -520,6 +522,8 @@ github.com/onsi/ginkgo/v2 v2.28.1 h1:S4hj+HbZp40fNKuLUQOYLDgZLwNUVn19N3Atb98NCyI
 github.com/onsi/ginkgo/v2 v2.28.1/go.mod h1:CLtbVInNckU3/+gC8LzkGUb9oF+e8W8TdUsxPwvdOgE=
 github.com/onsi/gomega v1.39.1 h1:1IJLAad4zjPn2PsnhH70V4DKRFlrCzGBNrNaru+Vf28=
 github.com/onsi/gomega v1.39.1/go.mod h1:hL6yVALoTOxeWudERyfppUcZXjMwIMLnuSfruD2lcfg=
+github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
+github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
 github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw=
 github.com/otiai10/copy v1.14.0 h1:dCI/t1iTdYGtkvCuBG2BgR6KZa83PTclw4U5n2wAllU=
 github.com/otiai10/copy v1.14.0/go.mod h1:ECfuL02W+/FkTWZWgQqXPWZgW9oeKCSQ5qVfSc4qc4w=

internal/database/database_test.go

@@ -4,10 +4,13 @@ import (
 	"database/sql"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 )
 
+const testContentHash = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+
 func TestCreateAndOpen(t *testing.T) {
 	dir := t.TempDir()
 	dbPath := filepath.Join(dir, "test.db")
@@ -200,7 +203,7 @@ func TestArtifactCRUD(t *testing.T) {
 			t.Error("expected artifact to not be cached yet")
 		}
 
-		err = db.MarkArtifactCached(versionPURL, "lodash-4.17.21.tgz", "/cache/npm/lodash-4.17.21.tgz", "sha256-abc", 12345, "application/gzip")
+		err = db.MarkArtifactCached(versionPURL, "lodash-4.17.21.tgz", "/cache/npm/lodash-4.17.21.tgz", testContentHash, 12345, "application/gzip")
 		if err != nil {
 			t.Fatalf("MarkArtifactCached failed: %v", err)
 		}
@@ -257,7 +260,7 @@ func TestGetCachedArtifact(t *testing.T) {
 		}
 
 		if err := db.MarkArtifactCached(versionPURL, filename, "/cache/npm/"+filename,
-			"sha256-abc", 12345, "application/gzip"); err != nil {
+			testContentHash, 12345, "application/gzip"); err != nil {
 			t.Fatalf("MarkArtifactCached failed: %v", err)
 		}
 
@@ -274,14 +277,20 @@ func TestGetCachedArtifact(t *testing.T) {
 		if cached.StoragePath != "/cache/npm/"+filename {
 			t.Errorf("expected cached storage path, got %q", cached.StoragePath)
 		}
-		if cached.ContentHash.String != "sha256-abc" {
+		if cached.Artifact.PURL != versionPURL {
-			t.Errorf("expected cached content hash, got %q", cached.ContentHash.String)
+			t.Errorf("expected cached PURL %q, got %q", versionPURL, cached.Artifact.PURL)
 		}
-		if cached.Size.Int64 != 12345 {
+		if cached.Artifact.Digest.String() != "sha256:"+testContentHash {
-			t.Errorf("expected cached size 12345, got %d", cached.Size.Int64)
+			t.Errorf("expected cached digest, got %q", cached.Artifact.Digest)
 		}
-		if cached.ContentType.String != "application/gzip" {
+		if cached.Artifact.Size != 12345 {
-			t.Errorf("expected cached content type, got %q", cached.ContentType.String)
+			t.Errorf("expected cached size 12345, got %d", cached.Artifact.Size)
+		}
+		if cached.Artifact.Filename != filename {
+			t.Errorf("expected cached filename %q, got %q", filename, cached.Artifact.Filename)
+		}
+		if cached.Artifact.MediaType != "application/gzip" {
+			t.Errorf("expected cached content type, got %q", cached.Artifact.MediaType)
 		}
 		if cached.Integrity.String != "sha512-abc123" {
 			t.Errorf("expected cached integrity, got %q", cached.Integrity.String)
@@ -297,6 +306,83 @@ func TestGetCachedArtifact(t *testing.T) {
 	})
 }
 
+func TestCachedArtifactRowArtifact(t *testing.T) {
+	validRow := cachedArtifactRow{
+		Ecosystem:   "npm",
+		StoragePath: "npm/example/1.0.0/example.tgz",
+		ContentHash: sql.NullString{String: testContentHash, Valid: true},
+		Size:        sql.NullInt64{Int64: 0, Valid: true},
+	}
+	tests := []struct {
+		name        string
+		row         cachedArtifactRow
+		versionPURL string
+		wantErr     string
+	}{
+		{name: "zero byte", row: validRow, versionPURL: "pkg:npm/example@1.0.0"},
+		{
+			name:        "missing hash",
+			row:         cachedArtifactRow{Size: sql.NullInt64{Int64: 1, Valid: true}},
+			versionPURL: "pkg:npm/example@1.0.0",
+			wantErr:     "content hash is missing",
+		},
+		{
+			name: "malformed hash",
+			row: cachedArtifactRow{
+				ContentHash: sql.NullString{String: "not-a-hash", Valid: true},
+				Size:        sql.NullInt64{Int64: 1, Valid: true},
+			},
+			versionPURL: "pkg:npm/example@1.0.0",
+			wantErr:     "digest",
+		},
+		{
+			name: "missing size",
+			row: cachedArtifactRow{
+				ContentHash: sql.NullString{String: testContentHash, Valid: true},
+			},
+			versionPURL: "pkg:npm/example@1.0.0",
+			wantErr:     "size is missing",
+		},
+		{
+			name: "negative size",
+			row: cachedArtifactRow{
+				ContentHash: sql.NullString{String: testContentHash, Valid: true},
+				Size:        sql.NullInt64{Int64: -1, Valid: true},
+			},
+			versionPURL: "pkg:npm/example@1.0.0",
+			wantErr:     "size",
+		},
+		{name: "malformed PURL", row: validRow, versionPURL: "not-a-purl", wantErr: "PURL"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cached, err := test.row.artifact(test.versionPURL, "example.tgz")
+			if test.wantErr != "" {
+				if err == nil {
+					t.Fatal("artifact() error = nil")
+				}
+				if !strings.Contains(err.Error(), test.wantErr) {
+					t.Errorf("error = %q, want %q", err, test.wantErr)
+				}
+				if !strings.Contains(err.Error(), test.versionPURL) {
+					t.Errorf("error = %q, want version PURL %q", err, test.versionPURL)
+				}
+				if !strings.Contains(err.Error(), "example.tgz") {
+					t.Errorf("error = %q, want filename", err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("artifact() error = %v", err)
+			}
+			if err := cached.Artifact.Validate(); err != nil {
+				t.Errorf("Artifact.Validate() error = %v", err)
+			}
+		})
+	}
+}
+
 func seedCachedArtifactTestData(t *testing.T, db *DB, packagePURL, versionPURL, filename string) {
 	t.Helper()
 

internal/database/queries.go

@@ -4,6 +4,9 @@ import (
 	"database/sql"
 	"fmt"
 	"time"
+
+	"github.com/git-pkgs/artifacts"
+	"github.com/opencontainers/go-digest"
 )
 
 // Package queries
@@ -193,7 +196,7 @@ func (db *DB) GetArtifact(versionPURL, filename string) (*Artifact, error) {
 
 // GetCachedArtifact returns the fields needed to serve a cached artifact.
 func (db *DB) GetCachedArtifact(packagePURL, versionPURL, filename string) (*CachedArtifact, error) {
-	var artifact CachedArtifact
+	var row cachedArtifactRow
 	query := db.Rebind(`
 		SELECT packages.ecosystem, artifacts.storage_path, artifacts.content_hash, artifacts.size,
 		       artifacts.content_type, versions.integrity
@@ -203,14 +206,54 @@ func (db *DB) GetCachedArtifact(packagePURL, versionPURL, filename string) (*Cac
 		WHERE packages.purl = ? AND artifacts.version_purl = ? AND artifacts.filename = ?
 		  AND artifacts.storage_path IS NOT NULL AND artifacts.fetched_at IS NOT NULL
 	`)
-	err := db.Get(&artifact, query, packagePURL, versionPURL, filename)
+	err := db.Get(&row, query, packagePURL, versionPURL, filename)
 	if err == sql.ErrNoRows {
 		return nil, nil
 	}
 	if err != nil {
 		return nil, err
 	}
-	return &artifact, nil
+	return row.artifact(versionPURL, filename)
+}
+
+type cachedArtifactRow struct {
+	Ecosystem   string         `db:"ecosystem"`
+	StoragePath string         `db:"storage_path"`
+	ContentHash sql.NullString `db:"content_hash"`
+	Size        sql.NullInt64  `db:"size"`
+	ContentType sql.NullString `db:"content_type"`
+	Integrity   sql.NullString `db:"integrity"`
+}
+
+func (row cachedArtifactRow) artifact(versionPURL, filename string) (*CachedArtifact, error) {
+	if !row.ContentHash.Valid || row.ContentHash.String == "" {
+		return nil, fmt.Errorf("cached artifact for %q, filename %q: content hash is missing", versionPURL, filename)
+	}
+	if !row.Size.Valid {
+		return nil, fmt.Errorf("cached artifact for %q, filename %q: size is missing", versionPURL, filename)
+	}
+
+	mediaType := ""
+	if row.ContentType.Valid {
+		mediaType = row.ContentType.String
+	}
+	sharedArtifact, err := artifacts.New(
+		versionPURL,
+		digest.Digest("sha256:"+row.ContentHash.String),
+		row.Size.Int64,
+		filename,
+		mediaType,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("invalid cached artifact for %q, filename %q: %w", versionPURL, filename, err)
+	}
+
+	return &CachedArtifact{
+		Ecosystem:   row.Ecosystem,
+		StoragePath: row.StoragePath,
+		Artifact:    sharedArtifact,
+		Integrity:   row.Integrity,
+	}, nil
 }
 
 func (db *DB) GetArtifactByPath(storagePath string) (*Artifact, error) {

internal/database/types.go

@@ -5,6 +5,8 @@ import (
 	"net/url"
 	"strings"
 	"time"
+
+	"github.com/git-pkgs/artifacts"
 )
 
 // Package represents a package in the database.
@@ -148,12 +150,10 @@ func (a *Artifact) IsCached() bool {
 
 // CachedArtifact contains the fields needed to serve a cached artifact.
 type CachedArtifact struct {
-	Ecosystem   string         `db:"ecosystem"`
+	Ecosystem   string
-	StoragePath string         `db:"storage_path"`
+	StoragePath string
-	ContentHash sql.NullString `db:"content_hash"`
+	Artifact    artifacts.Artifact
-	Size        sql.NullInt64  `db:"size"`
+	Integrity   sql.NullString
-	ContentType sql.NullString `db:"content_type"`
-	Integrity   sql.NullString `db:"integrity"`
 }
 
 // MetadataCacheEntry represents a cached metadata blob for offline serving.

internal/handler/container_test.go

@@ -436,8 +436,9 @@ func TestContainerHandler_BlobHead_DirectServeRedirects(t *testing.T) {
 	if got := w.Header().Get("Location"); got != store.signedURL {
 		t.Errorf("Location = %q, want %q", got, store.signedURL)
 	}
-	if got := w.Header().Get("ETag"); got != `"abc123"` {
+	wantETag := `"` + testArtifact("cached blob", "", "", "").Digest.Encoded() + `"`
-		t.Errorf("ETag = %q, want %q", got, `"abc123"`)
+	if got := w.Header().Get("ETag"); got != wantETag {
+		t.Errorf("ETag = %q, want %q", got, wantETag)
 	}
 	if w.Body.Len() != 0 {
 		t.Errorf("HEAD response body length = %d, want 0", w.Body.Len())

internal/handler/download_test.go

@@ -43,13 +43,14 @@ func seedPackageWithPURL(t *testing.T, db *database.DB, store *mockStorage, ecos
 
 	storagePath := storage.ArtifactPath(ecosystem, "", name, version, filename)
 	store.files[storagePath] = []byte(content)
+	sharedArtifact := testArtifact(content, versionPURL, filename, "application/octet-stream")
 
 	art := &database.Artifact{
 		VersionPURL: versionPURL,
 		Filename:    filename,
 		UpstreamURL: "https://example.com/" + filename,
 		StoragePath: sql.NullString{String: storagePath, Valid: true},
-		ContentHash: sql.NullString{String: "abc123", Valid: true},
+		ContentHash: sql.NullString{String: sharedArtifact.Digest.Encoded(), Valid: true},
 		Size:        sql.NullInt64{Int64: int64(len(content)), Valid: true},
 		ContentType: sql.NullString{String: "application/octet-stream", Valid: true},
 		FetchedAt:   sql.NullTime{Time: time.Now(), Valid: true},

internal/handler/handler.go

@@ -16,12 +16,14 @@ import (
 	"sync"
 	"time"
 
+	"github.com/git-pkgs/artifacts"
 	"github.com/git-pkgs/cooldown"
 	"github.com/git-pkgs/proxy/internal/database"
 	"github.com/git-pkgs/proxy/internal/metrics"
 	"github.com/git-pkgs/proxy/internal/storage"
 	"github.com/git-pkgs/purl"
 	"github.com/git-pkgs/registries/fetch"
+	"github.com/opencontainers/go-digest"
 )
 
 // containsPathTraversal returns true if the path contains ".." segments
@@ -138,9 +140,7 @@ func NewProxy(db *database.DB, store storage.Storage, fetcher fetch.FetcherInter
 type CacheResult struct {
 	Reader      io.ReadCloser
 	RedirectURL string
-	Size        int64
+	Artifact    artifacts.Artifact
-	ContentType string
-	Hash        string
 	Cached      bool
 }
 
@@ -176,10 +176,8 @@ func (p *Proxy) checkCache(ctx context.Context, pkgPURL, versionPURL, filename s
 	}
 
 	result := &CacheResult{
-		Size:        artifact.Size.Int64,
+		Artifact: artifact.Artifact,
-		ContentType: artifact.ContentType.String,
+		Cached:   true,
-		Hash:        artifact.ContentHash.String,
-		Cached:      true,
 	}
 
 	if p.DirectServe {
@@ -205,7 +203,7 @@ func (p *Proxy) checkCache(ctx context.Context, pkgPURL, versionPURL, filename s
 		return nil, nil
 	}
 
-	result.Reader = newVerifyingReader(reader, artifact.ContentHash.String, artifact.Integrity.String,
+	result.Reader = newVerifyingReader(reader, artifact.Artifact.Digest.Encoded(), artifact.Integrity.String,
 		func(reason string) {
 			p.Logger.Error("cached artifact failed integrity check",
 				"purl", versionPURL, "filename", filename,
@@ -291,9 +289,19 @@ func (p *Proxy) fetchAndCache(ctx context.Context, ecosystem, name, version, fil
 		metrics.RecordStorageError("write")
 		return nil, fmt.Errorf("storing artifact: %w", err)
 	}
+	sharedArtifact, err := artifacts.New(
+		versionPURL,
+		digest.Digest("sha256:"+hash),
+		size,
+		filename,
+		artifact.ContentType,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("describing stored artifact: %w", err)
+	}
 
 	// Update database
-	if err := p.updateCacheDB(ecosystem, name, filename, pkgPURL, versionPURL, info.URL, storagePath, hash, size, artifact.ContentType); err != nil {
+	if err := p.updateCacheDB(ecosystem, name, pkgPURL, info.URL, storagePath, sharedArtifact); err != nil {
 		p.Logger.Warn("failed to update cache database", "error", err)
 		// Continue anyway - we have the file
 	}
@@ -309,15 +317,13 @@ func (p *Proxy) fetchAndCache(ctx context.Context, ecosystem, name, version, fil
 	}
 
 	return &CacheResult{
-		Reader:      reader,
+		Reader:   reader,
-		Size:        size,
+		Artifact: sharedArtifact,
-		ContentType: artifact.ContentType,
+		Cached:   false,
-		Hash:        hash,
-		Cached:      false,
 	}, nil
 }
 
-func (p *Proxy) updateCacheDB(ecosystem, name, filename, pkgPURL, versionPURL, upstreamURL, storagePath, hash string, size int64, contentType string) error {
+func (p *Proxy) updateCacheDB(ecosystem, name, pkgPURL, upstreamURL, storagePath string, artifact artifacts.Artifact) error {
 	now := time.Now()
 
 	// Upsert package
@@ -334,7 +340,7 @@ func (p *Proxy) updateCacheDB(ecosystem, name, filename, pkgPURL, versionPURL, u
 
 	// Upsert version
 	ver := &database.Version{
-		PURL:        versionPURL,
+		PURL:        artifact.PURL,
 		PackagePURL: pkgPURL,
 		EnrichedAt:  sql.NullTime{Time: now, Valid: true},
 	}
@@ -344,13 +350,13 @@ func (p *Proxy) updateCacheDB(ecosystem, name, filename, pkgPURL, versionPURL, u
 
 	// Upsert artifact
 	art := &database.Artifact{
-		VersionPURL: versionPURL,
+		VersionPURL: artifact.PURL,
-		Filename:    filename,
+		Filename:    artifact.Filename,
 		UpstreamURL: upstreamURL,
 		StoragePath: sql.NullString{String: storagePath, Valid: true},
-		ContentHash: sql.NullString{String: hash, Valid: true},
+		ContentHash: sql.NullString{String: artifact.Digest.Encoded(), Valid: true},
-		Size:        sql.NullInt64{Int64: size, Valid: true},
+		Size:        sql.NullInt64{Int64: artifact.Size, Valid: true},
-		ContentType: sql.NullString{String: contentType, Valid: true},
+		ContentType: sql.NullString{String: artifact.MediaType, Valid: true},
 		FetchedAt:   sql.NullTime{Time: now, Valid: true},
 	}
 	if err := p.DB.UpsertArtifact(art); err != nil {
@@ -366,9 +372,13 @@ func ServeArtifact(w http.ResponseWriter, result *CacheResult) {
 }
 
 func serveArtifact(w http.ResponseWriter, method string, result *CacheResult) {
+	contentHash := ""
+	if result.Artifact.Digest != "" {
+		contentHash = result.Artifact.Digest.Encoded()
+	}
 	if result.RedirectURL != "" {
-		if result.Hash != "" {
+		if contentHash != "" {
-			w.Header().Set("ETag", `"`+result.Hash+`"`)
+			w.Header().Set("ETag", `"`+contentHash+`"`)
 		}
 		w.Header().Set("Location", result.RedirectURL)
 		w.WriteHeader(http.StatusFound)
@@ -379,14 +389,14 @@ func serveArtifact(w http.ResponseWriter, method string, result *CacheResult) {
 		defer func() { _ = result.Reader.Close() }()
 	}
 
-	if result.ContentType != "" {
+	if result.Artifact.MediaType != "" {
-		w.Header().Set("Content-Type", result.ContentType)
+		w.Header().Set("Content-Type", result.Artifact.MediaType)
 	}
-	if result.Size > 0 || (method == http.MethodHead && result.Size == 0) {
+	if result.Artifact.Size > 0 || (method == http.MethodHead && result.Artifact.Size == 0) {
-		w.Header().Set("Content-Length", strconv.FormatInt(result.Size, 10))
+		w.Header().Set("Content-Length", strconv.FormatInt(result.Artifact.Size, 10))
 	}
-	if result.Hash != "" {
+	if contentHash != "" {
-		w.Header().Set("ETag", `"`+result.Hash+`"`)
+		w.Header().Set("ETag", `"`+contentHash+`"`)
 	}
 
 	w.WriteHeader(http.StatusOK)
@@ -848,8 +858,18 @@ func (p *Proxy) fetchAndCacheFromURL(ctx context.Context, ecosystem, name, versi
 	if err != nil {
 		return nil, fmt.Errorf("storing artifact: %w", err)
 	}
+	sharedArtifact, err := artifacts.New(
+		versionPURL,
+		digest.Digest("sha256:"+hash),
+		size,
+		filename,
+		artifact.ContentType,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("describing stored artifact: %w", err)
+	}
 
-	if err := p.updateCacheDB(ecosystem, name, filename, pkgPURL, versionPURL, downloadURL, storagePath, hash, size, artifact.ContentType); err != nil {
+	if err := p.updateCacheDB(ecosystem, name, pkgPURL, downloadURL, storagePath, sharedArtifact); err != nil {
 		p.Logger.Warn("failed to update cache database", "error", err)
 	}
 
@@ -859,10 +879,8 @@ func (p *Proxy) fetchAndCacheFromURL(ctx context.Context, ecosystem, name, versi
 	}
 
 	return &CacheResult{
-		Reader:      reader,
+		Reader:   reader,
-		Size:        size,
+		Artifact: sharedArtifact,
-		ContentType: artifact.ContentType,
+		Cached:   false,
-		Hash:        hash,
-		Cached:      false,
 	}, nil
 }

internal/handler/handler_test.go

@@ -3,7 +3,9 @@ package handler
 import (
 	"bytes"
 	"context"
+	"crypto/sha256"
 	"database/sql"
+	"encoding/hex"
 	"errors"
 	"io"
 	"log/slog"
@@ -13,11 +15,13 @@ import (
 	"testing"
 	"time"
 
+	"github.com/git-pkgs/artifacts"
 	"github.com/git-pkgs/proxy/internal/config"
 	"github.com/git-pkgs/proxy/internal/database"
 	"github.com/git-pkgs/proxy/internal/storage"
 	"github.com/git-pkgs/purl"
 	"github.com/git-pkgs/registries/fetch"
+	"github.com/opencontainers/go-digest"
 )
 
 // mockStorage implements storage.Storage for testing.
@@ -25,6 +29,7 @@ type mockStorage struct {
 	files     map[string][]byte
 	storeErr  error
 	openErr   error
+	storeHash string
 	signedURL string
 	signErr   error
 }
@@ -42,7 +47,11 @@ func (s *mockStorage) Store(_ context.Context, path string, r io.Reader) (int64,
 		return 0, "", err
 	}
 	s.files[path] = data
-	return int64(len(data)), "fakehash123", nil
+	if s.storeHash != "" {
+		return int64(len(data)), s.storeHash, nil
+	}
+	sum := sha256.Sum256(data)
+	return int64(len(data)), hex.EncodeToString(sum[:]), nil
 }
 
 func (s *mockStorage) Open(_ context.Context, path string) (io.ReadCloser, error) {
@@ -147,6 +156,17 @@ func setupTestProxy(t testing.TB) (*Proxy, *database.DB, *mockStorage, *mockFetc
 	return proxy, db, store, fetcher
 }
 
+func testArtifact(content, packageURL, filename, mediaType string) artifacts.Artifact {
+	sum := sha256.Sum256([]byte(content))
+	return artifacts.Artifact{
+		PURL:      packageURL,
+		Digest:    digest.Digest("sha256:" + hex.EncodeToString(sum[:])),
+		Size:      int64(len(content)),
+		Filename:  filename,
+		MediaType: mediaType,
+	}
+}
+
 // seedPackage creates a package, version, and cached artifact in the test DB and storage.
 func seedPackage(t testing.TB, db *database.DB, store *mockStorage, ecosystem, name, version, filename, content string) {
 	t.Helper()
@@ -171,13 +191,14 @@ func seedPackage(t testing.TB, db *database.DB, store *mockStorage, ecosystem, n
 
 	storagePath := storage.ArtifactPath(ecosystem, "", name, version, filename)
 	store.files[storagePath] = []byte(content)
+	sharedArtifact := testArtifact(content, versionPURL, filename, "application/octet-stream")
 
 	art := &database.Artifact{
 		VersionPURL: versionPURL,
 		Filename:    filename,
 		UpstreamURL: "https://example.com/" + filename,
 		StoragePath: sql.NullString{String: storagePath, Valid: true},
-		ContentHash: sql.NullString{String: "abc123", Valid: true},
+		ContentHash: sql.NullString{String: sharedArtifact.Digest.Encoded(), Valid: true},
 		Size:        sql.NullInt64{Int64: int64(len(content)), Valid: true},
 		ContentType: sql.NullString{String: "application/octet-stream", Valid: true},
 		FetchedAt:   sql.NullTime{Time: time.Now(), Valid: true},
@@ -263,11 +284,38 @@ func TestGetOrFetchArtifact_CacheHit(t *testing.T) {
 	if string(body) != "cached content" {
 		t.Errorf("got body %q, want %q", body, "cached content")
 	}
-	if result.ContentType != "application/octet-stream" {
+	if result.Artifact.MediaType != "application/octet-stream" {
-		t.Errorf("got content type %q, want %q", result.ContentType, "application/octet-stream")
+		t.Errorf("got content type %q, want %q", result.Artifact.MediaType, "application/octet-stream")
 	}
-	if result.Hash != "abc123" {
+	wantDigest := testArtifact("cached content", "", "", "").Digest
-		t.Errorf("got hash %q, want %q", result.Hash, "abc123")
+	if result.Artifact.Digest != wantDigest {
+		t.Errorf("got digest %q, want %q", result.Artifact.Digest, wantDigest)
+	}
+}
+
+func TestGetOrFetchArtifact_CacheHit_MalformedMetadata(t *testing.T) {
+	proxy, db, store, fetcher := setupTestProxy(t)
+	const filename = "lodash-4.17.21.tgz"
+	seedPackage(t, db, store, "npm", "lodash", "4.17.21", filename, "cached content")
+
+	artifact, err := db.GetArtifact("pkg:npm/lodash@4.17.21", filename)
+	if err != nil {
+		t.Fatalf("GetArtifact failed: %v", err)
+	}
+	artifact.ContentHash = sql.NullString{String: "not-a-hash", Valid: true}
+	if err := db.UpsertArtifact(artifact); err != nil {
+		t.Fatalf("UpsertArtifact failed: %v", err)
+	}
+
+	_, err = proxy.GetOrFetchArtifact(context.Background(), "npm", "lodash", "4.17.21", filename)
+	if err == nil {
+		t.Fatal("GetOrFetchArtifact() error = nil")
+	}
+	if !strings.Contains(err.Error(), "invalid cached artifact") {
+		t.Errorf("error = %q, want invalid cached artifact", err)
+	}
+	if fetcher.fetchCalled {
+		t.Error("fetcher should not be called for malformed cache metadata")
 	}
 }
 
@@ -297,7 +345,7 @@ func TestGetOrFetchArtifactFromURL_CacheMiss_StorageMissing(t *testing.T) {
 		Filename:    "missing-1.0.0.tgz",
 		UpstreamURL: "https://example.com/missing.tgz",
 		StoragePath: sql.NullString{String: "nonexistent/path.tgz", Valid: true},
-		ContentHash: sql.NullString{String: "hash", Valid: true},
+		ContentHash: sql.NullString{String: strings.Repeat("a", sha256.Size*2), Valid: true},
 		Size:        sql.NullInt64{Int64: 100, Valid: true},
 		ContentType: sql.NullString{String: "application/octet-stream", Valid: true},
 		FetchedAt:   sql.NullTime{Time: time.Now(), Valid: true},
@@ -499,8 +547,10 @@ func TestServeArtifact_Redirect(t *testing.T) {
 	w := httptest.NewRecorder()
 	ServeArtifact(w, &CacheResult{
 		RedirectURL: "https://bucket.s3.amazonaws.com/file?sig=abc",
-		Hash:        "abc123",
+		Artifact: artifacts.Artifact{
-		Cached:      true,
+			Digest: digest.Digest("sha256:" + strings.Repeat("a", sha256.Size*2)),
+		},
+		Cached: true,
 	})
 
 	if w.Code != http.StatusFound {
@@ -509,8 +559,8 @@ func TestServeArtifact_Redirect(t *testing.T) {
 	if loc := w.Header().Get("Location"); loc != "https://bucket.s3.amazonaws.com/file?sig=abc" {
 		t.Errorf("Location = %q", loc)
 	}
-	if etag := w.Header().Get("ETag"); etag != `"abc123"` {
+	if etag := w.Header().Get("ETag"); etag != `"`+strings.Repeat("a", sha256.Size*2)+`"` {
-		t.Errorf("ETag = %q, want %q", etag, `"abc123"`)
+		t.Errorf("ETag = %q", etag)
 	}
 	if cl := w.Header().Get("Content-Length"); cl != "" {
 		t.Errorf("Content-Length should not be set on redirect, got %q", cl)
@@ -520,10 +570,13 @@ func TestServeArtifact_Redirect(t *testing.T) {
 func TestServeArtifact_Stream(t *testing.T) {
 	w := httptest.NewRecorder()
 	ServeArtifact(w, &CacheResult{
-		Reader:      io.NopCloser(strings.NewReader("payload")),
+		Reader: io.NopCloser(strings.NewReader("payload")),
-		Size:        7,
+		Artifact: testArtifact(
-		ContentType: "application/octet-stream",
+			"payload",
-		Hash:        "abc123",
+			"pkg:npm/example@1.0.0",
+			"example.tgz",
+			"application/octet-stream",
+		),
 	})
 
 	if w.Code != http.StatusOK {
@@ -583,6 +636,18 @@ func TestGetOrFetchArtifactFromURL_CacheMiss(t *testing.T) {
 	if string(body) != "fetched content" {
 		t.Errorf("got body %q, want %q", body, "fetched content")
 	}
+	if err := result.Artifact.Validate(); err != nil {
+		t.Errorf("Artifact.Validate() error = %v", err)
+	}
+	if result.Artifact.PURL != "pkg:pypi/newpkg@1.0.0" {
+		t.Errorf("PURL = %q", result.Artifact.PURL)
+	}
+	if result.Artifact.Size != int64(len("fetched content")) {
+		t.Errorf("Size = %d", result.Artifact.Size)
+	}
+	if result.Artifact.MediaType != "application/gzip" {
+		t.Errorf("MediaType = %q", result.Artifact.MediaType)
+	}
 
 	// Verify it was stored
 	storagePath := storage.ArtifactPath("pypi", "", "newpkg", "1.0.0", "newpkg-1.0.0.tar.gz")
@@ -621,13 +686,28 @@ func TestGetOrFetchArtifactFromURL_StoreError(t *testing.T) {
 	}
 }
 
+func TestGetOrFetchArtifactFromURL_RejectsMalformedStorageDigest(t *testing.T) {
+	proxy, _, store, fetcher := setupTestProxy(t)
+	store.storeHash = "not-a-hash"
+	fetcher.artifact = &fetch.Artifact{
+		Body:        io.NopCloser(strings.NewReader("data")),
+		ContentType: "application/gzip",
+	}
+
+	_, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "pypi", "fail", "1.0.0", "fail-1.0.0.tar.gz", "https://pypi.org/files/fail.tar.gz")
+	if err == nil {
+		t.Fatal("GetOrFetchArtifactFromURL() error = nil")
+	}
+	if !strings.Contains(err.Error(), "describing stored artifact") {
+		t.Errorf("error = %q, want stored artifact validation error", err)
+	}
+}
+
 func TestServeArtifact(t *testing.T) {
 	result := &CacheResult{
-		Reader:      io.NopCloser(strings.NewReader("file contents")),
+		Reader:   io.NopCloser(strings.NewReader("file contents")),
-		Size:        13,
+		Artifact: testArtifact("file contents", "pkg:npm/example@1.0.0", "example.tgz", "application/gzip"),
-		ContentType: "application/gzip",
+		Cached:   true,
-		Hash:        "sha256abc",
-		Cached:      true,
 	}
 
 	w := httptest.NewRecorder()
@@ -642,8 +722,9 @@ func TestServeArtifact(t *testing.T) {
 	if w.Header().Get("Content-Length") != "13" {
 		t.Errorf("Content-Length = %q, want %q", w.Header().Get("Content-Length"), "13")
 	}
-	if w.Header().Get("ETag") != `"sha256abc"` {
+	wantETag := `"` + result.Artifact.Digest.Encoded() + `"`
-		t.Errorf("ETag = %q, want %q", w.Header().Get("ETag"), `"sha256abc"`)
+	if w.Header().Get("ETag") != wantETag {
+		t.Errorf("ETag = %q, want %q", w.Header().Get("ETag"), wantETag)
 	}
 	if w.Body.String() != "file contents" {
 		t.Errorf("body = %q, want %q", w.Body.String(), "file contents")

internal/mirror/mirror.go

@@ -212,7 +212,9 @@ func (m *Mirror) mirrorOne(ctx context.Context, pv PackageVersion, tracker *prog
 		return
 	}
 
-	_ = result.Reader.Close()
+	if result.Reader != nil {
+		_ = result.Reader.Close()
+	}
 
 	if result.Cached {
 		tracker.skipped.Add(1)
@@ -220,9 +222,9 @@ func (m *Mirror) mirrorOne(ctx context.Context, pv PackageVersion, tracker *prog
 			"ecosystem", pv.Ecosystem, "name", pv.Name, "version", pv.Version)
 	} else {
 		tracker.completed.Add(1)
-		tracker.bytes.Add(result.Size)
+		tracker.bytes.Add(result.Artifact.Size)
 		m.logger.Info("mirrored",
 			"ecosystem", pv.Ecosystem, "name", pv.Name, "version", pv.Version,
-			"size", result.Size)
+			"size", result.Artifact.Size)
 	}
 }

internal/mirror/mirror_test.go

@@ -2,8 +2,11 @@ package mirror
 
 import (
 	"context"
+	"crypto/sha256"
+	"database/sql"
 	"log/slog"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -43,6 +46,14 @@ func setupTestMirror(t *testing.T, workers int) *Mirror {
 
 const testPackageLodash = "lodash"
 
+type signedURLStorage struct {
+	storage.Storage
+}
+
+func (signedURLStorage) SignedURL(context.Context, string, time.Duration) (string, error) {
+	return "https://storage.example/artifact", nil
+}
+
 func TestMirrorRunEmptySource(t *testing.T) {
 	m := setupTestMirror(t, 2)
 
@@ -111,6 +122,54 @@ func TestMirrorRunCanceled(t *testing.T) {
 	}
 }
 
+func TestMirrorOneDirectServeCacheHit(t *testing.T) {
+	m := setupTestMirror(t, 1)
+	m.proxy.DirectServe = true
+	m.proxy.Storage = signedURLStorage{Storage: m.storage}
+
+	packagePURL := "pkg:npm/example"
+	versionPURL := packagePURL + "@1.0.0"
+	if err := m.db.UpsertPackage(&database.Package{
+		PURL:      packagePURL,
+		Ecosystem: "npm",
+		Name:      "example",
+	}); err != nil {
+		t.Fatalf("UpsertPackage() error = %v", err)
+	}
+	if err := m.db.UpsertVersion(&database.Version{
+		PURL:        versionPURL,
+		PackagePURL: packagePURL,
+	}); err != nil {
+		t.Fatalf("UpsertVersion() error = %v", err)
+	}
+	if err := m.db.UpsertArtifact(&database.Artifact{
+		VersionPURL: versionPURL,
+		Filename:    "",
+		UpstreamURL: "https://registry.example/artifact",
+		StoragePath: sql.NullString{String: "npm/example/1.0.0/artifact", Valid: true},
+		ContentHash: sql.NullString{String: strings.Repeat("a", sha256.Size*2), Valid: true},
+		Size:        sql.NullInt64{Int64: 1, Valid: true},
+		FetchedAt:   sql.NullTime{Time: time.Now(), Valid: true},
+	}); err != nil {
+		t.Fatalf("UpsertArtifact() error = %v", err)
+	}
+
+	tracker := newProgressTracker()
+	m.mirrorOne(context.Background(), PackageVersion{
+		Ecosystem: "npm",
+		Name:      "example",
+		Version:   "1.0.0",
+	}, tracker)
+
+	progress := tracker.snapshot()
+	if progress.Skipped != 1 {
+		t.Errorf("skipped = %d, want 1", progress.Skipped)
+	}
+	if progress.Failed != 0 {
+		t.Errorf("failed = %d, want 0", progress.Failed)
+	}
+}
+
 func TestProgressTrackerSnapshot(t *testing.T) {
 	pt := newProgressTracker()
 	pt.total.Store(10)