diff --git a/bb-cli/Cargo.lock b/bb-cli/Cargo.lock index 5995cd54c..5eb401f81 100644 --- a/bb-cli/Cargo.lock +++ b/bb-cli/Cargo.lock @@ -468,16 +468,6 @@ dependencies = [ "percent-encoding", ] -[[package]] -name = "fs2" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" -dependencies = [ - "libc", - "winapi", -] - [[package]] name = "futures-channel" version = "0.3.34" @@ -1746,7 +1736,6 @@ dependencies = [ "builderbot-auth", "clap", "clap_complete", - "fs2", "pbjson", "pbjson-build", "pbjson-types", @@ -2298,22 +2287,6 @@ dependencies = [ "rustls-pki-types", ] -[[package]] -name = "winapi" -version = "0.3.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" -dependencies = [ - "winapi-i686-pc-windows-gnu", - "winapi-x86_64-pc-windows-gnu", -] - -[[package]] -name = "winapi-i686-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" - [[package]] name = "winapi-util" version = "0.1.11" @@ -2323,12 +2296,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "winapi-x86_64-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" - [[package]] name = "windows-link" version = "0.2.1" diff --git a/bb-cli/Cargo.toml b/bb-cli/Cargo.toml index a52a31321..ca433e4c9 100644 --- a/bb-cli/Cargo.toml +++ b/bb-cli/Cargo.toml @@ -19,7 +19,6 @@ anyhow = "1" builderbot-auth = { path = "../crates/builderbot-auth", features = ["blocking-client"] } clap = { version = "4.5", features = ["env"] } clap_complete = "4.5" -fs2 = "0.4" pbjson = "0.9" pbjson-types = "0.9" prost = "0.14" diff --git a/bb-cli/Justfile b/bb-cli/Justfile index 3043dcc35..44053d367 100644 --- a/bb-cli/Justfile +++ b/bb-cli/Justfile @@ -104,7 +104,7 @@ lint: fmt-check cargo clippy --locked --all-targets --all-features -- -D warnings test: - cargo test --locked + cargo test --locked --all-features package-smoke: build-sq ./sqbin/{{BIN_NAME}}.exoskeleton --version diff --git a/bb-cli/src/bb/apps.rs b/bb-cli/src/bb/apps.rs index bee70dd4a..2b925e79f 100644 --- a/bb-cli/src/bb/apps.rs +++ b/bb-cli/src/bb/apps.rs @@ -6,55 +6,51 @@ //! `bb tools appkit`, and it does not migrate the separate internal Compose //! workflow. Both internal paths remain unchanged. //! -//! The CLI exchanges its stored bbidentity session for a short-lived -//! Compose-purpose bearer token. Public ingress validates that token online -//! through kgoose `ext_authz`, removes it, and forwards only verified identity -//! headers to Compose. Compose never receives the bearer token. +//! The CLI sends its stored bbidentity session only to the allowlisted Compose +//! control-plane origins. Public ingress authorizes that session through kgoose +//! `ext_authz` and removes it before forwarding the request internally. Compose +//! never receives the session credential. -use std::fs::{self, File, OpenOptions}; +use std::fs; use std::io::Read; use std::path::{Path, PathBuf}; -use std::sync::Mutex; -use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use std::time::Duration; use anyhow::{Context, Result}; -use builderbot_auth::auth_login::{auth_url, build_auth_http_client, playpen_baggage}; +use builderbot_auth::auth_login::auth_url; +#[cfg(test)] +use builderbot_auth::auth_login::build_auth_http_client; +use builderbot_auth::auth_storage::StoredSessionCredential; use clap::{Arg, ArgMatches, Command}; -use fs2::FileExt; -use reqwest::blocking::{multipart, Client, RequestBuilder, Response}; +use reqwest::blocking::{multipart, Client, Request, RequestBuilder, Response}; use reqwest::header::{HeaderValue, ACCEPT, AUTHORIZATION, USER_AGENT}; +use reqwest::redirect::Policy; use reqwest::StatusCode; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use serde_json::{json, Map, Value}; -use sha2::{Digest, Sha256}; -use super::auth::SESSION_CREDENTIAL_HEADER; -use super::auth_storage::{ - default_session_storage, session_storage_key_from_config, PurposeTokenStorageKey, - SessionCredentialStorage, StoredPurposeTokenCredential, -}; +use super::auth_login::verify_stored_session; +use super::auth_storage::default_session_storage; use super::display::{print_json, terminal_safe_text, Style}; use super::runner; use super::skills_api::{exit_codes, failure}; -use super::skills_config::{kgoose_service_url, SkillsConfig}; +use super::skills_config::SkillsConfig; const APPS_BASE_URL_ENV_VAR: &str = "BB_APPS_CONTROL_PLANE_URL"; const APPS_CLIENT_VERSION_ENV_VAR: &str = "BB_APPS_CLIENT_VERSION"; +#[cfg(test)] +const APPS_E2E_CONTROL_PLANE_URL_ENV_VAR: &str = "BB_APPS_E2E_CONTROL_PLANE_URL"; +#[cfg(test)] +const APPS_E2E_AUTH_URL_ENV_VAR: &str = "BB_APPS_E2E_AUTH_URL"; +#[cfg(test)] +const APPS_E2E_CREDENTIAL_ENV_VAR: &str = "BB_APPS_E2E_CREDENTIAL"; const APPS_CONTRACT_PATH: &str = "/v1/agent/contract"; const APPS_PLAN_PATH: &str = "/v1/agent/apps/plan"; -const COMPOSE_TOKEN_EXCHANGE_PATH: &str = "/v1/auth/token/compose"; const HOTPOD_AGENT_CLIENT_VERSION_HEADER: &str = "X-Hotpod-Agent-Client-Version"; -const TOKEN_EXCHANGE_REQUEST_TIMEOUT: Duration = Duration::from_secs(30); // Compose may synchronously wait up to two minutes for an initialize or -// deploy rollout. Leave enough headroom for the response to traverse ingress -// without weakening the tighter credential-exchange bound above. +// deploy rollout. Leave enough headroom for the response to traverse ingress. const CONTROL_PLANE_REQUEST_TIMEOUT: Duration = Duration::from_secs(3 * 60); -const TOKEN_EXCHANGE_RESPONSE_MAX_BYTES: usize = 32 * 1024; const CONTROL_PLANE_RESPONSE_MAX_BYTES: usize = 2 * 1024 * 1024; -const COMPOSE_TOKEN_PURPOSE: &str = "compose"; -const PURPOSE_TOKEN_REFRESH_SKEW: Duration = Duration::from_secs(60); -const PURPOSE_TOKEN_REPLACEMENT_INTERVAL: Duration = Duration::from_secs(60); -const PURPOSE_TOKEN_LOCK_FILE: &str = "apps-purpose-token.lock"; const TRUSTED_CONTROL_PLANE_HOSTS: &[&str] = &[ "compose-ctrl.test.blockstaging.build", "compose-ctrl.app.builderlab.xyz", @@ -206,13 +202,13 @@ fn run_contract(config: &SkillsConfig, matches: &ArgMatches) -> Result<()> { .context("expected Apps Platform client version")?; let client = ControlPlaneClient::new(base_url, client_version, config.style)?; - let token_provider = KgoosePurposeTokenProvider::from_config(config)?; - let contract = client.contract(&token_provider)?; + let credential = ComposeSessionCredential::from_config(config)?; + let contract = client.contract(&credential)?; print_json(&contract) } fn run_create(config: &SkillsConfig, matches: &ArgMatches) -> Result<()> { - let (client, token_provider) = control_plane_context(config, matches)?; + let (client, credential) = control_plane_context(config, matches)?; let request = PlanRequest { app_id: matches.get_one::("app-id").map(String::as_str), name: matches.get_one::("name").map(String::as_str), @@ -223,7 +219,7 @@ fn run_create(config: &SkillsConfig, matches: &ArgMatches) -> Result<()> { persistence: matches.get_one::("persistence").map(String::as_str), client_version: client.client_version_text(), }; - let plan = client.plan(&token_provider, &request)?; + let plan = client.plan(&credential, &request)?; let app_id = required_response_string(&plan, "app_id", "Apps Platform plan")?.to_string(); let initialize_required = plan .pointer("/initialize/required") @@ -240,7 +236,7 @@ fn run_create(config: &SkillsConfig, matches: &ArgMatches) -> Result<()> { initialize_required.unwrap_or(false) || initialize_recommended.unwrap_or(false); let initialize = if should_initialize { let request = initialize_request_from_plan(&plan); - Some(client.initialize(&token_provider, &app_id, &request)?) + Some(client.initialize(&credential, &app_id, &request)?) } else { None }; @@ -280,15 +276,15 @@ fn run_deploy(config: &SkillsConfig, matches: &ArgMatches) -> Result<()> { version_id: matches.get_one::("version-id").cloned(), deployment_id: matches.get_one::("deployment-id").cloned(), }; - let (client, token_provider) = control_plane_context(config, matches)?; - let response = client.deploy(&token_provider, app_id, artifact, &options)?; + let (client, credential) = control_plane_context(config, matches)?; + let response = client.deploy(&credential, app_id, artifact, &options)?; print_json(&response) } fn control_plane_context( config: &SkillsConfig, matches: &ArgMatches, -) -> Result<(ControlPlaneClient, KgoosePurposeTokenProvider)> { +) -> Result<(ControlPlaneClient, ComposeSessionCredential)> { let base_url = matches .get_one::("apps-base-url") .context("expected Apps Platform control-plane URL")?; @@ -296,8 +292,8 @@ fn control_plane_context( .get_one::("apps-client-version") .context("expected Apps Platform client version")?; let client = ControlPlaneClient::new(base_url, client_version, config.style)?; - let token_provider = KgoosePurposeTokenProvider::from_config(config)?; - Ok((client, token_provider)) + let credential = ComposeSessionCredential::from_config(config)?; + Ok((client, credential)) } #[derive(Serialize)] @@ -363,272 +359,90 @@ fn validate_artifact_path(path: &Path) -> Result<()> { Ok(()) } -/// Supplies the short-lived Compose bearer accepted by public ingress. Keeping -/// exchange and request construction separate makes the credential lifecycle -/// independently testable. The provider shares a session-bound token across -/// CLI processes so kgoose's one-active-token contract is respected. -trait ComposeCredentialProvider { - fn authorization_header(&self) -> Result; - - /// Returns a replacement credential after ingress rejects `rejected`, or - /// `None` when retrying cannot help yet. Implementations must not return - /// the rejected value, which keeps the control-plane retry bounded. - fn authorization_header_after_rejection( - &self, - _rejected: &HeaderValue, - ) -> Result> { - Ok(None) - } +struct ComposeSessionCredential { + authorization: HeaderValue, + secret: String, } -struct KgoosePurposeTokenProvider { - client: Client, - exchange_url: url::Url, - session_credential: HeaderValue, - session_credential_sha256: String, - baggage: Option, - style: Style, - storage: Box, - storage_key: PurposeTokenStorageKey, - refresh_lock_path: PathBuf, - refresh_mutex: Mutex<()>, -} - -impl KgoosePurposeTokenProvider { +impl ComposeSessionCredential { fn from_config(config: &SkillsConfig) -> Result { let storage = default_session_storage(config)?; - let session_storage_key = session_storage_key_from_config(config); - let credential = storage - .get(&session_storage_key)? - .ok_or_else(auth_required_error)?; - let session_credential = credential + let verified = + verify_stored_session(config, storage.as_ref())?.ok_or_else(auth_required_error)?; + Self::from_stored(verified.credential) + } + + fn from_stored(credential: StoredSessionCredential) -> Result { + let secret = credential .session_credential_header_value() .ok_or_else(auth_required_error)?; - let session_credential_sha256 = sha256(&session_credential); - let session_credential = HeaderValue::from_str(&session_credential) - .context("stored BuilderBot CLI auth session is invalid; run `bb auth login`")?; - let server_url = kgoose_service_url(&config.kgoose_base_url, &config.kgoose_service_path); - let exchange_url = auth_url(&server_url, COMPOSE_TOKEN_EXCHANGE_PATH) - .context("build Compose credential exchange URL")?; - let baggage = playpen_baggage(config.playpen.as_deref()) - .map(|value| HeaderValue::from_str(&value).context("build kgoose playpen header")) - .transpose()?; + Self::new(secret) + } + fn new(secret: String) -> Result { + let authorization = HeaderValue::from_str(&format!("BBIdentity {secret}")) + .context("stored BuilderBot CLI auth session is invalid; run `bb auth login`")?; Ok(Self { - client: build_auth_http_client(TOKEN_EXCHANGE_REQUEST_TIMEOUT)?, - exchange_url, - session_credential, - session_credential_sha256, - baggage, - style: config.style, - storage, - storage_key: PurposeTokenStorageKey::new(&session_storage_key, COMPOSE_TOKEN_PURPOSE), - refresh_lock_path: config.bb_home.join(PURPOSE_TOKEN_LOCK_FILE), - refresh_mutex: Mutex::new(()), + authorization, + secret, }) } - fn exchange_purpose_token(&self) -> Result { - self.style - .verbose(&format!("POST {COMPOSE_TOKEN_EXCHANGE_PATH}")); - let mut request = self - .client - .post(self.exchange_url.clone()) - .header(USER_AGENT, apps_user_agent()) - .header(ACCEPT, "application/json") - .header(SESSION_CREDENTIAL_HEADER, self.session_credential.clone()); - if let Some(baggage) = &self.baggage { - request = request.header("Baggage", baggage.clone()); - } - let response = request - .send() - .map_err(|error| network_failure("POST", COMPOSE_TOKEN_EXCHANGE_PATH, error))?; - let status = response.status(); - if status == StatusCode::TOO_MANY_REQUESTS { - self.style - .verbose(&format!("POST {COMPOSE_TOKEN_EXCHANGE_PATH} -> {status}")); - return Ok(PurposeTokenExchangeOutcome::RateLimited); - } - if !status.is_success() { - self.style - .verbose(&format!("POST {COMPOSE_TOKEN_EXCHANGE_PATH} -> {status}")); - return Err(exchange_http_failure(status)); - } - let body = read_limited_response_body( - response, - TOKEN_EXCHANGE_RESPONSE_MAX_BYTES, - "Compose credential exchange", - )?; - self.style.verbose(&format!( - "POST {COMPOSE_TOKEN_EXCHANGE_PATH} -> {status} ({} bytes)", - body.len() - )); - let exchange: PurposeTokenExchangeResponse = - serde_json::from_str(&body).context("parse Compose credential exchange response")?; - if exchange.token_type.as_deref() != Some("Bearer") { - anyhow::bail!("Compose credential exchange returned an unsupported token type"); - } - let access_token = exchange - .access_token - .filter(|token| !token.trim().is_empty()) - .context("Compose credential exchange returned no access token")?; - purpose_token_authorization_header(&access_token) - .context("Compose credential exchange returned an invalid access token")?; - let expires_in_seconds = exchange - .expires_in_seconds - .filter(|seconds| *seconds > 0) - .context("Compose credential exchange returned no positive expiry")?; - let issued_at_unix_seconds = unix_time_seconds()?; - let expires_at_unix_seconds = issued_at_unix_seconds - .checked_add(expires_in_seconds) - .context("Compose credential exchange returned an invalid expiry")?; - Ok(PurposeTokenExchangeOutcome::Issued( - StoredPurposeTokenCredential { - access_token, - token_type: "Bearer".to_string(), - issued_at_unix_seconds, - expires_at_unix_seconds, - session_credential_sha256: self.session_credential_sha256.clone(), - }, - )) - } - - fn cached_authorization_header( - &self, - credential: &StoredPurposeTokenCredential, - ) -> Result> { - if credential.session_credential_sha256 != self.session_credential_sha256 - || credential.token_type != "Bearer" - || credential.access_token.trim().is_empty() - { - return Ok(None); - } - let refresh_after = unix_time_seconds()? - .checked_add(PURPOSE_TOKEN_REFRESH_SKEW.as_secs()) - .context("system time overflow while checking Compose credential")?; - if credential.expires_at_unix_seconds <= refresh_after { - return Ok(None); - } - purpose_token_authorization_header(&credential.access_token) - .map(Some) - .context("cached Compose credential is invalid") + fn authorization_header(&self) -> HeaderValue { + self.authorization.clone() } - fn read_cached_authorization_header( - &self, - ) -> Result> { - let Some(credential) = self.storage.get_purpose_token(&self.storage_key)? else { - return Ok(None); - }; - let Some(header) = self.cached_authorization_header(&credential)? else { - return Ok(None); - }; - Ok(Some((credential, header))) - } - - fn refresh_lock(&self) -> Result { - if let Some(parent) = self.refresh_lock_path.parent() { - fs::create_dir_all(parent).with_context(|| format!("create {}", parent.display()))?; - } - let file = OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(false) - .open(&self.refresh_lock_path) - .with_context(|| format!("open {}", self.refresh_lock_path.display()))?; - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - fs::set_permissions(&self.refresh_lock_path, fs::Permissions::from_mode(0o600)) - .with_context(|| format!("chmod 600 {}", self.refresh_lock_path.display()))?; - } - FileExt::lock_exclusive(&file) - .with_context(|| format!("lock {}", self.refresh_lock_path.display()))?; - Ok(file) - } - - fn refresh_or_adopt(&self, rejected: Option<&HeaderValue>) -> Result> { - let _process_guard = self - .refresh_mutex - .lock() - .map_err(|_| anyhow::anyhow!("Compose credential cache lock was poisoned"))?; - let _file_guard = self.refresh_lock()?; - - let cached = self.read_cached_authorization_header()?; - if let Some((credential, header)) = cached { - if rejected.is_none() || rejected != Some(&header) { - return Ok(Some(header)); - } - - let replacement_allowed_at = credential - .issued_at_unix_seconds - .saturating_add(PURPOSE_TOKEN_REPLACEMENT_INTERVAL.as_secs()); - if unix_time_seconds()? < replacement_allowed_at { - return Ok(None); - } - } - - match self.exchange_purpose_token()? { - PurposeTokenExchangeOutcome::Issued(credential) => { - let header = self.cached_authorization_header(&credential)?.context( - "Compose credential exchange returned a credential too close to expiry", - )?; - self.storage - .set_purpose_token(&self.storage_key, &credential) - .context("store Compose purpose token")?; - if rejected == Some(&header) { - return Ok(None); - } - Ok(Some(header)) - } - PurposeTokenExchangeOutcome::RateLimited => { - if let Some((_credential, header)) = self.read_cached_authorization_header()? { - if rejected != Some(&header) { - return Ok(Some(header)); - } - } - Err(exchange_http_failure(StatusCode::TOO_MANY_REQUESTS)) - } - } + fn redact(&self, value: &str) -> String { + value.replace(&self.secret, "[REDACTED]") } } -#[derive(Deserialize)] -struct PurposeTokenExchangeResponse { - access_token: Option, - token_type: Option, - expires_in_seconds: Option, +struct ControlPlaneClient { + client: Client, + #[cfg(test)] + test_transport: Option>, + base_url: String, + client_version: HeaderValue, + client_version_text: String, + style: Style, } -enum PurposeTokenExchangeOutcome { - Issued(StoredPurposeTokenCredential), - RateLimited, +#[cfg(test)] +trait ControlPlaneTransport { + fn execute(&self, request: Request) -> reqwest::Result; } -impl ComposeCredentialProvider for KgoosePurposeTokenProvider { - fn authorization_header(&self) -> Result { - if let Some((_credential, header)) = self.read_cached_authorization_header()? { - return Ok(header); - } - self.refresh_or_adopt(None)? - .context("Compose credential exchange did not return a usable credential") - } +#[cfg(test)] +struct LoopbackTestTransport { + client: Client, + base_url: url::Url, +} - fn authorization_header_after_rejection( - &self, - rejected: &HeaderValue, - ) -> Result> { - self.refresh_or_adopt(Some(rejected)) +#[cfg(test)] +impl LoopbackTestTransport { + fn new(base_url: &str, timeout: Duration) -> Result { + Ok(Self { + client: build_auth_http_client(timeout)?, + base_url: validate_apps_e2e_loopback_url(base_url, APPS_E2E_CONTROL_PLANE_URL_ENV_VAR)?, + }) } } -struct ControlPlaneClient { - client: Client, - base_url: String, - client_version: HeaderValue, - client_version_text: String, - style: Style, +#[cfg(test)] +impl ControlPlaneTransport for LoopbackTestTransport { + fn execute(&self, mut request: Request) -> reqwest::Result { + assert!( + is_trusted_control_plane_url(request.url()), + "request must retain its approved production URL until transport execution: {}", + request.url() + ); + let query = request.url().query().map(str::to_string); + let mut loopback_url = self.base_url.clone(); + loopback_url.set_path(request.url().path()); + loopback_url.set_query(query.as_deref()); + *request.url_mut() = loopback_url; + self.client.execute(request) + } } impl ControlPlaneClient { @@ -647,26 +461,34 @@ impl ControlPlaneClient { style: Style, request_timeout: Duration, ) -> Result { - let contract_url = auth_url(base_url, APPS_CONTRACT_PATH) - .context("build Apps Platform control-plane contract URL")?; - if !matches!(contract_url.scheme(), "http" | "https") { - anyhow::bail!("Apps Platform control-plane URL must use http or https"); - } - if contract_url.scheme() == "http" && !is_loopback_url(&contract_url) { - anyhow::bail!( - "Apps Platform control-plane URL must use https unless it targets loopback local development" - ); - } - if !is_trusted_control_plane_url(&contract_url) { - anyhow::bail!( - "Apps Platform control-plane URL must target an approved Builderlab ingress host or loopback local development" - ); - } + validate_control_plane_base_url(base_url)?; + let client = build_control_plane_http_client(request_timeout)?; + let control_plane = Self::build(base_url, client_version, style, client)?; + #[cfg(test)] + let control_plane = { + let mut control_plane = control_plane; + if let Some(loopback_url) = std::env::var_os(APPS_E2E_CONTROL_PLANE_URL_ENV_VAR) { + let loopback_url = loopback_url.into_string().map_err(|_| { + anyhow::anyhow!("{APPS_E2E_CONTROL_PLANE_URL_ENV_VAR} must be UTF-8") + })?; + control_plane.test_transport = Some(Box::new(LoopbackTestTransport::new( + &loopback_url, + request_timeout, + )?)); + } + control_plane + }; + Ok(control_plane) + } + + fn build(base_url: &str, client_version: &str, style: Style, client: Client) -> Result { let client_version_text = client_version.to_string(); let client_version = HeaderValue::from_str(client_version) .context("Apps Platform client version is not a valid HTTP header value")?; Ok(Self { - client: build_auth_http_client(request_timeout)?, + client, + #[cfg(test)] + test_transport: None, base_url: base_url.to_string(), client_version, client_version_text, @@ -674,74 +496,83 @@ impl ControlPlaneClient { }) } + #[cfg(test)] + fn new_for_test( + base_url: &str, + client_version: &str, + style: Style, + request_timeout: Duration, + transport: Box, + ) -> Result { + validate_control_plane_base_url(base_url)?; + let mut client = Self::build( + base_url, + client_version, + style, + build_auth_http_client(request_timeout)?, + )?; + client.test_transport = Some(transport); + Ok(client) + } + fn client_version_text(&self) -> &str { &self.client_version_text } - fn contract(&self, credential_provider: &dyn ComposeCredentialProvider) -> Result { + fn contract(&self, credential: &ComposeSessionCredential) -> Result { let url = self.endpoint(APPS_CONTRACT_PATH)?; - self.authorized_json_request( - credential_provider, - "GET", - APPS_CONTRACT_PATH, - |authorization| { - self.standard_request(self.client.get(url.clone()), authorization) - .send() - .map_err(|error| network_failure("GET", APPS_CONTRACT_PATH, error)) - }, - ) + self.authorized_json_request(credential, "GET", APPS_CONTRACT_PATH, |authorization| { + self.standard_request(self.client.get(url.clone()), authorization) + .build() + .context("build Apps Platform contract request") + }) } fn plan( &self, - credential_provider: &dyn ComposeCredentialProvider, + credential: &ComposeSessionCredential, request: &PlanRequest<'_>, ) -> Result { let url = self.endpoint(APPS_PLAN_PATH)?; - self.authorized_json_request( - credential_provider, - "POST", - APPS_PLAN_PATH, - |authorization| { - self.standard_request(self.client.post(url.clone()), authorization) - .json(request) - .send() - .map_err(|error| network_failure("POST", APPS_PLAN_PATH, error)) - }, - ) + self.authorized_json_request(credential, "POST", APPS_PLAN_PATH, |authorization| { + self.standard_request(self.client.post(url.clone()), authorization) + .json(request) + .build() + .context("build Apps Platform plan request") + }) } fn initialize( &self, - credential_provider: &dyn ComposeCredentialProvider, + credential: &ComposeSessionCredential, app_id: &str, request: &Value, ) -> Result { let url = self.app_action_url(app_id, "initialize")?; let path = url.path().to_string(); - self.authorized_json_request(credential_provider, "POST", &path, |authorization| { + self.authorized_json_request(credential, "POST", &path, |authorization| { self.standard_request(self.client.post(url.clone()), authorization) .json(request) - .send() - .map_err(|error| network_failure("POST", &path, error)) + .build() + .context("build Apps Platform initialize request") }) } fn deploy( &self, - credential_provider: &dyn ComposeCredentialProvider, + credential: &ComposeSessionCredential, app_id: &str, artifact: &Path, options: &DeployOptions, ) -> Result { let url = self.app_action_url(app_id, "deploy")?; let path = url.path().to_string(); - self.authorized_json_request(credential_provider, "POST", &path, |authorization| { + self.authorized_json_request(credential, "POST", &path, |authorization| { let form = deploy_form(artifact, options)?; self.standard_request(self.client.post(url.clone()), authorization) .multipart(form) - .send() - .map_err(|error| network_failure("POST", &path, error)) + .build() + .context("build Apps Platform deploy request") }) } @@ -778,29 +609,26 @@ impl ControlPlaneClient { fn authorized_json_request( &self, - credential_provider: &dyn ComposeCredentialProvider, + credential: &ComposeSessionCredential, method: &str, path: &str, send: F, ) -> Result where - F: Fn(HeaderValue) -> Result, + F: Fn(HeaderValue) -> Result, { - let authorization = credential_provider.authorization_header()?; - let (mut status, mut body) = - self.request_response(method, path, &send, authorization.clone())?; - if status == StatusCode::UNAUTHORIZED { - if let Some(replacement) = - credential_provider.authorization_header_after_rejection(&authorization)? - { - (status, body) = self.request_response(method, path, &send, replacement)?; - } - } + let authorization = credential.authorization_header(); + let (status, body) = self.request_response(method, path, &send, authorization)?; if !status.is_success() { - return Err(control_plane_http_failure(method, path, status, &body)); + return Err(control_plane_http_failure( + method, path, status, &body, credential, + )); } - serde_json::from_str(&body) - .with_context(|| format!("parse Apps Platform {method} {path} response")) + let mut value = serde_json::from_str(&body) + .with_context(|| format!("parse Apps Platform {method} {path} response"))?; + redact_json_value(&mut value, credential) + .with_context(|| format!("sanitize Apps Platform {method} {path} response"))?; + Ok(value) } fn request_response( @@ -811,10 +639,13 @@ impl ControlPlaneClient { authorization: HeaderValue, ) -> Result<(StatusCode, String)> where - F: Fn(HeaderValue) -> Result, + F: Fn(HeaderValue) -> Result, { self.style.verbose(&format!("{method} {path}")); - let response = send(authorization)?; + let request = send(authorization)?; + let response = self + .execute_request(request) + .map_err(|error| network_failure(method, path, error))?; let status = response.status(); let body = read_limited_response_body( response, @@ -827,6 +658,69 @@ impl ControlPlaneClient { )); Ok((status, body)) } + + fn execute_request(&self, request: Request) -> reqwest::Result { + #[cfg(test)] + if let Some(transport) = self.test_transport.as_ref() { + return transport.execute(request); + } + self.client.execute(request) + } +} + +fn build_control_plane_http_client(timeout: Duration) -> Result { + Client::builder() + .redirect(Policy::none()) + .timeout(timeout) + .build() + .context("build Apps Platform control-plane HTTP client") +} + +fn redact_json_value(value: &mut Value, credential: &ComposeSessionCredential) -> Result<()> { + match value { + Value::String(text) => *text = credential.redact(text), + Value::Array(items) => { + for item in items { + redact_json_value(item, credential)?; + } + } + Value::Object(object) => { + for (key, value) in object { + if credential.redact(key) != *key { + anyhow::bail!( + "Apps Platform response contained the session credential in an object key" + ); + } + redact_json_value(value, credential)?; + } + } + Value::Null | Value::Bool(_) | Value::Number(_) => {} + } + Ok(()) +} + +#[cfg(test)] +fn validate_apps_e2e_loopback_url(value: &str, name: &str) -> Result { + let url = url::Url::parse(value).with_context(|| format!("parse {name}"))?; + let loopback_ip = match url.host() { + Some(url::Host::Ipv4(address)) => address.is_loopback(), + Some(url::Host::Ipv6(address)) => address.is_loopback(), + Some(url::Host::Domain(_)) | None => false, + }; + if url.scheme() != "http" + || !loopback_ip + || url.port().is_none() + || !url.username().is_empty() + || url.password().is_some() + || url.path() != "/" + || url.query().is_some() + || url.fragment().is_some() + { + anyhow::bail!( + "{name} must be an HTTP loopback IP origin with an explicit port and no userinfo, path, query, or fragment" + ); + } + Ok(url) } fn deploy_form(artifact: &Path, options: &DeployOptions) -> Result { @@ -849,10 +743,11 @@ fn deploy_form(artifact: &Path, options: &DeployOptions) -> Result bool { - if is_loopback_url(url) { - return true; - } - if url.scheme() != "https" || url.port_or_known_default() != Some(443) { + if url.scheme() != "https" + || url.port_or_known_default() != Some(443) + || !url.username().is_empty() + || url.password().is_some() + { return false; } let Some(url::Host::Domain(host)) = url.host() else { @@ -863,13 +758,15 @@ fn is_trusted_control_plane_url(url: &url::Url) -> bool { .any(|trusted| host.eq_ignore_ascii_case(trusted)) } -fn is_loopback_url(url: &url::Url) -> bool { - match url.host() { - Some(url::Host::Domain(domain)) => domain.eq_ignore_ascii_case("localhost"), - Some(url::Host::Ipv4(address)) => address.is_loopback(), - Some(url::Host::Ipv6(address)) => address.is_loopback(), - None => false, +fn validate_control_plane_base_url(base_url: &str) -> Result<()> { + let contract_url = auth_url(base_url, APPS_CONTRACT_PATH) + .context("build Apps Platform control-plane contract URL")?; + if !is_trusted_control_plane_url(&contract_url) { + anyhow::bail!( + "Apps Platform control-plane URL must use HTTPS and target an approved Builderlab ingress host" + ); } + Ok(()) } fn read_limited_response_body( @@ -888,25 +785,6 @@ fn read_limited_response_body( String::from_utf8(bytes).with_context(|| format!("decode {description} response as UTF-8")) } -fn purpose_token_authorization_header(access_token: &str) -> Result { - HeaderValue::from_str(&format!("Bearer {access_token}")) - .context("build Compose authorization header") -} - -fn unix_time_seconds() -> Result { - SystemTime::now() - .duration_since(UNIX_EPOCH) - .context("system clock is before the Unix epoch") - .map(|duration| duration.as_secs()) -} - -fn sha256(value: &str) -> String { - Sha256::digest(value.as_bytes()) - .iter() - .map(|byte| format!("{byte:02x}")) - .collect() -} - fn apps_user_agent() -> String { format!("bb-apps/{}", env!("CARGO_PKG_VERSION")) } @@ -927,38 +805,12 @@ fn network_failure(method: &str, path: &str, error: reqwest::Error) -> anyhow::E ) } -fn exchange_http_failure(status: StatusCode) -> anyhow::Error { - let (exit_code, code, hint) = match status.as_u16() { - 401 => ( - exit_codes::AUTH_REQUIRED, - "auth_required", - "; run `bb auth login` to refresh your session", - ), - 403 => ( - exit_codes::FORBIDDEN, - "forbidden", - "; the current account does not have Builderlab access", - ), - 429 => ( - exit_codes::GENERAL, - "credential_exchange_rate_limited", - "; retry later", - ), - value if value >= 500 => (exit_codes::NETWORK, "credential_exchange_unavailable", ""), - _ => (exit_codes::GENERAL, "credential_exchange_failed", ""), - }; - failure( - exit_code, - code, - format!("Compose credential exchange failed with {status}{hint}"), - ) -} - fn control_plane_http_failure( method: &str, path: &str, status: StatusCode, body: &str, + credential: &ComposeSessionCredential, ) -> anyhow::Error { let parsed = serde_json::from_str::(body).ok(); let code = parsed @@ -966,18 +818,25 @@ fn control_plane_http_failure( .and_then(|value| value.pointer("/error/code")) .and_then(Value::as_str) .unwrap_or("control_plane_request_failed"); - let next_action = parsed - .as_ref() - .and_then(|value| { - value - .get("next_action") - .or_else(|| value.pointer("/error/next_action")) - }) - .and_then(Value::as_str); + let code = credential.redact(&terminal_safe_text(code)); + let next_action = if status == StatusCode::UNAUTHORIZED { + Some("Run `bb auth logout`, then `bb auth login` to replace your session.".to_string()) + } else { + parsed + .as_ref() + .and_then(|value| { + value + .get("next_action") + .or_else(|| value.pointer("/error/next_action")) + }) + .and_then(Value::as_str) + .map(terminal_safe_text) + .map(|value| credential.redact(&value)) + }; let mut message = format!("{method} {path} failed with {status}"); if let Some(next_action) = next_action { message.push_str("\nnext_action: "); - message.push_str(&terminal_safe_text(next_action)); + message.push_str(&next_action); } let exit_code = match status.as_u16() { 401 => exit_codes::AUTH_REQUIRED, @@ -985,112 +844,602 @@ fn control_plane_http_failure( value if value >= 500 => exit_codes::NETWORK, _ => exit_codes::GENERAL, }; - failure(exit_code, code, message) + failure(exit_code, &code, message) } #[cfg(test)] mod tests { + use std::collections::{BTreeMap, VecDeque}; + use std::process::Command as ProcessCommand; + use std::sync::{Arc, Mutex}; use std::thread; - use builderbot_auth::auth_storage::{FileSessionCredentialStorage, SessionStorageKey}; + use sha2::{Digest, Sha256}; use tiny_http::{Header, Response, Server}; use super::*; - fn test_provider( - exchange_url: url::Url, - temporary_directory: &tempfile::TempDir, - ) -> KgoosePurposeTokenProvider { - let session_credential = "test-bbidentity-session"; - let session_storage_key = SessionStorageKey::new("test", exchange_url.as_str()); - KgoosePurposeTokenProvider { - client: build_auth_http_client(TOKEN_EXCHANGE_REQUEST_TIMEOUT) - .expect("build HTTP client"), - exchange_url, - session_credential: HeaderValue::from_static(session_credential), - session_credential_sha256: sha256(session_credential), - baggage: None, - style: Style::new(true, false, false), - storage: Box::new(FileSessionCredentialStorage::new( - temporary_directory.path().join("sessions.json"), - )), - storage_key: PurposeTokenStorageKey::new(&session_storage_key, COMPOSE_TOKEN_PURPOSE), - refresh_lock_path: temporary_directory.path().join(PURPOSE_TOKEN_LOCK_FILE), - refresh_mutex: Mutex::new(()), + const APPROVED_TEST_BASE_URL: &str = "https://compose-ctrl.test.blockstaging.build"; + const PROCESS_STDOUT_BEGIN: &str = "BB_APPS_E2E_STDOUT_BEGIN"; + const PROCESS_STDOUT_END: &str = "BB_APPS_E2E_STDOUT_END"; + + #[derive(Clone)] + struct ProcessResponse { + status: u16, + body: Value, + } + + impl ProcessResponse { + fn json(body: Value) -> Self { + Self { status: 200, body } + } + } + + #[derive(Clone)] + struct ProcessRequest { + method: String, + path: String, + headers: BTreeMap, + body: Value, + body_bytes: Vec, + } + + struct ProcessServer { + base_url: String, + requests: Arc>>, + handle: Option>, + } + + impl ProcessServer { + fn start(responses: Vec) -> Self { + let server = Server::http("127.0.0.1:0").expect("bind Apps process test server"); + let base_url = format!("http://{}", server.server_addr()); + let requests = Arc::new(Mutex::new(Vec::new())); + let thread_requests = Arc::clone(&requests); + let handle = thread::spawn(move || { + let mut responses = VecDeque::from(responses); + while let Some(response) = responses.pop_front() { + let mut request = server + .recv_timeout(Duration::from_secs(10)) + .expect("receive Apps process request") + .expect("Apps process request before timeout"); + let headers = request + .headers() + .iter() + .map(|header| { + ( + header.field.as_str().to_string().to_ascii_lowercase(), + header.value.as_str().to_string(), + ) + }) + .collect::>(); + let mut body_bytes = Vec::new(); + request + .as_reader() + .read_to_end(&mut body_bytes) + .expect("read Apps process request"); + let body = if headers + .get("content-type") + .is_some_and(|value| value.starts_with("application/json")) + { + serde_json::from_slice(&body_bytes) + .expect("parse Apps process JSON request") + } else { + Value::Null + }; + thread_requests + .lock() + .expect("lock Apps process requests") + .push(ProcessRequest { + method: request.method().as_str().to_string(), + path: request.url().to_string(), + headers, + body, + body_bytes, + }); + request + .respond( + Response::from_string(response.body.to_string()) + .with_status_code(response.status) + .with_header( + Header::from_bytes("Content-Type", "application/json") + .expect("build Apps process content type"), + ), + ) + .expect("respond to Apps process request"); + } + }); + Self { + base_url, + requests, + handle: Some(handle), + } + } + + fn finish(mut self) -> Vec { + self.handle + .take() + .expect("Apps process server handle") + .join() + .expect("join Apps process server"); + self.requests + .lock() + .expect("lock Apps process requests") + .clone() } } #[test] - fn purpose_token_provider_reuses_token_across_provider_instances() { - let temporary_directory = tempfile::tempdir().expect("create temporary directory"); - let server = Server::http("127.0.0.1:0").expect("bind token exchange server"); - let exchange_url = url::Url::parse(&format!( - "http://{}{COMPOSE_TOKEN_EXCHANGE_PATH}", - server.server_addr() - )) - .expect("parse exchange URL"); - let server_thread = thread::spawn(move || { - let request = server.recv().expect("receive token exchange"); - assert_eq!(request.method().as_str(), "POST"); - assert_eq!(request.url(), COMPOSE_TOKEN_EXCHANGE_PATH); + fn bb_apps_e2e_process_helper() { + let Some(args) = std::env::var_os("BB_APPS_E2E_ARGS") else { + return; + }; + let args = serde_json::from_str::>( + args.to_str().expect("BB_APPS_E2E_ARGS must be UTF-8"), + ) + .expect("parse BB_APPS_E2E_ARGS"); + let auth_url = std::env::var(APPS_E2E_AUTH_URL_ENV_VAR) + .expect("Apps E2E helper requires an explicit auth URL"); + let auth_url = validate_apps_e2e_loopback_url(&auth_url, APPS_E2E_AUTH_URL_ENV_VAR) + .expect("validate Apps E2E auth URL"); + let credential = std::env::var(APPS_E2E_CREDENTIAL_ENV_VAR) + .expect("Apps E2E helper requires an explicit synthetic credential"); + assert!( + credential.starts_with("apps-e2e-only."), + "Apps E2E helper accepts only synthetic test credentials" + ); + let temp = tempfile::tempdir().expect("create isolated Apps E2E home"); + let bb_home = temp.path().join("bb-home"); + let storage_path = temp.path().join("auth-sessions.json"); + fs::create_dir_all(&bb_home).expect("create isolated Apps E2E bb home"); + fs::write(bb_home.join("config.yaml"), "org: test\n") + .expect("write isolated Apps E2E config"); + let service_url = format!("{}/api/goose", auth_url.as_str().trim_end_matches('/')); + let mut hasher = Sha256::new(); + hasher.update(b"default"); + hasher.update([0]); + hasher.update(service_url.as_bytes()); + let storage_key = hasher + .finalize() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::(); + fs::write( + &storage_path, + serde_json::to_vec_pretty(&json!({ + storage_key: { + "sessionCredential": credential, + "expiresAt": "2099-01-01T00:00:00Z" + } + })) + .expect("serialize isolated Apps E2E storage"), + ) + .expect("write isolated Apps E2E storage"); + std::env::set_var("BB_HOME", &bb_home); + std::env::set_var("BB_AUTH_STORAGE", "file"); + std::env::set_var("BB_AUTH_STORAGE_FILE", &storage_path); + std::env::set_var("KGOOSE_BASE_URL", auth_url.as_str()); + std::env::remove_var("BB_SKILLS_PROFILE"); + std::env::remove_var("KGOOSE_PLAYPEN"); + println!("{PROCESS_STDOUT_BEGIN}"); + crate::run_bb_with_argv(args).expect("run bb Apps process command"); + println!("{PROCESS_STDOUT_END}"); + } + + fn process_auth_response() -> ProcessResponse { + ProcessResponse::json(json!({ + "subject": "auth0|apps-user", + "email": "apps@example.com", + "name": "Apps User", + "expires_at": "2099-01-01T00:00:00Z", + "workspaces": {"active": [{"name": "Test Workspace"}]} + })) + } + + fn process_command( + auth_server: &ProcessServer, + control_plane: &ProcessServer, + args: &[&str], + credential: &str, + ) -> ProcessCommand { + assert!(credential.starts_with("apps-e2e-only.")); + let argv = std::iter::once("bb") + .chain(args.iter().copied()) + .map(str::to_string) + .collect::>(); + let mut command = ProcessCommand::new(std::env::current_exe().expect("current test exe")); + command + .args([ + "--exact", + "bb::apps::tests::bb_apps_e2e_process_helper", + "--nocapture", + ]) + .env("BB_APPS_E2E_ARGS", serde_json::to_string(&argv).unwrap()) + .env(APPS_E2E_CONTROL_PLANE_URL_ENV_VAR, &control_plane.base_url) + .env(APPS_E2E_AUTH_URL_ENV_VAR, &auth_server.base_url) + .env(APPS_E2E_CREDENTIAL_ENV_VAR, credential) + .env_remove("BB_HOME") + .env_remove("BB_AUTH_STORAGE") + .env_remove("BB_AUTH_STORAGE_FILE") + .env_remove("KGOOSE_BASE_URL") + .env_remove("BB_SKILLS_PROFILE") + .env_remove("KGOOSE_PLAYPEN"); + command + } + + fn process_stdout(output: &std::process::Output) -> String { + let stdout = String::from_utf8(output.stdout.clone()).expect("Apps process stdout UTF-8"); + let start = stdout + .find(PROCESS_STDOUT_BEGIN) + .expect("Apps process stdout begin marker") + + PROCESS_STDOUT_BEGIN.len(); + let end = stdout[start..] + .find(PROCESS_STDOUT_END) + .map(|offset| start + offset) + .expect("Apps process stdout end marker"); + stdout[start..end].trim().to_string() + } + + fn assert_process_auth(request: &ProcessRequest, credential: &str) { + assert_eq!(request.method, "GET"); + assert_eq!(request.path, "/api/goose/v1/auth/me"); + assert_eq!( request - .respond( - Response::from_string( - r#"{"access_token":"cached-compose-token","token_type":"Bearer","expires_in_seconds":300}"#, - ) - .with_header( - Header::from_bytes("Content-Type", "application/json") - .expect("build content type"), - ), - ) - .expect("respond to token exchange"); + .headers + .get("x-bb-session-credential") + .map(String::as_str), + Some(credential) + ); + } + + fn assert_process_control_plane( + request: &ProcessRequest, + method: &str, + path: &str, + credential: &str, + ) { + assert_eq!(request.method, method); + assert_eq!(request.path, path); + assert_eq!( + request.headers.get("authorization").map(String::as_str), + Some(format!("BBIdentity {credential}").as_str()) + ); + for forbidden in [ + "cookie", + "x-bb-session-credential", + "x-forwarded-user", + "x-forwarded-workspace-id", + ] { + assert!(!request.headers.contains_key(forbidden)); + } + } + + #[test] + fn apps_e2e_destinations_require_explicit_http_loopback_ip_origins() { + for valid in ["http://127.0.0.1:1234", "http://[::1]:4321"] { + assert!(validate_apps_e2e_loopback_url(valid, "test URL").is_ok()); + } + for invalid in [ + "http://192.0.2.1:1234", + "https://127.0.0.1:1234", + "http://localhost:1234", + "http://user@127.0.0.1:1234", + "http://127.0.0.1:1234/path", + "http://127.0.0.1:1234/?query=yes", + "http://127.0.0.1:1234/#fragment", + "http://127.0.0.1", + ] { + let error = validate_apps_e2e_loopback_url(invalid, "test URL") + .expect_err("reject unsafe Apps E2E destination"); + assert!(error.to_string().contains("HTTP loopback IP origin")); + } + } + + #[test] + fn bb_apps_contract_process_covers_auth_dispatch_output_and_redaction() { + let credential = "apps-e2e-only.contract.session+credential"; + let contract = json!({ + "ok": true, + "contract_version": "2026-06-30", + "reflected": credential, + "nested": {"message": format!("prefix {credential} suffix")} }); - let provider = test_provider(exchange_url.clone(), &temporary_directory); + let auth_server = ProcessServer::start(vec![process_auth_response()]); + let control_plane = ProcessServer::start(vec![ProcessResponse::json(contract)]); + let mut command = process_command( + &auth_server, + &control_plane, + &[ + "apps", + "contract", + "--base-url", + APPROVED_TEST_BASE_URL, + "--client-version", + "0.2.0", + "--json", + ], + credential, + ); - let first = provider - .authorization_header() - .expect("first authorization header"); - drop(provider); - let second = test_provider(exchange_url, &temporary_directory) - .authorization_header() - .expect("persisted authorization header"); + let output = command.output().expect("run Apps contract process command"); + assert!( + output.status.success(), + "stderr was: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = process_stdout(&output); + assert!(!stdout.contains(credential)); + let value = serde_json::from_str::(&stdout).expect("parse contract process output"); + assert_eq!(value["contract_version"], "2026-06-30"); + assert_eq!(value["reflected"], "[REDACTED]"); + assert_eq!(value["nested"]["message"], "prefix [REDACTED] suffix"); + let auth_requests = auth_server.finish(); + let control_requests = control_plane.finish(); + assert_process_auth(&auth_requests[0], credential); + assert_process_control_plane(&control_requests[0], "GET", APPS_CONTRACT_PATH, credential); + } + #[test] + fn bb_apps_create_process_runs_plan_and_initialize() { + let credential = "apps-e2e-only.create.session+credential"; + let plan = json!({ + "app_id": "merchant-lookup", + "display_name": "Merchant Lookup", + "environment": "staging", + "persistence": "sqlite", + "runtime_class": "default", + "initialize": {"required": true, "recommended": false} + }); + let initialized = json!({ + "app_id": "merchant-lookup-2", + "external_url": "https://merchant-lookup-2--bpsites.example/" + }); + let auth_server = ProcessServer::start(vec![process_auth_response()]); + let control_plane = ProcessServer::start(vec![ + ProcessResponse::json(plan.clone()), + ProcessResponse::json(initialized.clone()), + ]); + let mut command = process_command( + &auth_server, + &control_plane, + &[ + "apps", + "create", + "--app-id", + "merchant-lookup", + "--name", + "Merchant Lookup", + "--environment", + "staging", + "--runtime-profile", + "fetch-js", + "--persistence", + "sqlite", + "--base-url", + APPROVED_TEST_BASE_URL, + "--client-version", + "0.2.0", + "--json", + ], + credential, + ); + + let output = command.output().expect("run Apps create process command"); + assert!( + output.status.success(), + "stderr was: {}", + String::from_utf8_lossy(&output.stderr) + ); + let value = serde_json::from_str::(&process_stdout(&output)) + .expect("parse create process output"); + assert_eq!(value["app_id"], "merchant-lookup-2"); + assert_eq!(value["initialized"], true); + assert_eq!(value["plan"], plan); + assert_eq!(value["initialize"], initialized); + let auth_requests = auth_server.finish(); + let requests = control_plane.finish(); + assert_process_auth(&auth_requests[0], credential); + assert_eq!(requests.len(), 2); + assert_process_control_plane(&requests[0], "POST", APPS_PLAN_PATH, credential); assert_eq!( - first, - HeaderValue::from_static("Bearer cached-compose-token") + requests[0].body, + json!({ + "app_id": "merchant-lookup", + "name": "Merchant Lookup", + "environment": "staging", + "runtime_profile": "fetch-js", + "persistence": "sqlite", + "client_version": "0.2.0" + }) + ); + assert_process_control_plane( + &requests[1], + "POST", + "/v1/agent/apps/merchant-lookup/initialize", + credential, ); - assert_eq!(second, first); - server_thread.join().expect("join token exchange server"); } #[test] - fn control_plane_restricts_token_recipients() { - let style = Style::new(true, false, false); + fn bb_apps_create_process_skips_unrequested_initialize() { + let credential = "apps-e2e-only.existing.session+credential"; + let plan = json!({ + "app_id": "existing-app", + "external_url": "https://existing-app--bpsites.example/", + "initialize": {"required": false, "recommended": false} + }); + let auth_server = ProcessServer::start(vec![process_auth_response()]); + let control_plane = ProcessServer::start(vec![ProcessResponse::json(plan.clone())]); + let mut command = process_command( + &auth_server, + &control_plane, + &[ + "apps", + "create", + "--app-id", + "existing-app", + "--base-url", + APPROVED_TEST_BASE_URL, + "--client-version", + "0.2.0", + "--json", + ], + credential, + ); - assert!(ControlPlaneClient::new( - "https://compose-ctrl.test.blockstaging.build", - "1.0.0", - style - ) - .is_ok()); - assert!( - ControlPlaneClient::new("https://compose-ctrl.app.builderlab.xyz", "1.0.0", style,) - .is_ok() + let output = command.output().expect("run Apps create process command"); + assert!(output.status.success()); + let value = serde_json::from_str::(&process_stdout(&output)) + .expect("parse create process output"); + assert_eq!(value["app_id"], "existing-app"); + assert_eq!(value["initialized"], false); + assert_eq!(value["initialize"], Value::Null); + let auth_requests = auth_server.finish(); + let requests = control_plane.finish(); + assert_process_auth(&auth_requests[0], credential); + assert_eq!(requests.len(), 1); + assert_process_control_plane(&requests[0], "POST", APPS_PLAN_PATH, credential); + } + + #[test] + fn bb_apps_deploy_process_uploads_multipart_artifact() { + let credential = "apps-e2e-only.deploy.session+credential"; + let deployed = json!({ + "ok": true, + "app_id": "merchant-lookup", + "version_id": "ver-123", + "deployment_id": "dpl-123" + }); + let auth_server = ProcessServer::start(vec![process_auth_response()]); + let control_plane = ProcessServer::start(vec![ProcessResponse::json(deployed.clone())]); + let temp = tempfile::tempdir().expect("create deploy process temp directory"); + let artifact = temp.path().join("prepared-app.tar.gz"); + fs::write(&artifact, "test-hotpod-artifact-marker").expect("write deploy artifact"); + let artifact_text = artifact.to_str().expect("artifact path UTF-8"); + let mut command = process_command( + &auth_server, + &control_plane, + &[ + "apps", + "deploy", + "merchant-lookup", + artifact_text, + "--environment", + "production", + "--version-id", + "ver-123", + "--deployment-id", + "dpl-123", + "--base-url", + APPROVED_TEST_BASE_URL, + "--client-version", + "0.2.0", + "--json", + ], + credential, ); - assert!(ControlPlaneClient::new("http://localhost:8080", "1.0.0", style).is_ok()); - assert!(ControlPlaneClient::new("http://127.0.0.1:8080", "1.0.0", style).is_ok()); - assert!(ControlPlaneClient::new("http://[::1]:8080", "1.0.0", style).is_ok()); - let error = ControlPlaneClient::new( - "http://compose-ctrl.test.blockstaging.build", + let output = command.output().expect("run Apps deploy process command"); + assert!(output.status.success()); + assert_eq!( + serde_json::from_str::(&process_stdout(&output)) + .expect("parse deploy process output"), + deployed + ); + let auth_requests = auth_server.finish(); + let requests = control_plane.finish(); + assert_process_auth(&auth_requests[0], credential); + assert_eq!(requests.len(), 1); + assert_process_control_plane( + &requests[0], + "POST", + "/v1/agent/apps/merchant-lookup/deploy", + credential, + ); + let body = String::from_utf8_lossy(&requests[0].body_bytes); + for expected in [ + "test-hotpod-artifact-marker", + "name=\"environment\"\r\n\r\nproduction", + "name=\"version_id\"\r\n\r\nver-123", + "name=\"deployment_id\"\r\n\r\ndpl-123", + ] { + assert!(body.contains(expected), "multipart omitted {expected:?}"); + } + } + + fn test_control_plane_client(base_url: &str, timeout: Duration) -> ControlPlaneClient { + ControlPlaneClient::new_for_test( + APPROVED_TEST_BASE_URL, "1.0.0", - style, + Style::new(true, false, false), + timeout, + Box::new( + LoopbackTestTransport::new(base_url, timeout) + .expect("build loopback test transport"), + ), ) - .err() - .expect("reject cleartext external URL"); - assert!(error.to_string().contains("must use https")); + .expect("build test control-plane client") + } + + fn test_credential(secret: &str) -> ComposeSessionCredential { + ComposeSessionCredential::new(secret.to_string()).expect("build test credential") + } + + #[test] + fn compose_session_header_matches_kgoose_contract() { + let secret = "opaque.session+credential/with=punctuation"; + let credential = test_credential(secret); + + assert_eq!( + credential + .authorization_header() + .to_str() + .expect("authorization text"), + format!("BBIdentity {secret}") + ); + for invalid in ["credential\r\nInjected: header", "credential\nheader"] { + let error = ComposeSessionCredential::new(invalid.to_string()) + .err() + .expect("reject invalid session credential"); + assert!(!error.to_string().contains(invalid)); + } + } + + #[test] + fn compose_session_uses_the_exact_credential_returned_by_login() { + let secret = "session_stored_after_browser_login_12345"; + let credential = ComposeSessionCredential::from_stored(StoredSessionCredential { + session_credential: secret.to_string(), + expires_at: Some("2099-01-01T00:00:00Z".to_string()), + }) + .expect("use returned login credential"); + + assert_eq!( + credential + .authorization_header() + .to_str() + .expect("authorization text"), + format!("BBIdentity {secret}") + ); + } + + #[test] + fn control_plane_allowlist_is_exact_and_https_only() { + let style = Style::new(true, false, false); + + for trusted in [ + "https://compose-ctrl.test.blockstaging.build", + "https://compose-ctrl.app.builderlab.xyz", + "https://compose-ctrl.test.blockstaging.build:443", + ] { + assert!( + ControlPlaneClient::new(trusted, "1.0.0", style).is_ok(), + "allowlisted control-plane origin should be accepted" + ); + } for untrusted in [ + "http://compose-ctrl.test.blockstaging.build", "https://attacker.example", "https://test.blockstaging.build", "https://app.builderlab.xyz", @@ -1098,8 +1447,13 @@ mod tests { "https://compose-ctrl.test.blockstaging.build:444", "https://compose-ctrl.app.builderlab.xyz.attacker.example", "https://compose-ctrl.app.builderlab.xyz:444", - "https://test.blockstaging.build.attacker.example", - "https://test.blockstaging.build:444", + "https://user@compose-ctrl.test.blockstaging.build", + "http://localhost:8080", + "https://localhost:8080", + "http://127.0.0.1:8080", + "https://127.0.0.1:8443", + "http://[::1]:8080", + "https://[::1]:8443", ] { let error = ControlPlaneClient::new(untrusted, "1.0.0", style) .err() @@ -1109,198 +1463,169 @@ mod tests { } #[test] - fn cached_token_requires_current_session_and_safe_expiry() { - let temporary_directory = tempfile::tempdir().expect("create temporary directory"); - let provider = test_provider( - url::Url::parse("http://127.0.0.1:9/v1/auth/token/compose") - .expect("parse exchange URL"), - &temporary_directory, - ); - let now = unix_time_seconds().expect("read system time"); - let mut credential = StoredPurposeTokenCredential { - access_token: "cached-token".to_string(), - token_type: "Bearer".to_string(), - issued_at_unix_seconds: now, - expires_at_unix_seconds: now + 300, - session_credential_sha256: "different-session".to_string(), - }; - - assert!(provider - .cached_authorization_header(&credential) - .expect("validate different session") - .is_none()); - - credential.session_credential_sha256 = provider.session_credential_sha256.clone(); - credential.expires_at_unix_seconds = now + PURPOSE_TOKEN_REFRESH_SKEW.as_secs(); - assert!(provider - .cached_authorization_header(&credential) - .expect("validate near-expiry token") - .is_none()); - - credential.expires_at_unix_seconds = now + 300; - assert_eq!( - provider - .cached_authorization_header(&credential) - .expect("validate reusable token"), - Some(HeaderValue::from_static("Bearer cached-token")) - ); - } - - #[test] - fn control_plane_retries_once_with_rotated_cached_token() { - struct RotatingCredentialProvider; - - impl ComposeCredentialProvider for RotatingCredentialProvider { - fn authorization_header(&self) -> Result { - Ok(HeaderValue::from_static("Bearer rejected-token")) - } - - fn authorization_header_after_rejection( - &self, - rejected: &HeaderValue, - ) -> Result> { - assert_eq!(rejected, HeaderValue::from_static("Bearer rejected-token")); - Ok(Some(HeaderValue::from_static("Bearer rotated-token"))) - } - } - + fn control_plane_uses_bbidentity_authorization_without_identity_headers() { + let secret = "opaque_session_credential_1234567890"; let server = Server::http("127.0.0.1:0").expect("bind control-plane server"); let base_url = format!("http://{}", server.server_addr()); let server_thread = thread::spawn(move || { - let first = server.recv().expect("receive first contract request"); + let request = server.recv().expect("receive contract request"); + assert_eq!(request.method().as_str(), "GET"); + assert_eq!(request.url(), APPS_CONTRACT_PATH); assert_eq!( - first + request .headers() .iter() .find(|header| header.field.equiv("Authorization")) .map(|header| header.value.as_str()), - Some("Bearer rejected-token") + Some("BBIdentity opaque_session_credential_1234567890") ); - first - .respond(Response::from_string("unauthorized").with_status_code(401)) - .expect("reject first contract request"); - - let second = server.recv().expect("receive retried contract request"); - assert_eq!( - second + for forbidden in [ + "Cookie", + "X-BB-Session-Credential", + "X-Forwarded-User", + "X-Forwarded-Workspace-Id", + ] { + assert!(!request .headers() .iter() - .find(|header| header.field.equiv("Authorization")) - .map(|header| header.value.as_str()), - Some("Bearer rotated-token") - ); - second + .any(|header| header.field.equiv(forbidden))); + } + request .respond( Response::from_string(r#"{"contract_version":"test"}"#).with_header( Header::from_bytes("Content-Type", "application/json") .expect("build content type"), ), ) - .expect("respond to retried contract request"); + .expect("respond to contract request"); }); - let client = ControlPlaneClient::new(&base_url, "1.0.0", Style::new(true, false, false)) - .expect("build control-plane client"); + let client = test_control_plane_client(&base_url, Duration::from_secs(2)); let contract = client - .contract(&RotatingCredentialProvider) - .expect("retry contract request"); + .contract(&test_credential(secret)) + .expect("read contract"); assert_eq!(contract["contract_version"], "test"); - server_thread.join().expect("join control-plane server"); + server_thread.join().expect("join request server"); } #[test] - fn deploy_retries_once_and_reopens_the_artifact() { - struct RotatingCredentialProvider; + fn control_plane_does_not_follow_redirects_or_forward_the_session() { + let target = Server::http("127.0.0.1:0").expect("bind redirect target"); + let target_url = format!("http://{}/stolen", target.server_addr()); + let redirector = Server::http("127.0.0.1:0").expect("bind redirector"); + let base_url = format!("http://{}", redirector.server_addr()); + let redirect_thread = thread::spawn(move || { + let request = redirector.recv().expect("receive original request"); + request + .respond(Response::empty(302).with_header( + Header::from_bytes("Location", target_url).expect("build redirect header"), + )) + .expect("send redirect"); + }); + let client = test_control_plane_client(&base_url, Duration::from_secs(2)); + let secret = "redirect_session_credential_123456"; - impl ComposeCredentialProvider for RotatingCredentialProvider { - fn authorization_header(&self) -> Result { - Ok(HeaderValue::from_static("Bearer rejected-token")) - } + let error = client + .contract(&test_credential(secret)) + .expect_err("reject redirect response"); + + assert!(error.to_string().contains("302")); + assert!(!error.to_string().contains(secret)); + assert!(target + .recv_timeout(Duration::from_millis(250)) + .expect("wait for redirect target") + .is_none()); + redirect_thread.join().expect("join redirect server"); + } - fn authorization_header_after_rejection( - &self, - rejected: &HeaderValue, - ) -> Result> { - assert_eq!(rejected, HeaderValue::from_static("Bearer rejected-token")); - Ok(Some(HeaderValue::from_static("Bearer rotated-token"))) - } - } + #[test] + fn expired_session_is_not_retried_and_returns_login_guidance() { + let server = Server::http("127.0.0.1:0").expect("bind control-plane server"); + let base_url = format!("http://{}", server.server_addr()); + let server_thread = thread::spawn(move || { + let request = server.recv().expect("receive expired session request"); + request + .respond(Response::from_string("expired").with_status_code(401)) + .expect("reject expired session"); + assert!(server + .recv_timeout(Duration::from_millis(250)) + .expect("wait for unexpected retry") + .is_none()); + }); + let client = test_control_plane_client(&base_url, Duration::from_secs(2)); + let secret = "expired_session_credential_1234567"; - let temporary_directory = tempfile::tempdir().expect("create temporary directory"); - let artifact_path = temporary_directory.path().join("artifact.tar.gz"); - fs::write(&artifact_path, b"retryable-artifact-marker").expect("write artifact"); + let error = client + .contract(&test_credential(secret)) + .expect_err("reject expired session"); + let message = error.to_string(); + + assert!(message.contains("401")); + assert!(message.contains("bb auth logout")); + assert!(message.contains("bb auth login")); + assert!(!message.contains(secret)); + server_thread.join().expect("join request server"); + } + + #[test] + fn control_plane_errors_redact_the_session_credential() { let server = Server::http("127.0.0.1:0").expect("bind control-plane server"); let base_url = format!("http://{}", server.server_addr()); + let secret = "reflected_session_credential_123456"; + let response_body = json!({ + "error": {"code": secret}, + "next_action": format!("remove {secret} from the request") + }) + .to_string(); let server_thread = thread::spawn(move || { - for (index, expected_token) in ["Bearer rejected-token", "Bearer rotated-token"] - .into_iter() - .enumerate() - { - let mut request = server.recv().expect("receive deploy request"); - assert_eq!(request.method().as_str(), "POST"); - assert_eq!(request.url(), "/v1/agent/apps/retry-app/deploy"); - assert_eq!( - request - .headers() - .iter() - .find(|header| header.field.equiv("Authorization")) - .map(|header| header.value.as_str()), - Some(expected_token) - ); - let mut body = Vec::new(); - request - .as_reader() - .read_to_end(&mut body) - .expect("read deploy body"); - assert!(body - .windows(b"retryable-artifact-marker".len()) - .any(|window| window == b"retryable-artifact-marker")); - if index == 0 { - request - .respond(Response::from_string("unauthorized").with_status_code(401)) - .expect("reject first deploy request"); - } else { - request - .respond( - Response::from_string(r#"{"ok":true,"version_id":"ver-test"}"#) - .with_header( - Header::from_bytes("Content-Type", "application/json") - .expect("build content type"), - ), - ) - .expect("respond to retried deploy request"); - } - } + let request = server.recv().expect("receive request"); + request + .respond(Response::from_string(response_body).with_status_code(400)) + .expect("send reflected error"); }); - let client = ControlPlaneClient::new(&base_url, "1.0.0", Style::new(true, false, false)) - .expect("build control-plane client"); + let client = test_control_plane_client(&base_url, Duration::from_secs(2)); - let response = client - .deploy( - &RotatingCredentialProvider, - "retry-app", - &artifact_path, - &DeployOptions::default(), - ) - .expect("retry deploy request"); + let error = client + .contract(&test_credential(secret)) + .expect_err("reject failed request"); + let message = format!("{error:#}"); - assert_eq!(response["version_id"], "ver-test"); - server_thread.join().expect("join control-plane server"); + assert!(!message.contains(secret)); + assert!(message.contains("[REDACTED]")); + server_thread.join().expect("join request server"); } #[test] - fn initialize_and_deploy_allow_delayed_rollout_responses() { - struct StaticCredentialProvider; + fn successful_response_rejects_secret_bearing_keys_without_collisions() { + let server = Server::http("127.0.0.1:0").expect("bind control-plane server"); + let base_url = format!("http://{}", server.server_addr()); + let secret = "reflected_key_session_credential_123456"; + let mut nested = Map::new(); + nested.insert(secret.to_string(), json!("secret-key value")); + nested.insert("[REDACTED]".to_string(), json!("existing value")); + let response_body = json!({"nested": Value::Object(nested)}).to_string(); + let server_thread = thread::spawn(move || { + let request = server.recv().expect("receive request"); + request + .respond(Response::from_string(response_body)) + .expect("send reflected key response"); + }); + let client = test_control_plane_client(&base_url, Duration::from_secs(2)); - impl ComposeCredentialProvider for StaticCredentialProvider { - fn authorization_header(&self) -> Result { - Ok(HeaderValue::from_static("Bearer test-token")) - } - } + let error = client + .contract(&test_credential(secret)) + .expect_err("reject a successful response with the session in an object key"); + let message = format!("{error:#}"); + + assert!(message.contains("object key")); + assert!(!message.contains(secret)); + server_thread.join().expect("join request server"); + } + #[test] + fn initialize_and_deploy_allow_delayed_rollout_responses() { assert!(CONTROL_PLANE_REQUEST_TIMEOUT > Duration::from_secs(2 * 60)); - assert_eq!(TOKEN_EXCHANGE_REQUEST_TIMEOUT, Duration::from_secs(30)); let temporary_directory = tempfile::tempdir().expect("create temporary directory"); let artifact_path = temporary_directory.path().join("artifact.tar.gz"); @@ -1343,24 +1668,18 @@ mod tests { ) .expect("respond to deploy request"); }); - let client = ControlPlaneClient::new_with_timeout( - &base_url, - "1.0.0", - Style::new(true, false, false), - Duration::from_secs(1), - ) - .expect("build control-plane client"); + let client = test_control_plane_client(&base_url, Duration::from_secs(1)); let initialized = client .initialize( - &StaticCredentialProvider, + &test_credential("delayed_session_credential_123456"), "delayed-app", &json!({"environment": "staging"}), ) .expect("wait for delayed initialize response"); let deployed = client .deploy( - &StaticCredentialProvider, + &test_credential("delayed_session_credential_123456"), "delayed-app", &artifact_path, &DeployOptions::default(), @@ -1374,14 +1693,6 @@ mod tests { #[test] fn control_plane_bounds_plan_responses() { - struct StaticCredentialProvider; - - impl ComposeCredentialProvider for StaticCredentialProvider { - fn authorization_header(&self) -> Result { - Ok(HeaderValue::from_static("Bearer test-token")) - } - } - let server = Server::http("127.0.0.1:0").expect("bind control-plane server"); let base_url = format!("http://{}", server.server_addr()); let server_thread = thread::spawn(move || { @@ -1394,8 +1705,7 @@ mod tests { ])) .expect("respond with oversized plan response"); }); - let client = ControlPlaneClient::new(&base_url, "1.0.0", Style::new(true, false, false)) - .expect("build control-plane client"); + let client = test_control_plane_client(&base_url, Duration::from_secs(2)); let request = PlanRequest { app_id: Some("bounded-app"), name: None, @@ -1406,22 +1716,22 @@ mod tests { }; let error = client - .plan(&StaticCredentialProvider, &request) + .plan( + &test_credential("bounded_session_credential_123456"), + &request, + ) .expect_err("reject oversized plan response"); assert!(error.to_string().contains("exceeded 2097152 bytes")); - assert!(!error.to_string().contains("test-token")); + assert!(!error + .to_string() + .contains("bounded_session_credential_123456")); server_thread.join().expect("join control-plane server"); } #[test] fn app_ids_are_encoded_as_single_path_segments() { - let client = ControlPlaneClient::new( - "http://127.0.0.1:9", - "1.0.0", - Style::new(true, false, false), - ) - .expect("build control-plane client"); + let client = test_control_plane_client("http://127.0.0.1:9", Duration::from_secs(2)); let url = client .app_action_url("app/../../identity", "deploy") @@ -1429,34 +1739,4 @@ mod tests { assert_eq!(url.path(), "/v1/agent/apps/app%2F..%2F..%2Fidentity/deploy"); } - - #[test] - fn purpose_token_provider_rejects_oversized_exchange_response() { - let temporary_directory = tempfile::tempdir().expect("create temporary directory"); - let server = Server::http("127.0.0.1:0").expect("bind token exchange server"); - let exchange_url = url::Url::parse(&format!( - "http://{}{COMPOSE_TOKEN_EXCHANGE_PATH}", - server.server_addr() - )) - .expect("parse exchange URL"); - let server_thread = thread::spawn(move || { - let request = server.recv().expect("receive token exchange"); - request - .respond(Response::from_data(vec![ - b'x'; - TOKEN_EXCHANGE_RESPONSE_MAX_BYTES - + 1 - ])) - .expect("respond to token exchange"); - }); - let provider = test_provider(exchange_url, &temporary_directory); - - let error = provider - .authorization_header() - .expect_err("reject oversized exchange response"); - - assert!(error.to_string().contains("exceeded 32768 bytes")); - assert!(!error.to_string().contains("test-bbidentity-session")); - server_thread.join().expect("join token exchange server"); - } } diff --git a/bb-cli/src/bb/auth_login.rs b/bb-cli/src/bb/auth_login.rs index a385cf291..1e0b25ade 100644 --- a/bb-cli/src/bb/auth_login.rs +++ b/bb-cli/src/bb/auth_login.rs @@ -7,8 +7,9 @@ use std::time::Duration; use anyhow::{anyhow, Context, Result}; use builderbot_auth::auth_login::{ build_auth_http_client, exchange_login_code_and_verify, login_url, logout_session_credential, - verify_session_credential, AuthMeResponse, + verify_session_credential, AuthMeResponse, VerifiedLoginSession, }; +use builderbot_auth::auth_storage::StoredSessionCredential; use serde::Serialize; use sha2::{Digest, Sha256}; use tiny_http::{Header, Response, Server, StatusCode}; @@ -39,6 +40,15 @@ pub struct BrowserLoginSummary { pub credential_sha256_prefix: Option, } +struct BrowserLoginOutcome { + summary: BrowserLoginSummary, +} + +pub struct VerifiedStoredSession { + pub credential: StoredSessionCredential, + pub me: AuthMeResponse, +} + #[derive(Debug, Clone, Copy, Serialize)] #[serde(rename_all = "snake_case")] pub enum BrowserLoginCredentialSource { @@ -50,6 +60,13 @@ pub fn run_browser_login( config: &SkillsConfig, storage: &dyn SessionCredentialStorage, ) -> Result { + run_browser_login_inner(config, storage).map(|outcome| outcome.summary) +} + +fn run_browser_login_inner( + config: &SkillsConfig, + storage: &dyn SessionCredentialStorage, +) -> Result { let service_url = kgoose_service_url(&config.kgoose_base_url, &config.kgoose_service_path); let client = build_auth_http_client(Duration::from_secs(30))?; let storage_key = session_storage_key_from_config(config); @@ -70,15 +87,17 @@ pub fn run_browser_login( ), ); let workspace_name = me.active_workspace_name()?.to_string(); - return Ok(BrowserLoginSummary { - kgoose_base_url: config.kgoose_base_url.clone(), - kgoose_service_path: config.kgoose_service_path.clone(), - storage: storage.kind().to_string(), - source: BrowserLoginCredentialSource::Stored, - workspace_name, - expires_at: me.expires_at.or(stored.expires_at), - credential_prefix: None, - credential_sha256_prefix: None, + return Ok(BrowserLoginOutcome { + summary: BrowserLoginSummary { + kgoose_base_url: config.kgoose_base_url.clone(), + kgoose_service_path: config.kgoose_service_path.clone(), + storage: storage.kind().to_string(), + source: BrowserLoginCredentialSource::Stored, + workspace_name, + expires_at: me.expires_at.or_else(|| stored.expires_at.clone()), + credential_prefix: None, + credential_sha256_prefix: None, + }, }); } auth_info( @@ -129,10 +148,8 @@ pub fn run_browser_login( .context("loopback auth server stopped before login completed")??; let verified = exchange_login_code_and_verify(&client, config.playpen.as_deref(), &service_url, &code)?; - let stored = verified.credential; - let me = verified.me; - let workspace_name = me.active_workspace_name()?.to_string(); - storage.set(&storage_key, &stored)?; + let (stored, me, workspace_name) = + validate_and_store_login_credential(storage, &storage_key, verified)?; auth_info( config, &format!( @@ -141,30 +158,66 @@ pub fn run_browser_login( ), ); - Ok(BrowserLoginSummary { - kgoose_base_url: config.kgoose_base_url.clone(), - kgoose_service_path: config.kgoose_service_path.clone(), - storage: storage.kind().to_string(), - source: BrowserLoginCredentialSource::BrowserLogin, - workspace_name, - expires_at: me.expires_at.or_else(|| stored.expires_at.clone()), - credential_prefix: Some(safe_prefix(&stored.session_credential)), - credential_sha256_prefix: Some(sha256_prefix(&stored.session_credential)), + Ok(BrowserLoginOutcome { + summary: BrowserLoginSummary { + kgoose_base_url: config.kgoose_base_url.clone(), + kgoose_service_path: config.kgoose_service_path.clone(), + storage: storage.kind().to_string(), + source: BrowserLoginCredentialSource::BrowserLogin, + workspace_name, + expires_at: me.expires_at.or_else(|| stored.expires_at.clone()), + credential_prefix: Some(safe_prefix(&stored.session_credential)), + credential_sha256_prefix: Some(sha256_prefix(&stored.session_credential)), + }, }) } pub fn verify_stored_session( config: &SkillsConfig, storage: &dyn SessionCredentialStorage, -) -> Result> { +) -> Result> { let service_url = kgoose_service_url(&config.kgoose_base_url, &config.kgoose_service_path); let client = build_auth_http_client(Duration::from_secs(30))?; let storage_key = session_storage_key_from_config(config); - let Some(stored) = storage.get(&storage_key)? else { + verify_stored_session_with(storage, &storage_key, |stored| { + verify_session_credential(&client, config.playpen.as_deref(), &service_url, stored) + }) +} + +fn verify_stored_session_with( + storage: &dyn SessionCredentialStorage, + storage_key: &super::auth_storage::SessionStorageKey, + verify: F, +) -> Result> +where + F: FnOnce(&StoredSessionCredential) -> Result>, +{ + let Some(stored) = storage.get(storage_key)? else { return Ok(None); }; + Ok(verify(&stored)?.map(|me| VerifiedStoredSession { + credential: stored, + me, + })) +} - verify_session_credential(&client, config.playpen.as_deref(), &service_url, &stored) +fn store_login_credential( + storage: &dyn SessionCredentialStorage, + storage_key: &super::auth_storage::SessionStorageKey, + credential: StoredSessionCredential, +) -> Result { + storage.set(storage_key, &credential)?; + Ok(credential) +} + +fn validate_and_store_login_credential( + storage: &dyn SessionCredentialStorage, + storage_key: &super::auth_storage::SessionStorageKey, + verified: VerifiedLoginSession, +) -> Result<(StoredSessionCredential, AuthMeResponse, String)> { + let workspace_name = verified.me.active_workspace_name()?.to_string(); + let stored = store_login_credential(storage, storage_key, verified.credential)?; + Ok((stored, verified.me, workspace_name)) } pub fn logout_stored_session( @@ -308,7 +361,142 @@ fn auth_info(config: &SkillsConfig, message: &str) { #[cfg(test)] mod tests { - use super::{auth_callback_page, AuthCallbackPage}; + use std::cell::{Cell, RefCell}; + use std::collections::VecDeque; + + use anyhow::Result; + use builderbot_auth::auth_login::{ + AuthMeResponse, AuthMeWorkspace, AuthMeWorkspaces, VerifiedLoginSession, + }; + use builderbot_auth::auth_storage::{ + SessionCredentialStorage, SessionStorageKey, StoredSessionCredential, + }; + + use super::{ + auth_callback_page, store_login_credential, validate_and_store_login_credential, + verify_stored_session_with, AuthCallbackPage, + }; + + struct SwappingStorage { + reads: RefCell>, + read_count: Cell, + writes: RefCell>, + } + + impl SwappingStorage { + fn new(reads: impl IntoIterator) -> Self { + Self { + reads: RefCell::new(reads.into_iter().collect()), + read_count: Cell::new(0), + writes: RefCell::new(Vec::new()), + } + } + } + + impl SessionCredentialStorage for SwappingStorage { + fn kind(&self) -> &'static str { + "swapping" + } + + fn get(&self, _key: &SessionStorageKey) -> Result> { + self.read_count.set(self.read_count.get() + 1); + Ok(self.reads.borrow_mut().pop_front()) + } + + fn set( + &self, + _key: &SessionStorageKey, + credential: &StoredSessionCredential, + ) -> Result<()> { + self.writes.borrow_mut().push(credential.clone()); + Ok(()) + } + + fn delete(&self, _key: &SessionStorageKey) -> Result { + Ok(false) + } + } + + fn stored(value: &str) -> StoredSessionCredential { + StoredSessionCredential { + session_credential: value.to_string(), + expires_at: None, + } + } + + fn auth_me() -> AuthMeResponse { + AuthMeResponse { + subject: None, + email: None, + name: None, + expires_at: None, + workspaces: AuthMeWorkspaces { + active: vec![AuthMeWorkspace { + name: "Test Workspace".to_string(), + }], + }, + } + } + + #[test] + fn verification_returns_the_exact_credential_that_was_checked() { + let storage = SwappingStorage::new([stored("verified-a"), stored("substituted-b")]); + let key = SessionStorageKey::new("default", "https://kgoose.example"); + + let verified = verify_stored_session_with(&storage, &key, |credential| { + assert_eq!(credential.session_credential, "verified-a"); + Ok(Some(auth_me())) + }) + .expect("verify stored session") + .expect("verified session"); + + assert_eq!(verified.credential.session_credential, "verified-a"); + assert_eq!(storage.read_count.get(), 1); + assert_eq!( + storage + .get(&key) + .expect("read substituted credential") + .expect("substituted credential") + .session_credential, + "substituted-b" + ); + } + + #[test] + fn interactive_login_completion_returns_issued_credential_without_rereading_storage() { + let storage = SwappingStorage::new([stored("substituted-b")]); + let key = SessionStorageKey::new("default", "https://kgoose.example"); + + let returned = store_login_credential(&storage, &key, stored("issued-a")) + .expect("store completed login"); + + assert_eq!(returned.session_credential, "issued-a"); + assert_eq!(storage.read_count.get(), 0); + assert_eq!(storage.writes.borrow()[0].session_credential, "issued-a"); + } + + #[test] + fn browser_login_does_not_store_a_session_without_an_active_workspace() { + let storage = SwappingStorage::new([]); + let key = SessionStorageKey::new("default", "https://kgoose.example"); + let verified = VerifiedLoginSession { + credential: stored("issued-without-workspace"), + me: AuthMeResponse { + subject: None, + email: None, + name: None, + expires_at: None, + workspaces: AuthMeWorkspaces { active: vec![] }, + }, + }; + + let error = validate_and_store_login_credential(&storage, &key, verified) + .expect_err("reject login without an active workspace"); + + assert!(error.to_string().contains("no active workspaces")); + assert!(storage.writes.borrow().is_empty()); + assert!(storage.get(&key).expect("read storage").is_none()); + } #[test] fn callback_pages_are_self_contained_and_themed() { diff --git a/bb-cli/src/bb/auth_storage.rs b/bb-cli/src/bb/auth_storage.rs index 8bb0478d0..890c25550 100644 --- a/bb-cli/src/bb/auth_storage.rs +++ b/bb-cli/src/bb/auth_storage.rs @@ -2,8 +2,8 @@ use anyhow::Result; pub use builderbot_auth::auth_storage::{ default_session_storage_for_bb_home, stored_session_credential_header_value, - stored_session_credential_header_value_for_kgoose_base_url, PurposeTokenStorageKey, - SessionCredentialStorage, SessionStorageKey, StoredPurposeTokenCredential, + stored_session_credential_header_value_for_kgoose_base_url, SessionCredentialStorage, + SessionStorageKey, }; use super::skills_config::SkillsConfig; diff --git a/bb-cli/src/bb/skills.rs b/bb-cli/src/bb/skills.rs index ab83c77c1..888abed8e 100644 --- a/bb-cli/src/bb/skills.rs +++ b/bb-cli/src/bb/skills.rs @@ -13,7 +13,7 @@ use serde_json::{json, Value}; use super::auth_login::{ logout_stored_session, run_browser_login, verify_stored_session, BrowserLoginCredentialSource, }; -use super::auth_storage::{default_session_storage, PurposeTokenStorageKey}; +use super::auth_storage::default_session_storage; use super::description::describe_command_tree; use super::display::{print_json, stdin_is_tty, terminal_safe_text, Style}; use super::org_routing::{normalize_org, resolve_org_kgoose_base_url}; @@ -575,7 +575,7 @@ fn config_with_login_org(config: &SkillsConfig) -> Result { fn auth_status(config: &SkillsConfig) -> Result<()> { let storage = default_session_storage(config)?; - let Some(me) = verify_stored_session(config, storage.as_ref())? else { + let Some(verified) = verify_stored_session(config, storage.as_ref())? else { if !config.json { println!("BuilderBot CLI auth"); println!(" profile: {}", config.profile); @@ -591,6 +591,7 @@ fn auth_status(config: &SkillsConfig) -> Result<()> { "profile": config.profile, })); }; + let me = verified.me; if !config.json { let workspace_name = me.active_workspace_name()?; @@ -655,7 +656,6 @@ fn auth_login_browser(config: &SkillsConfig) -> Result<()> { fn auth_logout_browser(config: &SkillsConfig) -> Result<()> { let storage = default_session_storage(config)?; let storage_key = super::auth_storage::session_storage_key_from_config(config); - let purpose_token_key = PurposeTokenStorageKey::new(&storage_key, "compose"); let mut warnings = Vec::new(); let server_revoked = match logout_stored_session(config, storage.as_ref()) { Ok(server_revoked) => server_revoked, @@ -671,14 +671,15 @@ fn auth_logout_browser(config: &SkillsConfig) -> Result<()> { false } }; - let purpose_token_removed = match storage.delete_purpose_token(&purpose_token_key) { + let purpose_token_removed = match storage.delete_legacy_purpose_token_cache(&storage_key) { Ok(removed) => removed, Err(err) => { - warnings.push(format!("failed to remove cached Compose credential: {err}")); + warnings.push(format!( + "failed to remove legacy cached Compose credential: {err}" + )); false } }; - if config.json { return print_json(&json!({ "profile": config.profile, diff --git a/bb-cli/src/lib.rs b/bb-cli/src/lib.rs index c331fda34..1463f889f 100644 --- a/bb-cli/src/lib.rs +++ b/bb-cli/src/lib.rs @@ -109,6 +109,10 @@ fn run_agent_tools() -> Result<()> { fn run_bb() -> Result<()> { let argv = std::env::args().collect::>(); + run_bb_with_argv(argv) +} + +fn run_bb_with_argv(argv: Vec) -> Result<()> { let raw_args = &argv[1..]; if raw_args.first().map(String::as_str) == Some(TOOLS_COMMAND_NAME) { diff --git a/bb-cli/tests/bb_e2e.rs b/bb-cli/tests/bb_e2e.rs index 47b038ac2..dba7d9a52 100644 --- a/bb-cli/tests/bb_e2e.rs +++ b/bb-cli/tests/bb_e2e.rs @@ -1,13 +1,14 @@ //! End-to-end tests for the `bb` binary (skills marketplace + bb-specific //! surfaces). The sq/agent-tools CLI suite lives in `cli_e2e.rs`; shared mock //! server infrastructure lives in `common/`. - mod common; use std::fs; -use std::io::{Cursor, Write}; +use std::io::{Cursor, Read, Write}; use std::path::{Path, PathBuf}; use std::process::Stdio; +use std::thread; +use std::time::{Duration, Instant}; use serde_json::{json, Value}; use sha2::{Digest, Sha256}; @@ -1859,6 +1860,45 @@ fn bb_auth_status_uses_auth_me_for_stored_file_session() { fs::remove_dir_all(temp).expect("remove temp dir"); } +#[test] +fn bb_auth_status_errors_never_echo_a_reflected_session() { + let secret = "reflected_session_credential_123456"; + let server = MockServer::start(vec![ + MockResponse::text(500, secret), + MockResponse::text(500, secret), + ]); + let temp = temp_test_dir("bb-auth-status-redaction"); + let bb_home = temp.join("bb-home"); + let storage_path = temp.join("auth-sessions.json"); + write_bb_org_config(&bb_home, "test"); + write_browser_auth_session( + &storage_path, + &server.base_url, + secret, + "2099-01-01T00:00:00Z", + ); + + for args in [vec!["auth", "status"], vec!["auth", "status", "--json"]] { + let output = bb_command() + .env("BB_HOME", &bb_home) + .env("BB_AUTH_STORAGE", "file") + .env("BB_AUTH_STORAGE_FILE", &storage_path) + .env("KGOOSE_BASE_URL", &server.base_url) + .args(args) + .output() + .expect("run failing bb auth status"); + let (stdout, stderr) = output_text(&output); + + assert!(!output.status.success()); + assert!(!stdout.contains(secret)); + assert!(!stderr.contains(secret)); + assert!(stderr.contains("/v1/auth/me failed with 500")); + } + + assert_eq!(server.finish().len(), 2); + fs::remove_dir_all(temp).expect("remove temp dir"); +} + #[test] fn bb_auth_login_uses_valid_stored_file_session() { let temp = temp_test_dir("bb-auth-login-stored"); @@ -1981,6 +2021,7 @@ fn bb_auth_logout_removes_stored_file_session() { let server_url = format!("{}/api/goose", server.base_url); let default_key = browser_auth_storage_key("default", &server_url); let other_key = browser_auth_storage_key("other", &server_url); + let purpose_storage_path = PathBuf::from(format!("{}.purpose-tokens", storage_path.display())); fs::write( &storage_path, serde_json::to_string_pretty(&json!({ @@ -1996,6 +2037,14 @@ fn bb_auth_logout_removes_stored_file_session() { .expect("serialize storage"), ) .expect("write auth storage"); + fs::write( + &purpose_storage_path, + serde_json::to_string_pretty(&json!({ + "obsolete-purpose-token": { "accessToken": "legacy-secret" } + })) + .expect("serialize legacy purpose token storage"), + ) + .expect("write legacy purpose token storage"); let output = bb_command() .env("BB_HOME", &bb_home) @@ -2013,6 +2062,7 @@ fn bb_auth_logout_removes_stored_file_session() { assert_eq!(response["removed"], json!(true)); assert_eq!(response["server_revoked"], json!(true)); assert_eq!(response["storage"], json!("file")); + assert_eq!(response["purpose_token_removed"], json!(true)); assert_eq!(requests.len(), 1); assert_eq!(requests[0].method, "POST"); assert_eq!(requests[0].path, "/api/goose/v1/auth/logout"); @@ -2027,6 +2077,7 @@ fn bb_auth_logout_removes_stored_file_session() { let storage = fs::read_to_string(&storage_path).expect("read storage"); assert!(!storage.contains("default-session")); assert!(storage.contains("other-session")); + assert!(!purpose_storage_path.exists()); let output = bb_command() .env("BB_HOME", &bb_home) @@ -2042,6 +2093,7 @@ fn bb_auth_logout_removes_stored_file_session() { let response = serde_json::from_str::(&stdout).expect("parse logout output"); assert_eq!(response["removed"], json!(false)); assert_eq!(response["server_revoked"], json!(false)); + assert_eq!(response["purpose_token_removed"], json!(false)); fs::remove_dir_all(temp).expect("remove temp dir"); } @@ -3563,6 +3615,27 @@ fn bb_skills_doctor_offline_reports_server_failure() { // --------------------------------------------------------------------------- // External Apps Platform control plane +const APPROVED_APPS_BASE_URL: &str = "https://compose-ctrl.test.blockstaging.build"; + +#[test] +fn bb_shipped_artifact_contains_no_apps_test_transport() { + let binary = fs::read(env!("CARGO_BIN_EXE_bb")).expect("read shipped bb test artifact"); + for forbidden in [ + "BB_APPS_E2E_CONTROL_PLANE_URL", + "BB_APPS_E2E_AUTH_URL", + "BB_APPS_E2E_CREDENTIAL", + "BB_APPS_E2E_RESOLVE_ADDR", + "Berd Apps E2E Test CA", + ] { + assert!( + !binary + .windows(forbidden.len()) + .any(|window| window == forbidden.as_bytes()), + "shipped bb artifact contained Apps test-only material {forbidden:?}" + ); + } +} + #[test] fn bb_apps_help_distinguishes_external_and_internal_paths() { let output = bb_command() @@ -3588,34 +3661,17 @@ fn bb_apps_help_distinguishes_external_and_internal_paths() { } #[test] -fn bb_apps_contract_exchanges_session_and_calls_control_plane() { - let purpose_token = "compose-purpose-token"; - let session_credential = "stored-bbidentity-session"; - let contract = json!({ - "ok": true, - "contract_version": "2026-06-30", - "minimum_client_version": "0.1.0", - "supported_operations": [{ - "method": "GET", - "path": "/v1/agent/contract" - }] - }); - let server = MockServer::start(vec![ - MockResponse::json(json!({ - "access_token": purpose_token, - "token_type": "Bearer", - "expires_at": "2099-01-01T00:05:00Z", - "expires_in_seconds": 300 - })), - MockResponse::json(contract.clone()), - ]); - let temp = temp_test_dir("bb-apps-contract"); +fn bb_apps_contract_rejects_loopback_before_reading_or_sending_the_session() { + let kgoose = MockServer::start(vec![]); + let control_plane = MockServer::start(vec![]); + let temp = temp_test_dir("bb-apps-loopback-origin"); let bb_home = temp.join("bb-home"); let storage_path = temp.join("auth-sessions.json"); + let session_credential = "stored_session_credential_1234567890"; write_bb_org_config(&bb_home, "test"); write_browser_auth_session( &storage_path, - &server.base_url, + &kgoose.base_url, session_credential, "2099-01-01T00:00:00Z", ); @@ -3624,593 +3680,149 @@ fn bb_apps_contract_exchanges_session_and_calls_control_plane() { .env("BB_HOME", &bb_home) .env("BB_AUTH_STORAGE", "file") .env("BB_AUTH_STORAGE_FILE", &storage_path) - .env("KGOOSE_BASE_URL", &server.base_url) - .args([ - "apps", - "contract", - "--base-url", - &server.base_url, - "--client-version", - "0.2.0", - ]) + .env("KGOOSE_BASE_URL", &kgoose.base_url) + .args(["apps", "contract", "--base-url", &control_plane.base_url]) .output() - .expect("run bb apps contract"); - let requests = server.finish(); + .expect("run bb apps contract with loopback origin"); + let kgoose_requests = kgoose.finish(); + let control_plane_requests = control_plane.finish(); let (stdout, stderr) = output_text(&output); - assert!(output.status.success(), "stderr was: {stderr}"); - assert_eq!( - serde_json::from_str::(&stdout).expect("parse contract output"), - contract - ); - assert!(!stdout.contains(session_credential)); - assert!(!stdout.contains(purpose_token)); + assert!(!output.status.success()); + assert!(stdout.is_empty(), "stdout was: {stdout}"); + assert!(stderr.contains("approved Builderlab ingress")); assert!(!stderr.contains(session_credential)); - assert!(!stderr.contains(purpose_token)); - - assert_eq!(requests.len(), 2); - assert_eq!(requests[0].method, "POST"); - assert_eq!(requests[0].path, "/api/goose/v1/auth/token/compose"); - assert_eq!( - requests[0] - .headers - .get("x-bb-session-credential") - .map(String::as_str), - Some(session_credential) - ); - assert!(!requests[0].headers.contains_key("authorization")); - assert_eq!(requests[0].body, Value::Null); - - assert_eq!(requests[1].method, "GET"); - assert_eq!(requests[1].path, "/v1/agent/contract"); - assert_eq!( - requests[1].headers.get("authorization").map(String::as_str), - Some("Bearer compose-purpose-token") - ); - assert_eq!( - requests[1] - .headers - .get("x-hotpod-agent-client-version") - .map(String::as_str), - Some("0.2.0") - ); - for sensitive_header in [ - "x-bb-session-credential", - "x-forwarded-user", - "x-forwarded-workspace-id", - ] { - assert!( - !requests[1].headers.contains_key(sensitive_header), - "control-plane request unexpectedly included {sensitive_header}" - ); - } - assert_eq!(requests[1].body, Value::Null); + assert!(kgoose_requests.is_empty()); + assert!(control_plane_requests.is_empty()); fs::remove_dir_all(temp).expect("remove temp dir"); } #[test] -fn bb_apps_create_plans_and_initializes_when_recommended() { - let purpose_token = "compose-create-purpose-token"; - let session_credential = "stored-bbidentity-session"; - let plan = json!({ - "ok": true, - "app_id": "merchant-lookup", - "display_name": "Merchant Lookup", - "environment": "staging", - "external_url": "https://merchant-lookup--bpsites.example/", - "persistence": "sqlite", - "runtime_class": "default", - "initialize": { - "required": true, - "recommended": true, - "reason": "no active route exists" - } - }); - let initialized = json!({ - "ok": true, - "app_id": "merchant-lookup-2", - "renamed": true, - "external_url": "https://merchant-lookup-2--bpsites.example/" - }); - let server = MockServer::start(vec![ - MockResponse::json(json!({ - "access_token": purpose_token, - "token_type": "Bearer", - "expires_in_seconds": 300 - })), - MockResponse::json(plan.clone()), - MockResponse::json(initialized.clone()), - ]); - let temp = temp_test_dir("bb-apps-create"); +fn bb_apps_contract_rejects_arbitrary_https_origin() { + let temp = temp_test_dir("bb-apps-arbitrary-origin"); let bb_home = temp.join("bb-home"); - let storage_path = temp.join("auth-sessions.json"); write_bb_org_config(&bb_home, "test"); - write_browser_auth_session( - &storage_path, - &server.base_url, - session_credential, - "2099-01-01T00:00:00Z", - ); let output = bb_command() .env("BB_HOME", &bb_home) - .env("BB_AUTH_STORAGE", "file") - .env("BB_AUTH_STORAGE_FILE", &storage_path) - .env("KGOOSE_BASE_URL", &server.base_url) - .args([ - "apps", - "create", - "--app-id", - "merchant-lookup", - "--name", - "Merchant Lookup", - "--environment", - "staging", - "--runtime-profile", - "fetch-js", - "--persistence", - "sqlite", - "--base-url", - &server.base_url, - "--client-version", - "0.2.0", - ]) + .args(["apps", "contract", "--base-url", "https://attacker.example"]) .output() - .expect("run bb apps create"); - let requests = server.finish(); + .expect("run bb apps contract with arbitrary origin"); let (stdout, stderr) = output_text(&output); - assert!(output.status.success(), "stderr was: {stderr}"); - let response = serde_json::from_str::(&stdout).expect("parse create output"); - assert_eq!(response["app_id"], json!("merchant-lookup-2")); - assert_eq!( - response["external_url"], - json!("https://merchant-lookup-2--bpsites.example/") - ); - assert_eq!(response["initialized"], json!(true)); - assert_eq!(response["plan"], plan); - assert_eq!(response["initialize"], initialized); - assert!(!stdout.contains(session_credential)); - assert!(!stdout.contains(purpose_token)); - assert!(!stderr.contains(session_credential)); - assert!(!stderr.contains(purpose_token)); - - assert_eq!(requests.len(), 3); - assert_eq!(requests[1].method, "POST"); - assert_eq!(requests[1].path, "/v1/agent/apps/plan"); - assert_eq!( - requests[1].body, - json!({ - "app_id": "merchant-lookup", - "name": "Merchant Lookup", - "environment": "staging", - "runtime_profile": "fetch-js", - "persistence": "sqlite", - "client_version": "0.2.0" - }) - ); - assert_eq!(requests[2].method, "POST"); - assert_eq!( - requests[2].path, - "/v1/agent/apps/merchant-lookup/initialize" - ); - assert_eq!( - requests[2].body, - json!({ - "name": "Merchant Lookup", - "environment": "staging", - "persistence": "sqlite", - "runtime_class": "default" - }) - ); - for request in &requests[1..] { - assert_eq!( - request.headers.get("authorization").map(String::as_str), - Some("Bearer compose-create-purpose-token") - ); - assert_eq!( - request - .headers - .get("x-hotpod-agent-client-version") - .map(String::as_str), - Some("0.2.0") - ); - for sensitive_header in [ - "x-bb-session-credential", - "x-forwarded-user", - "x-forwarded-workspace-id", - ] { - assert!(!request.headers.contains_key(sensitive_header)); - } - } + assert!(!output.status.success()); + assert!(stdout.is_empty(), "stdout was: {stdout}"); + assert!(stderr.contains("approved Builderlab ingress")); fs::remove_dir_all(temp).expect("remove temp dir"); } #[test] -fn bb_apps_create_skips_initialize_when_plan_does_not_recommend_it() { - let server = MockServer::start(vec![ - MockResponse::json(json!({ - "access_token": "compose-create-purpose-token", - "token_type": "Bearer", - "expires_in_seconds": 300 - })), - MockResponse::json(json!({ - "ok": true, - "app_id": "existing-app", - "initialize": {"required": false, "recommended": false} - })), - ]); - let temp = temp_test_dir("bb-apps-create-existing"); +fn bb_apps_json_without_a_session_exits_promptly_with_auth_required() { + let temp = temp_test_dir("bb-apps-json-auth-required"); let bb_home = temp.join("bb-home"); - let storage_path = temp.join("auth-sessions.json"); + let storage_path = temp.join("missing-auth-sessions.json"); write_bb_org_config(&bb_home, "test"); - write_browser_auth_session( - &storage_path, - &server.base_url, - "stored-bbidentity-session", - "2099-01-01T00:00:00Z", - ); - let output = bb_command() + let mut child = bb_command() .env("BB_HOME", &bb_home) .env("BB_AUTH_STORAGE", "file") .env("BB_AUTH_STORAGE_FILE", &storage_path) - .env("KGOOSE_BASE_URL", &server.base_url) .args([ "apps", - "create", - "--app-id", - "existing-app", + "contract", "--base-url", - &server.base_url, + "https://compose-ctrl.test.blockstaging.build", + "--json", ]) - .output() - .expect("run bb apps create for existing app"); - let requests = server.finish(); - let (stdout, stderr) = output_text(&output); - - assert!(output.status.success(), "stderr was: {stderr}"); - let response = serde_json::from_str::(&stdout).expect("parse create output"); - assert_eq!(response["initialized"], json!(false)); - assert_eq!(response["initialize"], Value::Null); - assert_eq!(requests.len(), 2); - assert_eq!(requests[1].path, "/v1/agent/apps/plan"); - fs::remove_dir_all(temp).expect("remove temp dir"); -} - -#[test] -fn bb_apps_deploy_uploads_multipart_artifact_without_identity_headers() { - let purpose_token = "compose-deploy-purpose-token"; - let session_credential = "stored-bbidentity-session"; - let deploy_response = json!({ - "ok": true, - "app_id": "merchant-lookup", - "version_id": "ver-123", - "deployment_id": "dpl-123", - "external_url": "https://merchant-lookup--bpsites.example/", - "readiness": { - "control_plane_url": "/v1/agent/apps/merchant-lookup/ready?version_id=ver-123", - "diagnostics_url": "/v1/agent/apps/merchant-lookup/debug?version_id=ver-123" + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("start noninteractive bb apps contract"); + let deadline = Instant::now() + Duration::from_secs(10); + let status = loop { + if let Some(status) = child.try_wait().expect("poll bb apps contract") { + break status; } - }); - let server = MockServer::start(vec![ - MockResponse::json(json!({ - "access_token": purpose_token, - "token_type": "Bearer", - "expires_in_seconds": 300 - })), - MockResponse::json(deploy_response.clone()), - ]); - let temp = temp_test_dir("bb-apps-deploy"); - let bb_home = temp.join("bb-home"); - let storage_path = temp.join("auth-sessions.json"); - let artifact_path = temp.join("prepared-app.tar.gz"); - let artifact_marker = "test-hotpod-artifact-marker"; - fs::write(&artifact_path, artifact_marker).expect("write deploy artifact"); - write_bb_org_config(&bb_home, "test"); - write_browser_auth_session( - &storage_path, - &server.base_url, - session_credential, - "2099-01-01T00:00:00Z", - ); - - let output = bb_command() - .env("BB_HOME", &bb_home) - .env("BB_AUTH_STORAGE", "file") - .env("BB_AUTH_STORAGE_FILE", &storage_path) - .env("KGOOSE_BASE_URL", &server.base_url) - .args([ - "apps", - "deploy", - "merchant-lookup", - artifact_path.to_str().expect("artifact path text"), - "--environment", - "production", - "--version-id", - "ver-123", - "--deployment-id", - "dpl-123", - "--base-url", - &server.base_url, - "--client-version", - "0.2.0", - ]) - .output() - .expect("run bb apps deploy"); - let requests = server.finish(); - let (stdout, stderr) = output_text(&output); - - assert!(output.status.success(), "stderr was: {stderr}"); - assert_eq!( - serde_json::from_str::(&stdout).expect("parse deploy output"), - deploy_response - ); - assert!(!stdout.contains(session_credential)); - assert!(!stdout.contains(purpose_token)); - assert!(!stderr.contains(session_credential)); - assert!(!stderr.contains(purpose_token)); - - assert_eq!(requests.len(), 2); - let request = &requests[1]; - assert_eq!(request.method, "POST"); - assert_eq!(request.path, "/v1/agent/apps/merchant-lookup/deploy"); - assert_eq!( - request.headers.get("authorization").map(String::as_str), - Some("Bearer compose-deploy-purpose-token") - ); - assert_eq!( - request - .headers - .get("x-hotpod-agent-client-version") - .map(String::as_str), - Some("0.2.0") - ); - assert!(request - .headers - .get("content-type") - .is_some_and(|value| value.starts_with("multipart/form-data; boundary="))); - for sensitive_header in [ - "x-bb-session-credential", - "x-forwarded-user", - "x-forwarded-workspace-id", - ] { - assert!(!request.headers.contains_key(sensitive_header)); - } - let body = String::from_utf8_lossy(&request.body_bytes); - for expected in [ - "name=\"artifact\"; filename=\"artifact.tar.gz\"", - "Content-Type: application/gzip", - artifact_marker, - "name=\"environment\"\r\n\r\nproduction", - "name=\"version_id\"\r\n\r\nver-123", - "name=\"deployment_id\"\r\n\r\ndpl-123", - ] { - assert!( - body.contains(expected), - "multipart body omitted {expected:?}" - ); - } - assert!(!body.contains("publisher")); - fs::remove_dir_all(temp).expect("remove temp dir"); -} - -#[test] -fn bb_apps_contract_rejects_untrusted_origin_before_token_exchange() { - let kgoose = MockServer::start(vec![]); - let temp = temp_test_dir("bb-apps-untrusted-origin"); - let bb_home = temp.join("bb-home"); - let storage_path = temp.join("auth-sessions.json"); - write_bb_org_config(&bb_home, "test"); - write_browser_auth_session( - &storage_path, - &kgoose.base_url, - "stored-bbidentity-session", - "2099-01-01T00:00:00Z", - ); - - let output = bb_command() - .env("BB_HOME", &bb_home) - .env("BB_AUTH_STORAGE", "file") - .env("BB_AUTH_STORAGE_FILE", &storage_path) - .env("KGOOSE_BASE_URL", &kgoose.base_url) - .args(["apps", "contract", "--base-url", "https://attacker.example"]) - .output() - .expect("run bb apps contract with untrusted origin"); - let requests = kgoose.finish(); - let (stdout, stderr) = output_text(&output); - - assert!(!output.status.success()); - assert!(stdout.is_empty(), "stdout was: {stdout}"); - assert!(stderr.contains("approved Builderlab ingress")); - assert!(requests.is_empty(), "token exchange must not be attempted"); - fs::remove_dir_all(temp).expect("remove temp dir"); -} - -#[test] -fn bb_apps_contract_shares_purpose_token_between_concurrent_processes() { - let purpose_token = "shared-compose-purpose-token"; - let session_credential = "stored-bbidentity-session"; - let contract = json!({ - "contract_version": "2026-06-30", - "supported_operations": [] - }); - let server = MockServer::start(vec![ - MockResponse::json(json!({ - "access_token": purpose_token, - "token_type": "Bearer", - "expires_at": "2099-01-01T00:05:00Z", - "expires_in_seconds": 300 - })), - MockResponse::json(contract.clone()), - MockResponse::json(contract.clone()), - ]); - let temp = temp_test_dir("bb-apps-shared-purpose-token"); - let bb_home = temp.join("bb-home"); - let storage_path = temp.join("auth-sessions.json"); - write_bb_org_config(&bb_home, "test"); - write_browser_auth_session( - &storage_path, - &server.base_url, - session_credential, - "2099-01-01T00:00:00Z", - ); - - let mut children = Vec::new(); - for _ in 0..2 { - children.push( - bb_command() - .env("BB_HOME", &bb_home) - .env("BB_AUTH_STORAGE", "file") - .env("BB_AUTH_STORAGE_FILE", &storage_path) - .env("KGOOSE_BASE_URL", &server.base_url) - .args(["apps", "contract", "--base-url", &server.base_url]) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .expect("spawn bb apps contract"), - ); - } - - for (index, child) in children.into_iter().enumerate() { - let output = child.wait_with_output().expect("wait for bb apps contract"); - let (stdout, stderr) = output_text(&output); - - assert!( - output.status.success(), - "invocation {} failed: {stderr}", - index + 1 - ); - assert_eq!( - serde_json::from_str::(&stdout).expect("parse contract output"), - contract - ); - assert!(!stdout.contains(purpose_token)); - assert!(!stderr.contains(purpose_token)); - } - - let requests = server.finish(); - assert_eq!(requests.len(), 3); - assert_eq!( - requests - .iter() - .filter(|request| request.path == "/api/goose/v1/auth/token/compose") - .count(), - 1 - ); - let control_plane_requests = requests - .iter() - .filter(|request| request.path == "/v1/agent/contract") - .collect::>(); - assert_eq!(control_plane_requests.len(), 2); - for request in control_plane_requests { - assert_eq!( - request.headers.get("authorization").map(String::as_str), - Some("Bearer shared-compose-purpose-token") - ); - } - - fs::remove_dir_all(temp).expect("remove temp dir"); -} - -#[test] -fn bb_apps_exchange_failure_does_not_echo_response_or_credentials() { - let secret = "credential-that-must-not-be-logged"; - let server = MockServer::start(vec![MockResponse::text(403, secret)]); - let temp = temp_test_dir("bb-apps-exchange-failure"); - let bb_home = temp.join("bb-home"); - let storage_path = temp.join("auth-sessions.json"); - write_bb_org_config(&bb_home, "test"); - write_browser_auth_session( - &storage_path, - &server.base_url, - secret, - "2099-01-01T00:00:00Z", - ); - - let output = bb_command() - .env("BB_HOME", &bb_home) - .env("BB_AUTH_STORAGE", "file") - .env("BB_AUTH_STORAGE_FILE", &storage_path) - .env("KGOOSE_BASE_URL", &server.base_url) - .args(["apps", "contract", "--base-url", &server.base_url, "--json"]) - .output() - .expect("run failing bb apps contract"); - let requests = server.finish(); - let (stdout, stderr) = output_text(&output); - - assert!(!output.status.success()); + if Instant::now() >= deadline { + child.kill().expect("stop hung bb apps contract"); + child.wait().expect("reap hung bb apps contract"); + panic!("bb apps contract did not fail promptly without a session"); + } + thread::sleep(Duration::from_millis(10)); + }; + let mut stdout = String::new(); + let mut stderr = String::new(); + child + .stdout + .take() + .expect("capture stdout") + .read_to_string(&mut stdout) + .expect("read stdout"); + child + .stderr + .take() + .expect("capture stderr") + .read_to_string(&mut stderr) + .expect("read stderr"); + + assert_eq!(status.code(), Some(3), "stderr was: {stderr}"); assert!(stdout.is_empty(), "stdout was: {stdout}"); - assert!( - !stderr.contains(secret), - "stderr leaked credential: {stderr}" - ); - let error = parse_stderr_error(&stderr); - assert_eq!(error["error"]["code"], json!("forbidden")); - assert_eq!(requests.len(), 1, "request should stop after exchange"); + let payload = parse_stderr_error(&stderr); + assert_eq!(payload["error"]["code"], json!("auth_required")); + assert_eq!(payload["error"]["exit_code"], json!(3)); + assert!(!storage_path.exists()); fs::remove_dir_all(temp).expect("remove temp dir"); } #[test] -fn bb_apps_control_plane_failure_does_not_echo_response_details() { - let secret = "credential-like-control-plane-detail"; - let server = MockServer::start(vec![ - MockResponse::json(json!({ - "access_token": "compose-purpose-token", - "token_type": "Bearer", - "expires_in_seconds": 300 - })), - MockResponse::text( - 400, - &json!({ - "error": {"code": "invalid_app", "detail": secret}, - "next_action": "Choose a DNS-safe app id." - }) - .to_string(), - ), - ]); - let temp = temp_test_dir("bb-apps-control-plane-failure"); +fn bb_apps_pipeline_without_a_session_never_starts_browser_login() { + let temp = temp_test_dir("bb-apps-pipeline-auth-required"); let bb_home = temp.join("bb-home"); - let storage_path = temp.join("auth-sessions.json"); + let storage_path = temp.join("missing-auth-sessions.json"); write_bb_org_config(&bb_home, "test"); - write_browser_auth_session( - &storage_path, - &server.base_url, - "stored-bbidentity-session", - "2099-01-01T00:00:00Z", - ); - let output = bb_command() + let mut child = bb_command() .env("BB_HOME", &bb_home) .env("BB_AUTH_STORAGE", "file") .env("BB_AUTH_STORAGE_FILE", &storage_path) - .env("KGOOSE_BASE_URL", &server.base_url) - .args([ - "apps", - "create", - "--app-id", - "bad/app", - "--base-url", - &server.base_url, - "--json", - ]) - .output() - .expect("run failing bb apps create"); - let requests = server.finish(); - let (stdout, stderr) = output_text(&output); - - assert!(!output.status.success()); + .args(["apps", "contract", "--base-url", APPROVED_APPS_BASE_URL]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("start piped bb apps contract"); + let deadline = Instant::now() + Duration::from_secs(10); + let status = loop { + if let Some(status) = child.try_wait().expect("poll bb apps contract") { + break status; + } + if Instant::now() >= deadline { + child.kill().expect("stop hung bb apps contract"); + child.wait().expect("reap hung bb apps contract"); + panic!("piped bb apps contract did not fail promptly without a session"); + } + thread::sleep(Duration::from_millis(10)); + }; + let mut stdout = String::new(); + let mut stderr = String::new(); + child + .stdout + .take() + .expect("capture stdout") + .read_to_string(&mut stdout) + .expect("read stdout"); + child + .stderr + .take() + .expect("capture stderr") + .read_to_string(&mut stderr) + .expect("read stderr"); + + assert_eq!(status.code(), Some(3), "stderr was: {stderr}"); assert!(stdout.is_empty(), "stdout was: {stdout}"); - assert!( - !stderr.contains(secret), - "stderr leaked response detail: {stderr}" - ); - let error = parse_stderr_error(&stderr); - assert_eq!(error["error"]["code"], json!("invalid_app")); - assert!(error["error"]["message"] - .as_str() - .is_some_and(|message| message.contains("Choose a DNS-safe app id."))); - assert_eq!(requests.len(), 2); + assert!(stderr.contains("BuilderBot CLI auth is required")); + assert!(!stderr.contains("Opening BuilderBot auth login")); + assert!(!stderr.contains("127.0.0.1")); + assert!(!storage_path.exists()); fs::remove_dir_all(temp).expect("remove temp dir"); } diff --git a/crates/builderbot-auth/src/auth_login.rs b/crates/builderbot-auth/src/auth_login.rs index 81c9ae657..dcbd7029c 100644 --- a/crates/builderbot-auth/src/auth_login.rs +++ b/crates/builderbot-auth/src/auth_login.rs @@ -84,12 +84,10 @@ pub fn exchange_login_code( } let response = request.send().context("exchange login code")?; let status = response.status(); - let body = response.text().context("read login exchange response")?; if !status.is_success() { - return Err(anyhow!( - "/v1/auth/login/exchange failed with {status}: {body}" - )); + return Err(anyhow!("/v1/auth/login/exchange failed with {status}")); } + let body = response.text().context("read login exchange response")?; serde_json::from_str(&body).context("parse login exchange response") } @@ -133,13 +131,13 @@ pub fn verify_session_credential( .send() .context("verify stored BuilderBot CLI auth session")?; let status = response.status(); - let body = response.text().context("read /v1/auth/me response")?; if status == HttpStatusCode::UNAUTHORIZED || status == HttpStatusCode::FORBIDDEN { return Ok(None); } if !status.is_success() { - return Err(anyhow!("/v1/auth/me failed with {status}: {body}")); + return Err(anyhow!("/v1/auth/me failed with {status}")); } + let body = response.text().context("read /v1/auth/me response")?; let me: AuthMeResponse = serde_json::from_str(&body).context("parse /v1/auth/me response")?; Ok(Some(me)) } @@ -166,12 +164,11 @@ pub fn logout_session_credential( .send() .context("destroy stored BuilderBot CLI auth session")?; let status = response.status(); - let body = response.text().context("read /v1/auth/logout response")?; if status == HttpStatusCode::UNAUTHORIZED || status == HttpStatusCode::FORBIDDEN { return Ok(false); } if !status.is_success() { - return Err(anyhow!("/v1/auth/logout failed with {status}: {body}")); + return Err(anyhow!("/v1/auth/logout failed with {status}")); } Ok(true) } @@ -232,6 +229,26 @@ mod tests { ); } + #[test] + fn verify_session_credential_does_not_echo_failure_body() { + let secret = "reflected_session_credential_123456"; + let server = SingleResponseServer::start(500, secret); + let client = build_auth_http_client(Duration::from_secs(5)).expect("client"); + let credential = StoredSessionCredential { + session_credential: secret.to_string(), + expires_at: None, + }; + + let error = verify_session_credential(&client, None, &server.base_url, &credential) + .expect_err("reject failed session check"); + let request = server.finish(); + let message = format!("{error:#}"); + + assert!(message.contains("/v1/auth/me failed with 500")); + assert!(!message.contains(secret)); + assert_eq!(request.path, "/v1/auth/me"); + } + #[test] fn verify_session_credential_skips_empty_stored_credential() { let listener = TcpListener::bind("127.0.0.1:0").expect("bind unused server"); diff --git a/crates/builderbot-auth/src/auth_storage.rs b/crates/builderbot-auth/src/auth_storage.rs index e9eac78c4..69ed0c76a 100644 --- a/crates/builderbot-auth/src/auth_storage.rs +++ b/crates/builderbot-auth/src/auth_storage.rs @@ -1,10 +1,8 @@ use std::collections::BTreeMap; #[cfg(any(debug_assertions, test))] use std::collections::HashMap; -use std::fs::{self, OpenOptions}; -use std::io::Write; +use std::fs; use std::path::PathBuf; -use std::sync::atomic::{AtomicU64, Ordering}; #[cfg(any(debug_assertions, test))] use std::sync::Mutex; @@ -17,10 +15,9 @@ use crate::config::kgoose_service_url; #[cfg(target_os = "macos")] const KEYRING_SERVICE: &str = "com.squareup.builderbot.cli-auth"; #[cfg(target_os = "macos")] -const PURPOSE_TOKEN_KEYRING_SERVICE: &str = "com.squareup.builderbot.cli-auth-purpose-token"; +const LEGACY_PURPOSE_TOKEN_KEYRING_SERVICE: &str = "com.squareup.builderbot.cli-auth-purpose-token"; pub const BB_AUTH_STORAGE_ENV_VAR: &str = "BB_AUTH_STORAGE"; pub const BB_AUTH_STORAGE_FILE_ENV_VAR: &str = "BB_AUTH_STORAGE_FILE"; -static TEMP_FILE_SEQUENCE: AtomicU64 = AtomicU64::new(0); #[derive(Debug, Clone)] pub struct SessionStorageKey { @@ -65,38 +62,9 @@ impl SessionStorageKey { } } -#[derive(Debug, Clone)] -pub struct PurposeTokenStorageKey { - session: SessionStorageKey, - purpose: String, -} - -impl PurposeTokenStorageKey { - pub fn new(session: &SessionStorageKey, purpose: impl Into) -> Self { - Self { - session: session.clone(), - purpose: purpose.into(), - } - } - - #[cfg(target_os = "macos")] - fn account(&self) -> String { - format!("{}@{}", self.purpose, self.session.account()) - } - - fn hashed_id(&self) -> String { - let mut hasher = Sha256::new(); - hasher.update(b"purpose-token"); - hasher.update([0]); - hasher.update(self.purpose.as_bytes()); - hasher.update([0]); - hasher.update(self.session.hashed_id().as_bytes()); - hasher - .finalize() - .iter() - .map(|byte| format!("{byte:02x}")) - .collect() - } +#[cfg(target_os = "macos")] +fn legacy_compose_token_account(session: &SessionStorageKey) -> String { + format!("compose@{}", session.account()) } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -118,31 +86,14 @@ impl StoredSessionCredential { } } -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct StoredPurposeTokenCredential { - pub access_token: String, - pub token_type: String, - pub issued_at_unix_seconds: u64, - pub expires_at_unix_seconds: u64, - pub session_credential_sha256: String, -} - pub trait SessionCredentialStorage { fn kind(&self) -> &'static str; fn get(&self, key: &SessionStorageKey) -> Result>; fn set(&self, key: &SessionStorageKey, credential: &StoredSessionCredential) -> Result<()>; fn delete(&self, key: &SessionStorageKey) -> Result; - fn get_purpose_token( - &self, - key: &PurposeTokenStorageKey, - ) -> Result>; - fn set_purpose_token( - &self, - key: &PurposeTokenStorageKey, - credential: &StoredPurposeTokenCredential, - ) -> Result<()>; - fn delete_purpose_token(&self, key: &PurposeTokenStorageKey) -> Result; + fn delete_legacy_purpose_token_cache(&self, _key: &SessionStorageKey) -> Result { + Ok(false) + } } pub fn default_session_storage_for_bb_home( @@ -249,7 +200,6 @@ fn file_storage_from_env(bb_home: &std::path::Path) -> Result>, - purpose_tokens: Mutex>, } #[cfg(any(debug_assertions, test))] @@ -283,39 +233,6 @@ impl SessionCredentialStorage for InMemorySessionCredentialStorage { .remove(&key.hashed_id()) .is_some()) } - - fn get_purpose_token( - &self, - key: &PurposeTokenStorageKey, - ) -> Result> { - Ok(self - .purpose_tokens - .lock() - .expect("purpose token storage mutex poisoned") - .get(&key.hashed_id()) - .cloned()) - } - - fn set_purpose_token( - &self, - key: &PurposeTokenStorageKey, - credential: &StoredPurposeTokenCredential, - ) -> Result<()> { - self.purpose_tokens - .lock() - .expect("purpose token storage mutex poisoned") - .insert(key.hashed_id(), credential.clone()); - Ok(()) - } - - fn delete_purpose_token(&self, key: &PurposeTokenStorageKey) -> Result { - Ok(self - .purpose_tokens - .lock() - .expect("purpose token storage mutex poisoned") - .remove(&key.hashed_id()) - .is_some()) - } } #[derive(Debug)] @@ -346,29 +263,11 @@ impl FileSessionCredentialStorage { restrict_permissions(&self.path) } - fn purpose_tokens_path(&self) -> PathBuf { + fn legacy_purpose_tokens_path(&self) -> PathBuf { let mut path = self.path.as_os_str().to_os_string(); path.push(".purpose-tokens"); PathBuf::from(path) } - - fn read_purpose_tokens(&self) -> Result> { - let path = self.purpose_tokens_path(); - if !path.exists() { - return Ok(BTreeMap::new()); - } - let bytes = fs::read(&path).with_context(|| format!("read {}", path.display()))?; - serde_json::from_slice(&bytes).with_context(|| format!("parse {}", path.display())) - } - - fn write_purpose_tokens( - &self, - entries: &BTreeMap, - ) -> Result<()> { - let path = self.purpose_tokens_path(); - let json = serde_json::to_vec_pretty(entries).context("serialize purpose token storage")?; - write_private_file_atomically(&path, &json) - } } impl SessionCredentialStorage for FileSessionCredentialStorage { @@ -395,30 +294,15 @@ impl SessionCredentialStorage for FileSessionCredentialStorage { Ok(removed) } - fn get_purpose_token( - &self, - key: &PurposeTokenStorageKey, - ) -> Result> { - Ok(self.read_purpose_tokens()?.get(&key.hashed_id()).cloned()) - } - - fn set_purpose_token( - &self, - key: &PurposeTokenStorageKey, - credential: &StoredPurposeTokenCredential, - ) -> Result<()> { - let mut entries = self.read_purpose_tokens()?; - entries.insert(key.hashed_id(), credential.clone()); - self.write_purpose_tokens(&entries) - } - - fn delete_purpose_token(&self, key: &PurposeTokenStorageKey) -> Result { - let mut entries = self.read_purpose_tokens()?; - let removed = entries.remove(&key.hashed_id()).is_some(); - if removed { - self.write_purpose_tokens(&entries)?; + fn delete_legacy_purpose_token_cache(&self, _key: &SessionStorageKey) -> Result { + let path = self.legacy_purpose_tokens_path(); + if !path.exists() { + return Ok(false); } - Ok(removed) + // Purpose-token storage has no remaining readers or writers. Remove + // the obsolete file as a whole instead of rewriting secrets in place. + fs::remove_file(&path).with_context(|| format!("remove {}", path.display()))?; + Ok(true) } } @@ -442,23 +326,8 @@ impl SessionCredentialStorage for KeyringSessionCredentialStorage { keyring_delete(key) } - fn get_purpose_token( - &self, - key: &PurposeTokenStorageKey, - ) -> Result> { - keyring_get_purpose_token(key) - } - - fn set_purpose_token( - &self, - key: &PurposeTokenStorageKey, - credential: &StoredPurposeTokenCredential, - ) -> Result<()> { - keyring_set_purpose_token(key, credential) - } - - fn delete_purpose_token(&self, key: &PurposeTokenStorageKey) -> Result { - keyring_delete_purpose_token(key) + fn delete_legacy_purpose_token_cache(&self, key: &SessionStorageKey) -> Result { + keyring_delete_legacy_compose_token(key) } } @@ -497,39 +366,14 @@ fn keyring_delete(key: &SessionStorageKey) -> Result { } #[cfg(target_os = "macos")] -fn keyring_get_purpose_token( - key: &PurposeTokenStorageKey, -) -> Result> { - use crate::keychain; - - let value = - keychain::get_generic_password_unscoped(PURPOSE_TOKEN_KEYRING_SERVICE, &key.account()) - .context("read BuilderBot purpose token from keyring")?; - value - .map(|value| { - serde_json::from_slice(&value).context("parse BuilderBot purpose token from keyring") - }) - .transpose() -} - -#[cfg(target_os = "macos")] -fn keyring_set_purpose_token( - key: &PurposeTokenStorageKey, - credential: &StoredPurposeTokenCredential, -) -> Result<()> { +fn keyring_delete_legacy_compose_token(key: &SessionStorageKey) -> Result { use crate::keychain; - let value = serde_json::to_vec(credential).context("serialize BuilderBot purpose token")?; - keychain::set_generic_password_unscoped(PURPOSE_TOKEN_KEYRING_SERVICE, &key.account(), &value) - .context("write BuilderBot purpose token to keyring") -} - -#[cfg(target_os = "macos")] -fn keyring_delete_purpose_token(key: &PurposeTokenStorageKey) -> Result { - use crate::keychain; - - keychain::delete_generic_password_unscoped(PURPOSE_TOKEN_KEYRING_SERVICE, &key.account()) - .context("delete BuilderBot purpose token from keyring") + keychain::delete_generic_password_unscoped( + LEGACY_PURPOSE_TOKEN_KEYRING_SERVICE, + &legacy_compose_token_account(key), + ) + .context("delete legacy BuilderBot Compose token from keyring") } #[cfg(not(target_os = "macos"))] @@ -548,22 +392,7 @@ fn keyring_delete(_key: &SessionStorageKey) -> Result { } #[cfg(not(target_os = "macos"))] -fn keyring_get_purpose_token( - _key: &PurposeTokenStorageKey, -) -> Result> { - unsupported_keyring_storage() -} - -#[cfg(not(target_os = "macos"))] -fn keyring_set_purpose_token( - _key: &PurposeTokenStorageKey, - _credential: &StoredPurposeTokenCredential, -) -> Result<()> { - unsupported_keyring_storage() -} - -#[cfg(not(target_os = "macos"))] -fn keyring_delete_purpose_token(_key: &PurposeTokenStorageKey) -> Result { +fn keyring_delete_legacy_compose_token(_key: &SessionStorageKey) -> Result { unsupported_keyring_storage() } @@ -596,46 +425,6 @@ fn restrict_permissions(path: &PathBuf) -> Result<()> { Ok(()) } -fn write_private_file_atomically(path: &PathBuf, bytes: &[u8]) -> Result<()> { - if let Some(parent) = path.parent() { - let existed = parent.exists(); - fs::create_dir_all(parent).with_context(|| format!("create {}", parent.display()))?; - #[cfg(unix)] - if !existed { - use std::os::unix::fs::PermissionsExt; - fs::set_permissions(parent, fs::Permissions::from_mode(0o700)) - .with_context(|| format!("chmod 700 {}", parent.display()))?; - } - } - - let sequence = TEMP_FILE_SEQUENCE.fetch_add(1, Ordering::Relaxed); - let mut temporary_path = path.as_os_str().to_os_string(); - temporary_path.push(format!(".{}.{}.tmp", std::process::id(), sequence)); - let temporary_path = PathBuf::from(temporary_path); - let result = (|| { - let mut options = OpenOptions::new(); - options.write(true).create_new(true); - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - options.mode(0o600); - } - let mut file = options - .open(&temporary_path) - .with_context(|| format!("create {}", temporary_path.display()))?; - file.write_all(bytes) - .with_context(|| format!("write {}", temporary_path.display()))?; - file.sync_all() - .with_context(|| format!("sync {}", temporary_path.display()))?; - fs::rename(&temporary_path, path).with_context(|| format!("replace {}", path.display()))?; - restrict_permissions(path) - })(); - if result.is_err() { - let _ = fs::remove_file(&temporary_path); - } - result -} - #[cfg(test)] mod tests { use super::*; @@ -715,68 +504,27 @@ mod tests { } #[test] - fn file_storage_scopes_and_protects_purpose_tokens() { + fn file_storage_deletes_the_obsolete_legacy_purpose_token_cache() { let directory = std::env::temp_dir().join(format!( - "bb-purpose-token-storage-{}", + "bb-auth-storage-legacy-compose-delete-{}", std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .expect("clock") .as_nanos() )); + fs::create_dir_all(&directory).expect("create test directory"); let storage = FileSessionCredentialStorage::new(directory.join("sessions.json")); - let local_session = SessionStorageKey::new("default", "http://localhost:5173"); - let staging_session = SessionStorageKey::new("default", "https://staging.example"); - let local_compose = PurposeTokenStorageKey::new(&local_session, "compose"); - let staging_compose = PurposeTokenStorageKey::new(&staging_session, "compose"); - let local_other = PurposeTokenStorageKey::new(&local_session, "other"); - let credential = StoredPurposeTokenCredential { - access_token: "purpose-token".to_string(), - token_type: "Bearer".to_string(), - issued_at_unix_seconds: 100, - expires_at_unix_seconds: 400, - session_credential_sha256: "session-fingerprint".to_string(), - }; + let local = SessionStorageKey::new("default", "http://localhost:5173"); + let path = storage.legacy_purpose_tokens_path(); + fs::write(&path, b"legacy purpose-token contents").expect("write legacy tokens"); - storage - .set_purpose_token(&local_compose, &credential) - .expect("store purpose token"); - - assert_eq!( - storage - .get_purpose_token(&local_compose) - .expect("read purpose token") - .expect("purpose token") - .access_token, - "purpose-token" - ); - assert!(storage - .get_purpose_token(&staging_compose) - .expect("read staging purpose token") - .is_none()); assert!(storage - .get_purpose_token(&local_other) - .expect("read other purpose token") - .is_none()); - - let purpose_tokens_path = storage.purpose_tokens_path(); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - assert_eq!( - fs::metadata(&purpose_tokens_path) - .expect("purpose token metadata") - .permissions() - .mode() - & 0o777, - 0o600 - ); - } - assert!(storage - .delete_purpose_token(&local_compose) - .expect("delete purpose token")); + .delete_legacy_purpose_token_cache(&local) + .expect("delete legacy purpose-token cache")); assert!(!storage - .delete_purpose_token(&local_compose) - .expect("delete purpose token again")); + .delete_legacy_purpose_token_cache(&local) + .expect("delete legacy purpose-token cache again")); + assert!(!path.exists()); let _ = fs::remove_dir_all(directory); } @@ -818,14 +566,12 @@ mod tests { key.account(), "default@https://kgoose.stage.sqprod.co/cash-app/goose" ); - - let purpose_key = PurposeTokenStorageKey::new(&key, "compose"); assert_eq!( - PURPOSE_TOKEN_KEYRING_SERVICE, + LEGACY_PURPOSE_TOKEN_KEYRING_SERVICE, "com.squareup.builderbot.cli-auth-purpose-token" ); assert_eq!( - purpose_key.account(), + legacy_compose_token_account(&key), "compose@default@https://kgoose.stage.sqprod.co/cash-app/goose" ); }