From 12c99148e204ce5a0b82ac2fed1eb76b48540aa3 Mon Sep 17 00:00:00 2001 From: Christoph Leygraf Date: Sun, 1 Nov 2026 18:11:49 +0100 Subject: [PATCH] Forward upstream policy denials instead of masking them as 502 A firewall in front of an upstream registry denies an artifact with 403 (npm, PyPI, Maven, OCI) or 409 (NuGet) and explains why in the response body. The fetch layer reports those responses as an error, so every artifact download handler answered with its own generic 502 Bad Gateway and the reason was lost: the package manager reports a proxy fault where the upstream refused the artifact on policy grounds. Recover the status and body from the fetch error and forward them: - serveUpstreamBlock answers with the upstream status and report body, and serveArtifactError routes the plain-text download handlers through it - npm keeps its JSON error shape but passes the report through unchanged - container answers a refused blob with an OCI 403 DENIED Upstream 5xx responses and transport errors still become 502. --- internal/handler/container.go | 7 + internal/handler/handler.go | 3 + internal/handler/npm.go | 4 + internal/handler/upstream_block.go | 60 +++++++ internal/handler/upstream_block_test.go | 220 ++++++++++++++++++++++++ 5 files changed, 294 insertions(+) create mode 100644 internal/handler/upstream_block.go create mode 100644 internal/handler/upstream_block_test.go diff --git a/internal/handler/container.go b/internal/handler/container.go index d9360fbd..c9240dde 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -454,6 +454,13 @@ func (h *ContainerHandler) handleBlobDownload(w http.ResponseWriter, r *http.Req if writeUpstreamHeld(w, err) { return } + // A 403 from the upstream is a policy denial, not a proxy failure: + // report it the way an OCI registry reports a refused blob. + if status, _, ok := upstreamStatus(err); ok && status == http.StatusForbidden { + h.proxy.Logger.Warn("blob refused by upstream policy", "name", name, "digest", digest) + h.containerError(w, http.StatusForbidden, "DENIED", "blob refused by upstream policy") + return + } h.proxy.Logger.Error("failed to fetch blob", "error", err) h.containerError(w, http.StatusBadGateway, "INTERNAL_ERROR", "failed to fetch blob") return diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 4b7ebe72..9f205d2f 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1573,6 +1573,9 @@ func (p *Proxy) serveArtifactError(w http.ResponseWriter, err error, clientMsg s if writeUpstreamHeld(w, err) { return } + if p.serveUpstreamBlock(w, err) { + return + } p.Logger.Error("failed to get artifact", "error", err) http.Error(w, clientMsg, http.StatusBadGateway) } diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 62bf4409..4c023105 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -585,6 +585,10 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { if writeUpstreamHeld(w, err) { return } + // A policy denial keeps its own status and JSON report body. + if h.proxy.serveUpstreamBlock(w, err) { + return + } h.proxy.Logger.Error("failed to get artifact", "error", err) JSONError(w, http.StatusBadGateway, "failed to fetch package") } diff --git a/internal/handler/upstream_block.go b/internal/handler/upstream_block.go new file mode 100644 index 00000000..7e24c51d --- /dev/null +++ b/internal/handler/upstream_block.go @@ -0,0 +1,60 @@ +package handler + +import ( + "net/http" + "regexp" + "strconv" + "strings" +) + +// upstreamStatusPattern matches the error the fetch layer returns for an +// upstream response it does not handle itself: fmt.Errorf("unexpected status +// %d: %s", code, body). The fetch layer exposes neither the status nor the +// body any other way, and the body is the only place an upstream's reason for +// refusing an artifact survives, so both are read back out of the message. +var upstreamStatusPattern = regexp.MustCompile(`unexpected status (\d+): `) + +// upstreamStatus returns the status code and body of an upstream response the +// fetch layer rejected, and reports whether err carried one. Transport errors +// and the fetch layer's own sentinels (not found, rate limited, upstream down) +// carry no status and report false. +func upstreamStatus(err error) (code int, body string, ok bool) { + if err == nil { + return 0, "", false + } + msg := err.Error() + loc := upstreamStatusPattern.FindStringSubmatchIndex(msg) + if loc == nil { + return 0, "", false + } + code, convErr := strconv.Atoi(msg[loc[2]:loc[3]]) + if convErr != nil { + return 0, "", false + } + return code, msg[loc[1]:], true +} + +// serveUpstreamBlock answers a request whose upstream refused the artifact on +// policy grounds with that upstream's status and report body, and reports +// whether err was such a refusal. +// +// A firewall in front of the upstream denies with 403 for npm, PyPI, Maven and +// OCI, and with 409 for NuGet, and puts the reason in the body. Answering with +// the real status and body keeps that reason visible to the package manager +// instead of the 502 a failed fetch otherwise produces, which reads as a proxy +// fault and hides why the artifact was refused. +func (p *Proxy) serveUpstreamBlock(w http.ResponseWriter, err error) bool { + status, body, ok := upstreamStatus(err) + if !ok || (status != http.StatusForbidden && status != http.StatusConflict) { + return false + } + p.Logger.Warn("upstream refused artifact on policy grounds", "status", status, "detail", body) + contentType := "text/plain; charset=utf-8" + if strings.HasPrefix(strings.TrimSpace(body), "{") { + contentType = contentTypeJSON + } + w.Header().Set(headerContentType, contentType) + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + return true +} diff --git a/internal/handler/upstream_block_test.go b/internal/handler/upstream_block_test.go new file mode 100644 index 00000000..f6eecaae --- /dev/null +++ b/internal/handler/upstream_block_test.go @@ -0,0 +1,220 @@ +package handler + +import ( + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/git-pkgs/registries/fetch" +) + +const firewallReport = `{"status":403,"title":"Sonatype Firewall Report","detail":"Sonatype has identified this component as potentially malicious and blocked the download."}` + +// blockedByUpstream is the error the fetch layer produces for an upstream that +// refuses an artifact with a policy denial. +func blockedByUpstream(status int, body string) error { + return fmt.Errorf("fetching from upstream: %w", fmt.Errorf("unexpected status %d: %s", status, body)) +} + +func TestUpstreamStatus(t *testing.T) { + tests := []struct { + name string + err error + wantCode int + wantBody string + wantOK bool + }{ + { + name: "wrapped policy denial", + err: blockedByUpstream(http.StatusForbidden, firewallReport), + wantCode: http.StatusForbidden, + wantBody: firewallReport, + wantOK: true, + }, + { + name: "unwrapped policy denial", + err: errors.New("unexpected status 409: conflict"), + wantCode: http.StatusConflict, + wantBody: "conflict", + wantOK: true, + }, + { + name: "transport error carries no status", + err: errors.New("dial tcp 127.0.0.1:9: connect: connection refused"), + wantOK: false, + }, + { + name: "fetch layer sentinel carries no status", + err: fetch.ErrUpstreamDown, + wantOK: false, + }, + { + name: "no error", + err: nil, + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + code, body, ok := upstreamStatus(tt.err) + if ok != tt.wantOK { + t.Fatalf("ok = %v, want %v", ok, tt.wantOK) + } + if code != tt.wantCode { + t.Errorf("code = %d, want %d", code, tt.wantCode) + } + if body != tt.wantBody { + t.Errorf("body = %q, want %q", body, tt.wantBody) + } + }) + } +} + +func TestServeArtifactErrorForwardsPolicyBlock(t *testing.T) { + tests := []struct { + name string + err error + wantStatus int + wantBody string + wantCT string + }{ + { + name: "403 with a report body is forwarded verbatim", + err: blockedByUpstream(http.StatusForbidden, firewallReport), + wantStatus: http.StatusForbidden, + wantBody: firewallReport, + wantCT: contentTypeJSON, + }, + { + name: "409 with a plain-text body is forwarded verbatim", + err: blockedByUpstream(http.StatusConflict, "blocked by policy"), + wantStatus: http.StatusConflict, + wantBody: "blocked by policy", + wantCT: "text/plain; charset=utf-8", + }, + { + name: "an upstream 5xx is still a proxy failure", + err: blockedByUpstream(http.StatusInternalServerError, "boom"), + wantStatus: http.StatusBadGateway, + wantBody: "failed to fetch package\n", + wantCT: "text/plain; charset=utf-8", + }, + { + name: "a transport error is still a proxy failure", + err: errors.New("dial tcp 127.0.0.1:9: connect: connection refused"), + wantStatus: http.StatusBadGateway, + wantBody: "failed to fetch package\n", + wantCT: "text/plain; charset=utf-8", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + w := httptest.NewRecorder() + proxy.serveArtifactError(w, tt.err, "failed to fetch package") + + if w.Code != tt.wantStatus { + t.Errorf("status = %d, want %d", w.Code, tt.wantStatus) + } + if got := w.Body.String(); got != tt.wantBody { + t.Errorf("body = %q, want %q", got, tt.wantBody) + } + if got := w.Header().Get(headerContentType); got != tt.wantCT { + t.Errorf("content-type = %q, want %q", got, tt.wantCT) + } + }) + } +} + +// A policy denial from the upstream reaches the Maven client with the +// upstream's own status and report body, through the real fetch layer. +func TestMavenHandler_ForwardsUpstreamPolicyBlock(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(headerContentType, contentTypeJSON) + w.WriteHeader(http.StatusForbidden) + _, _ = io.WriteString(w, firewallReport) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + artifactFetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client())) + t.Cleanup(func() { _ = artifactFetcher.Close() }) + proxy.Fetcher = artifactFetcher + proxy.HTTPClient = upstream.Client() + + h := NewMavenHandler(proxy, "http://localhost", upstream.URL, upstream.URL) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, + "/org/sonatype/maven-policy-demo/1.1.0/maven-policy-demo-1.1.0.jar", nil)) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d; body: %s", w.Code, http.StatusForbidden, w.Body.String()) + } + if got := w.Body.String(); got != firewallReport { + t.Errorf("body = %q, want %q", got, firewallReport) + } +} + +// npm keeps its JSON error shape, but a policy denial is passed through as the +// upstream sent it rather than as a generic 502. +func TestNPMHandler_ForwardsUpstreamPolicyBlock(t *testing.T) { + var upstreamURL string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = io.WriteString(w, `{"name":"policy-demo","versions":{"2.1.0":{"dist":{"tarball":"`+ + upstreamURL+`/policy-demo/-/policy-demo-2.1.0.tgz"}}}}`) + })) + defer upstream.Close() + upstreamURL = upstream.URL + + proxy, _, _, fetcher := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + fetcher.fetchErrByURL = map[string]error{ + upstreamURL + "/policy-demo/-/policy-demo-2.1.0.tgz": blockedByUpstream(http.StatusForbidden, firewallReport), + } + + h := NewNPMHandler(proxy, "http://localhost", upstreamURL) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/policy-demo/-/policy-demo-2.1.0.tgz", nil)) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d; body: %s", w.Code, http.StatusForbidden, w.Body.String()) + } + if got := w.Body.String(); got != firewallReport { + t.Errorf("body = %q, want %q", got, firewallReport) + } +} + +// A refused blob is reported the way an OCI registry reports a denial, not as +// a proxy failure. +func TestContainerHandler_BlobPolicyBlockIsDenied(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = io.WriteString(w, firewallReport) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + artifactFetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client())) + t.Cleanup(func() { _ = artifactFetcher.Close() }) + proxy.Fetcher = artifactFetcher + proxy.HTTPClient = upstream.Client() + + h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost"} + digest := sha256Digest([]byte("blocked layer")) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/library/nginx/blobs/"+digest, nil)) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d; body: %s", w.Code, http.StatusForbidden, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "DENIED") { + t.Errorf("body = %q, want an OCI DENIED error", w.Body.String()) + } +}