From b2553ddd49187fa9f3f6c243ced07b86121bd105 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Tue, 27 Oct 2026 16:15:31 +0100 Subject: [PATCH 1/3] Pass Composer security advisories and filter lists through The proxy's packages.json listed only metadata, search and notify URLs, so a client using the proxy as its only Composer repository lost `composer audit`, `audit.block-insecure` and Packagist's malware filter list without any warning. The service index now carries the security-advisories and filter entries of the upstream repository's own packages.json, cached for the metadata TTL, with URLs pointing at the proxy. An upstream that does not offer them gets nothing advertised, so a custom upstream.composer_repository without the API keeps working as before. Advisories and filter entries embedded in p2 metadata were already passed on unchanged. Advisories queries and the filter summary are relayed uncached. Names of routed packages and hosted-only vendors are removed from advisories queries first, so private package names are not sent to the public API. --- README.md | 11 + docs/configuration.md | 3 + internal/handler/composer.go | 14 +- internal/handler/composer_advisories.go | 268 ++++++++++++++++ internal/handler/composer_advisories_test.go | 308 +++++++++++++++++++ 5 files changed, 602 insertions(+), 2 deletions(-) create mode 100644 internal/handler/composer_advisories.go create mode 100644 internal/handler/composer_advisories_test.go diff --git a/README.md b/README.md index 20c6d5db..eb0db0e3 100644 --- a/README.md +++ b/README.md @@ -387,6 +387,15 @@ Or set globally: composer config -g repositories.proxy composer http://localhost:8080/composer ``` +`composer audit`, `audit.block-insecure` and Packagist's malware filter list +work through the proxy when `upstream.composer_repository` offers them in its +`packages.json`, as repo.packagist.org does. The proxy advertises them in its +own `packages.json` and passes the advisories and filter list requests through +to the upstream uncached. Names of packages in `upstream.composer_routes` or +`hosted.composer_vendors` are removed from advisories requests before they are +sent. Versions withheld by [cooldown](#version-cooldown) are not excluded from +the report. + To serve private packages through the same URL, see [Private package routes](docs/configuration.md#private-package-routes). ### Conan (C/C++) @@ -1263,6 +1272,8 @@ Recently cached: | `GET /maven/*` | Maven repository protocol | | `GET /nuget/*` | NuGet V3 API | | `GET /composer/*` | Composer/Packagist protocol | +| `GET`, `POST /composer/api/security-advisories/` | Composer security advisories, passed through to upstream without private package names | +| `GET /composer/lists/summary.json` | Composer filter list summary, passed through to upstream uncached | | `GET /conan/*` | Conan C/C++ protocol | | `GET /chef/*` | Chef Supermarket protocol (universe, cookbook API and downloads) | | `GET /conda/*` | Conda/Anaconda protocol | diff --git a/docs/configuration.md b/docs/configuration.md index f10e4b23..5497b91c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -339,6 +339,8 @@ upstream: composer_repository: "https://upstream-proxy.example.com/composer" ``` +The proxy reads `packages.json` from `upstream.composer_repository`, cached for the metadata TTL, and passes on its `security-advisories` and `filter` entries (Packagist's malware list) with URLs pointing at the proxy. An upstream that offers neither is still usable; `composer audit` then reports nothing from it. A filter `api-url` is not passed on; Composer reads filter entries from package metadata instead. + `upstream.hex_api` is used for cooldown timestamps and must expose Hex's `/api/packages/{name}` JSON endpoint. `upstream.chef` is the Supermarket served at `/chef/`, for Berkshelf (`source "https://proxy.example.com/chef"`) and Policyfiles (`default_source :supermarket, "https://proxy.example.com/chef"`). The upstream has to serve the Supermarket API: `/universe`, `/api/v1/cookbooks/{name}`, its versions and their downloads. A private Supermarket that needs credentials takes them from `upstream.auth`, scoped to its URL. A download redirect to another host, such as the storage bucket behind Supermarket, does not receive them: @@ -379,6 +381,7 @@ upstream: - Composer metadata of routed packages carries an empty `notification-url`, so Composer does not report their installs to the default upstream's `notify-batch` endpoint. - Use `upstream.auth` for the registry's credentials. The proxy has no authentication of its own, so every client that can reach it can download routed packages. - Search (`/composer/search.json`) and the package list (`/composer/packages/list.json`) still come from the default upstream only. +- Composer security advisories and filter lists come from the default upstream. Routed package names, and those in `hosted.composer_vendors`, are removed from advisories requests before they are sent, so they only get advisories that their route's own metadata carries. - npm audit, signing-key, search and ping requests still go to `upstream.npm`. An audit request lists every package in the dependency tree, routed ones included. - Routes can only be set in the configuration file, not with environment variables. diff --git a/internal/handler/composer.go b/internal/handler/composer.go index 116d106e..bf04da02 100644 --- a/internal/handler/composer.go +++ b/internal/handler/composer.go @@ -100,12 +100,19 @@ func (h *ComposerHandler) Routes() http.Handler { mux.HandleFunc("GET /search.json", h.proxyUpstream) mux.HandleFunc("GET /packages/list.json", h.proxyUpstream) + // Security advisories and filter lists, as the default repository offers them + mux.HandleFunc("GET "+composerAdvisoriesPath, h.handleSecurityAdvisories) + mux.HandleFunc("POST "+composerAdvisoriesPath, h.handleSecurityAdvisories) + mux.HandleFunc("GET "+composerFilterSummaryPath, h.handleFilterSummary) + return mux } -// handleServiceIndex returns the Composer repository service index. +// handleServiceIndex returns the Composer repository service index. It +// carries the security advisories and filter list entries of the default +// repository's own index when that has them; if the repository cannot be +// reached and nothing is cached, the index goes out without them. func (h *ComposerHandler) handleServiceIndex(w http.ResponseWriter, r *http.Request) { - // Return a minimal service index pointing to our proxy index := map[string]any{ "packages": map[string]any{}, "metadata-url": h.proxyURL + "/composer/p2/%package%.json", @@ -113,6 +120,9 @@ func (h *ComposerHandler) handleServiceIndex(w http.ResponseWriter, r *http.Requ "search": h.proxyURL + "/composer/search.json?q=%query%&type=%type%", "providers-lazy-url": h.proxyURL + "/composer/p2/%package%.json", } + if upstream, ok := h.upstreamIndex(r.Context()); ok { + upstream.advertise(index, h.proxyURL) + } w.Header().Set(headerContentType, "application/json") _ = json.NewEncoder(w).Encode(index) diff --git a/internal/handler/composer_advisories.go b/internal/handler/composer_advisories.go new file mode 100644 index 00000000..e77da373 --- /dev/null +++ b/internal/handler/composer_advisories.go @@ -0,0 +1,268 @@ +package handler + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/url" + "strings" +) + +const ( + composerAdvisoriesPath = "/api/security-advisories/" + composerFilterSummaryPath = "/lists/summary.json" + composerPackagesParam = "packages" + + // composerAdvisoriesMaxBody caps an advisories query. Composer sends at + // most 500 package names per request. + composerAdvisoriesMaxBody = 1 << 20 +) + +// composerUpstreamIndex holds the parts of the default repository's +// packages.json that the proxy advertises in its own: the security +// advisories API and the filter lists (Packagist's malware list). Composer +// reads advisories and filter entries for each package from its p2 metadata, +// which the proxy passes on unchanged, and asks these URLs for the rest. +type composerUpstreamIndex struct { + advisoriesURL string + advisoriesMetadata bool + filterLists json.RawMessage + filterSummaryURL string +} + +// upstreamIndex fetches the default repository's packages.json, cached for +// the metadata TTL. ok is false when the repository could not be reached and +// nothing is cached; a document without the fields yields an empty index. +func (h *ComposerHandler) upstreamIndex(ctx context.Context) (composerUpstreamIndex, bool) { + indexURL := h.repoURL + "/packages.json" + cacheKey := hashedCacheKey(h.repoURL, "/packages.json") + res := h.proxy.fetchMetadataDocument(ctx, "composer", cacheKey, indexURL, contentTypeJSON, nil) + if res.err != nil { + h.proxy.Logger.Warn("failed to fetch upstream composer packages.json", "url", indexURL, "error", res.err) + return composerUpstreamIndex{}, false + } + return parseComposerUpstreamIndex(indexURL, res.body), true +} + +func parseComposerUpstreamIndex(indexURL string, body []byte) composerUpstreamIndex { + var doc struct { + SecurityAdvisories struct { + Metadata bool `json:"metadata"` + APIURL string `json:"api-url"` + } `json:"security-advisories"` + Filter struct { + Metadata bool `json:"metadata"` + Lists json.RawMessage `json:"lists"` + SummaryURL string `json:"summary-url"` + } `json:"filter"` + } + var index composerUpstreamIndex + if err := json.Unmarshal(body, &doc); err != nil { + return index + } + // Composer refuses advisories configured without an api-url unless the + // repository lists its packages, which the proxy does not. + if apiURL, ok := resolveComposerURL(indexURL, doc.SecurityAdvisories.APIURL); ok { + index.advisoriesURL = apiURL + index.advisoriesMetadata = doc.SecurityAdvisories.Metadata + } + // Composer only applies filter lists a repository marks with metadata. + // A filter api-url is not passed on: without it Composer reads the + // entries from package metadata, narrowed by the summary. + if doc.Filter.Metadata && len(doc.Filter.Lists) > 0 && doc.Filter.Lists[0] == '{' { + index.filterLists = doc.Filter.Lists + index.filterSummaryURL, _ = resolveComposerURL(indexURL, doc.Filter.SummaryURL) + } + return index +} + +// resolveComposerURL resolves a URL from packages.json against the document's +// own, as Composer does, and accepts only http and https. +func resolveComposerURL(base, ref string) (string, bool) { + if ref == "" { + return "", false + } + b, err := url.Parse(base) + if err != nil { + return "", false + } + u, err := b.Parse(ref) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" { + return "", false + } + return u.String(), true +} + +// advertise adds the advisories and filter entries to the proxy's +// packages.json, pointing at the proxy. +func (index composerUpstreamIndex) advertise(packages map[string]any, proxyURL string) { + if index.advisoriesURL != "" { + packages["security-advisories"] = map[string]any{ + "metadata": index.advisoriesMetadata, + "api-url": proxyURL + "/composer" + composerAdvisoriesPath, + } + } + if index.filterLists != nil { + filter := map[string]any{"metadata": true, "lists": index.filterLists} + if index.filterSummaryURL != "" { + filter["summary-url"] = proxyURL + "/composer" + composerFilterSummaryPath + } + packages["filter"] = filter + } +} + +// handleSecurityAdvisories relays a security advisories query to the URL the +// default repository advertises. Composer sends the package names as +// packages[] in a POST form; a GET query is accepted too. Names served by a +// package route or a hosted-only vendor are removed first, so private package +// names are never sent to the public API. +// +// Responses are not cached: each query names a different set of packages. +func (h *ComposerHandler) handleSecurityAdvisories(w http.ResponseWriter, r *http.Request) { + query, err := h.advisoriesQuery(w, r) + if err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + JSONError(w, http.StatusRequestEntityTooLarge, "advisories request too large") + return + } + JSONError(w, http.StatusBadRequest, "could not read advisories request") + return + } + + asked := len(query[composerPackagesParam+"[]"]) + public := h.publicPackageNames(query[composerPackagesParam+"[]"]) + if asked > 0 && len(public) == 0 { + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = io.WriteString(w, `{"advisories":{}}`) + return + } + if asked > 0 { + query[composerPackagesParam+"[]"] = public + } + + index, ok := h.upstreamIndex(r.Context()) + if !ok { + JSONError(w, http.StatusBadGateway, "upstream repository unavailable") + return + } + if index.advisoriesURL == "" { + JSONError(w, http.StatusNotFound, "upstream repository has no security advisories API") + return + } + + req, err := h.advisoriesRequest(r, index.advisoriesURL, query) + if err != nil { + JSONError(w, http.StatusInternalServerError, "failed to create request") + return + } + h.proxy.Logger.Debug("composer advisories request", "packages", len(public), "withheld", asked-len(public)) + + resp, err := h.proxy.HTTPClient.Do(req) + if err != nil { + if writeUpstreamHeld(w, err) { + return + } + h.proxy.Logger.Error("composer advisories request failed", "error", err) + JSONError(w, http.StatusBadGateway, "failed to reach upstream repository") + return + } + defer func() { _ = resp.Body.Close() }() + + h.proxy.relayResponse(w, r, resp, nil) +} + +// advisoriesQuery reads the query of an advisories request, from the form +// body of a POST or the URL of a GET. Package names come back under +// "packages[]" whichever spelling the client used: PHP's http_build_query, +// which Composer uses, numbers them as packages[0], packages[1], ... +func (h *ComposerHandler) advisoriesQuery(w http.ResponseWriter, r *http.Request) (url.Values, error) { + raw := r.URL.RawQuery + if r.Method == http.MethodPost { + body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, composerAdvisoriesMaxBody)) + if err != nil { + return nil, err + } + raw = string(body) + } + values, err := url.ParseQuery(raw) + if err != nil { + return nil, err + } + query := url.Values{} + for key, vals := range values { + if key == composerPackagesParam || strings.HasPrefix(key, composerPackagesParam+"[") { + key = composerPackagesParam + "[]" + } + query[key] = append(query[key], vals...) + } + return query, nil +} + +// publicPackageNames drops the names the default repository does not serve. +func (h *ComposerHandler) publicPackageNames(names []string) []string { + public := make([]string, 0, len(names)) + for _, name := range names { + if _, routed := h.routes.match(name); routed || h.proxy.hostedOnly("composer", name) { + continue + } + public = append(public, name) + } + return public +} + +// advisoriesRequest builds the upstream request, sending the query the way +// the client did. +func (h *ComposerHandler) advisoriesRequest(r *http.Request, apiURL string, query url.Values) (*http.Request, error) { + u, err := url.Parse(apiURL) + if err != nil { + return nil, err + } + var body io.Reader + if r.Method == http.MethodPost { + body = strings.NewReader(query.Encode()) + } else { + u.RawQuery = joinQuery(u.RawQuery, query.Encode()) + } + req, err := http.NewRequestWithContext(r.Context(), r.Method, u.String(), body) + if err != nil { + return nil, err + } + if r.Method == http.MethodPost { + req.Header.Set(headerContentType, "application/x-www-form-urlencoded") + } + for _, header := range []string{headerAccept, headerAcceptEncoding} { + if v := r.Header.Get(header); v != "" { + req.Header.Set(header, v) + } + } + return req, nil +} + +func joinQuery(a, b string) string { + if a == "" || b == "" { + return a + b + } + return a + "&" + b +} + +// handleFilterSummary relays the filter list summary the default repository +// advertises. It lists every package on a filter list, so the request names +// no packages. It is not cached, so a newly listed package is blocked as soon +// as the upstream lists it; Composer revalidates its own copy with +// If-Modified-Since, which is passed on. +func (h *ComposerHandler) handleFilterSummary(w http.ResponseWriter, r *http.Request) { + index, ok := h.upstreamIndex(r.Context()) + if !ok { + JSONError(w, http.StatusBadGateway, "upstream repository unavailable") + return + } + if index.filterSummaryURL == "" { + JSONError(w, http.StatusNotFound, "upstream repository has no filter list summary") + return + } + h.proxy.ProxyUpstream(w, r, index.filterSummaryURL, + []string{headerAccept, headerAcceptEncoding, "If-Modified-Since", "If-None-Match"}) +} diff --git a/internal/handler/composer_advisories_test.go b/internal/handler/composer_advisories_test.go new file mode 100644 index 00000000..6adc7d24 --- /dev/null +++ b/internal/handler/composer_advisories_test.go @@ -0,0 +1,308 @@ +package handler + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "net/url" + "slices" + "strings" + "sync" + "testing" +) + +// composerAdvisoryUpstream serves a packages.json advertising security +// advisories and a malware filter list, and records the advisories and +// summary requests it gets. +type composerAdvisoryUpstream struct { + *httptest.Server + index string + + mu sync.Mutex + requests []*recordedRequest +} + +type recordedRequest struct { + method string + path string + query url.Values + body string + header http.Header +} + +const composerAdvisoriesResponse = `{"advisories":{"public/library":[{"advisoryId":"PKSA-1","affectedVersions":"<1.0.1"}]}}` + +const composerFilterSummary = `{"filter":{"malware":{"evil/package":"1.0.0"}}}` + +func newComposerAdvisoryUpstream(t *testing.T) *composerAdvisoryUpstream { + t.Helper() + u := &composerAdvisoryUpstream{} + u.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + u.mu.Lock() + u.requests = append(u.requests, &recordedRequest{r.Method, r.URL.Path, r.URL.Query(), string(body), r.Header.Clone()}) + u.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/packages.json": + _, _ = io.WriteString(w, u.index) + case "/api/security-advisories/": + _, _ = io.WriteString(w, composerAdvisoriesResponse) + case "/lists/all/summary.json": + if r.Header.Get("If-Modified-Since") != "" { + w.WriteHeader(http.StatusNotModified) + return + } + _, _ = io.WriteString(w, composerFilterSummary) + default: + http.NotFound(w, r) + } + })) + // Like Packagist's: the API on one host, the summary as a relative URL. + u.index = `{ + "packages": [], + "metadata-url": "/p2/%package%.json", + "security-advisories": {"metadata": true, "api-url": "` + u.URL + `/api/security-advisories/"}, + "filter": {"metadata": true, "lists": {"malware": {"enabled": true}}, "summary-url": "/lists/all/summary.json"} + }` + t.Cleanup(u.Close) + return u +} + +// requestsTo returns the recorded requests for path. +func (u *composerAdvisoryUpstream) requestsTo(path string) []*recordedRequest { + u.mu.Lock() + defer u.mu.Unlock() + var out []*recordedRequest + for _, r := range u.requests { + if r.path == path { + out = append(out, r) + } + } + return out +} + +func newAdvisoryComposerHandler(t *testing.T, upstream *composerAdvisoryUpstream) *ComposerHandler { + t.Helper() + proxy, _, _, _ := setupTestProxy(t) + proxy.HostedEnabled = true + proxy.HostedComposerVendors = []string{"acme"} + return NewComposerHandlerWithUpstreams(proxy, "http://proxy.test", upstream.URL, upstream.URL). + WithPackageRoutes(map[string]string{"example/*": "https://packages.example.com/composer"}) +} + +func serveComposerIndex(t *testing.T, h *ComposerHandler) map[string]any { + t.Helper() + w := serveComposer(h, "/packages.json") + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body) + } + var index map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &index); err != nil { + t.Fatalf("parsing packages.json: %v", err) + } + return index +} + +func TestComposerServiceIndexAdvertisesAdvisoriesAndFilter(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + index := serveComposerIndex(t, newAdvisoryComposerHandler(t, upstream)) + + advisories, _ := index["security-advisories"].(map[string]any) + if advisories["api-url"] != "http://proxy.test/composer/api/security-advisories/" || advisories["metadata"] != true { + t.Errorf("security-advisories = %v, want the proxy's api-url with metadata", index["security-advisories"]) + } + filter, _ := index["filter"].(map[string]any) + if filter["summary-url"] != "http://proxy.test/composer/lists/summary.json" || filter["metadata"] != true { + t.Errorf("filter = %v, want the proxy's summary-url with metadata", index["filter"]) + } + lists, _ := filter["lists"].(map[string]any) + if _, ok := lists["malware"]; !ok { + t.Errorf("filter lists = %v, want upstream's malware list", filter["lists"]) + } + if index["metadata-url"] != "http://proxy.test/composer/p2/%package%.json" { + t.Errorf("metadata-url = %v, want the proxy's", index["metadata-url"]) + } +} + +func TestComposerServiceIndexWithoutUpstreamSupport(t *testing.T) { + tests := []struct { + name string + index string + }{ + {"not advertised", `{"packages": [], "metadata-url": "/p2/%package%.json"}`}, + {"advisories without api-url", `{"security-advisories": {"metadata": true}}`}, + {"filter without metadata", `{"filter": {"lists": {"malware": {"enabled": true}}, "api-url": "/filter"}}`}, + {"not json", ``}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + upstream.index = tt.index + index := serveComposerIndex(t, newAdvisoryComposerHandler(t, upstream)) + for _, key := range []string{"security-advisories", "filter"} { + if _, ok := index[key]; ok { + t.Errorf("%s advertised although upstream does not offer it: %v", key, index[key]) + } + } + }) + } +} + +func TestComposerServiceIndexUpstreamDown(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + h := newAdvisoryComposerHandler(t, upstream) + upstream.Close() + + index := serveComposerIndex(t, h) + if _, ok := index["security-advisories"]; ok { + t.Errorf("security-advisories advertised without an upstream index: %v", index["security-advisories"]) + } + if index["metadata-url"] == nil { + t.Error("packages.json lacks metadata-url while the upstream is down") + } +} + +func serveAdvisories(h *ComposerHandler, method, target, body string) *httptest.ResponseRecorder { + var reader io.Reader + if body != "" { + reader = strings.NewReader(body) + } + req := httptest.NewRequest(method, target, reader) + if method == http.MethodPost { + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + } + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, req) + return w +} + +func TestComposerAdvisoriesWithholdPrivateNames(t *testing.T) { + // The form Composer sends: PHP's http_build_query numbers the entries. + form := url.Values{ + "packages[0]": {"public/library"}, + "packages[1]": {"example/private"}, + "packages[2]": {"Acme/hosted"}, + "packages[3]": {"other/library"}, + } + tests := []struct { + name, method, target, body string + }{ + {"POST form", http.MethodPost, "/api/security-advisories/", form.Encode()}, + {"GET query", http.MethodGet, "/api/security-advisories/?" + form.Encode(), ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + h := newAdvisoryComposerHandler(t, upstream) + + w := serveAdvisories(h, tt.method, tt.target, tt.body) + + if w.Code != http.StatusOK || w.Body.String() != composerAdvisoriesResponse { + t.Fatalf("got %d %q, want upstream's advisories", w.Code, w.Body) + } + sent := upstream.requestsTo("/api/security-advisories/") + if len(sent) != 1 { + t.Fatalf("upstream got %d advisories requests, want 1", len(sent)) + } + if sent[0].method != tt.method { + t.Errorf("upstream method = %s, want %s", sent[0].method, tt.method) + } + query := sent[0].query + if tt.method == http.MethodPost { + query, _ = url.ParseQuery(sent[0].body) + } + got := query["packages[]"] + slices.Sort(got) + if want := []string{"other/library", "public/library"}; !slices.Equal(got, want) { + t.Errorf("upstream was asked about %v, want only the public packages %v", got, want) + } + }) + } +} + +func TestComposerAdvisoriesOnlyPrivateNames(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + h := newAdvisoryComposerHandler(t, upstream) + + w := serveAdvisories(h, http.MethodPost, "/api/security-advisories/", "packages%5B0%5D=example%2Fprivate") + + if w.Code != http.StatusOK || w.Body.String() != `{"advisories":{}}` { + t.Errorf("got %d %q, want an empty advisories answer", w.Code, w.Body) + } + if sent := upstream.requestsTo("/api/security-advisories/"); len(sent) != 0 { + t.Errorf("upstream was asked although every package is private: %+v", sent[0]) + } +} + +func TestComposerAdvisoriesUpdatedSince(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + h := newAdvisoryComposerHandler(t, upstream) + + w := serveAdvisories(h, http.MethodGet, "/api/security-advisories/?updatedSince=1700000000", "") + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", w.Code) + } + sent := upstream.requestsTo("/api/security-advisories/") + if len(sent) != 1 || sent[0].query.Get("updatedSince") != "1700000000" { + t.Errorf("updatedSince was not passed on: %+v", sent) + } +} + +func TestComposerAdvisoriesNotOffered(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + upstream.index = `{"packages": []}` + h := newAdvisoryComposerHandler(t, upstream) + + if w := serveAdvisories(h, http.MethodPost, "/api/security-advisories/", "packages%5B%5D=public%2Flibrary"); w.Code != http.StatusNotFound { + t.Errorf("advisories status = %d, want 404 when upstream has no API", w.Code) + } + if w := serveComposer(h, "/lists/summary.json"); w.Code != http.StatusNotFound { + t.Errorf("summary status = %d, want 404 when upstream has no filter", w.Code) + } +} + +func TestComposerFilterSummaryRelayed(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + h := newAdvisoryComposerHandler(t, upstream) + + w := serveComposer(h, "/lists/summary.json") + if w.Code != http.StatusOK || w.Body.String() != composerFilterSummary { + t.Fatalf("got %d %q, want upstream's summary", w.Code, w.Body) + } + + req := httptest.NewRequest(http.MethodGet, "/lists/summary.json", nil) + req.Header.Set("If-Modified-Since", "Wed, 21 Oct 2026 15:45:01 GMT") + w = httptest.NewRecorder() + h.Routes().ServeHTTP(w, req) + if w.Code != http.StatusNotModified { + t.Errorf("revalidation status = %d, want 304 passed on from upstream", w.Code) + } +} + +func TestComposerMetadataKeepsAdvisoriesAndFilter(t *testing.T) { + h := NewComposerHandler(testProxy(), "http://proxy.test") + body := []byte(`{ + "minified": "composer/2.0", + "packages": {"evil/package": [{"name": "evil/package", "version": "1.0.0", "dist": {"url": "https://example.com/evil.zip", "type": "zip"}}]}, + "security-advisories": [{"advisoryId": "PKSA-1", "affectedVersions": "<2.0.0"}], + "filter": {"malware": [{"constraint": "1.0.0", "reason": "malware", "id": "PKFE-1"}]} + }`) + + out, _, err := h.rewriteMetadataKeeping(body) + if err != nil { + t.Fatalf("rewrite: %v", err) + } + var doc map[string]json.RawMessage + if err := json.Unmarshal(out, &doc); err != nil { + t.Fatalf("parsing rewritten metadata: %v", err) + } + if !strings.Contains(string(doc["security-advisories"]), "PKSA-1") { + t.Errorf("security-advisories lost in rewrite: %s", out) + } + if !strings.Contains(string(doc["filter"]), "PKFE-1") { + t.Errorf("filter lost in rewrite: %s", out) + } +} From 9240f9edef2152a2873bf578dad97db28b67e056 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Wed, 28 Oct 2026 14:10:48 +0100 Subject: [PATCH 2/3] Keep the upstream Composer index in memory Serving packages.json fetched the upstream's on every request with cache_metadata off, and after each TTL with it on, so every Composer command waited on the upstream and on http_timeout when it was down. The parsed index is now kept for the metadata TTL, or an hour when it is 0, and a failed fetch is not retried within that time. --- docs/configuration.md | 2 +- internal/handler/composer.go | 6 +- internal/handler/composer_advisories.go | 72 +++++++++++++++-- internal/handler/composer_advisories_test.go | 85 ++++++++++++++++++++ 4 files changed, 157 insertions(+), 8 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 7eb8485d..1fe96297 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -347,7 +347,7 @@ upstream: composer_repository: "https://upstream-proxy.example.com/composer" ``` -The proxy reads `packages.json` from `upstream.composer_repository`, cached for the metadata TTL, and passes on its `security-advisories` and `filter` entries (Packagist's malware list) with URLs pointing at the proxy. An upstream that offers neither is still usable; `composer audit` then reports nothing from it. A filter `api-url` is not passed on; Composer reads filter entries from package metadata instead. +The proxy reads `packages.json` from `upstream.composer_repository` and keeps it in memory for the metadata TTL, or an hour when the TTL is 0, whether or not `cache_metadata` is on. A failed fetch is not retried within that time: the last good copy is used, or the entries are left out. It passes on its `security-advisories` and `filter` entries (Packagist's malware list) with URLs pointing at the proxy. An upstream that offers neither is still usable; `composer audit` then reports nothing from it. A filter `api-url` is not passed on; Composer reads filter entries from package metadata instead. `upstream.hex_api` is used for cooldown timestamps and must expose Hex's `/api/packages/{name}` JSON endpoint. diff --git a/internal/handler/composer.go b/internal/handler/composer.go index bf04da02..2084ae69 100644 --- a/internal/handler/composer.go +++ b/internal/handler/composer.go @@ -55,6 +55,7 @@ type ComposerHandler struct { repoURL string proxyURL string routes packageRoutes + indexCache composerIndexCache } // NewComposerHandler creates a new Composer protocol handler. @@ -110,8 +111,9 @@ func (h *ComposerHandler) Routes() http.Handler { // handleServiceIndex returns the Composer repository service index. It // carries the security advisories and filter list entries of the default -// repository's own index when that has them; if the repository cannot be -// reached and nothing is cached, the index goes out without them. +// repository's own index when that has them, kept in memory for the metadata +// TTL; if the repository has not been reached, the index goes out without +// them. func (h *ComposerHandler) handleServiceIndex(w http.ResponseWriter, r *http.Request) { index := map[string]any{ "packages": map[string]any{}, diff --git a/internal/handler/composer_advisories.go b/internal/handler/composer_advisories.go index e77da373..ed8f4bca 100644 --- a/internal/handler/composer_advisories.go +++ b/internal/handler/composer_advisories.go @@ -8,6 +8,10 @@ import ( "net/http" "net/url" "strings" + "sync" + "time" + + "golang.org/x/sync/singleflight" ) const ( @@ -32,18 +36,76 @@ type composerUpstreamIndex struct { filterSummaryURL string } -// upstreamIndex fetches the default repository's packages.json, cached for -// the metadata TTL. ok is false when the repository could not be reached and -// nothing is cached; a document without the fields yields an empty index. +// composerIndexFallbackTTL is how long the upstream index is kept when the +// metadata TTL is 0. +const composerIndexFallbackTTL = time.Hour + +// composerIndexCache keeps the default repository's parsed packages.json in +// memory, whether or not metadata caching is on, so that serving the proxy's +// own packages.json does not wait on the upstream. A failed fetch is +// remembered for the same interval: the last good copy, or none, is used +// until then instead of retrying on every request. The zero value is ready +// to use. +type composerIndexCache struct { + mu sync.Mutex + index composerUpstreamIndex + ok bool + fetchedAt time.Time + group singleflight.Group + now func() time.Time // for tests; nil means time.Now +} + +func (c *composerIndexCache) clock() time.Time { + if c.now != nil { + return c.now() + } + return time.Now() +} + +// upstreamIndex returns the default repository's packages.json fields. ok is +// false when the repository has not been reached since the proxy started; a +// document without the fields yields an empty index. func (h *ComposerHandler) upstreamIndex(ctx context.Context) (composerUpstreamIndex, bool) { + c := &h.indexCache + ttl := h.proxy.MetadataTTL + if ttl <= 0 { + ttl = composerIndexFallbackTTL + } + c.mu.Lock() + if !c.fetchedAt.IsZero() && c.clock().Sub(c.fetchedAt) < ttl { + index, ok := c.index, c.ok + c.mu.Unlock() + return index, ok + } + c.mu.Unlock() + + // One fetch serves every waiting request. It is detached from the + // request that started it, so a client hanging up neither fails the + // others nor gets remembered as an upstream failure. + _, _, _ = c.group.Do("index", func() (any, error) { + index, err := h.fetchUpstreamIndex(context.WithoutCancel(ctx)) + c.mu.Lock() + defer c.mu.Unlock() + if err == nil { + c.index, c.ok = index, true + } + c.fetchedAt = c.clock() + return nil, nil + }) + c.mu.Lock() + defer c.mu.Unlock() + return c.index, c.ok +} + +func (h *ComposerHandler) fetchUpstreamIndex(ctx context.Context) (composerUpstreamIndex, error) { indexURL := h.repoURL + "/packages.json" cacheKey := hashedCacheKey(h.repoURL, "/packages.json") res := h.proxy.fetchMetadataDocument(ctx, "composer", cacheKey, indexURL, contentTypeJSON, nil) if res.err != nil { h.proxy.Logger.Warn("failed to fetch upstream composer packages.json", "url", indexURL, "error", res.err) - return composerUpstreamIndex{}, false + return composerUpstreamIndex{}, res.err } - return parseComposerUpstreamIndex(indexURL, res.body), true + return parseComposerUpstreamIndex(indexURL, res.body), nil } func parseComposerUpstreamIndex(indexURL string, body []byte) composerUpstreamIndex { diff --git a/internal/handler/composer_advisories_test.go b/internal/handler/composer_advisories_test.go index 6adc7d24..5e1a2047 100644 --- a/internal/handler/composer_advisories_test.go +++ b/internal/handler/composer_advisories_test.go @@ -9,7 +9,9 @@ import ( "slices" "strings" "sync" + "sync/atomic" "testing" + "time" ) // composerAdvisoryUpstream serves a packages.json advertising security @@ -18,6 +20,7 @@ import ( type composerAdvisoryUpstream struct { *httptest.Server index string + down atomic.Bool // answer every request with 503 mu sync.Mutex requests []*recordedRequest @@ -43,6 +46,10 @@ func newComposerAdvisoryUpstream(t *testing.T) *composerAdvisoryUpstream { u.mu.Lock() u.requests = append(u.requests, &recordedRequest{r.Method, r.URL.Path, r.URL.Query(), string(body), r.Header.Clone()}) u.mu.Unlock() + if u.down.Load() { + w.WriteHeader(http.StatusServiceUnavailable) + return + } w.Header().Set("Content-Type", "application/json") switch r.URL.Path { case "/packages.json": @@ -164,6 +171,84 @@ func TestComposerServiceIndexUpstreamDown(t *testing.T) { } } +func advertisesAdvisories(index map[string]any) bool { + _, ok := index["security-advisories"] + return ok +} + +func TestComposerServiceIndexUpstreamDownNotRetried(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + upstream.down.Store(true) + h := newAdvisoryComposerHandler(t, upstream) + + serveComposerIndex(t, h) + fetched := len(upstream.requestsTo("/packages.json")) + if fetched == 0 { + t.Fatal("first packages.json made no upstream request") + } + index := serveComposerIndex(t, h) + if got := len(upstream.requestsTo("/packages.json")); got != fetched { + t.Errorf("second packages.json made %d upstream requests, want none", got-fetched) + } + if advertisesAdvisories(index) { + t.Errorf("security-advisories advertised without an upstream index: %v", index["security-advisories"]) + } +} + +func TestComposerServiceIndexKeptInMemory(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + h := newAdvisoryComposerHandler(t, upstream) + h.proxy.MetadataTTL = 0 + now := time.Now() + h.indexCache.now = func() time.Time { return now } + + if !advertisesAdvisories(serveComposerIndex(t, h)) { + t.Fatal("security-advisories not advertised") + } + if got := len(upstream.requestsTo("/packages.json")); got != 1 { + t.Fatalf("upstream packages.json requests = %d, want 1", got) + } + + // Within the interval the copy in memory is served without asking. + upstream.down.Store(true) + now = now.Add(composerIndexFallbackTTL - time.Minute) + if !advertisesAdvisories(serveComposerIndex(t, h)) { + t.Error("security-advisories dropped within the interval") + } + if got := len(upstream.requestsTo("/packages.json")); got != 1 { + t.Errorf("upstream packages.json requests = %d within the interval, want 1", got) + } + + // After it the index is fetched again; the failure keeps the last good + // copy and is not retried until the next interval. + now = now.Add(2 * time.Minute) + if !advertisesAdvisories(serveComposerIndex(t, h)) { + t.Error("last good copy dropped after a failed fetch") + } + fetched := len(upstream.requestsTo("/packages.json")) + if fetched < 2 { + t.Fatalf("upstream packages.json requests = %d after the interval, want a new fetch", fetched) + } + serveComposerIndex(t, h) + if got := len(upstream.requestsTo("/packages.json")); got != fetched { + t.Errorf("failed fetch retried %d times within the interval", got-fetched) + } +} + +func TestComposerServiceIndexFetchedOnce(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + h := newAdvisoryComposerHandler(t, upstream) + + var wg sync.WaitGroup + for range 10 { + wg.Go(func() { serveComposer(h, "/packages.json") }) + } + wg.Wait() + if got := len(upstream.requestsTo("/packages.json")); got != 1 { + t.Errorf("upstream packages.json requests = %d for parallel requests, want 1", got) + } +} + func serveAdvisories(h *ComposerHandler, method, target, body string) *httptest.ResponseRecorder { var reader io.Reader if body != "" { From 3ce582c7330335a707df8a286e357e65f10cee89 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Fri, 30 Oct 2026 19:25:41 +0100 Subject: [PATCH 3/3] Keep the last good Composer index when upstream sends one that doesn't parse parseComposerUpstreamIndex now returns an error for a packages.json that is not a JSON object or has the advisories and filter fields in the wrong shape, and fetchUpstreamIndex passes it to the metadata cache as validate. A malformed 200 is no longer stored: with metadata caching the stored copy is served as when upstream fails, and without it the error keeps the copy in memory. --- internal/handler/composer_advisories.go | 31 ++++++-- internal/handler/composer_advisories_test.go | 77 ++++++++++++++++++++ 2 files changed, 103 insertions(+), 5 deletions(-) diff --git a/internal/handler/composer_advisories.go b/internal/handler/composer_advisories.go index ed8f4bca..aa0fa265 100644 --- a/internal/handler/composer_advisories.go +++ b/internal/handler/composer_advisories.go @@ -1,9 +1,11 @@ package handler import ( + "bytes" "context" "encoding/json" "errors" + "fmt" "io" "net/http" "net/url" @@ -97,18 +99,34 @@ func (h *ComposerHandler) upstreamIndex(ctx context.Context) (composerUpstreamIn return c.index, c.ok } +// fetchUpstreamIndex fetches and parses the default repository's +// packages.json. A document that doesn't parse is rejected before it is +// cached, so a broken response never replaces a good copy: with metadata +// caching the stored copy is used instead, and without it the error keeps +// the copy in memory. func (h *ComposerHandler) fetchUpstreamIndex(ctx context.Context) (composerUpstreamIndex, error) { indexURL := h.repoURL + "/packages.json" cacheKey := hashedCacheKey(h.repoURL, "/packages.json") - res := h.proxy.fetchMetadataDocument(ctx, "composer", cacheKey, indexURL, contentTypeJSON, nil) + validate := func(body []byte) error { + _, err := parseComposerUpstreamIndex(indexURL, body) + return err + } + res := h.proxy.fetchMetadataDocument(ctx, "composer", cacheKey, indexURL, contentTypeJSON, validate) if res.err != nil { h.proxy.Logger.Warn("failed to fetch upstream composer packages.json", "url", indexURL, "error", res.err) return composerUpstreamIndex{}, res.err } - return parseComposerUpstreamIndex(indexURL, res.body), nil + return parseComposerUpstreamIndex(indexURL, res.body) } -func parseComposerUpstreamIndex(indexURL string, body []byte) composerUpstreamIndex { +// errComposerIndexNotObject is returned for a packages.json that is valid +// JSON but not an object, such as null. +var errComposerIndexNotObject = errors.New("composer packages.json is not a JSON object") + +// parseComposerUpstreamIndex reads the advisories and filter fields of a +// packages.json. A document without them yields an empty index; one that is +// not a JSON object, or has them in the wrong shape, is an error. +func parseComposerUpstreamIndex(indexURL string, body []byte) (composerUpstreamIndex, error) { var doc struct { SecurityAdvisories struct { Metadata bool `json:"metadata"` @@ -121,8 +139,11 @@ func parseComposerUpstreamIndex(indexURL string, body []byte) composerUpstreamIn } `json:"filter"` } var index composerUpstreamIndex + if trimmed := bytes.TrimSpace(body); len(trimmed) == 0 || trimmed[0] != '{' { + return index, errComposerIndexNotObject + } if err := json.Unmarshal(body, &doc); err != nil { - return index + return index, fmt.Errorf("parsing composer packages.json: %w", err) } // Composer refuses advisories configured without an api-url unless the // repository lists its packages, which the proxy does not. @@ -137,7 +158,7 @@ func parseComposerUpstreamIndex(indexURL string, body []byte) composerUpstreamIn index.filterLists = doc.Filter.Lists index.filterSummaryURL, _ = resolveComposerURL(indexURL, doc.Filter.SummaryURL) } - return index + return index, nil } // resolveComposerURL resolves a URL from packages.json against the document's diff --git a/internal/handler/composer_advisories_test.go b/internal/handler/composer_advisories_test.go index 5e1a2047..097d8cca 100644 --- a/internal/handler/composer_advisories_test.go +++ b/internal/handler/composer_advisories_test.go @@ -2,6 +2,7 @@ package handler import ( "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" @@ -12,6 +13,8 @@ import ( "sync/atomic" "testing" "time" + + "github.com/git-pkgs/proxy/internal/database" ) // composerAdvisoryUpstream serves a packages.json advertising security @@ -21,6 +24,8 @@ type composerAdvisoryUpstream struct { *httptest.Server index string down atomic.Bool // answer every request with 503 + // malformed answers packages.json with an HTML page and status 200. + malformed atomic.Bool mu sync.Mutex requests []*recordedRequest @@ -53,6 +58,11 @@ func newComposerAdvisoryUpstream(t *testing.T) *composerAdvisoryUpstream { w.Header().Set("Content-Type", "application/json") switch r.URL.Path { case "/packages.json": + if u.malformed.Load() { + w.Header().Set("Content-Type", "text/html") + _, _ = io.WriteString(w, "Service temporarily unavailable") + return + } _, _ = io.WriteString(w, u.index) case "/api/security-advisories/": _, _ = io.WriteString(w, composerAdvisoriesResponse) @@ -235,6 +245,73 @@ func TestComposerServiceIndexKeptInMemory(t *testing.T) { } } +// assertComposerSecurityAdvertised checks that h advertises the upstream's +// advisories API and malware filter, and relays both. +func assertComposerSecurityAdvertised(t *testing.T, h *ComposerHandler, when string) { + t.Helper() + index := serveComposerIndex(t, h) + if !advertisesAdvisories(index) { + t.Errorf("%s: security-advisories not advertised: %v", when, index) + } + if _, ok := index["filter"]; !ok { + t.Errorf("%s: filter not advertised: %v", when, index) + } + w := serveAdvisories(h, http.MethodPost, "/api/security-advisories/", "packages%5B%5D=public%2Flibrary") + if w.Code != http.StatusOK || w.Body.String() != composerAdvisoriesResponse { + t.Errorf("%s: advisories = %d %q, want upstream's advisories", when, w.Code, w.Body) + } + if w := serveComposer(h, "/lists/summary.json"); w.Code != http.StatusOK || w.Body.String() != composerFilterSummary { + t.Errorf("%s: filter summary = %d %q, want upstream's summary", when, w.Code, w.Body) + } +} + +// A packages.json that doesn't parse, sent with status 200, must not replace +// the last good index, with metadata caching on or off: the advisories API +// and the malware filter stay advertised and relayed. +func TestComposerServiceIndexMalformedRefreshKeepsLastGood(t *testing.T) { + for _, cacheMetadata := range []bool{false, true} { + t.Run(fmt.Sprintf("cache_metadata=%v", cacheMetadata), func(t *testing.T) { + upstream := newComposerAdvisoryUpstream(t) + h := newAdvisoryComposerHandler(t, upstream) + h.proxy.CacheMetadata = cacheMetadata + h.proxy.MetadataTTL = time.Hour + now := time.Now() + h.indexCache.now = func() time.Time { return now } + cacheKey := hashedCacheKey(upstream.URL, "/packages.json") + + assertComposerSecurityAdvertised(t, h, "before the refresh") + var stored *database.MetadataCacheEntry + if cacheMetadata { + var err error + if stored, err = h.proxy.DB.GetMetadataCache("composer", cacheKey); err != nil || stored == nil { + t.Fatalf("packages.json not cached: %v, %v", stored, err) + } + expireMetadata(t, h.proxy.DB, "composer", cacheKey) + } + + upstream.malformed.Store(true) + now = now.Add(2 * time.Hour) + fetched := len(upstream.requestsTo("/packages.json")) + assertComposerSecurityAdvertised(t, h, "after a malformed refresh") + if got := len(upstream.requestsTo("/packages.json")); got == fetched { + t.Fatal("packages.json was not fetched again after the TTL") + } + + if !cacheMetadata { + return + } + after, err := h.proxy.DB.GetMetadataCache("composer", cacheKey) + if err != nil || after == nil || after.ContentDigest != stored.ContentDigest { + t.Errorf("cached packages.json replaced by the malformed refresh: %v, %v", after, err) + } + // A fresh handler, as after a restart, has nothing in memory and + // reads the stored copy. + restarted := NewComposerHandlerWithUpstreams(h.proxy, "http://proxy.test", upstream.URL, upstream.URL) + assertComposerSecurityAdvertised(t, restarted, "after a restart") + }) + } +} + func TestComposerServiceIndexFetchedOnce(t *testing.T) { upstream := newComposerAdvisoryUpstream(t) h := newAdvisoryComposerHandler(t, upstream)