From 2f12a2ae845fc46f3d3b1d943d28619472464754 Mon Sep 17 00:00:00 2001 From: Miles Ward Date: Thu, 28 May 2026 21:34:41 -0400 Subject: [PATCH] v0.4.68: fix NFS secure-export mount, logo cache-bust, dashboard disk card, NFS form spacing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four operator-reported issues from v0.4.67 validation. ## 1. NFS MNT3ERR_ACCES even with the host IP allow-listed Root cause: Linux kernel nfsd (what UniFi UNAS / Synology / TrueNAS all run underneath) exports with the `secure` option by default, which only accepts mount/NFS requests from a privileged source port (<1024). v0.4.67 explicitly connected from a non-privileged port on the mistaken assumption that uid 10001 can't bind low ports — but the binary carries CAP_NET_BIND_SERVICE (granted via setcap for the DHCP/TFTP/HTTP low-port binds), which also covers privileged *source* ports for outbound connects. Fix: build_connection now tries a privileged source port first (the common case for every appliance NAS), then falls back to a non-privileged port for `insecure` exports or capability-less environments. Each attempt has its own connect timeout; a timeout on the first attempt skips the fallback (the server isn't answering — a retry would just double the wait). Also: hint_for now recognizes MNT3ERR_ACCES distinctly from NFS3ERR_ACCES and explains both the allow-list and the secure/insecure angle, with the UniFi /var/nfs/shared/ path convention called out. ## 2. Custom logo didn't update the top-left brand mark The brand and favicon were pinned to ?v=, which only changes on upgrade — so uploading a new logo left the cached bundled SVG in place. Added a monotonic `rev` counter to BrandingStore that bumps on every set/clear, persisted across restarts, surfaced through index_html as an extra &r= cache-bust token on the brand mark + favicon URLs. Since index.html is served no-cache, the fresh token lands on the next reload after upload and the new logo appears immediately. (Note: this updates the WebUI brand mark. The PXE *boot menu* still shows the ASCII wordmark — painting the operator's PNG there needs the IMAGE_PNG-enabled iPXE rebuild that remains queued for native x86_64 hardware. The /branding/pxe-logo compositor is ready for when it lands.) ## 3. Disk-space card on the Dashboard Extracted the Storage tab's disk card into a shared diskSpaceCard(disk) helper and added it to the Dashboard grid under the stat strip. Dashboard fetches /api/storage/disk with the same graceful-degradation fallback the Storage tab uses. ## 4. NFS "Add share" button touching the form field The NFS card has a single form row (vs SMB's two), so the button butted right against it. Added margin-top:14px to match SMB's effective spacing. Tests: 162 passing (+2 — logo_rev bump, MNT3ERR_ACCES hint). clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 16 ++-- Cargo.toml | 2 +- crates/core/src/branding.rs | 36 +++++++++ crates/http-api/src/app.rs | 9 ++- crates/iso-store/src/nfs_share.rs | 119 ++++++++++++++++++++++++------ crates/webui/src/app.js | 91 +++++++++++++---------- crates/webui/src/index.html | 4 +- crates/webui/src/lib.rs | 10 ++- 8 files changed, 211 insertions(+), 76 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c449d3b..3a7283a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1171,7 +1171,7 @@ checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" [[package]] name = "openpxe" -version = "0.4.67" +version = "0.4.68" dependencies = [ "anyhow", "axum", @@ -1193,7 +1193,7 @@ dependencies = [ [[package]] name = "openpxe-core" -version = "0.4.67" +version = "0.4.68" dependencies = [ "anyhow", "bcrypt", @@ -1212,7 +1212,7 @@ dependencies = [ [[package]] name = "openpxe-dhcp-proxy" -version = "0.4.67" +version = "0.4.68" dependencies = [ "anyhow", "bytes", @@ -1226,7 +1226,7 @@ dependencies = [ [[package]] name = "openpxe-http-api" -version = "0.4.67" +version = "0.4.68" dependencies = [ "anyhow", "axum", @@ -1257,7 +1257,7 @@ dependencies = [ [[package]] name = "openpxe-ipxe-assets" -version = "0.4.67" +version = "0.4.68" dependencies = [ "openpxe-core", "rust-embed", @@ -1267,7 +1267,7 @@ dependencies = [ [[package]] name = "openpxe-iso-store" -version = "0.4.67" +version = "0.4.68" dependencies = [ "anyhow", "bcrypt", @@ -1294,7 +1294,7 @@ dependencies = [ [[package]] name = "openpxe-tftp" -version = "0.4.67" +version = "0.4.68" dependencies = [ "anyhow", "bytes", @@ -1308,7 +1308,7 @@ dependencies = [ [[package]] name = "openpxe-webui" -version = "0.4.67" +version = "0.4.68" [[package]] name = "parking_lot" diff --git a/Cargo.toml b/Cargo.toml index a182366..cbad640 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ members = [ ] [workspace.package] -version = "0.4.67" +version = "0.4.68" edition = "2021" rust-version = "1.95" license = "MIT OR Apache-2.0" diff --git a/crates/core/src/branding.rs b/crates/core/src/branding.rs index 68041e0..0b98939 100644 --- a/crates/core/src/branding.rs +++ b/crates/core/src/branding.rs @@ -44,6 +44,14 @@ struct Inner { /// MIME of the active logo, mirroring `logo_filename`. Cached here /// so the HTTP layer can set Content-Type without re-sniffing. logo_mime: Option, + /// Monotonic counter bumped on every set/clear. Surfaces as a + /// cache-bust token (`/assets/logo.svg?r=`) so the browser + /// fetches the new bytes the moment the operator swaps the logo — + /// the app version alone can't do this since it doesn't change on + /// upload. Persisted so the token stays stable across restarts and + /// keeps climbing across multiple swaps. + #[serde(default)] + rev: u64, } /// In-memory + on-disk override registry. Cheap to clone; locks are @@ -150,6 +158,7 @@ impl BrandingStore { let mut g = self.inner.write(); g.logo_filename = Some(filename.clone()); g.logo_mime = Some(mime.to_string()); + g.rev = g.rev.wrapping_add(1); } self.persist(); tracing::info!( @@ -166,6 +175,7 @@ impl BrandingStore { let mut g = self.inner.write(); let removed = g.logo_filename.take(); g.logo_mime = None; + g.rev = g.rev.wrapping_add(1); removed }; if let Some(name) = removed { @@ -185,6 +195,14 @@ impl BrandingStore { self.inner.read().logo_filename.is_some() } + /// Cache-bust token for the logo asset URL. Changes on every + /// set/clear so `/assets/logo.svg?r=` resolves to a fresh URL + /// whenever the operator swaps the brand mark. Stable otherwise. + #[must_use] + pub fn logo_rev(&self) -> u64 { + self.inner.read().rev + } + fn persist(&self) { let snap = self.inner.read().clone(); let body = match serde_json::to_vec_pretty(&snap) { @@ -292,6 +310,23 @@ mod tests { assert!(!entries.iter().any(|n| n == "logo.png"), "stale PNG left over: {entries:?}"); } + #[test] + fn logo_rev_bumps_on_each_set_and_clear() { + let dir = tempdir().unwrap(); + let b = BrandingStore::load_or_default(dir.path()); + assert_eq!(b.logo_rev(), 0); + b.set_logo("image/png", "png", b"\x89PNG\r\n\x1a\nfake").unwrap(); + assert_eq!(b.logo_rev(), 1); + b.set_logo("image/png", "png", b"\x89PNG\r\n\x1a\nfake2").unwrap(); + assert_eq!(b.logo_rev(), 2); + b.clear_logo().unwrap(); + assert_eq!(b.logo_rev(), 3); + // Survives a restart. + drop(b); + let b2 = BrandingStore::load_or_default(dir.path()); + assert_eq!(b2.logo_rev(), 3); + } + #[test] fn sanitize_ext_strips_separators_and_path_chars() { assert_eq!(sanitize_ext("svg"), "svg"); @@ -319,6 +354,7 @@ mod tests { let inner = Inner { logo_filename: Some("logo.png".into()), logo_mime: Some("image/png".into()), + rev: 0, }; std::fs::write( brand_dir.join("branding.json"), diff --git a/crates/http-api/src/app.rs b/crates/http-api/src/app.rs index 7d29f66..c65489d 100644 --- a/crates/http-api/src/app.rs +++ b/crates/http-api/src/app.rs @@ -208,7 +208,14 @@ async fn index(State(state): State) -> Response { // browsers re-fetch JS/CSS after an upgrade. We use the OpenPXE // binary version — every release ships a new value, every release // forces a fresh URL on each asset. - let html = openpxe_webui::index_html(&state.public_base_url, env!("CARGO_PKG_VERSION")); + // logo_rev cache-busts the brand mark / favicon independently of + // the release version, so an operator who swaps the custom logo + // sees it update on the next reload without waiting for an upgrade. + let html = openpxe_webui::index_html( + &state.public_base_url, + env!("CARGO_PKG_VERSION"), + state.branding.logo_rev(), + ); ( [ ( diff --git a/crates/iso-store/src/nfs_share.rs b/crates/iso-store/src/nfs_share.rs index 291f23f..245ad81 100644 --- a/crates/iso-store/src/nfs_share.rs +++ b/crates/iso-store/src/nfs_share.rs @@ -510,10 +510,9 @@ async fn list_isos( export: &str, port: u16, ) -> std::result::Result, NfsClientError> { - let mut conn = - tokio::time::timeout(CONNECT_TIMEOUT, build_connection(server, export, port)) - .await - .map_err(|_| NfsClientError::Timeout(server.to_string(), port))??; + // build_connection applies its own per-attempt timeout (privileged + // source port first, then a non-privileged fallback). + let mut conn = build_connection(server, export, port).await?; let root = conn.root_nfs_fh3(); let mut entries = Vec::new(); @@ -610,10 +609,7 @@ async fn stream_loop( max_len: Option, tx: tokio::sync::mpsc::Sender>, ) -> std::result::Result<(), NfsClientError> { - let mut conn = - tokio::time::timeout(CONNECT_TIMEOUT, build_connection(server, export, port)) - .await - .map_err(|_| NfsClientError::Timeout(server.to_string(), port))??; + let mut conn = build_connection(server, export, port).await?; let root = conn.root_nfs_fh3(); // Look up the file to get its handle. @@ -686,13 +682,30 @@ async fn stream_loop( Ok(()) } -/// Hand the connection builder the user's settings. `mount_path` is -/// the server-side export (e.g. "/srv/isos"). We disable -/// `connect_from_privileged_port` because the openpxe process runs -/// as uid 10001 and can't bind sub-1024 source ports — and most -/// modern NFS servers no longer require them anyway. If a server -/// does demand it the operator's hint will guide them to the -/// `insecure` export option. +/// Mount the export and return a live connection. +/// +/// ## Privileged source port (the v0.4.68 fix) +/// +/// Linux kernel `nfsd` — which is what UniFi UNAS, Synology, TrueNAS, +/// and essentially every appliance NAS runs underneath — exports with +/// the `secure` option **by default**. `secure` means the server only +/// accepts MOUNT3 / NFS3 requests whose TCP **source** port is in the +/// privileged range (< 1024). A client connecting from an ephemeral +/// high port gets `MNT3ERR_ACCES` at mount time — which is exactly the +/// error operators hit in v0.4.67 even with their host IP correctly in +/// the export's allow-list. +/// +/// v0.4.67 disabled privileged source ports because the openpxe +/// process runs as uid 10001 and "can't bind sub-1024 ports". That +/// reasoning was wrong: the binary carries `CAP_NET_BIND_SERVICE` +/// (granted via `setcap` in the Dockerfile so it can bind the DHCP / +/// TFTP / HTTP low ports as non-root), and that capability also lets +/// it bind a privileged *source* port for an outbound connection. +/// +/// So we now try a privileged source port first — the common case for +/// real NAS appliances — and fall back to a non-privileged port for +/// servers exported `insecure` (or environments where we genuinely +/// can't grab a low port). Each attempt gets its own connect timeout. async fn build_connection( server: &str, export: &str, @@ -701,12 +714,42 @@ async fn build_connection( nfs3_client::Nfs3Connection>, NfsClientError, > { - Nfs3ConnectionBuilder::new(TokioConnector, server, export) - .connect_from_privileged_port(false) + match connect_once(server, export, port, true).await { + Ok(conn) => Ok(conn), + // A timeout means the server didn't answer at all — a + // non-privileged retry would just time out again and double + // the operator's wait. Surface the timeout immediately. + Err(primary @ NfsClientError::Timeout(..)) => Err(primary), + Err(primary) => match connect_once(server, export, port, false).await { + Ok(conn) => Ok(conn), + // Surface the privileged-attempt error: for the dominant + // `secure`-export case it's the one whose hint points at + // the real fix. + Err(_) => Err(primary), + }, + } +} + +/// One mount attempt with a specific source-port policy, bounded by +/// [`CONNECT_TIMEOUT`]. +async fn connect_once( + server: &str, + export: &str, + port: u16, + privileged: bool, +) -> std::result::Result< + nfs3_client::Nfs3Connection>, + NfsClientError, +> { + let fut = Nfs3ConnectionBuilder::new(TokioConnector, server, export) + .connect_from_privileged_port(privileged) .nfs3_port(port) - .mount() - .await - .map_err(|e| NfsClientError::Connect(e.to_string())) + .mount(); + match tokio::time::timeout(CONNECT_TIMEOUT, fut).await { + Ok(Ok(conn)) => Ok(conn), + Ok(Err(e)) => Err(NfsClientError::Connect(e.to_string())), + Err(_) => Err(NfsClientError::Timeout(server.to_string(), port)), + } } #[derive(Debug)] @@ -778,14 +821,34 @@ fn status_label(status: nfs3::nfsstat3) -> String { /// transport-level messages from the Rust crate. fn hint_for(text: &str) -> Option { let s = text.to_ascii_lowercase(); - if s.contains("nfs3err_acces") || s.contains("permission denied") { + if s.contains("mnt3err_acces") || s.contains("mount") && s.contains("acces") { + // Mount-protocol access denial. Two common causes, in order of + // likelihood for an appliance NAS: (1) the export requires a + // privileged source port (`secure`, the Linux default) — we + // already retry with one, so reaching here means even that was + // refused; (2) the client IP isn't in the allow-list. + Some( + "the NFS server denied the mount (MNT3ERR_ACCES). Two things to \ + check on the server: (1) this OpenPXE host's IP is in the \ + export's allowed-clients list, and (2) if your export uses the \ + default `secure` option, OpenPXE already connects from a \ + privileged port — but if the server still refuses, add \ + `insecure` to the export. On UniFi UNAS, confirm the host IP is \ + listed under the share's NFS permissions and the export path is \ + /var/nfs/shared/ (not just /)." + .into(), + ) + } else if s.contains("nfs3err_acces") || s.contains("permission denied") { Some( "the NFS server rejected this client. Most likely your export \ is restricted by client IP — add this OpenPXE host (or its \ subnet) to the export's allowed-clients list on the server." .into(), ) - } else if s.contains("nfs3err_noent") || s.contains("nfs3err_notdir") { + } else if s.contains("nfs3err_noent") + || s.contains("nfs3err_notdir") + || s.contains("mnt3err_noent") + { Some( "the export path doesn't exist on the server, or it isn't a \ directory. Double-check the path (e.g. /srv/isos vs /isos — \ @@ -888,6 +951,18 @@ mod tests { ); } + #[test] + fn hint_for_mount_acces_calls_out_privileged_port_and_allowlist() { + // The dominant v0.4.67 field failure: mount denied even with the + // host IP allow-listed, because the export is `secure` and the + // client used a high source port. The hint should mention both + // the allow-list and the secure/insecure angle. + let h = hint_for("connect failed: MNT3ERR_ACCES").unwrap(); + let lc = h.to_lowercase(); + assert!(lc.contains("insecure") || lc.contains("privileged"), "got: {h}"); + assert!(lc.contains("allow") || lc.contains("permission"), "got: {h}"); + } + #[test] fn hint_for_noent_points_to_export_path() { let h = hint_for("NFS server returned NFS3ERR_NOENT").unwrap(); diff --git a/crates/webui/src/app.js b/crates/webui/src/app.js index 3e85bde..bb3132a 100644 --- a/crates/webui/src/app.js +++ b/crates/webui/src/app.js @@ -116,6 +116,46 @@ return root; } + // Disk-space card. Free + used + total for the volume hosting the ISO + // directory, with a coloured bar. Warns at 80% and goes red at 95% so + // the operator sees the runway shrinking before uploads start failing + // with ENOSPC. Shared by the Storage tab and the Dashboard (v0.4.68). + function diskSpaceCard(disk) { + const total = Number(disk.total_bytes || 0); + const avail = Number(disk.available_bytes || 0); + const used = Number(disk.used_bytes || 0); + const pctUsed = total > 0 ? (used / total) * 100 : 0; + let barClass = 'diskbar'; + if (pctUsed >= 95) barClass += ' full'; + else if (pctUsed >= 80) barClass += ' warn'; + return el('div', {class:'card'}, [ + el('header', {}, [ + el('h2', {}, 'Disk space'), + el('span', {class:'sub'}, + total > 0 ? (pctUsed.toFixed(1) + '% used') : 'unavailable'), + ]), + el('div', {class:'body'}, [ + el('div', {style:'color:var(--fg-dim);font-size:12px;word-break:break-all'}, + disk.path ? ('Volume: ' + disk.path) : 'Volume path unknown'), + el('div', {class: barClass}, + el('div', {class:'fill', + style:'width:' + Math.min(100, pctUsed).toFixed(1) + '%'})), + el('div', {class:'disk-meta'}, [ + el('span', {}, ['Used ', el('strong', {}, fmtBytes(used))]), + el('span', {}, ['Free ', el('strong', {}, fmtBytes(avail))]), + el('span', {}, ['Total ', el('strong', {}, fmtBytes(total))]), + ]), + pctUsed >= 95 + ? el('p', {class:'msg err', style:'margin-top:10px'}, + '⚠ Less than 5% free. Remove old ISOs or grow the volume before uploading more.') + : (pctUsed >= 80 + ? el('p', {class:'msg', style:'color:var(--warn);margin-top:10px'}, + 'Volume is getting full. Consider pruning old ISOs.') + : null), + ]), + ]); + } + // Categorize an ISO row's "bootable now" status — drives the amber // tint borrowed from Bootimus v0.1.62. Returns {ok, reason}. function bootability(iso, settings) { @@ -142,6 +182,13 @@ const isos = await getJSON('/api/isos'); const clients = (await getJSON('/api/clients')).clients || []; const entries = (await getJSON('/api/queue')).entries || []; + // v0.4.68: surface the same disk-space card the Storage tab shows, + // so operators see capacity at a glance from the landing page. + // Tolerate the endpoint being unavailable (e.g. statvfs failure) + // the same way the Storage tab does. + const disk = await getJSON('/api/storage/disk').catch(() => ({ + total_bytes: 0, available_bytes: 0, used_bytes: 0, path: '?', + })); const ipxeOk = (status.ipxe_assets || []).length > 0; const stats = el('div', {class: 'statstrip'}, [ @@ -222,7 +269,7 @@ '⚠ ' + i.filename + ' — ' + b.reason))) ]) : null; - return el('div', {class:'grid'}, [stats, recentBlock, problemsBlock].filter(Boolean)); + return el('div', {class:'grid'}, [stats, diskSpaceCard(disk), recentBlock, problemsBlock].filter(Boolean)); }, network: async () => { @@ -737,7 +784,7 @@ const nfsMsg = el('div', {class:'msg'}); const nfsServerIn = el('input', {type:'text', placeholder:'10.0.0.5'}); const nfsExportIn = el('input', {type:'text', placeholder:'/srv/isos'}); - const addNfs = el('button', {onclick: async () => { + const addNfs = el('button', {style:'margin-top:14px', onclick: async () => { if (!nfsServerIn.value || !nfsExportIn.value) { nfsMsg.replaceChildren(document.createTextNode('Server and export are required.')); nfsMsg.className = 'msg err'; return; @@ -794,45 +841,7 @@ el('span'), ])) : [el('div', {class:'empty'}, 'No NFS shares configured.')]; - // Disk-space card. Free + used + total for the volume hosting the - // ISO directory, with a coloured bar. Warns at 80% and goes red at - // 95% so the operator sees the runway shrinking before uploads - // start failing with ENOSPC. - const total = Number(disk.total_bytes || 0); - const avail = Number(disk.available_bytes || 0); - const used = Number(disk.used_bytes || 0); - const pctUsed = total > 0 ? (used / total) * 100 : 0; - let barClass = 'diskbar'; - if (pctUsed >= 95) barClass += ' full'; - else if (pctUsed >= 80) barClass += ' warn'; - const diskCard = el('div', {class:'card'}, [ - el('header', {}, [ - el('h2', {}, 'Disk space'), - el('span', {class:'sub'}, - total > 0 - ? (pctUsed.toFixed(1) + '% used') - : 'unavailable'), - ]), - el('div', {class:'body'}, [ - el('div', {style:'color:var(--fg-dim);font-size:12px;word-break:break-all'}, - disk.path ? ('Volume: ' + disk.path) : 'Volume path unknown'), - el('div', {class: barClass}, - el('div', {class:'fill', - style:'width:' + Math.min(100, pctUsed).toFixed(1) + '%'})), - el('div', {class:'disk-meta'}, [ - el('span', {}, ['Used ', el('strong', {}, fmtBytes(used))]), - el('span', {}, ['Free ', el('strong', {}, fmtBytes(avail))]), - el('span', {}, ['Total ', el('strong', {}, fmtBytes(total))]), - ]), - pctUsed >= 95 - ? el('p', {class:'msg err', style:'margin-top:10px'}, - '⚠ Less than 5% free. Remove old ISOs or grow the volume before uploading more.') - : (pctUsed >= 80 - ? el('p', {class:'msg', style:'color:var(--warn);margin-top:10px'}, - 'Volume is getting full. Consider pruning old ISOs.') - : null), - ]), - ]); + const diskCard = diskSpaceCard(disk); return el('div', {class:'grid'}, [ diskCard, diff --git a/crates/webui/src/index.html b/crates/webui/src/index.html index b65e11c..abd245d 100644 --- a/crates/webui/src/index.html +++ b/crates/webui/src/index.html @@ -13,7 +13,7 @@ on the asset handlers, the practical caching window is one version. --> - + @@ -33,7 +33,7 @@