dylint: extended gotcha sweep (#826 — out-of-scope items)

Cargo.toml

@@ -27,6 +27,11 @@ exclude = [
     "dylints/ban_direct_serialport",
     "dylints/ban_file_based_locks",
     "dylints/ban_deploy_tool_direct_invocation",
+    "dylints/ban_process_exit_outside_main",
+    "dylints/ban_unwrap_in_daemon_handlers",
+    "dylints/cli_no_build_deploy_direct_use",
+    "dylints/require_multi_thread_flavor_when_spawning",
+    "dylints/ban_std_sync_mutex_in_async",
 ]
 
 [workspace.metadata.dylint]

ci/hooks/crate_guard.py

@@ -65,6 +65,11 @@
         "dylints/ban_direct_serialport",
         "dylints/ban_file_based_locks",
         "dylints/ban_deploy_tool_direct_invocation",
+        "dylints/ban_process_exit_outside_main",
+        "dylints/ban_unwrap_in_daemon_handlers",
+        "dylints/cli_no_build_deploy_direct_use",
+        "dylints/require_multi_thread_flavor_when_spawning",
+        "dylints/ban_std_sync_mutex_in_async",
     }
 )
 

dylints/README.md

@@ -36,6 +36,33 @@ itself stays on stable 1.94.1).
   `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.
+- **`ban_process_exit_outside_main/`** — bans `std::process::exit`
+  outside `crates/*/src/main.rs` and `crates/*/src/bin/*.rs`. Skipping
+  destructors leaks temp files, kills containment guards, and
+  truncates in-flight HTTP/WS responses. Legacy CLI subcommand
+  dispatchers exempted via `src/allowlist.txt`. See #826.
+- **`ban_unwrap_in_daemon_handlers/`** — bans `.unwrap()` inside
+  `crates/fbuild-daemon/src/handlers/**/*.rs` production code (tests
+  exempt by filename and by `#[cfg(test)]` module walking). Locks in
+  PR #833's hardening; new violations would crash the daemon. See
+  #826.
+- **`cli_no_build_deploy_direct_use/`** — bans `fbuild_build::*` /
+  `fbuild_deploy::*` references in `crates/fbuild-cli/src/` outside
+  the diagnostic-subcommand allowlist. Enforces the "thin HTTP
+  client" rule from `crates/CLAUDE.md` §"Dependency Graph". See
+  #826.
+- **`require_multi_thread_flavor_when_spawning/`** — flags
+  `#[tokio::test]` (without `flavor = "multi_thread"`) on async fns
+  that call `tokio::spawn(...)` in their body. The default
+  `current_thread` flavor deadlocks any test that awaits cross-task
+  state. Legacy violators exempted via `src/allowlist.txt`. See
+  #826.
+- **`ban_std_sync_mutex_in_async/`** — bans `std::sync::Mutex` /
+  `std::sync::RwLock` *type uses* inside `crates/fbuild-daemon/src/`
+  and `crates/fbuild-serial/src/` (tests exempt). Holding the guard
+  across `.await` starves the Tokio worker and a panic poisons the
+  lock. Existing synchronous uses exempted via `src/allowlist.txt`.
+  See #826.
 
 ## Running locally
 

dylints/ban_process_exit_outside_main/Cargo.toml

@@ -0,0 +1,20 @@
+[package]
+name = "ban_process_exit_outside_main"
+version = "0.1.0"
+description = "Ban std::process::exit outside main.rs / src/bin/*.rs"
+edition = "2021"
+publish = false
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+# Pinned to the same dylint_linting rev the other fbuild dylints use —
+# same rustup channel in rust-toolchain.toml, same upstream commit.
+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_process_exit_outside_main/README.md

@@ -0,0 +1,59 @@
+# `ban_process_exit_outside_main`
+
+Custom [dylint](https://github.com/trailofbits/dylint) that forbids
+calling `std::process::exit(_)` (and its `process::exit` re-imports)
+outside the canonical entry-point files: `crates/*/src/main.rs` and
+`crates/*/src/bin/*.rs`.
+
+## Why
+
+`std::process::exit` skips destructors. In fbuild that translates to:
+
+- Temp files in the cache root that were created via `NamedTempFile` /
+  `TempDir` are not unlinked → cache grows without bound.
+- `running-process` containment guards do not run their drop handlers →
+  child processes (esptool / qemu / simavr) may keep running after the
+  parent exits.
+- Tokio runtime is dropped abruptly, so in-flight HTTP/WS responses are
+  truncated and clients see EOF mid-response.
+
+The only places where `process::exit` is safe are the program entry
+points (`main.rs` / `src/bin/*.rs`) where the runtime is being torn
+down anyway. Everywhere else, return a `Result` (or `FbuildError`) and
+let the entry point translate it into an exit code in one place.
+
+## Scope
+
+Only files whose path contains BOTH `crates/` and a subsequent `/src/`
+segment are linted. Out of scope by design:
+
+- `crates/*/tests/`, `crates/*/examples/`, `crates/*/benches/`
+- `ci/`, `dylints/`, build scripts
+- Anything outside `crates/`
+
+Within scope, **two exemptions are unconditional** (no allowlist entry
+needed):
+
+- `crates/*/src/main.rs` — every crate's library/binary entry point.
+- `crates/*/src/bin/*.rs` — any extra binary in `src/bin/`.
+
+## Allowlist
+
+Files in scope that legitimately need `process::exit` (for example,
+subcommand dispatchers that have already printed an error and want to
+short-circuit `main`'s return path) are listed in `src/allowlist.txt`.
+Each entry needs an inline comment explaining why.
+
+## Toolchain
+
+Pinned to the same `nightly-2026-03-26` channel and the same
+`trailofbits/dylint` git rev (`4bd91ce…`) the other fbuild dylints use.
+
+## Running locally
+
+See `dylints/README.md` for the full local-run recipe. CI runs all
+dylints on every push/PR via `.github/workflows/dylint.yml`.
+
+## See also
+
+- Issue #826 — this lint's tracking issue (gotcha sweep)

dylints/ban_process_exit_outside_main/rust-toolchain.toml

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

dylints/ban_process_exit_outside_main/src/allowlist.txt

@@ -0,0 +1,34 @@
+# 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.
+#
+# `crates/*/src/main.rs` and `crates/*/src/bin/*.rs` are UNCONDITIONALLY
+# exempt in lib.rs (they are the canonical program entry points). They
+# do NOT need entries here.
+#
+# Rollout strategy:
+#   - The lint is ON and denies new `std::process::exit` calls in
+#     production code outside `main.rs` / `src/bin/*.rs`.
+#   - This file lists the *legacy* call sites that existed when the
+#     lint was first enabled (FastLED/fbuild#826). They are exempted so
+#     the lint can land without a repo-wide migration to `Result`-based
+#     control flow.
+#   - New files MUST NOT be added here. The target state is zero
+#     entries.
+
+# CLI subcommand dispatchers that translate daemon HTTP responses
+# into shell exit codes. The canonical pattern is "print the message
+# the daemon already wrote, then exit with the daemon's exit_code".
+# These are effectively a thin layer between the daemon and the
+# shell — migrating them to bubble Result back into main() requires
+# threading exit codes through fbuild_core::Result, which is a
+# separate refactor (#826 follow-up).
+crates/fbuild-cli/src/cli/build.rs
+crates/fbuild-cli/src/cli/deploy.rs
+crates/fbuild-cli/src/cli/dispatch.rs
+crates/fbuild-cli/src/cli/lnk.rs
+crates/fbuild-cli/src/cli/pio.rs
+crates/fbuild-cli/src/cli/purge.rs
+crates/fbuild-cli/src/cli/reset.rs
+crates/fbuild-cli/src/cli/serial_probe.rs
+crates/fbuild-cli/src/cli/show.rs

dylints/ban_process_exit_outside_main/src/lib.rs

@@ -0,0 +1,200 @@
+#![feature(rustc_private)]
+
+extern crate rustc_errors;
+extern crate rustc_hir;
+extern crate rustc_span;
+
+use rustc_errors::DiagDecorator;
+use rustc_hir::{def::Res, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_span::{symbol::Symbol, FileName, RemapPathScopeComponents};
+
+dylint_linting::declare_late_lint! {
+    /// ### What it does
+    ///
+    /// Bans `std::process::exit(_)` calls in fbuild production code
+    /// (`crates/*/src/`) outside the canonical program entry-point
+    /// files (`crates/*/src/main.rs` and `crates/*/src/bin/*.rs`).
+    ///
+    /// ### Why is this bad?
+    ///
+    /// `std::process::exit` skips destructors. In fbuild that
+    /// translates to: temp files / `TempDir`s under the cache root are
+    /// not cleaned up; `running-process` containment guards do not run
+    /// their drop handlers (child processes keep running after the
+    /// parent dies); the tokio runtime is dropped abruptly so
+    /// in-flight HTTP/WS responses are truncated.
+    ///
+    /// The only safe places to call `process::exit` are the program
+    /// entry points where the runtime is being torn down anyway.
+    /// Everywhere else, return a `Result` (or `FbuildError`) and let
+    /// the entry point translate it into an exit code in one place.
+    ///
+    /// ### Known problems
+    ///
+    /// `std::process::abort` and `libc::_exit` are out of scope by
+    /// design — they're explicit "I know what I'm doing" calls and
+    /// already used inside post-fork containment paths.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,ignore
+    /// // Banned outside `main.rs` / `src/bin/*.rs`:
+    /// if config.broken {
+    ///     eprintln!("error: bad config");
+    ///     std::process::exit(1);
+    /// }
+    /// ```
+    ///
+    /// Use instead:
+    ///
+    /// ```rust,ignore
+    /// if config.broken {
+    ///     return Err(FbuildError::msg("bad config"));
+    /// }
+    /// ```
+    pub BAN_PROCESS_EXIT_OUTSIDE_MAIN,
+    Deny,
+    "ban std::process::exit outside main.rs / src/bin/*.rs"
+}
+
+/// Banned path. Matched exactly against the resolved DefId's def-path.
+const BANNED_PATH: &[&str] = &["std", "process", "exit"];
+
+const ALLOWLIST: &str = include_str!("allowlist.txt");
+
+/// Production-code scope. Only files whose path contains BOTH
+/// `crates/` and a subsequent `/src/` segment are linted.
+const CRATES_PREFIX: &str = "crates/";
+const SRC_SEGMENT: &str = "/src/";
+
+impl<'tcx> LateLintPass<'tcx> for BanProcessExitOutsideMain {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        let filename = source_filename(cx, expr.span);
+        let normalized = normalize_slashes(&filename);
+        if !in_production_scope(&normalized)
+            || is_entry_point(&normalized)
+            || is_allowlisted(&normalized)
+        {
+            return;
+        }
+
+        if let ExprKind::Path(ref qpath) = expr.kind {
+            if let Res::Def(_, def_id) = cx.qpath_res(qpath, expr.hir_id) {
+                if def_path_equals(cx, def_id, BANNED_PATH) {
+                    emit_lint(cx, expr.span);
+                }
+            }
+        }
+    }
+}
+
+fn emit_lint(cx: &LateContext<'_>, span: rustc_span::Span) {
+    cx.opt_span_lint(
+        BAN_PROCESS_EXIT_OUTSIDE_MAIN,
+        Some(span),
+        DiagDecorator(|diag| {
+            diag.primary_message(
+                "`std::process::exit` outside `main.rs` / `src/bin/*.rs` skips destructors \
+                 — temp files leak, containment guards don't run, the tokio runtime is \
+                 dropped abruptly, and in-flight HTTP/WS responses get truncated. Return a \
+                 `Result` / `FbuildError` and let `main` translate it into an exit code in \
+                 one place. If `process::exit` is truly justified here, allowlist the file \
+                 in `dylints/ban_process_exit_outside_main/src/allowlist.txt` with a \
+                 one-line reason.",
+            );
+        }),
+    );
+}
+
+fn source_filename(cx: &LateContext<'_>, span: rustc_span::Span) -> String {
+    match cx.sess().source_map().span_to_filename(span) {
+        FileName::Real(real_filename) => real_filename
+            .local_path()
+            .map(|path| path.to_string_lossy().into_owned())
+            .unwrap_or_else(|| {
+                real_filename
+                    .path(RemapPathScopeComponents::DIAGNOSTICS)
+                    .to_string_lossy()
+                    .into_owned()
+            }),
+        filename => filename
+            .display(RemapPathScopeComponents::DIAGNOSTICS)
+            .to_string(),
+    }
+}
+
+fn in_production_scope(normalized: &str) -> bool {
+    let Some(crates_at) = normalized.find(CRATES_PREFIX) else {
+        return false;
+    };
+    let after_crates = &normalized[crates_at + CRATES_PREFIX.len()..];
+    after_crates.contains(SRC_SEGMENT)
+}
+
+/// Unconditional exemptions for canonical program entry points:
+///   * `crates/*/src/main.rs`
+///   * `crates/*/src/bin/*.rs`
+fn is_entry_point(normalized: &str) -> bool {
+    normalized.ends_with("/src/main.rs")
+        || normalized.contains("/src/bin/") && normalized.ends_with(".rs")
+}
+
+fn is_allowlisted(normalized: &str) -> bool {
+    ALLOWLIST
+        .lines()
+        .map(str::trim)
+        .filter(|line| !line.is_empty() && !line.starts_with('#'))
+        .any(|allowed| normalized.ends_with(allowed))
+}
+
+fn normalize_slashes(path: &str) -> String {
+    path.replace('\\', "/")
+}
+
+fn def_path_equals(
+    cx: &LateContext<'_>,
+    def_id: rustc_hir::def_id::DefId,
+    expected: &[&str],
+) -> bool {
+    let def_path = cx.get_def_path(def_id);
+    if def_path.len() != expected.len() {
+        return false;
+    }
+    def_path
+        .iter()
+        .zip(expected.iter())
+        .all(|(actual, expected_segment)| *actual == Symbol::intern(expected_segment))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn in_production_scope_matches_src_files() {
+        assert!(in_production_scope("crates/fbuild-cli/src/cli/build.rs"));
+        assert!(in_production_scope("crates/fbuild-daemon/src/main.rs"));
+    }
+
+    #[test]
+    fn in_production_scope_rejects_non_src() {
+        assert!(!in_production_scope("crates/fbuild-cli/tests/foo.rs"));
+        assert!(!in_production_scope("build.rs"));
+    }
+
+    #[test]
+    fn entry_points_are_recognised() {
+        assert!(is_entry_point("crates/fbuild-cli/src/main.rs"));
+        assert!(is_entry_point("crates/fbuild-daemon/src/main.rs"));
+        assert!(is_entry_point(
+            "crates/fbuild-config/src/bin/enrich_boards.rs"
+        ));
+        assert!(is_entry_point(
+            "crates/fbuild-daemon/src/bin/containment_harness.rs"
+        ));
+        assert!(!is_entry_point("crates/fbuild-cli/src/cli/build.rs"));
+        // `lib.rs` is NOT an entry point — the function `main()` isn't there.
+        assert!(!is_entry_point("crates/fbuild-cli/src/lib.rs"));
+    }
+}

dylints/ban_std_sync_mutex_in_async/Cargo.toml

@@ -0,0 +1,20 @@
+[package]
+name = "ban_std_sync_mutex_in_async"
+version = "0.1.0"
+description = "Ban std::sync::Mutex / std::sync::RwLock in async-reachable fbuild code (daemon, serial)"
+edition = "2021"
+publish = false
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+# Pinned to the same dylint_linting rev the other fbuild dylints use —
+# same rustup channel in rust-toolchain.toml, same upstream commit.
+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