GeezerVison is showing github.com — Check upstream destinations sent through a forward proxy by simonchrz · Pull Request #577 · git-pkgs/proxy
Conversation
simonchrz
commented
Oct 24, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Hardens the upstream address check for setups that use a forward proxy.
The upstream client refuses loopback, private, CGNAT and link-local addresses when it dials. With HTTP_PROXY or HTTPS_PROXY set it dials the forward proxy instead of the destination, so only the forward proxy's address was checked, and URLs taken from upstream metadata (Helm chart URLs, Composer dist URLs, token realms, redirects) were passed through to whatever they named.
The check now lives in registries' safehttp (git-pkgs/registries#116): its transport resolves the destination of each request sent to a forward proxy and applies the same rules, including upstream.allow_loopback and upstream.allow_private_hosts, before handing it over. Link-local addresses stay blocked. A name that can't be resolved locally is still passed on, since some networks only resolve public names at the forward proxy. Direct requests are unchanged. Git fetches and OCI token requests go through the same client, so they are covered too.
This PR bumps registries and documents how the rules apply with a forward proxy. The proxy-side guard from the first commit is removed again in the second, so newUpstreamClient is unchanged against main; squashing gives just the bump and the docs.
go.mod points at a pseudo-version of registries main until #116 is released. I'll switch it to the tag then.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The upstream client refuses loopback, private and link-local addresses
when it dials, but with HTTP_PROXY or HTTPS_PROXY set it dials the
forward proxy, so only the forward proxy's address was checked. A URL
taken from upstream metadata, such as a Helm chart URL, a Composer dist
URL, a token realm or a redirect, could then reach 169.254.169.254 or
an internal host through the forward proxy.
The transport's proxy function now resolves the destination of every
request it sends to a forward proxy and applies the same rules,
including upstream.allow_loopback and upstream.allow_private_hosts,
before handing it over. A name that cannot be resolved locally is still
passed on, since some networks only resolve public names at the
forward proxy. Direct requests are unchanged.
Git fetches and token requests use the same transport, so they are
covered too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bypass is real, but the gap is in safehttp: newClient keeps ProxyFromEnvironment from http.DefaultTransport and only gates DialContext, so every consumer of it has the same hole. Please move the destination check into safehttp in git-pkgs/registries (git-pkgs/registries#115), wrapping transport.Proxy in newClient next to the dial gate. The approach here carries over as is: resolve the destination, apply the same checks, and pass names that can't be resolved locally through to the forward proxy. This PR then becomes a dependency bump plus the docs paragraph.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks, agreed, the gap belongs in safehttp. I've moved the destination check there in git-pkgs/registries#116, wrapping transport.Proxy in newClient next to the dial gate with the same approach. Once that's released I'll reduce this PR to the dependency bump and the docs paragraph.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
registries' safehttp now checks the destination of requests sent through
a forward proxy (git-pkgs/registries#116), so the proxy's own guard is
removed and newUpstreamClient is back to its earlier form.
go.mod points at a pseudo-version of registries main until git-pkgs#116 is
released.
# Conflicts:
# go.mod
# go.sum
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
andrew
andrew requested changes
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
breaking
Changes behaviour users rely on; needs a minor release
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|