Merge reviewed update metadata repair into Core candidate

Change-source: pulse-maintainer

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

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

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

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

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