Skip to content

Forward upstream policy denials instead of masking them as 502 - #650

Open
cleygraf wants to merge 1 commit into
git-pkgs:mainfrom
sonatype-demo:upstream/policy-block-passthrough
Open

cleygraf wants to merge 1 commit into
git-pkgs:mainfrom
sonatype-demo:upstream/policy-block-passthrough

Conversation

@cleygraf

@cleygraf cleygraf commented Nov 1, 2026 •

Copy link
Copy Markdown

Problem

A firewall in front of an upstream registry (Sonatype Firewall Pro, for one) denies an artifact with 403 for npm, PyPI, Maven and OCI, and with 409 for NuGet, and explains the denial in the response body. fetch.Fetcher reports those responses as an error, so every artifact download handler answered with its own generic 502 Bad Gateway. The package manager then reports a proxy fault, and the reason the artifact was refused never reaches the user.

Change

Recover the status code and body from the fetch error and forward them:

  • serveUpstreamBlock writes the upstream status and report body (JSON body keeps application/json, anything else is text/plain), and serveArtifactError routes the plain-text download handlers through it, so composer, conan, conda, cran, debian, julia, nuget, pypi, rpm, cargo, pub, go, gem and maven all behave the same way.
  • npm keeps its JSON error shape, but a policy denial is passed through as the upstream sent it.
  • container answers a refused blob with an OCI 403 DENIED instead of 502 INTERNAL_ERROR.

Upstream 5xx responses and transport errors still become 502.

The fetch layer exposes neither the status nor the body as a typed error, so both are read back out of its message (unexpected status %d: %s); the helper is the one place that knows this format. git-pkgs/registries#129 adds fetch.StatusError, which would let this drop the parsing.

Test

internal/handler/upstream_block_test.go:

  • TestUpstreamStatus — wrapped and unwrapped denials, transport errors and fetch sentinels.
  • TestServeArtifactErrorForwardsPolicyBlock — 403 and 409 forwarded verbatim with the right content type; upstream 5xx and transport errors still 502.
  • TestMavenHandler_ForwardsUpstreamPolicyBlock — a real fetch.Fetcher against an upstream that answers 403 with a report body; the client receives that status and body unchanged.
  • TestNPMHandler_ForwardsUpstreamPolicyBlock — same for the npm tarball path.
  • TestContainerHandler_BlobPolicyBlockIsDenied — a refused blob is reported as OCI DENIED.

go build ./... && go vet ./... && go test ./... -count=1 pass.

Context

Found while running the proxy in front of Sonatype Firewall Pro, where a blocked package was indistinguishable from a broken proxy. The change is generic: any upstream that refuses an artifact with 403/409 and a body benefits.

Companion change: git-pkgs/registries#129 — a 4xx answer no longer counts against the per-host circuit breaker, so a run of policy denials cannot open it for every registry that host serves.

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.

@andrew andrew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, forwarding the upstream's 403/409 instead of a generic 502 is the right behaviour. A few things before this can go in:

  1. upstreamStatus parses the status and body back out of the fetcher's error string with a regex. registries is ours to change, and git-pkgs/registries#129 already adds fetch.StatusError{Code, Body}. Once that is merged and tagged, please bump registries here and replace upstreamStatus and upstreamStatusPattern with errors.As(err, &statusErr). TestUpstreamStatus can then go, or shrink to an errors.As check.

  2. The fetcher truncates error bodies to 1024 bytes (maxErrBodySize), but serveUpstreamBlock labels any body starting with { as application/json. A JSON report over 1KB gets served cut off mid-object as invalid JSON, which npm then fails to parse. Either only use the JSON content type when json.Valid(body) passes, or carry the upstream Content-Type on StatusError in registries#129 and forward that.

  3. In container.go a refused blob answers DENIED with a fixed "blob refused by upstream policy" message and the Warn log drops the body too, so Docker clients still lose the reason the upstream gave. Put the upstream body in the OCI error message (or detail) and include it in the log line.

  4. The comments repeat the commit message. Cut the doc comment on serveUpstreamBlock to one line, drop the upstreamStatusPattern comment along with the regex, and remove the comments added in npm.go and above the 403 check in container.go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants