Merge update feed asset verification candidate for coordination

Change-source: pulse-maintainer
2026-09-09 18:15:50 +00:00 · 2026-09-04 13:24:09 +01:00
parent 79e0f9a673 b60dfd5041
commit eb0a02fb07
2 changed files with 163 additions and 42 deletions
@@ -91,6 +91,7 @@ const (
 	defaultUpdateAPIBaseURL  string = "https://api.github.com"
 	maxReleaseFeedBytes      int64  = 1 << 20   // 1 MiB
 	maxReleaseMetadataBytes  int64  = 1 << 20   // 1 MiB
+	maxReleaseFeedCandidates int    = 20        // Bound asset probes from a feed.
 	maxChecksumFileBytes     int64  = 1 << 20   // 1 MiB
 	maxUpdateDownloadBytes   int64  = 512 << 20 // 512 MiB
 	minUpdateTempFreeBytes   int64  = 128 << 20 // 128 MiB
@@ -901,6 +902,14 @@ func (m *Manager) getLatestReleaseForChannel(ctx context.Context, channel string

 	if resp.StatusCode == http.StatusForbidden {
 		body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
+		detail := strings.TrimSpace(string(body))
+		if detail == "" {
+			detail = resp.Status
+		}
+		if strings.TrimSpace(os.Getenv("PULSE_UPDATE_SERVER")) != "" {
+			return nil, fmt.Errorf("update server returned status %d: %s", resp.StatusCode, detail)
+		}
+
 		log.Warn().
 			Str("channel", channel).
 			Str("rateLimitRemaining", resp.Header.Get("X-RateLimit-Remaining")).
@@ -913,11 +922,6 @@ func (m *Manager) getLatestReleaseForChannel(ctx context.Context, channel string
 			return feedRelease, nil
 		}

-		detail := strings.TrimSpace(string(body))
-		if detail == "" {
-			detail = resp.Status
-		}
-
 		return nil, fmt.Errorf("%w: %s", errGitHubRateLimited, detail)
 	}

@@ -1098,11 +1102,16 @@ func (m *Manager) getLatestReleaseFromFeed(ctx context.Context, channel string)
 	// 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
-	// interleave with v6 releases in the same repo.
-	var best *ReleaseInfo
-	var bestVer *Version
+	// Pick by version rather than feed order: v5-line maintenance releases
+	// interleave with v6 releases in the same repo. Keep candidates until their
+	// runtime asset has been checked. GitHub's feed can retain an entry for a
+	// deleted release tag, so synthesising a URL from the first matching title
+	// alone can advertise a retracted build whose download now returns 404.
+	type feedCandidate struct {
+		release ReleaseInfo
+		version *Version
+	}
+	candidates := make([]feedCandidate, 0, len(feed.Entries))
 	for _, entry := range feed.Entries {
 		match := versionTitleRegex.FindStringSubmatch(strings.TrimSpace(entry.Title))
 		if len(match) < 2 {
@@ -1123,9 +1132,6 @@ func (m *Manager) getLatestReleaseFromFeed(ctx context.Context, channel string)
 			continue
 		}

-		if best != nil && !ver.IsNewerThan(bestVer) {
-			continue
-		}
 		publishedAt := time.Time{}
 		for _, rawTimestamp := range []string{entry.Published, entry.Updated} {
 			if parsed, err := time.Parse(time.RFC3339, strings.TrimSpace(rawTimestamp)); err == nil {
@@ -1133,36 +1139,87 @@ func (m *Manager) getLatestReleaseFromFeed(ctx context.Context, channel string)
 				break
 			}
 		}
-		assets := []ReleaseAsset{}
-		if asset, ok := updateReleaseAssetForRuntime(tagName); ok {
-			assets = append(assets, asset)
-		}
-		best = &ReleaseInfo{
+		candidates = append(candidates, feedCandidate{release: ReleaseInfo{
 			TagName:     tagName,
 			Name:        "Pulse " + tagName,
 			Prerelease:  isPrerelease,
 			PublishedAt: publishedAt,
-			// Atom does not list release assets. Published Pulse versions have a
-			// deterministic runtime archive name, so synthesize only the exact
-			// current-platform URL; ApplyUpdate still verifies its SSHSIG and
-			// checksum before installing anything.
-			Assets: assets,
-		}
-		bestVer = ver
+		}, version: ver})
 	}

-	if best != nil {
+	sort.Slice(candidates, func(i, j int) bool {
+		return candidates[i].version.IsNewerThan(candidates[j].version)
+	})
+	if len(candidates) > maxReleaseFeedCandidates {
+		candidates = candidates[:maxReleaseFeedCandidates]
+	}
+
+	for i := range candidates {
+		candidate := &candidates[i].release
+		asset, ok := updateReleaseAssetForRuntime(candidate.TagName)
+		if !ok {
+			continue
+		}
+		available, err := m.releaseAssetAvailable(ctx, asset)
+		if err != nil {
+			return nil, fmt.Errorf("verify feed release %s: %w", candidate.TagName, err)
+		}
+		if !available {
+			log.Warn().
+				Str("tag", candidate.TagName).
+				Msg("Skipping release feed entry without a published runtime asset")
+			continue
+		}
+
+		// Atom does not list release assets. Published Pulse versions have a
+		// deterministic runtime archive name; expose only the exact asset just
+		// proven to exist. ApplyUpdate still verifies its SSHSIG and checksum.
+		candidate.Assets = []ReleaseAsset{asset}
 		log.Debug().
-			Str("tag", best.TagName).
-			Bool("prerelease", best.Prerelease).
+			Str("tag", candidate.TagName).
+			Bool("prerelease", candidate.Prerelease).
 			Str("channel", channel).
 			Msg("Found release from feed")
-		return best, nil
+		return candidate, nil
 	}

 	return nil, fmt.Errorf("no suitable release found for channel %s", channel)
 }

+// releaseAssetAvailable verifies a deterministic GitHub release URL without
+// downloading the archive. Redirects are deliberately not followed: GitHub's
+// release route returns a redirect only after resolving a published asset,
+// whereas stale feed entries return 404.
+func (m *Manager) releaseAssetAvailable(ctx context.Context, asset ReleaseAsset) (bool, error) {
+	target, err := validateApplyDownloadURL(asset.BrowserDownloadURL)
+	if err != nil {
+		return false, err
+	}
+
+	client := &http.Client{
+		Timeout: 30 * time.Second,
+		CheckRedirect: func(_ *http.Request, _ []*http.Request) error {
+			return http.ErrUseLastResponse
+		},
+	}
+	resp, err := m.requestWithRetry(ctx, client, http.MethodHead, target, map[string]string{
+		"User-Agent": "Pulse-Update-Checker",
+	}, "verify GitHub release asset")
+	if err != nil {
+		return false, err
+	}
+	defer resp.Body.Close()
+
+	switch resp.StatusCode {
+	case http.StatusOK, http.StatusMovedPermanently, http.StatusFound, http.StatusSeeOther, http.StatusTemporaryRedirect, http.StatusPermanentRedirect:
+		return true, nil
+	case http.StatusNotFound, http.StatusGone:
+		return false, nil
+	default:
+		return false, fmt.Errorf("release asset returned status %d", resp.StatusCode)
+	}
+}
+
 func (m *Manager) createHistoryEntry(ctx context.Context, entry UpdateHistoryEntry) string {
 	if m.history == nil {
 		return ""
@@ -1322,6 +1379,10 @@ func sleepWithContext(ctx context.Context, delay time.Duration) error {
 }

 func (m *Manager) getWithRetry(ctx context.Context, client *http.Client, target *url.URL, headers map[string]string, operation string) (*http.Response, error) {
+	return m.requestWithRetry(ctx, client, http.MethodGet, target, headers, operation)
+}
+
+func (m *Manager) requestWithRetry(ctx context.Context, client *http.Client, method string, target *url.URL, headers map[string]string, operation string) (*http.Response, error) {
 	if client == nil {
 		client = &http.Client{Timeout: 30 * time.Second}
 	}
@@ -1335,7 +1396,7 @@ func (m *Manager) getWithRetry(ctx context.Context, client *http.Client, target

 	targetURL := target.String()
 	for attempt := 1; attempt <= updateHTTPAttempts; attempt++ {
-		req, err := securityutil.NewValidatedRequestWithContext(ctx, http.MethodGet, target, nil)
+		req, err := securityutil.NewValidatedRequestWithContext(ctx, method, target, nil)
 		if err != nil {
 			return nil, err
 		}
@@ -174,6 +174,14 @@ func TestGetLatestReleaseForChannelRetriesTransientStatus(t *testing.T) {

 func TestGetLatestReleaseForChannelFallsBackWhenReleaseMetadataIsOversized(t *testing.T) {
 	setRetrySettingsForTest(t, 1, time.Millisecond, time.Millisecond)
+	staleAsset, supported := updateReleaseAssetForRuntime("v6.4.2")
+	if !supported {
+		t.Skipf("no release asset mapping for %s", runtime.GOARCH)
+	}
+	publishedAsset, supported := updateReleaseAssetForRuntime("v6.4.1")
+	if !supported {
+		t.Skipf("no release asset mapping for %s", runtime.GOARCH)
+	}

 	feed := `<?xml version="1.0" encoding="UTF-8"?>
 <feed xmlns="http://www.w3.org/2005/Atom">
@@ -185,6 +193,10 @@ func TestGetLatestReleaseForChannelFallsBackWhenReleaseMetadataIsOversized(t *te
    <title>v6.4.2</title>
    <updated>2026-08-31T19:08:45Z</updated>
  </entry>
+  <entry>
+    <title>Pulse v6.4.1</title>
+    <updated>2026-08-29T11:06:45Z</updated>
+  </entry>
 </feed>`

 	origTransport := http.DefaultTransport
@@ -202,6 +214,16 @@ func TestGetLatestReleaseForChannelFallsBackWhenReleaseMetadataIsOversized(t *te
 			status = http.StatusOK
 			body = feed
 			header.Set("Content-Type", "application/atom+xml")
+		case staleAsset.BrowserDownloadURL:
+			if req.Method != http.MethodHead {
+				t.Errorf("stale asset probe method = %s, want HEAD", req.Method)
+			}
+			status = http.StatusNotFound
+		case publishedAsset.BrowserDownloadURL:
+			if req.Method != http.MethodHead {
+				t.Errorf("published asset probe method = %s, want HEAD", req.Method)
+			}
+			status = http.StatusFound
 		}
 		return &http.Response{
 			StatusCode:    status,
@@ -224,15 +246,11 @@ func TestGetLatestReleaseForChannelFallsBackWhenReleaseMetadataIsOversized(t *te
 	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)
+	if release.TagName != "v6.4.1" {
+		t.Fatalf("release tag = %q, want published release v6.4.1 after stale 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)
+	if len(release.Assets) != 1 || release.Assets[0] != publishedAsset {
+		t.Fatalf("release assets = %+v, want %+v", release.Assets, publishedAsset)
 	}
 }

@@ -261,6 +279,43 @@ func TestGetLatestReleaseForChannelDoesNotReplaceCustomOversizedMetadata(t *test
 	}
 }

+func TestGetLatestReleaseForChannelDoesNotReplaceForbiddenCustomUpdateServer(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) {
+		if req.URL.Host == "github.com" {
+			feedHits.Add(1)
+		}
+		body := `{"message":"forbidden"}`
+		return &http.Response{
+			StatusCode:    http.StatusForbidden,
+			Status:        http.StatusText(http.StatusForbidden),
+			Body:          io.NopCloser(strings.NewReader(body)),
+			ContentLength: int64(len(body)),
+			Header:        make(http.Header),
+			Request:       req,
+		}, nil
+	})
+	t.Cleanup(func() { http.DefaultTransport = origTransport })
+
+	t.Setenv("PULSE_UPDATE_SERVER", "https://updates.example.test")
+	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(), "update server returned status 403") {
+		t.Fatalf("getLatestReleaseForChannel error = %v, want custom server rejection", err)
+	}
+	if got := feedHits.Load(); got != 0 {
+		t.Fatalf("public GitHub feed requests = %d, want 0 for custom update server", got)
+	}
+}
+
 func TestGetLatestReleaseForChannelDoesNotMaskMalformedMetadata(t *testing.T) {
 	setRetrySettingsForTest(t, 1, time.Millisecond, time.Millisecond)

@@ -302,6 +357,10 @@ func TestGetLatestReleaseForChannelDoesNotMaskMalformedMetadata(t *testing.T) {
 func TestGetLatestReleaseForChannelRateLimitFallbackIncludesRuntimeAsset(t *testing.T) {
 	setRetrySettingsForTest(t, 1, time.Millisecond, time.Millisecond)
 	withBuildVersion(t, "6.4.0-rc.10")
+	expectedAsset, supported := updateReleaseAssetForRuntime("v6.4.0-rc.11")
+	if !supported {
+		t.Skipf("no release asset mapping for %s", runtime.GOARCH)
+	}

 	releaseTime := time.Date(2026, 8, 28, 12, 52, 29, 0, time.UTC)
 	feed := `<?xml version="1.0" encoding="UTF-8"?>
@@ -326,6 +385,11 @@ func TestGetLatestReleaseForChannelRateLimitFallbackIncludesRuntimeAsset(t *test
 			status = http.StatusOK
 			body = feed
 			header.Set("Content-Type", "application/atom+xml")
+		case expectedAsset.BrowserDownloadURL:
+			if req.Method != http.MethodHead {
+				t.Errorf("asset probe method = %s, want HEAD", req.Method)
+			}
+			status = http.StatusFound
 		}
 		return &http.Response{
 			StatusCode: status,
@@ -345,10 +409,6 @@ func TestGetLatestReleaseForChannelRateLimitFallbackIncludesRuntimeAsset(t *test
 	if !info.Available || info.LatestVersion != "6.4.0-rc.11" {
 		t.Fatalf("fallback update = %+v, want available v6.4.0-rc.11", info)
 	}
-	expectedAsset, supported := updateReleaseAssetForRuntime("v6.4.0-rc.11")
-	if !supported {
-		t.Fatalf("test runner architecture %q must map to a release asset", runtime.GOARCH)
-	}
 	if info.DownloadURL != expectedAsset.BrowserDownloadURL {
 		t.Fatalf("fallback DownloadURL = %q, want %q", info.DownloadURL, expectedAsset.BrowserDownloadURL)
 	}