Fix github_alpine .apk redirect to resolve stored FilePath
apk reconstructs the download URL itself as <arch>/<name>-<version>.apk because APKINDEX carries no filename field (unlike rpm's <location> 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.
This commit is contained in:
@@ -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
|
||||
// "<arch>/<name>-<version>.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: <arch>/<name>-<version>.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, "/")
|
||||
|
||||
@@ -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
|
||||
// "<arch>/<name>-<version>.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 "<arch>/<name>-<version>.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"))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user