feat(updater): GitHub release source with CDN fallback

[yzs15]

Summary

Add a GitHub Releases upgrade source that runs before the existing CDN, with automatic fallback on timeout, rate-limit, or sustained slow download. Default OFF (UPGRADE_GITHUB_ENABLED=false), so day-1 behavior is byte-identical to today.

• Public repo agentserver/app, anonymous queries (no token).
• Fallback triggers: manifest timeout, HTTP 403/429, download first-byte timeout, sustained speed < 100 KB/s over 10s window.
• SHA256 verification stays in the scheduler; mismatch triggers fallback (stage="verify").
• Rolling 5-entry LastFallbacks history in update-state.json so ops can see days later why every attempt failed.
• Release pipeline publishes latest.json to the GitHub release alongside the .exe (in-scope: .github/workflows/release.yml + scripts/windows-package-common.sh).

Design & review artifacts

• Spec (v3, 2 review rounds): docs/superpowers/specs/2026-06-29-upgrade-github-source-fallback-design.md
• Plan (v3, 2 review rounds): docs/superpowers/plans/2026-06-29-upgrade-github-source-fallback.md
• Code review round: independent codex reviewer greenlit at final round (BLOCKER=GAP=security-RISK=0).

Security posture

• Host allowlist per source; GitHub source accepts github.com, codeload.github.com, *.githubusercontent.com (defends against suffix bypass, trailing dot, IPv4/IPv6 literals, userinfo bypass).
• CDN source preserves the assets.agent.cs.ac.cn pin — Manifest.Validate() is now format-only, so the host check moved to source_cdn.validateInstallerURL with adversarial tests (5 migrated from manifest_test.go + trailing-dot).
• Per-request ManifestTimeout (not shared across 2 GitHub hops).
• UPGRADE_GITHUB_REPO regex-validated (defeats ../etc/passwd path traversal).
• Rate-limit reason string redacts X-GitHub-Request-Id (avoid leaking identifying tokens to state.json / console API).
• Release workflow refuses tag re-run, uploads .exe first without --clobber, round-trip verifies remote SHA before publishing latest.json.

Zero-regression guarantee

Compat shortcut (Sources==nil): Service.effectiveSources() lazily builds [cdnSource] from ManifestURL + Client with a zero-policy (no timeout, no speed monitor). All 28 existing service_test.go fixtures using assetsHostClient (custom RoundTripper) work unchanged — the new applyFirstByteTimeout preserves custom transports.

Test plan

• go test ./... -count=1 — all packages green (one flake in internal/slave/ unrelated to updater).
• UPGRADE_GITHUB_ENABLED=true go test ./internal/updater/... ./cmd/launcher/... — green.
• service_source_test.go substring lint intact (start = StartInstaller + startContext = context.Background() preserved in refactor).
• Adversarial host matcher tests (TestGithubAssetHostMatcher, TestCDNSourceRejects*) all pass.
• Release workflow validated on next tagged canary (requires actual tag push).
• Flip UPGRADE_GITHUB_ENABLED=true on canary launcher after v0.0.9 publishes latest.json.

🤖 Generated with Claude Code

[yzs15]

Fresh reviewer round — P2/P3 findings (recorded, not fixed)

Per user direction: P0 + P1 fixed in b725075; P2 and P3 findings tracked here for a follow-up.

P2 — Nice to fix (deferred)

1. config.go:24 — repo slug regex admits owner/. / owner/... GitHub API rejects them so no real damage, but the request line looks suspicious in logs. Tighten regex to forbid all-dot segments.
2. source_github.go — rate-limit reason string includes X-RateLimit-Remaining in LastError/LastFallbacks.Reason exposed via console API. Low risk (not identifying alone) but consider bucketing to "exhausted" / "throttled".
3. service.go — saveError vs saveErrorWithFallbacks split is fragile. Two nearly-identical methods; picking the wrong one silently drops history. Could merge via mergeFallbacks(prior, nil) == prior.
4. service_fallback_test.go:167 — TestServiceParentCtxCancellationSkipsFallback passes for wrong reason. Uses context.Canceled as fetchErr; can't distinguish "returned early via ctx check" from "appended fallback then continued". Swap to plain error to make the invariant meaningful.
5. source_github_test.go — TestGithubAssetHostMatcher uses bracketed [::1] but url.Hostname() strips brackets. Add unbracketed variants (::1, fe80::1) and a punycode/IDN case.
6. service.go — Check treats vcmp <= 0 as StatusLatest (stops iteration); DownloadAndStart treats it as fallback (continues). If GitHub serves stale 0.9.0 while CDN has 1.0.0, Check never queries CDN. Spec risk section covers this as a publish-order concern — leaving as documented behavior.
7. source_cdn.go — redundant outer defer cancel() alongside the inner defer. Both run; not a bug, but invites future breakage.
8. release.yml — no OIDC / attestation. Consider attestations: write + actions/attest-build-provenance for a later provenance-verification story.
9. release.yml — dist/latest-cdn.json generated but never uploaded. Either publish it or drop the render call.
10. source_github.go — no assert that manifest.Version matches release tag_name. Cheap sanity check: "v"+m.Version == release.TagName.

P3 — Nits

1. source.go — applyFirstByteTimeout doc overstates savings — shallow c := *base still allocates a client per call. Trivial.
2. speed_monitor.go — trailingBPS and instantBPS are the same function. Delete one.
3. config.go — silent-fail on bad env vars — no operator feedback. Add log.Printf("upgrade: ignoring invalid %s=%q", …).
4. source_github.go — redirectPinned re-sets Accept + User-Agent but Go's stdlib preserves them. Defensive no-op — add comment or drop.
5. service.go — compatCDNPolicy() returns zero value. Inline SourcePolicy{} at call site.
6. source_cdn.go + source_github.go — redirectPinned skeleton duplicated. Extract shared helper in source.go.

[yzs15]

Round 2 review — P2/P3 (recorded, not fixed)

Round 2 reviewer found 0 P0, 2 P1 (both fixed in 9fd037b). Remaining P2/P3 tracked here.

P2

1. Missing redirect-security tests — CheckRedirect enforces https/no-userinfo/host on every hop but no test exercises those branches. Add: 302 → http://… rejected; 302 → https://user@github.com/… rejected; 302 → https://evil.example.com/… rejected. Same for cdn source.
2. context.WithTimeout(dlCtx, FirstByteTimeout) in the custom-RoundTripper fallback puts a whole-request deadline, not a first-byte deadline — semantically divergent from the Transport path (ResponseHeaderTimeout). Test-only, but rename or document.
3. Release workflow guard only checks latest.json — if a prior run uploaded .exe then failed before latest.json, the .exe guard passes but the upload will error (no --clobber). Detect orphan .exe or document recovery.
4. X-RateLimit-Remaining inserted verbatim into err.Error() and stored in LastFallbacks[].Reason. Low risk (TLS-sourced from GitHub) but add a hard truncate (if len(rem) > 32 { rem = rem[:32] }).
5. Source interface doc says "Callers MUST NOT pass a manifest from a different source" but compat mode does exactly that (safe because per-source validator re-checks). Clarify contract: "Each source MUST validate m.URL before use; callers may pass a foreign manifest and rely on source-side rejection."
6. tryOneSource attemptOutcome discriminated-union struct is fragile — a continueLoop: true on a terminalState != nil path silently confuses caller. Consider splitting into resolveManifest + runAttempt, or Go-native return triple.

P3

1. config.go:24 repo slug regex allows owner/.hidden — GitHub rejects but tighten regex.
2. service.go saveError/saveErrorWithFallbacks re-load state that caller already loaded; cosmetic (guarded by serviceStateMu).
3. service.go appendFallback + mergeFallbacks are near-duplicates; collapse.
4. state.go time.Time field has both omitempty (deprecated for time in Go 1.24+) and omitzero; drop omitempty.
5. service.go saveFinalState → saveError → saveState recursion depth worth a comment.
6. source_github.go redirectPinned prior chain is dead code in production — base.CheckRedirect always nil today.
7. .github/workflows/release.yml references ./scripts/package-windows.sh in a comment — verify path (it exists, checked).

[yzs15]

Round 3 review — 0 P0, 2 P1 (1 fixed, 1 rejected as out-of-scope)

P1 fixed in e0d6990

• assetURL https-scheme guard (defense-in-depth on the first hop from release API to latest.json).

P1 pushback (out-of-scope)

• Grep-based tests in service_source_test.go / installer_windows_source_test.go / manifest_test.go:145 — these are pre-existing master tech debt from commit 1e5b681 fix: remove updater test hooks and detach installer start, not introduced by this PR. Rewriting them as behavior tests (per reviewer's suggestion: Windows-tagged tests invoking StartInstaller with cancelled ctx) is a separate cleanup and doesn't block this feature.

P2 (deferred)

1. X-RateLimit-Remaining in err.Error() — consistency with "no identifying tokens" — strip or document.
2. 403 on asset fetch → ErrRateLimited — should be ErrHostNotAllowed/generic; fallback behavior still correct.
3. saveError vs saveErrorWithFallbacks split enforced by comment only.
4. monitorRequired returns true whenever onProgress != nil — a future UI progress hookup could accidentally break compat "byte-identical" guarantee.
5. Comment "per-request ManifestTimeout not shared across 2 GitHub hops" oversells — it's per-Go-request, not per-network-hop.
6. Workflow .exe upload has no orphan detection on retry.
7. Workflow EXE_NAME=$(ls ... | head -1) picks arbitrary if multiple -setup.exe present.
8. installerCachePath derives from basename without version scope.
9. stat -c%s || stat -f%z fallback — prefer wc -c (POSIX).

P3

1. normalizeHost trims only one trailing dot; use strings.TrimRight(host, ".").
2. TestGithubAssetHostMatcher uses [::1] (brackets) — production Hostname() strips them; use ::1.
3. strings.ContainsAny(sub, "/@") dead code — add // defensive: comment.
4. applyFirstByteTimeout doc references test-only symbol.
5. TestGitHubSourceManifestTimeoutFires elapsed > 300ms bound may flake on loaded CI; widen or drop.
6. state.go omitempty,omitzero redundant.
7. trailingBPS == instantBPS; inline.
8. attemptOutcome struct — tagged-union style would be cleaner.
9. tryOneSource captures fallbacks by reference (read-only); add explanatory comment.
10. TestServiceCheckAllSourcesFail doesn't assert Reason contents.
11. manifest.go Validate() doc doesn't warn "host allowlist enforced elsewhere".
12. config.go fallback to bare &http.Transport{} if DefaultTransport type-asserts fail.
13. start == nil → StartInstaller/context.Background() swap inline — hoist to method.

[yzs15]

Round 4 review — 0 P0, 4 P1 all fixed in 285e005

• Release workflow: added gh release create (published, not draft — draft assets aren't served via public download URL that round-trip verify uses).
• Speed monitor: start now captured on first non-zero byte via atomic CAS (was captured at goroutine spawn → dial/TLS bled into elapsed → false slow-download trips on slow-start).
• Fallback-path context.WithTimeout(dlCtx, FirstByteTimeout) scoped to headers only (matches production ResponseHeaderTimeout semantics) — fbCancel() fires right after client.Do returns.
• classifyDownload wraps context.DeadlineExceeded as ErrFetchTimeout so state reason distinguishes first-byte timeout from dial error.

Round 4 P2/P3 recorded

P2

1. service.go attemptOutcome.continueLoop is dead code — every fallback branch also sets it true, terminal branches return early. Delete field or add a non-retryable-error path.
2. source_github.go:361 req.Err() == context.DeadlineExceeded should be errors.Is for wrapped-ctx forward-compat.
3. source_github.go githubAssetHost accepts evil..githubusercontent.com (double-dot host: sub = "evil.", passes). Add strings.Contains(sub, "..") reject and restrict sub to [A-Za-z0-9.-].
4. service.go installerCachePath doesn't handle name == ".." (URL path /..). Produces "..exe". Add ".." to reserved-name list.
5. service.go appendFallback/mergeFallbacks overlap — collapse into capTail(slice, n).
6. source_github.go:257 LimitReader(_, 512*1024) for release JSON — hardcoded, extract named const.
7. scripts/windows-package-common.sh two jq -n blocks byte-identical except URL; factor helper.
8. .github/workflows/release.yml latest-cdn.json generated + test -f'd but never uploaded/archived. Add actions/upload-artifact or drop.

P3

1. config.go:86-91 newIsolatedHTTPClient bare &http.Transport{} fallback loses proxy/dialer. Log or panic in the else-branch.
2. service.go:262-289 attemptOutcome doc longer than struct — tagged-union style clearer.
3. source_github.go:120-122 apiClient/assetClient/installerClient accessor methods add nothing — read fields directly.
4. service.go:450 name = filepath.Base(name) after name += ".exe" is a no-op.
5. source_github.go:181 first latest.json-named asset picked; case-mismatched (Latest.json) upload silently produces "missing" error.
6. source_github_test.go:271-294 TestGitHubSourceRejectsUnwhitelistedInstallerHost accepts either fetch OR download rejection — split into two.
7. source_github_test.go matcher coverage: no evil..githubusercontent.com, no xn-- punycode, no [::1]:443 port case.
8. source_github.go:156-159 len(via) >= 10 redirect limit duplicated per redirectPinned — package const.

[yzs15]

Round 5 review — 0 P0, 3 P1 all fixed in dd5adb3

• fbCancel semantics rewrite: my round-4 "release" pattern was materially wrong — context.WithTimeout cancel does not release, it cancels. Reproducer confirmed. Rewrote firstByteDeadlineCtx to return (ctx, markHeadersReceived, stop) with time.AfterFunc that cancels iff !gotHeaders. Body reads survive the first-byte timeout.
• Test coverage for the custom-RT path: added two behavior tests using a slowHeadersRT fake — one proves timeout fires before headers, one proves body streaming (30ms/byte × 100 bytes ≈ 3s) survives a 100ms first-byte timeout. Second would have failed under round-4 code.
• Disk-fill DoS via m.Size: Manifest.Validate now caps at MaxInstallerBytes = 500 * 1024 * 1024. Hostile manifest can no longer make the download loop write arbitrary bytes to disk before SHA verify.

Round 5 P2/P3 recorded

P2

1. githubAssetHost accepts sub == "." (host ..githubusercontent.com). DNS-invalid but tighten to reject leading/trailing dot and ...
2. githubAPIHost inherits githubAssetHost allowing github.com/ghapi → github.com/codeload.github.com redirect. Narrow to *.githubusercontent.com only for API-hop.
3. service.go per-attempt context.WithCancel(ctx) + immediate cancel() in Check/DownloadAndStart adds no value — sources are synchronous. Simplify.
4. saveError/saveErrorWithFallbacks split still fragile — collapse to saveError(now, err, freshFallbacks...).
5. source_github.go release JSON limit 512*1024 hardcoded — extract const alongside manifestMaxBytes.
6. newIsolatedHTTPClient bare &http.Transport{} fallback — log or panic.
7. NewGitHubSource should reject non-https apiBase.
8. speedMonitor startNS==0 sentinel + m.now().UnixNano() — if now() returns Unix(0,0), CAS never sets. Use separate atomic.Bool started.

P3

1. FetchManifest returns manifest without host validation (only userinfo) — state persists suspect URL briefly. Fail-fast at fetch.
2. Workflow latest-cdn.json rendered but never shipped.
3. Workflow ${TAG}/${EXE_NAME} interpolated without URL-encoding — belt-and-braces jq @uri.
4. sha256sum not on macOS default; use openssl dgst -sha256 for cross-OS.
5. DownloadAndStart fallback Stage always "download" — doesn't split by sentinel (ErrRateLimited vs ErrFetchTimeout vs ErrSlowDownload).

[yzs15]

Round 6 review — 0 P0, 2 P1 all fixed in bdff511

• Headers-then-hang attack closed. Unified first-byte signal via speedMonitor.onFirstByte callback fired from countingReader on first non-zero body Read. Sources wire it as markFirstByte so the deadline timer only stops when a BODY byte arrives, not on header receipt. monitorRequired widens to FirstByteTimeout > 0 so countingReader exists whenever a timer is armed. Regression test: TestGitHubSourceRejectsHeadersThenHangBody — 100ms timeout + hanging body → aborts at ~100ms.
• saveError unified. Collapsed into single variadic method that always merges prior + fresh. Wrapper saveErrorWithFallbacks retained for existing call sites. New code can't pick the wrong one.

Round 6 P2/P3 recorded

P2

1. Workflow recovery path: .exe uploaded but latest.json upload failure → next re-run passes guard but gh release upload fails on existing .exe. Consider SHA-match skip.
2. CDN policy in multi-source mode uses DefaultSourcePolicy — no UPGRADE_CDN_* env override. Document coupling or add knobs.
3. s.pinned = s.buildPinnedClient(); applyFirstByteTimeout(s.pinned, …) clones Transport → manifest + installer get separate connection pools. Wastes one TLS handshake per download. Consider building install from s.client directly.
4. SHA256 verify uses strings.EqualFold, not constant-time — hash is public, so not exploitable, flagged for the sweep.
5. redirectPinned re-uses s.validateInstallerURL for both manifest and installer redirects. Behavior correct, naming hazardous. Rename to validateAssetURL or split.

P3

1. source_github.go "Go's stdlib strips Authorization" comment — Accept/UA are actually preserved; comment reads as fix. Reword to "defense-in-depth".
2. newIsolatedHTTPClient bare &http.Transport{} fallback is dead (http.DefaultTransport is always *http.Transport). Drop or comment as defensive.
3. MaxInstallerBytes = 500 MB — cross-link to verifyInstaller (streams via io.Copy, RAM fine, disk-fill capped).
4. TestGitHubSourceRejectsUnwhitelistedInstallerHost accepts either fetch OR download rejection — tighten to specific stage.
5. defer stopFB() order — worth a one-line comment noting LIFO defer semantics.

[yzs15]

Round 7 review — 0 P0, 3 P1 all fixed in a69bc26

• Production headers-then-hang defense: round-6 only fixed the fallback (test) path — production still relied on ResponseHeaderTimeout (headers only). Removed the isRealTransport() short-circuit — firstByteDeadlineCtx now armed on all paths. New TestGitHubSourceRejectsHeadersThenHangBodyProduction uses a real TLS server + real *http.Transport and verifies abort at ~150ms with errors.Is(err, ErrFetchTimeout).
• tripped signal: added a return value from firstByteDeadlineCtx so classify can distinguish timer-fired from other context.Canceled. Prior code recorded raw wrapped-canceled strings; now wraps as ErrFetchTimeout.
• CAS single-winner for the timer/markFirstByte race — the O(ns) window where both fire simultaneously.
• classifyFetch now uses errors.Is for context.DeadlineExceeded (was raw ==).

Round 7 P2/P3 recorded

P2

1. service.go per-attempt context.WithCancel(ctx) + immediate cancel() in Check — dead code (fetch is synchronous).
2. verifyInstaller returns "size mismatch" and "sha256 mismatch" both wrapped as ErrSHA256Mismatch — split into ErrSHA256Mismatch and ErrSizeMismatch, or rename to ErrVerifyFailed.
3. manifest_test.go — add TestManifestValidateRejectsSizeOverMax (Size = 500 MB + 1).
4. source_cdn.go FetchManifest doesn't validate s.manifestURL scheme — reject non-https at construction time.
5. .github/workflows/release.yml gh release create without --latest=false — during .exe-uploaded/latest.json-not-yet window, GET /releases/latest returns a release without latest.json → spurious FallbackRecord on every launcher.
6. saveError's variadic-of-slices signature — replace with plain fresh []FallbackRecord for less ambiguity.
7. source_github.go — no cross-check that release.TagName == "v"+m.Version.

P3

1. service.go s.appendFallback caps + mergeFallbacks caps again — redundant.
2. source.go hasRealTransport doc: fallback exists only for tests.
3. source_github.go "stdlib strips Authorization" comment — Accept/UA preserved, comment misleads.
4. scripts/windows-package-common.sh latest-cdn.json written but never uploaded in CI — dead output.
5. service.go compatCDNPolicy() returns zero — rename or inline.
6. source_github.go rebuildClients as a test seam — consider WithInstallerHostMatcherForTest.
7. source_github_test.go:283-289 stale comment about "userinfo rejected inline".

[yzs15]

Round 8 review — 0 P0, 0 P1 ✅ LOOP TERMINATES

Fresh reviewer confirms: seven prior rounds resolved all security- and correctness-critical items.

Round 8 P2/P3 (recorded, not fixed)

P2

1. saveFinalState drops in-progress fallbacks on save failure — nested-failure edge case; thread fallbacks through.
2. Workflow partial-failure re-run wedges tag if .exe uploaded but latest.json failed — add cleanup or runbook.
3. Coverage gap: no test explicitly exercises CheckRedirect rejecting 302 to non-allowlisted host.
4. githubAssetHost strings.ContainsAny(sub, "/@") dead defensive — Hostname() strips both.
5. saveErrorWithFallbacks wrapper prior param unused.

P3

1. applyFirstByteTimeout docstring stale.
2. redirectPinned Accept+UA re-install is no-op (stdlib preserves).
3. FetchManifest asset-host vs installer-host validation asymmetric.
4. attemptOutcome 5-field struct — tagged union tidier.
5. RateLimit403 test doesn't assert X-GitHub-Request-Id omitted.
6. Multi-source DownloadAndStart still Validate()s caller-m before loop.
7. cdnSource.classify DeadlineExceeded internal-path note.