From 3bfe494c9bba0514321b2106ace66c9be213bea4 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Thu, 15 Oct 2026 05:45:16 -0700 Subject: [PATCH 1/2] github: hold its own credentials in a private Held, off the contract's sealed Secret busbar #1021 renames the contract's Redacted to a SEALED Secret: its raw accessor takes a kernel seal and expose_secret is gone; a plugin holds no seal. The access token, the OAuth nonce and the client secret this plugin keeps across a login flow are its own memory, so they move to a crate-private Held (src/held.rs) with the former wrapper's semantics: Debug and Display print [REDACTED], no serde, zeroize on drop, constant-time PartialEq, Clone, and one pub(crate) reveal(). Same pattern as auth-oauth #16. The plugin no longer names Redacted or expose_secret and compiles against the contract before and after #1021. constant_time_eq is already imported from the contract root. --- busbar-auth-github/src/held.rs | 97 +++++++++++++++++++++++++++++++++ busbar-auth-github/src/lib.rs | 12 ++-- busbar-auth-github/src/login.rs | 22 ++++---- 3 files changed, 113 insertions(+), 18 deletions(-) create mode 100644 busbar-auth-github/src/held.rs diff --git a/busbar-auth-github/src/held.rs b/busbar-auth-github/src/held.rs new file mode 100644 index 0000000..54dbe41 --- /dev/null +++ b/busbar-auth-github/src/held.rs @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Busbar Inc and contributors + +//! [`Held`] — a credential THIS PLUGIN holds in its own memory: the GitHub access token it receives +//! from the token endpoint, and the OAuth `state` nonce / confidential-client secret it keeps for the +//! duration of a login flow. +//! +//! These are the plugin's own material, not a value the kernel resolved and lent it, so they are not +//! the contract's sealed `Secret` (busbar #1021: that type's one raw accessor takes a kernel seal, +//! which a plugin neither holds nor can obtain). `Held` keeps exactly the hygiene the plugin relied on +//! when it wrapped them in the contract's former redaction wrapper: +//! +//! * **`Debug` / `Display` never reveal it.** Both print the literal `"[REDACTED]"`. +//! * **It does not serialize.** `Held` implements neither `Serialize` nor `Deserialize`. +//! * **It zeroizes its backing memory on drop.** +//! * **Equality is constant-time** (the contract's `constant_time_eq` over the byte form). +//! +//! The value is reached ONLY through [`Held::reveal`]: every call site is an audit point where the +//! plaintext escapes on purpose. + +use core::fmt; + +use zeroize::Zeroize; + +/// A credential this plugin holds. `Debug`/`Display` print `"[REDACTED]"`; it never serializes; its +/// backing memory is zeroized on drop. See the module docs. +pub(crate) struct Held(T); + +impl Held { + /// Hold `secret`. + pub(crate) fn new(secret: T) -> Self { + Self(secret) + } + + /// Borrow the held value. AUDIT POINT: the plaintext escapes redaction here. + pub(crate) fn reveal(&self) -> &T { + &self.0 + } +} + +/// `Debug` NEVER reveals the value. +impl fmt::Debug for Held { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("[REDACTED]") + } +} + +/// `Display` NEVER reveals the value either. +impl fmt::Display for Held { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("[REDACTED]") + } +} + +impl Drop for Held { + fn drop(&mut self) { + self.0.zeroize(); + } +} + +impl Clone for Held { + fn clone(&self) -> Self { + Self(self.0.clone()) + } +} + +/// Equality on two held values is CONSTANT-TIME (the contract's `constant_time_eq`), never a +/// byte-by-byte compare with a data-dependent early exit. +impl> PartialEq for Held { + fn eq(&self, other: &Self) -> bool { + busbar_contract::constant_time_eq(&self.0, &other.0) + } +} + +impl> Eq for Held {} + +#[cfg(test)] +mod tests { + use super::Held; + + #[test] + fn debug_and_display_print_only_the_redaction_marker() { + let h = Held::new(String::from("sk-live-plaintext")); + assert_eq!(format!("{h:?}"), "[REDACTED]"); + assert_eq!(format!("{h}"), "[REDACTED]"); + assert_eq!(format!("{:?}", Some(&h)), "Some([REDACTED])"); + assert_eq!(h.reveal(), "sk-live-plaintext"); + } + + #[test] + fn equality_compares_the_held_bytes() { + let a = Held::new(String::from("same")); + assert_eq!(a, a.clone()); + assert_ne!(a, Held::new(String::from("other"))); + assert_ne!(a, Held::new(String::from("sam"))); + } +} diff --git a/busbar-auth-github/src/lib.rs b/busbar-auth-github/src/lib.rs index bdb96a3..aa82c4c 100644 --- a/busbar-auth-github/src/lib.rs +++ b/busbar-auth-github/src/lib.rs @@ -47,13 +47,14 @@ pub mod door; pub mod guard; +mod held; pub mod login; +use crate::held::Held; use busbar_contract::auth::{ AuthModule, AuthVerdict, BeginLogin, CompleteLogin, LoginHop, LoginHttpResponse, LoginModule, LoginOutcome, Principal, }; -use busbar_contract::Redacted; use serde::Deserialize; use serde_json::Value; use std::collections::HashMap; @@ -480,7 +481,7 @@ const PENDING_MAX: usize = 4096; struct PendingLogin { /// The opaque GitHub bearer, wrapped so a stray `{:?}` on this struct, or on the map that holds /// it, cannot print it. Nothing formats these today; this is to keep it that way. - access_token: Redacted, + access_token: Held, user: Option, /// When this entry was stashed, for the TTL sweep. inserted_at: Instant, @@ -629,7 +630,7 @@ impl GithubModule { pending.insert( key.to_string(), PendingLogin { - access_token: Redacted::new(token), + access_token: Held::new(token), user: None, inserted_at: Instant::now(), }, @@ -674,10 +675,7 @@ impl GithubModule { entry.user = Some(user); // The `/user/orgs` GET is authenticated with the SAME opaque access token stashed at the token // step, attached via the ABI v2 `headers` field. - LoginOutcome::Exchange(build_orgs_get( - &self.cfg, - entry.access_token.expose_secret(), - )) + LoginOutcome::Exchange(build_orgs_get(&self.cfg, entry.access_token.reveal())) } /// `/user/orgs` array was fed back: pair the stashed `/user` identity with the parsed diff --git a/busbar-auth-github/src/login.rs b/busbar-auth-github/src/login.rs index 39aab6c..3b9b75b 100644 --- a/busbar-auth-github/src/login.rs +++ b/busbar-auth-github/src/login.rs @@ -30,13 +30,13 @@ //! [`LoginFlow`] per ticket and each [`Exchange`] one framed `exchange()` over the plugin's need. use crate::guard::{collect_allowed_hosts, sanitize_hop_header, vet_hop_url, Refused}; +use crate::held::Held; use crate::{ build_github_authorize_url, build_orgs_get, build_principal, build_token_exchange, build_userinfo_get, is_json_array, parse_access_token, parse_org_groups, parse_user, GhUser, GitHubConfig, }; use busbar_contract::auth::{CompleteLogin, LoginHop, LoginHttpResponse, LoginOutcome, Principal}; -use busbar_contract::Redacted; use serde_json::Value; use std::collections::HashSet; use std::fmt; @@ -122,7 +122,7 @@ pub enum LoginStep { /// The state 1.5.5 kept per flow between hops (its `PendingLogin`), owned by the flow. #[derive(Default)] struct FlowState { - access_token: Option>, + access_token: Option>, user: Option, } @@ -136,7 +136,7 @@ pub struct LoginFlow { inflight: Option, done: Option, /// The nonce the kernel minted at begin (`CompleteLoginIn.nonce`), bound to any `id_token`. - nonce: Option>, + nonce: Option>, } impl fmt::Debug for LoginFlow { @@ -153,7 +153,7 @@ impl fmt::Debug for LoginFlow { /// The GitHub login, holding its own client secret. pub struct GithubLogin { cfg: GitHubConfig, - client_secret: Option>, + client_secret: Option>, allowed_hosts: HashSet, } @@ -191,7 +191,7 @@ impl GithubLogin { cfg, client_secret: client_secret .filter(|s| !s.is_empty()) - .map(|s| Redacted::new(s.to_string())), + .map(|s| Held::new(s.to_string())), allowed_hosts: collect_allowed_hosts(&raw), }) } @@ -235,7 +235,7 @@ impl GithubLogin { turns: 0, inflight: None, done: None, - nonce: nonce.map(|n| Redacted::new(n.to_string())), + nonce: nonce.map(|n| Held::new(n.to_string())), } } @@ -276,7 +276,7 @@ impl GithubLogin { // trusted. GitHub issues no id_token, so this never fires against github.com. if !id_token_nonce_binds( &body, - flow.nonce.as_ref().map(|n| n.expose_secret().as_str()), + flow.nonce.as_ref().map(|n| n.reveal().as_str()), ) { return finish(flow, LoginStep::SecurityCheckFailed); } @@ -325,7 +325,7 @@ impl GithubLogin { if let Some(token) = parse_access_token(&resp.body) { let hop = build_userinfo_get(&self.cfg, &token); flow.state = FlowState { - access_token: Some(Redacted::new(token)), + access_token: Some(Held::new(token)), user: None, }; return LoginOutcome::Exchange(hop); @@ -351,7 +351,7 @@ impl GithubLogin { let Some(token) = &flow.state.access_token else { return LoginOutcome::Reject; }; - let hop = build_orgs_get(&self.cfg, token.expose_secret()); + let hop = build_orgs_get(&self.cfg, token.reveal()); flow.state.user = Some(user); LoginOutcome::Exchange(hop) } @@ -367,9 +367,9 @@ impl GithubLogin { match &self.client_secret { Some(secret) => { if let Some(slot) = form.iter_mut().find(|(k, _)| k == field) { - slot.1 = secret.expose_secret().clone(); + slot.1 = secret.reveal().clone(); } else { - form.push((field.to_string(), secret.expose_secret().clone())); + form.push((field.to_string(), secret.reveal().clone())); } } None => form.retain(|(k, _)| k != field), From 3bdf1deed1be199ccc6147f3a5685a5d6e150b53 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Thu, 15 Oct 2026 08:00:10 -0700 Subject: [PATCH 2/2] held: declare the zeroize dependency the private Held names --- Cargo.lock | 1 + busbar-auth-github/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 776ffc1..38949d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -65,6 +65,7 @@ dependencies = [ "busbar-contract", "serde", "serde_json", + "zeroize", ] [[package]] diff --git a/busbar-auth-github/Cargo.toml b/busbar-auth-github/Cargo.toml index 38c6d1b..f04f2e8 100644 --- a/busbar-auth-github/Cargo.toml +++ b/busbar-auth-github/Cargo.toml @@ -15,3 +15,4 @@ license = "Apache-2.0" busbar-contract = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +zeroize = { workspace = true }