Add generic HTTP download proxy for GitHub release assets (mise/aqua) - #302
Add generic HTTP download proxy for GitHub release assets (mise/aqua)#302gilesw wants to merge 1 commit into
Conversation
Adds a /generic/{name}/ route backed by a new upstream.generic named-upstream
map, so tools that download from fixed URL shapes (mise's aqua backend
fetching GitHub release assets, and its tag lookups on api.github.com) can be
pointed at the proxy with client-side URL rewriting. Only configured
upstreams are reachable, so this is not an open HTTP proxy.
Paths shaped like {owner}/{repo}/releases/download/{tag}/{asset} are
version-pinned and go through the artifact cache: fetched once, hashed,
served without revalidation, and still served when the upstream is down.
Every other path goes through the metadata cache with the client's Accept
header and query string replayed, so API responses are fresh within
metadata_ttl, revalidated after that, and served stale when the upstream
fails or rate-limits the request.
Tests cover path classification, unknown upstreams and traversal, cache
hits with the upstream down, HEAD, 404 pass-through, Accept and query
forwarding, stale-on-429, cache isolation between upstreams, and that an
upstream token scoped to the release host is not sent to the object store
it redirects to.
Closes git-pkgs#183.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RDjDeq27CKzEP2o3GWBY7F
There was a problem hiding this comment.
🟡 Changes recommended
There is a confirmed metadata caching key correctness bug (Accept not included) and multiple data races in new tests that can fail under go test -race.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new allow-listed “generic” HTTP proxy route intended for caching GitHub release assets and GitHub API responses (e.g., for mise/aqua), wiring it into server routing and configuration while documenting setup and rewrite rules.
Changes:
- Introduces
/generic/{name}/*backed byupstream.genericnamed upstreams and implements artifact-vs-metadata caching behavior for GitHub release asset URL shapes. - Extends configuration schema + validation and adds dedicated handler tests for routing, caching behavior, and auth scoping across redirects.
- Updates README/docs/config examples to describe the new route and provide mise
url_replacementsexamples.
File summaries
| File | Description |
|---|---|
| README.md | Documents the new generic route and mise/aqua rewrite configuration. |
| internal/server/server.go | Mounts the new /generic handler. |
| internal/handler/generic.go | Implements the generic proxy handler with GitHub release-asset classification and caching. |
| internal/handler/generic_test.go | Adds tests for path classification, caching behavior, and auth scoping. |
| internal/config/config.go | Adds upstream.generic to config and validates named upstreams. |
| internal/config/config_test.go | Extends validation tests to cover upstream.generic. |
| docs/configuration.md | Documents generic upstream configuration and cache behavior. |
| config.example.yaml | Adds example commented config for upstream.generic. |
Review details
Suppressed comments (4)
internal/handler/generic_test.go:210
- Follow-up to the atomic.Value change above: these reads should use Load() instead of reading unsynchronized variables.
if gotAccept != "application/vnd.github+json" {
t.Errorf("upstream Accept = %q, want the client's header replayed", gotAccept)
}
if gotQuery != "per_page=1" {
t.Errorf("upstream query = %q, want %q", gotQuery, "per_page=1")
internal/handler/generic_test.go:300
- releaseAuth is assigned in the httptest releaseHost handler goroutine and read later in the test goroutine without synchronization, which can cause a data race under
go test -race.
var releaseAuth string
releaseHost := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
releaseAuth = r.Header.Get("Authorization")
if releaseAuth != "Bearer github-token" {
http.Error(w, "unauthorized", http.StatusUnauthorized)
internal/handler/generic_test.go:238
- gotAccept is set inside the httptest server goroutine and read in the test goroutine without synchronization, which can cause a data race under
go test -race.
var gotAccept string
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
gotAccept = r.Header.Get("Accept")
_, _ = w.Write([]byte("checksums"))
}))
internal/handler/generic_test.go:329
- Follow-up to switching releaseAuth to atomic.Value: this comparison should Load() the stored value.
if releaseAuth != "Bearer github-token" {
t.Errorf("release host Authorization = %q, want the configured token", releaseAuth)
}
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| accept := r.Header.Get("Accept") | ||
| if accept == "" { | ||
| accept = genericAcceptAny | ||
| } | ||
|
|
||
| h.proxy.ProxyCached(w, r, target, genericEcosystem, | ||
| h.metadataCacheKey(repository, upstreamURL, path, r.URL.RawQuery), accept) | ||
| } | ||
|
|
||
| // metadataCacheKey derives the metadata cache key from the upstream name, its | ||
| // URL, the request path and query. Hashing the identity keeps distinct | ||
| // upstreams from sharing entries and drops cached entries when an upstream is | ||
| // repointed, mirroring APKHandler.metadataCacheKey. | ||
| func (h *GenericHandler) metadataCacheKey(repository, upstreamURL, path, query string) string { | ||
| identity := repository + "\x00" + upstreamURL + "\x00" + path + "\x00" + query | ||
| digest := sha256.Sum256([]byte(identity)) | ||
| return hex.EncodeToString(digest[:]) | ||
| } |
| body := `{"tag_name":"jq-1.7.1"}` | ||
| var throttled atomic.Bool | ||
| var gotAccept, gotQuery string | ||
|
|
||
| upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| if r.URL.Path != apiPath { | ||
| http.NotFound(w, r) | ||
| return | ||
| } | ||
| gotAccept = r.Header.Get("Accept") | ||
| gotQuery = r.URL.RawQuery | ||
| if throttled.Load() { |
Closes #183.
What
A
/generic/{name}/route backed by a newupstream.genericnamed-upstream map, following the same shape asupstream.helmandupstream.apk:Only configured upstreams are reachable, so this is not an open HTTP proxy. The remaining request path and query string are appended to the upstream URL.
Cache behaviour
{owner}/{repo}/releases/download/{tag}/{asset}are version-pinned GitHub release assets. They go through the artifact cache: fetched once, hashed, served without revalidation, and still served when the upstream is down. The configured upstream name is part of the cache identity so two upstreams serving the same path never share bytes.Acceptheader and query string replayed. That coversapi.github.com/repos/{owner}/{repo}/releases/tags/{tag}: fresh withinmetadata_ttl, revalidated with the upstream's validators after that, and served stale (withWarning: 110) when the upstream fails or rate-limits.Upstream auth uses the existing
upstream.authURL-scope map, so a token forhttps://api.github.comis applied per hop and never reaches the object store GitHub redirects asset downloads to.Why
mise's aqua backend installs most tools from GitHub release assets and resolves tags via
api.github.com. With mise'surl_replacementsboth can be pointed at this route, giving a fleet one shared cache and installs that keep working during a GitHub outage or rate-limit. The README gains a section with the two rewrite rules. This matches the design sketched in #183 (allow-listed upstreams, upstream URL in the path so it works as a prefix, cache-forever for pinned assets).Tests
releases/latest/downloadshape and API paths that must not be treated as assetsAcceptand query forwarded; stale body withWarning: 110when the upstream returns 429go test ./...andgo tool golangci-lint run ./...are clean.Out of scope, happy to follow up
Retry-After-aware backoff when the upstream throttles