Verify npm and Composer archives against their SHA-1 shasum

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/git-pkgs/cooldown v0.2.0
 	github.com/git-pkgs/enrichment v0.7.2
 	github.com/git-pkgs/gcs v0.1.0
-	github.com/git-pkgs/integrity v0.1.1
+	github.com/git-pkgs/integrity v0.2.0
 	github.com/git-pkgs/magic v0.5.0
 	github.com/git-pkgs/purl v0.1.21
 	github.com/git-pkgs/registries v0.9.3

go.sum

@@ -135,8 +135,8 @@ github.com/git-pkgs/enrichment v0.7.2 h1:flD/M89HxkOegUi6tC8EOspFiIMsl7m0o576IQK
 github.com/git-pkgs/enrichment v0.7.2/go.mod h1:xGyc/iz7u1UnbrOHk8lIX5ZaNF3mKFDEZc3VqBJ/LOk=
 github.com/git-pkgs/gcs v0.1.0 h1:E3awGtsO0xZyHT9FUfEwMHjMkRxw41Bh+c7lgTmPvBo=
 github.com/git-pkgs/gcs v0.1.0/go.mod h1:bdkCFD66ryaWnU8MBhokVA3WkJfyAEchm9qec5woBpE=
-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/integrity v0.2.0 h1:sxpAk7EUllZnTYttOoPTW/Yfe5J2imglpuJiuBUZOxM=
+github.com/git-pkgs/integrity v0.2.0/go.mod h1:vPLb2RB+hR/iCMIgqph0PZbQ7iktXDTK+uupoHQikKk=
 github.com/git-pkgs/magic v0.5.0 h1:Pm+4fkHR+K6kg9pwJM4hnwWUuBlkYdLmF1yIAI8lU9Q=
 github.com/git-pkgs/magic v0.5.0/go.mod h1:SXOqcsNmbmpZjJZHEEWnwxprbsFvpwWgTAZrgXp4Jm4=
 github.com/git-pkgs/pom v0.1.8 h1:mXCuWw6XkrKKZb6IurRZyIznYLPhV5AYg5ZO38Uj/l8=

internal/handler/composer.go

@@ -711,7 +711,8 @@ func (h *ComposerHandler) handleDownload(w http.ResponseWriter, r *http.Request)
 	if commit := strings.TrimSuffix(filename, path.Ext(filename)); isDevVersion(version) && composerCommitPattern.MatchString(commit) {
 		h.discardOtherCommit(packageName, version, cacheFilename, commit)
 	}
-	result, err := h.proxy.GetOrFetchArtifactFromURL(r.Context(), "composer", packageName, version, cacheFilename, downloadURL)
+	result, err := h.proxy.GetOrFetchArtifactFromURLWithIntegrity(r.Context(), "composer", packageName, version, cacheFilename, downloadURL,
+		composerIntegrity(dist.shasum, version, downloadURL == listedURL))
 	if err != nil {
 		h.proxy.serveArtifactError(w, err, "failed to fetch package")
 		return
@@ -776,10 +777,11 @@ func (h *ComposerHandler) metadataURLsForVersion(vendor, pkg, version string) []
 }
 
 // composerDist is what a version's metadata says about its archive: the dist
-// URL and type, and the raw publish time the cooldown is checked against.
+// URL, type and shasum, and the raw publish time the cooldown is checked
+// against.
 type composerDist struct {
-	url, distType string
-	published     []byte
+	url, distType, shasum string
+	published             []byte
 }
 
 // findDownloadURLFromMetadata fetches a metadata document and returns the dist
@@ -862,6 +864,7 @@ func composerDistURL(body []byte, packageName, version string) (composerDist, er
 		dist.url, _, _ = lookupJSONString(distRaw, "url")
 		if dist.url != "" {
 			dist.distType, _, _ = lookupJSONString(distRaw, "type")
+			dist.shasum, _, _ = lookupJSONString(distRaw, "shasum")
 			dist.published = fields.get("time")
 			return errStopScan
 		}
@@ -873,6 +876,22 @@ func composerDistURL(body []byte, packageName, version string) (composerDist, er
 	return dist, err
 }
 
+// composerIntegrity returns the integrity to verify an archive of version
+// against: the SHA-1 its dist lists as shasum. A dev branch is left
+// unverified: its listed shasum describes only the current head, while every
+// commit of the branch is cached under the same version, which holds a single
+// recorded integrity, so a cache hit for an older pinned commit would be
+// checked against the head's shasum. Composer checks dist.shasum itself
+// anyway. A shasum also only describes the listed archive, and Packagist
+// lists none for GitHub archives; an unusable one is ignored. Those archives
+// are served unverified as before.
+func composerIntegrity(shasum, version string, listed bool) string {
+	if !listed || isDevVersion(version) {
+		return ""
+	}
+	return sha1Checksum(shasum)
+}
+
 // proxyUpstream forwards a request to packagist.org without caching.
 func (h *ComposerHandler) proxyUpstream(w http.ResponseWriter, r *http.Request) {
 	upstreamURL := h.upstreamURL + r.URL.Path

internal/handler/composer_shasum_test.go

@@ -0,0 +1,144 @@
+package handler
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"testing"
+
+	"github.com/git-pkgs/registries/fetch"
+)
+
+func TestComposerDownloadVerifiesDeclaredShasum(t *testing.T) {
+	const archive = "archive bytes"
+
+	tests := []struct {
+		name       string
+		shasum     string
+		wantStatus int
+	}{
+		{"matching shasum", sha1Hex(archive), http.StatusOK},
+		{"mismatched shasum", sha1Hex("other bytes"), http.StatusBadGateway},
+		{"empty shasum", "", http.StatusOK},
+		{"unparseable shasum", "not-a-sha1", http.StatusOK},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			h, proxy := setupComposerShasum(t, "1.0.0", "/zip/p.zip", tt.shasum, map[string]string{"/zip/p.zip": archive})
+
+			w := httptest.NewRecorder()
+			h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/files/v/p/1.0.0/p.zip", nil))
+			if w.Code != tt.wantStatus {
+				t.Fatalf("status = %d, want %d; body: %s", w.Code, tt.wantStatus, w.Body.String())
+			}
+			if wantCached := tt.wantStatus == http.StatusOK; isComposerArchiveCached(t, proxy, "1.0.0", "p.zip") != wantCached {
+				t.Errorf("cached = %v, want %v", !wantCached, wantCached)
+			}
+		})
+	}
+}
+
+// TestComposerDownloadPinnedCommitIgnoresListedShasum checks that the shasum
+// listed for a branch's head is not held against an older commit a lock file
+// pins: it describes another archive.
+func TestComposerDownloadPinnedCommitIgnoresListedShasum(t *testing.T) {
+	h, _ := setupComposerShasum(t, "dev-master", "/zip/"+composerCommitB, sha1Hex("commit B"), map[string]string{
+		"/zip/" + composerCommitA: "commit A",
+		"/zip/" + composerCommitB: "commit B",
+	})
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/files/v/p/dev-master/"+composerCommitA+".zip", nil))
+	if w.Code != http.StatusOK || w.Body.String() != "commit A" {
+		t.Errorf("status %d, body %q, want 200 %q", w.Code, w.Body.String(), "commit A")
+	}
+}
+
+func setupComposerShasum(t *testing.T, version, distPath, shasum string, archives map[string]string) (http.Handler, *Proxy) {
+	t.Helper()
+	h, proxy, _ := setupComposerShasumCounting(t, version, distPath, shasum, archives)
+	return h, proxy
+}
+
+// setupComposerShasumCounting is setupComposerShasum that also reports how
+// many times each archive path was fetched from upstream.
+func setupComposerShasumCounting(t *testing.T, version, distPath, shasum string, archives map[string]string) (http.Handler, *Proxy, func(string) int) {
+	t.Helper()
+	var mu sync.Mutex
+	fetched := map[string]int{}
+	var srv *httptest.Server
+	srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/p2/v/p.json" || r.URL.Path == "/p2/v/p~dev.json" {
+			_, _ = fmt.Fprintf(w, `{"packages":{"v/p":[{"version":%q,"dist":{"url":%q,"type":"zip","shasum":%q}}]}}`,
+				version, srv.URL+distPath, shasum)
+			return
+		}
+		body, ok := archives[r.URL.Path]
+		if !ok {
+			http.NotFound(w, r)
+			return
+		}
+		mu.Lock()
+		fetched[r.URL.Path]++
+		mu.Unlock()
+		_, _ = io.WriteString(w, body)
+	}))
+	t.Cleanup(srv.Close)
+
+	proxy, _, _, _ := setupTestProxy(t)
+	fetcher := fetch.NewFetcher(fetch.WithHTTPClient(srv.Client()), fetch.WithMaxRetries(0))
+	proxy.Fetcher = fetcher
+	t.Cleanup(func() { _ = fetcher.Close() })
+	return NewComposerHandlerWithUpstreams(proxy, "http://proxy", srv.URL, srv.URL).Routes(), proxy, func(path string) int {
+		mu.Lock()
+		defer mu.Unlock()
+		return fetched[path]
+	}
+}
+
+func isComposerArchiveCached(t *testing.T, proxy *Proxy, version, filename string) bool {
+	t.Helper()
+	cached, err := proxy.GetCachedArtifact(context.Background(), "composer", "v/p", version, filename)
+	if err != nil {
+		t.Fatalf("reading cache: %v", err)
+	}
+	if cached == nil {
+		return false
+	}
+	_ = cached.Reader.Close()
+	return true
+}
+
+// TestComposerDevBranchArchivesStayServableFromCache is a regression test: a
+// version's integrity is one value shared by all its cached archives, but a
+// dev branch caches one archive per commit. Recording the head's shasum made
+// a pinned older commit fail its check when served from the cache, so its
+// archive was discarded and fetched again, which in turn cleared the head's
+// recorded shasum.
+func TestComposerDevBranchArchivesStayServableFromCache(t *testing.T) {
+	h, proxy, fetched := setupComposerShasumCounting(t, "dev-master", "/zip/"+composerCommitB, sha1Hex("commit B"), map[string]string{
+		"/zip/" + composerCommitA: "commit A",
+		"/zip/" + composerCommitB: "commit B",
+	})
+
+	for i, commit := range []string{composerCommitA, composerCommitB, composerCommitA, composerCommitB} {
+		want := map[string]string{composerCommitA: "commit A", composerCommitB: "commit B"}[commit]
+		w := httptest.NewRecorder()
+		h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/files/v/p/dev-master/"+commit+".zip", nil))
+		if w.Code != http.StatusOK || w.Body.String() != want {
+			t.Errorf("request %d for %s: status %d, body %q; want 200 %q", i+1, commit[:4], w.Code, w.Body.String(), want)
+		}
+	}
+	for _, commit := range []string{composerCommitA, composerCommitB} {
+		if n := fetched("/zip/" + commit); n != 1 {
+			t.Errorf("commit %s fetched from upstream %d times, want once and then served from the cache", commit[:4], n)
+		}
+		if !isComposerArchiveCached(t, proxy, "dev-master", commit+".zip") {
+			t.Errorf("commit %s is no longer cached", commit[:4])
+		}
+	}
+}

internal/handler/handler.go

@@ -1808,10 +1808,11 @@ func (p *Proxy) GetOrFetchArtifactFromURLWithDigest(ctx context.Context, ecosyst
 }
 
 // GetOrFetchArtifactFromURLWithIntegrity retrieves an artifact and verifies a
-// newly fetched response against declared, the Subresource Integrity string
-// the registry's metadata declares, before serving or caching it. declared is
-// recorded with the cached version, so cache hits are verified against it as
-// well. An empty declared fetches without verification.
+// newly fetched response against declared, the integrity the registry's
+// metadata declares, before serving or caching it: a Subresource Integrity
+// string, or a single algorithm-prefixed hex checksum such as "sha1-<hex>".
+// declared is recorded with the cached version, so cache hits are verified
+// against it as well. An empty declared fetches without verification.
 func (p *Proxy) GetOrFetchArtifactFromURLWithIntegrity(ctx context.Context, ecosystem, name, version, filename, downloadURL, declared string) (*CacheResult, error) {
 	return p.getOrFetchArtifactFromURL(ctx, ecosystem, name, version, filename, downloadURL, nil, "", declared)
 }

internal/handler/integrity.go

@@ -10,6 +10,10 @@ import (
 type integrityChecks struct {
 	contentHash integrity.SRI
 	native      integrity.SRI
+	// checksum is native metadata given as a single algorithm-prefixed hex
+	// checksum, such as a registry's SHA-1 shasum, which is not SRI.
+	checksum    integrity.Digest
+	hasChecksum bool
 	algorithms  []integrity.Algorithm
 }
 
@@ -28,7 +32,13 @@ func newIntegrityChecks(contentHash, native string) (integrityChecks, error) {
 	if native != "" {
 		digests, err := integrity.ParseSRI(native)
 		if err != nil {
-			return integrityChecks{}, fmt.Errorf("parse integrity: %w", err)
+			checksum, checksumErr := integrity.ParseChecksum(native)
+			if checksumErr != nil {
+				return integrityChecks{}, fmt.Errorf("parse integrity: %w", err)
+			}
+			checks.checksum, checks.hasChecksum = checksum, true
+			checks.algorithms = append(checks.algorithms, checksum.Algorithm())
+			return checks, nil
 		}
 		checks.native = digests
 		for _, digest := range digests {
@@ -189,4 +199,24 @@ func (r *verifyingReader) verify() {
 			r.onMismatch("integrity: " + err.Error())
 		}
 	}
+	if r.checks.hasChecksum {
+		if err := result.VerifyDigest(r.checks.checksum); err != nil {
+			r.mismatched = true
+			r.onMismatch("integrity: " + err.Error())
+		}
+	}
+}
+
+// sha1Checksum returns a registry's hexadecimal SHA-1 shasum as the
+// "sha1-<hex>" checksum newIntegrityChecks accepts, or "" when it is empty or
+// not a SHA-1 digest.
+func sha1Checksum(shasum string) string {
+	if shasum == "" {
+		return ""
+	}
+	digest, err := integrity.ParseHex(integrity.SHA1, shasum)
+	if err != nil {
+		return ""
+	}
+	return digest.Checksum()
 }

internal/handler/integrity_test.go

@@ -8,6 +8,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"io"
+	"slices"
 	"strings"
 	"testing"
 )
@@ -84,6 +85,38 @@ func TestNewIntegrityChecksRejectsMalformedMetadata(t *testing.T) {
 	}
 }
 
+// TestIntegrityChecksVerifyRegistryChecksum checks native metadata given as
+// an algorithm-prefixed hex checksum, the form recorded for a registry's SHA-1
+// shasum, which is not SRI.
+func TestIntegrityChecksVerifyRegistryChecksum(t *testing.T) {
+	const data = "hello world"
+	checksum := "sha1-" + sha1Hex(data)
+	checks, err := newIntegrityChecks(sha256Hex(data), checksum)
+	if err != nil {
+		t.Fatalf("newIntegrityChecks(%q): %v", checksum, err)
+	}
+	for _, tt := range []struct {
+		name, body   string
+		wantMismatch bool
+	}{
+		{"matching", data, false},
+		{"mismatched", "other bytes", true},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			var reasons []string
+			reader, err := checks.wrapFailOnMismatch(io.NopCloser(strings.NewReader(tt.body)), func(reason string) { reasons = append(reasons, reason) })
+			if err != nil {
+				t.Fatal(err)
+			}
+			_, _ = io.ReadAll(reader)
+			gotNative := slices.ContainsFunc(reasons, func(r string) bool { return strings.HasPrefix(r, "integrity: ") })
+			if gotNative != tt.wantMismatch {
+				t.Errorf("checksum mismatch reported = %v, want %v (reasons %q)", gotNative, tt.wantMismatch, reasons)
+			}
+		})
+	}
+}
+
 func TestVerifyingReader(t *testing.T) {
 	const data = "hello world"
 	goodSHA := sha256Hex(data)

internal/handler/npm.go

@@ -16,6 +16,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/git-pkgs/integrity"
 	"github.com/git-pkgs/proxy/internal/retention"
 )
 
@@ -878,18 +879,21 @@ func (h *NPMHandler) getTarball(r *http.Request, registryURL, packageName, versi
 	return h.proxy.GetOrFetchArtifactFromURLWithIntegrity(r.Context(), "npm", packageName, version, cacheFilename, downloadURL, sri)
 }
 
-// npmVersionIntegrity returns versions[version].dist.integrity from a
-// packument when it is a Subresource Integrity string the proxy can verify,
-// or "" otherwise, in which case the tarball is fetched unverified as before.
+// npmVersionIntegrity returns the integrity a packument declares for a
+// version's tarball: dist.integrity when it is a Subresource Integrity string
+// the proxy can verify, or else the SHA-1 in dist.shasum, which is all
+// versions published before integrity existed carry, as "sha1-<hex>". It
+// returns "" when neither is usable, and the tarball is fetched unverified as
+// before.
 func npmVersionIntegrity(body []byte, version string) string {
 	sri, _, _ := lookupJSONString(body, "versions", version, "dist", "integrity")
-	if sri == "" {
-		return ""
-	}
-	if _, err := newIntegrityChecks("", sri); err != nil {
-		return ""
+	if sri != "" {
+		if _, err := integrity.ParseSRI(sri); err == nil {
+			return sri
+		}
 	}
-	return sri
+	shasum, _, _ := lookupJSONString(body, "versions", version, "dist", "shasum")
+	return sha1Checksum(shasum)
 }
 
 // npmVersionTarball returns versions[version].dist.tarball from a packument,

internal/handler/npm_integrity_test.go

@@ -27,7 +27,11 @@ var npmIntegrityCases = []struct {
 	{"strongest algorithm decides", map[string]string{"integrity": sha256SRI("other bytes") + " " + sha512SRI(npmIntegrityTarball)}, true},
 	{"any digest of the strongest algorithm", map[string]string{"integrity": sha512SRI("other bytes") + " " + sha512SRI(npmIntegrityTarball)}, true},
 	{"unparseable integrity", map[string]string{"integrity": "sha512-nope"}, true},
-	{"shasum alone is not checked yet", map[string]string{"shasum": sha1Hex("other bytes")}, true},
+	{"matching shasum", map[string]string{"shasum": sha1Hex(npmIntegrityTarball)}, true},
+	{"mismatched shasum", map[string]string{"shasum": sha1Hex("other bytes")}, false},
+	{"integrity wins over shasum", map[string]string{"integrity": sha512SRI("other bytes"), "shasum": sha1Hex(npmIntegrityTarball)}, false},
+	{"unparseable integrity falls back to shasum", map[string]string{"integrity": "sha512-nope", "shasum": sha1Hex("other bytes")}, false},
+	{"unparseable shasum", map[string]string{"shasum": "not-a-sha1"}, true},
 	{"no checksum", map[string]string{}, true},
 }