fix(colibri-vault): correct field contract (name=KEY, not username=KEY)
- 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
This commit is contained in:
parent
3e1951762c
commit
fa7fe1c42b
1 changed files with 50 additions and 6 deletions
|
|
@ -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()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue