[fix] Pin updater download URLs to trusted GitHub hosts (#281) (#296)

* [fix] Pin updater asset URLs to trusted GitHub hosts (#281)

Add allowedAssetHosts allowlist (github.com + objects.githubusercontent.com)
and validateAssetURL helper. Call it in Check() after the empty-URL guards,
before assigning DownloadURL/ChecksumsURL to the result — a MITM/DNS-hijack
that tampers with the GitHub API response can no longer redirect the binary
or checksums.txt to an attacker-controlled host.

Replace the stale comment in findReleaseAssets that incorrectly argued
checksum verification alone was sufficient mitigation.

* [test] Validate asset URL helper and migrate fixtures (#281)

Add TestValidateAssetURL (7 cases: happy paths for both allowed hosts, http
scheme rejection, untrusted host, subdomain spoof, unparseable URL, empty
string). Add TestCheckRejectsUntrustedDownloadHost and
TestCheckRejectsUntrustedChecksumsHost integration tests at the Check() level.

Migrate all example.com fixture URLs in updater_test.go and cmd/ tests to
https://github.com/... so existing tests pass through the new URL validation.

* [fix] Address review feedback on validateAssetURL (#281)

Add inline comment on the empty-string guard clarifying it is defensive
(Check() pre-screens, but future callers may not). Add test case for
evil.objects.githubusercontent.com subdomain spoof to document that the
exact-match allowlist correctly rejects it.

* [chore] Simplify comments in validateAssetURL and related vars

Author: lugassawan

cmd/root_test.go

@@ -284,8 +284,8 @@ func TestRootHelpFunctionRoot(t *testing.T) {
 		_, _ = w.Write([]byte(`{
 			"tag_name":"v99.0.0",
 			"assets":[
-				{"name":"rimba_99.0.0_linux_amd64.tar.gz","browser_download_url":"https://example.com/download"},
-				{"name":"checksums.txt","browser_download_url":"https://example.com/checksums.txt"}
+				{"name":"rimba_99.0.0_linux_amd64.tar.gz","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v99.0.0/rimba_99.0.0_linux_amd64.tar.gz"},
+				{"name":"checksums.txt","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v99.0.0/checksums.txt"}
 			]
 		}`))
 	}))

cmd/update_hint_test.go

@@ -40,8 +40,8 @@ func TestCheckUpdateHintNewVersionAvailable(t *testing.T) {
 		_, _ = w.Write([]byte(`{
 			"tag_name":"` + testVersionNew + `",
 			"assets":[
-				{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://example.com/download"},
-				{"name":"checksums.txt","browser_download_url":"https://example.com/checksums.txt"}
+				{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz"},
+				{"name":"checksums.txt","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v2.0.0/checksums.txt"}
 			]
 		}`))
 	}))
@@ -95,7 +95,7 @@ func TestCheckUpdateHintTimeout(t *testing.T) {
 		_, _ = w.Write([]byte(`{
 			"tag_name":"` + testVersionNew + `",
 			"assets":[
-				{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://example.com/download"}
+				{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz"}
 			]
 		}`))
 	}))

internal/updater/updater.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -26,6 +27,13 @@ const (
 	goosWindows        = "windows"
 )
 
+// allowedAssetHosts pins asset downloads to GitHub-controlled hosts.
+// objects.githubusercontent.com is included for CDN redirects followed at HTTP fetch time.
+var allowedAssetHosts = map[string]bool{
+	"github.com":                    true,
+	"objects.githubusercontent.com": true,
+}
+
 // maxBinarySize is the decompressed size limit for the extracted binary (100 MiB).
 // Tests may override this to exercise the size-limit path.
 var maxBinarySize int64 = 100 << 20
@@ -138,6 +146,12 @@ func (u *Updater) Check(ctx context.Context) (*CheckResult, error) {
 		if checksumsURL == "" {
 			return nil, fmt.Errorf("%s not found in release %s", checksumsFileName, release.TagName)
 		}
+		if err := validateAssetURL(downloadURL); err != nil {
+			return nil, fmt.Errorf("untrusted download URL: %w", err)
+		}
+		if err := validateAssetURL(checksumsURL); err != nil {
+			return nil, fmt.Errorf("untrusted checksums URL: %w", err)
+		}
 		result.DownloadURL = downloadURL
 		result.AssetName = assetName
 		result.ChecksumsURL = checksumsURL
@@ -353,10 +367,7 @@ func assetNameFor(goos, goarch, version string) string {
 }
 
 // findReleaseAssets scans release assets for the platform archive and checksums file.
-// Returns empty strings for any asset not found.
-// BrowserDownloadURL values are used verbatim; checksum verification (issue #222) is
-// the intended mitigation against redirected-URL attacks rather than host-prefix
-// validation, which would need to enumerate all valid GitHub CDN hostnames.
+// Returns empty strings for any asset not found. URLs are validated by Check before use.
 func findReleaseAssets(assetName string, assets []Asset) (downloadURL, checksumsURL string) {
 	for _, a := range assets {
 		if a.Name == assetName {
@@ -368,3 +379,23 @@ func findReleaseAssets(assetName string, assets []Asset) (downloadURL, checksums
 	}
 	return
 }
+
+// validateAssetURL rejects asset URLs with a non-https scheme or untrusted host,
+// guarding against a tampered API response redirecting downloads to an attacker (#281).
+func validateAssetURL(rawURL string) error {
+	// Check() pre-screens for empty, but guard here for direct callers.
+	if rawURL == "" {
+		return errors.New("asset URL must not be empty")
+	}
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return fmt.Errorf("parsing asset URL %q: %w", rawURL, err)
+	}
+	if u.Scheme != "https" {
+		return fmt.Errorf("asset URL %q must use https, got scheme %q", rawURL, u.Scheme)
+	}
+	if !allowedAssetHosts[u.Hostname()] {
+		return fmt.Errorf("asset URL %q host %q is not a trusted GitHub host", rawURL, u.Hostname())
+	}
+	return nil
+}

internal/updater/updater_test.go

@@ -88,7 +88,7 @@ func serveOctetStream(t *testing.T, data []byte) *httptest.Server {
 func releaseJSON(tagName string, extras ...string) string {
 	assets := make([]string, 0, 1+len(extras))
 	assets = append(assets,
-		`{"name":"checksums.txt","browser_download_url":"https://example.com/checksums.txt"}`,
+		`{"name":"checksums.txt","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v2.0.0/checksums.txt"}`,
 	)
 	assets = append(assets, extras...)
 	return fmt.Sprintf(`{"tag_name":%q,"assets":[%s]}`, tagName, strings.Join(assets, ","))
@@ -110,7 +110,7 @@ func TestCheckUpToDate(t *testing.T) {
 
 func TestCheckNewVersionAvailable(t *testing.T) {
 	srv := serveJSON(t, releaseJSON(testVersionNew,
-		`{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://example.com/rimba_2.0.0_linux_amd64.tar.gz"}`,
+		`{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz"}`,
 	))
 	u := newTestUpdater(srv)
 
@@ -120,7 +120,7 @@ func TestCheckNewVersionAvailable(t *testing.T) {
 		t.Errorf("expected not up to date")
 	}
 
-	wantURL := "https://example.com/rimba_2.0.0_linux_amd64.tar.gz"
+	wantURL := "https://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz"
 	if result.DownloadURL != wantURL {
 		t.Errorf("download URL = %q, want %q", result.DownloadURL, wantURL)
 	}
@@ -130,7 +130,7 @@ func TestCheckNewVersionAvailable(t *testing.T) {
 		t.Errorf("AssetName = %q, want %q", result.AssetName, wantAsset)
 	}
 
-	wantChecksums := "https://example.com/checksums.txt"
+	wantChecksums := "https://github.com/lugassawan/rimba/releases/download/v2.0.0/checksums.txt"
 	if result.ChecksumsURL != wantChecksums {
 		t.Errorf("ChecksumsURL = %q, want %q", result.ChecksumsURL, wantChecksums)
 	}
@@ -140,7 +140,7 @@ func TestCheckNoMatchingAsset(t *testing.T) {
 	srv := serveJSON(t, `{
 		"tag_name":"`+testVersionNew+`",
 		"assets":[
-			{"name":"rimba_2.0.0_windows_amd64.zip","browser_download_url":"https://example.com/rimba_2.0.0_windows_amd64.zip"}
+			{"name":"rimba_2.0.0_windows_amd64.zip","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_windows_amd64.zip"}
 		]
 	}`)
 	u := newTestUpdater(srv)
@@ -160,7 +160,7 @@ func TestCheckMissingChecksums(t *testing.T) {
 	srv := serveJSON(t, fmt.Sprintf(`{
 		"tag_name":%q,
 		"assets":[
-			{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://example.com/rimba_2.0.0_linux_amd64.tar.gz"}
+			{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz"}
 		]
 	}`, testVersionNew))
 	u := newTestUpdater(srv)
@@ -176,7 +176,7 @@ func TestCheckMissingChecksums(t *testing.T) {
 
 func TestCheckWindowsAsset(t *testing.T) {
 	srv := serveJSON(t, releaseJSON(testVersionNew,
-		`{"name":"rimba_2.0.0_windows_amd64.zip","browser_download_url":"https://example.com/rimba_2.0.0_windows_amd64.zip"}`,
+		`{"name":"rimba_2.0.0_windows_amd64.zip","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_windows_amd64.zip"}`,
 	))
 	u := &Updater{
 		CurrentVersion: testVersion,
@@ -1534,3 +1534,102 @@ func TestAssetNameFor(t *testing.T) {
 		})
 	}
 }
+
+// ---- validateAssetURL tests ----
+
+func TestValidateAssetURL(t *testing.T) {
+	tests := []struct {
+		name    string
+		url     string
+		wantErr string
+	}{
+		{
+			name: "github.com happy path",
+			url:  "https://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz",
+		},
+		{
+			name: "objects.githubusercontent.com happy path",
+			url:  "https://objects.githubusercontent.com/github-production-release-asset/12345/rimba_2.0.0_linux_amd64.tar.gz",
+		},
+		{
+			name:    "http scheme rejected",
+			url:     "http://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz",
+			wantErr: "must use https",
+		},
+		{
+			name:    "untrusted host rejected",
+			url:     "https://evil.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz",
+			wantErr: "not a trusted GitHub host",
+		},
+		{
+			name:    "subdomain spoof rejected",
+			url:     "https://github.com.evil.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz",
+			wantErr: "not a trusted GitHub host",
+		},
+		{
+			name:    "objects.githubusercontent.com subdomain spoof rejected",
+			url:     "https://evil.objects.githubusercontent.com/rimba_2.0.0_linux_amd64.tar.gz",
+			wantErr: "not a trusted GitHub host",
+		},
+		{
+			name:    "unparseable URL rejected",
+			url:     "https://github.com/%zz",
+			wantErr: "parsing asset URL",
+		},
+		{
+			name:    "empty URL rejected",
+			url:     "",
+			wantErr: "must not be empty",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateAssetURL(tt.url)
+			if tt.wantErr == "" {
+				if err != nil {
+					t.Errorf("unexpected error: %v", err)
+				}
+				return
+			}
+			if err == nil {
+				t.Fatalf("expected error containing %q, got nil", tt.wantErr)
+			}
+			if !strings.Contains(err.Error(), tt.wantErr) {
+				t.Errorf("error = %q, want to contain %q", err.Error(), tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestCheckRejectsUntrustedDownloadHost(t *testing.T) {
+	srv := serveJSON(t, releaseJSON(testVersionNew,
+		`{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://evil.com/rimba_2.0.0_linux_amd64.tar.gz"}`,
+	))
+	u := newTestUpdater(srv)
+
+	_, err := u.Check(context.Background())
+	if err == nil {
+		t.Fatal("expected error for untrusted download URL host")
+	}
+	if !strings.Contains(err.Error(), "untrusted download URL") {
+		t.Errorf("error = %q, want to contain 'untrusted download URL'", err.Error())
+	}
+}
+
+func TestCheckRejectsUntrustedChecksumsHost(t *testing.T) {
+	body := fmt.Sprintf(`{"tag_name":%q,"assets":[
+		{"name":"rimba_2.0.0_linux_amd64.tar.gz","browser_download_url":"https://github.com/lugassawan/rimba/releases/download/v2.0.0/rimba_2.0.0_linux_amd64.tar.gz"},
+		{"name":"checksums.txt","browser_download_url":"https://evil.com/checksums.txt"}
+	]}`, testVersionNew)
+	srv := serveJSON(t, body)
+	u := newTestUpdater(srv)
+
+	_, err := u.Check(context.Background())
+	if err == nil {
+		t.Fatal("expected error for untrusted checksums URL host")
+	}
+	if !strings.Contains(err.Error(), "untrusted checksums URL") {
+		t.Errorf("error = %q, want to contain 'untrusted checksums URL'", err.Error())
+	}
+}