ci: make public-hygiene-lint green

.github/scripts/next-version.sh

@@ -2,7 +2,7 @@
 # next-version.sh — compute THIS repo's next release tag for release-on-upstream.yml.
 #
 # Single source of truth for the version math, exercised in CI by release-selftest.yml so the
-# release automation can't silently rot (guard #135.8). Prints "v<MAJOR>.<MINOR>.<PATCH>" to stdout.
+# release automation can't silently rot. Prints "v<MAJOR>.<MINOR>.<PATCH>" to stdout.
 #
 # Inputs (env, all optional):
 #   INPUT_VERSION    explicit version to cut (leading "v" tolerated) -> used verbatim.

.github/workflows/release-selftest.yml

@@ -1,4 +1,4 @@
-# CI self-test for the release-on-upstream version-compute logic (guard #135.8).
+# CI self-test for the release-on-upstream version-compute logic.
 # Runs the REAL .github/scripts/next-version.sh against synthetic repos and asserts it produces a
 # valid next version for BOTH the has-prior-tag and no-prior-tag cases — WITHOUT publishing anything.
 # This is what keeps the release automation from silently rotting before the fleet fan-out is armed.

store-postgres-plugin/tests/admin_api_e2e.rs

@@ -40,6 +40,8 @@
 //! Together: real admin-API install -> real (admin-API-driven) plugin load -> real admin-API write
 //! -> real, independently-verified Postgres persistence.
 
+mod common;
+
 use base64::Engine as _;
 use std::path::PathBuf;
 use std::process::{Child, Command, Stdio};
@@ -80,9 +82,8 @@ fn postgres_url() -> Option<String> {
 /// Checks BOTH the "uplifted" `<profile_dir>/<name>` copy (only refreshed when `[lib]` is a ROOT
 /// build target of the invocation, e.g. `cargo build --all-targets`) and the raw
 /// `<profile_dir>/deps/<name>` compiler output (refreshed on every build that recompiles the lib,
-/// uplifted or not). A bare `cargo test --release` (what `release-check.sh`'s Phase 2 runs, and what
-/// cargo-mutants runs) does NOT uplift the cdylib to the top-level profile dir, only to
-/// `target/deps` — checking only `profile_dir` silently finds nothing and this test's CI hard-panic
+/// uplifted or not). A bare `cargo test --release` (what `release-check.sh`'s Phase 2 runs) does
+/// NOT uplift the cdylib to the top-level profile dir, only to `target/deps` — checking only `profile_dir` silently finds nothing and this test's CI hard-panic
 /// fires even though the cdylib really was built (confirmed against `release-check.sh`'s CI run:
 /// "the store-postgres-plugin cdylib is not built under CI" despite the prior `cargo test
 /// --workspace --release` step compiling it). Same fix already applied to auth-oidc-plugin's and
@@ -287,7 +288,9 @@ fn install_over_admin_api_then_mint_a_key_and_verify_postgres_directly() {
     let config1 = work.join("config1.yaml");
     std::fs::write(&config1, &providers_and_common).unwrap();
 
-    let child1 = Command::new(&busbar_bin)
+    let mut boot1 = Command::new(&busbar_bin);
+    common::apply_placeholder_secrets_from_files(&mut boot1, &[&config1, &providers]);
+    let child1 = boot1
         .env("BUSBAR_CONFIG", &config1)
         .env("BUSBAR_PROVIDERS", &providers)
         .env("BUSBAR_ADMIN_TOKEN", admin_token)
@@ -381,7 +384,9 @@ fn install_over_admin_api_then_mint_a_key_and_verify_postgres_directly() {
     )
     .unwrap();
 
-    let child2 = Command::new(&busbar_bin)
+    let mut boot2 = Command::new(&busbar_bin);
+    common::apply_placeholder_secrets_from_files(&mut boot2, &[&config2, &providers]);
+    let child2 = boot2
         .env("BUSBAR_CONFIG", &config2)
         .env("BUSBAR_PROVIDERS", &providers)
         .env("BUSBAR_ADMIN_TOKEN", admin_token)

store-postgres-plugin/tests/common/mod.rs

@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright (C) 2026 Busbar Inc and contributors
+
+//! Support code shared by this crate's integration tests: supplying the secrets a config fixture
+//! references so the real `busbar` binary can start.
+//!
+//! busbar resolves every `env:`/`file:` secret reference during `--validate` AND at boot, and exits
+//! non-zero when one of them cannot resolve. A fixture that names a variable nobody exported is
+//! therefore a gateway that cannot come up, and the test reports that as an opaque timeout or
+//! early-exit a long way from the YAML that caused it.
+//!
+//! The variable names are derived FROM the fixture text rather than listed here on purpose: the
+//! next person to add a secret reference to a config in these tests must not have to also remember
+//! to export it, because forgetting produces a failure that does not name the variable.
+//!
+//! This lives under `tests/common/` rather than `tests/` so cargo does not compile it as a test
+//! binary of its own; its own coverage lives in `tests/config_secrets.rs`.
+
+#![allow(dead_code)]
+// Each integration test binary compiles this module separately and uses a different subset of it,
+// so anything only one of them needs is genuinely dead code from the other's point of view.
+
+use std::path::Path;
+use std::process::Command;
+
+/// Stands in for every secret a fixture references but the test does not otherwise set. The content
+/// is irrelevant — what matters is only that resolution SUCCEEDS.
+pub const SECRET_PLACEHOLDER: &str = "e2e-placeholder";
+
+/// Every environment variable `config` names, whether through a `{ env: NAME }` secret reference or
+/// through a legacy `*_env: NAME` field, in first-seen order and without duplicates.
+pub fn referenced_env_vars(config: &str) -> Vec<String> {
+    let mut names = Vec::new();
+    for (idx, _) in config.match_indices("env:") {
+        let name: String = config[idx + "env:".len()..]
+            .trim_start_matches([' ', '\t'])
+            .chars()
+            .take_while(|c| c.is_ascii_alphanumeric() || *c == '_')
+            .collect();
+        // A bare `env:` with nothing usable after it (a line break, or the key of a nested mapping)
+        // names no variable at all; skip it rather than exporting an empty-named one.
+        if !name.is_empty() && !names.contains(&name) {
+            names.push(name);
+        }
+    }
+    names
+}
+
+/// Sets [`SECRET_PLACEHOLDER`] for every environment variable `config` references.
+///
+/// Call this BEFORE any `.env()` carrying a value the test actually asserts on (an admin token, a
+/// signing key): the later `.env()` wins, so the placeholder only ever fills in the secrets whose
+/// value the test does not care about.
+pub fn apply_placeholder_secrets(cmd: &mut Command, config: &str) {
+    for name in referenced_env_vars(config) {
+        cmd.env(name, SECRET_PLACEHOLDER);
+    }
+}
+
+/// [`apply_placeholder_secrets`] over the files busbar will actually read.
+///
+/// Reading the files back, rather than the strings the test built them from, is what makes this
+/// impossible to get out of step: whatever landed on disk is exactly what busbar resolves.
+pub fn apply_placeholder_secrets_from_files(cmd: &mut Command, paths: &[&Path]) {
+    for path in paths {
+        let text = std::fs::read_to_string(path).unwrap_or_else(|e| {
+            panic!(
+                "read config fixture {} for its secrets: {e}",
+                path.display()
+            )
+        });
+        apply_placeholder_secrets(cmd, &text);
+    }
+}

store-postgres-plugin/tests/config_secrets.rs

@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright (C) 2026 Busbar Inc and contributors
+
+//! Coverage for `tests/common`'s secret-reference scan.
+//!
+//! That scan is what keeps the real-binary e2e tests bootable, and when it misses a reference the
+//! symptom surfaces a long way from here — as `busbar` exiting 1 before its listener comes up —
+//! so it is worth testing directly rather than only through the e2e tests it supports.
+
+mod common;
+
+use common::referenced_env_vars;
+
+#[test]
+fn finds_secret_references_and_legacy_env_fields() {
+    let config = "\
+providers:\n  mock:\n    api_key: { env: MOCK_KEY }\n\
+identity-providers:\n  admin-tokens: { module: admin-tokens, token: { env: BUSBAR_ADMIN_TOKEN } }\n\
+auth:\n  signing_key: { env: BUSBAR_SIGNING_KEY }\n";
+    assert_eq!(
+        referenced_env_vars(config),
+        vec!["MOCK_KEY", "BUSBAR_ADMIN_TOKEN", "BUSBAR_SIGNING_KEY"],
+    );
+
+    // The flat providers catalog still names variables through the legacy `*_env:` field rather
+    // than a secret reference, and busbar resolves those too, so the same scan has to see them.
+    assert_eq!(
+        referenced_env_vars("mock:\n  api_key_env: MOCK_KEY\n"),
+        vec!["MOCK_KEY"],
+    );
+}
+
+#[test]
+fn deduplicates_and_ignores_references_that_name_nothing() {
+    assert_eq!(
+        referenced_env_vars("a: { env: SAME }\nb: { env: SAME }\n"),
+        vec!["SAME"],
+        "a variable referenced twice must be exported once, not twice"
+    );
+
+    // A bare `env:` opening a nested mapping, or ending a line, names no variable; exporting an
+    // empty-named one would be nonsense and on some platforms an outright error.
+    assert!(referenced_env_vars("a: { env:\n  b: c }\n").is_empty());
+    assert!(referenced_env_vars("listen: \"127.0.0.1:0\"\nauth:\n  chain: []\n").is_empty());
+}

store-postgres-plugin/tests/e2e.rs

@@ -38,6 +38,8 @@
 //! install work," and converting them to a full process-boot-and-capture-stderr harness for each
 //! error shape is a much larger, lower-value lift than the persistence test's conversion.
 
+mod common;
+
 use busbar_store_postgres::PostgresStore;
 use std::path::PathBuf;
 use std::process::{Child, Command, Stdio};
@@ -74,9 +76,8 @@ fn postgres_url() -> Option<String> {
 /// Checks BOTH the "uplifted" `<profile_dir>/<name>` copy (only refreshed when `[lib]` is a ROOT
 /// build target of the invocation, e.g. `cargo build --all-targets`) and the raw
 /// `<profile_dir>/deps/<name>` compiler output (refreshed on every build that recompiles the lib,
-/// uplifted or not). A bare `cargo test --release` (what `release-check.sh`'s Phase 2 runs, and what
-/// cargo-mutants runs) does NOT uplift the cdylib to the top-level profile dir, only to
-/// `target/deps` — checking only `profile_dir` silently finds nothing. Same fix already applied to
+/// uplifted or not). A bare `cargo test --release` (what `release-check.sh`'s Phase 2 runs) does
+/// NOT uplift the cdylib to the top-level profile dir, only to `target/deps` — checking only `profile_dir` silently finds nothing. Same fix already applied to
 /// this crate's sibling `admin_api_e2e.rs` and to auth-oidc-plugin's/webrequest-hook's equivalent
 /// helpers.
 fn plugin_path() -> Option<PathBuf> {
@@ -245,7 +246,9 @@ fn load_and_exercise_postgres_plugin_via_file_drop() {
     // file-dropped plugin passes the trust/manifest gate; then a REAL BOOT (no `--validate` flag,
     // below) is the only thing that actually `dlopen`s the plugin and runs
     // `Store::connect`/migration.
-    let out = Command::new(&busbar_bin)
+    let mut validate = Command::new(&busbar_bin);
+    common::apply_placeholder_secrets_from_files(&mut validate, &[&config, &providers]);
+    let out = validate
         .arg("--validate")
         .env("BUSBAR_CONFIG", &config)
         .env("BUSBAR_PROVIDERS", &providers)
@@ -263,7 +266,9 @@ fn load_and_exercise_postgres_plugin_via_file_drop() {
     // PostgresStore::connect, so this check can't accidentally create the schema itself -- for the
     // `keys` table to appear. This is the only genuine proof that boot actually dlopened the plugin
     // and called Store::connect (which runs migrate()) before ever handling a request.
-    let child = Command::new(&busbar_bin)
+    let mut boot = Command::new(&busbar_bin);
+    common::apply_placeholder_secrets_from_files(&mut boot, &[&config, &providers]);
+    let child = boot
         .env("BUSBAR_CONFIG", &config)
         .env("BUSBAR_PROVIDERS", &providers)
         .env("BUSBAR_STATE_FILE", "") // disable the state-snapshot file; not under test here

store-postgres/src/lib.rs

@@ -61,7 +61,7 @@ fn is_undefined_table(e: &postgres::Error) -> bool {
     e.code() == Some(&postgres::error::SqlState::UNDEFINED_TABLE)
 }
 
-/// Extract the PASSWORD from a Postgres DSN (L2). Supports both the URL form
+/// Extract the PASSWORD from a Postgres DSN. Supports both the URL form
 /// (`postgres://user:pass@host:5432/db`) and the libpq keyword form (`... password=secret ...`), so
 /// a connect-error string can be scrubbed of the secret regardless of which shape the operator used.
 fn dsn_password(dsn: &str) -> Option<String> {

store-postgres/src/tests.rs

@@ -115,8 +115,8 @@ fn connect_store_with_retry(url: &str) -> StoreResult<PostgresStore> {
 
 /// TRUE reset for test isolation -- unlike `delete_key` (a deliberate tombstone that can never fully
 /// reset a row by design), this raw-SQL wipe gives each test a genuinely clean slate for its id, so
-/// re-running the suite (or running it twice, as red-before-green proofs do) never sees stale state
-/// from a prior run leaking into the CHECK constraints (e.g. re-minting into a row still marked
+/// re-running the suite (including running it twice in a row) never sees stale state from a prior
+/// run leaking into the CHECK constraints (e.g. re-minting into a row still marked
 /// `deleted_at` from a previous run's tombstone would violate `keys_tombstone_disabled`).
 fn hard_reset(store: &PostgresStore, id: &str) {
     let mut client = store.lock();
@@ -214,10 +214,9 @@ fn delete_key_is_a_tombstone_not_a_hard_delete() {
 
     store.delete_key(id).unwrap();
 
-    // RED-BEFORE-GREEN evidence lives in the assertions below: this test only passes if delete_key
-    // genuinely tombstones rather than hard-deletes. Confirmed by temporarily reverting delete_key
-    // to `DELETE FROM keys WHERE id=$1` during development: get_key(id) then returned None and this
-    // test failed at the very first assertion, exactly as expected.
+    // The assertions below are non-vacuous: they pass only if delete_key genuinely tombstones
+    // rather than hard-deletes. Were delete_key a plain `DELETE FROM keys WHERE id=$1`, get_key(id)
+    // would return None and the very first assertion below would fail.
     let after = store.get_key(id).unwrap();
     assert!(
         after.is_some(),
@@ -248,11 +247,10 @@ fn delete_key_is_a_tombstone_not_a_hard_delete() {
 /// that was previously deleted (deleted_at set, enabled=false by delete_key) must produce a fully
 /// LIVE key, not one that is enabled=true while still marked deleted_at.
 ///
-/// RED-BEFORE-GREEN: reverting the ON CONFLICT UPDATE SET clause to omit `deleted_at=NULL` (its
-/// state before this fix) makes this test fail at the `deleted_at` assertion below: `enabled` flips
-/// to `true` as the caller intended, but `deleted_at` is left at whatever the tombstone set it to,
-/// so the row is simultaneously "enabled" and "deleted" -- exactly the corrupt state this test
-/// guards against. Confirmed by temporarily reverting the fix and re-running: this assertion failed.
+/// The `deleted_at` assertion below is the load-bearing one: an ON CONFLICT UPDATE SET clause that
+/// omits `deleted_at=NULL` flips `enabled` to `true` as the caller intended but leaves `deleted_at`
+/// at whatever the tombstone set it to, so the row is simultaneously "enabled" and "deleted" --
+/// exactly the corrupt state this test rejects.
 #[test]
 fn put_key_with_credential_on_conflict_clears_a_stale_tombstone() {
     let Some(url) = live_url() else { return };
@@ -366,11 +364,10 @@ fn credential_slot_guard_rejects_clobbering_a_live_credential() {
 /// put_credential_tx must bind CredentialMeta::updated_at to its own column, not silently reuse
 /// created_at's parameter for both.
 ///
-/// RED-BEFORE-GREEN: reverting the fix (VALUES ...,$8,$8,$9,... binding created_at's placeholder
-/// twice, with `updated_at` never bound at all) makes this test fail: the round-tripped
-/// `updated_at` comes back equal to `created_at` (100) instead of the distinct value (200) this
-/// test mints with. Confirmed by temporarily reverting and re-running: assertion failed with
-/// `left: 100, right: 200`.
+/// The test is non-vacuous: were the INSERT to bind created_at's placeholder twice
+/// (VALUES ...,$8,$8,$9,...) and never bind `updated_at` at all, the round-tripped `updated_at`
+/// would come back equal to `created_at` (100) instead of the distinct value (200) this test mints
+/// with, and the assertion below would fail with `left: 100, right: 200`.
 #[test]
 fn put_credential_binds_updated_at_to_its_own_column_not_created_at() {
     let Some(url) = live_url() else { return };
@@ -574,11 +571,10 @@ fn get_usage_transaction_is_actually_repeatable_read() {
 /// between the two steps on a SEPARATE connection, then asserts REPEATABLE READ's snapshot held: the
 /// second read still sees the pre-interleave state, not the concurrent writer's new model row.
 ///
-/// RED-BEFORE-GREEN: this test is a genuine regression guard rather than a fresh finding (get_usage
-/// already opens REPEATABLE READ via snapshot_consistent_tx) -- confirmed non-vacuous by temporarily
-/// downgrading snapshot_consistent_tx's isolation level to READ COMMITTED and re-running: the
-/// `model_count` assertion below failed (it observed the interleaved writer's new model row), then
-/// passed again after restoring REPEATABLE READ.
+/// This is a regression guard on an isolation level get_usage already opens (REPEATABLE READ, via
+/// snapshot_consistent_tx). It is non-vacuous: at READ COMMITTED the `model_count` assertion below
+/// observes the interleaved writer's new model row and fails; only REPEATABLE READ holds the
+/// snapshot across both reads.
 #[test]
 fn get_usage_snapshot_does_not_observe_a_concurrent_add_usage_between_its_two_reads() {
     let Some(url) = live_url() else { return };
@@ -719,9 +715,9 @@ fn metering_roundtrip_new_fields() {
 }
 
 // ---------------------------------------------------------------------------------------------
-// Mutation-testing gap fixes (cargo-mutants round 1 against this file): each test below is named
-// for, and directly targets, one or more MISSED mutants -- confirmed red against the mutant before
-// being folded in green here.
+// Boundary and arithmetic guards for this file: each test below is named for, and directly
+// targets, one specific predicate or binding whose behaviour was not otherwise pinned. Each fails
+// if that predicate is perturbed in the way its name describes.
 // ---------------------------------------------------------------------------------------------
 
 /// `percent_decode`'s length guard and hi/lo-nibble arithmetic, pinned with cases the existing
@@ -1265,10 +1261,9 @@ fn migrate_v6_does_not_rerun_the_backfill_on_an_already_migrated_database() {
 /// unchanged in both cases -- `migrate_locked` unconditionally runs `CREATE TABLE IF NOT EXISTS
 /// busbar_schema` as its very first statement, so by the time the guarded `SELECT` runs, the table
 /// either already exists (guard never fires) or the preceding `CREATE TABLE` itself already
-/// propagated the error several lines earlier (guard never reached). This is a confirmed equivalent
-/// mutant / dead branch given the current code structure, not a test-coverage gap; left unfixed
-/// per policy (no test written to "kill" it, since none can, without changing the source itself --
-/// out of scope for a mutation-testing coverage pass).
+/// propagated the error several lines earlier (guard never reached). Given the current statement
+/// order the guarded branch is therefore unreachable, so no test can drive it without changing the
+/// source itself; the test below pins the reachable half of the behaviour instead.
 #[test]
 fn migrate_propagates_a_non_undefined_table_error_and_never_silently_succeeds() {
     let Some(url) = live_url() else { return };