diff --git a/go.mod b/go.mod index c8bb4f6..beb444e 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/git-pkgs/archives v0.5.1 github.com/git-pkgs/cooldown v0.1.1 github.com/git-pkgs/enrichment v0.6.5 + github.com/git-pkgs/integrity v0.1.1 github.com/git-pkgs/magic v0.2.0 github.com/git-pkgs/purl v0.1.17 github.com/git-pkgs/registries v0.7.0 diff --git a/go.sum b/go.sum index 699e1cc..e8efd91 100644 --- a/go.sum +++ b/go.sum @@ -250,6 +250,8 @@ github.com/git-pkgs/cooldown v0.1.1 h1:9OqqzCB8gANz/y44SmqGD0Jp8Qtu81D1sCbKl6Ehg 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= github.com/git-pkgs/enrichment v0.6.5/go.mod h1:Vt2PLMvWPOio9DLyC8Gdhh1yxsHwcRcG+L2Kkc9+kak= +github.com/git-pkgs/integrity v0.1.1 h1:nHQ7SktOiGM1dOb5BFnkdtttG/6FCgE6r5ru6QnsGts= +github.com/git-pkgs/integrity v0.1.1/go.mod h1:hxu24lcd230377hCF28JQW7sGcCbuNLqo/0ULeb+F1Q= github.com/git-pkgs/magic v0.2.0 h1:c7HqVxnP8c88EaVMH0/KraDFVTcmiXckRiSvNZEnvMQ= github.com/git-pkgs/magic v0.2.0/go.mod h1:3ndidt+yvFaI1M0aEkkzkOlFnLPkeVQASIUojazcxCI= github.com/git-pkgs/packageurl-go v0.3.1 h1:WM3RBABQZLaRBxgKyYughc3cVBE8KyQxbSC6Jt5ak7M= diff --git a/internal/database/database_test.go b/internal/database/database_test.go index 3e92b91..bb2b195 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -8,6 +8,11 @@ import ( "time" ) +const ( + testContentHash = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + testIntegrity = "sha512-z4PhNX7vuL3xVChQ1m2AB9Yg5AULVxXcg/SpIdNs6c5H0NE8XYXysP+DGNKHfuwvY7kxvUdBeoGlODJ6+SfaPg==" +) + func TestCreateAndOpen(t *testing.T) { dir := t.TempDir() dbPath := filepath.Join(dir, "test.db") @@ -132,7 +137,7 @@ func TestVersionCRUD(t *testing.T) { v := &Version{ PURL: "pkg:npm/lodash@4.17.21", PackagePURL: "pkg:npm/lodash", - Integrity: sql.NullString{String: "sha512-abc123", Valid: true}, + Integrity: sql.NullString{String: testIntegrity, Valid: true}, } err = db.UpsertVersion(v) @@ -200,7 +205,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 +262,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,7 +279,7 @@ 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.ContentHash.String != testContentHash { t.Errorf("expected cached content hash, got %q", cached.ContentHash.String) } if cached.Size.Int64 != 12345 { @@ -283,7 +288,7 @@ func TestGetCachedArtifact(t *testing.T) { if cached.ContentType.String != "application/gzip" { t.Errorf("expected cached content type, got %q", cached.ContentType.String) } - if cached.Integrity.String != "sha512-abc123" { + if cached.Integrity.String != testIntegrity { t.Errorf("expected cached integrity, got %q", cached.Integrity.String) } @@ -306,7 +311,7 @@ func seedCachedArtifactTestData(t *testing.T, db *DB, packagePURL, versionPURL, if err := db.UpsertVersion(&Version{ PURL: versionPURL, PackagePURL: packagePURL, - Integrity: sql.NullString{String: "sha512-abc123", Valid: true}, + Integrity: sql.NullString{String: testIntegrity, Valid: true}, }); err != nil { t.Fatalf("UpsertVersion failed: %v", err) } diff --git a/internal/handler/container_test.go b/internal/handler/container_test.go index 81c6807..04f00a7 100644 --- a/internal/handler/container_test.go +++ b/internal/handler/container_test.go @@ -482,8 +482,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"` { - t.Errorf("ETag = %q, want %q", got, `"abc123"`) + wantETag := `"` + sha256Hex("cached blob") + `"` + 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()) diff --git a/internal/handler/download_test.go b/internal/handler/download_test.go index 8192eeb..dda3e84 100644 --- a/internal/handler/download_test.go +++ b/internal/handler/download_test.go @@ -49,7 +49,7 @@ func seedPackageWithPURL(t *testing.T, db *database.DB, store *mockStorage, ecos 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: sha256Hex(content), 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}, diff --git a/internal/handler/handler.go b/internal/handler/handler.go index cfbe5e0..6c65682 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -194,6 +194,11 @@ func (p *Proxy) checkCache(ctx context.Context, pkgPURL, versionPURL, filename s if artifact == nil { return nil, nil } + checks, err := newIntegrityChecks(artifact.ContentHash.String, artifact.Integrity.String) + if err != nil { + p.rejectUnusableCacheRecord(artifact, versionPURL, filename, err) + return nil, nil + } result := &CacheResult{ Size: artifact.Size.Int64, @@ -225,16 +230,21 @@ 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, err = checks.wrap(reader, func(reason string) { p.Logger.Error("cached artifact failed integrity check", "purl", versionPURL, "filename", filename, "path", artifact.StoragePath, "reason", reason) - metrics.RecordIntegrityFailure(artifact.Ecosystem) + metrics.RecordIntegrityFailure(purl.NormalizeEcosystem(artifact.Ecosystem)) if err := p.DB.ClearArtifactCache(versionPURL, filename); err != nil { p.Logger.Warn("failed to clear corrupt artifact from cache", "error", err) } }) + if err != nil { + _ = reader.Close() + p.rejectUnusableCacheRecord(artifact, versionPURL, filename, err) + return nil, nil + } p.recordCacheHit(artifact.Ecosystem, versionPURL, filename) return result, nil } @@ -264,6 +274,16 @@ func (p *Proxy) recordCacheHit(ecosystem, versionPURL, filename string) { metrics.RecordCacheHit(ecosystem) } +func (p *Proxy) rejectUnusableCacheRecord(artifact *database.CachedArtifact, versionPURL, filename string, cause error) { + p.Logger.Warn("cached artifact has unusable integrity metadata", + "purl", versionPURL, "filename", filename, + "path", artifact.StoragePath, "error", cause) + metrics.RecordIntegrityFailure(purl.NormalizeEcosystem(artifact.Ecosystem)) + if err := p.DB.ClearArtifactCache(versionPURL, filename); err != nil { + p.Logger.Warn("failed to clear unusable artifact from cache", "error", err) + } +} + func (p *Proxy) fetchAndCache(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL string) (*CacheResult, error) { // Resolve download URL info, err := p.Resolver.Resolve(ctx, ecosystem, name, version) diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go index d8660d7..ec0e300 100644 --- a/internal/handler/handler_test.go +++ b/internal/handler/handler_test.go @@ -3,9 +3,7 @@ package handler import ( "bytes" "context" - "crypto/sha256" "database/sql" - "encoding/hex" "errors" "io" "log/slog" @@ -46,8 +44,7 @@ func (s *mockStorage) Store(_ context.Context, path string, r io.Reader) (int64, return 0, "", err } s.files[path] = data - digest := sha256.Sum256(data) - return int64(len(data)), hex.EncodeToString(digest[:]), nil + return int64(len(data)), sha256Hex(string(data)), nil } func (s *mockStorage) Open(_ context.Context, path string) (io.ReadCloser, error) { @@ -182,7 +179,7 @@ func seedPackage(t testing.TB, db *database.DB, store *mockStorage, ecosystem, n 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: sha256Hex(content), 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}, @@ -271,8 +268,74 @@ func TestGetOrFetchArtifact_CacheHit(t *testing.T) { if result.ContentType != "application/octet-stream" { t.Errorf("got content type %q, want %q", result.ContentType, "application/octet-stream") } - if result.Hash != "abc123" { - t.Errorf("got hash %q, want %q", result.Hash, "abc123") + if result.Hash != sha256Hex("cached content") { + t.Errorf("got hash %q, want %q", result.Hash, sha256Hex("cached content")) + } +} + +func TestGetCachedArtifactRejectsMalformedIntegrityMetadata(t *testing.T) { + tests := []struct { + name string + malformedHash string + malformedIntegrity string + }{ + {name: "content hash", malformedHash: "abc123"}, + {name: "native integrity", malformedIntegrity: "sha512-abc123"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assertMalformedCacheRejected(t, test.malformedHash, test.malformedIntegrity) + }) + } +} + +func assertMalformedCacheRejected(t *testing.T, malformedHash, malformedIntegrity string) { + t.Helper() + proxy, db, store, _ := setupTestProxy(t) + const ( + packageName = "broken" + version = "1.0.0" + filename = "broken-1.0.0.tgz" + ) + seedPackage(t, db, store, "npm", packageName, version, filename, "cached content") + versionPURL := purl.MakePURLString("npm", packageName, version) + + if malformedHash != "" { + artifact, err := db.GetArtifact(versionPURL, filename) + if err != nil { + t.Fatal(err) + } + artifact.ContentHash = sql.NullString{String: malformedHash, Valid: true} + if err := db.UpsertArtifact(artifact); err != nil { + t.Fatal(err) + } + } + if malformedIntegrity != "" { + versionRecord := &database.Version{ + PURL: versionPURL, + PackagePURL: purl.MakePURLString("npm", packageName, ""), + Integrity: sql.NullString{String: malformedIntegrity, Valid: true}, + } + if err := db.UpsertVersion(versionRecord); err != nil { + t.Fatal(err) + } + } + + proxy.DirectServe = true + store.signedURL = "https://cache.example/broken" + result, err := proxy.GetCachedArtifact(context.Background(), "npm", packageName, version, filename) + if err != nil { + t.Fatalf("GetCachedArtifact: %v", err) + } + if result != nil { + t.Errorf("GetCachedArtifact = %+v, want nil", result) + } + artifact, err := db.GetArtifact(versionPURL, filename) + if err != nil { + t.Fatal(err) + } + if artifact.StoragePath.Valid { + t.Error("unusable cache record retained its storage path") } } @@ -307,7 +370,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: sha256Hex("missing content"), 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}, diff --git a/internal/handler/integrity.go b/internal/handler/integrity.go index bb29a21..07963b9 100644 --- a/internal/handler/integrity.go +++ b/internal/handler/integrity.go @@ -1,140 +1,100 @@ package handler import ( - "crypto/sha256" - "crypto/sha512" - "crypto/subtle" - "encoding/base64" - "encoding/hex" "fmt" - "hash" "io" - "strings" + + "github.com/git-pkgs/integrity" ) -// parseSRI parses a Subresource Integrity string (e.g. "sha512-abc==") into -// an algorithm name and raw digest bytes. Returns ok=false for empty, -// malformed, or unsupported entries. Only the first hash in a multi-hash -// SRI string is considered. -func parseSRI(s string) (algo string, digest []byte, ok bool) { - s = strings.TrimSpace(s) - if s == "" { - return "", nil, false - } - if i := strings.IndexByte(s, ' '); i >= 0 { - s = s[:i] - } - algo, b64, found := strings.Cut(s, "-") - if !found { - return "", nil, false - } - d, err := base64.StdEncoding.DecodeString(b64) - if err != nil { - return "", nil, false +type integrityChecks struct { + contentHash integrity.SRI + native integrity.SRI + algorithms []integrity.Algorithm +} + +func newIntegrityChecks(contentHash, native string) (integrityChecks, error) { + checks := integrityChecks{} + + if contentHash != "" { + digest, err := integrity.ParseHex(integrity.SHA256, contentHash) + if err != nil { + return integrityChecks{}, fmt.Errorf("parse content_hash: %w", err) + } + checks.contentHash = integrity.SRI{digest} + checks.algorithms = append(checks.algorithms, integrity.SHA256) } - switch algo { - case "sha256", "sha384", "sha512": - return algo, d, true - default: - return "", nil, false + + if native != "" { + digests, err := integrity.ParseSRI(native) + if err != nil { + return integrityChecks{}, fmt.Errorf("parse integrity: %w", err) + } + checks.native = digests + for _, digest := range digests { + checks.algorithms = append(checks.algorithms, digest.Algorithm()) + } } + + return checks, nil } -func newSRIHash(algo string) hash.Hash { - switch algo { - case "sha256": - return sha256.New() - case "sha384": - return sha512.New384() - case "sha512": - return sha512.New() +func (c integrityChecks) wrap(source io.ReadCloser, onMismatch func(string)) (io.ReadCloser, error) { + if len(c.algorithms) == 0 { + return source, nil + } + reader, err := integrity.NewReader(source, c.algorithms...) + if err != nil { + return nil, fmt.Errorf("create integrity reader: %w", err) } - return nil + return &verifyingReader{ + source: source, + reader: reader, + checks: c, + onMismatch: onMismatch, + }, nil } -// verifyingReader wraps an io.ReadCloser and computes SHA256 (and optionally -// a second SRI hash) as bytes are read. When the underlying reader reaches -// EOF it compares the digests against the expected values and calls -// onMismatch for each failure. Verification is skipped if the stream was -// not fully consumed (e.g. client disconnect) to avoid false positives. +// verifyingReader forwards Close to its source and reports completed digest +// mismatches after its shared integrity reader observes EOF. type verifyingReader struct { - r io.ReadCloser - sha256 hash.Hash - wantSHA256 string - sri hash.Hash - sriAlgo string - wantSRI []byte + source io.ReadCloser + reader *integrity.Reader + checks integrityChecks onMismatch func(reason string) - eof bool verified bool } -func newVerifyingReader(r io.ReadCloser, contentHash, sri string, onMismatch func(string)) io.ReadCloser { - if contentHash == "" && sri == "" { - return r - } - v := &verifyingReader{ - r: r, - onMismatch: onMismatch, - } - if contentHash != "" { - v.sha256 = sha256.New() - v.wantSHA256 = contentHash - } - if algo, digest, ok := parseSRI(sri); ok { - v.sri = newSRIHash(algo) - v.sriAlgo = algo - v.wantSRI = digest - } - if v.sha256 == nil && v.sri == nil { - return r - } - return v -} - -func (v *verifyingReader) Read(p []byte) (int, error) { - n, err := v.r.Read(p) - if n > 0 { - if v.sha256 != nil { - v.sha256.Write(p[:n]) - } - if v.sri != nil { - v.sri.Write(p[:n]) - } - } +func (r *verifyingReader) Read(p []byte) (int, error) { + n, err := r.reader.Read(p) if err == io.EOF { - v.eof = true - v.verify() + r.verify() } return n, err } -func (v *verifyingReader) Close() error { - if v.eof { - v.verify() - } - return v.r.Close() +func (r *verifyingReader) Close() error { + return r.source.Close() } -func (v *verifyingReader) verify() { - if v.verified { +func (r *verifyingReader) verify() { + if r.verified { + return + } + r.verified = true + result := r.reader.Result() + if !result.Complete { return } - v.verified = true - if v.sha256 != nil { - got := hex.EncodeToString(v.sha256.Sum(nil)) - if subtle.ConstantTimeCompare([]byte(got), []byte(v.wantSHA256)) != 1 { - v.onMismatch(fmt.Sprintf("content_hash mismatch: stored=%s computed=%s", v.wantSHA256, got)) + if len(r.checks.contentHash) > 0 { + if err := result.Verify(r.checks.contentHash); err != nil { + r.onMismatch("content_hash: " + err.Error()) } } - if v.sri != nil { - got := v.sri.Sum(nil) - if subtle.ConstantTimeCompare(got, v.wantSRI) != 1 { - v.onMismatch(fmt.Sprintf("integrity mismatch: %s expected=%s computed=%s", - v.sriAlgo, - base64.StdEncoding.EncodeToString(v.wantSRI), - base64.StdEncoding.EncodeToString(got))) + if len(r.checks.native) > 0 { + if err := result.Verify(r.checks.native); err != nil { + r.onMismatch("integrity: " + err.Error()) } } } diff --git a/internal/handler/integrity_test.go b/internal/handler/integrity_test.go index 93c448c..95992c0 100644 --- a/internal/handler/integrity_test.go +++ b/internal/handler/integrity_test.go @@ -5,6 +5,7 @@ import ( "crypto/sha512" "encoding/base64" "encoding/hex" + "errors" "io" "strings" "testing" @@ -15,42 +16,68 @@ func sha256Hex(data string) string { return hex.EncodeToString(sum[:]) } +func sha256SRI(data string) string { + sum := sha256.Sum256([]byte(data)) + return "sha256-" + base64.StdEncoding.EncodeToString(sum[:]) +} + +func sha384SRI(data string) string { + sum := sha512.Sum384([]byte(data)) + return "sha384-" + base64.StdEncoding.EncodeToString(sum[:]) +} + func sha512SRI(data string) string { sum := sha512.Sum512([]byte(data)) return "sha512-" + base64.StdEncoding.EncodeToString(sum[:]) } -func TestParseSRI(t *testing.T) { +func wrapIntegrityReader(t *testing.T, source io.ReadCloser, contentHash, native string, onMismatch func(string)) io.ReadCloser { + t.Helper() + checks, err := newIntegrityChecks(contentHash, native) + if err != nil { + t.Fatalf("newIntegrityChecks: %v", err) + } + reader, err := checks.wrap(source, onMismatch) + if err != nil { + t.Fatalf("wrap: %v", err) + } + return reader +} + +func TestNewIntegrityChecksCollectsAlgorithms(t *testing.T) { + checks, err := newIntegrityChecks( + sha256Hex("hello"), + strings.Join([]string{sha256SRI("first"), sha512SRI("second"), sha384SRI("third"), sha512SRI("alternative")}, " "), + ) + if err != nil { + t.Fatal(err) + } + if len(checks.algorithms) != 5 { + t.Fatalf("algorithms = %v, want 5 entries", checks.algorithms) + } + if len(checks.native) != 4 { + t.Errorf("native digests = %d, want 4", len(checks.native)) + } +} + +func TestNewIntegrityChecksRejectsMalformedMetadata(t *testing.T) { tests := []struct { - name string - input string - algo string - ok bool + name string + contentHash string + native string }{ - {"sha512", sha512SRI("hello"), "sha512", true}, - {"sha256", "sha256-" + base64.StdEncoding.EncodeToString([]byte("0123456789012345678901234567890123456789")), "sha256", true}, - {"empty", "", "", false}, - {"no dash", "sha512abc", "", false}, - {"bad base64", "sha512-not!base64", "", false}, - {"unsupported algo", "md5-" + base64.StdEncoding.EncodeToString([]byte("x")), "", false}, - {"multi hash takes first", sha512SRI("a") + " " + sha512SRI("b"), "sha512", true}, - {"whitespace", " " + sha512SRI("x") + " ", "sha512", true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - algo, digest, ok := parseSRI(tt.input) - if ok != tt.ok { - t.Fatalf("ok = %v, want %v", ok, tt.ok) - } - if !tt.ok { - return - } - if algo != tt.algo { - t.Errorf("algo = %q, want %q", algo, tt.algo) - } - if len(digest) == 0 { - t.Error("digest is empty") + {name: "short content hash", contentHash: "abc123"}, + {name: "non-hex content hash", contentHash: strings.Repeat("z", sha256.Size*2)}, + {name: "missing SRI separator", native: "sha512"}, + {name: "malformed SRI base64", native: "sha512-not!base64"}, + {name: "wrong SRI length", native: "sha512-" + base64.StdEncoding.EncodeToString([]byte("short"))}, + {name: "unsupported SRI algorithm", native: "md5-1B2M2Y8AsgTpgAmY7PhCfg=="}, + {name: "invalid SRI alternative", native: sha512SRI("valid") + " sha384-nope"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if _, err := newIntegrityChecks(test.contentHash, test.native); err == nil { + t.Fatal("newIntegrityChecks returned nil error") } }) } @@ -67,69 +94,156 @@ func TestVerifyingReader(t *testing.T) { sri string wantCalls int }{ - {"both match", goodSHA, goodSRI, 0}, - {"sha256 only match", goodSHA, "", 0}, - {"sri only match", "", goodSRI, 0}, - {"sha256 mismatch", sha256Hex("other"), "", 1}, - {"sri mismatch", "", sha512SRI("other"), 1}, - {"both mismatch", sha256Hex("other"), sha512SRI("other"), 2}, - {"no checks", "", "", 0}, - {"unparseable sri ignored", goodSHA, "garbage", 0}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + {name: "both match", hash: goodSHA, sri: goodSRI}, + {name: "SHA-256 only match", hash: goodSHA}, + {name: "SRI only match", sri: goodSRI}, + {name: "SHA-256 mismatch", hash: sha256Hex("other"), wantCalls: 1}, + {name: "SRI mismatch", sri: sha512SRI("other"), wantCalls: 1}, + {name: "both mismatch", hash: sha256Hex("other"), sri: sha512SRI("other"), wantCalls: 2}, + {name: "no checks"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { var calls []string - r := newVerifyingReader(io.NopCloser(strings.NewReader(data)), tt.hash, tt.sri, + reader := wrapIntegrityReader(t, io.NopCloser(strings.NewReader(data)), test.hash, test.sri, func(reason string) { calls = append(calls, reason) }) - got, err := io.ReadAll(r) + got, err := io.ReadAll(reader) if err != nil { t.Fatalf("ReadAll: %v", err) } if string(got) != data { t.Errorf("data corrupted: got %q", got) } - if err := r.Close(); err != nil { + if err := reader.Close(); err != nil { t.Fatalf("Close: %v", err) } + if len(calls) != test.wantCalls { + t.Errorf("onMismatch called %d times, want %d: %v", len(calls), test.wantCalls, calls) + } + }) + } +} - if len(calls) != tt.wantCalls { - t.Errorf("onMismatch called %d times, want %d: %v", len(calls), tt.wantCalls, calls) +func TestVerifyingReaderUsesStrongestNativeAlgorithm(t *testing.T) { + const data = "artifact" + tests := []struct { + name string + native string + wantCalls int + }{ + { + name: "weaker match does not override stronger mismatch", + native: sha256SRI(data) + " " + sha512SRI("other"), + wantCalls: 1, + }, + { + name: "stronger match ignores weaker mismatch", + native: sha256SRI("other") + " " + sha512SRI(data), + }, + { + name: "same algorithm alternative matches", + native: sha512SRI("other") + " " + sha512SRI(data), + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var calls int + reader := wrapIntegrityReader(t, io.NopCloser(strings.NewReader(data)), "", test.native, func(string) { calls++ }) + if _, err := io.Copy(io.Discard, reader); err != nil { + t.Fatal(err) + } + if calls != test.wantCalls { + t.Errorf("onMismatch called %d times, want %d", calls, test.wantCalls) } }) } } +func TestVerifyingReaderMismatchMessages(t *testing.T) { + const data = "actual" + wantHash := sha256Hex("expected") + wantSRI := sha512SRI("expected") + var reasons []string + reader := wrapIntegrityReader(t, io.NopCloser(strings.NewReader(data)), wantHash, wantSRI, + func(reason string) { reasons = append(reasons, reason) }) + if _, err := io.Copy(io.Discard, reader); err != nil { + t.Fatal(err) + } + if len(reasons) != 2 { + t.Fatalf("reasons = %v, want two", reasons) + } + wantContentReason := "content_hash: integrity mismatch: expected " + sha256SRI("expected") + ", calculated " + sha256SRI(data) + if reasons[0] != wantContentReason { + t.Errorf("content reason = %q, want %q", reasons[0], wantContentReason) + } + wantNativeReason := "integrity: integrity mismatch: expected " + wantSRI + ", calculated " + sha512SRI(data) + if reasons[1] != wantNativeReason { + t.Errorf("native reason = %q, want %q", reasons[1], wantNativeReason) + } +} + func TestVerifyingReaderPassthrough(t *testing.T) { - src := io.NopCloser(strings.NewReader("x")) - r := newVerifyingReader(src, "", "", func(string) { t.Fatal("should not be called") }) - if r != src { - t.Error("expected passthrough when no hashes provided") + source := io.NopCloser(strings.NewReader("x")) + reader := wrapIntegrityReader(t, source, "", "", func(string) { t.Fatal("should not be called") }) + if reader != source { + t.Error("expected passthrough when no hashes were provided") } } +type closeTrackingReader struct { + io.Reader + closed bool +} + +func (r *closeTrackingReader) Close() error { + r.closed = true + return nil +} + func TestVerifyingReaderPartialRead(t *testing.T) { + source := &closeTrackingReader{Reader: strings.NewReader("hello world")} var calls int - r := newVerifyingReader(io.NopCloser(strings.NewReader("hello world")), - sha256Hex("hello world"), "", func(string) { calls++ }) + reader := wrapIntegrityReader(t, source, sha256Hex("other"), "", func(string) { calls++ }) - buf := make([]byte, 5) - _, _ = r.Read(buf) - _ = r.Close() + buffer := make([]byte, 5) + _, _ = reader.Read(buffer) + _ = reader.Close() if calls != 0 { t.Errorf("onMismatch called %d times for partial read, want 0", calls) } + if !source.closed { + t.Error("Close was not forwarded to the source") + } +} + +func TestVerifyingReaderNonEOFError(t *testing.T) { + var calls int + reader := wrapIntegrityReader(t, io.NopCloser(errorFixtureReader{}), sha256Hex("data"), "", func(string) { calls++ }) + if _, err := io.ReadAll(reader); !errors.Is(err, errIntegrityReadFixture) { + t.Fatalf("ReadAll error = %v", err) + } + if calls != 0 { + t.Errorf("onMismatch called %d times after non-EOF error", calls) + } +} + +var errIntegrityReadFixture = errors.New("integrity read fixture") + +type errorFixtureReader struct{} + +func (errorFixtureReader) Read(p []byte) (int, error) { + return copy(p, "data"), errIntegrityReadFixture } func TestVerifyingReaderVerifyOnce(t *testing.T) { var calls int - r := newVerifyingReader(io.NopCloser(strings.NewReader("x")), sha256Hex("y"), "", - func(string) { calls++ }) - _, _ = io.ReadAll(r) - _ = r.Close() - _ = r.Close() + reader := wrapIntegrityReader(t, io.NopCloser(strings.NewReader("x")), sha256Hex("y"), "", func(string) { calls++ }) + _, _ = io.ReadAll(reader) + _ = reader.Close() + _ = reader.Close() if calls != 1 { t.Errorf("onMismatch called %d times, want 1", calls) }