feat(update): self-update via suprim-server feed

[sant1ago-da-hanoi]

Summary

Wires SuprimSQL into the /suprim/update/latest feed served by
suprim-server. On startup the app polls the feed; when the server reports
a newer release, a compact "Update available!" badge appears next to
the existing Premium/Free pill in the status bar. Click → download →
verify SHA-256 → mount DMG → replace /Applications/SuprimSQL.app →
relaunch.

Stack

Layer	Detail
HTTP client	reqwest 0.13 (rustls, json, stream, query)
Version compare	semver 1
Hashing	sha2 0.10
Async	existing tokio runtime
UI	custom egui painter sharing `draw_badge()` with tier badge
macOS install	`/usr/bin/hdiutil` + `/bin/cp` + `open -n`

Modules

• `update/check.rs` — endpoint poll, semver compare, error taxonomy
• `update/state.rs` — state machine shared between UI and worker
• `update/install.rs` — download / verify / mount / copy / relaunch
• `ui/statusbar.rs` — badge rendering + click dispatch
• `ui/update_banner.rs` — handler-only wiring for status-bar actions

Endpoint override

`SUPRIM_UPDATE_ENDPOINT=http://localhost:8888/update/latest\` redirects
the client at a local mock server (FastAPI or similar) for dev testing
without touching the live feed.

Tests

• 114 passing, 0 failed across the `suprim-app` binary (up from 65
  on main — 49 new tests for this PR)
• 0 clippy warnings under `cargo clippy -p suprim-app --all-targets -- -D warnings`

Coverage highlights:

• Semver comparison edge cases (equal / newer / older / invalid)
• Snake_case JSON deserialization against the Jackson output shape
• Wiremock-mocked HTTP flows: newer / older / equal / envelope-error /
  null-payload / network-failure
• SHA-256 verification: match / case-insensitive / mismatch-deletes-file /
  missing-file
• `hdiutil` plist parsing: canonical output / empty / key-missing /
  first-occurrence-on-duplicate
• Bundle replacement: copy / remove-old-first / missing-source error
• Streaming download with monotonic progress callback
• `install_inner` error propagation (bad hash / 5xx)
• Every status-bar state renders without panic (painter-based smoke test)
• Status-bar action taxonomy + dismiss semantics

Testability refactors

Four extract-and-inject refactors so the platform shell-outs aren't in
the way of unit tests:

• `download_dmg` → `stream_to_file(url, dest, fallback, on_progress)`
• `cache_path` → `cache_path_in(root, version)`
• `copy_app` → `copy_app_to(mount, dest)`
• `mount_dmg` → `parse_mount_point(plist)`

Intentional gaps

Documented in code comments:

• `install_update` orchestrator (calls `std::process::exit(0)`)
• `relaunch` (`open -n` subprocess)
• `mount_dmg` / `unmount_dmg` (macOS `hdiutil`)
• `spawn_update_check` in `app.rs` (tokio spawn + egui ctx)

These are end-to-end UI flows covered manually against the mock server;
wiring them into the test harness would require a macOS simulator fake
that outweighs the value.

Screenshot

Status bar with the new badge and tier pill side-by-side:

`[ ⬇ Update available! ] [ 👑 Premium ]`

Hover brightens the pill; click triggers the install pipeline.

[sant1ago-da-hanoi]

Code Review

Files changed: 12 | Lines: +1829 / -54

Summary

Wires self-update pipeline: polls /suprim/update/latest, shows badge in status bar, downloads DMG, verifies SHA-256, swaps /Applications/SuprimSQL.app, relaunches. Architecture clean, testability refactors solid, 49 new tests with wiremock. But ships self-update without signature verification and points to a personal domain — both are hard blockers for a prod auto-updater.

Findings

Critical

#	File	Category	Issue	Suggestion
1	crates/suprim-app/src/update/mod.rs:25	Security	DEFAULT_ENDPOINT points to api.sant1ago.dev — personal domain. If transferred/expired/compromised, attacker controls payload (incl. sha256) for every SuprimSQL user.	Move to api.suprim.dev (corp-owned). Add CI check to block non-corp domains in release builds.
2	crates/suprim-app/src/update/install.rs:1343	Security	SHA-256 only validates transport integrity, not authenticity. Compromised server or MITM with valid cert can serve malicious DMG + matching hash. No code signature or signed manifest check.	Add one of: (a) codesign --verify --deep --strict + Team ID match after copy, (b) Ed25519-signed manifest with pubkey baked into binary (Sparkle EdDSA model), (c) TLS cert pinning on reqwest::ClientBuilder.
3	crates/suprim-app/src/update/install.rs:1418-1420	Data	remove_dir_all(dest) before cp -R. Comment claims running binary survives — true for mach-o text, false for lazy resources (.icns, localized strings, fonts, frameworks, dylibs). If cp fails mid-way, user loses the app entirely with no recovery.	Rename-swap pattern: rename(dest, backup) → cp -R → on success remove backup, on failure rename back. Or use ditto to preserve xattrs + code signature.
4	crates/suprim-app/src/update/install.rs:1310-1341	Failure	stream_to_file has no size cap. Hostile/compromised server streams 50GB → fills user disk → macOS swap death.	Cap at 500MB (or whatever the largest expected DMG is). Check content_length before streaming + track written inside the loop.
5	crates/suprim-app/src/update/install.rs:1211	Failure	std::process::exit(0) kills all background tasks mid-flight: pending queries, autosave, workspace.json flush. User loses unsaved tab/query.	Signal UI thread via eframe::Frame::close() or shared flag, let App::on_exit flush state, then exit.
6	crates/suprim-app/src/ui/statusbar.rs:329	Performance	update_state.lock() on UI render thread. If async worker holds lock during a 10s network timeout, every repaint blocks → beachball for 10s at startup.	Use try_lock() with stale-state fallback, or arc-swap::ArcSwap<UpdateState> for lock-free reads.

Important

#	File	Category	Issue	Suggestion
7	crates/suprim-app/src/update/install.rs:1260	Observability	let _ = unmount_dmg(...) silently drops detach errors. If detach fails, /Volumes/ leaks until reboot with no signal.	if let Err(e) = unmount_dmg(...) { tracing::warn!(error = %e, "detach failed; /Volumes leak until reboot"); }
8	crates/suprim-app/src/update/install.rs:1166	Portability	Module compiles on Linux/Windows but shells out to hdiutil and open -n which don't exist there. Runtime failure instead of compile-time.	Gate whole module with #[cfg(target_os = "macos")] or return UpdateError::UnsupportedPlatform early per-platform.
9	crates/suprim-app/src/update/state.rs:86	Reliability	set() silently swallows poison. Once poisoned, entire update subsystem dies quietly — no way to diagnose. Test set_recovers_from_poisoned_lock_silently even documents the state stays Idle.	lock().unwrap_or_else(|e| e.into_inner()) to recover and keep functioning.
10	crates/suprim-app/src/update/check.rs:934-936	Observability	.map_err(|_| UpdateError::SemverParse(...)) drops the underlying semver::Error. Debugging why a version failed to parse = impossible from logs.	.map_err(|e| UpdateError::SemverParse(format!("{}: {e}", release.version)))

Minor

#	File	Category	Issue	Suggestion
11	crates/suprim-app/src/ui/statusbar.rs:515	Performance	draw_badge runs layout_no_wrap every frame for every badge. Egui repaints at 60fps → text layout recompute constantly.	Use ui.label(RichText::new(...).background_color(...)) to hit egui's text layout cache.
12	crates/suprim-app/src/app.rs:51	Standards	tooltip_delay = 0.25 applied globally, not scoped to status bar. Commit message doesn't mention this side effect.	Either comment explicitly "intentional global change" or scope to status bar via local Ui::style_mut.
13	crates/suprim-app/src/update/check.rs:915	Standards	"stable" hardcoded. When beta channel lands you'll grep the codebase.	const CHANNEL: &str = "stable"; at module level.
14	crates/suprim-app/Cargo.toml:24	Build	tokio = features = ["full", "test-util"] in dev-deps pulls features unused by tests.	Narrow to ["rt-multi-thread", "macros", "time", "test-util"].
15	crates/suprim-app/src/update/install.rs:1391	Parsing	parse_mount_point doesn't HTML-unescape. Volume name with & in metadata → PathBuf("&") which doesn't exist on disk.	Unescape & / < / > / " / ' after extraction. Low priority since SuprimSQL controls DMG metadata today.

Positive Patterns

• Testability refactors (stream_to_file, parse_mount_point, copy_app_to, cache_path_in) — textbook extract-pure-core.
• mount_dmg uses -plist instead of grep-ing stdout. Robust.
• SHA-256 mismatch deletes the cached file so retry doesn't trust corruption.
• Envelope deserialization handles code != 1, null data, snake_case with explicit test cases.

Verdict: REQUEST CHANGES

6 critical, 4 important, 5 minor. Findings 1, 2, 3 are hard blockers for a self-updater shipping to prod — personal-domain endpoint + no signature verify + destructive replace = supply-chain attack surface + data-loss risk. Fix the six criticals before merging.

Unresolved questions

1. Linux/Windows plan for the install flow? Module compiles everywhere but only runs on macOS.
2. Is /suprim/update/latest rate-limited / authed server-side?
3. Startup-only check or periodic poll? Users leaving the app open all day miss updates.
4. Delta updates planned or full-DMG replacement forever? (Affects finding 4's cap.)

[sant1ago-da-hanoi]

Re-review — commits 5780fe1 → 8d734d7

Verified all 15 findings against the fix-diff.

Findings 1-15: resolved

#	Fix	Verified at
1	Endpoint → api.suprim.dev + option_env! build-time override	update/mod.rs:35-39
2	verify_code_signature — codesign --verify --deep --strict + TeamIdentifier match	update/install.rs (see N1 below)
3	Rename-swap atomic replace with rollback	copy_app_to
4	500MB cap, double-check (header + streaming accumulator), partial file cleanup	stream_to_file
5	ViewportCommand::Close instead of exit(0) — fires App::on_exit	install_update
6	try_lock() on UI render thread	statusbar.rs:48
7	tracing::warn! on unmount failure	install_inner:130-136
8	#[cfg(target_os = "macos")] gate on install pipeline	install.rs
9	set() recovers via poisoned.into_inner()	state.rs
10	SemverParse carries underlying error	check.rs:111-115
11	WidgetText::into_galley hits text layout cache	draw_badge
12	Tooltip delay comment marks it "GLOBAL ... intentional"	app.rs:101-107
13	STABLE_CHANNEL const	check.rs:12
14	Tokio features narrowed	Cargo.toml:77
15	unescape_xml + decode order preserves <	parse_mount_point

New findings from the fix commits

Critical

#	File	Category	Issue	Suggestion
N1	crates/suprim-app/src/update/install.rs (EXPECTED_TEAM_ID + verify_code_signature)	Security	verify_code_signature silently skips when SUPRIM_TEAM_ID is unset (only tracing::warn). A release build that forgets the env var ships with signature verification disabled — regressing finding #2 with nothing screaming in the build log.	Add a build.rs guard or compile_error! when target is macOS + release profile + SUPRIM_TEAM_ID unset. Fail the build instead of warning at runtime.

Minor

#	File	Category	Issue	Suggestion
N2	crates/suprim-app/src/update/install.rs (install_update non-macOS branch)	UX	On Linux/Windows the "Update available!" badge still renders and clicking spawns a tokio task that immediately sets Failed. User sees a clickable update that can't install.	Short-circuit in handle_status_action or update_badge_style on non-macOS so the badge either hides or renders as "Download manually" with a URL.
N3	crates/suprim-app/src/update/install.rs (stream_to_file_enforces_cap_when_content_length_is_missing)	Tests	Test name says "missing Content-Length" but wiremock always sets it — test doesn't exercise what the name claims. Comment even admits the combined behaviour isn't covered.	Rename to reflect what it actually tests, or add a test that genuinely streams past the cap without a Content-Length header.
N4	crates/suprim-app/src/update/install.rs (copy_app_to_rolls_back_when_cp_fails)	Tests	Test name says "rolls back when cp fails" but the test only covers the success path and backup cleanup. Doesn't trigger a cp failure.	Either rename to copy_app_to_removes_backup_on_success or force cp failure (read-only dest parent) and verify rollback restores the old bundle.
N5	crates/suprim-app/src/update/install.rs (verify_code_signature)	Docs	Reading metadata from stderr (display.stderr) is correct — codesign -dvv writes to stderr. Non-obvious; future reader might "fix" it to stdout.	Add a one-line comment: // codesign -dvv writes metadata to stderr, not stdout — do not "fix" this.

Verdict: Nearly ready — fix N1 before merge

N1 is a footgun: release pipeline forgets one env var → signature verification silently off → finding #2 regresses without a compile error or test catching it. Make the build fail loudly instead.

N2-N5 are non-blocking cleanups; fine as follow-up.