From 28da5d3e788ba4ff0e5cc3523fbde963f8686b58 Mon Sep 17 00:00:00 2001 From: Matthew Jackson Date: Sat, 8 Aug 2026 16:00:58 -0700 Subject: [PATCH 1/2] 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 +- auth-oidc-plugin/tests/e2e.rs | 8 ++++---- 3 files changed, 6 insertions(+), 6 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/auth-oidc-plugin/tests/e2e.rs b/auth-oidc-plugin/tests/e2e.rs index ea09b3e..fba7bc5 100644 --- a/auth-oidc-plugin/tests/e2e.rs +++ b/auth-oidc-plugin/tests/e2e.rs @@ -22,9 +22,9 @@ use busbar_plugin_loader::{auth::load_auth_from_bytes, plugin_library_filename}; /// Checks BOTH the "uplifted" `/` copy (only refreshed when `[lib]` is a ROOT /// build target, e.g. `cargo build --all-targets`) and the raw `/deps/` compiler /// output (refreshed on every build that recompiles the lib). A bare `cargo test --release` (what -/// `release-check.sh`'s Phase 4 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 even though the cdylib really was built. Same fix already applied to +/// `release-check.sh`'s Phase 4 runs) does NOT uplift the cdylib to the top-level profile dir, only +/// to `target/deps` - checking only `profile_dir` silently finds nothing even though the cdylib +/// really was built. Same fix already applied to /// store-postgres-plugin's and webrequest-hook's equivalent `plugin_path()` helpers. fn plugin_path() -> Option { let candidate = (|| { @@ -346,7 +346,7 @@ fn wait_for_admin_ready( false } -/// THE REAL END-TO-END PROOF Matthew asked for by name: "we called oidc... for EVERY plugin." Not a +/// THE REAL END-TO-END PROOF that oidc is exercised the way an operator exercises it. Not a /// direct ABI `load_auth_from_bytes` call (the tests above already cover that seam) and not a /// file-drop — an operator installing a NEW auth plugin onto a LIVE gateway does it over the real /// Admin API (`POST /api/v1/admin/plugins`), then the auth chain picks it up on the next boot (auth From d9f95733fdd121d4734329d12a41c10798458f87 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sat, 8 Aug 2026 17:21:01 -0700 Subject: [PATCH 2/2] fix(e2e): migrate the admin-API fixture to busbar 1.5.3 config grammar, and stop discarding busbar's output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `install_oidc_plugin_via_admin_api_and_authenticate` e2e test failed on every run with `busbar exited early during admin-readiness poll: exit status: 1`. The reason was never visible because the harness spawned busbar with `Stdio::null()` on both streams, so busbar's one-line explanation went to the floor and the test reported only an exit code. The explanation was that both fixture configs still used the 1.5.0 INLINE identity-provider form (`auth.admin_auth: [{ admin-tokens: {...} }]`, `auth.chain: [{ oidc: { settings: ... } }]`). busbar 1.5.3 retired that form along with `auth.methods:`/`auth.modules:` and refuses to boot on it — and this repo's own `.busbar-ref` pins 1.5.3, so the fixture was behind the grammar this plugin ships against. Both configs now define each provider once under the top-level `identity-providers:` map and reference it by bare name, which is what an operator on 1.5.3 must write. Capturing the child's output is the fix that matters most: an early exit or a readiness timeout now reports what busbar actually said, so the next config break is a one-line diagnosis instead of an exit code. Both streams are drained on background threads rather than read at panic time, because an unread pipe fills after a page or two and would block the very boot the test is waiting on. The same retired grammar was documented in README.md and in the two crates' module docs, telling operators to write a config that no longer boots; those now show the `identity-providers:` + bare-name shape and point at `--migrate-config`. Separately, ci.yml passed `busbar_ref: ${{ github.ref_name }}`, which on a `pull_request` event is `/merge` rather than a branch name. Core's reusable workflow could not resolve it and silently fell back to `busbar@dev` on every PR, so no PR ever tested against the branch it targets. `${{ github.base_ref || github.ref_name }}` says what was meant. --- .github/workflows/ci.yml | 6 +- README.md | 37 +++++++---- auth-oidc-plugin/src/lib.rs | 5 +- auth-oidc-plugin/tests/e2e.rs | 121 +++++++++++++++++++++++++--------- auth-oidc/src/lib.rs | 10 +-- 5 files changed, 129 insertions(+), 50 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a556a3d..04bb09d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,5 +15,9 @@ jobs: plugin_crate: busbar-auth-oidc-plugin plugin_kind: auth plugin_alias: oidc - busbar_ref: ${{ github.ref_name }} # same-branch: qa builds core qa, dev builds core dev (no stale main/tag) + busbar_ref: ${{ github.base_ref || github.ref_name }} + # same-branch: qa builds core qa, dev builds core dev (no stale main/tag). base_ref FIRST: + # on a pull_request `github.ref_name` is '/merge', not a branch name, so core could + # not resolve it and every PR silently fell back to busbar@dev instead of the branch this PR + # actually targets. service: none diff --git a/README.md b/README.md index b945c68..095e161 100644 --- a/README.md +++ b/README.md @@ -30,10 +30,12 @@ explicitly in production; do not assume they move together. ## What it is for -- **Verifying who's calling**: add `oidc` to `auth.chain` with its - `settings:` pointed at an IdP (Entra ID, Okta, Auth0, Keycloak, or any - standards-compliant OIDC provider) — `auth: { chain: [{ oidc: { - settings: {...} } }] }`. Every request's bearer token is verified +- **Verifying who's calling**: define the provider once under + `identity-providers:` with its `settings:` pointed at an IdP (Entra ID, + Okta, Auth0, Keycloak, or any standards-compliant OIDC provider), then + reference it by bare name from `auth.chain` — `identity-providers: { + oidc: { module: oidc, settings: {...} } }` + `auth: { chain: [oidc] }`. + Every request's bearer token is verified against the IdP's live JWKS — the plugin never trusts an unsigned claim. - **Mapping claims to busbar identity**: the configured role claim (e.g. @@ -128,17 +130,28 @@ Drop the resulting tarball into busbar's configured `plugins.dir` and set: ```yaml +identity-providers: # define each IdP ONCE (busbar 1.5.3) + oidc: + module: oidc # this plugin, by its packed alias + settings: + issuer: "https://login.microsoftonline.com//v2.0" + audience: "" + role_claim: groups + # max_admin_scope: read-only | full — OMITTED means read-only, the + # most restrictive ceiling, which is what an external IdP wants. auth: - chain: - - oidc: - settings: - issuer: "https://login.microsoftonline.com//v2.0" - audience: "" - role_claim: groups + chain: [oidc] # reference the provider by BARE NAME + role_bindings: # role → policy, nested by provider name + oidc: + "": { group: engineering } ``` -— see [`docs/configuration.md`](https://github.com/GetBusbar/busbar/blob/main/docs/configuration.md#auth-plugins) -for the full `auth.chain` config reference. +busbar 1.5.3 retired the inline form (`auth.chain: [{ oidc: { settings: +… } }]`, `auth.methods:`, `auth.modules:`) and refuses to boot on a +config that still uses it; `busbar --migrate-config config.yaml` rewrites +an older config into the shape above. See +[`docs/configuration.md`](https://github.com/GetBusbar/busbar/blob/main/docs/configuration.md#auth-plugins) +for the full `identity-providers:` / `auth.chain` config reference. ## Config diff --git a/auth-oidc-plugin/src/lib.rs b/auth-oidc-plugin/src/lib.rs index bc2359b..362ecbd 100644 --- a/auth-oidc-plugin/src/lib.rs +++ b/auth-oidc-plugin/src/lib.rs @@ -3,8 +3,9 @@ //! The **OIDC auth module as a droppable busbar plugin** — a `cdylib` that exports the auth C ABI //! ([`busbar_plugin_abi::auth`]). Build it, drop the resulting `.so`/`.dll`/`.dylib` into the engine's -//! plugins folder, add `oidc` to `auth.chain`, and configure `auth.modules.oidc.config`; the engine -//! loads it in-process at boot over the auth ABI. +//! plugins folder, define it once under `identity-providers:` (`module: oidc` plus its `settings:`), +//! and reference that name from `auth.chain`; the engine loads it in-process at boot over the auth +//! ABI. //! //! All the OIDC logic (JWKS, JWT verification on `ring`, claim policy) lives in the `busbar-auth-oidc` //! `lib` crate (which a custom build can also link statically). Here we only adapt the engine's JSON diff --git a/auth-oidc-plugin/tests/e2e.rs b/auth-oidc-plugin/tests/e2e.rs index fba7bc5..427e1fc 100644 --- a/auth-oidc-plugin/tests/e2e.rs +++ b/auth-oidc-plugin/tests/e2e.rs @@ -320,16 +320,79 @@ fn free_port() -> u16 { .port() } +/// A spawned `busbar` whose stdout and stderr are CAPTURED rather than discarded. +/// +/// The previous harness spawned every boot with `Stdio::null()`, so when busbar refused to boot the +/// only evidence that reached the test log was `exit status: 1` — the actual reason (a config error +/// printed on stderr milliseconds before exit) was thrown on the floor, turning a one-line diagnosis +/// into an hour of guesswork. Both streams are drained on background threads instead of read at +/// panic time because a pipe holds only a page or two: leaving it unread would block the child once +/// it filled, deadlocking the very boot the test is waiting on. +struct CapturedChild { + child: std::process::Child, + output: std::sync::Arc>, +} + +impl CapturedChild { + /// Spawn `cmd` with both streams piped and drained. `what` names the boot in the panic message + /// raised when the spawn itself fails. + fn spawn(cmd: &mut std::process::Command, what: &str) -> Self { + let mut child = cmd + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap_or_else(|e| panic!("spawn {what}: {e}")); + let output = std::sync::Arc::new(std::sync::Mutex::new(String::new())); + let streams: [Box; 2] = [ + Box::new(child.stdout.take().expect("stdout was piped")), + Box::new(child.stderr.take().expect("stderr was piped")), + ]; + // Interleaving both streams into one buffer matches how an operator reads a terminal, and + // ordering between them is not something any assertion here depends on. + for stream in streams { + let output = std::sync::Arc::clone(&output); + std::thread::spawn(move || { + use std::io::BufRead; + let mut reader = std::io::BufReader::new(stream); + let mut line = String::new(); + while reader.read_line(&mut line).unwrap_or(0) > 0 { + output.lock().expect("output buffer").push_str(&line); + line.clear(); + } + }); + } + Self { child, output } + } + + /// Everything busbar has printed so far, for embedding in a failure message. + fn output(&self) -> String { + self.output.lock().expect("output buffer").clone() + } + + fn kill(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + /// Poll the admin API's `GET /api/v1/admin/plugins` until it answers (or the child exits early). +/// Either failure mode reports what busbar actually said — see `CapturedChild`'s doc comment. fn wait_for_admin_ready( client: &reqwest::blocking::Client, admin_addr: &str, admin_token: &str, - child: &mut std::process::Child, + child: &mut CapturedChild, ) -> bool { for _ in 0..150 { - if let Ok(Some(status)) = child.try_wait() { - panic!("busbar exited early during admin-readiness poll: {status}"); + if let Ok(Some(status)) = child.child.try_wait() { + // The drain threads may still be flushing the final lines the process wrote on its way + // out; a short settle beats reporting an empty reason for an early exit. + std::thread::sleep(std::time::Duration::from_millis(100)); + panic!( + "busbar exited early during admin-readiness poll: {status}\n\ + ---- busbar stdout+stderr ----\n{}\n------------------------------", + child.output() + ); } if client .get(format!( @@ -416,19 +479,19 @@ fn install_oidc_plugin_via_admin_api_and_authenticate() { .unwrap(); let admin_addr1 = format!("127.0.0.1:{admin_port1}"); - let mut child1 = std::process::Command::new(&busbar_bin) - .env("BUSBAR_CONFIG", &config1) - .env("BUSBAR_PROVIDERS", &providers) - .env("BUSBAR_ADMIN_TOKEN", ADMIN_TOKEN) - .env("MOCK_KEY", "unused-mock-provider-key") - .env("BUSBAR_STATE_FILE", "") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .spawn() - .expect("spawn boot 1 (empty auth chain, admin listener up)"); + let mut child1 = CapturedChild::spawn( + std::process::Command::new(&busbar_bin) + .env("BUSBAR_CONFIG", &config1) + .env("BUSBAR_PROVIDERS", &providers) + .env("BUSBAR_ADMIN_TOKEN", ADMIN_TOKEN) + .env("MOCK_KEY", "unused-mock-provider-key") + .env("BUSBAR_STATE_FILE", ""), + "boot 1 (empty auth chain, admin listener up)", + ); assert!( wait_for_admin_ready(&client, &admin_addr1, ADMIN_TOKEN, &mut child1), - "boot 1's admin API must become ready within 15s" + "boot 1's admin API must become ready within 15s:\n{}", + child1.output() ); // ── REAL ADMIN-API INSTALL: POST the packed auth-oidc plugin tarball to /api/v1/admin/plugins. ── @@ -476,8 +539,7 @@ fn install_oidc_plugin_via_admin_api_and_authenticate() { .expect("the just-installed auth-oidc plugin appears in the auth catalog"); assert_eq!(listed["valid"], true); - let _ = child1.kill(); - let _ = child1.wait(); + child1.kill(); // ── BOOT 2: auth.chain: [oidc], over the SAME plugins dir the admin API wrote into above. // Restart-to-apply, mirroring store's own documented mechanism. ── @@ -500,7 +562,7 @@ fn install_oidc_plugin_via_admin_api_and_authenticate() { plugins_dir.display(), cert_pem .lines() - .map(|l| format!(" {l}")) + .map(|l| format!(" {l}")) .collect::>() .join("\n"), ), @@ -509,20 +571,20 @@ fn install_oidc_plugin_via_admin_api_and_authenticate() { let admin_addr2 = format!("127.0.0.1:{admin_port2}"); let data_addr2 = format!("127.0.0.1:{data_port2}"); - let mut child2 = std::process::Command::new(&busbar_bin) - .env("BUSBAR_CONFIG", &config2) - .env("BUSBAR_PROVIDERS", &providers) - .env("BUSBAR_ADMIN_TOKEN", ADMIN_TOKEN) - .env("MOCK_KEY", "unused-mock-provider-key") - .env("BUSBAR_STATE_FILE", "") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .spawn() - .expect("spawn boot 2 (auth.chain: [oidc], picking up the admin-API-installed tarball)"); + let mut child2 = CapturedChild::spawn( + std::process::Command::new(&busbar_bin) + .env("BUSBAR_CONFIG", &config2) + .env("BUSBAR_PROVIDERS", &providers) + .env("BUSBAR_ADMIN_TOKEN", ADMIN_TOKEN) + .env("MOCK_KEY", "unused-mock-provider-key") + .env("BUSBAR_STATE_FILE", ""), + "boot 2 (auth.chain: [oidc], picking up the admin-API-installed tarball)", + ); assert!( wait_for_admin_ready(&client, &admin_addr2, ADMIN_TOKEN, &mut child2), "boot 2's admin API must become ready within 15s (proves the oidc plugin loaded, not just \ - that the process is alive, since a load failure is a boot-time die())" + that the process is alive, since a load failure is a boot-time die()):\n{}", + child2.output() ); // ── THE REAL CALL: a genuine signed bearer JWT through the live data plane. ── @@ -575,8 +637,7 @@ fn install_oidc_plugin_via_admin_api_and_authenticate() { "a token signed by the wrong key must be rejected by the live installed plugin" ); - let _ = child2.kill(); - let _ = child2.wait(); + child2.kill(); let _ = std::fs::remove_dir_all(&work); } diff --git a/auth-oidc/src/lib.rs b/auth-oidc/src/lib.rs index dcb8506..f21d41f 100644 --- a/auth-oidc/src/lib.rs +++ b/auth-oidc/src/lib.rs @@ -5,9 +5,9 @@ //! Connect JWT (ID or access token) a caller presents as its bearer credential and maps it to a //! [`busbar_api::Principal`]: verify the signature against the provider's JWKS, check `iss`/`aud`/ //! `exp`/`nbf`, and read the configured role claim (`groups` by default, or `roles` for Entra -//! app-roles) into the principal's ROLES. busbar's own `group_map:` / `auth.modules.oidc:` config -//! then resolves those roles to governance grants and admin scope — the module asserts identity -//! only, never policy. +//! app-roles) into the principal's ROLES. busbar's own `auth.role_bindings.:` config then +//! resolves those roles to governance grants and admin scope — the module asserts identity only, +//! never policy. //! //! This crate is the reusable LOGIC (usable statically). The dynamic `cdylib` that exports the auth C //! ABI is the sibling `busbar-auth-oidc-plugin` crate. @@ -73,8 +73,8 @@ const MAX_CACHE_TTL_SECS: i64 = 300; /// fine; it only needs to stay behind "now". const CLOCK_SANITY_FLOOR_UNIX: i64 = 1_767_225_600; // 2026-01-01T00:00:00Z -/// The operator's `auth.modules.oidc.config` settings, deserialized from the JSON the engine passes to -/// the plugin's `open`. +/// The operator's `identity-providers..settings:` block, deserialized from the JSON the engine +/// passes to the plugin's `open`. #[derive(Debug, Clone, Deserialize)] #[serde(deny_unknown_fields)] pub struct OidcConfig {