From 7c03aa51531f4b7a5840e9513fb3eea58648db3d Mon Sep 17 00:00:00 2001 From: Matthew Jackson Date: Sat, 8 Aug 2026 16:00:57 -0700 Subject: [PATCH 1/3] ci: make public-hygiene-lint green The public-hygiene gate flags text in customer-readable files that describes how the software was BUILT rather than what it DOES. Every hit in this repo is rewritten to state the behaviour or the invariant; no allow markers are added, so the gate keeps its teeth. --- .github/scripts/next-version.sh | 2 +- .github/workflows/release-selftest.yml | 2 +- store-sqlite-plugin/tests/e2e.rs | 2 +- store-sqlite/src/tests.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/scripts/next-version.sh b/.github/scripts/next-version.sh index 27fce0e..a0c3758 100755 --- a/.github/scripts/next-version.sh +++ b/.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.." to stdout. +# release automation can't silently rot. Prints "v.." to stdout. # # Inputs (env, all optional): # INPUT_VERSION explicit version to cut (leading "v" tolerated) -> used verbatim. diff --git a/.github/workflows/release-selftest.yml b/.github/workflows/release-selftest.yml index a6aafb6..9cfa502 100644 --- a/.github/workflows/release-selftest.yml +++ b/.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. diff --git a/store-sqlite-plugin/tests/e2e.rs b/store-sqlite-plugin/tests/e2e.rs index 6db19d7..18f85ca 100644 --- a/store-sqlite-plugin/tests/e2e.rs +++ b/store-sqlite-plugin/tests/e2e.rs @@ -396,7 +396,7 @@ fn wait_for_admin_ready( } } -/// THE REAL "PROD READY" PROOF (Matthew's bar): the sqlite plugin installed the way a real operator +/// THE REAL "PROD READY" PROOF: the sqlite plugin installed the way a real operator /// actually installs it — a `POST /api/v1/admin/plugins` call against a REAL running busbar admin /// listener, never file-drop, never a direct `load_store()`/loader call — then EXERCISED through /// that same live instance's own admin API (mint a virtual key + an attached AWS SigV4 credential diff --git a/store-sqlite/src/tests.rs b/store-sqlite/src/tests.rs index ba0906c..98e3550 100644 --- a/store-sqlite/src/tests.rs +++ b/store-sqlite/src/tests.rs @@ -751,7 +751,7 @@ fn foreign_keys_cascade_is_real_not_just_the_app_level_delete() { ); } -// ── Mutation-testing regressions (cargo-mutants round 1, store-sqlite/src/lib.rs) ────────────── +// ── Regression guards for `store-sqlite/src/lib.rs` predicate boundaries ──────────────────────── #[test] fn is_memory_path_rejects_a_plain_file_path() { From c2b49f42eabb7195c91e448051c4b630ae563cc0 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sat, 8 Aug 2026 19:14:33 -0700 Subject: [PATCH 2/3] tests: supply every secret the e2e fixtures reference, and a signing key Two independent reasons the real-binary e2e tests could not boot busbar, both of them core changes the fixtures never caught up with. 1. Secret references are RESOLVED now, at `--validate` and at boot, and busbar exits non-zero when one cannot resolve. The file-drop fixture named `providers.mock.api_key: { env: MOCK_KEY }` and nobody exported MOCK_KEY: [error] providers.mock.api_key: secret env:MOCK_KEY cannot resolve: environment variable 'MOCK_KEY' is unset which surfaced in CI only as `busbar --validate must succeed with the file-dropped sqlite plugin` and a panic 200 lines from the YAML at fault. Fixed by deriving the variable names FROM the config files busbar is about to read, rather than listing them at the call site: the next person to add a secret reference to one of these fixtures must not also have to remember to export it, because forgetting produces a failure that never names the variable. `tests/common/mod.rs` holds the scan, `tests/config_secrets.rs` covers it directly. 2. The admin-API fixture set no `auth.signing_key`, and busbar no longer auto-generates one, so the mint it asserts on came back 409: {"error":{"code":"conflict","message":"signed-token minting is unavailable: no signing key is configured (set auth.signing_key - busbar no longer auto-generates one; run `busbar --generate-signing-key`)"}} The assertion printed only `left: 409, right: 201`, so the reason was invisible; it now carries the response body, which is how the above was read. Red-before-green, against a `busbar` built from the core branch CI tests against: before, `install_sqlite_plugin_via_admin_api_and_verify_persistence` and `load_and_exercise_sqlite_plugin_via_file_drop` both FAILED; after, `test result: ok. 3 passed; 0 failed`. --- store-sqlite-plugin/tests/common/mod.rs | 74 +++++++++++++++++++++ store-sqlite-plugin/tests/config_secrets.rs | 45 +++++++++++++ store-sqlite-plugin/tests/e2e.rs | 63 +++++++++++------- 3 files changed, 159 insertions(+), 23 deletions(-) create mode 100644 store-sqlite-plugin/tests/common/mod.rs create mode 100644 store-sqlite-plugin/tests/config_secrets.rs diff --git a/store-sqlite-plugin/tests/common/mod.rs b/store-sqlite-plugin/tests/common/mod.rs new file mode 100644 index 0000000..4691343 --- /dev/null +++ b/store-sqlite-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 { + 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); + } +} diff --git a/store-sqlite-plugin/tests/config_secrets.rs b/store-sqlite-plugin/tests/config_secrets.rs new file mode 100644 index 0000000..85a2075 --- /dev/null +++ b/store-sqlite-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()); +} diff --git a/store-sqlite-plugin/tests/e2e.rs b/store-sqlite-plugin/tests/e2e.rs index e372b89..ee84230 100644 --- a/store-sqlite-plugin/tests/e2e.rs +++ b/store-sqlite-plugin/tests/e2e.rs @@ -44,6 +44,8 @@ //! /api/v1/admin/keys`, and independently verifies both landed in the real on-disk file with a //! second `SqliteStore::open` that never touches the plugin/ABI/admin-API/loader. +mod common; + use busbar_api::{ModelTokens, Store, TierTokens, UsageLedger}; use busbar_store_sqlite::SqliteStore; use std::path::PathBuf; @@ -200,20 +202,17 @@ fn load_and_exercise_sqlite_plugin_via_file_drop() { "mock:\n protocol: anthropic\n base_url: \"http://127.0.0.1:9\"\n api_key_env: MOCK_KEY\n", ) .unwrap(); - std::fs::write( - &config, - format!( - "listen: \"127.0.0.1:0\"\n\ - store:\n module: sqlite\n settings: {{ db_path: \"{}\" }}\n\ - plugins:\n enabled: true\n dir: {}\n trust:\n allow_unsigned: true\n\ - auth:\n chain: []\n\ - providers:\n mock:\n api_key: {{ env: MOCK_KEY }}\n\ - models:\n test-model:\n provider: mock\n", - db_path.display(), - plugins_dir.display() - ), - ) - .unwrap(); + let config_text = format!( + "listen: \"127.0.0.1:0\"\n\ + store:\n module: sqlite\n settings: {{ db_path: \"{}\" }}\n\ + plugins:\n enabled: true\n dir: {}\n trust:\n allow_unsigned: true\n\ + auth:\n chain: []\n\ + providers:\n mock:\n api_key: {{ env: MOCK_KEY }}\n\ + models:\n test-model:\n provider: mock\n", + db_path.display(), + plugins_dir.display() + ); + std::fs::write(&config, &config_text).unwrap(); // `--validate` is DELIBERATELY not used for the load-proof itself: it is manifest-only by // design ("no server, no network, no state, no dlopen" -- crates/busbar/src/main.rs's own @@ -224,7 +223,9 @@ fn load_and_exercise_sqlite_plugin_via_file_drop() { // plugin passes the trust/manifest gate; then a REAL BOOT (no `--validate` flag) is the only // thing that actually `dlopen`s the plugin and runs `Store::open`/migration, so that's what // proves the persistence claim. - 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) @@ -241,7 +242,9 @@ fn load_and_exercise_sqlite_plugin_via_file_drop() { // plugin + config, and poll for the real sqlite file to appear -- the only genuine proof that // boot actually dlopened the plugin and called Store::open (which creates/migrates the file) // before ever handling a request. - let mut child = Command::new(&busbar_bin) + let mut boot = Command::new(&busbar_bin); + common::apply_placeholder_secrets_from_files(&mut boot, &[&config, &providers]); + let mut child = boot .env("BUSBAR_CONFIG", &config) .env("BUSBAR_PROVIDERS", &providers) .env("BUSBAR_STATE_FILE", "") // disable the state-snapshot file; not under test here @@ -423,6 +426,10 @@ fn install_sqlite_plugin_via_admin_api_and_verify_persistence() { std::fs::create_dir_all(&plugins_dir).unwrap(); let db_path = work.join("governance.db"); const ADMIN_TOKEN: &str = "e2e-admin-api-token"; + // Fixed ed25519 signing secret (64 hex = 32 bytes). busbar no longer auto-generates one, and + // without it the mint below is refused with a `conflict` ("signed-token minting is unavailable: + // no signing key is configured") rather than anything that points at the config. + const SIGNING_KEY: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; let providers = work.join("providers.yaml"); std::fs::write( @@ -444,7 +451,7 @@ fn install_sqlite_plugin_via_admin_api_and_verify_persistence() { {store_yaml}\n\ plugins:\n enabled: true\n dir: {}\n trust:\n allow_unsigned: true\n\ identity-providers:\n admin-tokens: {{ module: admin-tokens, token: {{ env: BUSBAR_ADMIN_TOKEN }} }}\n\ - auth:\n chain: []\n admin_auth: [admin-tokens]\n\ + auth:\n chain: []\n signing_key: {{ env: BUSBAR_SIGNING_KEY }}\n admin_auth: [admin-tokens]\n\ providers:\n mock:\n api_key: {{ env: MOCK_KEY }}\n\ models:\n test-model:\n provider: mock\n", plugins_dir.display() @@ -476,10 +483,13 @@ fn install_sqlite_plugin_via_admin_api_and_verify_persistence() { }; let admin_addr1 = format!("127.0.0.1:{admin_addr1}"); - let mut child1 = Command::new(&busbar_bin) + let mut boot1 = Command::new(&busbar_bin); + common::apply_placeholder_secrets_from_files(&mut boot1, &[&config1, &providers]); + let mut child1 = boot1 .env("BUSBAR_CONFIG", &config1) .env("BUSBAR_PROVIDERS", &providers) .env("BUSBAR_ADMIN_TOKEN", ADMIN_TOKEN) + .env("BUSBAR_SIGNING_KEY", SIGNING_KEY) .env("BUSBAR_STATE_FILE", "") .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) @@ -562,10 +572,13 @@ fn install_sqlite_plugin_via_admin_api_and_verify_persistence() { }; let admin_addr2 = format!("127.0.0.1:{admin_addr2}"); - let mut child2 = Command::new(&busbar_bin) + let mut boot2 = Command::new(&busbar_bin); + common::apply_placeholder_secrets_from_files(&mut boot2, &[&config2, &providers]); + let mut child2 = boot2 .env("BUSBAR_CONFIG", &config2) .env("BUSBAR_PROVIDERS", &providers) .env("BUSBAR_ADMIN_TOKEN", ADMIN_TOKEN) + .env("BUSBAR_SIGNING_KEY", SIGNING_KEY) .env("BUSBAR_STATE_FILE", "") .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) @@ -592,12 +605,16 @@ fn install_sqlite_plugin_via_admin_api_and_verify_persistence() { })) .send() .expect("POST /api/v1/admin/keys"); + // Carry the response body into the failure message: a bare status number says nothing about + // WHY the mint was refused, and this assertion has already once cost a debugging round-trip. + let mint_status = mint_resp.status().as_u16(); + let mint_body = mint_resp.text().unwrap_or_default(); assert_eq!( - mint_resp.status().as_u16(), - 201, - "minting a key + AWS credential through the live sqlite-backed instance must succeed" + mint_status, 201, + "minting a key + AWS credential through the live sqlite-backed instance must succeed: \ + {mint_body}" ); - let minted: serde_json::Value = mint_resp.json().unwrap(); + let minted: serde_json::Value = serde_json::from_str(&mint_body).unwrap(); let key_id = minted["id"].as_str().expect("minted key id").to_string(); let access_key_id = minted["aws_access_key_id"] .as_str() From d77ff519f8df95e7106e92c4395dc78f68d50c26 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sat, 8 Aug 2026 19:36:03 -0700 Subject: [PATCH 3/3] tests: stop the plugin test silently skipping itself on a developer machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `plugin_path()` looked only at the "uplifted" `/` copy of the cdylib. A bare `cargo test` does not uplift it — only `cargo build` does — so on a developer machine the lookup returned None and all three tests returned early with a cheerful green: running 3 tests test load_and_exercise_sqlite_plugin_via_file_drop ... ok test result: ok. 3 passed; ... finished in 0.00s Nothing ran. That 0.00s is the tell, and it is easy to miss. CI is protected by the `is_none() && CI` panic below, so this never shipped a false green there, but it does mean the only over-the-ABI coverage of this plugin was routinely not running locally — which is where it is most useful. Now checks both the uplifted path and the raw `deps/` compiler output, preferring whichever is newer. store-mysql's `admin_api_e2e.rs` already had exactly this fix; its own `e2e.rs` did not, which is the drift this closes. --- store-sqlite-plugin/tests/e2e.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/store-sqlite-plugin/tests/e2e.rs b/store-sqlite-plugin/tests/e2e.rs index ee84230..83aed5f 100644 --- a/store-sqlite-plugin/tests/e2e.rs +++ b/store-sqlite-plugin/tests/e2e.rs @@ -90,8 +90,22 @@ fn plugin_path() -> Option { let exe = std::env::current_exe().ok()?; // .../target//deps/e2e- let profile_dir = exe.parent()?.parent()?; // .../target/ let name = busbar_plugin_loader::plugin_library_filename("busbar_store_sqlite_plugin"); - let candidate = profile_dir.join(&name); - candidate.exists().then_some(candidate) + // BOTH the "uplifted" `/` copy and the raw `/deps/` compiler + // output: a bare `cargo test` does not uplift the cdylib, only `cargo build` does, so + // checking the uplifted path alone made this test silently skip itself on a developer + // machine — the exact coverage it exists to provide, gone, with a green result. + let uplifted = profile_dir.join(&name); + let raw = profile_dir.join("deps").join(&name); + [uplifted, raw] + .into_iter() + .filter_map(|p| { + std::fs::metadata(&p) + .and_then(|m| m.modified()) + .ok() + .map(|mtime| (p, mtime)) + }) + .max_by_key(|(_, mtime)| *mtime) + .map(|(p, _)| p) })(); if candidate.is_none() && std::env::var_os("CI").is_some() { panic!(