From 0985726415e5c4dd58e1edbb7c28c9786074c43e Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 1 Aug 2026 00:37:52 +0200 Subject: [PATCH] fix(host,web): an empty update channel stops looking like a broken host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A channel nobody has published to answers `manifest.json` with a 404, and the check reported that the same way it reports a dead registry or a bad signature: "Last check failed: feed returned HTTP 404". Every host on the stable channel shows it today, because the stable manifest only publishes when someone dispatches `announce` for a release tag — so the first thing an operator sees from the new Updates card is a red failure caused by nothing being wrong. The shared checker now distinguishes the two. `feed::fetch_manifest_blocking` returns a typed `FeedError` instead of a string, and only a 404 on the manifest ITSELF becomes `NotPublished` — a 404 on the detached signature still fails loudly, because that is the half-published pair the manifest-then-signature upload order can produce, and it must stay fail-closed. The host carries that through as `UpdateStatus.not_published`, mutually exclusive with `last_error`. It is benign only while no manifest has ever been seen for the channel: once a check has succeeded, the same 404 means the feed LOST a document it used to serve, which stays an error. The console then shows a plain sentence naming the channel instead of the failure banner, and "None published yet" rather than "Not checked yet". The Linux client makes the same distinction but deliberately NOT the same choice: `--check-update` keeps exiting 1 and keeps `error` set, because its consumer is a shell script and an empty channel is the absence of evidence that this build is current — not a confirmation that it is. Co-Authored-By: Claude Opus 5 (1M context) --- api/openapi.json | 9 +- clients/linux/src/cli.rs | 19 +++- crates/pf-client-core/src/update.rs | 16 +++- crates/pf-update-check/src/feed.rs | 111 ++++++++++++++++++++--- crates/pf-update-check/src/lib.rs | 1 + crates/punktfunk-host/src/mgmt/update.rs | 9 +- crates/punktfunk-host/src/update.rs | 68 ++++++++++++-- web/messages/de.json | 2 + web/messages/en.json | 2 + web/src/sections/Host/UpdateCard.tsx | 15 ++- 10 files changed, 225 insertions(+), 27 deletions(-) diff --git a/api/openapi.json b/api/openapi.json index 2cd138f7..13ed549f 100644 --- a/api/openapi.json +++ b/api/openapi.json @@ -3495,7 +3495,7 @@ "operationId": "forceUpdateCheck", "responses": { "200": { - "description": "Refreshed update-check state (`last_error` carries a failed check)", + "description": "Refreshed update-check state (`last_error` carries a failed check; `not_published` an empty channel, which is not one)", "content": { "application/json": { "schema": { @@ -7372,7 +7372,8 @@ "apply", "channel_hint", "check_disabled", - "available" + "available", + "not_published" ], "properties": { "apply": { @@ -7452,6 +7453,10 @@ } ] }, + "not_published": { + "type": "boolean", + "description": "The check reached the feed and found this channel has **no release published yet** —\nan expected state (a channel nobody has announced to answers with a 404), not a\nfailure. Mutually exclusive with `last_error`, so a UI can say \"nothing published yet\"\ninstead of painting an empty feed as a broken host. Never set once a manifest has been\nseen for this channel: a feed that loses a document it used to serve stays an error." + }, "opt_in_hint": { "type": [ "string", diff --git a/clients/linux/src/cli.rs b/clients/linux/src/cli.rs index 97fddc09..f4729d00 100644 --- a/clients/linux/src/cli.rs +++ b/clients/linux/src/cli.rs @@ -480,8 +480,23 @@ fn headless_check_update() -> glib::ExitCode { "installed {} ({}, {})", status.current, status.kind, status.channel ); - println!("available {}", status.latest); - if let Some(err) = &status.error { + // `latest` falls back to `current` when the check couldn't run — printing that as + // "available" would read as a confirmed answer we don't have. + if status.error.is_some() { + println!("available unknown"); + } else { + println!("available {}", status.latest); + } + if status.not_published { + // Says what it is, in words, instead of a raw HTTP status. The exit code still + // reports "could not tell" (see the doc comment above): an empty channel is the + // absence of evidence that this build is current, and a mistyped + // PUNKTFUNK_UPDATE_FEED is indistinguishable from one out here. + println!( + "update nothing published on the {} channel yet", + status.channel + ); + } else if let Some(err) = &status.error { eprintln!("check-update: {err}"); } else if status.update_available { println!("update yes"); diff --git a/crates/pf-client-core/src/update.rs b/crates/pf-client-core/src/update.rs index 5d788087..b33d9cb3 100644 --- a/crates/pf-client-core/src/update.rs +++ b/crates/pf-client-core/src/update.rs @@ -99,6 +99,18 @@ pub struct Status { /// Why the check couldn't complete. `update_available` is always false when set. #[serde(skip_serializing_if = "Option::is_none")] pub error: Option, + /// The feed answered, but this channel has **no release published yet** — an expected + /// state rather than a malfunction, so a caller can say so plainly instead of showing a + /// raw "HTTP 404". + /// + /// Deliberately NOT symmetric with the host's `UpdateStatus`, which clears `last_error` + /// for this case: there the consumer is a human reading a console, and a red "last check + /// failed" on an empty feed is the bug being fixed. Here the consumer is a shell script + /// reading an exit code, so `error` stays set and `--check-update` keeps returning 1. + /// An empty channel is not evidence that this build is current, and a mistyped + /// `PUNKTFUNK_UPDATE_FEED` is indistinguishable from one out here. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub not_published: bool, } /// The Ed25519 keys trusted for update manifests — pinned once in [`pf_update_check`] so the @@ -302,6 +314,7 @@ pub fn check(current: &str) -> Status { opt_in_hint: opt_in_would_help(kind, caps).then(opt_in_hint), notes_url: String::new(), error: None, + not_published: false, }; let (apply, applier) = apply_route(kind, caps); status.apply = apply; @@ -320,7 +333,8 @@ pub fn check(current: &str) -> Status { ) { Ok(m) => m, Err(e) => { - status.error = Some(e); + status.not_published = e.is_not_published(); + status.error = Some(e.to_string()); return status; } }; diff --git a/crates/pf-update-check/src/feed.rs b/crates/pf-update-check/src/feed.rs index a44bd84d..002b0daa 100644 --- a/crates/pf-update-check/src/feed.rs +++ b/crates/pf-update-check/src/feed.rs @@ -19,6 +19,43 @@ pub const DEFAULT_FEED_BASE: &str = /// One fetch's wall-clock budget. const FETCH_TIMEOUT: Duration = Duration::from_secs(15); +/// Why a fetch didn't produce a manifest. +/// +/// Exactly one failure mode is an *expected* steady state: a channel nobody has published to +/// answers `manifest.json` with a 404. Collapsing that into the same string as a transport or +/// signature failure is what made an empty stable feed render as "last check failed: feed +/// returned HTTP 404" — telling operators their box is broken when the feed is merely empty. +/// Everything else stays a real failure, loudly. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum FeedError { + /// This channel has no manifest at all. Note the deliberate narrowness: only a 404 on + /// `manifest.json` itself counts. A 404 on the *signature* means the manifest exists + /// without its proof — a half-published pair (the publisher's manifest-then-signature + /// window, or a botched upload), which must stay fail-closed and noisy. + NotPublished, + /// A real failure: transport, an HTTP status that isn't the empty-channel 404, the size + /// cap, or signature/schema rejection. + Failed(String), +} + +impl FeedError { + /// Is this the benign "nothing published on this channel yet" state? + pub fn is_not_published(&self) -> bool { + matches!(self, Self::NotPublished) + } +} + +impl std::fmt::Display for FeedError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NotPublished => f.write_str("no release has been published on this channel yet"), + Self::Failed(msg) => f.write_str(msg), + } + } +} + +impl std::error::Error for FeedError {} + /// The feed base, with a `PUNKTFUNK_UPDATE_FEED` override for tests and dev feeds. This is /// operator config (an env var on the process), never request-time input; the `https://` (or /// loopback) requirement keeps a stray value from silently downgrading the transport. @@ -36,9 +73,11 @@ pub fn fetch_manifest_blocking( channel: &str, keys: &[PublicKey], user_agent: &str, -) -> Result { +) -> Result { if keys.is_empty() { - return Err("no update key is pinned in this build".into()); + return Err(FeedError::Failed( + "no update key is pinned in this build".into(), + )); } let agent = ureq::AgentBuilder::new() .timeout(FETCH_TIMEOUT) @@ -48,29 +87,42 @@ pub fn fetch_manifest_blocking( let url = format!("{base}/{channel}/manifest.json"); let sig_url = format!("{url}.sig"); - let body = read_capped(agent.get(&url).call().map_err(fetch_err)?)?; + // Only the MANIFEST leg can report an empty channel; see [`FeedError::NotPublished`]. + let body = read_capped(agent.get(&url).call().map_err(manifest_err)?)?; let sig = read_capped(agent.get(&sig_url).call().map_err(fetch_err)?)?; - let sig_text = String::from_utf8(sig).map_err(|_| "signature file is not text".to_string())?; + let sig_text = String::from_utf8(sig) + .map_err(|_| FeedError::Failed("signature file is not text".into()))?; - manifest::verify_and_parse(&body, &sig_text, keys, channel).map_err(|e| format!("{e:#}")) + manifest::verify_and_parse(&body, &sig_text, keys, channel) + .map_err(|e| FeedError::Failed(format!("{e:#}"))) } -fn fetch_err(e: ureq::Error) -> String { +/// The manifest leg: a 404 here means the channel is empty, not broken. +fn manifest_err(e: ureq::Error) -> FeedError { match e { - ureq::Error::Status(code, _) => format!("feed returned HTTP {code}"), - other => format!("feed fetch failed: {other}"), + ureq::Error::Status(404, _) => FeedError::NotPublished, + other => fetch_err(other), } } -fn read_capped(resp: ureq::Response) -> Result, String> { +fn fetch_err(e: ureq::Error) -> FeedError { + FeedError::Failed(match e { + ureq::Error::Status(code, _) => format!("feed returned HTTP {code}"), + other => format!("feed fetch failed: {other}"), + }) +} + +fn read_capped(resp: ureq::Response) -> Result, FeedError> { use std::io::Read as _; let mut buf = Vec::new(); let mut reader = resp.into_reader().take(MAX_MANIFEST_BYTES as u64 + 1); reader .read_to_end(&mut buf) - .map_err(|e| format!("read failed: {e}"))?; + .map_err(|e| FeedError::Failed(format!("read failed: {e}")))?; if buf.len() > MAX_MANIFEST_BYTES { - return Err("response exceeds the manifest size cap".into()); + return Err(FeedError::Failed( + "response exceeds the manifest size cap".into(), + )); } Ok(buf) } @@ -85,7 +137,42 @@ mod tests { // licence to trust whatever the feed serves. let err = fetch_manifest_blocking("https://127.0.0.1:1", "stable", &[], "test").unwrap_err(); - assert!(err.contains("no update key"), "{err}"); + assert!(err.to_string().contains("no update key"), "{err}"); + // And it is a real failure — never the benign empty-channel state. + assert!(!err.is_not_published()); + } + + fn status(code: u16) -> ureq::Error { + ureq::Error::Status(code, ureq::Response::new(code, "status", "").unwrap()) + } + + /// The whole point of the split: an empty channel is not a broken feed. + #[test] + fn manifest_404_is_not_published_but_other_statuses_are_failures() { + assert_eq!(manifest_err(status(404)), FeedError::NotPublished); + for code in [403, 500, 502] { + let e = manifest_err(status(code)); + assert!(!e.is_not_published(), "HTTP {code} must stay a failure"); + assert!(e.to_string().contains(&code.to_string()), "{e}"); + } + } + + /// A missing SIGNATURE is a half-published pair, not an empty channel — the manifest leg + /// already answered 200. Treating it as "nothing published yet" would quietly excuse the + /// one window where a manifest exists without its proof. + #[test] + fn signature_404_stays_a_failure() { + let e = fetch_err(status(404)); + assert!(!e.is_not_published()); + assert_eq!(e.to_string(), "feed returned HTTP 404"); + } + + #[test] + fn not_published_reads_as_plain_english() { + assert_eq!( + FeedError::NotPublished.to_string(), + "no release has been published on this channel yet" + ); } #[test] diff --git a/crates/pf-update-check/src/lib.rs b/crates/pf-update-check/src/lib.rs index 000d8c12..5fdf6cfb 100644 --- a/crates/pf-update-check/src/lib.rs +++ b/crates/pf-update-check/src/lib.rs @@ -35,6 +35,7 @@ pub mod sig; pub mod version; pub use detect::{InstallKind, Product}; +pub use feed::FeedError; pub use manifest::{Manifest, MAX_MANIFEST_BYTES, SCHEMA}; pub use sig::{verify_signature, PublicKey}; pub use version::{canary_run, is_newer, triple, Channel}; diff --git a/crates/punktfunk-host/src/mgmt/update.rs b/crates/punktfunk-host/src/mgmt/update.rs index ba263af9..7b9e2d39 100644 --- a/crates/punktfunk-host/src/mgmt/update.rs +++ b/crates/punktfunk-host/src/mgmt/update.rs @@ -83,6 +83,12 @@ pub(crate) struct UpdateStatus { pub last_checked_unix: Option, /// Why the last check failed, verbatim, if it did. pub last_error: Option, + /// The check reached the feed and found this channel has **no release published yet** — + /// an expected state (a channel nobody has announced to answers with a 404), not a + /// failure. Mutually exclusive with `last_error`, so a UI can say "nothing published yet" + /// instead of painting an empty feed as a broken host. Never set once a manifest has been + /// seen for this channel: a feed that loses a document it used to serve stays an error. + pub not_published: bool, /// This install could one-click apply, but the operator hasn't opted in yet — the /// command to run (Linux: join the `punktfunk-update` group). #[serde(default, skip_serializing_if = "Option::is_none")] @@ -142,6 +148,7 @@ fn status_from(snap: update::Snapshot) -> UpdateStatus { }), last_checked_unix: snap.checked.as_ref().map(|c| c.fetched_unix), last_error: snap.last_error, + not_published: snap.not_published, opt_in_hint: update::opt_in_hint(), job, last_result: snap.last_result.as_ref().map(|r| UpdateResultInfo { @@ -186,7 +193,7 @@ pub(crate) async fn get_update_status() -> Json { tag = "update", operation_id = "forceUpdateCheck", responses( - (status = OK, description = "Refreshed update-check state (`last_error` carries a failed check)", body = UpdateStatus), + (status = OK, description = "Refreshed update-check state (`last_error` carries a failed check; `not_published` an empty channel, which is not one)", body = UpdateStatus), (status = CONFLICT, description = "Update checks are disabled on this host", body = ApiError), (status = TOO_MANY_REQUESTS, description = "A forced check ran less than 30 s ago", body = ApiError), (status = UNAUTHORIZED, description = "Missing or invalid bearer token", body = ApiError), diff --git a/crates/punktfunk-host/src/update.rs b/crates/punktfunk-host/src/update.rs index 86398fd9..095af660 100644 --- a/crates/punktfunk-host/src/update.rs +++ b/crates/punktfunk-host/src/update.rs @@ -27,7 +27,7 @@ pub(crate) use pf_update_check::manifest; pub(crate) mod windows; use manifest::Manifest; -use pf_update_check::PublicKey; +use pf_update_check::{FeedError, PublicKey}; use std::path::{Path, PathBuf}; use std::sync::{Mutex, OnceLock}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; @@ -141,6 +141,9 @@ pub(crate) struct Checked { struct Runtime { checked: Option, last_error: Option, + /// The channel has nothing published (and never had, for us) — an expected state, kept + /// out of `last_error` so a console never paints an empty feed as a broken host. + not_published: bool, /// Refresh in flight (status kicks at most one). refreshing: bool, /// Wall-clock guard for the forced-check rate limit. @@ -214,7 +217,7 @@ fn store_floor(path: &Path, channel: &str, serial: u64) { /// Fetch + verify the channel manifest through the shared checker. Blocking — call from a /// blocking thread. -fn fetch_manifest_blocking(channel: &str) -> Result { +fn fetch_manifest_blocking(channel: &str) -> Result { pf_update_check::feed::fetch_manifest_blocking( &pf_update_check::feed::feed_base(), channel, @@ -227,18 +230,18 @@ fn fetch_manifest_blocking(channel: &str) -> Result { } /// One full refresh: fetch, verify, enforce + raise the serial floor, update the cache, -/// announce a newly available release on the event bus. Returns the user-facing error string -/// on failure (also cached for status). -pub(crate) fn refresh_blocking() -> Result { +/// announce a newly available release on the event bus. Returns the user-facing error on +/// failure (also cached for status). +pub(crate) fn refresh_blocking() -> Result { let (kind, channel) = detect::detect(); let result = fetch_manifest_blocking(channel.as_str()).and_then(|m| { let path = state_path(); let floor = load_floor(&path, channel.as_str()); if m.serial < floor { - return Err(format!( + return Err(FeedError::Failed(format!( "manifest serial {} is older than the last accepted {} — refusing rollback", m.serial, floor - )); + ))); } store_floor(&path, channel.as_str(), m.serial); Ok(m) @@ -268,16 +271,32 @@ pub(crate) fn refresh_blocking() -> Result { }); } rt.last_error = None; + rt.not_published = false; rt.checked = Some(checked.clone()); Ok(checked) } Err(e) => { - rt.last_error = Some(e.clone()); + let (last_error, not_published) = classify_failure(&e, rt.checked.is_some()); + rt.last_error = last_error; + rt.not_published = not_published; Err(e) } } } +/// Split a failed refresh into `(last_error, not_published)` — the two are never both set. +/// +/// An empty channel is only benign while we have never seen a manifest for it. Once a check +/// has succeeded, the same 404 means the feed LOST a document it used to serve, which is a +/// regression that must stay a loud error rather than being excused as "nothing yet". +fn classify_failure(e: &FeedError, had_manifest: bool) -> (Option, bool) { + if e.is_not_published() && !had_manifest { + (None, true) + } else { + (Some(e.to_string()), false) + } +} + /// The status handler's read: current cache + errors, kicking a background refresh when the /// cache is cold and checks are enabled. pub(crate) fn snapshot_and_maybe_refresh() -> Snapshot { @@ -296,6 +315,7 @@ pub(crate) fn snapshot_and_maybe_refresh() -> Snapshot { Snapshot { checked: rt.checked.clone(), last_error: rt.last_error.clone(), + not_published: rt.not_published, job: rt.job.clone(), last_result: jobs::read_result(&jobs::result_path()), } @@ -329,6 +349,7 @@ pub(crate) async fn force_check() -> Result { Ok(Snapshot { checked: rt.checked.clone(), last_error: rt.last_error.clone(), + not_published: rt.not_published, job: rt.job.clone(), last_result: jobs::read_result(&jobs::result_path()), }) @@ -570,6 +591,8 @@ pub(crate) fn reconcile_at_boot() { pub(crate) struct Snapshot { pub checked: Option, pub last_error: Option, + /// The channel simply has no release yet — mutually exclusive with `last_error`. + pub not_published: bool, /// The live apply job, when one runs. When the process was restarted mid-apply this is /// `None` but a fresh intent still reads as in-flight — the API layer surfaces that via /// [`Snapshot::applying_from_intent`]. @@ -640,6 +663,34 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// The console paints `last_error` as a failure and `not_published` as a plain sentence, + /// so the two must never arrive together — and the benign reading must not survive a + /// channel that has already served us a manifest. + #[test] + fn empty_channel_is_benign_only_until_a_manifest_has_been_seen() { + let (err, not_published) = classify_failure(&FeedError::NotPublished, false); + assert_eq!(err, None); + assert!(not_published); + + // Same 404, but the feed used to answer — that is a regression, not "nothing yet". + let (err, not_published) = classify_failure(&FeedError::NotPublished, true); + assert_eq!( + err.as_deref(), + Some("no release has been published on this channel yet") + ); + assert!(!not_published); + + // Every real failure stays an error whether or not we have a cached manifest. + for had_manifest in [false, true] { + let (err, not_published) = classify_failure( + &FeedError::Failed("feed returned HTTP 500".into()), + had_manifest, + ); + assert_eq!(err.as_deref(), Some("feed returned HTTP 500")); + assert!(!not_published); + } + } + #[test] fn pinned_keys_skip_empty_rotation_slot() { let keys = pinned_keys(); @@ -664,6 +715,7 @@ mod tests { fetched_unix: now_unix(), }), last_error: None, + not_published: false, job: None, last_result: None, }; diff --git a/web/messages/de.json b/web/messages/de.json index 3a51bf70..0f78fabe 100644 --- a/web/messages/de.json +++ b/web/messages/de.json @@ -543,6 +543,8 @@ "update_checking": "Prüfe…", "update_last_checked": "Zuletzt geprüft", "update_never_checked": "Noch nicht geprüft", + "update_none_published": "Noch keine veröffentlicht", + "update_not_published": "Auf dem Kanal {channel} wurde noch nichts veröffentlicht. Die Prüfung funktioniert — es gibt nur noch keine Version zum Vergleichen.", "update_stale": "Der Update-Feed hat sich seit über 45 Tagen nicht geändert — Prüfungen gelingen, aber es kommt nichts Neues an. Falls das nicht stimmen kann, prüfe die Verbindung dieses Hosts zu git.unom.io.", "update_disabled": "Update-Prüfungen sind auf diesem Host deaktiviert (PUNKTFUNK_UPDATE_CHECK=0).", "update_error": "Letzte Prüfung fehlgeschlagen:", diff --git a/web/messages/en.json b/web/messages/en.json index 6b095454..7104f83a 100644 --- a/web/messages/en.json +++ b/web/messages/en.json @@ -543,6 +543,8 @@ "update_checking": "Checking…", "update_last_checked": "Last checked", "update_never_checked": "Not checked yet", + "update_none_published": "None published yet", + "update_not_published": "Nothing has been published to the {channel} channel yet. The check itself is working — there's just no release to compare against.", "update_stale": "The update feed hasn't changed in over 45 days — checks succeed but nothing new arrives. If that seems wrong, check this host's connectivity to git.unom.io.", "update_disabled": "Update checks are disabled on this host (PUNKTFUNK_UPDATE_CHECK=0).", "update_error": "Last check failed:", diff --git a/web/src/sections/Host/UpdateCard.tsx b/web/src/sections/Host/UpdateCard.tsx index 0e1978ef..710ea30c 100644 --- a/web/src/sections/Host/UpdateCard.tsx +++ b/web/src/sections/Host/UpdateCard.tsx @@ -162,7 +162,9 @@ export const UpdateCard: FC<{ ) : ( - {m.update_never_checked()} + {s.not_published + ? m.update_none_published() + : m.update_never_checked()} ) } @@ -211,6 +213,17 @@ export const UpdateCard: FC<{ {m.update_stale()}

)} + {/* An empty channel is a normal state, not a fault: it looks like a + 404 down at the transport, but it means "nobody has announced a + release here yet". Rendering it in the failure style told + operators their host was broken when nothing was. The host keeps + the two apart (`not_published` is never set alongside + `last_error`), so this stays a straight either/or. */} + {!inFlight && s.not_published && ( +

+ {m.update_not_published({ channel: s.channel })} +

+ )} {!inFlight && s.last_error && (

{m.update_error()} {s.last_error}