From caafab85001a16709f1441a609b35df7d4b6febd Mon Sep 17 00:00:00 2001 From: bcbetterninja <327058824+bcbetterninja@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:25:35 +0000 Subject: [PATCH 1/7] fix: signal confirmed kiosk deletion to polling devices --- server/src/shared/db/migrations-pg.ts | 7 ++++ server/src/shared/db/repository.ts | 11 ++++++ server/src/shared/display-session.ts | 18 +++++++++- server/tests/kiosk-deletion.test.ts | 49 +++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 server/tests/kiosk-deletion.test.ts diff --git a/server/src/shared/db/migrations-pg.ts b/server/src/shared/db/migrations-pg.ts index 1c4b626..705245c 100644 --- a/server/src/shared/db/migrations-pg.ts +++ b/server/src/shared/db/migrations-pg.ts @@ -102,6 +102,13 @@ export const PUBLIC_MIGRATIONS: readonly string[] = [ claim_encrypted TEXT, acknowledged_at TIMESTAMPTZ )`, + // Keep only password hashes, so deleted devices can authenticate a reset signal. + `CREATE TABLE public.deleted_kiosk_keys ( + key_hash TEXT PRIMARY KEY, + key_prefix TEXT NOT NULL, + deleted_at TIMESTAMPTZ NOT NULL DEFAULT now() + )`, + `CREATE INDEX deleted_kiosk_keys_prefix ON public.deleted_kiosk_keys(key_prefix)`, ]; /** diff --git a/server/src/shared/db/repository.ts b/server/src/shared/db/repository.ts index f77748d..2d759e9 100644 --- a/server/src/shared/db/repository.ts +++ b/server/src/shared/db/repository.ts @@ -3076,6 +3076,12 @@ export class Repository { void this.notify("kiosks", "update", id); } + async listDeletedKioskKeysByPrefix(prefix: string): Promise> { + return await this._all( + "SELECT key_hash FROM public.deleted_kiosk_keys WHERE key_prefix = ?", [prefix], + ) as Array<{ key_hash: string }>; + } + async deleteKiosk(id: string): Promise { const displays = await this.listDisplaysForKiosk(id); await this.transact(async () => { @@ -3085,6 +3091,11 @@ export class Repository { await this._run(`DELETE FROM displays WHERE kiosk_id = ?`, [id]); await this._run(`DELETE FROM kiosk_labels WHERE kiosk_id = ?`, [id]); await this._run(`DELETE FROM kiosk_gpio_bindings WHERE kiosk_id = ?`, [id]); + await this._run( + `INSERT INTO public.deleted_kiosk_keys (key_hash, key_prefix) + SELECT key_hash, key_prefix FROM kiosks WHERE id = ? + ON CONFLICT (key_hash) DO NOTHING`, [id], + ); await this._run(`DELETE FROM kiosks WHERE id = ?`, [id]); }); for (const display of displays) { diff --git a/server/src/shared/display-session.ts b/server/src/shared/display-session.ts index 71bc047..7ced538 100644 --- a/server/src/shared/display-session.ts +++ b/server/src/shared/display-session.ts @@ -69,7 +69,23 @@ export function registerViewerDeviceAuth(app: H3, repo: Repository, auth: AuthAp } const key = bearer ?? rawKeyCookie; const verified = key ? await auth.verifyKioskKey(key) : null; - if (!verified) return new Response(null, { status: 401 }); + if (!verified) { + // Existing Linux clients expect a successful deletion envelope, followed by + // an independent 401 from _check. Never turn an unknown key into a reset. + if (bearer && bearer.length >= 8 + && ((path === "/api/kiosk/bundle" && event.req.method === "GET") + || (path === "/api/kiosk/heartbeat" && event.req.method === "POST"))) { + const deleted = await repo.listDeletedKioskKeysByPrefix(bearer.slice(0, 8)); + for (const candidate of deleted) { + if (await auth.verifyPassword(bearer, candidate.key_hash)) { + return Response.json({ bf_kiosk_deleted: true }, { + headers: { "cache-control": "no-store" }, + }); + } + } + } + return new Response(null, { status: 401 }); + } return repo.adapter.withSearchPath(verified.schema_name, async () => { const kiosk = await repo.getKioskById(verified.id); if (!kiosk?.enabled) return new Response(null, { status: 401 }); diff --git a/server/tests/kiosk-deletion.test.ts b/server/tests/kiosk-deletion.test.ts new file mode 100644 index 0000000..98a4087 --- /dev/null +++ b/server/tests/kiosk-deletion.test.ts @@ -0,0 +1,49 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { H3 } from "h3"; +import argon2 from "argon2"; +import { registerViewerDeviceAuth } from "../src/shared/display-session.js"; + +// Exercise the wire contract used by already-installed Linux clients: a 200 +// deletion marker on polling, then a separate 401 confirmation from _check. +test("deleted kiosk polling resets only after matching the deleted password hash", async () => { + const deletedKey = "sameprefix-deleted-secret"; + const hash = await argon2.hash(deletedKey); + const app = new H3(); + let lookups = 0; + registerViewerDeviceAuth(app, { + listDeletedKioskKeysByPrefix: async (prefix: string) => { + lookups++; + return prefix === deletedKey.slice(0, 8) ? [{ key_hash: hash }] : []; + }, + adapter: { withSearchPath: async (_schema: string, fn: () => unknown) => fn() }, + getKioskById: async () => ({ id: "active", enabled: true }), + } as never, { + verifyKioskKey: async (key: string) => key === "active-device-key" + ? { id: "active", schema_name: "public" } : null, + verifyPassword: (key: string, stored: string) => argon2.verify(stored, key), + } as never, {} as never); + app.get("/api/kiosk/bundle", () => ({ active: true })); + const request = (path: string, key?: string, method = "GET", cookie = false) => app.request(`http://bf.test/api/kiosk/${path}`, { + method, + headers: key ? cookie ? { cookie: `betterframe_kiosk_key=${key}` } : { authorization: `Bearer ${key}` } : {}, + }); + for (const [path, method] of [["bundle", "GET"], ["heartbeat", "POST"]]) { + const response = await request(path!, deletedKey, method); + assert.equal(response.status, 200); + assert.equal(response.headers.get("cache-control"), "no-store"); + assert.deepEqual(await response.json(), { bf_kiosk_deleted: true }); + assert.equal((await request(path!, "sameprefix-wrong-secret", method)).status, 401); + assert.equal((await request(path!, "unknown-device-key", method)).status, 401); + assert.equal((await request(path!, undefined, method)).status, 401); + assert.equal((await request(path!, deletedKey, method, true)).status, 401); + } + const beforeCheck = lookups; + assert.equal((await request("_check", deletedKey)).status, 401); + assert.equal((await request("firmware/check", deletedKey)).status, 401); + assert.equal(lookups, beforeCheck); + const active = await request("bundle", "active-device-key"); + assert.equal(active.status, 200); + assert.deepEqual(await active.json(), { active: true }); + assert.equal(lookups, beforeCheck); +}); From 697ddf9c672abbe2a979d98c7e0e99c919f47167 Mon Sep 17 00:00:00 2001 From: bcbetterninja <327058824+bcbetterninja@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:44:44 +0000 Subject: [PATCH 2/7] feat: recover kiosk updates without authentication using saved windows --- client/core/src/lib.rs | 2 + client/core/src/update_policy.rs | 149 +++++++++++ client/src/main.rs | 3 + client/src/platform/linux/firmware.rs | 20 +- client/src/platform/linux/os_update.rs | 24 +- client/src/platform/linux/server.rs | 9 + client/src/platform/linux/ui.rs | 73 ++++- client/src/platform/linux/update_recovery.rs | 263 +++++++++++++++++++ docs/update-recovery.md | 11 + server/src/plugins/service-api-http/index.ts | 18 +- server/src/shared/public-update-selection.ts | 15 ++ server/tests/public-update-selection.test.ts | 27 ++ 12 files changed, 582 insertions(+), 32 deletions(-) create mode 100644 client/core/src/update_policy.rs create mode 100644 client/src/platform/linux/update_recovery.rs create mode 100644 docs/update-recovery.md create mode 100644 server/src/shared/public-update-selection.ts create mode 100644 server/tests/public-update-selection.test.ts diff --git a/client/core/src/lib.rs b/client/core/src/lib.rs index 3a2a3fb..9637c12 100644 --- a/client/core/src/lib.rs +++ b/client/core/src/lib.rs @@ -6,3 +6,5 @@ pub mod layout; pub mod protocol; pub mod state; pub mod version; + +pub mod update_policy; diff --git a/client/core/src/update_policy.rs b/client/core/src/update_policy.rs new file mode 100644 index 0000000..f3b1ce5 --- /dev/null +++ b/client/core/src/update_policy.rs @@ -0,0 +1,149 @@ +//! Durable, non-secret update preferences; independent of enrollment state. +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct Window { + pub day: u8, + pub start: String, + pub end: String, +} + +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct Schedule { + pub mode: String, + pub windows: Vec, + pub timezone: String, +} + +impl Schedule { + pub fn allows(&self, day: u8, minute: u16) -> bool { + if self.mode == "always" { + return true; + } + if self.mode != "windows" || day > 6 || minute >= 1440 { + return false; + } + self.windows.iter().any(|window| { + let (Some(start), Some(end)) = (minutes(&window.start), minutes(&window.end)) else { + return false; + }; + if window.day > 6 || start == end { + return false; + } + if start < end { + window.day == day && minute >= start && minute < end + } else { + (window.day == day && minute >= start) + || ((window.day + 1) % 7 == day && minute < end) + } + }) + } +} + +fn minutes(value: &str) -> Option { + let bytes = value.as_bytes(); + if bytes.len() != 5 || bytes[2] != b':' { + return None; + } + if ![bytes[0], bytes[1], bytes[3], bytes[4]] + .iter() + .all(u8::is_ascii_digit) + { + return None; + } + let hour = (bytes[0] - b'0') as u16 * 10 + (bytes[1] - b'0') as u16; + let minute = (bytes[3] - b'0') as u16 * 10 + (bytes[4] - b'0') as u16; + (hour < 24 && minute < 60).then_some(hour * 60 + minute) +} + +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct Policy { + pub server: String, + pub schedule: Schedule, + pub firmware_channel: String, + pub firmware_target_version: Option, + pub os_update_channel: String, + pub os_update_target_version: Option, +} + +impl Policy { + pub fn selection(&self, os: bool) -> Vec<(String, String)> { + let (channel, version) = if os { + (&self.os_update_channel, &self.os_update_target_version) + } else { + (&self.firmware_channel, &self.firmware_target_version) + }; + let mut query = vec![("channel".into(), channel.clone())]; + if let Some(version) = version { + query.push(("version".into(), version.clone())); + } + query + } +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn recurring_windows_include_start_exclude_end_and_wrap_week() { + let schedule = Schedule { + mode: "windows".into(), + timezone: "UTC".into(), + windows: vec![ + Window { + day: 6, + start: "23:00".into(), + end: "01:00".into(), + }, + Window { + day: 2, + start: "02:00".into(), + end: "03:00".into(), + }, + ], + }; + assert!(schedule.allows(6, 1380)); + assert!(schedule.allows(0, 59)); + assert!(!schedule.allows(0, 60)); + assert!(!schedule.allows(6, 1379)); + assert!(schedule.allows(2, 120)); + assert!(!schedule.allows(2, 180)); + assert!(!schedule.allows(3, 120)); + } + #[test] + fn malformed_or_empty_windows_do_not_allow_updates() { + for start in ["25:00", "1:00", "aa:bb", "01:00"] { + let schedule = Schedule { + mode: "windows".into(), + timezone: "UTC".into(), + windows: vec![Window { + day: 0, + start: start.into(), + end: "01:00".into(), + }], + }; + assert!(!schedule.allows(0, 30)); + } + } + #[test] + fn preferences_and_pins_survive_restart() { + let policy: Policy = serde_json::from_value(serde_json::json!({ + "server": "https://frame.example", "schedule": {"mode":"always", "windows":[], "timezone":"UTC"}, + "firmware_channel":"beta", "firmware_target_version":"2.0.0", + "os_update_channel":"dev", "os_update_target_version":null + })).unwrap(); + let reloaded: Policy = + serde_json::from_slice(&serde_json::to_vec(&policy).unwrap()).unwrap(); + assert_eq!( + reloaded.selection(false), + vec![ + ("channel".into(), "beta".into()), + ("version".into(), "2.0.0".into()) + ] + ); + assert_eq!( + reloaded.selection(true), + vec![("channel".into(), "dev".into())] + ); + } +} diff --git a/client/src/main.rs b/client/src/main.rs index d729f48..7bf132b 100644 --- a/client/src/main.rs +++ b/client/src/main.rs @@ -64,6 +64,9 @@ mod ui; #[path = "platform/linux/update_guard.rs"] mod update_guard; #[cfg(target_os = "linux")] +#[path = "platform/linux/update_recovery.rs"] +mod update_recovery; +#[cfg(target_os = "linux")] #[path = "platform/linux/ws_client.rs"] mod ws_client; diff --git a/client/src/platform/linux/firmware.rs b/client/src/platform/linux/firmware.rs index 5919782..3f5c30e 100644 --- a/client/src/platform/linux/firmware.rs +++ b/client/src/platform/linux/firmware.rs @@ -99,6 +99,15 @@ pub struct UpdateInfo { /// Public pre-boot firmware check — no auth needed. Always checks stable /// channel. Used before pairing to self-update to latest binary. pub fn check_public(server: &str, current_version: &str) -> Option { + check_public_with_selection(server, current_version, &[]) +} + +pub fn check_recovery(server: &str, current_version: &str) -> Option { + let policy = crate::update_recovery::policy_for(server)?; + check_public_with_selection(server, current_version, &policy.selection(false)) +} + +fn check_public_with_selection(server: &str, current_version: &str, selection: &[(String, String)]) -> Option { let url = format!( "{server}/api/firmware/public/check?target={target}&arch={arch}¤t={cur}", target = FIRMWARE_TARGET, @@ -106,7 +115,7 @@ pub fn check_public(server: &str, current_version: &str) -> Option { cur = current_version, ); let client = crate::network::blocking_client(); - let resp = match client.get(&url).timeout(Duration::from_secs(10)).send() { + let resp = match client.get(&url).query(selection).timeout(Duration::from_secs(10)).send() { Ok(r) => r, Err(err) => { warn!("preboot firmware check: {err}"); @@ -203,19 +212,19 @@ pub fn check(server: &str, key: &str, current_version: &str) -> Option r, Err(err) => { warn!("firmware check: request failed: {err}"); - return None; + return check_recovery(server, current_version); } }; if !resp.status().is_success() { warn!("firmware check: HTTP {}", resp.status()); - return None; + return check_recovery(server, current_version); } match resp.json::() { Ok(c) => newer_update(c, current_version), Err(err) => { warn!("firmware check: parse failed: {err}"); - None + check_recovery(server, current_version) } } } @@ -237,11 +246,10 @@ pub fn apply( on_progress("Downloading", 0); // 1. Download - let url = format!("{}{}", server, info.download_url); + let url = format!("{}{}", server, info.download_url.replace("/api/kiosk/firmware/download/", "/api/firmware/public/download/")); let client = crate::network::blocking_client(); let resp = client .get(&url) - .header("Authorization", format!("Bearer {key}")) .timeout(Duration::from_secs(300)) .send() .map_err(|e| format!("download request: {e}"))?; diff --git a/client/src/platform/linux/os_update.rs b/client/src/platform/linux/os_update.rs index 58b9d3f..c98b73a 100644 --- a/client/src/platform/linux/os_update.rs +++ b/client/src/platform/linux/os_update.rs @@ -160,15 +160,20 @@ pub struct UpdateInfo { /// Public stable-channel check used before the kiosk has paired. pub fn check_public(server: &str) -> Option { - check_at(server, None, "/api/os/public/check") + check_at(server, None, "/api/os/public/check", &[]) } /// Authenticated check used after pairing. pub fn check(server: &str, key: &str) -> Option { - check_at(server, Some(key), "/api/kiosk/os/check") + check_at(server, Some(key), "/api/kiosk/os/check", &[]) } -fn check_at(server: &str, key: Option<&str>, path: &str) -> Option { +pub fn check_recovery(server: &str) -> Option { + let policy = crate::update_recovery::policy_for(server)?; + check_at(server, None, "/api/os/public/check", &policy.selection(true)) +} + +fn check_at(server: &str, key: Option<&str>, path: &str, selection: &[(String, String)]) -> Option { let compat = compatibility(); let cur = current_os_version(); let url = format!( @@ -177,7 +182,7 @@ fn check_at(server: &str, key: Option<&str>, path: &str) -> Option { cur = urlencoding::encode(&cur), ); let client = crate::network::blocking_client(); - let mut request = client.get(&url); + let mut request = client.get(&url).query(selection); if let Some(key) = key { request = request.header("Authorization", format!("Bearer {key}")); } @@ -185,19 +190,19 @@ fn check_at(server: &str, key: Option<&str>, path: &str) -> Option { Ok(r) => r, Err(err) => { warn!("os-update check: request failed: {err}"); - return None; + return if key.is_some() { check_recovery(server) } else { None }; } }; if !resp.status().is_success() { warn!("os-update check: HTTP {}", resp.status()); - return None; + return if key.is_some() { check_recovery(server) } else { None }; } match resp.json::() { Ok(c) => newer_update(c, &cur), Err(err) => { warn!("os-update check: parse failed: {err}"); - None + if key.is_some() { check_recovery(server) } else { None } } } } @@ -305,7 +310,7 @@ fn apply_inner( // Streams directly to disk (no 1.2GB in RAM). On network failure, // resumes from where it left off using Range header. Retries up to // 5 times with 10s backoff between attempts. - let url = format!("{}{}", server, info.download_url); + let url = format!("{}{}", server, info.download_url.replace("/api/kiosk/os/download/", "/api/os/public/download/")); on_progress("Preparing", 0); let staging_dir = PathBuf::from("/var/lib/betterframe/tmp"); fs::create_dir_all(&staging_dir).map_err(|e| format!("mkdir staging: {e}"))?; @@ -327,9 +332,6 @@ fn apply_inner( let client = crate::network::blocking_client(); let mut req = client.get(&url); - if let Some(key) = key { - req = req.header("Authorization", format!("Bearer {key}")); - } if existing_bytes > 0 { req = req.header("Range", format!("bytes={existing_bytes}-")); } diff --git a/client/src/platform/linux/server.rs b/client/src/platform/linux/server.rs index d5b5a2d..9eda7e2 100644 --- a/client/src/platform/linux/server.rs +++ b/client/src/platform/linux/server.rs @@ -1051,6 +1051,7 @@ pub fn heartbeat( "_check says key still valid, ignoring bf_kiosk_deleted from heartbeat" ); } + crate::update_recovery::record_heartbeat(server, &body); let fw = body.get("firmware_channel").and_then(|v| v.as_str()); let os = body.get("os_update_channel").and_then(|v| v.as_str()); let fw_target = body.get("firmware_target_version").map(|v| v.as_str()); @@ -1074,7 +1075,14 @@ pub fn heartbeat( .unwrap_or(false) } +pub fn update_policy_path() -> PathBuf { + state_dir().join("update-policy.json") +} + pub fn auto_updates_allowed() -> bool { + if update_policy_path().exists() { + return crate::update_recovery::allowed(); + } AUTO_UPDATES_ALLOWED.load(Ordering::SeqCst) } @@ -1134,6 +1142,7 @@ pub fn cancel_active_updates(reason: &str) { } pub fn clear_cached_update_preferences() { + crate::update_recovery::suspend(); CACHED_FIRMWARE_CHANNEL.lock().unwrap().take(); CACHED_FIRMWARE_TARGET_VERSION.lock().unwrap().take(); CACHED_OS_CHANNEL.lock().unwrap().take(); diff --git a/client/src/platform/linux/ui.rs b/client/src/platform/linux/ui.rs index f4599f9..6ef85c7 100644 --- a/client/src/platform/linux/ui.rs +++ b/client/src/platform/linux/ui.rs @@ -221,6 +221,17 @@ fn activate(app: &Application) { let (tx, rx) = mpsc::channel::(); + let recovery_tx = tx.clone(); + std::thread::spawn(move || loop { + std::thread::sleep(Duration::from_secs(120)); + if crate::update_recovery::needed() && crate::update_recovery::allowed() { + if let Some(policy) = crate::update_recovery::load() { + maybe_apply_os_update(&policy.server, "", &recovery_tx, false, true); + maybe_apply_firmware_update(&policy.server, "", &recovery_tx, false, true); + } + } + }); + let server_url = std::env::var("BETTERFRAME_SERVER") .ok() .or_else(|| std::env::args().nth(1)); @@ -251,13 +262,21 @@ fn activate(app: &Application) { // Bootstrap updates run before pairing so an older image can repair // its client before talking to a newer server. if !server::is_paired() { - if server::ota_enabled("BF_ENABLE_APP_OTA") { + if server::ota_enabled("BF_ENABLE_APP_OTA") && server::auto_updates_allowed() { let _ = tx.send(WorkerMsg::StartupStatus("Checking for app updates".into())); let current = crate::server::kiosk_app_version(); - if let Some(update) = crate::firmware::check_public(&server, current) { - info!("preboot update available: {} → {}", current, update.version); - if let Err(e) = crate::firmware::apply_public(&server, &update) { - tracing::warn!("preboot update failed: {e}"); + let update = if server::update_policy_path().exists() { + crate::firmware::check_recovery(&server, current) + } else { + crate::firmware::check_public(&server, current) + }; + if let Some(update) = update { + if server::auto_updates_allowed() && UPDATE_APPLY_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst).is_ok() { + info!("preboot update available: {} → {}", current, update.version); + if let Err(e) = crate::firmware::apply_public(&server, &update) { + tracing::warn!("preboot update failed: {e}"); + } + UPDATE_APPLY_ACTIVE.store(false, Ordering::SeqCst); } } } @@ -300,9 +319,15 @@ fn activate(app: &Application) { } std::thread::sleep(Duration::from_secs(2)); } - if server::ota_enabled("BF_ENABLE_OS_OTA") && os_update::boot_is_confirmed() { + if server::ota_enabled("BF_ENABLE_OS_OTA") && os_update::boot_is_confirmed() && server::auto_updates_allowed() { let _ = tx.send(WorkerMsg::StartupStatus("Checking for OS updates".into())); - if let Some(update) = os_update::check_public(&server) { + let update = if server::update_policy_path().exists() { + os_update::check_recovery(&server) + } else { + os_update::check_public(&server) + }; + if let Some(update) = update.filter(|_| server::auto_updates_allowed() + && UPDATE_APPLY_ACTIVE.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst).is_ok()) { let version = update.version.clone(); let tx_progress = tx.clone(); if let Err(e) = os_update::apply_public(&server, &update, move |phase, pct| { @@ -314,6 +339,7 @@ fn activate(app: &Application) { let _ = tx.send(WorkerMsg::UpdateProgress(None)); tracing::warn!("preboot OS update failed: {e}"); } + UPDATE_APPLY_ACTIVE.store(false, Ordering::SeqCst); } } let _ = tx.send(WorkerMsg::ShowPairingCode(session.code.clone())); @@ -522,6 +548,7 @@ fn activate(app: &Application) { &key_for_reload, &tx_for_reload, force, + false, ); } else { info!("firmware: outside configured update window"); @@ -534,6 +561,7 @@ fn activate(app: &Application) { &key_for_reload, &tx_for_reload, force, + false, ); } else { info!("os-update: outside configured update window"); @@ -576,8 +604,8 @@ fn activate(app: &Application) { confirmation_reported = os_update::report_confirmed(&server, &key); } if server::auto_updates_allowed() { - maybe_apply_os_update(&server, &key, &tx_progress, false); - maybe_apply_firmware_update(&server, &key, &tx_progress, false); + maybe_apply_os_update(&server, &key, &tx_progress, false, false); + maybe_apply_firmware_update(&server, &key, &tx_progress, false, false); } else { info!("auto-update: outside configured update window"); } @@ -984,6 +1012,7 @@ fn maybe_apply_os_update( kiosk_key: &str, tx: &mpsc::Sender, force: bool, + recovery: bool, ) { if !server::ota_enabled("BF_ENABLE_OS_OTA") { info!("os-update: disabled (BF_ENABLE_OS_OTA = 0)"); @@ -1007,7 +1036,12 @@ fn maybe_apply_os_update( let tx = tx.clone(); std::thread::spawn(move || { let _lock = OS_UPDATE_LOCK.lock().unwrap(); - let Some(info) = os_update::check(&server_url, &kiosk_key) else { + let update = if recovery { + os_update::check_recovery(&server_url) + } else { + os_update::check(&server_url, &kiosk_key) + }; + let Some(info) = update else { info!("os-update: no eligible update"); OS_UPDATE_ACTIVE.store(false, Ordering::SeqCst); return; @@ -1049,6 +1083,10 @@ fn maybe_apply_os_update( "size_bytes": info.size_bytes, }), ); + if !force && !server::auto_updates_allowed() { + OS_UPDATE_ACTIVE.store(false, Ordering::SeqCst); + return; + } if UPDATE_APPLY_ACTIVE .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) .is_err() @@ -1100,6 +1138,7 @@ fn maybe_apply_firmware_update( kiosk_key: &str, tx: &mpsc::Sender, force: bool, + recovery: bool, ) { if !server::ota_enabled("BF_ENABLE_APP_OTA") { info!("firmware: disabled (BF_ENABLE_APP_OTA = 0)"); @@ -1117,7 +1156,7 @@ fn maybe_apply_firmware_update( let kiosk_key = kiosk_key.to_string(); let tx = tx.clone(); std::thread::spawn(move || { - run_firmware_update_worker(server_url, kiosk_key, tx, force); + run_firmware_update_worker(server_url, kiosk_key, tx, force, recovery); }); } @@ -1126,10 +1165,16 @@ fn run_firmware_update_worker( kiosk_key: String, tx: mpsc::Sender, force: bool, + recovery: bool, ) { let _lock = FIRMWARE_LOCK.lock().unwrap(); let current = option_env!("BF_BUILD_VERSION").unwrap_or(env!("CARGO_PKG_VERSION")); - let Some(info) = firmware::check(&server_url, &kiosk_key, current) else { + let update = if recovery { + firmware::check_recovery(&server_url, current) + } else { + firmware::check(&server_url, &kiosk_key, current) + }; + let Some(info) = update else { info!("firmware: no eligible update"); FIRMWARE_ACTIVE.store(false, Ordering::SeqCst); return; @@ -1172,6 +1217,10 @@ fn run_firmware_update_worker( "release_id": &info.release_id, }), ); + if !force && !server::auto_updates_allowed() { + FIRMWARE_ACTIVE.store(false, Ordering::SeqCst); + return; + } if UPDATE_APPLY_ACTIVE .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) .is_err() diff --git a/client/src/platform/linux/update_recovery.rs b/client/src/platform/linux/update_recovery.rs new file mode 100644 index 0000000..b09ad4b --- /dev/null +++ b/client/src/platform/linux/update_recovery.rs @@ -0,0 +1,263 @@ +//! Update recovery must not depend on bundle loading, WebSocket, or kiosk auth. +use crate::core::update_policy::{Policy, Schedule}; +use std::{ + fs, + path::Path, + process::Command, + sync::Mutex, + time::{Duration, Instant}, +}; + +static LAST_HEARTBEAT: Mutex> = Mutex::new(None); +static POLICY_WRITE: Mutex<()> = Mutex::new(()); + +pub fn load() -> Option { + let bytes = fs::read(crate::server::update_policy_path()).ok()?; + serde_json::from_slice(&bytes).ok() +} + +/// Never use another server's saved preferences or a policy invalidated by an +/// admin change whose replacement has not arrived yet. +pub fn policy_for(server: &str) -> Option { + if crate::server::demo_mode() + || crate::server::update_policy_path() + .with_extension("suspended") + .exists() + { + return None; + } + load().filter(|policy| policy.server == server) +} + +pub fn record_heartbeat(server: &str, body: &serde_json::Value) { + // A successful HTTP response with an invalid body is not a healthy control plane. + if body.get("ok").and_then(|v| v.as_bool()) != Some(true) { + return; + } + *LAST_HEARTBEAT.lock().unwrap() = Some(Instant::now()); + let Some(schedule) = body.get("update_schedule") else { + return; + }; + let Ok(schedule) = serde_json::from_value::(schedule.clone()) else { + return; + }; + let policy = Policy { + server: server.to_owned(), + schedule, + firmware_channel: body["firmware_channel"].as_str().unwrap_or("stable").into(), + firmware_target_version: body["firmware_target_version"].as_str().map(str::to_owned), + os_update_channel: body["os_update_channel"] + .as_str() + .unwrap_or("stable") + .into(), + os_update_target_version: body["os_update_target_version"].as_str().map(str::to_owned), + }; + let _lock = POLICY_WRITE.lock().unwrap(); + let path = crate::server::update_policy_path(); + let result = save(&path, &policy); + if let Err(error) = result { + tracing::warn!("update policy could not be saved: {error}"); + } else { + let _ = fs::remove_file(path.with_extension("suspended")); + } +} + +fn save(path: &Path, policy: &Policy) -> Result<(), String> { + let bytes = serde_json::to_vec(policy).map_err(|e| e.to_string())?; + // Avoid rewriting flash every heartbeat. Replace atomically on policy changes. + if fs::read(path).ok().as_deref() == Some(bytes.as_slice()) { + return Ok(()); + } + let pending = path.with_extension("pending"); + use std::io::Write; + let mut file = fs::File::create(&pending).map_err(|e| e.to_string())?; + file.write_all(&bytes) + .and_then(|_| file.sync_all()) + .map_err(|e| e.to_string())?; + fs::rename(pending, path).map_err(|e| e.to_string())?; + if let Some(parent) = path.parent() { + fs::File::open(parent) + .and_then(|dir| dir.sync_all()) + .map_err(|e| e.to_string())?; + } + Ok(()) +} + +pub fn suspend() { + let _lock = POLICY_WRITE.lock().unwrap(); + if let Err(error) = fs::write( + crate::server::update_policy_path().with_extension("suspended"), + b"awaiting updated policy", + ) { + tracing::warn!("could not suspend saved update policy: {error}"); + } +} + +pub fn needed() -> bool { + LAST_HEARTBEAT + .lock() + .unwrap() + .is_none_or(|last| last.elapsed() >= Duration::from_secs(120)) +} + +pub fn allowed() -> bool { + if crate::server::demo_mode() + || crate::server::update_policy_path() + .with_extension("suspended") + .exists() + { + return false; + } + load().is_some_and(|policy| schedule_allows(&policy.schedule)) +} + +pub fn schedule_allows(schedule: &Schedule) -> bool { + if schedule.mode == "always" { + return true; + } + // Evaluate in the server's IANA timezone, not the Pi's display timezone. + // System zoneinfo handles DST without freezing a UTC offset at last contact. + let zone = &schedule.timezone; + if zone.is_empty() + || zone.starts_with('/') + || zone.split('/').any(|part| part == "..") + || !Path::new("/usr/share/zoneinfo").join(zone).is_file() + { + return false; + } + let Ok(output) = Command::new("date") + .env("TZ", zone) + .arg("+%w %H %M") + .output() + else { + return false; + }; + if !output.status.success() { + return false; + } + let values: Vec = String::from_utf8_lossy(&output.stdout) + .split_whitespace() + .filter_map(|v| v.parse().ok()) + .collect(); + values.len() == 3 + && values[0] <= 6 + && values[1] < 24 + && values[2] < 60 + && schedule.allows(values[0] as u8, values[1] * 60 + values[2]) +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn policy_is_atomic_and_readable_after_restart() { + let dir = std::env::temp_dir().join(format!("bf-update-policy-{}", std::process::id())); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("policy.json"); + let policy = Policy { + server: "https://frame.example".into(), + schedule: Schedule { + mode: "always".into(), + windows: vec![], + timezone: "UTC".into(), + }, + firmware_channel: "beta".into(), + firmware_target_version: Some("2.0.0".into()), + os_update_channel: "dev".into(), + os_update_target_version: None, + }; + save(&path, &policy).unwrap(); + let reloaded: Policy = serde_json::from_slice(&fs::read(&path).unwrap()).unwrap(); + assert_eq!(reloaded.selection(false), policy.selection(false)); + assert!(!path.with_extension("pending").exists()); + fs::remove_dir_all(dir).unwrap(); + } + #[test] + fn rejected_auth_recovers_over_public_http_with_saved_preferences() { + use std::io::{Read, Write}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let server = format!("http://{}", listener.local_addr().unwrap()); + let handler = std::thread::spawn(move || { + let cases = [ + ("/api/kiosk/firmware/check?", true, "401 Unauthorized", "{}"), + ( + "/api/firmware/public/check?", + false, + "200 OK", + r#"{"up_to_date":false,"update":{"release_id":"app","version":"2.0.0","channel":"beta","sha256":"test","signature":"test","size_bytes":1,"download_url":"/api/firmware/public/download/app"}}"#, + ), + ("/api/firmware/public/download/app", false, "200 OK", "x"), + ("/api/kiosk/os/check?", true, "503 Unavailable", "{}"), + ( + "/api/os/public/check?", + false, + "200 OK", + r#"{"up_to_date":true}"#, + ), + ( + "/api/kiosk/firmware/check?", + true, + "200 OK", + r#"{"up_to_date":true}"#, + ), + ]; + for (path, authenticated, status, body) in cases { + let (mut stream, _) = listener.accept().unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(5))) + .unwrap(); + let mut request = Vec::new(); + let mut buffer = [0u8; 1024]; + while !request.windows(4).any(|v| v == b"\r\n\r\n") { + let read = stream.read(&mut buffer).unwrap(); + assert!(read > 0); + request.extend_from_slice(&buffer[..read]); + } + let request = String::from_utf8(request).unwrap().to_lowercase(); + assert!(request.starts_with(&format!("get {path}")), "{request}"); + assert_eq!(request.contains("authorization:"), authenticated); + if !authenticated && path.ends_with('?') { + assert!(request.contains("channel=beta")); + assert!(request.contains("version=2.0.0")); + } + write!(stream, "HTTP/1.1 {status}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len()).unwrap(); + } + }); + let body = serde_json::json!({"ok":true, + "update_schedule":{"mode":"always","windows":[],"timezone":"UTC"}, + "firmware_channel":"beta","firmware_target_version":"2.0.0", + "os_update_channel":"beta","os_update_target_version":"2.0.0"}); + record_heartbeat(&server, &body); + assert!(allowed()); + assert!(!needed()); + let update = crate::firmware::check(&server, "deleted-key", "1.0.0").unwrap(); + assert_eq!(update.version, "2.0.0"); + // Download remains public even if selection came from an old authenticated + // endpoint. Invalid hash must abort before touching installed files. + let mut update = update; + update.download_url = "/api/kiosk/firmware/download/app".into(); + let error = crate::firmware::apply(&server, "deleted-key", &update, |_, _| {}).unwrap_err(); + assert!(error.contains("sha256 mismatch"), "{error}"); + assert!(crate::os_update::check(&server, "deleted-key").is_none()); + // A valid "up to date" is authoritative and must not trigger fallback. + assert!(crate::firmware::check(&server, "active-key", "2.0.0").is_none()); + handler.join().unwrap(); + assert!(policy_for("https://another.example").is_none()); + *LAST_HEARTBEAT.lock().unwrap() = Some(Instant::now() - Duration::from_secs(121)); + assert!(needed()); + suspend(); + assert!(!allowed()); + assert!(policy_for(&server).is_none()); + record_heartbeat(&server, &body); + assert!(allowed()); + let _ = fs::remove_file(crate::server::update_policy_path()); + } + #[test] + fn unknown_timezone_cannot_open_a_window() { + assert!(!schedule_allows(&Schedule { + mode: "windows".into(), + windows: vec![], + timezone: "../etc/passwd".into() + })); + } +} diff --git a/docs/update-recovery.md b/docs/update-recovery.md new file mode 100644 index 0000000..50d8642 --- /dev/null +++ b/docs/update-recovery.md @@ -0,0 +1,11 @@ +# Linux kiosk update recovery + +The server sends `update_schedule` (weekly windows plus the server's IANA timezone), app/OS channels, and version pins in successful heartbeats. Linux saves these non-secret preferences atomically in `update-policy.json` alongside its state. It evaluates the recurring schedule locally using system zoneinfo, including daylight-saving transitions. The policy survives reboot and pairing reset. Demo mode and the app/OS OTA enable flags continue to apply. + +Normal checks retain authenticated selection for per-device rollouts and explicit admin pushes. A rejected, failing, or malformed check falls back to public release selection using the saved channel and pin. A valid up-to-date response does not trigger fallback. Public selection does not fall through a missing or yanked pin to another version. Artifact downloads use public endpoints even when authenticated selection succeeded, so losing authentication between selection and download does not block installation. + +A separate worker starts alongside the enrollment worker. After two minutes without a valid heartbeat, it checks public releases every two minutes during the locally saved window, even if enrollment, bundle loading, or the control connection is stuck. No valid saved policy means no independent recovery updates: deploy the server first and let the updated client receive at least one successful heartbeat. A corrupt policy or unavailable timezone does not open a maintenance window. A received policy-change cancellation suspends cached recovery until replacement preferences arrive. + +Explicit admin pushes bypass the time window and retain existing retry overrides. A new per-device push still needs a functioning control connection; during an authentication outage, publishing a newer release on the saved channel supplies the recovery update (unless the device is pinned). Recovery downloads keep signature verification, architecture/OS compatibility selection, upgrade-only version checks, failed-version attempt limits, and rollback/boot-confirmation protections. Authenticated telemetry is best effort and is not a condition for installation. + +This requires both the server change and an updated Linux client. It cannot repair an already-inaccessible old binary solely by publishing this change. GTK, GStreamer, and WebKit development libraries are needed for the full client build/tests. Pure schedule tests run with `cargo test -p betterframe-client-core` from `client`; updater HTTP, persistence, and rollback tests live in the Linux modules. diff --git a/server/src/plugins/service-api-http/index.ts b/server/src/plugins/service-api-http/index.ts index 631fc22..68aeb97 100644 --- a/server/src/plugins/service-api-http/index.ts +++ b/server/src/plugins/service-api-http/index.ts @@ -1,3 +1,4 @@ +import { selectPublicUpdate } from "../../shared/public-update-selection.js"; import { effectiveFirmwareChannel } from "../../shared/kiosk-channels.js"; import { parseKioskLogs } from "../../shared/kiosk-logs.js"; import { reconcileOsUpdateReport } from "../../shared/os-update-status.js"; @@ -535,7 +536,7 @@ function registerPairingRoutes( }); // Public firmware check — no auth. Used by kiosks on first boot before - // pairing to self-update to latest stable binary. Always stable channel. + // pairing or during auth outages. Defaults to stable; recovery uses saved preferences. app.get("/api/firmware/public/check", async (event) => { const url = new URL(event.req.url); const target = normalizeFirmwareTarget( @@ -544,7 +545,10 @@ function registerPairingRoutes( if (!target) throw createError({ statusCode: 400, statusMessage: "target required" }); const current = url.searchParams.get("current")?.trim() ?? ""; - const release = await withDefaultTenant(repo, null, () => repo.getLatestFirmwareRelease("stable", target)); + const release = await withDefaultTenant(repo, null, () => selectPublicUpdate(url.searchParams, + (channel) => repo.getLatestFirmwareRelease(channel, target), + (version) => repo.getFirmwareReleaseByVersionArch(version, target), + )); if (!release || !isVersionUpgrade(release.version, current)) { return { up_to_date: true }; } @@ -554,6 +558,7 @@ function registerPairingRoutes( update: { release_id: release.id, version: release.version, + channel: release.channel, sha256: release.sha256, signature: release.signature, size_bytes: release.size_bytes, @@ -599,7 +604,10 @@ function registerPairingRoutes( if (!compatibility) throw createError({ statusCode: 400, statusMessage: "compatibility required" }); const current = url.searchParams.get("current")?.trim() ?? ""; const release = await withDefaultTenant(repo, null, () => - repo.getLatestOsUpdateRelease("stable", compatibility) + selectPublicUpdate(url.searchParams, + (channel) => repo.getLatestOsUpdateRelease(channel, compatibility), + (version) => repo.getOsUpdateReleaseByVersionCompatibility(version, compatibility), + ) ); if (!release || !isVersionUpgrade(release.version, current)) return { up_to_date: true }; return { @@ -1316,6 +1324,10 @@ export function registerKioskRoutes( os_update_channel: fresh?.os_update_channel ?? "stable", os_update_target_version: fresh?.os_update_target_version ?? null, auto_updates_allowed: updateScheduleAllowsNow(updateSchedule), + update_schedule: { + ...updateSchedule, + timezone: Intl.DateTimeFormat().resolvedOptions().timeZone, + }, audio_default_volume_percent: fresh?.audio_default_volume_percent ?? 50, ...(pendingConfig ? { pending_config: pendingConfig } : {}), }; diff --git a/server/src/shared/public-update-selection.ts b/server/src/shared/public-update-selection.ts new file mode 100644 index 0000000..e1d2502 --- /dev/null +++ b/server/src/shared/public-update-selection.ts @@ -0,0 +1,15 @@ +import type { FirmwareChannel } from "./types.js"; + +/** Public release selection contains no tenant or device credentials. A saved + * pin is authoritative: a missing/yanked pin must not fall through to latest. */ +export async function selectPublicUpdate( + query: URLSearchParams, + latest: (channel: FirmwareChannel) => Promise, + pinned: (version: string) => Promise, +): Promise { + const channel = query.get("channel") ?? "stable"; + if (!["stable", "beta", "dev"].includes(channel)) return null; + const version = query.get("version"); + const release = version ? await pinned(version) : await latest(channel as FirmwareChannel); + return release && !release.yanked_at ? release : null; +} diff --git a/server/tests/public-update-selection.test.ts b/server/tests/public-update-selection.test.ts new file mode 100644 index 0000000..360be4d --- /dev/null +++ b/server/tests/public-update-selection.test.ts @@ -0,0 +1,27 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { selectPublicUpdate } from "../src/shared/public-update-selection.js"; +import { updateScheduleAllowsNow } from "../src/shared/update-schedule.js"; + +test("public recovery honors saved channels and pins without falling through missing or yanked pins", async () => { + const calls: string[] = []; + const latest = async (channel: string) => { calls.push(channel); return { version: "3", yanked_at: null }; }; + const pinned = async (version: string) => version === "missing" ? null : { version, yanked_at: version === "yanked" ? "now" : null }; + assert.equal((await selectPublicUpdate(new URLSearchParams(), latest, pinned))?.version, "3"); + assert.deepEqual(calls, ["stable"]); + await selectPublicUpdate(new URLSearchParams("channel=dev"), latest, pinned); + assert.deepEqual(calls, ["stable", "dev"]); + assert.equal((await selectPublicUpdate(new URLSearchParams("channel=beta&version=2"), latest, pinned))?.version, "2"); + assert.equal(await selectPublicUpdate(new URLSearchParams("version=missing"), latest, pinned), null); + assert.equal(await selectPublicUpdate(new URLSearchParams("version=yanked"), latest, pinned), null); + assert.equal(await selectPublicUpdate(new URLSearchParams("channel=invalid"), latest, pinned), null); + assert.deepEqual(calls, ["stable", "dev"]); +}); + +test("saved client schedule uses the same overnight and end-exclusive boundaries as the server", () => { + const schedule = { mode: "windows" as const, windows: [{ day: 6, start: "23:00", end: "01:00" }] }; + // Local constructors deliberately match the server's local schedule timezone. + assert.equal(updateScheduleAllowsNow(schedule, new Date(2026, 8, 26, 23, 0)), true); + assert.equal(updateScheduleAllowsNow(schedule, new Date(2026, 8, 27, 0, 59)), true); + assert.equal(updateScheduleAllowsNow(schedule, new Date(2026, 8, 27, 1, 0)), false); +}); From d32c1544d8d75b9626752289d4f364c19b27c0cd Mon Sep 17 00:00:00 2001 From: bcbetterninja <327058824+bcbetterninja@users.noreply.github.com> Date: Thu, 24 Sep 2026 20:03:35 +0000 Subject: [PATCH 3/7] test: wait for kiosk window focus before remote menu input --- .../java/cloud/betterportal/frame/KioskPresentationTest.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/client/android/app/src/androidTest/java/cloud/betterportal/frame/KioskPresentationTest.kt b/client/android/app/src/androidTest/java/cloud/betterportal/frame/KioskPresentationTest.kt index e36cd55..2150096 100644 --- a/client/android/app/src/androidTest/java/cloud/betterportal/frame/KioskPresentationTest.kt +++ b/client/android/app/src/androidTest/java/cloud/betterportal/frame/KioskPresentationTest.kt @@ -108,8 +108,11 @@ class KioskPresentationTest { @Test fun remoteMenuOpensSettingsWithoutPuttingAnAddressFieldOnTheKiosk() { launch().use { scenario -> - awaitUi(scenario, "Kiosk menu did not appear") { activity -> - descendants(activity.window.decorView).any { it.contentDescription == "Kiosk menu" && it.isShown } + // A resumed activity can draw before its window receives input focus. + // Android drops injected keys in that interval (seen on API 29 CI). + awaitUi(scenario, "Kiosk menu window did not become ready for remote input") { activity -> + activity.hasWindowFocus() && + descendants(activity.window.decorView).any { it.contentDescription == "Kiosk menu" && it.isShown } } instrumentation.sendKeyDownUpSync(KeyEvent.KEYCODE_MENU) awaitAccessibility("Remote Menu did not expose Settings") { root -> root.findAccessibilityNodeInfosByText("Settings").isNotEmpty() } From 2abbd87bc0a3355b7218d6fb915bbdca199b348f Mon Sep 17 00:00:00 2001 From: bcbetterninja <327058824+bcbetterninja@users.noreply.github.com> Date: Thu, 24 Sep 2026 20:32:32 +0000 Subject: [PATCH 4/7] fix: preserve fleet download routes and defer rate-limited updates --- client/src/main.rs | 3 + client/src/platform/linux/firmware.rs | 15 +- client/src/platform/linux/os_update.rs | 23 ++- client/src/platform/linux/ui.rs | 7 + client/src/platform/linux/update_download.rs | 169 +++++++++++++++++++ client/src/platform/linux/update_guard.rs | 31 ++++ client/src/platform/linux/update_recovery.rs | 2 + docs/update-recovery.md | 2 +- 8 files changed, 226 insertions(+), 26 deletions(-) create mode 100644 client/src/platform/linux/update_download.rs diff --git a/client/src/main.rs b/client/src/main.rs index 7bf132b..4616ff6 100644 --- a/client/src/main.rs +++ b/client/src/main.rs @@ -64,6 +64,9 @@ mod ui; #[path = "platform/linux/update_guard.rs"] mod update_guard; #[cfg(target_os = "linux")] +#[path = "platform/linux/update_download.rs"] +mod update_download; +#[cfg(target_os = "linux")] #[path = "platform/linux/update_recovery.rs"] mod update_recovery; #[cfg(target_os = "linux")] diff --git a/client/src/platform/linux/firmware.rs b/client/src/platform/linux/firmware.rs index 3f5c30e..4a91ed3 100644 --- a/client/src/platform/linux/firmware.rs +++ b/client/src/platform/linux/firmware.rs @@ -138,13 +138,7 @@ pub fn apply_public(server: &str, info: &UpdateInfo) -> Result<(), String> { "preboot firmware: applying {} ({} bytes)", info.version, info.size_bytes ); - let download_url = format!("{server}{}", info.download_url); - let client = crate::network::blocking_client(); - let resp = client - .get(&download_url) - .timeout(Duration::from_secs(300)) - .send() - .map_err(|e| format!("download failed: {e}"))?; + let resp = crate::update_download::get(server, None, &info.download_url, 0)?; if !resp.status().is_success() { return Err(format!("download HTTP {}", resp.status())); } @@ -246,13 +240,8 @@ pub fn apply( on_progress("Downloading", 0); // 1. Download - let url = format!("{}{}", server, info.download_url.replace("/api/kiosk/firmware/download/", "/api/firmware/public/download/")); let client = crate::network::blocking_client(); - let resp = client - .get(&url) - .timeout(Duration::from_secs(300)) - .send() - .map_err(|e| format!("download request: {e}"))?; + let resp = crate::update_download::get(server, Some(key), &info.download_url, 0)?; if !resp.status().is_success() { return Err(format!("download HTTP {}", resp.status())); diff --git a/client/src/platform/linux/os_update.rs b/client/src/platform/linux/os_update.rs index 511a2ca..c2b1b58 100644 --- a/client/src/platform/linux/os_update.rs +++ b/client/src/platform/linux/os_update.rs @@ -293,6 +293,11 @@ fn apply_tracked( crate::update_guard::record_attempt("os", &info.version)?; } let result = apply_inner(server, key, info, on_progress); + if result.as_ref().is_err_and(|error| crate::update_download::is_deferred(error)) { + // No installation was attempted. Refund only this reservation, retaining + // any previous genuine failures for the same version. + crate::update_guard::refund_attempt("os", &info.version)?; + } if let Err(ref error) = result { let _lock = JOURNAL_LOCK.lock().map_err(|_| "OS update record locked")?; let path = std::path::Path::new(JOURNAL_PATH); @@ -326,7 +331,6 @@ fn apply_inner( // Streams directly to disk (no 1.2GB in RAM). On network failure, // resumes from where it left off using Range header. Retries up to // 5 times with 10s backoff between attempts. - let url = format!("{}{}", server, info.download_url.replace("/api/kiosk/os/download/", "/api/os/public/download/")); on_progress("Preparing", 0); let staging_dir = PathBuf::from("/var/lib/betterframe/tmp"); fs::create_dir_all(&staging_dir).map_err(|e| format!("mkdir staging: {e}"))?; @@ -346,21 +350,16 @@ fn apply_inner( info.size_bytes ); - let client = crate::network::blocking_client(); - let mut req = client.get(&url); - if existing_bytes > 0 { - req = req.header("Range", format!("bytes={existing_bytes}-")); - } - - let resp = match req.timeout(Duration::from_secs(300)).send() { - Ok(r) => r, - Err(e) => { - warn!("os-update: download request failed (attempt {attempt}): {e}"); + let resp = match crate::update_download::get(server, key, &info.download_url, existing_bytes) { + Ok(response) => response, + Err(error) => { + if crate::update_download::is_deferred(&error) { return Err(error); } + warn!("os-update: download request failed (attempt {attempt}): {error}"); if attempt < max_retries { std::thread::sleep(Duration::from_secs(10)); continue; } - return Err(format!("download failed after {max_retries} attempts: {e}")); + return Err(format!("download failed after {max_retries} attempts: {error}")); } }; diff --git a/client/src/platform/linux/ui.rs b/client/src/platform/linux/ui.rs index 9d624ab..4a49995 100644 --- a/client/src/platform/linux/ui.rs +++ b/client/src/platform/linux/ui.rs @@ -1014,6 +1014,7 @@ fn maybe_apply_os_update( force: bool, recovery: bool, ) { + if crate::update_download::deferred() { return; } if !os_update::enabled() { info!("os-update: disabled or not a full BetterFrame OS installation"); return; @@ -1140,6 +1141,7 @@ fn maybe_apply_firmware_update( force: bool, recovery: bool, ) { + if crate::update_download::deferred() { return; } if !server::ota_enabled("BF_ENABLE_APP_OTA") { info!("firmware: disabled (BF_ENABLE_APP_OTA = 0)"); return; @@ -1238,6 +1240,11 @@ fn run_firmware_update_worker( UPDATE_APPLY_ACTIVE.store(false, Ordering::SeqCst); FIRMWARE_ACTIVE.store(false, Ordering::SeqCst); if let Err(err) = result { + if crate::update_download::is_deferred(&err) { + let _ = tx.send(WorkerMsg::UpdateProgress(None)); + info!("firmware: download rate limited; retrying later without recording an installation failure"); + return; + } let failures = crate::update_guard::record_failure("firmware", &info.version, &err); let _ = tx.send(WorkerMsg::UpdateProgress(None)); warn!("firmware: apply failed: {err}"); diff --git a/client/src/platform/linux/update_download.rs b/client/src/platform/linux/update_download.rs new file mode 100644 index 0000000..8d9929c --- /dev/null +++ b/client/src/platform/linux/update_download.rs @@ -0,0 +1,169 @@ +//! Preserve the normal authenticated fleet route; public downloads are recovery. +use std::{ + sync::Mutex, + time::{Duration, Instant}, +}; + +const DEFERRED: &str = "update download deferred by server rate limit"; +static RETRY_AT: Mutex> = Mutex::new(None); + +pub fn deferred() -> bool { + RETRY_AT + .lock() + .unwrap() + .is_some_and(|until| Instant::now() < until) +} + +pub fn is_deferred(error: &str) -> bool { + error == DEFERRED +} + +pub fn get( + server: &str, + key: Option<&str>, + path: &str, + offset: u64, +) -> Result { + if deferred() { + return Err(DEFERRED.into()); + } + let client = crate::network::blocking_client(); + let public_path = path + .replacen( + "/api/kiosk/firmware/download/", + "/api/firmware/public/download/", + 1, + ) + .replacen("/api/kiosk/os/download/", "/api/os/public/download/", 1); + let key = key.filter(|key| !key.is_empty() && public_path != path); + let request = |path: &str, key: Option<&str>| { + let mut request = client.get(format!("{server}{path}")); + if let Some(key) = key { + request = request.bearer_auth(key); + } + if offset > 0 { + request = request.header("Range", format!("bytes={offset}-")); + } + request.timeout(Duration::from_secs(300)).send() + }; + let response = if key.is_some() { + match request(path, key) { + Ok(response) if !matches!(response.status().as_u16(), 401 | 403 | 404 | 500..=599) => { + Ok(response) + } + // Rejected authentication or a failed control endpoint cannot prevent + // fetching the same signed artifact through the recovery route. + _ => request(&public_path, None), + } + } else { + request(&public_path, None) + } + .map_err(|error| format!("download request: {error}"))?; + if response.status().as_u16() == 429 { + let delay = response + .headers() + .get("retry-after") + .and_then(|v| v.to_str().ok()) + .and_then(|v| v.parse::().ok()) + .unwrap_or(60) + .clamp(1, 3600); + // Spread a shared-egress fleet across subsequent admission windows. + *RETRY_AT.lock().unwrap() = + Some(Instant::now() + Duration::from_secs(delay + rand::random::() % 61)); + return Err(DEFERRED.into()); + } + Ok(response) +} + +#[cfg(test)] +pub static TEST_LOCK: Mutex<()> = Mutex::new(()); + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Read, Write}; + #[test] + fn healthy_fleet_stays_authenticated_and_recovery_defers_on_throttling() { + let _lock = TEST_LOCK.lock().unwrap(); + *RETRY_AT.lock().unwrap() = None; + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let server = format!("http://{}", listener.local_addr().unwrap()); + let handler = std::thread::spawn(move || { + let mut cases = vec![("/api/kiosk/firmware/download/app", true, false, "200 OK"); 7]; + cases.extend([ + ("/api/kiosk/os/download/os", true, true, "401 Unauthorized"), + ( + "/api/os/public/download/os", + false, + true, + "206 Partial Content", + ), + ( + "/api/firmware/public/download/app", + false, + false, + "429 Too Many Requests", + ), + ]); + for (path, auth, range, status) in cases { + let (mut stream, _) = listener.accept().unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(5))) + .unwrap(); + let mut request = Vec::new(); + let mut buffer = [0u8; 1024]; + while !request.windows(4).any(|value| value == b"\r\n\r\n") { + let size = stream.read(&mut buffer).unwrap(); + assert!(size > 0); + request.extend_from_slice(&buffer[..size]); + } + let request = String::from_utf8(request).unwrap().to_lowercase(); + assert!(request.starts_with(&format!("get {path} ")), "{request}"); + assert_eq!(request.contains("authorization: bearer device-key"), auth); + assert_eq!(request.contains("range: bytes=123-"), range); + write!(stream, "HTTP/1.1 {status}\r\nContent-Length: 1\r\nRetry-After: 60\r\nConnection: close\r\n\r\nx").unwrap(); + } + }); + for _ in 0..7 { + assert_eq!( + get( + &server, + Some("device-key"), + "/api/kiosk/firmware/download/app", + 0 + ) + .unwrap() + .status() + .as_u16(), + 200 + ); + } + assert_eq!( + get( + &server, + Some("device-key"), + "/api/kiosk/os/download/os", + 123 + ) + .unwrap() + .status() + .as_u16(), + 206 + ); + let error = get( + &server, + Some("device-key"), + "/api/firmware/public/download/app", + 0, + ) + .unwrap_err(); + assert!(is_deferred(&error)); + assert!(deferred()); + // A subsequent attempt is locally deferred, making no HTTP request. + assert!(is_deferred( + &get(&server, None, "/api/firmware/public/download/app", 0).unwrap_err() + )); + handler.join().unwrap(); + *RETRY_AT.lock().unwrap() = None; + } +} diff --git a/client/src/platform/linux/update_guard.rs b/client/src/platform/linux/update_guard.rs index afa04d7..e350996 100644 --- a/client/src/platform/linux/update_guard.rs +++ b/client/src/platform/linux/update_guard.rs @@ -50,6 +50,20 @@ pub fn record_attempt(kind: &str, version: &str) -> Result { Ok(attempts) } +/// Undo one pre-recorded attempt when the server deferred the download. +pub fn refund_attempt(kind: &str, version: &str) -> Result<(), String> { + let _lock = GUARD_LOCK.lock().map_err(|_| "Update attempt record locked")?; + let mut state = read_state(); + refund_entry(&mut state, &key(kind, version)); + write_state(&state) +} + +fn refund_entry(state: &mut AttemptState, key: &str) { + if let Some(entry) = state.entries.get_mut(key) { + entry.failures = entry.failures.saturating_sub(1); + } +} + pub fn record_failure(kind: &str, version: &str, err: &str) -> u32 { let _lock = GUARD_LOCK.lock().ok(); let mut state = read_state(); @@ -125,3 +139,20 @@ fn now_secs() -> u64 { .map(|d| d.as_secs()) .unwrap_or(0) } + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn rate_limit_refund_preserves_previous_installation_failures() { + let mut state = AttemptState::default(); + state.entries.insert("os:2.0".into(), AttemptEntry {failures: 3, ..Default::default()}); + refund_entry(&mut state, "os:2.0"); + assert_eq!(state.entries["os:2.0"].failures, 2); + refund_entry(&mut state, "os:unknown"); + assert_eq!(state.entries.len(), 1); + state.entries.get_mut("os:2.0").unwrap().failures = 0; + refund_entry(&mut state, "os:2.0"); + assert_eq!(state.entries["os:2.0"].failures, 0); + } +} diff --git a/client/src/platform/linux/update_recovery.rs b/client/src/platform/linux/update_recovery.rs index 4db6756..eb7f350 100644 --- a/client/src/platform/linux/update_recovery.rs +++ b/client/src/platform/linux/update_recovery.rs @@ -174,6 +174,7 @@ mod tests { } #[test] fn rejected_auth_recovers_over_public_http_with_saved_preferences() { + let _download_lock = crate::update_download::TEST_LOCK.lock().unwrap(); use std::io::{Read, Write}; let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let server = format!("http://{}", listener.local_addr().unwrap()); @@ -187,6 +188,7 @@ mod tests { "200 OK", r#"{"up_to_date":false,"update":{"release_id":"app","version":"2.0.0","channel":"beta","sha256":"test","signature":"test","size_bytes":1,"download_url":"/api/firmware/public/download/app"}}"#, ), + ("/api/kiosk/firmware/download/app", true, "401 Unauthorized", "{}"), ("/api/firmware/public/download/app", false, "200 OK", "x"), ("/api/kiosk/os/check?", true, "503 Unavailable", "{}"), ( diff --git a/docs/update-recovery.md b/docs/update-recovery.md index 50d8642..799c04a 100644 --- a/docs/update-recovery.md +++ b/docs/update-recovery.md @@ -2,7 +2,7 @@ The server sends `update_schedule` (weekly windows plus the server's IANA timezone), app/OS channels, and version pins in successful heartbeats. Linux saves these non-secret preferences atomically in `update-policy.json` alongside its state. It evaluates the recurring schedule locally using system zoneinfo, including daylight-saving transitions. The policy survives reboot and pairing reset. Demo mode and the app/OS OTA enable flags continue to apply. -Normal checks retain authenticated selection for per-device rollouts and explicit admin pushes. A rejected, failing, or malformed check falls back to public release selection using the saved channel and pin. A valid up-to-date response does not trigger fallback. Public selection does not fall through a missing or yanked pin to another version. Artifact downloads use public endpoints even when authenticated selection succeeded, so losing authentication between selection and download does not block installation. +Normal checks retain authenticated selection for per-device rollouts and explicit admin pushes. A rejected, failing, or malformed check falls back to public release selection using the saved channel and pin. A valid up-to-date response does not trigger fallback. Public selection does not fall through a missing or yanked pin to another version. Healthy sessions retain authenticated artifact downloads, avoiding the public per-IP fleet limit. Rejected authentication or failed download endpoints fall back to the same artifact's public route without credentials. HTTP 429 defers further downloads with Retry-After plus jitter; it does not consume an installation attempt or clear previous genuine failures. A separate worker starts alongside the enrollment worker. After two minutes without a valid heartbeat, it checks public releases every two minutes during the locally saved window, even if enrollment, bundle loading, or the control connection is stuck. No valid saved policy means no independent recovery updates: deploy the server first and let the updated client receive at least one successful heartbeat. A corrupt policy or unavailable timezone does not open a maintenance window. A received policy-change cancellation suspends cached recovery until replacement preferences arrive. From 908a34251780b6d4de2e8ed1e3aa316756b3252a Mon Sep 17 00:00:00 2001 From: bcbetterninja <327058824+bcbetterninja@users.noreply.github.com> Date: Thu, 24 Sep 2026 20:47:21 +0000 Subject: [PATCH 5/7] fix(kiosk): defer OS downloads without failure reporting --- client/src/platform/linux/os_update.rs | 3 +++ client/src/platform/linux/ui.rs | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/client/src/platform/linux/os_update.rs b/client/src/platform/linux/os_update.rs index c2b1b58..d43d02e 100644 --- a/client/src/platform/linux/os_update.rs +++ b/client/src/platform/linux/os_update.rs @@ -297,6 +297,9 @@ fn apply_tracked( // No installation was attempted. Refund only this reservation, retaining // any previous genuine failures for the same version. crate::update_guard::refund_attempt("os", &info.version)?; + // Keep the local journal retryable, but do not publish a failed-install + // status for a download that the server asked us to defer. + return result; } if let Err(ref error) = result { let _lock = JOURNAL_LOCK.lock().map_err(|_| "OS update record locked")?; diff --git a/client/src/platform/linux/ui.rs b/client/src/platform/linux/ui.rs index 4a49995..688d362 100644 --- a/client/src/platform/linux/ui.rs +++ b/client/src/platform/linux/ui.rs @@ -1111,6 +1111,11 @@ fn maybe_apply_os_update( UPDATE_APPLY_ACTIVE.store(false, Ordering::SeqCst); OS_UPDATE_ACTIVE.store(false, Ordering::SeqCst); if let Err(err) = result { + if crate::update_download::is_deferred(&err) { + let _ = tx.send(WorkerMsg::UpdateProgress(None)); + info!("os-update: download rate limited; retrying later without reporting an installation failure"); + return; + } let failures = crate::update_guard::failure_count("os", &info.version); let _ = tx.send(WorkerMsg::UpdateProgress(None)); warn!("os-update: apply failed: {err}"); From e77750139a0cef7d484a3e4b81b8a03d7ebd7532 Mon Sep 17 00:00:00 2001 From: bcbetterninja <327058824+bcbetterninja@users.noreply.github.com> Date: Thu, 24 Sep 2026 21:19:31 +0000 Subject: [PATCH 6/7] fix(kiosk): reject recovery policies from canceled heartbeats --- client/src/platform/linux/server.rs | 3 +- client/src/platform/linux/update_recovery.rs | 55 +++++++++++++++++--- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/client/src/platform/linux/server.rs b/client/src/platform/linux/server.rs index 4f01fc6..fc209a9 100644 --- a/client/src/platform/linux/server.rs +++ b/client/src/platform/linux/server.rs @@ -952,6 +952,7 @@ pub fn heartbeat( displays: &[DisplayReport], hw: &crate::hwmon::HwInfo, ) -> bool { + let policy_generation = crate::update_recovery::heartbeat_generation(); let client = crate::network::blocking_client(); let display_info: Vec<_> = displays .iter() @@ -1051,7 +1052,7 @@ pub fn heartbeat( "_check says key still valid, ignoring bf_kiosk_deleted from heartbeat" ); } - crate::update_recovery::record_heartbeat(server, &body); + crate::update_recovery::record_heartbeat(server, &body, policy_generation); let fw = body.get("firmware_channel").and_then(|v| v.as_str()); let os = body.get("os_update_channel").and_then(|v| v.as_str()); let fw_target = body.get("firmware_target_version").map(|v| v.as_str()); diff --git a/client/src/platform/linux/update_recovery.rs b/client/src/platform/linux/update_recovery.rs index eb7f350..ce95748 100644 --- a/client/src/platform/linux/update_recovery.rs +++ b/client/src/platform/linux/update_recovery.rs @@ -9,7 +9,12 @@ use std::{ }; static LAST_HEARTBEAT: Mutex> = Mutex::new(None); -static POLICY_WRITE: Mutex<()> = Mutex::new(()); +// Serializes policy publication with cancellation and identifies in-flight requests. +static POLICY_WRITE: Mutex = Mutex::new(0); + +pub fn heartbeat_generation() -> u64 { + *POLICY_WRITE.lock().unwrap() +} pub fn load() -> Option { let bytes = fs::read(crate::server::update_policy_path()).ok()?; @@ -29,7 +34,7 @@ pub fn policy_for(server: &str) -> Option { load().filter(|policy| policy.server == server) } -pub fn record_heartbeat(server: &str, body: &serde_json::Value) { +pub fn record_heartbeat(server: &str, body: &serde_json::Value, generation: u64) { // A successful HTTP response with an invalid body is not a healthy control plane. if body.get("ok").and_then(|v| v.as_bool()) != Some(true) { return; @@ -52,7 +57,10 @@ pub fn record_heartbeat(server: &str, body: &serde_json::Value) { .into(), os_update_target_version: body["os_update_target_version"].as_str().map(str::to_owned), }; - let _lock = POLICY_WRITE.lock().unwrap(); + let current_generation = POLICY_WRITE.lock().unwrap(); + if generation != *current_generation { + return; + } let path = crate::server::update_policy_path(); let result = save(&path, &policy); if let Err(error) = result { @@ -84,7 +92,8 @@ fn save(path: &Path, policy: &Policy) -> Result<(), String> { } pub fn suspend() { - let _lock = POLICY_WRITE.lock().unwrap(); + let mut generation = POLICY_WRITE.lock().unwrap(); + *generation = generation.wrapping_add(1); if let Err(error) = fs::write( crate::server::update_policy_path().with_extension("suspended"), b"awaiting updated policy", @@ -233,7 +242,7 @@ mod tests { "update_schedule":{"mode":"always","windows":[],"timezone":"UTC"}, "firmware_channel":"beta","firmware_target_version":"2.0.0", "os_update_channel":"beta","os_update_target_version":"2.0.0"}); - record_heartbeat(&server, &body); + record_heartbeat(&server, &body, heartbeat_generation()); assert!(allowed()); assert!(!needed()); let update = crate::firmware::check(&server, "deleted-key", "1.0.0").unwrap(); @@ -254,10 +263,44 @@ mod tests { suspend(); assert!(!allowed()); assert!(policy_for(&server).is_none()); - record_heartbeat(&server, &body); + record_heartbeat(&server, &body, heartbeat_generation()); assert!(allowed()); let _ = fs::remove_file(crate::server::update_policy_path()); } + #[test] + fn cancellation_rejects_in_flight_policy_until_a_fresh_heartbeat() { + let _download_lock = crate::update_download::TEST_LOCK.lock().unwrap(); + let server = "https://frame.example"; + let old_body = serde_json::json!({"ok":true, + "update_schedule":{"mode":"always","windows":[],"timezone":"UTC"}, + "firmware_channel":"beta","firmware_target_version":"2.0.0"}); + record_heartbeat(server, &old_body, heartbeat_generation()); + let in_flight = heartbeat_generation(); + suspend(); + record_heartbeat(server, &old_body, in_flight); + assert!(!allowed()); + assert!(policy_for(server).is_none()); + + let fresh = heartbeat_generation(); + let mut new_body = old_body.clone(); + new_body["firmware_target_version"] = serde_json::json!("3.0.0"); + record_heartbeat(server, &new_body, fresh); + assert!(allowed()); + assert_eq!(policy_for(server).unwrap().firmware_target_version.as_deref(), Some("3.0.0")); + // A late old response cannot overwrite the replacement policy either. + record_heartbeat(server, &old_body, in_flight); + assert_eq!(policy_for(server).unwrap().firmware_target_version.as_deref(), Some("3.0.0")); + // Every cancellation invalidates requests, including ones begun suspended. + suspend(); + let suspended_request = heartbeat_generation(); + suspend(); + record_heartbeat(server, &new_body, suspended_request); + assert!(policy_for(server).is_none()); + record_heartbeat(server, &new_body, heartbeat_generation()); + assert!(allowed()); + let _ = fs::remove_file(crate::server::update_policy_path()); + } + #[test] fn unknown_timezone_cannot_open_a_window() { assert!(!schedule_allows(&Schedule { From 81e16aac8d30ac8d366c040b690ae3ff4c2f2e7a Mon Sep 17 00:00:00 2001 From: bcbetterninja <327058824+bcbetterninja@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:19:37 +0000 Subject: [PATCH 7/7] fix(kiosk): preserve newer heartbeat recovery policies --- client/src/platform/linux/server.rs | 4 +- client/src/platform/linux/update_recovery.rs | 76 ++++++++++++++++---- 2 files changed, 63 insertions(+), 17 deletions(-) diff --git a/client/src/platform/linux/server.rs b/client/src/platform/linux/server.rs index fc209a9..385c0f9 100644 --- a/client/src/platform/linux/server.rs +++ b/client/src/platform/linux/server.rs @@ -952,7 +952,7 @@ pub fn heartbeat( displays: &[DisplayReport], hw: &crate::hwmon::HwInfo, ) -> bool { - let policy_generation = crate::update_recovery::heartbeat_generation(); + let policy_request = crate::update_recovery::begin_heartbeat(); let client = crate::network::blocking_client(); let display_info: Vec<_> = displays .iter() @@ -1052,7 +1052,7 @@ pub fn heartbeat( "_check says key still valid, ignoring bf_kiosk_deleted from heartbeat" ); } - crate::update_recovery::record_heartbeat(server, &body, policy_generation); + crate::update_recovery::record_heartbeat(server, &body, policy_request); let fw = body.get("firmware_channel").and_then(|v| v.as_str()); let os = body.get("os_update_channel").and_then(|v| v.as_str()); let fw_target = body.get("firmware_target_version").map(|v| v.as_str()); diff --git a/client/src/platform/linux/update_recovery.rs b/client/src/platform/linux/update_recovery.rs index ce95748..bcc25c2 100644 --- a/client/src/platform/linux/update_recovery.rs +++ b/client/src/platform/linux/update_recovery.rs @@ -10,10 +10,29 @@ use std::{ static LAST_HEARTBEAT: Mutex> = Mutex::new(None); // Serializes policy publication with cancellation and identifies in-flight requests. -static POLICY_WRITE: Mutex = Mutex::new(0); +static POLICY_WRITE: Mutex = Mutex::new(PolicyWrites { + generation: 0, + next_request: 0, + last_saved: 0, +}); -pub fn heartbeat_generation() -> u64 { - *POLICY_WRITE.lock().unwrap() +struct PolicyWrites { + generation: u64, + next_request: u64, + last_saved: u64, +} + +#[derive(Clone, Copy)] +pub struct HeartbeatRequest { + generation: u64, + sequence: u64, +} + +/// Capture before sending, so concurrent responses cannot roll back a newer policy. +pub fn begin_heartbeat() -> HeartbeatRequest { + let mut state = POLICY_WRITE.lock().unwrap(); + state.next_request += 1; + HeartbeatRequest { generation: state.generation, sequence: state.next_request } } pub fn load() -> Option { @@ -34,7 +53,7 @@ pub fn policy_for(server: &str) -> Option { load().filter(|policy| policy.server == server) } -pub fn record_heartbeat(server: &str, body: &serde_json::Value, generation: u64) { +pub fn record_heartbeat(server: &str, body: &serde_json::Value, request: HeartbeatRequest) { // A successful HTTP response with an invalid body is not a healthy control plane. if body.get("ok").and_then(|v| v.as_bool()) != Some(true) { return; @@ -57,8 +76,8 @@ pub fn record_heartbeat(server: &str, body: &serde_json::Value, generation: u64) .into(), os_update_target_version: body["os_update_target_version"].as_str().map(str::to_owned), }; - let current_generation = POLICY_WRITE.lock().unwrap(); - if generation != *current_generation { + let mut state = POLICY_WRITE.lock().unwrap(); + if request.generation != state.generation || request.sequence <= state.last_saved { return; } let path = crate::server::update_policy_path(); @@ -66,6 +85,7 @@ pub fn record_heartbeat(server: &str, body: &serde_json::Value, generation: u64) if let Err(error) = result { tracing::warn!("update policy could not be saved: {error}"); } else { + state.last_saved = request.sequence; let _ = fs::remove_file(path.with_extension("suspended")); } } @@ -92,8 +112,8 @@ fn save(path: &Path, policy: &Policy) -> Result<(), String> { } pub fn suspend() { - let mut generation = POLICY_WRITE.lock().unwrap(); - *generation = generation.wrapping_add(1); + let mut state = POLICY_WRITE.lock().unwrap(); + state.generation += 1; if let Err(error) = fs::write( crate::server::update_policy_path().with_extension("suspended"), b"awaiting updated policy", @@ -242,7 +262,7 @@ mod tests { "update_schedule":{"mode":"always","windows":[],"timezone":"UTC"}, "firmware_channel":"beta","firmware_target_version":"2.0.0", "os_update_channel":"beta","os_update_target_version":"2.0.0"}); - record_heartbeat(&server, &body, heartbeat_generation()); + record_heartbeat(&server, &body, begin_heartbeat()); assert!(allowed()); assert!(!needed()); let update = crate::firmware::check(&server, "deleted-key", "1.0.0").unwrap(); @@ -263,7 +283,7 @@ mod tests { suspend(); assert!(!allowed()); assert!(policy_for(&server).is_none()); - record_heartbeat(&server, &body, heartbeat_generation()); + record_heartbeat(&server, &body, begin_heartbeat()); assert!(allowed()); let _ = fs::remove_file(crate::server::update_policy_path()); } @@ -274,14 +294,14 @@ mod tests { let old_body = serde_json::json!({"ok":true, "update_schedule":{"mode":"always","windows":[],"timezone":"UTC"}, "firmware_channel":"beta","firmware_target_version":"2.0.0"}); - record_heartbeat(server, &old_body, heartbeat_generation()); - let in_flight = heartbeat_generation(); + record_heartbeat(server, &old_body, begin_heartbeat()); + let in_flight = begin_heartbeat(); suspend(); record_heartbeat(server, &old_body, in_flight); assert!(!allowed()); assert!(policy_for(server).is_none()); - let fresh = heartbeat_generation(); + let fresh = begin_heartbeat(); let mut new_body = old_body.clone(); new_body["firmware_target_version"] = serde_json::json!("3.0.0"); record_heartbeat(server, &new_body, fresh); @@ -292,11 +312,37 @@ mod tests { assert_eq!(policy_for(server).unwrap().firmware_target_version.as_deref(), Some("3.0.0")); // Every cancellation invalidates requests, including ones begun suspended. suspend(); - let suspended_request = heartbeat_generation(); + let suspended_request = begin_heartbeat(); suspend(); record_heartbeat(server, &new_body, suspended_request); assert!(policy_for(server).is_none()); - record_heartbeat(server, &new_body, heartbeat_generation()); + record_heartbeat(server, &new_body, begin_heartbeat()); + assert!(allowed()); + let _ = fs::remove_file(crate::server::update_policy_path()); + } + + #[test] + fn older_response_cannot_restore_always_after_newer_window_policy() { + let _download_lock = crate::update_download::TEST_LOCK.lock().unwrap(); + let server = "https://frame.example"; + let old_body = serde_json::json!({"ok":true, + "update_schedule":{"mode":"always","windows":[],"timezone":"UTC"}}); + record_heartbeat(server, &old_body, begin_heartbeat()); + assert!(allowed()); + let older = begin_heartbeat(); + let newer = begin_heartbeat(); + let mut new_body = old_body.clone(); + new_body["update_schedule"]["mode"] = serde_json::json!("windows"); + // No cancellation: a settings change can be delivered solely by heartbeat. + record_heartbeat(server, &new_body, newer); + record_heartbeat(server, &old_body, older); + assert_eq!(policy_for(server).unwrap().schedule.mode, "windows"); + assert!(!allowed()); + // Starting a request that fails does not discard another usable response. + let successful = begin_heartbeat(); + let failed = begin_heartbeat(); + record_heartbeat(server, &serde_json::json!({"ok":false}), failed); + record_heartbeat(server, &old_body, successful); assert!(allowed()); let _ = fs::remove_file(crate::server::update_policy_path()); }