From c69a357859683194fe24352bd1f4877926ed94a5 Mon Sep 17 00:00:00 2001 From: Rinat Khaziev Date: Fri, 11 Sep 2026 16:56:01 -0500 Subject: [PATCH 1/2] fix: accept GitHub repository pagination URLs --- internal/update/github.go | 53 ++++++++++++++++++++++----------- internal/update/release_test.go | 52 +++++++++++++++++++++++--------- 2 files changed, 74 insertions(+), 31 deletions(-) diff --git a/internal/update/github.go b/internal/update/github.go index ec9871168..da9463ff5 100644 --- a/internal/update/github.go +++ b/internal/update/github.go @@ -64,24 +64,26 @@ func (g GitHub) Releases(ctx context.Context) ([]Release, error) { } releases = append(releases, batch...) next := 0 - for _, part := range strings.Split(resp.Header.Get("Link"), ",") { - if !strings.Contains(part, `rel="next"`) { - continue + for _, value := range resp.Header.Values("Link") { + for _, part := range strings.Split(value, ",") { + if !strings.Contains(part, `rel="next"`) { + continue + } + left, right := strings.Index(part, "<"), strings.Index(part, ">") + if left < 0 || right <= left { + return nil, fmt.Errorf("invalid release pagination") + } + u, err := url.Parse(part[left+1 : right]) + if err != nil { + return nil, fmt.Errorf("invalid release pagination") + } + u = origin.ResolveReference(u) + n, err := strconv.Atoi(u.Query().Get("page")) + if err != nil || n <= page || u.Scheme != origin.Scheme || u.Host != origin.Host || !isReleasePaginationPath(u.Path) || u.User != nil || next != 0 { + return nil, fmt.Errorf("invalid release pagination") + } + next = n } - left, right := strings.Index(part, "<"), strings.Index(part, ">") - if left < 0 || right <= left { - return nil, fmt.Errorf("invalid release pagination") - } - u, err := url.Parse(part[left+1 : right]) - if err != nil { - return nil, fmt.Errorf("invalid release pagination") - } - u = origin.ResolveReference(u) - n, err := strconv.Atoi(u.Query().Get("page")) - if err != nil || n <= page || u.Scheme != origin.Scheme || u.Host != origin.Host || u.Path != "/repos/"+Repository+"/releases" || u.User != nil || next != 0 { - return nil, fmt.Errorf("invalid release pagination") - } - next = n } if next == 0 { return releases, nil @@ -90,6 +92,23 @@ func (g GitHub) Releases(ctx context.Context) ([]Release, error) { } return nil, fmt.Errorf("GitHub release pagination limit exceeded") } + +func isReleasePaginationPath(path string) bool { + if path == "/repos/"+Repository+"/releases" { + return true + } + repositoryID, ok := strings.CutPrefix(path, "/repositories/") + if !ok { + return false + } + repositoryID, ok = strings.CutSuffix(repositoryID, "/releases") + if !ok { + return false + } + id, err := strconv.ParseUint(repositoryID, 10, 64) + return err == nil && id > 0 +} + func readBounded(r io.Reader, max int64) ([]byte, error) { b, err := io.ReadAll(io.LimitReader(r, max+1)) if err != nil { diff --git a/internal/update/release_test.go b/internal/update/release_test.go index eb01f1a77..8234a9ece 100644 --- a/internal/update/release_test.go +++ b/internal/update/release_test.go @@ -87,33 +87,57 @@ func TestSelectRejectsIncompleteAndInconsistentReleases(t *testing.T) { } } func TestGitHubPagination(t *testing.T) { + for _, tc := range []struct { + name, nextPath string + }{{"named repository", "/repos/Automattic/vip-cli/releases"}, {"numeric repository ID", "/repositories/116313791/releases"}} { + t.Run(tc.name, func(t *testing.T) { + calls := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + if r.Header.Get("Authorization") != "" { + t.Error("credentials") + } + if r.URL.Path != "/repos/Automattic/vip-cli/releases" { + t.Error(r.URL.Path) + } + if r.URL.Query().Get("page") == "1" { + w.Header().Set("Link", "; rel=\"next\"") + fmt.Fprint(w, `[{"tag_name":"4.9.0"}]`) + } else { + fmt.Fprint(w, `[{"tag_name":"5.0.0"}]`) + } + })) + defer s.Close() + rs, err := (GitHub{Client: s.Client(), BaseURL: s.URL}).Releases(context.Background()) + if err != nil || len(rs) != 2 || calls != 2 { + t.Fatal(rs, err, calls) + } + }) + } +} + +func TestGitHubRejectsDuplicateNextLinkHeaders(t *testing.T) { calls := 0 s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { calls++ - if r.Header.Get("Authorization") != "" { - t.Error("credentials") - } - if r.URL.Path != "/repos/Automattic/vip-cli/releases" { - t.Error(r.URL.Path) - } if r.URL.Query().Get("page") == "1" { - w.Header().Set("Link", "; rel=\"next\"") - fmt.Fprint(w, `[{"tag_name":"4.9.0"}]`) - } else { - fmt.Fprint(w, `[{"tag_name":"5.0.0"}]`) + w.Header().Add("Link", "; rel=\"next\"") + w.Header().Add("Link", "; rel=\"next\"") } + fmt.Fprint(w, `[]`) })) defer s.Close() - rs, err := (GitHub{Client: s.Client(), BaseURL: s.URL}).Releases(context.Background()) - if err != nil || len(rs) != 2 || calls != 2 { - t.Fatal(rs, err, calls) + _, err := (GitHub{Client: s.Client(), BaseURL: s.URL}).Releases(context.Background()) + if err == nil || calls != 1 { + t.Fatal(err, calls) } } + func TestGitHubErrors(t *testing.T) { for _, tc := range []struct { status int body, link string - }{{429, `{}`, ""}, {200, `broken`, ""}, {200, `[]`, `; rel="next"`}, {200, `[]`, `; rel="next"`}} { + }{{429, `{}`, ""}, {200, `broken`, ""}, {200, `[]`, `; rel="next"`}, {200, `[]`, `; rel="next"`}, {200, `[]`, `; rel="next"`}} { s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Link", tc.link) w.WriteHeader(tc.status) From 09f1486aca31ed4292b7eb5a5da7d7612e4c8d74 Mon Sep 17 00:00:00 2001 From: Rinat Khaziev Date: Mon, 14 Sep 2026 13:51:34 -0500 Subject: [PATCH 2/2] fix: reject encoded release pagination paths --- internal/update/github.go | 2 +- internal/update/release_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/internal/update/github.go b/internal/update/github.go index da9463ff5..9f6bb8c05 100644 --- a/internal/update/github.go +++ b/internal/update/github.go @@ -79,7 +79,7 @@ func (g GitHub) Releases(ctx context.Context) ([]Release, error) { } u = origin.ResolveReference(u) n, err := strconv.Atoi(u.Query().Get("page")) - if err != nil || n <= page || u.Scheme != origin.Scheme || u.Host != origin.Host || !isReleasePaginationPath(u.Path) || u.User != nil || next != 0 { + if err != nil || n <= page || u.Scheme != origin.Scheme || u.Host != origin.Host || !isReleasePaginationPath(u.EscapedPath()) || u.User != nil || next != 0 { return nil, fmt.Errorf("invalid release pagination") } next = n diff --git a/internal/update/release_test.go b/internal/update/release_test.go index 8234a9ece..da26113ee 100644 --- a/internal/update/release_test.go +++ b/internal/update/release_test.go @@ -133,6 +133,22 @@ func TestGitHubRejectsDuplicateNextLinkHeaders(t *testing.T) { } } +func TestGitHubRejectsEncodedReleasePaginationSeparator(t *testing.T) { + calls := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + if r.URL.Query().Get("page") == "1" { + w.Header().Set("Link", "; rel=\"next\"") + } + fmt.Fprint(w, `[]`) + })) + defer s.Close() + _, err := (GitHub{Client: s.Client(), BaseURL: s.URL}).Releases(context.Background()) + if err == nil || calls != 1 { + t.Fatal(err, calls) + } +} + func TestGitHubErrors(t *testing.T) { for _, tc := range []struct { status int