feat(dylints): add ban_env_var_set_after_import + require_oncelock_install_before_use (#840)

Cargo.toml

@@ -44,6 +44,11 @@ exclude = [
     "dylints/ban_runtime_new_outside_main",
     "dylints/ban_poison_panic",
     "dylints/ban_print_in_production",
+    # FastLED/fbuild#840 initializer-order sweep — convention-based
+    # checks for env-var mutation and OnceLock installation outside
+    # binary entry points. MIR-based dataflow check is a follow-up.
+    "dylints/ban_env_var_set_after_import",
+    "dylints/require_oncelock_install_before_use",
 ]
 
 [workspace.metadata.dylint]

ci/hooks/crate_guard.py

@@ -84,6 +84,12 @@
         "dylints/ban_runtime_new_outside_main",
         "dylints/ban_poison_panic",
         "dylints/ban_print_in_production",
+        # FastLED/fbuild#840 initializer-order sweep — convention-based
+        # checks for env-var mutation and OnceLock installation outside
+        # binary entry points. See dylints/ban_env_var_set_after_import
+        # and dylints/require_oncelock_install_before_use READMEs.
+        "dylints/ban_env_var_set_after_import",
+        "dylints/require_oncelock_install_before_use",
     }
 )
 

dylints/ban_env_var_set_after_import/Cargo.toml

@@ -0,0 +1,18 @@
+[package]
+name = "ban_env_var_set_after_import"
+version = "0.1.0"
+description = "Ban std::env::set_var outside binary entry points (FastLED/fbuild#840)"
+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_env_var_set_after_import/README.md

@@ -0,0 +1,50 @@
+# `ban_env_var_set_after_import`
+
+Custom [dylint](https://github.com/trailofbits/dylint) for the
+initializer-order sweep (FastLED/fbuild#840).
+
+## What
+
+Bans `std::env::set_var` in production code outside the following
+allow-listed scopes:
+
+- `**/main.rs` (binary entry points)
+- `**/src/bin/**` (alternate binary entry points)
+- `**/tests/**` (integration tests)
+- any module annotated `#[cfg(test)]`
+
+## Why
+
+Process-global env-var mutation has order-of-init hazards. The Python
+fbuild hit exactly this bug: `paths.py` cached `FBUILD_DEV_MODE` at
+module import time, but `cli.py` set the variable *after* that import
+ran, so dev-mode paths silently resolved to prod (see the project
+memory "Daemon paths.py import-time bug"). The Rust-side analog is a
+module that reads `std::env::var` into a `static` / `OnceLock` before
+another module ran `std::env::set_var`.
+
+The convention this lint enforces: only `main()` (and tests, which set
+up their own isolated process state) may mutate env vars. Every other
+site should accept the value as an argument, read it lazily through a
+function (not a static), or — for path resolution — go through
+`fbuild-paths`'s lazy helpers.
+
+## Limitation (convention-based, not dataflow)
+
+This is a *call-site* check. The lint catches every `std::env::set_var`
+outside the allow-list; it does NOT prove the variable was actually
+read by another module before this call ran. A proper "set after read"
+check requires MIR-level dataflow across the whole crate graph
+("**MIR for completeness later**" — tracked as a follow-up on #840).
+The convention is the fastest-landing prevention available today.
+
+## Allowlist
+
+Empty by design. Scope exemptions live in `lib.rs` by file path, not
+in `src/allowlist.txt`.
+
+## Toolchain
+
+Pinned to `nightly-2026-03-26` to match every other dylint in this
+repo. See the top-level `dylints/README.md` for the full setup
+instructions and the rationale for `build_dylint_driver.py`.

dylints/ban_env_var_set_after_import/rust-toolchain.toml

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

dylints/ban_env_var_set_after_import/src/README.md

@@ -0,0 +1,14 @@
+# `ban_env_var_set_after_import` — sources
+
+See the top-level [`../README.md`](../README.md) for the lint contract,
+allowlist policy, and the convention-vs-MIR limitation note. This
+directory contains:
+
+- **`lib.rs`** — the late-pass `LintContext` visitor.
+- **`allowlist.txt`** — empty by design for the #840 sweep. Scope
+  exemptions live in `lib.rs` by file path.
+
+Both files are loaded at lint-compile time via `include_str!` in
+`lib.rs`. To regenerate after editing the allowlist bump the version
+in this lint's `Cargo.toml` (the dylint .so cache is keyed off the
+manifest, not the allowlist).

dylints/ban_env_var_set_after_import/src/allowlist.txt

@@ -0,0 +1,6 @@
+# FastLED/fbuild#840 ships this lint with ZERO allowlist entries.
+# Scope exemptions (main.rs, src/bin/**, tests/**, #[cfg(test)]) live
+# in `lib.rs` by file path, not here.
+#
+# Each non-empty, non-comment line is matched against the tail of each
+# source file path (slashes normalized to /).

dylints/ban_env_var_set_after_import/src/lib.rs

@@ -0,0 +1,252 @@
+#![feature(rustc_private)]
+
+extern crate rustc_errors;
+extern crate rustc_hir;
+extern crate rustc_middle;
+extern crate rustc_span;
+
+use rustc_errors::DiagDecorator;
+use rustc_hir::{def::Res, Expr, ExprKind, HirId};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_span::{symbol::Symbol, FileName, RemapPathScopeComponents};
+
+dylint_linting::declare_late_lint! {
+    /// ### What it does
+    ///
+    /// Bans `std::env::set_var` in production code EXCEPT in:
+    ///
+    /// - `**/main.rs` (binary entry points)
+    /// - `**/src/bin/**` (alternate binary entry points)
+    /// - `**/tests/**` (integration tests)
+    /// - any module annotated `#[cfg(test)]`
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Process-global env-var mutation has order-of-init hazards: a
+    /// module that reads the var at import time (e.g. caches a path
+    /// derived from `FBUILD_DEV_MODE` in a `OnceLock`/`static`)
+    /// silently captures the wrong value if another module ran
+    /// `set_var` *after* that import. The Python fbuild had exactly
+    /// this bug — `paths.py` cached `FBUILD_DEV_MODE` at module import
+    /// time but `cli.py` set the variable afterward, so dev-mode paths
+    /// resolved to prod. See the project memory note "Daemon paths.py
+    /// import-time bug" and FastLED/fbuild#840.
+    ///
+    /// The convention-based fix: only `main()` (and tests, which set
+    /// up their own isolated process state) may call `set_var`. Every
+    /// other site should accept the value as an argument, read it
+    /// lazily through a function (not a `static`), or — for the
+    /// daemon — go through `fbuild-paths`'s lazy `__getattr__`-style
+    /// helpers.
+    ///
+    /// ### Limitation
+    ///
+    /// This is a *call-site* check, not a dataflow check. The lint
+    /// catches every `std::env::set_var` outside the allow-list; it
+    /// does NOT prove the variable was actually read by another module
+    /// before this call ran. A proper "set after read" check requires
+    /// MIR-level dataflow across the whole crate graph and is tracked
+    /// as a follow-up on #840 ("MIR for completeness later"). The
+    /// convention is the fastest-landing prevention available today.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,ignore
+    /// // crates/fbuild-something/src/foo.rs — banned
+    /// fn configure() {
+    ///     std::env::set_var("FBUILD_DEV_MODE", "1");
+    /// }
+    /// ```
+    ///
+    /// Use instead: set the env var inside `main()` before any module
+    /// that reads it is touched, or accept the value as an argument
+    /// and pass it explicitly.
+    pub BAN_ENV_VAR_SET_AFTER_IMPORT,
+    Deny,
+    "ban std::env::set_var outside binary entry points (FastLED/fbuild#840)"
+}
+
+const BANNED_PATHS: &[&[&str]] = &[
+    &["std", "env", "set_var"],
+];
+
+const CRATES_PREFIX: &str = "crates/";
+const SRC_SEGMENT: &str = "/src/";
+
+impl<'tcx> LateLintPass<'tcx> for BanEnvVarSetAfterImport {
+    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) {
+            return;
+        }
+        if is_entry_point(&normalized) || is_test_file(&normalized) {
+            return;
+        }
+        // `#[cfg(test)]` module walk: a `set_var` inside a unit-test
+        // module is fine even when the surrounding file is production.
+        if owned_by_cfg_test_module(cx, expr.hir_id) {
+            return;
+        }
+
+        match expr.kind {
+            ExprKind::Path(ref qpath) => {
+                if let Res::Def(_, def_id) = cx.qpath_res(qpath, expr.hir_id) {
+                    check_def_id(cx, expr.span, def_id);
+                }
+            }
+            ExprKind::Call(ref func, _) => {
+                if let ExprKind::Path(ref qpath) = func.kind {
+                    if let Res::Def(_, def_id) = cx.qpath_res(qpath, func.hir_id) {
+                        check_def_id(cx, expr.span, def_id);
+                    }
+                }
+            }
+            _ => {}
+        }
+    }
+}
+
+fn check_def_id(cx: &LateContext<'_>, span: rustc_span::Span, def_id: rustc_hir::def_id::DefId) {
+    for banned in BANNED_PATHS {
+        if def_path_equals(cx, def_id, banned) {
+            emit_lint(cx, span, banned);
+            return;
+        }
+    }
+}
+
+fn emit_lint(cx: &LateContext<'_>, span: rustc_span::Span, banned: &[&str]) {
+    let joined = banned.join("::");
+    cx.opt_span_lint(
+        BAN_ENV_VAR_SET_AFTER_IMPORT,
+        Some(span),
+        DiagDecorator(move |diag| {
+            diag.primary_message(format!(
+                "`{joined}` is only allowed in `main.rs`, `src/bin/*.rs`, \
+                 `#[cfg(test)]` modules, or `tests/**`. Process-global \
+                 env-var mutation has order-of-init hazards: any module \
+                 that already read the variable into a `static`/`OnceLock` \
+                 silently captures the pre-mutation value. Set the env var \
+                 in `main()` before any module reads it, or pass the value \
+                 explicitly as a function argument. See FastLED/fbuild#840."
+            ));
+        }),
+    );
+}
+
+fn owned_by_cfg_test_module(cx: &LateContext<'_>, hir_id: HirId) -> bool {
+    let mut current = cx.tcx.hir_get_parent_item(hir_id);
+    loop {
+        let attrs = cx.tcx.hir_attrs(current.into());
+        for attr in attrs {
+            if attr_is_cfg_test(attr) {
+                return true;
+            }
+        }
+        let parent = cx.tcx.hir_get_parent_item(current.into());
+        if parent == current {
+            return false;
+        }
+        current = parent;
+    }
+}
+
+fn attr_is_cfg_test(attr: &rustc_hir::Attribute) -> bool {
+    let Some(meta) = attr.meta() else {
+        return false;
+    };
+    let path = meta.path();
+    if path.segments.len() != 1 || path.segments[0].ident.as_str() != "cfg" {
+        return false;
+    }
+    let Some(list) = meta.meta_item_list() else {
+        return false;
+    };
+    list.iter().any(|nested| {
+        nested
+            .ident()
+            .map(|id| id.as_str() == "test")
+            .unwrap_or(false)
+    })
+}
+
+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)
+}
+
+fn is_entry_point(normalized: &str) -> bool {
+    // `**/main.rs` or `**/src/bin/**`.
+    normalized.ends_with("/main.rs")
+        || normalized.contains("/src/bin/")
+}
+
+fn is_test_file(normalized: &str) -> bool {
+    if normalized.contains("/tests/") {
+        return true;
+    }
+    let Some(name) = normalized.rsplit('/').next() else {
+        return false;
+    };
+    name.contains("tests") && name.ends_with(".rs")
+}
+
+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 entry_points_are_exempt() {
+        assert!(is_entry_point("crates/fbuild-cli/src/main.rs"));
+        assert!(is_entry_point("crates/fbuild-daemon/src/bin/containment_harness.rs"));
+        assert!(!is_entry_point("crates/fbuild-packages/src/library/library_manager.rs"));
+    }
+
+    #[test]
+    fn test_files_are_exempt() {
+        assert!(is_test_file("crates/fbuild-cli/tests/integration.rs"));
+        assert!(is_test_file("crates/fbuild-core/src/subprocess_tests.rs"));
+        assert!(!is_test_file("crates/fbuild-cli/src/main.rs"));
+    }
+}

dylints/require_oncelock_install_before_use/Cargo.toml

@@ -0,0 +1,18 @@
+[package]
+name = "require_oncelock_install_before_use"
+version = "0.1.0"
+description = "Require OnceLock::set/set_blocking install sites to live in binary entry points (FastLED/fbuild#840)"
+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