Forward upstream policy denials instead of masking them as 502

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

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)
 }

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")
 		}

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
+}

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())
+	}
+}