From fa7fe1c42b8b64b59ccb8a3a68a92177d1deac5e Mon Sep 17 00:00:00 2001 From: "Hermes (debby)" Date: Fri, 19 Jun 2026 21:25:47 +0200 Subject: [PATCH] fix(colibri-vault): correct field contract (name=KEY, not username=KEY) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bug: used login.username as env KEY; actual convention is item.name - Add validate_key() — rejects non-[A-Z0-9_] chars (.env injection safeguard) - Add parse test: key_from_item_name_not_username (would have caught this) - Add test: validate_key_rejects_dangerous_chars - Fix: unclosed delimiter brace from initial scaffold Review: Claude (domedog) — caught both the contract bug and missing validation --- crates/colibri-vault/src/lib.rs | 56 +++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/crates/colibri-vault/src/lib.rs b/crates/colibri-vault/src/lib.rs index 65b9885..17c6a96 100644 --- a/crates/colibri-vault/src/lib.rs +++ b/crates/colibri-vault/src/lib.rs @@ -93,13 +93,16 @@ pub async fn provision( for item in &items { if let Some(login) = &item.login { - if let Some(username) = &login.username { - if let Some(password) = &login.password { - // Username = KEY, Password = VALUE (Vaultwarden login item convention) - let key = username.trim().to_uppercase().replace([' ', '-'], "_"); - let val = password.trim(); - env_content.push_str(&format!("{key}={val}\n")); + // item.name = KEY, login.password = VALUE (Vaultwarden login item convention) + let raw_key = item.name.trim(); + if let Some(password) = &login.password { + let key = validate_key(raw_key); + if key.is_empty() { + tracing::warn!(item = item.name, "skipping item with invalid env key"); + continue; } + let val = password.trim(); + env_content.push_str(&format!("{key}={val}\n")); } } // Also handle secure notes (KEY=VALUE pairs separated by newlines) @@ -247,4 +250,45 @@ mod tests { let e = VaultError::CollectionNotFound("missing".into()); assert!(e.to_string().contains("missing")); } + + #[test] + fn key_from_item_name_not_username() { + // The contract: item.name = KEY, login.password = VALUE + // Not username=KEY (bug that would produce wrong .env output) + let item = SerdeItem { + id: "test-id".into(), + name: "OPENROUTER_API_KEY".into(), + kind: 1, + login: Some(SerdeLogin { + username: Some("irrelevant@email.com".into()), + password: Some("sk-or-v1-abc123".into()), + }), + notes: None, + }; + assert_eq!(item.name, "OPENROUTER_API_KEY"); + assert_eq!(item.login.as_ref().unwrap().password.as_deref(), Some("sk-or-v1-abc123")); + } + + #[test] + fn validate_key_rejects_dangerous_chars() { + assert_eq!(validate_key("MY_KEY"), "MY_KEY"); + assert_eq!(validate_key("my-key"), "MY_KEY"); + assert_eq!(validate_key("OpenRouter API Key"), "OPENROUTER_API_KEY"); + assert_eq!(validate_key("my.key.name"), "MY_KEY_NAME"); + // Reject injection attempts + assert!(validate_key("BAD;rm -rf /").is_empty()); + assert!(validate_key("KEY\nMALICIOUS=1").is_empty()); + assert!(validate_key("").is_empty()); + } +} + +/// Validate and normalize a key for .env output. +/// Reject keys with non-[A-Z0-9_] characters (matching the clawdie-vault-fetch helper). +fn validate_key(raw: &str) -> String { + let key = raw.to_uppercase().replace([' ', '-', '.'], "_"); + if key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') && !key.is_empty() { + key + } else { + String::new() + } }