diff --git a/crates/colibri-vault/Cargo.toml b/crates/colibri-vault/Cargo.toml index 7555cf8..0c19f0e 100644 --- a/crates/colibri-vault/Cargo.toml +++ b/crates/colibri-vault/Cargo.toml @@ -9,4 +9,4 @@ serde = { version = "1", features = ["derive"] } serde_json = "1" thiserror = "2" tracing = "0.1" -tokio = { version = "1", features = ["process", "rt"] } +tokio = { version = "1", features = ["process", "rt", "sync", "macros"] } diff --git a/crates/colibri-vault/src/lib.rs b/crates/colibri-vault/src/lib.rs index 4bbfc7f..4c7abcb 100644 --- a/crates/colibri-vault/src/lib.rs +++ b/crates/colibri-vault/src/lib.rs @@ -22,8 +22,10 @@ use std::path::{Path, PathBuf}; use std::process::Stdio; +use std::sync::OnceLock; use thiserror::Error; use tokio::process::Command; +use tokio::sync::Mutex; #[derive(Error, Debug)] pub enum VaultError { @@ -39,6 +41,9 @@ pub enum VaultError { #[error("bw unlock returned an empty session")] EmptySession, + #[error("bw is logged in to server '{current}' but expected '{expected}' — logout and rerun")] + ServerMismatch { expected: String, current: String }, + #[error("collection not found: {0}")] CollectionNotFound(String), @@ -78,6 +83,12 @@ pub async fn provision( std::fs::write(&test, b"")?; std::fs::remove_file(&test)?; + // Serialize the whole login→unlock→fetch→lock lifecycle: `bw` keeps + // process-global state (one configured server + session per process), so + // concurrent provisions could otherwise tear down each other's session + // mid-fetch or race on `bw config server`. + let _guard = vault_lock().lock().await; + let session = VaultSession::login_unlock().await?; let result = provision_with_session(collection_name, target_dir, &session).await; finish_with_lock(result, session).await @@ -104,8 +115,26 @@ impl VaultSession { require_bootstrap_env("BW_PASSWORD")?; if let Ok(server) = std::env::var("BW_SERVER") { - if !server.trim().is_empty() { - bw(&["config", "server", server.trim()]).await?; + let server = server.trim(); + if !server.is_empty() { + match bw(&["config", "server", server]).await { + Ok(_) => {} + // bw refuses `config server` while already logged in. Tolerate + // that ONLY if the already-configured server matches what we + // expect; otherwise fail closed, so a stale login can't fetch + // from the wrong Bitwarden host. + Err(VaultError::BwFailed(msg)) if is_server_config_locked(&msg) => { + let current = bw(&["config", "server"]).await.unwrap_or_default(); + let current = current.trim(); + if current != server { + return Err(VaultError::ServerMismatch { + expected: server.to_string(), + current: current.to_string(), + }); + } + } + Err(e) => return Err(e), + } } } @@ -185,14 +214,27 @@ async fn provision_with_session( env_content.push_str(&format!("{key}={val}\n")); } } - // Also handle secure notes (KEY=VALUE pairs separated by newlines) + // Also handle secure notes (KEY=VALUE pairs separated by newlines). + // Validate the key the same way as login items, so a note can't inject + // an unchecked key into the .env. if item.kind == 2 { // Secure note if let Some(notes) = &item.notes { for line in notes.lines() { let line = line.trim(); - if line.contains('=') && !line.starts_with('#') { - env_content.push_str(&format!("{line}\n")); + if line.is_empty() || line.starts_with('#') { + continue; + } + if let Some((raw_key, val)) = line.split_once('=') { + let key = validate_key(raw_key.trim()); + if key.is_empty() { + tracing::warn!( + item = item.name, + "skipping note line with invalid env key" + ); + continue; + } + env_content.push_str(&format!("{key}={}\n", val.trim())); } } } @@ -287,6 +329,21 @@ fn is_already_logged_in(message: &str) -> bool { msg.contains("already logged in") || msg.contains("you are logged in") } +/// True when `bw config server` is refused because the CLI is already +/// authenticated (matches the clawdie-vault-fetch helper's tolerated cases). +fn is_server_config_locked(message: &str) -> bool { + let msg = message.to_ascii_lowercase(); + msg.contains("logout required") + || msg.contains("already configured") + || msg.contains("already set") +} + +/// Process-wide lock serializing all `bw` interaction (login/unlock/fetch/lock). +fn vault_lock() -> &'static Mutex<()> { + static VAULT_LOCK: OnceLock> = OnceLock::new(); + VAULT_LOCK.get_or_init(|| Mutex::new(())) +} + fn which(cmd: &str) -> Option { std::env::var_os("PATH").and_then(|path| { std::env::split_paths(&path).find_map(|dir| { @@ -379,6 +436,24 @@ mod tests { assert!(!is_already_logged_in("Invalid client secret.")); } + #[test] + fn server_config_locked_detection_matches_bw_cli_text() { + assert!(is_server_config_locked( + "Logout required before server config update." + )); + assert!(is_server_config_locked("Server is already configured.")); + assert!(!is_server_config_locked("Invalid client secret.")); + } + + #[tokio::test] + async fn vault_lock_serializes() { + // Two holders cannot both hold the lock at once. + let g1 = vault_lock().lock().await; + assert!(vault_lock().try_lock().is_err()); + drop(g1); + assert!(vault_lock().try_lock().is_ok()); + } + #[test] fn validate_key_rejects_dangerous_chars() { assert_eq!(validate_key("MY_KEY"), "MY_KEY");