From dda6f44d1eadc97debefc2dd262ce02423bdc170 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 23 Apr 2026 15:24:33 -0400 Subject: [PATCH] ci: drop check-env-mutation.sh, rely on clippy disallowed_methods clippy.toml already bans std::env::{set_var,remove_var} via disallowed_methods, and every existing call site carries a scoped #[expect(clippy::disallowed_methods, reason = "...")]. The shell grep is redundant and forced a second, less granular allowlist. Also update server-secrets-strategy.md to describe clippy as the enforcement mechanism. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/rust.yml | 1 - bin/dev/check-env-mutation.sh | 44 ------------------------ docs-internal/server-secrets-strategy.md | 2 +- 3 files changed, 1 insertion(+), 46 deletions(-) delete mode 100755 bin/dev/check-env-mutation.sh diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index d47092a01..ac96ed440 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -47,7 +47,6 @@ jobs: with: persist-credentials: false - run: bin/dev/check-boundary.sh - - run: bin/dev/check-env-mutation.sh fmt: name: Format diff --git a/bin/dev/check-env-mutation.sh b/bin/dev/check-env-mutation.sh deleted file mode 100755 index dff4ac811..000000000 --- a/bin/dev/check-env-mutation.sh +++ /dev/null @@ -1,44 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -cd "$(dirname "$0")/../.." - -if command -v rg >/dev/null 2>&1; then - matches=$(rg -n 'std::env::(set_var|remove_var)' --glob '*.rs' || true) -else - matches=$(grep -R -n -E 'std::env::(set_var|remove_var)' . --include='*.rs' --exclude-dir=target --exclude-dir=.git || true) -fi - -fail=0 -while IFS= read -r match; do - [[ -z "$match" ]] && continue - - path=${match%%:*} - rest=${match#*:} - line=${rest#*:} - line=${line#"${line%%[![:space:]]*}"} - - case "$path:$line" in - "lib/crates/fabro-telemetry/src/spawn.rs:std::env::set_var(key, value);" | \ - "lib/crates/fabro-telemetry/src/spawn.rs:std::env::remove_var(key);" | \ - 'lib/crates/fabro-cli/src/main.rs:std::env::remove_var("FABRO_WORKER_TOKEN");' | \ - 'lib/crates/fabro-server/src/install.rs:std::env::set_var("FABRO_TEST_IN_MEMORY_STORE", "1");') - continue - ;; - esac - - echo "process env mutation check failed: $match" >&2 - fail=1 -done <<< "$matches" - -if [[ $fail -ne 0 ]]; then - cat >&2 <<'EOF' - -Do not mutate process-wide env with std::env::set_var/remove_var. -Inject env at construction time or on child-process Command values instead. -See docs-internal/server-secrets-strategy.md. -EOF - exit 1 -fi - -echo "Process env mutation checks passed." diff --git a/docs-internal/server-secrets-strategy.md b/docs-internal/server-secrets-strategy.md index 17ee5e724..1c092c3e5 100644 --- a/docs-internal/server-secrets-strategy.md +++ b/docs-internal/server-secrets-strategy.md @@ -9,7 +9,7 @@ This document defines how Fabro handles server-level secrets. - Resolution is snapshot-based: env and file are read once at construction, then treated as immutable for the life of the process. - `process env` wins over `server.env` on conflicts. - `fabro server start` never generates secrets. Missing required secrets are a startup error. -- `std::env::set_var` and `std::env::remove_var` are banned workspace-wide. Tests are not exempt. CI enforces this with `bin/dev/check-env-mutation.sh` so broad clippy suppressions cannot bypass it. +- `std::env::set_var` and `std::env::remove_var` are banned workspace-wide. Tests are not exempt. Enforced by clippy via `disallowed_methods` in `clippy.toml`; intentional exceptions must be annotated with a scoped `#[expect(clippy::disallowed_methods, reason = "...")]` at the call site. ## Active Server Secrets