Skip to content

Make selfupdate avoid GitHub API rate limits#22

Merged
wesm merged 8 commits into
mainfrom
selfupdate-release-discovery
Jun 16, 2026
Merged

Make selfupdate avoid GitHub API rate limits#22
wesm merged 8 commits into
mainfrom
selfupdate-release-discovery

Conversation

@wesm

@wesm wesm commented Jun 15, 2026

Copy link
Copy Markdown
Member

Summary

  • Move selfupdate release discovery away from unauthenticated api.github.com as the default path by following the GitHub web /releases/latest redirect and constructing conventional release asset URLs.
  • Add optional static ReleaseManifestURL support; a manifest can be as small as { "tag_name": "vX.Y.Z" }, or can provide GitHub-style asset metadata for custom download URLs.
  • Keep the GitHub REST API as a fallback, with optional GitHubToken auth scoped to the API request only, plus EnvironmentGitHubToken() for callers using GH_TOKEN/GITHUB_TOKEN.
  • Document the migration path so roborev, agentsview, and msgvault can share this behavior through kit instead of duplicating updater workarounds.

Repo-wide golangci-lint run and go run ./cmd/testify-helper-check ./... currently report pre-existing issues in git/cmd; the selfupdate-scoped lint checks pass.

@roborev-ci

roborev-ci Bot commented Jun 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (e2f7ed4)

Medium issues remain in selfupdate release discovery; no Critical or High findings.

Medium

  • selfupdate/selfupdate.go:895 - Web and tag-only manifest discovery call addConventionalAssets before Check compares the discovered tag with CurrentVersion. An already-current client can still fail the check if the platform asset HEAD request fails or the asset is not published, even though no update would be offered.

    • Fix: Return the discovered tag first, perform shouldOfferUpdate, and only synthesize/probe conventional assets when an update is actually needed.

  • selfupdate/selfupdate.go:911 - Conventional discovery only synthesizes the first checksum asset name and only the .sha256.sig signature URL, dropping the existing fallback support for checksums.txt and <asset>.sig. Releases that worked through the API path can now return update info without a checksum or with the wrong signature URL, causing install to fail.

    • Fix: Preserve all configured checksum/signature candidates and try them in priority order, or verify which metadata URLs exist before selecting one.

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 2m29s), codex_security (codex/security, done, 4m38s) | Total: 7m16s

@mariusvniekerk
Fix selfupdate conventional release fallbacks
3c90f8b 
Tag-only web and manifest discovery previously synthesized and probed platform assets before comparing the discovered tag with CurrentVersion. That meant an already-current client could fail because optional release asset metadata was missing even though no update would be offered.

Conventional releases also collapsed checksum and signature metadata to a single guessed URL, which broke releases that publish checksums.txt or <asset>.sig instead of the first default candidate. Keep discovery tag-first, try configured checksum candidates in order, and only select synthesized signature URLs that exist.

Validation: go test ./selfupdate; go test ./...

Generated with Codex
Co-authored-by: Marius van Niekerk <[email protected]>
@roborev-ci

roborev-ci Bot commented Jun 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (3c90f8b)

Medium-confidence issue found: one install-blocking regression should be addressed before merge.

Medium

  • selfupdate/selfupdate.go:189 - The new web discovery path only falls back to the GitHub API when the conventional archive probe fails. If the archive exists but SHA256SUMS/checksums.txt is missing, unreadable, or does not contain the asset, Check returns an Info with an empty checksum, and Install later refuses it. This also regresses releases whose checksum is only available in the GitHub release body, which the API fallback could still recover.

    Suggested fix: When a web-discovered conventional release cannot produce a checksum, treat that as web-path failure and fetch the API release before returning, or return a check error instead of unusable install metadata.

Panel: ci_default_security | Synthesis: codex, 34s | Members: codex_default (codex/default, done, 4m37s), codex_security (codex/security, done, 5m18s) | Total: 10m29s

@mariusvniekerk
Recover API checksums for web-discovered releases
8f88957 
Web-discovered conventional releases can prove the archive exists while still lacking usable checksum metadata. Returning that Info shifts the failure to Install, which is too late and regresses releases where the GitHub API body or asset metadata can still provide the checksum.

Retry the API release path before returning install metadata when a web-synthesized release has no checksum. If the API cannot recover a checksum either, fail Check with an explicit checksum error instead of producing unusable update info.

Validation: go test ./selfupdate; go test ./...

Generated with Codex
Co-authored-by: Marius van Niekerk <[email protected]>
@roborev-ci

roborev-ci Bot commented Jun 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (8f88957)

High severity finding: fetchReleaseManifest allows insecure manifest-derived downloads.

selfupdate/selfupdate.go:873 and selfupdate/selfupdate.go:223

A manifest URL can be arbitrary, and the decoded manifest can supply arbitrary archive and checksum URLs that are later returned as install metadata. If a caller follows the migration guidance with AllowUnsignedChecksums: true, an active network attacker could rewrite an http:// manifest, asset, or checksum response to point at a malicious archive with a matching SHA256. Install would accept it because unsigned checksums are allowed, leading to code execution as the updating user.

Remediation: reject non-HTTPS manifest, archive, checksum, and signature URLs whenever unsigned checksums are allowed, or require trusted signature verification for manifest/custom URL sources. If custom hosts are needed, validate them against an explicit allowlist.

Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 6m26s), codex_security (codex/security, done, 5m28s) | Total: 12m4s

@mariusvniekerk
Require HTTPS for unsigned manifest updates
74a6160 
Release manifests can provide arbitrary archive and checksum URLs. In the temporary migration mode where callers allow unsigned checksums, accepting manifest-derived http URLs leaves the checksum and archive responses vulnerable to active network rewriting.

Fail manifest checks early unless the manifest URL and manifest-derived asset metadata use HTTPS whenever unsigned checksums are actually allowed. Signed configurations keep their existing behavior because signature verification authenticates the update metadata.

Validation: go test ./selfupdate; go test ./...

Generated with Codex
Co-authored-by: Marius van Niekerk <[email protected]>
@roborev-ci

roborev-ci Bot commented Jun 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (74a6160)

High: The PR still has security blockers around HTTPS/token handling.

High

  • selfupdate/selfupdate.go:1023
    GitHubToken is added to API requests without requiring GitHubAPIBaseURL to use HTTPS. If a caller uses GitHubToken: EnvironmentGitHubToken() with an HTTP GitHub Enterprise/API endpoint, or an attacker-controlled configured endpoint, the token can be sent in plaintext.
    Fix: Reject or skip GitHubToken unless apiBaseURL() is HTTPS, and reject redirects from authenticated API requests to non-HTTPS URLs.

  • selfupdate/selfupdate.go:918 / selfupdate/selfupdate.go:936
    HTTPS validation for unsigned manifest updates only checks the originally configured manifest/asset/checksum URL, but the default HTTP client follows redirects. An HTTPS manifest, checksum, asset HEAD, or archive URL can redirect to HTTP, allowing unsigned metadata or archives to be fetched over plaintext when AllowUnsignedChecksums is enabled.
    Fix: For unsigned manifest updates, reject redirects to non-HTTPS URLs for manifest, checksum, asset HEAD, and archive downloads, and verify the final response URL remains HTTPS before trusting the response.

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 3m26s), codex_security (codex/security, done, 4m10s) | Total: 7m45s

@mariusvniekerk
Block HTTP downgrades in selfupdate metadata fetches
3da24ea 
Token-bearing API requests and unsigned manifest update flows should not trust redirects that downgrade HTTPS to HTTP. Without that guard, credentials can be exposed during API fallback and unsigned manifest metadata can still be rewritten after the initial URL validation.

Reject GitHub token use with non-HTTPS API bases, reject HTTPS-to-HTTP redirects for authenticated API requests, and carry manifest provenance into install so unsigned manifest-derived archive downloads keep the same downgrade protection.

Validation: go test ./selfupdate; go test ./...

Generated with Codex
Co-authored-by: Marius van Niekerk <[email protected]>
@roborev-ci

roborev-ci Bot commented Jun 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (3da24ea)

High: HTTPS downgrade protection is incomplete for unsigned self-updates.

  • Severity: High
  • Locations: selfupdate/selfupdate.go:298, selfupdate/selfupdate.go:965, selfupdate/selfupdate.go:1158
  • Problem: HTTPS downgrade protection is only enforced for manifest-derived unsigned updates. The new default web/conventional discovery path leaves manifestDerived false, so clients using AllowUnsignedChecksums can still follow HTTPS-to-HTTP redirects, or use a configured HTTP GitHubWebBaseURL, for both checksum and archive downloads. An active network attacker could replace both the unsigned checksum file and the archive, causing installation of a malicious update.
  • Fix: Enforce HTTPS and reject HTTPS-to-HTTP redirects for all unsigned update metadata, checksum, and archive requests, regardless of whether the source is release manifest, web discovery, or API fallback. Key the protection to unsigned checksum trust mode rather than manifestDerived, and add tests for downgrade attempts in the web/conventional path.

Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 5m2s), codex_security (codex/security, done, 5m3s) | Total: 10m13s

@mariusvniekerk
Enforce HTTPS for all unsigned selfupdate fetches
36ea614 
Unsigned checksum mode trusts release metadata, checksum bytes, and archive bytes without signature verification. The previous downgrade protection only covered manifest-derived releases, leaving web and conventional discovery able to follow HTTPS-to-HTTP redirects or use HTTP bases while still relying on checksum-only validation.

Apply the HTTPS requirement to unsigned checksum mode itself so every metadata, checksum, and archive fetch used by that mode rejects cleartext origins and downgrade redirects, regardless of whether the source was a manifest, web discovery, or API fallback.

Validation: go test ./selfupdate; go test ./...

Generated with Codex
Co-authored-by: Marius van Niekerk <[email protected]>
@roborev-ci

roborev-ci Bot commented Jun 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (36ea614)

High-level verdict: changes need fixes before merge due to remaining unsigned-update HTTP downgrade paths.

High

  • selfupdate/selfupdate.go:1026, selfupdate/selfupdate.go:1093
    Unsigned release discovery still permits HTTPS-to-HTTP downgrades. Web discovery accepts any redirect scheme, and API discovery only rejects downgraded redirects when a token is present. In AllowUnsignedChecksums mode, downgraded metadata can provide the tag, checksum, and asset URL used to authorize an update.
    Fix: Use downgrade-rejecting redirect/final-URL checks for both web and API discovery whenever c.requireHTTPSForUnsignedChecksums() is true, and add tests for web and unauthenticated API redirects to HTTP.

Medium

  • selfupdate/selfupdate.go:216
    After the web-conventional checksum-missing path falls back to the API, the replacement release assets are not revalidated for unsigned HTTPS requirements. An API response with a body checksum and an http:// archive URL can make Check return installable-looking metadata with an insecure DownloadURL.
    Fix: Call validateUnsignedReleaseSource or validateUnsignedAssetURLs immediately after assigning the API fallback release and before using its assets.

  • selfupdate/selfupdate.go:738
    doHTTPRequest rejects a non-HTTPS final URL only after client.Do has already sent the initial request. Public Install passes Info.DownloadURL into this path, so unsigned installs can still initiate an http:// archive request before failing.
    Fix: When HTTPS is required, reject req.URL.Scheme != "https" before calling client.Do.

Panel: ci_default_security | Synthesis: codex, 11s | Members: codex_default (codex/default, done, 4m14s), codex_security (codex/security, done, 2m28s) | Total: 6m53s

@mariusvniekerk
Close unsigned selfupdate redirect gaps
023c726 
Unsigned checksum mode needs transport authentication before any release metadata, checksum, or archive bytes are trusted. Applying the shared redirect guard to most fetches still left web/API discovery with their own redirect behavior and allowed a direct HTTP archive request to be sent before failure.

Route web and API discovery through the same unsigned-mode HTTPS checks, reject non-HTTPS initial requests before issuing them, and revalidate API fallback assets before returning install metadata recovered after a missing web checksum.

Validation: go test ./selfupdate; go test ./...

Generated with Codex
Co-authored-by: Marius van Niekerk <[email protected]>
@roborev-ci

roborev-ci Bot commented Jun 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (023c726)

Medium severity findings remain and are not duplicates.

  • Medium: fetchReleaseManifest allows cleartext signed-update metadata fetches
    Location: selfupdate/selfupdate.go:1006
    fetchReleaseManifest only enforces HTTPS when unsignedChecksumsAllowed() is true. With signed updates configured, a caller can still use an http:// ReleaseManifestURL, allowing a network attacker to tamper with release discovery and steer clients to an older but still valid signed release. Require HTTPS and block HTTPS-to-HTTP redirects for manifest fetches regardless of checksum signature mode.

  • Medium: checksum asset fetch errors are over-suppressed
    Location: selfupdate/selfupdate.go:1192
    fetchChecksumFromAssets suppresses every checksum download error except HTTP downgrades, including cancellation, deadline expiry, TLS failures, and read-limit errors. This can let Check continue after caller cancellation or hide real checksum-source failures as missing checksums. Propagate cancellation/deadline and unexpected transport/read errors; only ignore expected not-found/no-checksum cases while trying fallback asset names.

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 7m10s), codex_security (codex/security, done, 4m39s) | Total: 11m58s

@mariusvniekerk
Harden manifest and checksum fallback fetches
dc42b1c 
Release manifests decide which signed or unsigned update metadata a client will inspect, so they should not be fetched over cleartext transport even when later archive verification uses signatures. Checksum fallback probing also should not hide real request, cancellation, or read errors as though a checksum asset was merely absent.

Require HTTPS and downgrade rejection for manifest fetches in every mode, and only suppress a typed not-found checksum asset miss while propagating cancellation, transport, and response-size failures.

Validation: go test ./selfupdate; go test ./...

Generated with Codex
Co-authored-by: Marius van Niekerk <[email protected]>
@roborev-ci

roborev-ci Bot commented Jun 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (dc42b1c)

Summary verdict: No Medium, High, or Critical findings to report.

The only reported findings were Low severity, and the security review found no exploitable issue.

Panel: ci_default_security | Synthesis: codex, 4s | Members: codex_default (codex/default, done, 8m5s), codex_security (codex/security, done, 3m34s) | Total: 11m43s

@wesm wesm merged commit d685a87 into main Jun 16, 2026
8 checks passed
@wesm wesm deleted the selfupdate-release-discovery branch June 16, 2026 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariusvniekerk mariusvniekerk mariusvniekerk approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wesm @mariusvniekerk