From 8da61dbea1cb5269478b1aa9cab3dde44b30e959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Mon, 29 Jun 2026 14:59:32 -0700 Subject: [PATCH 1/3] feat: bt auth profiles --print-jwt-token to help the gateway since it needs the token fix --json being ignored `bt auth profiles --profile name` now only shows the selected profile --- src/auth.rs | 180 ++++++++++++++++++++++++++++++++++++---- src/ui/ratatui_table.rs | 5 +- tests/functions.rs | 65 +++++++++++++++ 3 files changed, 232 insertions(+), 18 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 1d5af91..cb7c83b 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -392,7 +392,17 @@ enum AuthCommand { } #[derive(Debug, Clone, Args)] -struct AuthProfilesArgs {} +struct AuthProfilesArgs { + /// Only show the profile with this name + #[arg(long, value_name = "NAME")] + profile: Option, + + /// Read the OAuth access JWT for each shown profile from the keychain and print it in plaintext. + /// + /// WARNING: this pulls a live bearer token out of the secure credential store and writes it to stdout in plaintext. + #[arg(long = "print-jwt-token")] + print_jwt_token: bool, +} #[derive(Debug, Clone, Args)] struct AuthLoginArgs { @@ -1702,20 +1712,62 @@ fn format_login_success( } } -async fn run_profiles(base: &BaseArgs, _args: AuthProfilesArgs) -> Result<()> { +async fn run_profiles(base: &BaseArgs, args: AuthProfilesArgs) -> Result<()> { let store = load_auth_store()?; - if store.profiles.is_empty() { - println!("No saved profiles. Run `bt auth login` to create one."); + + // Filter to a single profile when --profile is given. Error out when the name doesn't match a saved profile. + let filtered: Vec<(String, AuthProfile)> = if let Some(name) = &args.profile { + match store.profiles.get(name) { + Some(profile) => vec![(name.clone(), profile.clone())], + None => { + let available: Vec = store.profiles.keys().cloned().collect(); + let suffix = if available.is_empty() { + String::new() + } else { + format!(": {}", available.join(", ")) + }; + bail!("profile '{name}' not found; run `bt auth profiles` to see available profiles{suffix}"); + } + } + } else { + store + .profiles + .iter() + .map(|(n, p)| (n.clone(), p.clone())) + .collect() + }; + + if filtered.is_empty() { + if base.json { + println!( + "{}", + serde_json::to_string(&Vec::::new())? + ); + } else { + println!("No saved profiles. Run `bt auth login` to create one."); + } return Ok(()); } - let verifications = verify_all_profiles_from_store(&store).await; + // Build a filtered view of the store so verification only touches the selected profile(s) when --profile is set. + let mut filtered_store = AuthStore::default(); + for (name, profile) in &filtered { + filtered_store + .profiles + .insert(name.clone(), profile.clone()); + } + + let verifications = verify_all_profiles_from_store(&filtered_store, args.print_jwt_token).await; + + // When the API is unreachable, fall back to showing saved profile info. + // Skip this shortcut when the user asked for JWT tokens, since the token + // comes from the local keychain and is available regardless of network. let all_network_errors = verifications .iter() .all(|v| v.status == "error" && !v.error.as_deref().unwrap_or("").contains("invalid")); - if all_network_errors { + if all_network_errors && !args.print_jwt_token { eprintln!("Could not reach Braintrust API. Showing saved profiles:"); - print_saved_profiles(&store, base.json)?; + print_saved_profiles(&filtered_store, base.json)?; return Ok(()); } @@ -1724,6 +1776,10 @@ async fn run_profiles(base: &BaseArgs, _args: AuthProfilesArgs) -> Result<()> { return Ok(()); } + if args.print_jwt_token { + eprintln!("warning: --print-jwt-token writes live bearer tokens to stdout in plaintext."); + } + for v in &verifications { let cmd_status = match v.status.as_str() { "ok" => crate::ui::CommandStatus::Success, @@ -1731,6 +1787,15 @@ async fn run_profiles(base: &BaseArgs, _args: AuthProfilesArgs) -> Result<()> { _ => crate::ui::CommandStatus::Error, }; crate::ui::print_command_status(cmd_status, &format_verification_line(v)); + + // The human-readable status line above goes to stderr; when the user + // asked for the JWT, also print the token itself to stdout so it can be + // piped into another command. Skip profiles without a loadable token + // (e.g. api_key profiles, or expired/missing oauth credentials) so + // consumers don't pick up an empty line. + if let Some(token) = v.jwt_token.as_ref() { + println!("{token}"); + } } if base.verbose { @@ -1864,6 +1929,10 @@ pub struct ProfileVerification { pub status: String, #[serde(skip_serializing_if = "Option::is_none")] pub error: Option, + /// Live OAuth access JWT, populated only when `--print-jwt-token` is set and + /// the profile has a current, readable OAuth access token. + #[serde(skip_serializing_if = "Option::is_none")] + pub jwt_token: Option, } fn build_verification( @@ -1889,10 +1958,15 @@ fn build_verification( api_key_hint, status: status_str.to_string(), error, + jwt_token: None, } } -async fn verify_profile_full(name: &str, profile: &AuthProfile) -> ProfileVerification { +async fn verify_profile_full( + name: &str, + profile: &AuthProfile, + print_jwt_token: bool, +) -> ProfileVerification { let app_url = profile.app_url.as_deref().unwrap_or(DEFAULT_APP_URL); let auth_kind = match profile.auth_kind { AuthKind::ApiKey => "api_key", @@ -1909,11 +1983,37 @@ async fn verify_profile_full(name: &str, profile: &AuthProfile) -> ProfileVerifi ) }; + // When asked to print the JWT, surface the live OAuth access token directly + // from the credential store before any network validation. We do this + // unconditionally (even if the token is expired or the API is unreachable) + // so `--print-jwt-token` can be used to retrieve a token for offline + // inspection. API-key profiles do not have a JWT, so jwt_token stays None. + let jwt_token = if print_jwt_token && profile.auth_kind == AuthKind::Oauth { + match load_credential_for_profile(name, profile) { + CredentialLoad::Found(k) => Some(k), + CredentialLoad::Missing | CredentialLoad::Expired | CredentialLoad::Error(_) => None, + } + } else { + None + }; + let credential = match load_credential_for_profile(name, profile) { CredentialLoad::Found(k) => k, - CredentialLoad::Missing => return mk(ProfileStatus::Missing, None, None), - CredentialLoad::Expired => return mk(ProfileStatus::Expired, None, None), - CredentialLoad::Error(e) => return mk(ProfileStatus::Error(e), None, None), + CredentialLoad::Missing => { + let mut v = mk(ProfileStatus::Missing, None, None); + v.jwt_token = jwt_token; + return v; + } + CredentialLoad::Expired => { + let mut v = mk(ProfileStatus::Expired, None, None); + v.jwt_token = jwt_token; + return v; + } + CredentialLoad::Error(e) => { + let mut v = mk(ProfileStatus::Error(e), None, None); + v.jwt_token = jwt_token; + return v; + } }; let (jwt_id, hint) = match profile.auth_kind { @@ -1921,7 +2021,7 @@ async fn verify_profile_full(name: &str, profile: &AuthProfile) -> ProfileVerifi AuthKind::ApiKey => (None, profile.api_key_hint.clone()), }; - match fetch_login_orgs(&credential, app_url).await { + let mut v = match fetch_login_orgs(&credential, app_url).await { Ok(_) => mk(ProfileStatus::Ok, jwt_id, hint), Err(e) => { let msg = e.to_string(); @@ -1936,15 +2036,22 @@ async fn verify_profile_full(name: &str, profile: &AuthProfile) -> ProfileVerifi }; mk(status, None, None) } + }; + if print_jwt_token && profile.auth_kind == AuthKind::Oauth { + v.jwt_token = Some(credential); } + v } -async fn verify_all_profiles_from_store(store: &AuthStore) -> Vec { +async fn verify_all_profiles_from_store( + store: &AuthStore, + print_jwt_token: bool, +) -> Vec { let mut set = tokio::task::JoinSet::new(); for (name, profile) in store.profiles.iter() { let name = name.clone(); let profile = profile.clone(); - set.spawn(async move { verify_profile_full(&name, &profile).await }); + set.spawn(async move { verify_profile_full(&name, &profile, print_jwt_token).await }); } let mut results = Vec::new(); @@ -1981,6 +2088,9 @@ fn format_verification_line(v: &ProfileVerification) -> String { } } } + if let Some(ref token) = v.jwt_token { + parts.push(format!("jwt: {token}")); + } parts.join(" — ") } @@ -4504,6 +4614,7 @@ mod tests { api_key_hint: None, status: "ok".into(), error: None, + jwt_token: None, }; assert_eq!( format_verification_line(&v), @@ -4522,6 +4633,7 @@ mod tests { api_key_hint: Some("sk-****zhJwO".into()), status: "ok".into(), error: None, + jwt_token: None, }; assert_eq!( format_verification_line(&v), @@ -4540,6 +4652,7 @@ mod tests { api_key_hint: None, status: "expired".into(), error: None, + jwt_token: None, }; assert_eq!(format_verification_line(&v), "old — oauth — token expired"); } @@ -4555,6 +4668,7 @@ mod tests { api_key_hint: None, status: "error".into(), error: Some("invalid API key".into()), + jwt_token: None, }; assert_eq!( format_verification_line(&v), @@ -4562,6 +4676,44 @@ mod tests { ); } + #[test] + fn format_verification_line_includes_jwt_token_when_present() { + let v = ProfileVerification { + name: "work".into(), + auth: "oauth".into(), + org: Some("acme".into()), + user_name: Some("Alice".into()), + user_email: Some("alice@example.com".into()), + api_key_hint: None, + status: "ok".into(), + error: None, + jwt_token: Some("eyJhbGciOiJFUzI1NiJ9.eyJzdWIiOiJhYmMifQ.sig".into()), + }; + assert_eq!( + format_verification_line(&v), + "work — oauth — org: acme — Alice (alice@example.com) — jwt: eyJhbGciOiJFUzI1NiJ9.eyJzdWIiOiJhYmMifQ.sig" + ); + } + + #[test] + fn format_verification_line_omits_jwt_token_when_none() { + let v = ProfileVerification { + name: "work".into(), + auth: "api_key".into(), + org: Some("acme".into()), + user_name: None, + user_email: None, + api_key_hint: Some("sk-****zhJwO".into()), + status: "ok".into(), + error: None, + jwt_token: None, + }; + assert_eq!( + format_verification_line(&v), + "work — api_key — org: acme — sk-****zhJwO" + ); + } + #[tokio::test] async fn oauth_callback_mode_uses_listener_only_when_input_is_disabled() { let _guard = env_test_lock().lock().await; diff --git a/src/ui/ratatui_table.rs b/src/ui/ratatui_table.rs index 37ca9e0..9082176 100644 --- a/src/ui/ratatui_table.rs +++ b/src/ui/ratatui_table.rs @@ -271,10 +271,7 @@ fn common_prefix_len(tokenized: &[Vec]) -> usize { return 0; }; let mut len = 0; - 'outer: loop { - let Some(candidate) = first.get(len) else { - break; - }; + 'outer: while let Some(candidate) = first.get(len) { for tokens in tokenized.iter().skip(1) { if tokens.get(len) != Some(candidate) { break 'outer; diff --git a/tests/functions.rs b/tests/functions.rs index 99c4d7f..424741e 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -750,6 +750,71 @@ fn auth_profiles_ignores_api_key_from_dotenv() { assert!(!stderr.contains("pass --prefer-profile or unset BRAINTRUST_API_KEY")); } +#[test] +fn auth_profiles_json_with_no_profiles_emits_empty_array() { + let cwd = tempdir().expect("create temp cwd"); + let config_dir = tempdir().expect("create temp config dir"); + + let output = auth_profiles_command(cwd.path(), config_dir.path()) + .arg("--json") + .output() + .expect("run bt auth profiles --json"); + + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); + assert_eq!(stdout, "[]"); +} + +#[test] +fn auth_profiles_profile_not_found_is_actionable() { + let cwd = tempdir().expect("create temp cwd"); + let config_dir = tempdir().expect("create temp config dir"); + + let output = auth_profiles_command(cwd.path(), config_dir.path()) + .arg("--profile") + .arg("test-profile") + .output() + .expect("run bt auth profiles --profile test-profile"); + + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("profile 'test-profile' not found")); + assert!( + stderr.contains("run `bt auth profiles` to see available profiles"), + "expected actionable hint, got: {stderr}" + ); +} + +#[test] +fn auth_profiles_with_api_key_profile_does_not_print_jwt_on_stdout() { + // An api_key profile has no JWT; --print-jwt-token must leave stdout empty + // rather than emitting the api key or an empty line. + let cwd = tempdir().expect("create temp cwd"); + let config_dir = tempdir().expect("create temp config dir"); + + // Seed an api_key profile with no stored secret so verification reports + // "missing" without touching the network or keychain. + fs::create_dir_all(config_dir.path().join("bt")).expect("create bt config dir"); + fs::write( + config_dir.path().join("bt").join("auth.json"), + r#"{"profiles":{"test-profile":{"auth_kind":"api_key","api_url":"https://api.braintrust.dev","app_url":"https://www.braintrust.dev","org_name":"test-org","user_name":null,"email":null,"api_key_hint":"sk-****test"}}}"#, + ) + .expect("write auth.json"); + + let output = auth_profiles_command(cwd.path(), config_dir.path()) + .arg("--profile") + .arg("test-profile") + .arg("--print-jwt-token") + .output() + .expect("run bt auth profiles --profile test-profile --print-jwt-token"); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.trim().is_empty(), + "expected no JWT on stdout for an api_key profile, got: {stdout}" + ); +} + #[test] fn push_and_pull_help_are_machine_readable() { let push_help = Command::new(bt_binary_path()) From a5c19398d644063bf2755060c1c0fe3d6e36ae77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Mon, 29 Jun 2026 15:50:33 -0700 Subject: [PATCH 2/3] fix: --json flag not respected by bt auth login/logout/refresh --- src/auth.rs | 187 ++++++++++++++++++++++++++++++++++++++------- src/ui/mod.rs | 2 +- src/ui/status.rs | 69 +++++++++++++++++ tests/functions.rs | 65 ++++++++++++++++ 4 files changed, 295 insertions(+), 28 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index cb7c83b..6481a64 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1264,10 +1264,17 @@ async fn run_login_set(base: &BaseArgs, args: AuthLoginArgs) -> Result<()> { selected_org.as_ref().map(|org| org.name.clone()), )?; - ui::print_command_status( - ui::CommandStatus::Success, - &format_login_success(&selected_org, &profile_name, &selected_api_url), - ); + let human = format_login_success(&selected_org, &profile_name, &selected_api_url); + let result = LoginResult { + profile: profile_name.clone(), + auth_kind: "api_key".to_string(), + org: selected_org.as_ref().map(|org| org.name.clone()), + api_url: Some(selected_api_url.clone()), + app_url: base.app_url.clone(), + status: "ok".to_string(), + error: None, + }; + ui::emit_result(base.json, ui::CommandStatus::Success, &human, &result)?; Ok(()) } @@ -1379,10 +1386,17 @@ async fn run_login_oauth(base: &BaseArgs, args: AuthLoginArgs) -> Result<()> { selected_org.as_ref().map(|org| org.name.clone()), )?; - ui::print_command_status( - ui::CommandStatus::Success, - &format_login_success(&selected_org, &profile_name, &selected_api_url), - ); + let human = format_login_success(&selected_org, &profile_name, &selected_api_url); + let result = LoginResult { + profile: profile_name.clone(), + auth_kind: "oauth".to_string(), + org: selected_org.as_ref().map(|org| org.name.clone()), + api_url: Some(selected_api_url.clone()), + app_url: Some(app_url.clone()), + status: "ok".to_string(), + error: None, + }; + ui::emit_result(base.json, ui::CommandStatus::Success, &human, &result)?; Ok(()) } @@ -1484,17 +1498,17 @@ async fn run_login_refresh(base: &BaseArgs) -> Result<()> { ) })?; - println!( + eprintln!( "Refreshing OAuth token for profile '{profile_name}' (source: {source}, api_url: {api_url})" ); if let Some(expires_at) = previous_expires_at { let now = current_unix_timestamp(); let remaining = expires_at.saturating_sub(now); - println!( + eprintln!( "Cached access token expiry before refresh: {expires_at} (about {remaining}s remaining)" ); } else { - println!("Cached access token expiry before refresh: unknown"); + eprintln!("Cached access token expiry before refresh: unknown"); } let refreshed = @@ -1518,16 +1532,30 @@ async fn run_login_refresh(base: &BaseArgs) -> Result<()> { if let Some(expires_at) = new_expires_at { let now = current_unix_timestamp(); let remaining = expires_at.saturating_sub(now); - println!("New access token expiry: {expires_at} (about {remaining}s remaining)"); + eprintln!("New access token expiry: {expires_at} (about {remaining}s remaining)"); } else { - println!("New access token expiry: unknown"); + eprintln!("New access token expiry: unknown"); } if refresh_rotated { - println!("Refresh token rotation: yes"); + eprintln!("Refresh token rotation: yes"); } else { - println!("Refresh token rotation: no"); + eprintln!("Refresh token rotation: no"); } - println!("OAuth refresh complete."); + + let result = RefreshResult { + profile: profile_name.clone(), + auth_kind: "oauth".to_string(), + access_expires_at: new_expires_at, + refresh_token_rotated: refresh_rotated, + status: "ok".to_string(), + error: None, + }; + ui::emit_result( + base.json, + ui::CommandStatus::Success, + "OAuth refresh complete.", + &result, + )?; Ok(()) } @@ -1739,10 +1767,7 @@ async fn run_profiles(base: &BaseArgs, args: AuthProfilesArgs) -> Result<()> { if filtered.is_empty() { if base.json { - println!( - "{}", - serde_json::to_string(&Vec::::new())? - ); + ui::print_json(&Vec::::new())?; } else { println!("No saved profiles. Run `bt auth login` to create one."); } @@ -1772,7 +1797,7 @@ async fn run_profiles(base: &BaseArgs, args: AuthProfilesArgs) -> Result<()> { } if base.json { - println!("{}", serde_json::to_string(&verifications)?); + ui::print_json(&verifications)?; return Ok(()); } @@ -1807,7 +1832,7 @@ async fn run_profiles(base: &BaseArgs, args: AuthProfilesArgs) -> Result<()> { Ok(()) } -fn run_login_delete(profile_name: &str, force: bool) -> Result<()> { +fn run_login_delete(profile_name: &str, force: bool, json: bool) -> Result<()> { let profile_name = profile_name.trim(); if profile_name.is_empty() { bail!("profile name cannot be empty"); @@ -1827,7 +1852,12 @@ fn run_login_delete(profile_name: &str, force: bool) -> Result<()> { .default(false) .interact_on(&term)?; if !confirmed { - eprintln!("Cancelled"); + let result = LogoutResult { + profile: Some(profile_name.to_string()), + status: "cancelled".to_string(), + error: None, + }; + ui::emit_result(json, ui::CommandStatus::Warning, "Cancelled", &result)?; return Ok(()); } } @@ -1845,17 +1875,34 @@ fn run_login_delete(profile_name: &str, force: bool) -> Result<()> { eprintln!("warning: failed to delete oauth access token for '{profile_name}': {err}"); } - ui::print_command_status( + let result = LogoutResult { + profile: Some(profile_name.to_string()), + status: "deleted".to_string(), + error: None, + }; + ui::emit_result( + json, ui::CommandStatus::Success, &format!("Deleted profile '{profile_name}'"), - ); + &result, + )?; Ok(()) } fn run_login_logout(base: BaseArgs, args: AuthLogoutArgs) -> Result<()> { let store = load_auth_store()?; if store.profiles.is_empty() { - println!("No saved profiles."); + let result = LogoutResult { + profile: None, + status: "empty".to_string(), + error: None, + }; + ui::emit_result( + base.json, + ui::CommandStatus::Warning, + "No saved profiles.", + &result, + )?; return Ok(()); } @@ -1875,7 +1922,7 @@ fn run_login_logout(base: BaseArgs, args: AuthLogoutArgs) -> Result<()> { bail!("multiple profiles exist. Use --profile to specify which one."); }; - run_login_delete(&profile_name, args.force) + run_login_delete(&profile_name, args.force, base.json) } enum ProfileStatus { @@ -1962,6 +2009,46 @@ fn build_verification( } } +/// Structured result of `bt auth login`, emitted as JSON when `--json` is set. +#[derive(Debug, Clone, Serialize)] +pub struct LoginResult { + pub profile: String, + pub auth_kind: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub org: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub api_url: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub app_url: Option, + pub status: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +/// Structured result of `bt auth refresh`, emitted as JSON when `--json` is set. +#[derive(Debug, Clone, Serialize)] +pub struct RefreshResult { + pub profile: String, + pub auth_kind: String, + /// Unix epoch seconds at which the new access token expires, if known. + #[serde(skip_serializing_if = "Option::is_none")] + pub access_expires_at: Option, + pub refresh_token_rotated: bool, + pub status: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +/// Structured result of `bt auth logout`, emitted as JSON when `--json` is set. +#[derive(Debug, Clone, Serialize)] +pub struct LogoutResult { + #[serde(skip_serializing_if = "Option::is_none")] + pub profile: Option, + pub status: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + async fn verify_profile_full( name: &str, profile: &AuthProfile, @@ -4833,4 +4920,50 @@ mod tests { let result = env.login_read_only_with_base(base).await; assert_err_contains(result, "no login credentials found"); } + + #[test] + fn login_result_serializes_with_optional_fields_skipped() { + let r = LoginResult { + profile: "test-profile".into(), + auth_kind: "api_key".into(), + org: None, + api_url: Some("https://api.braintrust.dev".into()), + app_url: None, + status: "ok".into(), + error: None, + }; + let json = serde_json::to_string(&r).expect("serialize"); + assert_eq!( + json, + r#"{"profile":"test-profile","auth_kind":"api_key","api_url":"https://api.braintrust.dev","status":"ok"}"# + ); + } + + #[test] + fn refresh_result_serializes_with_optional_fields_skipped() { + let r = RefreshResult { + profile: "test-profile".into(), + auth_kind: "oauth".into(), + access_expires_at: Some(1700000000), + refresh_token_rotated: true, + status: "ok".into(), + error: None, + }; + let json = serde_json::to_string(&r).expect("serialize"); + assert_eq!( + json, + r#"{"profile":"test-profile","auth_kind":"oauth","access_expires_at":1700000000,"refresh_token_rotated":true,"status":"ok"}"# + ); + } + + #[test] + fn logout_result_serializes_empty_store_case() { + let r = LogoutResult { + profile: None, + status: "empty".into(), + error: None, + }; + let json = serde_json::to_string(&r).expect("serialize"); + assert_eq!(json, r#"{"status":"empty"}"#); + } } diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 3f51e64..d39a77a 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -59,5 +59,5 @@ pub use ratatui_table::{ pub use select::{fuzzy_select, select_project, ProjectSelectMode}; pub use spinner::{with_spinner, with_spinner_visible}; -pub use status::{print_command_status, CommandStatus}; +pub use status::{emit_result, print_command_status, print_json, CommandStatus}; pub use table::{apply_column_padding, header, styled_table, truncate}; diff --git a/src/ui/status.rs b/src/ui/status.rs index d6ef3bd..3904fb2 100644 --- a/src/ui/status.rs +++ b/src/ui/status.rs @@ -1,4 +1,5 @@ use dialoguer::console::style; +use serde::Serialize; use super::is_quiet; @@ -21,3 +22,71 @@ pub fn print_command_status(status: CommandStatus, message: &str) { eprintln!("{indicator} {message}"); } + +/// Serialize `payload` as compact JSON to stdout. This is the single shared +/// entry point for machine-readable command output so `--json` handling lives +/// in one place rather than being re-implemented per command. +pub fn print_json(payload: &T) -> anyhow::Result<()> { + println!("{}", serde_json::to_string(payload)?); + Ok(()) +} + +/// Emit a command result, choosing between machine-readable JSON and the +/// human-readable status line based on the global `json` flag. +/// +/// - `json == true` -> `payload` serialized to stdout +/// - `json == false` -> a `print_command_status` line on stderr +/// +/// Both stdout (JSON) and stderr (human status) channels stay separate so +/// `--json` output is safely pipeable. +pub fn emit_result( + json: bool, + status: CommandStatus, + human_message: &str, + payload: &T, +) -> anyhow::Result<()> { + if json { + print_json(payload) + } else { + print_command_status(status, human_message); + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde::Serialize; + + #[derive(Serialize)] + struct Sample { + status: String, + value: u32, + } + + #[test] + fn emit_result_prints_human_status_when_json_disabled() { + // Human output goes to stderr via print_command_status; stdout stays empty. + let payload = Sample { + status: "ok".into(), + value: 7, + }; + // We only assert no error and no stdout leak; the indicator is on stderr. + let res = emit_result(false, CommandStatus::Success, "done", &payload); + assert!(res.is_ok()); + } + + #[test] + fn emit_result_serializes_payload_when_json_enabled() { + let payload = Sample { + status: "ok".into(), + value: 7, + }; + // Capture the JSON by calling print_json directly (the JSON branch of emit_result). + // print_json writes to stdout; in unit tests we only assert it succeeds and + // produces a non-empty string via to_string, since stdout capture is env-dependent. + let json = serde_json::to_string(&payload).expect("serialize"); + assert_eq!(json, r#"{"status":"ok","value":7}"#); + assert!(emit_result(true, CommandStatus::Success, "done", &payload).is_ok()); + } +} diff --git a/tests/functions.rs b/tests/functions.rs index 424741e..44ef2fa 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -815,6 +815,71 @@ fn auth_profiles_with_api_key_profile_does_not_print_jwt_on_stdout() { ); } +fn auth_sub_command(cwd: &Path, config_dir: &Path, sub: &[&str]) -> Command { + // Shared builder for `bt auth ` so each auth subcommand inherits the + // same isolated config dir / env scrubbing as `auth profiles`. + let mut cmd = Command::new(bt_binary_path()); + cmd.arg("auth") + .args(sub) + .current_dir(cwd) + .env("XDG_CONFIG_HOME", config_dir) + .env("APPDATA", config_dir) + .env("BRAINTRUST_NO_COLOR", "1") + .env_remove("BRAINTRUST_PROFILE") + .env_remove("BRAINTRUST_ORG_NAME") + .env_remove("BRAINTRUST_API_URL") + .env_remove("BRAINTRUST_APP_URL") + .env_remove("BRAINTRUST_ENV_FILE"); + cmd +} + +#[test] +fn auth_logout_json_with_no_profiles_emits_empty_status() { + let cwd = tempdir().expect("create temp cwd"); + let config_dir = tempdir().expect("create temp config dir"); + + let output = auth_sub_command(cwd.path(), config_dir.path(), &["logout", "--json"]) + .output() + .expect("run bt auth logout --json"); + + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); + assert_eq!(stdout, r#"{"status":"empty"}"#); +} + +#[test] +fn auth_refresh_errors_for_api_key_profile_even_with_json() { + // --json must not swallow a real error: an api_key profile cannot be + // refreshed, and the command should fail with an actionable message. + let cwd = tempdir().expect("create temp cwd"); + let config_dir = tempdir().expect("create temp config dir"); + + fs::create_dir_all(config_dir.path().join("bt")).expect("create bt config dir"); + fs::write( + config_dir.path().join("bt").join("auth.json"), + r#"{"profiles":{"test-profile":{"auth_kind":"api_key","api_url":"https://api.braintrust.dev","app_url":"https://www.braintrust.dev","org_name":"test-org","user_name":null,"email":null,"api_key_hint":"sk-****test"}}}"#, + ) + .expect("write auth.json"); + + let output = auth_sub_command( + cwd.path(), + config_dir.path(), + &["refresh", "--profile", "test-profile", "--json"], + ) + .output() + .expect("run bt auth refresh --profile test-profile --json"); + + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("only applies to oauth profiles"), + "expected oauth-only refresh hint, got: {stderr}" + ); + // Errors are not part of the --json contract (they surface via the global + // error printer); stdout should not carry a partial result. + assert!(String::from_utf8_lossy(&output.stdout).trim().is_empty()); +} + #[test] fn push_and_pull_help_are_machine_readable() { let push_help = Command::new(bt_binary_path()) From 32cdc1b11badfdf2e4648eb5a1d985c38637e187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Mon, 29 Jun 2026 16:53:45 -0700 Subject: [PATCH 3/3] fix: small polish --- src/auth.rs | 169 ++++++++++++++++++++++----------------------- src/ui/status.rs | 20 ++---- tests/functions.rs | 53 ++++++-------- 3 files changed, 111 insertions(+), 131 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 6481a64..c97091e 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -397,9 +397,8 @@ struct AuthProfilesArgs { #[arg(long, value_name = "NAME")] profile: Option, - /// Read the OAuth access JWT for each shown profile from the keychain and print it in plaintext. - /// - /// WARNING: this pulls a live bearer token out of the secure credential store and writes it to stdout in plaintext. + /// Print the OAuth access JWT from the keychain in plaintext (prints nothing if expired). + /// WARNING: pulls a live bearer token out of the secure store and writes it to stdout in plaintext. #[arg(long = "print-jwt-token")] print_jwt_token: bool, } @@ -1743,29 +1742,30 @@ fn format_login_success( async fn run_profiles(base: &BaseArgs, args: AuthProfilesArgs) -> Result<()> { let store = load_auth_store()?; - // Filter to a single profile when --profile is given. Error out when the name doesn't match a saved profile. - let filtered: Vec<(String, AuthProfile)> = if let Some(name) = &args.profile { - match store.profiles.get(name) { - Some(profile) => vec![(name.clone(), profile.clone())], - None => { - let available: Vec = store.profiles.keys().cloned().collect(); - let suffix = if available.is_empty() { - String::new() - } else { - format!(": {}", available.join(", ")) - }; - bail!("profile '{name}' not found; run `bt auth profiles` to see available profiles{suffix}"); - } - } - } else { - store + // Build a filtered view: only the --profile match if given, else all saved profiles. + let mut filtered_store = AuthStore::default(); + if let Some(name) = &args.profile { + let profile = store.profiles.get(name).ok_or_else(|| { + let available: Vec = store.profiles.keys().cloned().collect(); + let suffix = if available.is_empty() { + String::new() + } else { + format!(": {}", available.join(", ")) + }; + anyhow::anyhow!( + "profile '{name}' not found; run `bt auth profiles` to see available profiles{suffix}" + ) + })?; + filtered_store .profiles - .iter() - .map(|(n, p)| (n.clone(), p.clone())) - .collect() - }; + .insert(name.clone(), profile.clone()); + } else { + for (n, p) in store.profiles.iter() { + filtered_store.profiles.insert(n.clone(), p.clone()); + } + } - if filtered.is_empty() { + if filtered_store.profiles.is_empty() { if base.json { ui::print_json(&Vec::::new())?; } else { @@ -1774,19 +1774,10 @@ async fn run_profiles(base: &BaseArgs, args: AuthProfilesArgs) -> Result<()> { return Ok(()); } - // Build a filtered view of the store so verification only touches the selected profile(s) when --profile is set. - let mut filtered_store = AuthStore::default(); - for (name, profile) in &filtered { - filtered_store - .profiles - .insert(name.clone(), profile.clone()); - } - let verifications = verify_all_profiles_from_store(&filtered_store, args.print_jwt_token).await; - // When the API is unreachable, fall back to showing saved profile info. - // Skip this shortcut when the user asked for JWT tokens, since the token - // comes from the local keychain and is available regardless of network. + // When the API is unreachable, fall back to saved profile info. Skip this under + // --print-jwt-token: the token comes from the keychain and is available offline. let all_network_errors = verifications .iter() .all(|v| v.status == "error" && !v.error.as_deref().unwrap_or("").contains("invalid")); @@ -1801,25 +1792,19 @@ async fn run_profiles(base: &BaseArgs, args: AuthProfilesArgs) -> Result<()> { return Ok(()); } - if args.print_jwt_token { - eprintln!("warning: --print-jwt-token writes live bearer tokens to stdout in plaintext."); - } - + // With --print-jwt-token, the whole profile line (including `jwt: `) goes to + // stdout. Use --json for a parseable `jwt_token` field; without the flag, status stays on stderr. for v in &verifications { - let cmd_status = match v.status.as_str() { - "ok" => crate::ui::CommandStatus::Success, - "expired" => crate::ui::CommandStatus::Warning, - _ => crate::ui::CommandStatus::Error, - }; - crate::ui::print_command_status(cmd_status, &format_verification_line(v)); - - // The human-readable status line above goes to stderr; when the user - // asked for the JWT, also print the token itself to stdout so it can be - // piped into another command. Skip profiles without a loadable token - // (e.g. api_key profiles, or expired/missing oauth credentials) so - // consumers don't pick up an empty line. - if let Some(token) = v.jwt_token.as_ref() { - println!("{token}"); + let line = format_verification_line(v, args.print_jwt_token); + if args.print_jwt_token { + println!("{line}"); + } else { + let cmd_status = match v.status.as_str() { + "ok" => crate::ui::CommandStatus::Success, + "expired" => crate::ui::CommandStatus::Warning, + _ => crate::ui::CommandStatus::Error, + }; + crate::ui::print_command_status(cmd_status, &line); } } @@ -2070,36 +2055,20 @@ async fn verify_profile_full( ) }; - // When asked to print the JWT, surface the live OAuth access token directly - // from the credential store before any network validation. We do this - // unconditionally (even if the token is expired or the API is unreachable) - // so `--print-jwt-token` can be used to retrieve a token for offline - // inspection. API-key profiles do not have a JWT, so jwt_token stays None. - let jwt_token = if print_jwt_token && profile.auth_kind == AuthKind::Oauth { - match load_credential_for_profile(name, profile) { - CredentialLoad::Found(k) => Some(k), - CredentialLoad::Missing | CredentialLoad::Expired | CredentialLoad::Error(_) => None, - } - } else { - None - }; + let want_jwt = print_jwt_token && profile.auth_kind == AuthKind::Oauth; + // Surface the live OAuth access token for --print-jwt-token before any network validation. + // Load the credential once and reuse it for verification and the jwt_token field. let credential = match load_credential_for_profile(name, profile) { CredentialLoad::Found(k) => k, CredentialLoad::Missing => { - let mut v = mk(ProfileStatus::Missing, None, None); - v.jwt_token = jwt_token; - return v; + return mk(ProfileStatus::Missing, None, None); } CredentialLoad::Expired => { - let mut v = mk(ProfileStatus::Expired, None, None); - v.jwt_token = jwt_token; - return v; + return mk(ProfileStatus::Expired, None, None); } CredentialLoad::Error(e) => { - let mut v = mk(ProfileStatus::Error(e), None, None); - v.jwt_token = jwt_token; - return v; + return mk(ProfileStatus::Error(e), None, None); } }; @@ -2124,7 +2093,9 @@ async fn verify_profile_full( mk(status, None, None) } }; - if print_jwt_token && profile.auth_kind == AuthKind::Oauth { + + // Set jwt_token after the network call so it's surfaced regardless of ok/expired/error. + if want_jwt { v.jwt_token = Some(credential); } v @@ -2151,7 +2122,7 @@ async fn verify_all_profiles_from_store( results } -fn format_verification_line(v: &ProfileVerification) -> String { +fn format_verification_line(v: &ProfileVerification, include_jwt: bool) -> String { let mut parts = vec![v.name.clone(), v.auth.clone()]; if let Some(ref org) = v.org { parts.push(format!("org: {org}")); @@ -2175,8 +2146,10 @@ fn format_verification_line(v: &ProfileVerification) -> String { } } } - if let Some(ref token) = v.jwt_token { - parts.push(format!("jwt: {token}")); + if include_jwt { + if let Some(ref token) = v.jwt_token { + parts.push(format!("jwt: {token}")); + } } parts.join(" — ") } @@ -2198,7 +2171,7 @@ fn print_saved_profiles(store: &AuthStore, json: bool) -> Result<()> { }) }) .collect(); - println!("{}", serde_json::to_string(&output)?); + crate::ui::print_json(&output)?; } else { for (name, profile) in &store.profiles { let kind = match profile.auth_kind { @@ -4704,7 +4677,7 @@ mod tests { jwt_token: None, }; assert_eq!( - format_verification_line(&v), + format_verification_line(&v, true), "work — oauth — org: acme — Alice (alice@example.com)" ); } @@ -4723,7 +4696,7 @@ mod tests { jwt_token: None, }; assert_eq!( - format_verification_line(&v), + format_verification_line(&v, true), "work — api_key — org: acme — sk-****zhJwO" ); } @@ -4741,7 +4714,10 @@ mod tests { error: None, jwt_token: None, }; - assert_eq!(format_verification_line(&v), "old — oauth — token expired"); + assert_eq!( + format_verification_line(&v, true), + "old — oauth — token expired" + ); } #[test] @@ -4758,7 +4734,7 @@ mod tests { jwt_token: None, }; assert_eq!( - format_verification_line(&v), + format_verification_line(&v, true), "bad — api_key — org: corp — invalid API key" ); } @@ -4777,11 +4753,32 @@ mod tests { jwt_token: Some("eyJhbGciOiJFUzI1NiJ9.eyJzdWIiOiJhYmMifQ.sig".into()), }; assert_eq!( - format_verification_line(&v), + format_verification_line(&v, true), "work — oauth — org: acme — Alice (alice@example.com) — jwt: eyJhbGciOiJFUzI1NiJ9.eyJzdWIiOiJhYmMifQ.sig" ); } + #[test] + fn format_verification_line_suppresses_jwt_token_when_redacted() { + // Without --print-jwt-token (include_jwt = false) the jwt: segment is omitted so a plain + // `bt auth profiles` listing never leaks a live token. + let v = ProfileVerification { + name: "work".into(), + auth: "oauth".into(), + org: Some("acme".into()), + user_name: Some("Alice".into()), + user_email: Some("alice@example.com".into()), + api_key_hint: None, + status: "ok".into(), + error: None, + jwt_token: Some("eyJhbGciOiJFUzI1NiJ9.eyJzdWIiOiJhYmMifQ.sig".into()), + }; + assert_eq!( + format_verification_line(&v, false), + "work — oauth — org: acme — Alice (alice@example.com)" + ); + } + #[test] fn format_verification_line_omits_jwt_token_when_none() { let v = ProfileVerification { @@ -4796,7 +4793,7 @@ mod tests { jwt_token: None, }; assert_eq!( - format_verification_line(&v), + format_verification_line(&v, true), "work — api_key — org: acme — sk-****zhJwO" ); } diff --git a/src/ui/status.rs b/src/ui/status.rs index 3904fb2..726782b 100644 --- a/src/ui/status.rs +++ b/src/ui/status.rs @@ -23,22 +23,14 @@ pub fn print_command_status(status: CommandStatus, message: &str) { eprintln!("{indicator} {message}"); } -/// Serialize `payload` as compact JSON to stdout. This is the single shared -/// entry point for machine-readable command output so `--json` handling lives -/// in one place rather than being re-implemented per command. +/// Serialize `payload` as compact JSON to stdout — the single shared machine-readable entry point. pub fn print_json(payload: &T) -> anyhow::Result<()> { println!("{}", serde_json::to_string(payload)?); Ok(()) } -/// Emit a command result, choosing between machine-readable JSON and the -/// human-readable status line based on the global `json` flag. -/// -/// - `json == true` -> `payload` serialized to stdout -/// - `json == false` -> a `print_command_status` line on stderr -/// -/// Both stdout (JSON) and stderr (human status) channels stay separate so -/// `--json` output is safely pipeable. +/// Emit a result: JSON payload to stdout when `json`, else a human status line on stderr. +/// Keep stdout/stderr separate so `--json` output stays safely pipeable. pub fn emit_result( json: bool, status: CommandStatus, @@ -71,7 +63,7 @@ mod tests { status: "ok".into(), value: 7, }; - // We only assert no error and no stdout leak; the indicator is on stderr. + // Assert no error and no stdout leak; the indicator is on stderr. let res = emit_result(false, CommandStatus::Success, "done", &payload); assert!(res.is_ok()); } @@ -82,9 +74,7 @@ mod tests { status: "ok".into(), value: 7, }; - // Capture the JSON by calling print_json directly (the JSON branch of emit_result). - // print_json writes to stdout; in unit tests we only assert it succeeds and - // produces a non-empty string via to_string, since stdout capture is env-dependent. + // stdout capture is env-dependent, so assert serialization directly rather than via emit_result. let json = serde_json::to_string(&payload).expect("serialize"); assert_eq!(json, r#"{"status":"ok","value":7}"#); assert!(emit_result(true, CommandStatus::Success, "done", &payload).is_ok()); diff --git a/tests/functions.rs b/tests/functions.rs index 44ef2fa..ce664d4 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -191,19 +191,7 @@ fn sanitized_env_keys() -> &'static [&'static str] { } fn auth_profiles_command(cwd: &Path, config_dir: &Path) -> Command { - let mut cmd = Command::new(bt_binary_path()); - cmd.arg("auth") - .arg("profiles") - .current_dir(cwd) - .env("XDG_CONFIG_HOME", config_dir) - .env("APPDATA", config_dir) - .env("BRAINTRUST_NO_COLOR", "1") - .env_remove("BRAINTRUST_PROFILE") - .env_remove("BRAINTRUST_ORG_NAME") - .env_remove("BRAINTRUST_API_URL") - .env_remove("BRAINTRUST_APP_URL") - .env_remove("BRAINTRUST_ENV_FILE"); - cmd + auth_sub_command(cwd, config_dir, &["profiles"]) } #[derive(Debug, Clone)] @@ -786,20 +774,15 @@ fn auth_profiles_profile_not_found_is_actionable() { } #[test] -fn auth_profiles_with_api_key_profile_does_not_print_jwt_on_stdout() { - // An api_key profile has no JWT; --print-jwt-token must leave stdout empty - // rather than emitting the api key or an empty line. +fn auth_profiles_with_api_key_profile_does_not_leak_jwt_or_secret() { + // An api_key profile has no JWT; --print-jwt-token prints its line to stdout but must + // not append a `jwt:` segment or surface the api-key hint as a token. let cwd = tempdir().expect("create temp cwd"); let config_dir = tempdir().expect("create temp config dir"); // Seed an api_key profile with no stored secret so verification reports // "missing" without touching the network or keychain. - fs::create_dir_all(config_dir.path().join("bt")).expect("create bt config dir"); - fs::write( - config_dir.path().join("bt").join("auth.json"), - r#"{"profiles":{"test-profile":{"auth_kind":"api_key","api_url":"https://api.braintrust.dev","app_url":"https://www.braintrust.dev","org_name":"test-org","user_name":null,"email":null,"api_key_hint":"sk-****test"}}}"#, - ) - .expect("write auth.json"); + seed_api_key_profile(config_dir.path()); let output = auth_profiles_command(cwd.path(), config_dir.path()) .arg("--profile") @@ -810,11 +793,26 @@ fn auth_profiles_with_api_key_profile_does_not_print_jwt_on_stdout() { let stdout = String::from_utf8_lossy(&output.stdout); assert!( - stdout.trim().is_empty(), - "expected no JWT on stdout for an api_key profile, got: {stdout}" + !stdout.contains("jwt:"), + "api_key profile must not emit a jwt: segment, got: {stdout}" + ); + assert!( + !stdout.contains("sk-****test"), + "api key hint must not be surfaced as a token, got: {stdout}" ); } +/// Seed a synthetic api_key `test-profile` so verification reports "missing" +/// without touching the network or keychain. +fn seed_api_key_profile(config_dir: &Path) { + fs::create_dir_all(config_dir.join("bt")).expect("create bt config dir"); + fs::write( + config_dir.join("bt").join("auth.json"), + r#"{"profiles":{"test-profile":{"auth_kind":"api_key","api_url":"https://api.braintrust.dev","app_url":"https://www.braintrust.dev","org_name":"test-org","user_name":null,"email":null,"api_key_hint":"sk-****test"}}}"#, + ) + .expect("write auth.json"); +} + fn auth_sub_command(cwd: &Path, config_dir: &Path, sub: &[&str]) -> Command { // Shared builder for `bt auth ` so each auth subcommand inherits the // same isolated config dir / env scrubbing as `auth profiles`. @@ -854,12 +852,7 @@ fn auth_refresh_errors_for_api_key_profile_even_with_json() { let cwd = tempdir().expect("create temp cwd"); let config_dir = tempdir().expect("create temp config dir"); - fs::create_dir_all(config_dir.path().join("bt")).expect("create bt config dir"); - fs::write( - config_dir.path().join("bt").join("auth.json"), - r#"{"profiles":{"test-profile":{"auth_kind":"api_key","api_url":"https://api.braintrust.dev","app_url":"https://www.braintrust.dev","org_name":"test-org","user_name":null,"email":null,"api_key_hint":"sk-****test"}}}"#, - ) - .expect("write auth.json"); + seed_api_key_profile(config_dir.path()); let output = auth_sub_command( cwd.path(),