dylint: code-quality gotcha sweep (#826) — 5 new lints + websocket hardening

Cargo.toml

@@ -20,7 +20,14 @@ members = [
 # `dylints/*/rust-toolchain.toml`) and must not be part of the
 # stable workspace build — `cargo dylint --all` builds them with
 # the pinned nightly via the discovery below. See #264.
-exclude = ["dylints/ban_raw_subprocess"]
+exclude = [
+    "dylints/ban_raw_subprocess",
+    "dylints/ban_std_pathbuf",
+    "dylints/ban_unrooted_tempdir",
+    "dylints/ban_direct_serialport",
+    "dylints/ban_file_based_locks",
+    "dylints/ban_deploy_tool_direct_invocation",
+]
 
 [workspace.metadata.dylint]
 libraries = [{ path = "dylints/*" }]

ci/hooks/crate_guard.py

@@ -60,6 +60,11 @@
         "bench/fastled-examples",
         # Workspace-excluded crates with their own toolchains:
         "dylints/ban_raw_subprocess",
+        "dylints/ban_std_pathbuf",
+        "dylints/ban_unrooted_tempdir",
+        "dylints/ban_direct_serialport",
+        "dylints/ban_file_based_locks",
+        "dylints/ban_deploy_tool_direct_invocation",
     }
 )
 

crates/fbuild-daemon/src/handlers/websockets.rs

@@ -12,6 +12,22 @@ use std::sync::Arc;
 use std::time::Duration;
 use tokio::sync::{mpsc, oneshot};
 
+/// Serialize a `SerialServerMessage` (or any `serde::Serialize` value)
+/// to JSON, falling back to a hardcoded JSON error frame if serialization
+/// somehow fails. Used on WebSocket error-reply paths where panicking
+/// would tear the whole socket down instead of just dropping one frame.
+///
+/// `serde_json::to_string` only fails for values whose `Serialize` impl
+/// returns an error or that contain non-finite floats inside a map key
+/// — neither shape occurs in `SerialServerMessage`. The fallback exists
+/// purely as a panic-free guarantee for the cold path; FastLED/fbuild#826
+/// flagged the prior `.unwrap()` calls as a stability hazard.
+fn serialize_or_fallback<T: serde::Serialize>(value: &T) -> String {
+    serde_json::to_string(value).unwrap_or_else(|_| {
+        r#"{"type":"error","message":"<internal serde failure>"}"#.to_string()
+    })
+}
+
 // ReaderControl -- inbound -> reader cross-task RPC for the small set
 // of `SerialClientMessage`s that need read-only access to the reader-
 // owned broadcast receiver (`ClearBuffer` and `GetInWaiting`).
@@ -116,7 +132,7 @@ async fn handle_serial_ws(mut socket: WebSocket, ctx: Arc<DaemonContext>) {
                                 message: format!("failed to open port: {}", e),
                             };
                             let _ = socket
-                                .send(Message::Text(serde_json::to_string(&err_msg).unwrap()))
+                                .send(Message::Text(serialize_or_fallback(&err_msg)))
                                 .await;
                             return;
                         }
@@ -128,7 +144,7 @@ async fn handle_serial_ws(mut socket: WebSocket, ctx: Arc<DaemonContext>) {
                         message: "expected attach message first".to_string(),
                     };
                     let _ = socket
-                        .send(Message::Text(serde_json::to_string(&err_msg).unwrap()))
+                        .send(Message::Text(serialize_or_fallback(&err_msg)))
                         .await;
                     return;
                 }
@@ -137,7 +153,7 @@ async fn handle_serial_ws(mut socket: WebSocket, ctx: Arc<DaemonContext>) {
                         message: format!("invalid message: {}", e),
                     };
                     let _ = socket
-                        .send(Message::Text(serde_json::to_string(&err_msg).unwrap()))
+                        .send(Message::Text(serialize_or_fallback(&err_msg)))
                         .await;
                     return;
                 }
@@ -170,7 +186,7 @@ async fn handle_serial_ws(mut socket: WebSocket, ctx: Arc<DaemonContext>) {
                 message: format!("port {} not open", port),
             };
             let _ = socket
-                .send(Message::Text(serde_json::to_string(&err_msg).unwrap()))
+                .send(Message::Text(serialize_or_fallback(&err_msg)))
                 .await;
             cleanup_ws_serial_session(&ctx, &port, &client_id, writer_acquired).await;
             return;
@@ -184,7 +200,7 @@ async fn handle_serial_ws(mut socket: WebSocket, ctx: Arc<DaemonContext>) {
         writer_pre_acquired: writer_acquired,
     };
     if socket
-        .send(Message::Text(serde_json::to_string(&attached).unwrap()))
+        .send(Message::Text(serialize_or_fallback(&attached)))
         .await
         .is_err()
     {

dylints/README.md

@@ -12,6 +12,30 @@ itself stays on stable 1.94.1).
   code (`crates/*/src/`). All subprocess spawns must flow through
   `fbuild_core::subprocess::run_command` /
   `fbuild_core::containment::*`. See #264.
+- **`ban_std_pathbuf/`** — bans raw `std::path::PathBuf` in workspace
+  code; steers callers at `fbuild_core::path::NormalizedPath` so paths
+  carry the normalization invariant Windows requires. Legacy call sites
+  exempted via `src/allowlist.txt`. See #826 / #436 / #437 / #282.
+- **`ban_unrooted_tempdir/`** — bans `tempfile::tempdir()` /
+  `tempfile::TempDir::new()` / `tempfile::NamedTempFile::new()` /
+  `std::env::temp_dir()` in production code; steers callers at the
+  `_in(...)` variants rooted under `fbuild_paths::get_cache_root()` so
+  every byte fbuild writes lives under one user-visible directory.
+  Legacy call sites exempted via `src/allowlist.txt`. See #826.
+- **`ban_direct_serialport/`** — bans direct use of the `serialport`
+  crate outside `crates/fbuild-serial/` and a small set of diagnostic
+  CLI entry points. All serial access must flow through
+  `fbuild-serial`'s blessed APIs so DTR/RTS rules, retry counts, and
+  the Windows USB-CDC contract stay consistent. See #826.
+- **`ban_file_based_locks/`** — bans file-based locking primitives
+  (`OpenOptions::create_new(true)` lock-file pattern, `fs2::FileExt`,
+  `flock`). All locking flows through the daemon's in-memory managers
+  per the `CLAUDE.md` "no file-based locks" rule. Locks in the
+  invariant; allowlist is empty. See #826.
+- **`ban_deploy_tool_direct_invocation/`** — bans direct
+  `Command::new("esptool" | "avrdude" | "picotool" | "dfu-util" |
+  "pyocd")` invocations outside `crates/fbuild-deploy/`. All deploy-
+  tool spawns must flow through `fbuild deploy`. See #826 / #694.
 
 ## Running locally
 
@@ -34,8 +58,8 @@ CI runs this on every push/PR via `.github/workflows/dylint.yml`.
 
 `dylint_linting` builds against a specific nightly rustc; the rustc
 internal API (`rustc_lint`, `rustc_hir`, `rustc_span`) changes between
-nightlies. Keeping the dylint crate in `[workspace.exclude]` lets it
-pin `nightly-2026-03-26` in its own `rust-toolchain.toml` without
+nightlies. Keeping each dylint crate out of the stable workspace lets
+it pin `nightly-2026-03-26` in its own `rust-toolchain.toml` without
 forcing the entire workspace to nightly.
 
 The workspace registers the lint directory via:
@@ -45,7 +69,7 @@ The workspace registers the lint directory via:
 libraries = [{ path = "dylints/*" }]
 ```
 
-so `cargo dylint --all` picks it up automatically.
+so `cargo dylint --all` picks every dylint up automatically.
 
 ## Why `build_dylint_driver.py`
 

dylints/ban_deploy_tool_direct_invocation/Cargo.toml

@@ -0,0 +1,18 @@
+[package]
+name = "ban_deploy_tool_direct_invocation"
+version = "0.1.0"
+description = "Ban Command::new(\"esptool\"|...) outside fbuild-deploy"
+edition = "2021"
+publish = false
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+dylint_linting = { git = "https://github.com/trailofbits/dylint", rev = "4bd91ce7729b74c7ee5664bbb588f7baf30b4a09" }
+
+[dev-dependencies]
+dylint_testing = { git = "https://github.com/trailofbits/dylint", rev = "4bd91ce7729b74c7ee5664bbb588f7baf30b4a09" }
+
+[package.metadata.rust-analyzer]
+rustc_private = true

dylints/ban_deploy_tool_direct_invocation/README.md

@@ -0,0 +1,58 @@
+# `ban_deploy_tool_direct_invocation`
+
+This lint bans `Command::new("esptool" | "esptool.py" | "avrdude" |
+"picotool" | "dfu-util" | "pyocd")` (and the `tokio::process::Command`
+equivalent) in fbuild production code (`crates/*/src/`) outside
+`crates/fbuild-deploy/`.
+
+## Why
+
+Per the agent docs (FastLED/fbuild#694 and `CLAUDE.md`'s "Essential
+Rules"), all deploy-tool spawns must flow through `fbuild deploy`.
+Direct invocation outside `fbuild-deploy` regresses the deploy contract:
+no `Deployer::post_deploy_recovery`, no consistent error reporting, no
+serial-port hand-off — and bypasses the `fbuild deploy --to emu`
+emulator routing.
+
+This lint complements `ban_raw_subprocess` (which catches
+`.spawn()/.output()/.status()` on `Command` regardless of the binary).
+`ban_raw_subprocess` is scoped to *how* you spawn; this one is scoped to
+*what* you spawn. Together they form an L-shape: even with legitimate
+allowlisted raw-spawn entries, you still can't spawn a deploy tool
+outside `fbuild-deploy`.
+
+## Scope
+
+Only files whose path contains BOTH `crates/` and a subsequent `/src/`
+segment are linted. Files anywhere under `crates/fbuild-deploy/` are
+unconditionally exempt — that crate is the legitimate owner.
+
+## Banned binary names
+
+Matched against the string literal passed to `Command::new(...)` (case
+sensitive):
+
+- `esptool`
+- `esptool.py`
+- `avrdude`
+- `picotool`
+- `dfu-util`
+- `pyocd`
+
+The match is exact. Paths like `/usr/local/bin/esptool` and quoted-arg
+shapes (`Command::new(path).arg("flash")`) won't trip the lint — those
+are caller-resolved paths and are likely going through `fbuild-deploy`
+already. The lint is about the *quick string-literal sketch* that's
+easy to add and easy to miss in review.
+
+## Allowlist
+
+Empty. `crates/fbuild-deploy/` is the only legitimate caller and is
+exempted by directory match, not by allowlist.
+
+## See also
+
+- FastLED/fbuild#826 — the dylint sweep tracking issue
+- FastLED/fbuild#694 — the `fbuild deploy` ownership rule
+- `crates/fbuild-deploy/` — the legitimate owner
+- `dylints/ban_raw_subprocess/` — the spawn-shape sibling

dylints/ban_deploy_tool_direct_invocation/rust-toolchain.toml

@@ -0,0 +1,3 @@
+[toolchain]
+channel = "nightly-2026-03-26"
+components = ["llvm-tools-preview", "rust-src", "rustc-dev"]

dylints/ban_deploy_tool_direct_invocation/src/allowlist.txt

@@ -0,0 +1,11 @@
+# Each non-empty, non-comment line is matched against the tail of each
+# source file path (slashes normalized to /). Use forward-slash paths
+# only — the lint normalizes \ to / before matching.
+#
+# Files under `crates/fbuild-deploy/` are unconditionally exempt in
+# lib.rs (that crate IS the legitimate owner). They do NOT need entries
+# here.
+#
+# The allowlist is intentionally EMPTY. Direct `Command::new(<deploy
+# tool>)` outside fbuild-deploy bypasses the deploy contract. Add an
+# entry ONLY with a maintainer-approved rationale in the PR.