From a186bd205e412681e1bde635d240ea5c8427ab58 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 08:10:23 +0100 Subject: [PATCH] Fall back when release metadata exceeds bounds GitHub's release list now exceeds the update check's 1 MiB safety limit, causing valid installations to report that updates are unavailable. Keep the bound and use the existing bounded Atom feed only for the typed over-limit condition so malformed metadata remains a hard error. Accept both release-name and bare-tag feed entries, and do not substitute the public GitHub feed for a configured custom update server. Contract-Neutral: Update discovery error handling only; no wire, persisted-data, or subsystem contract change Refs: #1881 Change-source: pulse-maintainer --- internal/securityutil/responsebody.go | 25 ++++- internal/securityutil/responsebody_test.go | 6 ++ internal/updates/manager.go | 23 ++++- internal/updates/manager_retry_test.go | 114 +++++++++++++++++++-- 4 files changed, 158 insertions(+), 10 deletions(-) diff --git a/internal/securityutil/responsebody.go b/internal/securityutil/responsebody.go index d837fc964..25e4a9cd1 100644 --- a/internal/securityutil/responsebody.go +++ b/internal/securityutil/responsebody.go @@ -1,11 +1,30 @@ package securityutil import ( + "errors" "fmt" "io" "net/http" ) +// ResponseBodyTooLargeError reports that an HTTP response exceeded its +// caller-defined byte limit. The concrete type lets callers choose a bounded +// fallback without treating malformed or truncated responses the same way. +type ResponseBodyTooLargeError struct { + Limit int64 +} + +func (e *ResponseBodyTooLargeError) Error() string { + return fmt.Sprintf("response body exceeds %d bytes", e.Limit) +} + +// IsResponseBodyTooLarge reports whether err was caused by a response crossing +// the limit enforced by LimitResponseBody. +func IsResponseBodyTooLarge(err error) bool { + var target *ResponseBodyTooLargeError + return errors.As(err, &target) +} + // LimitResponseBody bounds the bytes a caller can read from an HTTP response. // It closes responses whose declared size already exceeds the limit. Responses // without a trustworthy Content-Length remain bounded while they are read. @@ -18,7 +37,7 @@ func LimitResponseBody(resp *http.Response, limit int64) error { } if resp.ContentLength > limit { _ = resp.Body.Close() - return fmt.Errorf("response body exceeds %d bytes", limit) + return &ResponseBodyTooLargeError{Limit: limit} } resp.Body = &limitedResponseBody{ @@ -41,7 +60,7 @@ func (r *limitedResponseBody) Read(p []byte) (int, error) { return 0, nil } if r.exceeded { - return 0, fmt.Errorf("response body exceeds %d bytes", r.limit) + return 0, &ResponseBodyTooLargeError{Limit: r.limit} } if r.remaining > 0 { if int64(len(p)) > r.remaining { @@ -58,7 +77,7 @@ func (r *limitedResponseBody) Read(p []byte) (int, error) { n, err := r.body.Read(probe[:]) if n > 0 { r.exceeded = true - return 0, fmt.Errorf("response body exceeds %d bytes", r.limit) + return 0, &ResponseBodyTooLargeError{Limit: r.limit} } return 0, err } diff --git a/internal/securityutil/responsebody_test.go b/internal/securityutil/responsebody_test.go index 75da612a7..f393b5f01 100644 --- a/internal/securityutil/responsebody_test.go +++ b/internal/securityutil/responsebody_test.go @@ -26,6 +26,9 @@ func TestLimitResponseBody(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "response body exceeds 8 bytes") { t.Fatalf("LimitResponseBody() error = %v", err) } + if !IsResponseBodyTooLarge(err) { + t.Fatalf("IsResponseBodyTooLarge(%v) = false, want true", err) + } if !body.closed { t.Fatal("oversized response body was not closed") } @@ -62,6 +65,9 @@ func TestLimitResponseBody(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "response body exceeds 8 bytes") { t.Fatalf("ReadAll() error = %v", err) } + if !IsResponseBodyTooLarge(err) { + t.Fatalf("IsResponseBodyTooLarge(%v) = false, want true", err) + } if string(got) != "12345678" { t.Fatalf("ReadAll() returned bytes beyond limit: %q", got) } diff --git a/internal/updates/manager.go b/internal/updates/manager.go index 5de90f264..cd1c1295e 100644 --- a/internal/updates/manager.go +++ b/internal/updates/manager.go @@ -932,6 +932,25 @@ func (m *Manager) getLatestReleaseForChannel(ctx context.Context, channel string var releases []ReleaseInfo if err := decodeReleaseMetadata(resp, &releases); err != nil { + // The GitHub collection embeds every asset for every returned release, + // so a valid response can outgrow the metadata limit as history grows. + // Retain the bound and use GitHub's separately bounded Atom feed only for + // that typed condition. A custom update server must not be silently + // replaced by github.com, and malformed metadata remains a hard failure. + if securityutil.IsResponseBodyTooLarge(err) && strings.TrimSpace(os.Getenv("PULSE_UPDATE_SERVER")) == "" { + log.Warn(). + Str("channel", channel). + Int64("limitBytes", maxReleaseMetadataBytes). + Msg("GitHub release metadata exceeds safety limit, trying Atom fallback") + feedRelease, feedErr := m.getLatestReleaseFromFeed(ctx, channel) + if feedErr == nil { + log.Info(). + Str("version", feedRelease.TagName). + Msg("Got release info from Atom fallback") + return feedRelease, nil + } + return nil, fmt.Errorf("failed to decode releases: %w; release feed fallback failed: %v", err, feedErr) + } return nil, fmt.Errorf("failed to decode releases: %w", err) } @@ -1075,7 +1094,9 @@ func (m *Manager) getLatestReleaseFromFeed(ctx context.Context, channel string) if len(feed.Entries) == 0 { return nil, fmt.Errorf("no version tags found in feed") } - versionTitleRegex := regexp.MustCompile(`^Pulse (v\d+\.\d+\.\d+(?:-[a-zA-Z0-9.]+)?)$`) + // GitHub uses the release name when present ("Pulse v6.4.1") and otherwise + // falls back to the bare tag ("v6.4.2") for Atom entry titles. + versionTitleRegex := regexp.MustCompile(`^(?:Pulse )?(v\d+\.\d+\.\d+(?:-[a-zA-Z0-9.]+)?)$`) // Pick the highest version matching the channel rather than the first // entry: the feed is publication-ordered and v5-line maintenance releases diff --git a/internal/updates/manager_retry_test.go b/internal/updates/manager_retry_test.go index b8e88de7e..7f4f30691 100644 --- a/internal/updates/manager_retry_test.go +++ b/internal/updates/manager_retry_test.go @@ -5,7 +5,6 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" - "fmt" "io" "net/http" "net/http/httptest" @@ -18,6 +17,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" ) func setRetrySettingsForTest(t *testing.T, attempts int, backoff, maxBackoff time.Duration) { @@ -172,10 +172,74 @@ func TestGetLatestReleaseForChannelRetriesTransientStatus(t *testing.T) { } } -func TestGetLatestReleaseForChannelRejectsStreamingOversizedResponse(t *testing.T) { +func TestGetLatestReleaseForChannelFallsBackWhenReleaseMetadataIsOversized(t *testing.T) { setRetrySettingsForTest(t, 1, time.Millisecond, time.Millisecond) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + feed := ` + + + Pulse v6.4.0-rc.11 + 2026-08-28T12:52:29Z + + + v6.4.2 + 2026-08-31T19:08:45Z + +` + + origTransport := http.DefaultTransport + http.DefaultTransport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + status := http.StatusNotFound + body := "not found" + header := http.Header{"Content-Type": []string{"text/plain"}} + switch req.URL.String() { + case "https://api.github.com/repos/rcourtman/Pulse/releases": + status = http.StatusOK + body = `[{"tag_name":"v9.9.9","body":"` + + strings.Repeat("x", int(maxReleaseMetadataBytes)) + `"}]` + header.Set("Content-Type", "application/json") + case "https://github.com/rcourtman/Pulse/releases.atom": + status = http.StatusOK + body = feed + header.Set("Content-Type", "application/atom+xml") + } + return &http.Response{ + StatusCode: status, + Status: http.StatusText(status), + Body: io.NopCloser(strings.NewReader(body)), + ContentLength: -1, + Header: header, + Request: req, + }, nil + }) + t.Cleanup(func() { http.DefaultTransport = origTransport }) + + manager := NewManager(&config.Config{UpdateChannel: "stable"}) + currentVer, err := ParseVersion("6.4.1") + if err != nil { + t.Fatalf("ParseVersion: %v", err) + } + + release, err := manager.getLatestReleaseForChannel(context.Background(), "stable", currentVer) + if err != nil { + t.Fatalf("getLatestReleaseForChannel error: %v", err) + } + if release.TagName != "v6.4.2" { + t.Fatalf("release tag = %q, want bare-tag feed release v6.4.2", release.TagName) + } + expectedAsset, supported := updateReleaseAssetForRuntime(release.TagName) + if !supported { + t.Fatalf("test runner architecture %q must map to a release asset", runtime.GOARCH) + } + if len(release.Assets) != 1 || release.Assets[0] != expectedAsset { + t.Fatalf("release assets = %+v, want %+v", release.Assets, expectedAsset) + } +} + +func TestGetLatestReleaseForChannelDoesNotReplaceCustomOversizedMetadata(t *testing.T) { + setRetrySettingsForTest(t, 1, time.Millisecond, time.Millisecond) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") w.(http.Flusher).Flush() _, _ = io.WriteString(w, `[{"tag_name":"v9.9.9","body":"`) @@ -186,14 +250,52 @@ func TestGetLatestReleaseForChannelRejectsStreamingOversizedResponse(t *testing. t.Setenv("PULSE_UPDATE_SERVER", server.URL) manager := NewManager(&config.Config{UpdateChannel: "stable"}) - currentVer, err := ParseVersion("1.0.0") + currentVer, err := ParseVersion("6.4.1") if err != nil { t.Fatalf("ParseVersion: %v", err) } _, err = manager.getLatestReleaseForChannel(context.Background(), "stable", currentVer) - if err == nil || !strings.Contains(err.Error(), fmt.Sprintf("response body exceeds %d bytes", maxReleaseMetadataBytes)) { - t.Fatalf("getLatestReleaseForChannel error = %v, want response size rejection", err) + if !securityutil.IsResponseBodyTooLarge(err) { + t.Fatalf("getLatestReleaseForChannel error = %v, want typed response size rejection", err) + } +} + +func TestGetLatestReleaseForChannelDoesNotMaskMalformedMetadata(t *testing.T) { + setRetrySettingsForTest(t, 1, time.Millisecond, time.Millisecond) + + var feedHits atomic.Int32 + origTransport := http.DefaultTransport + http.DefaultTransport = roundTripperFunc(func(req *http.Request) (*http.Response, error) { + body := "{" + status := http.StatusOK + if req.URL.String() == "https://github.com/rcourtman/Pulse/releases.atom" { + feedHits.Add(1) + body = `Pulse v9.9.9` + } + return &http.Response{ + StatusCode: status, + Status: http.StatusText(status), + Body: io.NopCloser(strings.NewReader(body)), + ContentLength: int64(len(body)), + Header: make(http.Header), + Request: req, + }, nil + }) + t.Cleanup(func() { http.DefaultTransport = origTransport }) + + manager := NewManager(&config.Config{UpdateChannel: "stable"}) + currentVer, err := ParseVersion("6.4.1") + if err != nil { + t.Fatalf("ParseVersion: %v", err) + } + + _, err = manager.getLatestReleaseForChannel(context.Background(), "stable", currentVer) + if err == nil || !strings.Contains(err.Error(), "failed to decode releases") { + t.Fatalf("getLatestReleaseForChannel error = %v, want JSON decode failure", err) + } + if got := feedHits.Load(); got != 0 { + t.Fatalf("Atom feed requests = %d, want 0 for malformed metadata", got) } }