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) }