fix: public github release downloads should not be authenticated#27
fix: public github release downloads should not be authenticated#27yeikel wants to merge 1 commit intodependabot:mainfrom
Conversation
internal/handlers/git_server.go
Outdated
| func isGitHubReleaseDownload(path string) bool { | ||
| return strings.Contains(path, "/releases/download/") | ||
| } | ||
|
|
There was a problem hiding this comment.
This approach doesn’t feel entirely safe, as it could also match URLs like myExample.com/releases/download/.
However, I didn’t see any reliable way to verify that the host is GitHub-related.
For example, github.com is obvious, but GitHub Enterprise and other environments are less straightforward
There was a problem hiding this comment.
@JamieMagee I'd appreciate your input here and in the pull request altogether
There was a problem hiding this comment.
I agree. A few possible options I can think of:
- Scope this to only github.com (not a complete fix since GHES exists, but narrows the blast radius)
- Instead of skipping auth on the initial request, handle the auth failure on the redirect target. Though that's trickier since it's a separate request through the proxy
- Strip the Authorization header on redirect responses from known GitHub hosts (302 -> asset CDN) rather than preventing auth on the original request
There was a problem hiding this comment.
Strip the Authorization header on redirect responses from known GitHub hosts (302 -> asset CDN) rather than preventing auth on the original request
Unfortunately, this does not work because the redirect GitHub gives depends on whether you pass authentication or not. It is an interesting behavior/I ignore why that is:
HEAD request without auth: https://github.com:443/gradle/gradle-distributions/releases/download/v9.3.0/gradle-9.3.0-bin.zip → Redirects to https://release-assets.githubusercontent.com/github-production-release-asset..., which resolves correctly.
HEAD request with auth(it does not matter if it is valid): Same URL → Redirects to https://objects.githubusercontent.com/…, which doesn’t resolve and consistently returns 401.
There was a problem hiding this comment.
I tested this on GitHub Enterprise, and the behavior is slightly different. In our environment, SAML/SSO is required, so public download URLs do not work outside of a browser session. I can only assume that even there, the download must be done using the GitHub API
There was a problem hiding this comment.
Scope this to only github.com (not a complete fix since GHES exists, but narrows the blast radius)
I updated it to scope it only for github.com. But as described above, in GHE this won't work because no client in GHE should try to reach public urls (at least not if using SSO). That said, I am not sure about all the combinations or deployment options
yeikel
force-pushed
the
fix/public-auth-calls
branch
2 times, most recently
from
4dd5055 to
a745ec4
Compare
internal/handlers/git_server.go
Outdated
|
|
||
| // GitHub release download URLs are public | ||
| // and do not require authentication | ||
| if len(credsForRequest) != 0 && isGitHubReleaseDownload(r.URL.Path) { |
There was a problem hiding this comment.
My attempt at preventing potential cross-type detection is to ensure that this is considered a git host and that we have stored credentials for it. I don't know if this is good enough
There was a problem hiding this comment.
This worries me for private repos. If someone has a private GitHub repo with release assets, they need auth on the initial request to github.com to even get the 302 redirect URL. This returns nil for all release downloads, public or private.
The existing retry logic in HandleResponse handles 401/403/404 by stripping auth and retrying, but that only applies to the same request. The one failing here is the redirect target (objects.githubusercontent.com), which is a different request from the proxy's perspective.
Have you tested this against a private repo with release assets?
There was a problem hiding this comment.
Have you tested this against a private repo with release assets?
I tested this and it does not work in its current form. However, the behavior is consistent with GitHub’s documented expectations.
Artifacts from private repositories cannot be downloaded via "public" (what we can see from the browser) release asset URL (for example, https://github.com/gradle/gradle-distributions/releases/download/v9.3.0/gradle-9.3.0-bin.zip), even when authentication is provided. GitHub instead requires private release assets to be accessed through the GitHub API and downloaded using the ASSET-ID. In the proxy, the closest equivalent would be the github_api handler.
I may be mistaken, but from the proxy’s perspective this appears to be out of scope. Rather, the client (such as Dependabot Core) would need to be aware of this in advance and issue an API call instead.
yeikel
force-pushed
the
fix/public-auth-calls
branch
2 times, most recently
from
f8ef079 to
5d7b9c9
Compare
yeikel
force-pushed
the
fix/public-auth-calls
branch
from
5d7b9c9 to
5ffa3af
Compare
2 participants
The proxy should not assume that asset release downloads require authentication. These endpoints are public, and including a token can actually cause access to fail with an unauthorized error.
For additional context and analysis see #15 (comment)
Fixes #15
Logs:
Before this change
After this change