From d1c3c2fb6c1198f709ce991f5c21c6cc696e16f5 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Wed, 12 Aug 2026 01:28:25 +1000 Subject: [PATCH] Fix github_alpine .apk redirect to resolve stored FilePath apk reconstructs the download URL itself as /-.apk because APKINDEX carries no filename field (unlike rpm's or deb's Filename:). ServeRemote forwarded that synthesized path verbatim into the releases_remote redirect, pointing at a nonexistent, allowlist-denied github.com path (404/403). Look up the cached metadata row by arch plus the full reconstructed filename (no hyphen-split, so -rN suffixes are preserved) and redirect to the stored github-relative FilePath. Unknown packages now 404 instead of redirecting to a bad path. --- internal/provider/alpine/github.go | 39 +++++++++++++++++++++++-- internal/provider/alpine/github_test.go | 34 ++++++++++++++++++--- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/internal/provider/alpine/github.go b/internal/provider/alpine/github.go index fda585c..13108ca 100644 --- a/internal/provider/alpine/github.go +++ b/internal/provider/alpine/github.go @@ -146,14 +146,49 @@ func (p *GitHubProvider) ServeRemote(w http.ResponseWriter, r *http.Request, rem http.Error(w, "github_alpine remote has no releases_remote configured for downloads", http.StatusInternalServerError) return true } - loc := strings.TrimRight(proxyBaseURL, "/") + "/api/v1/remote/" + remote.ReleasesRemote + "/" + strings.TrimLeft(path, "/") - http.Redirect(w, r, loc, http.StatusFound) + p.serveApkRedirect(w, r, remote, path, proxyBaseURL, store) return true } return false } +// serveApkRedirect resolves an apk-reconstructed download path — apk builds +// "/-.apk" itself because APKINDEX carries no filename — to +// the real github-relative asset path stored on the metadata row, then redirects +// to the backend releases_remote. Passing the inbound path through verbatim would +// point at a nonexistent, allowlist-denied github.com path. +func (p *GitHubProvider) serveApkRedirect(w http.ResponseWriter, r *http.Request, remote models.Remote, path, proxyBaseURL string, store provider.RemoteMetadataStore) { + arch := strings.TrimSuffix(path[:strings.LastIndex(path, "/")+1], "/") + basename := path[strings.LastIndex(path, "/")+1:] + if arch == "" || strings.Contains(arch, "/") { + http.Error(w, "apk download must be requested per-arch: /-.apk", http.StatusNotFound) + return + } + + reader, ok := store.(provider.AlpineMetadataReader) + if !ok { + http.Error(w, "alpine metadata not available", http.StatusInternalServerError) + return + } + sctx, cancel := context.WithTimeout(context.WithoutCancel(r.Context()), p.serveTimeout) + defer cancel() + rows, err := reader.ListAlpineMetadataEntries(sctx, remote.Name) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + for _, row := range rows { + if row.Arch == arch && row.Name+"-"+row.Version+".apk" == basename { + loc := strings.TrimRight(proxyBaseURL, "/") + "/api/v1/remote/" + remote.ReleasesRemote + "/" + strings.TrimLeft(row.FilePath, "/") + http.Redirect(w, r, loc, http.StatusFound) + return + } + } + http.Error(w, "package not found", http.StatusNotFound) +} + func (p *GitHubProvider) serveIndex(w http.ResponseWriter, r *http.Request, remote models.Remote, path string, store provider.RemoteMetadataStore) { arch := strings.TrimSuffix(path, "APKINDEX.tar.gz") arch = strings.Trim(arch, "/") diff --git a/internal/provider/alpine/github_test.go b/internal/provider/alpine/github_test.go index b038eaf..9f33fb0 100644 --- a/internal/provider/alpine/github_test.go +++ b/internal/provider/alpine/github_test.go @@ -273,10 +273,13 @@ func TestGitHubServeRemoteIndexAndRedirect(t *testing.T) { t.Fatalf("aarch64 index should not carry the x86_64 package: %s", got) } - // An .apk request redirects to the backend releases_remote. + // An .apk request arrives in apk's reconstructed shape + // "/-.apk" (APKINDEX carries no filename), NOT as the + // github-relative FilePath. ServeRemote must resolve it back to the stored + // FilePath before redirecting to the backend releases_remote. rec = httptest.NewRecorder() - req = httptest.NewRequest(http.MethodGet, "/api/v1/remote/acme-apk/"+demoPath, nil) - if !p.ServeRemote(rec, req, remote, demoPath, proxyBase, store) { + req = httptest.NewRequest(http.MethodGet, "/api/v1/remote/acme-apk/x86_64/demo-1.2.3-r0.apk", nil) + if !p.ServeRemote(rec, req, remote, "x86_64/demo-1.2.3-r0.apk", proxyBase, store) { t.Fatal("ServeRemote did not handle .apk") } if rec.Code != http.StatusFound { @@ -284,7 +287,30 @@ func TestGitHubServeRemoteIndexAndRedirect(t *testing.T) { } wantLoc := proxyBase + "/api/v1/remote/github/" + demoPath if got := rec.Header().Get("Location"); got != wantLoc { - t.Fatalf("Location = %q, want %q", got, wantLoc) + t.Fatalf("Location = %q, want %q (must be the stored FilePath, not the inbound path)", got, wantLoc) + } +} + +// An apk download whose reconstructed "/-.apk" matches no +// cached row must 404, never redirect to a bad path. +func TestGitHubServeRemoteApkRedirectNotFound(t *testing.T) { + fx := newGitHubFixture(t, true) + p := newTestProvider() + store := newFakeStore() + remote := fx.remote() + + // Warm the cache so the store is populated but lacks the requested package. + if err := p.scan(context.Background(), remote, store); err != nil { + t.Fatalf("warm scan: %v", err) + } + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/v1/remote/acme-apk/x86_64/nope-9.9.9.apk", nil) + if !p.ServeRemote(rec, req, remote, "x86_64/nope-9.9.9.apk", "https://x", store) { + t.Fatal("ServeRemote did not handle .apk") + } + if rec.Code != http.StatusNotFound { + t.Fatalf("want 404 for unknown package, got %d (Location=%q)", rec.Code, rec.Header().Get("Location")) } }