From daef38dd3cf38eef4852a52e47f475a4803e4cd3 Mon Sep 17 00:00:00 2001 From: "courtmanr@gmail.com" Date: Sun, 27 Sep 2026 21:54:03 +0100 Subject: [PATCH] Stream release metadata so update checks see stable releases (#2282) GitHub's release list is now ~9.6 MB because every 6.4.x release embeds 349 assets (~600 KB of JSON each). The update check buffered the whole response behind a 1 MiB bound, so the primary path always failed and fell through to the Atom feed. That feed holds ten entries and each release now spends three of them, so stable-channel checks on main failed outright ("no suitable release found for channel stable") and 6.4.5 stable would have shipped relying on it. Decode the release list and the single-release notes response as a token stream, keeping only tag, name, body, draft, prerelease, publish time and the Pulse linux server archives. Memory now tracks what the updater keeps, so the byte bound rises to 64 MiB while staying typed; the Atom feed stays as the last resort for that over-limit condition only. Verified against live GitHub: stable resolves v6.4.1 and rc resolves v6.4.5-rc.3 through the primary path with no feed fallback. --- .../subsystems/deployment-installability.md | 17 ++ .../issue2282_release_metadata_stream_test.go | 207 ++++++++++++++++++ internal/updates/manager.go | 33 +-- internal/updates/release_metadata.go | 182 +++++++++++++++ internal/updates/release_notes.go | 4 +- 5 files changed, 418 insertions(+), 25 deletions(-) create mode 100644 internal/updates/issue2282_release_metadata_stream_test.go create mode 100644 internal/updates/release_metadata.go diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 9ef0c11c6f..8c45a5d594 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -4027,6 +4027,23 @@ contract; the normal apply pipeline must still verify its pinned SSHSIG and checksum before installation. A fallback response that advertises an update with an empty download URL is a release-blocking defect because the UI cannot start the apply request. Proof: `internal/updates/manager_retry_test.go`. +The Atom feed is a last resort only, never the primary release source: it +carries just the ten newest entries and every Pulse release now spends three +of them (the release, its Helm chart release, and a release-convergence tag), +so it cannot see a stable release that sits behind a preview cycle. The +release-list path must therefore cope with GitHub's real payload, where each +6.4.x release embeds hundreds of assets and one 30-release page is several +megabytes. `internal/updates/release_metadata.go` streams the list and the +single-release notes response token by token under the +`maxReleaseMetadataBytes` byte bound, retaining only tag, name, body, draft, +prerelease, publish time, and the Pulse server archive assets, so memory +tracks what the updater keeps rather than the response size. Buffering the +whole response behind a small bound is a release-blocking regression: it made +every stable-channel check fail on main and left 6.4.3-rc.1, which has the +bound without the fallback, unable to discover any update (#1881, #2282). +Malformed metadata stays a hard error and only the typed over-limit condition +may fall through to the feed. Proof: +`internal/updates/issue2282_release_metadata_stream_test.go`. Those same workflows must also fetch and dispatch the governed release branch derived from release-control metadata instead of hardcoding `pulse/v6`, `pulse/v6-release`, `main`, or any later branch literal inline; when a stable diff --git a/internal/updates/issue2282_release_metadata_stream_test.go b/internal/updates/issue2282_release_metadata_stream_test.go new file mode 100644 index 0000000000..5c5d3cac99 --- /dev/null +++ b/internal/updates/issue2282_release_metadata_stream_test.go @@ -0,0 +1,207 @@ +package updates + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" +) + +// issue2282ReleaseJSON renders a release shaped like GitHub's real payload: +// hundreds of assets, each with a nested uploader object, so a 30-release page +// is several megabytes — the shape that broke the 1 MiB buffered decode. +func issue2282ReleaseJSON(t *testing.T, tag string, prerelease bool, assetCount int) map[string]any { + t.Helper() + uploader := map[string]any{ + "login": "rcourtman", "id": 1, "node_id": strings.Repeat("n", 40), + "avatar_url": "https://avatars.githubusercontent.com/u/1?v=4", + "url": "https://api.github.com/users/rcourtman", + "html_url": "https://github.com/rcourtman", + "type": "User", "site_admin": false, + } + assets := make([]map[string]any, 0, assetCount+1) + for i := 0; i < assetCount; i++ { + name := fmt.Sprintf("pulse-agent-%s-component-%03d.zip", tag, i) + assets = append(assets, map[string]any{ + "url": "https://api.github.com/repos/rcourtman/Pulse/releases/assets/1", + "id": i, + "name": name, + "label": nil, + "uploader": uploader, + "content_type": "application/zip", + "state": "uploaded", + "size": 123456, + "download_count": 42, + "browser_download_url": "https://github.com/rcourtman/Pulse/releases/download/" + tag + "/" + name, + }) + } + if runtimeAsset, ok := updateReleaseAssetForRuntime(tag); ok { + assets = append(assets, map[string]any{ + "name": runtimeAsset.Name, + "uploader": uploader, + "browser_download_url": runtimeAsset.BrowserDownloadURL, + }) + } + return map[string]any{ + "url": "https://api.github.com/repos/rcourtman/Pulse/releases/1", + "tag_name": tag, + "name": "Pulse " + tag, + "draft": false, + "prerelease": prerelease, + "published_at": "2026-09-27T12:00:00Z", + "author": uploader, + "assets": assets, + "body": "notes for " + tag, + "reactions": map[string]any{"+1": 3, "total_count": 3}, + } +} + +func TestIssue2282StableChannelSelectsStableFromMultiMegabyteReleaseList(t *testing.T) { + setRetrySettingsForTest(t, 1, time.Millisecond, time.Millisecond) + + // Mirror the live page that broke stable checks: prereleases and + // helm-chart releases crowd the newest entries and the stable sits deep. + var releases []map[string]any + for _, tag := range []string{"v6.4.5-rc.3", "v6.4.5-rc.2", "v6.4.5-rc.1", "v6.4.5-beta.3", "v6.4.5-beta.1", "v6.4.4-beta.2", "v6.4.4-beta.1", "v6.4.3-rc.1"} { + releases = append(releases, issue2282ReleaseJSON(t, tag, true, 349)) + releases = append(releases, issue2282ReleaseJSON(t, "helm-chart-"+strings.TrimPrefix(tag, "v"), true, 0)) + } + releases = append(releases, + issue2282ReleaseJSON(t, "v6.4.1", false, 219), + issue2282ReleaseJSON(t, "v6.4.0", false, 226), + issue2282ReleaseJSON(t, "helm-chart-6.4.1", false, 0), + ) + payload, err := json.Marshal(releases) + if err != nil { + t.Fatalf("marshal releases: %v", err) + } + if len(payload) <= 1<<20 { + t.Fatalf("fixture is %d bytes, want it larger than the old 1 MiB bound", len(payload)) + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(payload) + })) + defer server.Close() + t.Setenv("PULSE_UPDATE_SERVER", server.URL) + + manager := NewManager(&config.Config{UpdateChannel: "stable"}) + for _, tc := range []struct { + channel, current, want string + }{ + {"stable", "6.4.1", "v6.4.1"}, + {"stable", "6.0.0-rc.2", "v6.4.1"}, + {"rc", "6.4.3-rc.1", "v6.4.5-rc.3"}, + } { + currentVer, err := ParseVersion(tc.current) + if err != nil { + t.Fatalf("ParseVersion(%q): %v", tc.current, err) + } + release, err := manager.getLatestReleaseForChannel(context.Background(), tc.channel, currentVer) + if err != nil { + t.Fatalf("%s channel from %s: getLatestReleaseForChannel error: %v", tc.channel, tc.current, err) + } + if release.TagName != tc.want { + t.Fatalf("%s channel from %s: release = %q, want %q", tc.channel, tc.current, release.TagName, tc.want) + } + if release.Body != "notes for "+tc.want { + t.Fatalf("%s channel: release notes = %q, want the streamed body", tc.channel, release.Body) + } + runtimeAsset, ok := updateReleaseAssetForRuntime(tc.want) + if !ok { + continue + } + if len(release.Assets) != 1 || release.Assets[0] != runtimeAsset { + t.Fatalf("%s channel: retained assets = %+v, want only runtime asset %+v", tc.channel, release.Assets, runtimeAsset) + } + } +} + +func TestIssue2282ReleaseNotesDecodeLargeSingleRelease(t *testing.T) { + payload, err := json.Marshal(issue2282ReleaseJSON(t, "v6.4.5", false, 2000)) + if err != nil { + t.Fatalf("marshal release: %v", err) + } + if len(payload) <= 1<<20 { + t.Fatalf("fixture is %d bytes, want it larger than the old 1 MiB bound", len(payload)) + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(payload) + })) + defer server.Close() + t.Setenv("PULSE_UPDATE_SERVER", server.URL) + + manager := NewManager(&config.Config{UpdateChannel: "stable"}) + info, err := manager.GetReleaseNotes(context.Background(), "v6.4.5") + if err != nil { + t.Fatalf("GetReleaseNotes error: %v", err) + } + if info.Version != "6.4.5" || info.ReleaseNotes != "notes for v6.4.5" || info.IsPrerelease { + t.Fatalf("release notes = %+v, want 6.4.5 stable notes", info) + } +} + +func issue2282Response(body string) *http.Response { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(body)), + ContentLength: -1, + } +} + +func TestIssue2282DecodeReleaseListShapes(t *testing.T) { + releases, err := decodeReleaseList(issue2282Response(`[ + {"tag_name":"v6.4.1","name":null,"body":null,"published_at":null,"assets":null, + "author":{"nested":[{"deep":[1,2,{"x":"y"}]}]},"draft":false,"prerelease":false}, + {"draft":true,"tag_name":"v6.5.0","published_at":"2026-09-30T08:00:00Z", + "assets":[{"name":"pulse-v6.5.0-linux-amd64.tar.gz","browser_download_url":"https://example.invalid/a"}, + {"name":"pulse-v6.5.0-darwin-arm64.tar.gz","browser_download_url":"https://example.invalid/b"}, + {"name":"checksums.txt","browser_download_url":"https://example.invalid/c"}]} + ]`)) + if err != nil { + t.Fatalf("decodeReleaseList error: %v", err) + } + if len(releases) != 2 { + t.Fatalf("decoded %d releases, want 2", len(releases)) + } + if first := releases[0]; first.TagName != "v6.4.1" || first.Name != "" || first.Body != "" || !first.PublishedAt.IsZero() || first.Assets != nil || first.Draft || first.Prerelease { + t.Fatalf("null-field release = %+v", first) + } + second := releases[1] + if !second.Draft || second.TagName != "v6.5.0" || !second.PublishedAt.Equal(time.Date(2026, 9, 30, 8, 0, 0, 0, time.UTC)) { + t.Fatalf("second release = %+v", second) + } + if len(second.Assets) != 1 || second.Assets[0].Name != "pulse-v6.5.0-linux-amd64.tar.gz" { + t.Fatalf("retained assets = %+v, want only the linux server archive", second.Assets) + } + + for name, body := range map[string]string{ + "object instead of list": `{"message":"Not Found"}`, + "trailing data": `[] []`, + "truncated": `[{"tag_name":"v6.4.1"`, + "wrong field type": `[{"prerelease":"yes"}]`, + "assets not a list": `[{"assets":{"name":"x"}}]`, + } { + if _, err := decodeReleaseList(issue2282Response(body)); err == nil { + t.Fatalf("%s: decodeReleaseList accepted malformed metadata", name) + } + } +} + +func TestIssue2282StreamedDecodeKeepsTypedByteBound(t *testing.T) { + body := `[{"tag_name":"v9.9.9","body":"` + strings.Repeat("x", int(maxReleaseMetadataBytes)) + `"}]` + _, err := decodeReleaseList(issue2282Response(body)) + if !securityutil.IsResponseBodyTooLarge(err) { + t.Fatalf("decodeReleaseList error = %v, want typed response size rejection", err) + } +} diff --git a/internal/updates/manager.go b/internal/updates/manager.go index 1235a30bd2..90911d2422 100644 --- a/internal/updates/manager.go +++ b/internal/updates/manager.go @@ -6,7 +6,6 @@ import ( "context" "crypto/sha256" "encoding/hex" - "encoding/json" "encoding/xml" "errors" "fmt" @@ -90,7 +89,7 @@ const ( defaultUpdateReleaseRepo string = "rcourtman/Pulse" defaultUpdateAPIBaseURL string = "https://api.github.com" maxReleaseFeedBytes int64 = 1 << 20 // 1 MiB - maxReleaseMetadataBytes int64 = 1 << 20 // 1 MiB + maxReleaseMetadataBytes int64 = 64 << 20 // 64 MiB, streamed (release_metadata.go) maxReleaseFeedCandidates int = 20 // Bound asset probes from a feed. maxChecksumFileBytes int64 = 1 << 20 // 1 MiB maxUpdateDownloadBytes int64 = 512 << 20 // 512 MiB @@ -100,17 +99,6 @@ const ( maxRetainedUpdateBackups int = 3 ) -func decodeReleaseMetadata(resp *http.Response, destination any) error { - if err := securityutil.LimitResponseBody(resp, maxReleaseMetadataBytes); err != nil { - return err - } - body, err := io.ReadAll(resp.Body) - if err != nil { - return err - } - return json.Unmarshal(body, destination) -} - func updateReleaseRepo() string { repo := strings.TrimSpace(os.Getenv("PULSE_GITHUB_REPO")) if repo == "" { @@ -525,9 +513,7 @@ func (m *Manager) CheckForUpdatesWithOptions(ctx context.Context, options Update // Fallback to any pulse tarball if exact match not found if downloadURL == "" { for _, asset := range release.Assets { - if strings.HasPrefix(asset.Name, "pulse-") && - strings.Contains(asset.Name, "linux") && - strings.HasSuffix(asset.Name, ".tar.gz") { + if isRuntimeReleaseAssetName(asset.Name) { downloadURL = asset.BrowserDownloadURL break } @@ -950,13 +936,14 @@ func (m *Manager) getLatestReleaseForChannel(ctx context.Context, channel string return nil, fmt.Errorf("GitHub API returned status %d: %s", resp.StatusCode, detail) } - 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. + releases, err := decodeReleaseList(resp) + if err != nil { + // The list is streamed, so the byte bound only trips on a response far + // larger than GitHub's current release pages. Keep GitHub's separately + // bounded Atom feed as the last resort for that typed condition only; + // the feed holds ten entries and cannot be the primary source. 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). diff --git a/internal/updates/release_metadata.go b/internal/updates/release_metadata.go new file mode 100644 index 0000000000..b196746122 --- /dev/null +++ b/internal/updates/release_metadata.go @@ -0,0 +1,182 @@ +package updates + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "strings" + + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" +) + +// GitHub release metadata embeds every uploaded asset. A 6.4.x release carries +// hundreds of assets (~600 KB of JSON each) and the default 30-release page is +// several megabytes, so buffering the whole response behind a small bound made +// the update check fail for every install (#1881, #2282). The decoders below +// stream the response token by token and retain only the fields the updater +// consumes, so memory stays proportional to what is kept rather than to the +// response size. maxReleaseMetadataBytes still bounds the bytes read. + +// decodeReleaseList streams a GitHub release-list response, keeping only the +// assets that isRuntimeReleaseAssetName accepts. +func decodeReleaseList(resp *http.Response) ([]ReleaseInfo, error) { + if err := securityutil.LimitResponseBody(resp, maxReleaseMetadataBytes); err != nil { + return nil, err + } + dec := json.NewDecoder(resp.Body) + if err := expectJSONDelim(dec, '['); err != nil { + return nil, err + } + var releases []ReleaseInfo + for dec.More() { + release, err := decodeReleaseObject(dec, isRuntimeReleaseAssetName) + if err != nil { + return nil, err + } + releases = append(releases, release) + } + if err := expectJSONDelim(dec, ']'); err != nil { + return nil, err + } + if err := expectJSONEnd(dec); err != nil { + return nil, err + } + return releases, nil +} + +// decodeSingleRelease streams one GitHub release object without retaining any +// assets; callers that only need release notes and dates never read them. +func decodeSingleRelease(resp *http.Response) (ReleaseInfo, error) { + if err := securityutil.LimitResponseBody(resp, maxReleaseMetadataBytes); err != nil { + return ReleaseInfo{}, err + } + dec := json.NewDecoder(resp.Body) + release, err := decodeReleaseObject(dec, func(string) bool { return false }) + if err != nil { + return ReleaseInfo{}, err + } + if err := expectJSONEnd(dec); err != nil { + return ReleaseInfo{}, err + } + return release, nil +} + +// isRuntimeReleaseAssetName matches the Pulse server archives the update check +// can offer: the exact runtime asset and the any-linux-tarball fallback. +func isRuntimeReleaseAssetName(name string) bool { + return strings.HasPrefix(name, "pulse-") && + strings.Contains(name, "linux") && + strings.HasSuffix(name, ".tar.gz") +} + +func decodeReleaseObject(dec *json.Decoder, keepAsset func(string) bool) (ReleaseInfo, error) { + var release ReleaseInfo + if err := expectJSONDelim(dec, '{'); err != nil { + return release, err + } + for dec.More() { + token, err := dec.Token() + if err != nil { + return release, err + } + key, ok := token.(string) + if !ok { + return release, fmt.Errorf("unexpected release field token %v", token) + } + switch key { + case "tag_name": + err = dec.Decode(&release.TagName) + case "name": + err = dec.Decode(&release.Name) + case "body": + err = dec.Decode(&release.Body) + case "prerelease": + err = dec.Decode(&release.Prerelease) + case "draft": + err = dec.Decode(&release.Draft) + case "published_at": + err = dec.Decode(&release.PublishedAt) + case "assets": + release.Assets, err = decodeReleaseAssets(dec, keepAsset) + default: + err = skipJSONValue(dec) + } + if err != nil { + return release, fmt.Errorf("decode release field %q: %w", key, err) + } + } + return release, expectJSONDelim(dec, '}') +} + +func decodeReleaseAssets(dec *json.Decoder, keepAsset func(string) bool) ([]ReleaseAsset, error) { + token, err := dec.Token() + if err != nil { + return nil, err + } + if token == nil { + return nil, nil + } + if delim, ok := token.(json.Delim); !ok || delim != '[' { + return nil, fmt.Errorf("expected assets array, got %v", token) + } + var assets []ReleaseAsset + for dec.More() { + var asset ReleaseAsset + if err := dec.Decode(&asset); err != nil { + return nil, err + } + if keepAsset(asset.Name) { + assets = append(assets, asset) + } + } + if err := expectJSONDelim(dec, ']'); err != nil { + return nil, err + } + return assets, nil +} + +// skipJSONValue consumes the next value token by token so large fields that +// the updater ignores are never buffered as a whole. +func skipJSONValue(dec *json.Decoder) error { + depth := 0 + for { + token, err := dec.Token() + if err != nil { + return err + } + if delim, ok := token.(json.Delim); ok { + switch delim { + case '{', '[': + depth++ + case '}', ']': + depth-- + } + } + if depth == 0 { + return nil + } + } +} + +func expectJSONDelim(dec *json.Decoder, want json.Delim) error { + token, err := dec.Token() + if err != nil { + return err + } + if delim, ok := token.(json.Delim); !ok || delim != want { + return fmt.Errorf("expected %q in release metadata, got %v", want, token) + } + return nil +} + +func expectJSONEnd(dec *json.Decoder) error { + if _, err := dec.Token(); !errors.Is(err, io.EOF) { + if err != nil { + return err + } + return fmt.Errorf("unexpected data after release metadata") + } + return nil +} diff --git a/internal/updates/release_notes.go b/internal/updates/release_notes.go index f3e0552075..9d5027e00f 100644 --- a/internal/updates/release_notes.go +++ b/internal/updates/release_notes.go @@ -98,8 +98,8 @@ func (m *Manager) GetReleaseNotes(ctx context.Context, version string) (*Release return nil, fmt.Errorf("GitHub API returned status %d: %s", resp.StatusCode, detail) } - var release ReleaseInfo - if err := decodeReleaseMetadata(resp, &release); err != nil { + release, err := decodeSingleRelease(resp) + if err != nil { return nil, fmt.Errorf("failed to decode release notes: %w", err) }