From 20c8878dd924c5a108ff77476ca5c9f2e5920984 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Thu, 22 Oct 2026 20:44:02 +0200 Subject: [PATCH 1/2] Match denylist entries the way each ecosystem spells names Denylist entries were compared as plain PURL strings, but handlers build PURLs from download URLs that spell packages differently, so some denied versions were still served. Entries and checked PURLs now go through one per-ecosystem comparison form: PyPI names follow PEP 503 (case and runs of -, _ and . are equivalent), NuGet ids and versions ignore case, a gem version also matches its platform builds, and a conda entry matches the package in every channel unless it carries a channel qualifier. Cache keys are unchanged. --- docs/configuration.md | 15 ++++- internal/denylist/denylist.go | 98 +++++++++++++++++++++++++++--- internal/denylist/denylist_test.go | 40 +++++++++++- internal/handler/denylist_test.go | 56 +++++++++++++++++ 4 files changed, 196 insertions(+), 13 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 6f7be456..88d7470b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -969,11 +969,22 @@ denylist: - "pkg:npm/%40scope/example@4.5.6" ``` -Entries must be versioned PURLs without qualifiers or subpaths. Package names -are canonicalized (including PyPI case/separator normalization and npm scopes); +Entries must be versioned PURLs without subpaths or qualifiers (except a conda +`channel`, below). Package names are canonicalized (including npm scopes); versions match exactly, not by range or glob. Invalid entries prevent startup. An empty list disables the policy. Restart the proxy after changing the list. +Names and versions are compared the way each ecosystem treats them: + +- PyPI names ignore case, and runs of `-`, `_` and `.` are equivalent (PEP 503), + so `pkg:pypi/zope.interface@6.0` also covers `zope_interface-6.0-*.whl`. +- NuGet ids and versions ignore case. +- A RubyGems entry also covers that version's platform gems: + `pkg:gem/nokogiri@1.16.0` denies `nokogiri-1.16.0-x86_64-linux.gem`. +- A conda entry covers the package in every channel. Add a `channel` qualifier, + such as `pkg:conda/numpy@1.26.4?channel=conda-forge`, to deny it in one + channel only. + Denied versions are removed from npm packuments, Cargo sparse indexes, and PyPI simple HTML/JSON and release metadata, including responses served from cached metadata during an upstream outage. No publication timestamp or enabled cooldown diff --git a/internal/denylist/denylist.go b/internal/denylist/denylist.go index c4384ad4..32374965 100644 --- a/internal/denylist/denylist.go +++ b/internal/denylist/denylist.go @@ -3,6 +3,7 @@ package denylist import ( "fmt" + "regexp" "strings" "github.com/git-pkgs/purl" @@ -14,7 +15,8 @@ type Policy struct { } // New validates and canonicalizes versioned PURLs. Qualifiers and subpaths are -// rejected because the proxy cannot reliably distinguish them at every endpoint. +// rejected because the proxy cannot reliably distinguish them at every endpoint, +// except a conda channel, which download URLs always name. func New(packages []string) (*Policy, error) { if len(packages) == 0 { return nil, nil @@ -25,19 +27,32 @@ func New(packages []string) (*Policy, error) { if err != nil { return nil, fmt.Errorf("invalid denylist package %q: %w", value, err) } - if pkg.Version == "" || len(pkg.Qualifiers) != 0 || pkg.Subpath != "" || + if pkg.Version == "" || !allowedQualifiers(pkg) || pkg.Subpath != "" || strings.ContainsAny(pkg.Namespace+pkg.Name+pkg.Version, "*?[]<>=| \t\r\n") { return nil, fmt.Errorf("invalid denylist package %q: expected an exact versioned PURL without qualifiers or subpath", value) } - key := pkg.WithoutVersion().String() + id := identify(pkg) + key := id.key + if id.channel != "" { + key = id.channelKey() + } if p.packages[key] == nil { p.packages[key] = make(map[string]bool) } - p.packages[key][pkg.Version] = true + p.packages[key][id.version] = true } return p, nil } +func allowedQualifiers(pkg *purl.PURL) bool { + for _, q := range pkg.Qualifiers { + if pkg.Type != "conda" || q.Key != "channel" { + return false + } + } + return true +} + // Denied reports whether a versioned PURL is denied. func (p *Policy) Denied(value string) bool { if p == nil || len(p.packages) == 0 { @@ -47,21 +62,84 @@ func (p *Policy) Denied(value string) bool { if err != nil { return false } - return p.packages[pkg.WithoutVersion().String()][pkg.Version] + id := identify(pkg) + versions := []string{id.version} + // A platform gem's version carries the platform ("1.16.0-x86_64-linux"), + // so also match the plain version. + if pkg.Type == "gem" { + if base, _, found := strings.Cut(id.version, "-"); found && base != "" { + versions = append(versions, base) + } + } + for _, version := range versions { + if p.packages[id.key][version] || (id.channel != "" && p.packages[id.channelKey()][version]) { + return true + } + } + return false } // Versions returns a caller-owned set of denied versions for a package PURL. func (p *Policy) Versions(packagePURL string) map[string]bool { - if p == nil { + if p == nil || len(p.packages) == 0 { return nil } - versions := p.packages[packagePURL] - if len(versions) == 0 { + pkg, err := purl.Parse(packagePURL) + if err != nil { return nil } - result := make(map[string]bool, len(versions)) - for version := range versions { + id := identify(pkg) + result := make(map[string]bool) + for version := range p.packages[id.key] { result[version] = true } + if id.channel != "" { + for version := range p.packages[id.channelKey()] { + result[version] = true + } + } + if len(result) == 0 { + return nil + } return result } + +// identity is the form a PURL is compared in: the same for denylist entries +// and for the PURLs handlers build, however each spells the package. +type identity struct { + key string // versionless PURL without channel + channel string // conda channel, if any + version string +} + +func (id identity) channelKey() string { + return id.key + "?channel=" + id.channel +} + +var pypiSeparators = regexp.MustCompile(`[-_.]+`) + +func identify(pkg *purl.PURL) identity { + namespace, name, version := pkg.Namespace, pkg.Name, pkg.Version + var channel string + switch pkg.Type { + case "pypi": + // PEP 503: case-insensitive, runs of -, _ and . are equivalent. + name = pypiSeparators.ReplaceAllString(strings.ToLower(name), "-") + case "nuget": + // NuGet ids and versions are case-insensitive. + name, version = strings.ToLower(name), strings.ToLower(version) + case "conda": + // The conda handler names packages "channel/name"; entries may give the + // channel as a qualifier or namespace. An entry without one matches + // every channel. + channel = pkg.Qualifier("channel") + if namespace != "" { + channel, namespace = namespace, "" + } + if i := strings.LastIndex(name, "/"); i >= 0 { + channel, name = name[:i], name[i+1:] + } + } + key := purl.New(pkg.Type, namespace, name, "", nil).String() + return identity{key: key, channel: channel, version: version} +} diff --git a/internal/denylist/denylist_test.go b/internal/denylist/denylist_test.go index 380f1ee6..ba74ddb7 100644 --- a/internal/denylist/denylist_test.go +++ b/internal/denylist/denylist_test.go @@ -29,7 +29,7 @@ func TestPolicy(t *testing.T) { } func TestInvalidPolicy(t *testing.T) { - for _, value := range []string{"", "not-a-purl", "pkg:npm/foo", "pkg:npm/foo@*", "pkg:npm/foo@>=1", "pkg:npm/foo@1?arch=x86", "pkg:npm/foo@1#src"} { + for _, value := range []string{"", "not-a-purl", "pkg:npm/foo", "pkg:npm/foo@*", "pkg:npm/foo@>=1", "pkg:npm/foo@1?arch=x86", "pkg:npm/foo@1#src", "pkg:conda/numpy@1?subdir=linux-64", "pkg:pypi/foo@1?channel=x"} { t.Run(value, func(t *testing.T) { if _, err := New([]string{value}); err == nil { t.Fatal("invalid denylist accepted") @@ -37,3 +37,41 @@ func TestInvalidPolicy(t *testing.T) { }) } } + +func TestEcosystemSpellings(t *testing.T) { + p, err := New([]string{ + "pkg:pypi/zope.interface@6.0", + "pkg:nuget/Newtonsoft.Json@13.0.1-Beta", + "pkg:gem/nokogiri@1.16.0", + "pkg:gem/sorbet-static@0.5.1-universal-darwin-22", + "pkg:conda/numpy@1.26.4", + "pkg:conda/scipy@1.0?channel=conda-forge", + "pkg:conda/bioconda/samtools@1.0", + }) + if err != nil { + t.Fatal(err) + } + for _, value := range []string{ + "pkg:pypi/zope-interface@6.0", "pkg:pypi/Zope__Interface@6.0", + "pkg:nuget/newtonsoft.json@13.0.1-beta", + "pkg:gem/nokogiri@1.16.0", "pkg:gem/nokogiri@1.16.0-x86_64-linux", + "pkg:gem/sorbet-static@0.5.1-universal-darwin-22", + "pkg:conda/numpy@1.26.4", "pkg:conda/conda-forge%2Fnumpy@1.26.4", + "pkg:conda/conda-forge%2Fscipy@1.0", "pkg:conda/bioconda%2Fsamtools@1.0", + } { + if !p.Denied(value) { + t.Errorf("not denied: %s", value) + } + } + for _, value := range []string{ + "pkg:pypi/zopeinterface@6.0", "pkg:gem/nokogiri@1.16.1-x86_64-linux", "pkg:gem/Nokogiri@1.16.0", + "pkg:conda/bioconda%2Fscipy@1.0", "pkg:conda/scipy@1.0", "pkg:conda/conda-forge%2Fsamtools@1.0", + } { + if p.Denied(value) { + t.Errorf("unexpected denial: %s", value) + } + } + if !p.Versions("pkg:pypi/zope_interface")["6.0"] { + t.Error("pypi versions not found under another spelling") + } +} diff --git a/internal/handler/denylist_test.go b/internal/handler/denylist_test.go index e08d662f..e55e36f5 100644 --- a/internal/handler/denylist_test.go +++ b/internal/handler/denylist_test.go @@ -252,3 +252,59 @@ func TestDenylistMalformedMetadataFailsClosed(t *testing.T) { }) } } + +// Denylist entries must match however the ecosystem spells the name or +// version in download URLs. +func TestDenylistMatchesEcosystemSpellings(t *testing.T) { + pypi := func(p *Proxy) http.Handler { + return NewPyPIHandlerWithUpstreams(p, "http://proxy.test", "", "").Routes() + } + for _, tc := range []struct { + name, entry, path string + ecosystem, pkg string + version, filename string + handler func(*Proxy) http.Handler + }{ + {"pypi wheel", "pkg:pypi/zope.interface@6.0", "/packages/a/b/zope_interface-6.0-py3-none-any.whl", + "pypi", "zope_interface", "6.0", "zope_interface-6.0-py3-none-any.whl", pypi}, + {"pypi dotted sdist", "pkg:pypi/zope-interface@6.0", "/packages/a/b/zope.interface-6.0.tar.gz", + "pypi", "zope.interface", "6.0", "zope.interface-6.0.tar.gz", pypi}, + {"nuget", "pkg:nuget/Newtonsoft.Json@13.0.1-Beta", "/v3-flatcontainer/newtonsoft.json/13.0.1-beta/newtonsoft.json.13.0.1-beta.nupkg", + "nuget", "newtonsoft.json", "13.0.1-beta", "newtonsoft.json.13.0.1-beta.nupkg", + func(p *Proxy) http.Handler { return NewNuGetHandler(p, "http://proxy.test").Routes() }}, + {"gem platform", "pkg:gem/nokogiri@1.16.0", "/gems/nokogiri-1.16.0-x86_64-linux.gem", + "gem", "nokogiri", "1.16.0-x86_64-linux", "nokogiri-1.16.0-x86_64-linux.gem", + func(p *Proxy) http.Handler { return NewGemHandler(p, "http://proxy.test").Routes() }}, + {"conda any channel", "pkg:conda/numpy@1.26.4", "/conda-forge/linux-64/numpy-1.26.4-py311_0.conda", + "conda", "conda-forge/numpy", "1.26.4", "numpy-1.26.4-py311_0.conda", + func(p *Proxy) http.Handler { return NewCondaHandler(p, "http://proxy.test").Routes() }}, + {"conda channel qualifier", "pkg:conda/numpy@1.26.4?channel=conda-forge", "/conda-forge/linux-64/numpy-1.26.4-py311_0.conda", + "conda", "conda-forge/numpy", "1.26.4", "numpy-1.26.4-py311_0.conda", + func(p *Proxy) http.Handler { return NewCondaHandler(p, "http://proxy.test").Routes() }}, + } { + t.Run(tc.name, func(t *testing.T) { + // The seeded artifact would be served if the entry didn't match. + p, db, store, _ := setupTestProxy(t) + seedPackage(t, db, store, tc.ecosystem, tc.pkg, tc.version, tc.filename, "cached content") + setTestDenylist(t, p, tc.entry) + w := httptest.NewRecorder() + tc.handler(p).ServeHTTP(w, httptest.NewRequest(http.MethodGet, tc.path, nil)) + if w.Code != http.StatusForbidden { + t.Fatalf("denied download status=%d body=%s", w.Code, w.Body.String()) + } + }) + } +} + +func TestPyPIDenylistSimpleMatchesSeparators(t *testing.T) { + body := `xy` + h, p := setupPyPIHandler(t, func(r *http.Request) (*http.Response, error) { + return pypiHTTPResponse(r, "text/html", body), nil + }) + setTestDenylist(t, p, "pkg:pypi/zope.interface@6.0") + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/simple/zope-interface/", nil)) + if w.Code != http.StatusOK || strings.Contains(w.Body.String(), "6.0") || !strings.Contains(w.Body.String(), "7.0") { + t.Fatalf("status=%d body=%s", w.Code, w.Body.String()) + } +} From e1650e202d929b5768d586c4016e6dc1de33da04 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Fri, 23 Oct 2026 09:55:06 +0200 Subject: [PATCH 2/2] Match more denylist spellings Denylist entries now also match Composer versions with Packagist's leading v, OCI repositories written as a path, Swift registry packages in either form (also when streaming), Go versions with escaped uppercase letters, and gems whose names contain a dash followed by a digit. OCI manifests are checked by tag and digest, and denied blobs and Swift archives answer 403 instead of 500 or 502. --- docs/configuration.md | 15 ++++ internal/denylist/denylist.go | 35 ++++++++ internal/denylist/denylist_test.go | 15 ++++ internal/handler/container.go | 11 ++- internal/handler/container_manifest.go | 42 +++++++-- internal/handler/denylist_test.go | 113 ++++++++++++++++++++++++- internal/handler/gem.go | 39 +++++++-- internal/handler/gem_test.go | 6 ++ internal/handler/go.go | 9 ++ internal/handler/handler.go | 7 +- internal/handler/swift.go | 4 + 11 files changed, 277 insertions(+), 19 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 88d7470b..2d122ccb 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -981,6 +981,21 @@ Names and versions are compared the way each ecosystem treats them: - NuGet ids and versions ignore case. - A RubyGems entry also covers that version's platform gems: `pkg:gem/nokogiri@1.16.0` denies `nokogiri-1.16.0-x86_64-linux.gem`. +- Composer versions ignore a leading `v`, which Packagist keeps from the tag: + `pkg:composer/symfony/console@6.1.7` also covers `v6.1.7`. +- Go versions are matched as written, `pkg:golang/example.com/mod@v1.0.0-RC1`, + though download URLs escape uppercase letters (`v1.0.0-!r!c1`). +- A Swift entry names the registry scope and package, ignoring case: + `pkg:swift/apple/swift-nio@2.62.0`. The stored form, + `pkg:generic/swift-registry/apple.swift-nio@2.62.0`, works too. +- An OCI entry names the repository as it is pulled through the proxy and a + digest or tag: `pkg:oci/library/alpine@sha256:` refuses the blob or + manifest with that digest, including a manifest a tag resolves to, and + `pkg:oci/library/alpine@3.19` refuses pulling that tag. Repositories on a + named upstream keep their prefix, as in `pkg:oci/upstream/ghcr/org/image@...`. + An entry covers only the digest it names: denying an image index does not + deny the platform manifests and layers it lists to a client that requests + them by digest directly. - A conda entry covers the package in every channel. Add a `channel` qualifier, such as `pkg:conda/numpy@1.26.4?channel=conda-forge`, to deny it in one channel only. diff --git a/internal/denylist/denylist.go b/internal/denylist/denylist.go index 32374965..77b076aa 100644 --- a/internal/denylist/denylist.go +++ b/internal/denylist/denylist.go @@ -118,6 +118,20 @@ func (id identity) channelKey() string { var pypiSeparators = regexp.MustCompile(`[-_.]+`) +// swiftRegistryIdentity returns the registry scope and name of a Swift +// package. A source coordinate such as pkg:swift/github.com/apple/swift-nio +// names no registry identity. +func swiftRegistryIdentity(pkg *purl.PURL) (scope, name string, ok bool) { + switch { + case pkg.Type == "swift" && pkg.Namespace != "" && !strings.ContainsAny(pkg.Namespace, "./"): + return pkg.Namespace, pkg.Name, true + case pkg.Type == "generic" && strings.EqualFold(pkg.Namespace, "swift-registry"): + scope, name, found := strings.Cut(pkg.Name, ".") + return scope, name, found && scope != "" && name != "" + } + return "", "", false +} + func identify(pkg *purl.PURL) identity { namespace, name, version := pkg.Namespace, pkg.Name, pkg.Version var channel string @@ -139,6 +153,27 @@ func identify(pkg *purl.PURL) identity { if i := strings.LastIndex(name, "/"); i >= 0 { channel, name = name[:i], name[i+1:] } + case "composer": + // Packagist keeps a tag's leading v ("v6.1.7"); Composer treats it + // as the same version. + if len(version) > 1 && (version[0] == 'v' || version[0] == 'V') && version[1] >= '0' && version[1] <= '9' { + version = version[1:] + } + case "oci": + // Handlers name the repository as one name, library%2Falpine; + // entries write it as a path, library/alpine. + if namespace != "" { + name, namespace = namespace+"/"+name, "" + } + case "swift", "generic": + // Swift registry archives are stored as + // pkg:generic/swift-registry/{scope}.{name}; entries may write + // pkg:swift/{scope}/{name}. Registry identities ignore case. + if scope, pkgName, ok := swiftRegistryIdentity(pkg); ok { + namespace, name = strings.ToLower(scope), strings.ToLower(pkgName) + key := purl.New("swift", namespace, name, "", nil).String() + return identity{key: key, version: version} + } } key := purl.New(pkg.Type, namespace, name, "", nil).String() return identity{key: key, channel: channel, version: version} diff --git a/internal/denylist/denylist_test.go b/internal/denylist/denylist_test.go index ba74ddb7..a1106088 100644 --- a/internal/denylist/denylist_test.go +++ b/internal/denylist/denylist_test.go @@ -47,6 +47,12 @@ func TestEcosystemSpellings(t *testing.T) { "pkg:conda/numpy@1.26.4", "pkg:conda/scipy@1.0?channel=conda-forge", "pkg:conda/bioconda/samtools@1.0", + "pkg:composer/symfony/console@6.1.7", + "pkg:composer/monolog/monolog@v3.5.0", + "pkg:oci/library/alpine@sha256:abc", + "pkg:oci/upstream%2Fghcr%2Forg%2Fimage@sha256:def", + "pkg:swift/Apple/Swift-NIO@2.0.0", + "pkg:generic/swift-registry/apple.example@1.0.0", }) if err != nil { t.Fatal(err) @@ -58,6 +64,12 @@ func TestEcosystemSpellings(t *testing.T) { "pkg:gem/sorbet-static@0.5.1-universal-darwin-22", "pkg:conda/numpy@1.26.4", "pkg:conda/conda-forge%2Fnumpy@1.26.4", "pkg:conda/conda-forge%2Fscipy@1.0", "pkg:conda/bioconda%2Fsamtools@1.0", + "pkg:composer/symfony/console@v6.1.7", "pkg:composer/symfony/console@V6.1.7", + "pkg:composer/monolog/monolog@3.5.0", + "pkg:oci/library%2Falpine@sha256:abc", "pkg:oci/library/alpine@sha256:abc", + "pkg:oci/upstream/ghcr/org/image@sha256:def", + "pkg:generic/swift-registry/apple.swift-nio@2.0.0", "pkg:swift/apple/swift-nio@2.0.0", + "pkg:swift/apple/example@1.0.0", } { if !p.Denied(value) { t.Errorf("not denied: %s", value) @@ -66,6 +78,9 @@ func TestEcosystemSpellings(t *testing.T) { for _, value := range []string{ "pkg:pypi/zopeinterface@6.0", "pkg:gem/nokogiri@1.16.1-x86_64-linux", "pkg:gem/Nokogiri@1.16.0", "pkg:conda/bioconda%2Fscipy@1.0", "pkg:conda/scipy@1.0", "pkg:conda/conda-forge%2Fsamtools@1.0", + "pkg:composer/symfony/console@6.1.70", "pkg:composer/symfony/console@vv6.1.7", "pkg:npm/demo@v6.1.7", + "pkg:oci/alpine@sha256:abc", "pkg:oci/library%2Falpine@sha256:abd", + "pkg:generic/other/apple.swift-nio@2.0.0", "pkg:swift/github.com/apple/swift-nio@2.0.0", } { if p.Denied(value) { t.Errorf("unexpected denial: %s", value) diff --git a/internal/handler/container.go b/internal/handler/container.go index 1d906050..57c30d84 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -397,6 +397,10 @@ func (h *ContainerHandler) handleBlobDownload(w http.ResponseWriter, r *http.Req filename := digest cached, err := h.proxy.GetCachedArtifact(r.Context(), "oci", cacheName, digest, filename) + if errors.Is(err, ErrVersionDenied) { + h.containerError(w, http.StatusForbidden, "DENIED", err.Error()) + return + } if err != nil { h.proxy.Logger.Error("failed to check blob cache", "error", err) h.containerError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to check blob cache") @@ -471,14 +475,17 @@ func (h *ContainerHandler) handleManifest(w http.ResponseWriter, r *http.Request return } - registryURL, upstreamName, _, ok := h.registryForRequest(r, name) + registryURL, upstreamName, cacheName, ok := h.registryForRequest(r, name) if !ok { h.containerError(w, http.StatusNotFound, "NAME_UNKNOWN", "unknown upstream registry") return } h.proxy.Logger.Info("container manifest request", "name", upstreamName, "reference", reference) - h.serveManifest(w, r, registryURL, upstreamName, reference) + if h.manifestDenied(w, cacheName, reference) { + return + } + h.serveManifest(w, r, registryURL, upstreamName, cacheName, reference) } // handleTagsList caches tag list responses for offline OCI pulls. diff --git a/internal/handler/container_manifest.go b/internal/handler/container_manifest.go index 62609da4..82eed454 100644 --- a/internal/handler/container_manifest.go +++ b/internal/handler/container_manifest.go @@ -49,14 +49,16 @@ type cachedContainerManifest struct { fetchedAt time.Time } -func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, registryURL, name, reference string) { +// serveManifest serves a manifest unless the denylist names the digest it +// resolves to under cacheName, the name blobs are stored under. +func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, registryURL, name, cacheName, reference string) { accept := containerManifestAccept(r) cacheAccept := normalizeContainerManifestAccept(accept) cached, fromFallback := h.loadContainerManifestForAccept(r.Context(), registryURL, name, reference, accept, cacheAccept) immutable := manifestDigestReferencePattern.MatchString(reference) if cached != nil && (immutable || h.containerManifestFresh(cached)) { - writeContainerManifest(w, r, cached, false) + h.writeManifest(w, r, cacheName, cached, false) return } @@ -73,7 +75,7 @@ func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, resp, err := h.proxy.HTTPClient.Do(req) if err != nil { - h.serveStaleManifestOrError(w, r, reference, cached, err) + h.serveStaleManifestOrError(w, r, cacheName, reference, cached, err) return } defer func() { _ = resp.Body.Close() }() @@ -87,12 +89,12 @@ func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, } else { h.storeContainerManifestForAccept(r.Context(), registryURL, name, reference, accept, cacheAccept, cached) } - writeContainerManifest(w, r, cached, false) + h.writeManifest(w, r, cacheName, cached, false) return } if resp.StatusCode != http.StatusOK { if shouldServeStaleManifest(resp.StatusCode) && (cached != nil || referrersTagPattern.MatchString(reference)) { - h.serveStaleManifestOrError(w, r, reference, cached, fmt.Errorf("upstream status %d", resp.StatusCode)) + h.serveStaleManifestOrError(w, r, cacheName, reference, cached, fmt.Errorf("upstream status %d", resp.StatusCode)) return } h.proxy.relayResponse(w, r, resp, copyContainerManifestHeaders) @@ -100,13 +102,16 @@ func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, } if r.Method == http.MethodHead { + if h.manifestDenied(w, cacheName, resp.Header.Get("Docker-Content-Digest")) { + return + } h.proxy.relayResponse(w, r, resp, copyContainerManifestHeaders) return } body, err := h.proxy.ReadMetadata(resp.Body) if err != nil { - h.serveStaleManifestOrError(w, r, reference, cached, fmt.Errorf("reading manifest: %w", err)) + h.serveStaleManifestOrError(w, r, cacheName, reference, cached, fmt.Errorf("reading manifest: %w", err)) return } computedDigest := sha256Digest(body) @@ -136,13 +141,13 @@ func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, if manifest.contentDigest != reference && manifestDigestReferencePattern.MatchString(manifest.contentDigest) { h.storeContainerManifestForAccept(r.Context(), registryURL, name, manifest.contentDigest, accept, cacheAccept, manifest) } - writeContainerManifest(w, r, manifest, false) + h.writeManifest(w, r, cacheName, manifest, false) } -func (h *ContainerHandler) serveStaleManifestOrError(w http.ResponseWriter, r *http.Request, reference string, cached *cachedContainerManifest, err error) { +func (h *ContainerHandler) serveStaleManifestOrError(w http.ResponseWriter, r *http.Request, cacheName, reference string, cached *cachedContainerManifest, err error) { if cached != nil { h.proxy.Logger.Warn("upstream manifest fetch failed, serving stale cache", "error", err) - writeContainerManifest(w, r, cached, true) + h.writeManifest(w, r, cacheName, cached, true) return } if referrersTagPattern.MatchString(reference) { @@ -157,6 +162,25 @@ func (h *ContainerHandler) serveStaleManifestOrError(w http.ResponseWriter, r *h h.containerError(w, http.StatusBadGateway, "INTERNAL_ERROR", "failed to fetch from upstream") } +// manifestDenied writes a 403 and reports true when the denylist names +// reference, a tag or digest, under the repository's cache name. +func (h *ContainerHandler) manifestDenied(w http.ResponseWriter, cacheName, reference string) bool { + if reference == "" || !h.proxy.versionDenied("oci", cacheName, reference) { + return false + } + h.containerError(w, http.StatusForbidden, "DENIED", + ErrVersionDenied.Error()+": "+canonicalVersionPURL("oci", cacheName, reference)) + return true +} + +// writeManifest writes manifest unless the denylist names its digest. +func (h *ContainerHandler) writeManifest(w http.ResponseWriter, r *http.Request, cacheName string, manifest *cachedContainerManifest, stale bool) { + if h.manifestDenied(w, cacheName, manifest.contentDigest) { + return + } + writeContainerManifest(w, r, manifest, stale) +} + func (h *ContainerHandler) containerManifestFresh(manifest *cachedContainerManifest) bool { return h.proxy.MetadataTTL > 0 && !manifest.fetchedAt.IsZero() && time.Since(manifest.fetchedAt) < h.proxy.MetadataTTL } diff --git a/internal/handler/denylist_test.go b/internal/handler/denylist_test.go index e55e36f5..8e924718 100644 --- a/internal/handler/denylist_test.go +++ b/internal/handler/denylist_test.go @@ -14,6 +14,7 @@ import ( "github.com/git-pkgs/cooldown" "github.com/git-pkgs/proxy/internal/denylist" + "github.com/git-pkgs/registries/fetch" ) func setTestDenylist(t testing.TB, p *Proxy, packages ...string) { @@ -281,10 +282,22 @@ func TestDenylistMatchesEcosystemSpellings(t *testing.T) { {"conda channel qualifier", "pkg:conda/numpy@1.26.4?channel=conda-forge", "/conda-forge/linux-64/numpy-1.26.4-py311_0.conda", "conda", "conda-forge/numpy", "1.26.4", "numpy-1.26.4-py311_0.conda", func(p *Proxy) http.Handler { return NewCondaHandler(p, "http://proxy.test").Routes() }}, + {"gem name with digit segment", "pkg:gem/devise-2fa@1.0.0", "/gems/devise-2fa-1.0.0.gem", + "gem", "devise-2fa", "1.0.0", "devise-2fa-1.0.0.gem", + func(p *Proxy) http.Handler { return NewGemHandler(p, "http://proxy.test").Routes() }}, + {"go case-escaped version", "pkg:golang/github.com/example/mod@v1.0.0-RC1", "/github.com/example/mod/@v/v1.0.0-!r!c1.zip", + "golang", "github.com/example/mod", "v1.0.0-!r!c1", "mod@v1.0.0-!r!c1.zip", + func(p *Proxy) http.Handler { return NewGoHandler(p, "http://proxy.test").Routes() }}, + {"oci repository path", "pkg:oci/library/alpine@" + ociTestDigest, "/library/alpine/blobs/" + ociTestDigest, + "oci", "library/alpine", ociTestDigest, ociTestDigest, + func(p *Proxy) http.Handler { + return NewContainerHandlerWithRegistry(p, "http://proxy.test", "https://registry.invalid").Routes() + }}, } { t.Run(tc.name, func(t *testing.T) { // The seeded artifact would be served if the entry didn't match. - p, db, store, _ := setupTestProxy(t) + p, db, store, fetcher := setupTestProxy(t) + fetcher.fetchErr = errors.New("offline") seedPackage(t, db, store, tc.ecosystem, tc.pkg, tc.version, tc.filename, "cached content") setTestDenylist(t, p, tc.entry) w := httptest.NewRecorder() @@ -296,6 +309,104 @@ func TestDenylistMatchesEcosystemSpellings(t *testing.T) { } } +const ociTestDigest = "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + +// Packagist keeps a tag's leading v in the version it lists, and so in the +// download URL the proxy hands out. +func TestComposerDenylistMatchesTagPrefix(t *testing.T) { + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/p2/symfony/console.json": + _, _ = fmt.Fprintf(w, `{"packages":{"symfony/console":[{"version":"v6.1.7","version_normalized":"6.1.7.0","dist":{"url":%q,"type":"zip"}}]}}`, + srv.URL+"/archives/console.zip") + case "/archives/console.zip": + _, _ = io.WriteString(w, "archive") + default: + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + p, _, _, _ := setupTestProxy(t) + p.Fetcher = fetch.NewFetcher(fetch.WithHTTPClient(srv.Client()), fetch.WithMaxRetries(0)) + setTestDenylist(t, p, "pkg:composer/symfony/console@6.1.7") + h := NewComposerHandlerWithUpstreams(p, "http://proxy.test", srv.URL, srv.URL).Routes() + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/files/symfony/console/v6.1.7/console.zip", nil)) + if w.Code != http.StatusForbidden { + t.Fatalf("status=%d body=%s", w.Code, w.Body.String()) + } +} + +// Swift archives are cached under pkg:generic/swift-registry/{scope}.{name}, +// which is also what streaming checks. +func TestSwiftDenylistBlocksArchive(t *testing.T) { + for _, stream := range []bool{false, true} { + for _, entry := range []string{"pkg:swift/Apple/Example@1.2.3", "pkg:generic/swift-registry/apple.example@1.2.3"} { + t.Run(fmt.Sprintf("stream=%t/%s", stream, entry), func(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, `{"id":"apple.example","version":"1.2.3","resources":[{"name":"source-archive","type":"application/zip","checksum":"`+strings.Repeat("0", 64)+`"}]}`) + })) + t.Cleanup(upstream.Close) + p, _, _, fetcher := setupTestProxy(t) + fetcher.artifact = &fetch.Artifact{Body: io.NopCloser(strings.NewReader("archive")), Size: 7} + p.StreamArtifacts = stream + setTestDenylist(t, p, entry) + req := httptest.NewRequest(http.MethodGet, "/apple/example/1.2.3.zip", nil) + req.Header.Set("Accept", swiftAcceptArchive) + w := httptest.NewRecorder() + NewSwiftHandler(p, "http://proxy.test", upstream.URL).Routes().ServeHTTP(w, req) + if w.Code != http.StatusForbidden || fetcher.fetchCalled { + t.Fatalf("status=%d fetched=%t body=%s", w.Code, fetcher.fetchCalled, w.Body.String()) + } + }) + } + } +} + +// A denied digest is refused as a manifest as well as a blob, whether the +// client names it or a tag resolves to it. +func TestContainerDenylistBlocksManifests(t *testing.T) { + manifest := `{"schemaVersion":2}` + digest := sha256Digest([]byte(manifest)) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set(headerContentType, "application/vnd.oci.image.manifest.v1+json") + w.Header().Set("Docker-Content-Digest", digest) + _, _ = io.WriteString(w, manifest) + })) + t.Cleanup(upstream.Close) + for _, tc := range []struct{ entry, reference string }{ + {"pkg:oci/library/alpine@" + digest, digest}, + {"pkg:oci/library/alpine@" + digest, "3.19"}, + {"pkg:oci/library/alpine@3.19", "3.19"}, + } { + t.Run(tc.entry+"/"+tc.reference, func(t *testing.T) { + p := testProxy() + p.HTTPClient = upstream.Client() + h := NewContainerHandlerWithRegistry(p, "http://proxy.test", upstream.URL).Routes() + // Cache the manifest first, then deny it. + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/library/alpine/manifests/"+tc.reference, nil)) + if w.Code != http.StatusOK { + t.Fatalf("allowed status=%d", w.Code) + } + setTestDenylist(t, p, tc.entry) + for _, method := range []string{http.MethodGet, http.MethodHead} { + w = httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(method, "/library/alpine/manifests/"+tc.reference, nil)) + if w.Code != http.StatusForbidden { + t.Fatalf("%s status=%d body=%s", method, w.Code, w.Body.String()) + } + } + w = httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/library/alpine/manifests/other", nil)) + if strings.HasPrefix(tc.entry, "pkg:oci/library/alpine@3.19") && w.Code != http.StatusOK { + t.Fatalf("other tag status=%d", w.Code) + } + }) + } +} + func TestPyPIDenylistSimpleMatchesSeparators(t *testing.T) { body := `xy` h, p := setupPyPIHandler(t, func(r *http.Request) (*http.Response, error) { diff --git a/internal/handler/gem.go b/internal/handler/gem.go index 2d2136b2..e1acb129 100644 --- a/internal/handler/gem.go +++ b/internal/handler/gem.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "regexp" "strings" "time" @@ -88,19 +89,45 @@ func (h *GemHandler) handleDownload(w http.ResponseWriter, r *http.Request) { // e.g., "rails-7.1.0.gem" -> ("rails", "7.1.0") // e.g., "aws-sdk-s3-1.142.0.gem" -> ("aws-sdk-s3", "1.142.0") // e.g., "sorbet-static-0.5.11002-universal-darwin-22.gem" -> ("sorbet-static", "0.5.11002-universal-darwin-22") +// e.g., "devise-2fa-1.0.0.gem" -> ("devise-2fa", "1.0.0") // -// The version starts at the first hyphen followed by a digit. Platforms can -// end in a number (universal-java-17, arm64-darwin-23), so splitting at the -// last one would cut the platform; only gem names with a "-" segment -// are misread this way, and those are rare. +// A gem file is named {name}-{version}[-{platform}]. RubyGems writes a +// version as dot-separated alphanumeric segments starting with a digit, +// turning any "-" into ".pre.", and a platform starts with a letter (its +// CPU, or java), though it can end in a number (universal-java-17), so +// splitting at the last "-" would cut the platform. The version +// starts at the first "-" that leaves such a version and platform, +// so a name with a "-" segment of its own, such as devise-2fa, is +// kept whole. A filename no split fits is split at the first "-". func (h *GemHandler) parseGemFilename(filename string) (name, version string) { base := strings.TrimSuffix(filename, ".gem") + first := -1 for i := 0; i+1 < len(base); i++ { - if base[i] == '-' && base[i+1] >= '0' && base[i+1] <= '9' { + if base[i] != '-' || base[i+1] < '0' || base[i+1] > '9' { + continue + } + if first < 0 { + first = i + } + if gemVersionAndPlatform(base[i+1:]) { return base[:i], base[i+1:] } } - return "", "" + if first < 0 { + return "", "" + } + return base[:first], base[first+1:] +} + +var ( + gemVersionPattern = regexp.MustCompile(`^[0-9]+(\.[0-9A-Za-z]+)*$`) + gemPlatformPattern = regexp.MustCompile(`^[A-Za-z][0-9A-Za-z_]*(-[0-9A-Za-z_]+)*$`) +) + +// gemVersionAndPlatform reports whether s reads as {version}[-{platform}]. +func gemVersionAndPlatform(s string) bool { + version, platform, hasPlatform := strings.Cut(s, "-") + return gemVersionPattern.MatchString(version) && (!hasPlatform || gemPlatformPattern.MatchString(platform)) } // handleCompactIndex serves the compact index for a gem, filtering versions diff --git a/internal/handler/gem_test.go b/internal/handler/gem_test.go index ce63deb5..9b710748 100644 --- a/internal/handler/gem_test.go +++ b/internal/handler/gem_test.go @@ -30,6 +30,12 @@ func TestGemParseFilename(t *testing.T) { {"google-protobuf-3.25.3-arm64-darwin-23.gem", "google-protobuf", "3.25.3-arm64-darwin-23"}, {"nokogiri-1.16.0-x64-mingw-ucrt.gem", "nokogiri", "1.16.0-x64-mingw-ucrt"}, {"rake-13.1.0.pre.1.gem", "rake", "13.1.0.pre.1"}, + {"devise-2fa-1.0.0.gem", "devise-2fa", "1.0.0"}, + {"devise-2fa-1.0.0-java.gem", "devise-2fa", "1.0.0-java"}, + {"foo-2-1.0.0.gem", "foo-2", "1.0.0"}, + {"foo-2-1.gem", "foo-2", "1"}, + {"foo-2-x86_64-linux.gem", "foo", "2-x86_64-linux"}, + {"odd-1_0.gem", "odd", "1_0"}, {"no-version.gem", "", ""}, {"invalid", "", ""}, } diff --git a/internal/handler/go.go b/internal/handler/go.go index 7a1eb862..c8ae81ae 100644 --- a/internal/handler/go.go +++ b/internal/handler/go.go @@ -10,6 +10,7 @@ import ( "github.com/git-pkgs/proxy/internal/database" "github.com/git-pkgs/proxy/internal/retention" "github.com/git-pkgs/registries/fetch" + gomodule "golang.org/x/mod/module" ) const ( @@ -126,6 +127,14 @@ func (h *GoHandler) handleDownload(w http.ResponseWriter, r *http.Request, modul h.proxy.Logger.Info("go module download request", "module", decodedModule, "version", version) + // The URL escapes uppercase letters in the version (v1.0.0-!r!c1 for + // v1.0.0-RC1), and the zip is cached under that form, so the denylist + // is checked against the version itself. + if unescaped, err := gomodule.UnescapeVersion(version); err == nil && h.proxy.versionDenied("golang", decodedModule, unescaped) { + http.Error(w, ErrVersionDenied.Error()+": "+canonicalVersionPURL("golang", decodedModule, unescaped), http.StatusForbidden) + return + } + downloadURL := h.upstreamURL + r.URL.Path h.discardOtherModulesZip(decodedModule, module, version, filename) result, err := h.proxy.GetOrFetchArtifactFromURL( diff --git a/internal/handler/handler.go b/internal/handler/handler.go index ff5466a4..164e1619 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1924,10 +1924,15 @@ func (p *Proxy) getOrFetchArtifactFromURL(ctx context.Context, ecosystem, name, } func (p *Proxy) getOrFetchArtifactFromURLWithCachePURLs(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL, downloadURL string, headers http.Header, upstreamHash, declared string) (*CacheResult, error) { - // Some protocols use qualified cache keys. Check their package identity too. + // Some protocols use qualified cache keys. Check their package identity + // too, and the cache key itself for those, such as Swift, whose name has + // no PURL of its own. if p.versionDenied(ecosystem, name, version) { return nil, fmt.Errorf("%w: %s", ErrVersionDenied, canonicalVersionPURL(ecosystem, name, version)) } + if p.Denylist.Denied(versionPURL) { + return nil, fmt.Errorf("%w: %s", ErrVersionDenied, versionPURL) + } if p.StreamArtifacts { return p.streamFromUpstream(ctx, ecosystem, name, version, filename, versionPURL, downloadURL, upstreamHash, declared, func(fetchCtx context.Context) (*fetch.Artifact, error) { diff --git a/internal/handler/swift.go b/internal/handler/swift.go index b506f1c2..d7d403f5 100644 --- a/internal/handler/swift.go +++ b/internal/handler/swift.go @@ -628,6 +628,10 @@ func (h *SwiftHandler) writeArtifactError(w http.ResponseWriter, err error) { writeSwiftProblem(w, http.StatusNotFound, "release not found") return } + if errors.Is(err, ErrArtifactBlocked) || errors.Is(err, ErrVersionDenied) { + writeSwiftProblem(w, http.StatusForbidden, err.Error()) + return + } if writeUpstreamHeld(w, err) { return }