From 9944e29ef056d77b14a121971a15bff20f4ae603 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Sun, 1 Feb 2026 16:15:13 -0700 Subject: [PATCH] fix: A critical security flaw was discovered that essentially had all local secrets be encrypted with an all-zero key --- src/bin/gman/cli.rs | 4 +- src/bin/gman/main.rs | 55 ++++- src/bin/gman/utils.rs | 2 +- src/lib.rs | 93 +++++--- src/providers/local.rs | 303 ++++++++++++++++++++++--- tests/bin/cli_tests.rs | 30 +-- tests/prop_crypto.proptest-regressions | 8 + tests/prop_crypto.rs | 5 +- 8 files changed, 412 insertions(+), 88 deletions(-) create mode 100644 tests/prop_crypto.proptest-regressions diff --git a/src/bin/gman/cli.rs b/src/bin/gman/cli.rs index b21c1b3..563d2df 100644 --- a/src/bin/gman/cli.rs +++ b/src/bin/gman/cli.rs @@ -1,8 +1,8 @@ use crate::command::preview_command; -use anyhow::{anyhow, Context, Result}; +use anyhow::{Context, Result, anyhow}; use clap_complete::CompletionCandidate; use futures::future::join_all; -use gman::config::{load_config, Config, RunConfig}; +use gman::config::{Config, RunConfig, load_config}; use log::{debug, error}; use regex::Regex; use std::collections::HashMap; diff --git a/src/bin/gman/main.rs b/src/bin/gman/main.rs index f4cf0fa..a01f4a2 100644 --- a/src/bin/gman/main.rs +++ b/src/bin/gman/main.rs @@ -4,12 +4,12 @@ use crate::cli::secrets_completer; use anyhow::{Context, Result}; use clap::Subcommand; use clap::{ - crate_authors, crate_description, crate_name, crate_version, CommandFactory, Parser, ValueEnum, + CommandFactory, Parser, ValueEnum, crate_authors, crate_description, crate_name, crate_version, }; use clap_complete::{ArgValueCompleter, CompleteEnv}; use crossterm::execute; -use crossterm::terminal::{disable_raw_mode, LeaveAlternateScreen}; -use gman::config::{get_config_file_path, load_config, Config}; +use crossterm::terminal::{LeaveAlternateScreen, disable_raw_mode}; +use gman::config::{Config, get_config_file_path, load_config}; use std::ffi::OsString; use std::io::{self, IsTerminal, Read, Write}; use std::panic::PanicHookInfo; @@ -116,6 +116,12 @@ enum Commands { /// Sync secrets with remote storage (if supported by the provider) Sync {}, + // TODO: Remove once all users have migrated their local vaults + /// Migrate local vault secrets to the current secure encryption format. + /// This is only needed if you have secrets encrypted with older versions of gman. + /// Only works with the local provider. + Migrate {}, + /// Open and edit the config file in the default text editor Config {}, @@ -258,6 +264,49 @@ async fn main() -> Result<()> { } })?; } + // TODO: Remove once all users have migrated their local vaults + Commands::Migrate {} => { + use gman::providers::SupportedProvider; + use gman::providers::local::LocalProvider; + + let provider_config_for_migrate = + config.extract_provider_config(cli.provider.clone())?; + + let local_provider: LocalProvider = match provider_config_for_migrate.provider_type { + SupportedProvider::Local { provider_def } => provider_def, + _ => { + anyhow::bail!("The migrate command only works with the local provider."); + } + }; + + println!("Migrating vault secrets to current secure format..."); + let result = local_provider.migrate_vault().await?; + + if result.total == 0 { + println!("Vault is empty, nothing to migrate."); + } else { + println!( + "Migration complete: {} total, {} migrated, {} already current", + result.total, result.migrated, result.already_current + ); + + if !result.failed.is_empty() { + eprintln!("\n⚠ Failed to migrate {} secret(s):", result.failed.len()); + for (key, error) in &result.failed { + eprintln!(" - {}: {}", key, error); + } + } + + if result.migrated > 0 { + println!( + "\n✓ Successfully migrated {} secret(s) to the secure format.", + result.migrated + ); + } else if result.failed.is_empty() { + println!("\n✓ All secrets are already using the current secure format."); + } + } + } Commands::External(tokens) => { wrap_and_run_command(cli.provider, &config, tokens, cli.profile, cli.dry_run).await?; } diff --git a/src/bin/gman/utils.rs b/src/bin/gman/utils.rs index 9172159..587f016 100644 --- a/src/bin/gman/utils.rs +++ b/src/bin/gman/utils.rs @@ -1,5 +1,5 @@ use anyhow::{Context, Result}; -use gman::config::{get_config_file_path, Config}; +use gman::config::{Config, get_config_file_path}; use log::LevelFilter; use log4rs::append::console::ConsoleAppender; use log4rs::append::file::FileAppender; diff --git a/src/lib.rs b/src/lib.rs index af50e8b..3acc808 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,10 +22,7 @@ //! filesystem. Prefer `no_run` doctests for those. use anyhow::{Context, Result, anyhow, bail}; -use argon2::{ - Algorithm, Argon2, Params, Version, - password_hash::{SaltString, rand_core::RngCore}, -}; +use argon2::{Algorithm, Argon2, Params, Version, password_hash::rand_core::RngCore}; use base64::{Engine as _, engine::general_purpose::STANDARD as B64}; use chacha20poly1305::{ Key, XChaCha20Poly1305, XNonce, @@ -43,8 +40,8 @@ pub(crate) const HEADER: &str = "$VAULT"; pub(crate) const VERSION: &str = "v1"; pub(crate) const KDF: &str = "argon2id"; -pub(crate) const ARGON_M_COST_KIB: u32 = 19_456; -pub(crate) const ARGON_T_COST: u32 = 2; +pub(crate) const ARGON_M_COST_KIB: u32 = 65_536; +pub(crate) const ARGON_T_COST: u32 = 3; pub(crate) const ARGON_P: u32 = 1; pub(crate) const SALT_LEN: usize = 16; @@ -84,14 +81,22 @@ fn derive_key(password: &SecretString, salt: &[u8]) -> Result { pub fn encrypt_string(password: impl Into, plaintext: &str) -> Result { let password = password.into(); - let salt = SaltString::generate(&mut OsRng); + if password.expose_secret().is_empty() { + bail!("password cannot be empty"); + } + + let mut salt = [0u8; SALT_LEN]; + OsRng.fill_bytes(&mut salt); let mut nonce_bytes = [0u8; NONCE_LEN]; OsRng.fill_bytes(&mut nonce_bytes); - let key = derive_key(&password, salt.as_str().as_bytes())?; + let mut key = derive_key(&password, &salt)?; let cipher = XChaCha20Poly1305::new(&key); - let aad = format!("{};{}", HEADER, VERSION); + let aad = format!( + "{};{};{};m={},t={},p={}", + HEADER, VERSION, KDF, ARGON_M_COST_KIB, ARGON_T_COST, ARGON_P + ); let nonce: XNonce = nonce_bytes.into(); let mut pt = plaintext.as_bytes().to_vec(); @@ -115,13 +120,14 @@ pub fn encrypt_string(password: impl Into, plaintext: &str) -> Res m = ARGON_M_COST_KIB, t = ARGON_T_COST, p = ARGON_P, - salt = B64.encode(salt.as_str().as_bytes()), + salt = B64.encode(salt), nonce = B64.encode(nonce_bytes), ct = B64.encode(&ct), ); drop(cipher); - let _ = key; + key.zeroize(); + salt.zeroize(); nonce_bytes.zeroize(); Ok(env) @@ -132,6 +138,9 @@ pub fn encrypt_string(password: impl Into, plaintext: &str) -> Res /// Returns the original plaintext on success or an error if the password is /// wrong, the envelope was tampered with, or the input is malformed. /// +/// This function supports both the current format (with KDF params in AAD) and +/// the legacy format (without KDF params in AAD) for backwards compatibility. +/// /// Example /// ``` /// use gman::{encrypt_string, decrypt_string}; @@ -145,6 +154,10 @@ pub fn encrypt_string(password: impl Into, plaintext: &str) -> Res pub fn decrypt_string(password: impl Into, envelope: &str) -> Result { let password = password.into(); + if password.expose_secret().is_empty() { + bail!("password cannot be empty"); + } + let parts: Vec<&str> = envelope.split(';').collect(); if parts.len() < 7 { bail!("invalid envelope format"); @@ -178,7 +191,7 @@ pub fn decrypt_string(password: impl Into, envelope: &str) -> Resu let nonce_b64 = parts[5].strip_prefix("nonce=").context("missing nonce")?; let ct_b64 = parts[6].strip_prefix("ct=").context("missing ct")?; - let salt_bytes = B64.decode(salt_b64).context("bad salt b64")?; + let mut salt_bytes = B64.decode(salt_b64).context("bad salt b64")?; let nonce_bytes = B64.decode(nonce_b64).context("bad nonce b64")?; let mut ct = B64.decode(ct_b64).context("bad ct b64")?; @@ -186,27 +199,47 @@ pub fn decrypt_string(password: impl Into, envelope: &str) -> Resu bail!("nonce length mismatch"); } - let key = derive_key(&password, &salt_bytes)?; + let mut key = derive_key(&password, &salt_bytes)?; let cipher = XChaCha20Poly1305::new(&key); - let aad = format!("{};{}", HEADER, VERSION); - let mut nonce_arr: [u8; NONCE_LEN] = nonce_bytes.try_into().map_err(|_| anyhow!("invalid nonce length"))?; - let nonce: XNonce = nonce_arr.into(); - let pt = cipher - .decrypt( - &nonce, - chacha20poly1305::aead::Payload { - msg: &ct, - aad: aad.as_bytes(), - }, - ) - .map_err(|_| anyhow!("decryption failed (wrong password or corrupted data)"))?; + let aad_new = format!("{};{};{};m={},t={},p={}", HEADER, VERSION, KDF, m, t, p); + let aad_legacy = format!("{};{}", HEADER, VERSION); + let mut nonce_arr: [u8; NONCE_LEN] = nonce_bytes + .try_into() + .map_err(|_| anyhow!("invalid nonce length"))?; + let nonce: XNonce = nonce_arr.into(); + + let decrypt_result = cipher.decrypt( + &nonce, + chacha20poly1305::aead::Payload { + msg: &ct, + aad: aad_new.as_bytes(), + }, + ); + + let mut pt = match decrypt_result { + Ok(pt) => pt, + Err(_) => cipher + .decrypt( + &nonce, + chacha20poly1305::aead::Payload { + msg: &ct, + aad: aad_legacy.as_bytes(), + }, + ) + .map_err(|_| anyhow!("decryption failed (wrong password or corrupted data)"))?, + }; + + let s = String::from_utf8(pt.clone()).context("plaintext not valid UTF-8")?; + + key.zeroize(); + salt_bytes.zeroize(); nonce_arr.zeroize(); ct.zeroize(); + pt.zeroize(); - let s = String::from_utf8(pt).context("plaintext not valid UTF-8")?; Ok(s) } @@ -248,12 +281,10 @@ mod tests { } #[test] - fn empty_password() { + fn empty_password_rejected() { let pw = SecretString::new("".into()); let msg = "hello"; - let env = encrypt_string(pw.clone(), msg).unwrap(); - let out = decrypt_string(pw, &env).unwrap(); - assert_eq!(msg, out); + assert!(encrypt_string(pw.clone(), msg).is_err()); } #[test] @@ -275,7 +306,7 @@ mod tests { let mut ct = base64::engine::general_purpose::STANDARD .decode(ct_b64) .unwrap(); - ct[0] ^= 0x01; // Flip a bit + ct[0] ^= 0x01; let new_ct_b64 = base64::engine::general_purpose::STANDARD.encode(&ct); let new_ct_part = format!("ct={}", new_ct_b64); parts[6] = &new_ct_part; diff --git a/src/providers/local.rs b/src/providers/local.rs index 45f69b2..6e9fa36 100644 --- a/src/providers/local.rs +++ b/src/providers/local.rs @@ -321,6 +321,22 @@ impl LocalProvider { fn get_password(&self) -> Result { if let Some(password_file) = &self.password_file { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let metadata = fs::metadata(password_file).with_context(|| { + format!("failed to read password file metadata {:?}", password_file) + })?; + let mode = metadata.permissions().mode(); + if mode & 0o077 != 0 { + bail!( + "password file {:?} has insecure permissions {:o} (should be 0600 or 0400)", + password_file, + mode & 0o777 + ); + } + } + let password = SecretString::new( fs::read_to_string(password_file) .with_context(|| format!("failed to read password file {:?}", password_file))? @@ -369,18 +385,35 @@ fn store_vault(path: &Path, map: &HashMap) -> Result<()> { fs::create_dir_all(parent).with_context(|| format!("create {}", parent.display()))?; } let s = serde_yaml::to_string(map).with_context(|| "serialize vault")?; - fs::write(path, s).with_context(|| format!("write {}", path.display())) + fs::write(path, &s).with_context(|| format!("write {}", path.display()))?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(path, fs::Permissions::from_mode(0o600)) + .with_context(|| format!("set permissions on {}", path.display()))?; + } + + Ok(()) } fn encrypt_string(password: &SecretString, plaintext: &str) -> Result { + if password.expose_secret().is_empty() { + bail!("password cannot be empty"); + } + let mut salt = [0u8; SALT_LEN]; OsRng.fill_bytes(&mut salt); let mut nonce_bytes = [0u8; NONCE_LEN]; OsRng.fill_bytes(&mut nonce_bytes); - let key = derive_key(password, &salt)?; + let mut key = derive_key(password, &salt)?; let cipher = XChaCha20Poly1305::new(&key); - let aad = format!("{};{}", HEADER, VERSION); + + let aad = format!( + "{};{};{};m={},t={},p={}", + HEADER, VERSION, KDF, ARGON_M_COST_KIB, ARGON_T_COST, ARGON_P + ); let nonce: XNonce = nonce_bytes.into(); let mut pt = plaintext.as_bytes().to_vec(); @@ -409,6 +442,7 @@ fn encrypt_string(password: &SecretString, plaintext: &str) -> Result { ); drop(cipher); + key.zeroize(); salt.zeroize(); nonce_bytes.zeroize(); @@ -438,7 +472,21 @@ fn derive_key(password: &SecretString, salt: &[u8]) -> Result { derive_key_with_params(password, salt, ARGON_M_COST_KIB, ARGON_T_COST, ARGON_P) } -fn decrypt_string(password: &SecretString, envelope: &str) -> Result { +/// Attempts to decrypt with the given cipher, nonce, ciphertext, and AAD. +fn try_decrypt( + cipher: &XChaCha20Poly1305, + nonce: &XNonce, + ct: &[u8], + aad: &[u8], +) -> std::result::Result, chacha20poly1305::aead::Error> { + cipher.decrypt(nonce, chacha20poly1305::aead::Payload { msg: ct, aad }) +} + +type EnvelopeComponents = (u32, u32, u32, Vec, [u8; NONCE_LEN], Vec); + +/// Parse an envelope string and extract its components. +/// Returns (m, t, p, salt, nonce_arr, ct) on success. +fn parse_envelope(envelope: &str) -> Result { let parts: Vec<&str> = envelope.trim().split(';').collect(); if parts.len() < 7 { debug!("Invalid envelope format: {:?}", parts); @@ -480,41 +528,202 @@ fn decrypt_string(password: &SecretString, envelope: &str) -> Result { .with_context(|| "missing nonce")?; let ct_b64 = parts[6].strip_prefix("ct=").with_context(|| "missing ct")?; - let mut salt = B64.decode(salt_b64).with_context(|| "bad salt b64")?; + let salt = B64.decode(salt_b64).with_context(|| "bad salt b64")?; let nonce_bytes = B64.decode(nonce_b64).with_context(|| "bad nonce b64")?; - let mut ct = B64.decode(ct_b64).with_context(|| "bad ct b64")?; + let ct = B64.decode(ct_b64).with_context(|| "bad ct b64")?; - if salt.len() != SALT_LEN || nonce_bytes.len() != NONCE_LEN { - debug!( - "Salt/nonce length mismatch: salt {}, nonce {}", - salt.len(), - nonce_bytes.len() - ); - bail!("salt/nonce length mismatch"); + if nonce_bytes.len() != NONCE_LEN { + debug!("Nonce length mismatch: {}", nonce_bytes.len()); + bail!("nonce length mismatch"); } - let key = derive_key_with_params(password, &salt, m, t, p)?; - let cipher = XChaCha20Poly1305::new(&key); - let aad = format!("{};{}", HEADER, VERSION); - let mut nonce_arr: [u8; NONCE_LEN] = nonce_bytes.try_into().map_err(|_| anyhow!("invalid nonce length"))?; + let nonce_arr: [u8; NONCE_LEN] = nonce_bytes + .try_into() + .map_err(|_| anyhow!("invalid nonce length"))?; + + Ok((m, t, p, salt, nonce_arr, ct)) +} + +fn decrypt_string(password: &SecretString, envelope: &str) -> Result { + if password.expose_secret().is_empty() { + bail!("password cannot be empty"); + } + + let (m, t, p, mut salt, mut nonce_arr, mut ct) = parse_envelope(envelope)?; let nonce: XNonce = nonce_arr.into(); - let pt = cipher - .decrypt( - &nonce, - chacha20poly1305::aead::Payload { - msg: &ct, - aad: aad.as_bytes(), - }, - ) - .map_err(|_| anyhow!("decryption failed (wrong password or corrupted data)"))?; + let aad_current = format!("{};{};{};m={},t={},p={}", HEADER, VERSION, KDF, m, t, p); + let mut key = derive_key_with_params(password, &salt, m, t, p)?; + let cipher = XChaCha20Poly1305::new(&key); + + if let Ok(pt) = try_decrypt(&cipher, &nonce, &ct, aad_current.as_bytes()) { + let s = String::from_utf8(pt.clone()).with_context(|| "plaintext not valid UTF-8")?; + key.zeroize(); + salt.zeroize(); + nonce_arr.zeroize(); + ct.zeroize(); + return Ok(s); + } + + key.zeroize(); salt.zeroize(); nonce_arr.zeroize(); ct.zeroize(); - let s = String::from_utf8(pt).with_context(|| "plaintext not valid UTF-8")?; - Ok(s) + // TODO: Remove once all users have migrated their local vaults + if let Ok(plaintext) = legacy::decrypt_string_legacy(password, envelope) { + return Ok(plaintext); + } + + bail!("decryption failed (wrong password or corrupted data)") +} + +// TODO: Remove this entire module once all users have migrated their vaults. +mod legacy { + use super::*; + + fn legacy_aad() -> String { + format!("{};{}", HEADER, VERSION) + } + + pub fn decrypt_string_legacy(password: &SecretString, envelope: &str) -> Result { + if password.expose_secret().is_empty() { + bail!("password cannot be empty"); + } + + let (m, t, p, mut salt, mut nonce_arr, mut ct) = parse_envelope(envelope)?; + let nonce: XNonce = nonce_arr.into(); + let aad = legacy_aad(); + + let mut key = derive_key_with_params(password, &salt, m, t, p)?; + let cipher = XChaCha20Poly1305::new(&key); + + if let Ok(pt) = try_decrypt(&cipher, &nonce, &ct, aad.as_bytes()) { + let s = String::from_utf8(pt.clone()).with_context(|| "plaintext not valid UTF-8")?; + key.zeroize(); + salt.zeroize(); + nonce_arr.zeroize(); + ct.zeroize(); + return Ok(s); + } + + key.zeroize(); + + let mut zeros_key: Key = [0u8; KEY_LEN].into(); + let zeros_cipher = XChaCha20Poly1305::new(&zeros_key); + + if let Ok(pt) = try_decrypt(&zeros_cipher, &nonce, &ct, aad.as_bytes()) { + debug!("Decrypted using legacy all-zeros key - secret needs migration"); + let s = String::from_utf8(pt.clone()).with_context(|| "plaintext not valid UTF-8")?; + zeros_key.zeroize(); + salt.zeroize(); + nonce_arr.zeroize(); + ct.zeroize(); + return Ok(s); + } + + zeros_key.zeroize(); + salt.zeroize(); + nonce_arr.zeroize(); + ct.zeroize(); + + bail!("legacy decryption failed") + } + + pub fn is_current_format(password: &SecretString, envelope: &str) -> Result { + if password.expose_secret().is_empty() { + bail!("password cannot be empty"); + } + + let (m, t, p, salt, nonce_arr, ct) = parse_envelope(envelope)?; + let nonce: XNonce = nonce_arr.into(); + + let aad_current = format!("{};{};{};m={},t={},p={}", HEADER, VERSION, KDF, m, t, p); + let key = derive_key_with_params(password, &salt, m, t, p)?; + let cipher = XChaCha20Poly1305::new(&key); + + Ok(try_decrypt(&cipher, &nonce, &ct, aad_current.as_bytes()).is_ok()) + } +} + +// TODO: Remove once all users have migrated their local vaults +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SecretStatus { + Current, + NeedsMigration, +} + +// TODO: Remove once all users have migrated their local vaults +#[derive(Debug)] +pub struct MigrationResult { + pub total: usize, + pub migrated: usize, + pub already_current: usize, + pub failed: Vec<(String, String)>, +} + +impl LocalProvider { + // TODO: Remove once all users have migrated their local vaults + pub async fn migrate_vault(&self) -> Result { + let vault_path = self.active_vault_path()?; + let vault: HashMap = load_vault(&vault_path).unwrap_or_default(); + + if vault.is_empty() { + return Ok(MigrationResult { + total: 0, + migrated: 0, + already_current: 0, + failed: vec![], + }); + } + + let password = self.get_password()?; + let mut migrated_vault = HashMap::new(); + let mut migrated_count = 0; + let mut already_current_count = 0; + let mut failed = vec![]; + + for (key, envelope) in &vault { + match legacy::is_current_format(&password, envelope) { + Ok(true) => { + migrated_vault.insert(key.clone(), envelope.clone()); + already_current_count += 1; + } + Ok(false) => match decrypt_string(&password, envelope) { + Ok(plaintext) => match encrypt_string(&password, &plaintext) { + Ok(new_envelope) => { + migrated_vault.insert(key.clone(), new_envelope); + migrated_count += 1; + } + Err(e) => { + failed.push((key.clone(), format!("re-encryption failed: {}", e))); + migrated_vault.insert(key.clone(), envelope.clone()); + } + }, + Err(e) => { + failed.push((key.clone(), format!("decryption failed: {}", e))); + migrated_vault.insert(key.clone(), envelope.clone()); + } + }, + Err(e) => { + failed.push((key.clone(), format!("status check failed: {}", e))); + migrated_vault.insert(key.clone(), envelope.clone()); + } + } + } + + if migrated_count > 0 { + store_vault(&vault_path, &migrated_vault)?; + } + + Ok(MigrationResult { + total: vault.len(), + migrated: migrated_count, + already_current: already_current_count, + failed, + }) + } } #[cfg(test)] @@ -530,7 +739,7 @@ mod tests { let password = SecretString::new("test_password".to_string().into()); let salt = [0u8; 16]; let key = derive_key(&password, &salt).unwrap(); - assert_eq!(key.as_slice().len(), 32); + assert_eq!(key.len(), 32); } #[test] @@ -538,7 +747,7 @@ mod tests { let password = SecretString::new("test_password".to_string().into()); let salt = [0u8; 16]; let key = derive_key_with_params(&password, &salt, 10, 1, 1).unwrap(); - assert_eq!(key.as_slice().len(), 32); + assert_eq!(key.len(), 32); } #[test] @@ -551,6 +760,40 @@ mod tests { } #[test] + #[cfg(unix)] + fn get_password_reads_password_file() { + use std::os::unix::fs::PermissionsExt; + let dir = tempdir().unwrap(); + let file = dir.path().join("pw.txt"); + fs::write(&file, "secretpw\n").unwrap(); + fs::set_permissions(&file, fs::Permissions::from_mode(0o600)).unwrap(); + let provider = LocalProvider { + password_file: Some(file), + runtime_provider_name: None, + ..LocalProvider::default() + }; + let pw = provider.get_password().unwrap(); + assert_eq!(pw.expose_secret(), "secretpw"); + } + + #[test] + #[cfg(unix)] + fn get_password_rejects_insecure_file() { + use std::os::unix::fs::PermissionsExt; + let dir = tempdir().unwrap(); + let file = dir.path().join("pw.txt"); + fs::write(&file, "secretpw\n").unwrap(); + fs::set_permissions(&file, fs::Permissions::from_mode(0o644)).unwrap(); + let provider = LocalProvider { + password_file: Some(file), + runtime_provider_name: None, + ..LocalProvider::default() + }; + assert!(provider.get_password().is_err()); + } + + #[test] + #[cfg(not(unix))] fn get_password_reads_password_file() { let dir = tempdir().unwrap(); let file = dir.path().join("pw.txt"); diff --git a/tests/bin/cli_tests.rs b/tests/bin/cli_tests.rs index dca4175..b0d1f4f 100644 --- a/tests/bin/cli_tests.rs +++ b/tests/bin/cli_tests.rs @@ -46,20 +46,26 @@ providers: password_file.display() ) }; - // Confy with yaml feature typically uses .yml; write both to be safe. fs::write(app_dir.join("config.yml"), &cfg).unwrap(); fs::write(app_dir.join("config.yaml"), &cfg).unwrap(); } +fn create_password_file(path: &Path, content: &[u8]) { + fs::write(path, content).unwrap(); + #[cfg(unix)] + { + fs::set_permissions(path, fs::Permissions::from_mode(0o600)).unwrap(); + } +} + #[test] #[cfg(unix)] fn cli_config_no_changes() { let (td, xdg_cfg, xdg_cache) = setup_env(); let pw_file = td.path().join("pw.txt"); - fs::write(&pw_file, b"pw\n").unwrap(); + create_password_file(&pw_file, b"pw\n"); write_yaml_config(&xdg_cfg, &pw_file, None); - // Create a no-op editor script that exits successfully without modifying the file let editor = td.path().join("noop-editor.sh"); fs::write(&editor, b"#!/bin/sh\nexit 0\n").unwrap(); let mut perms = fs::metadata(&editor).unwrap().permissions(); @@ -82,10 +88,9 @@ fn cli_config_no_changes() { fn cli_config_updates_and_persists() { let (td, xdg_cfg, xdg_cache) = setup_env(); let pw_file = td.path().join("pw.txt"); - fs::write(&pw_file, b"pw\n").unwrap(); + create_password_file(&pw_file, b"pw\n"); write_yaml_config(&xdg_cfg, &pw_file, None); - // Editor script appends a valid run_configs section to the YAML file let editor = td.path().join("append-run-config.sh"); let script = r#"#!/bin/sh FILE="$1" @@ -111,7 +116,6 @@ exit 0 "Configuration updated successfully", )); - // Verify that the config file now contains the run_configs key let cfg_path = xdg_cfg.join("gman").join("config.yml"); let written = fs::read_to_string(&cfg_path).expect("config file readable"); assert!(written.contains("run_configs:")); @@ -134,10 +138,9 @@ fn cli_shows_help() { fn cli_add_get_list_update_delete_roundtrip() { let (td, xdg_cfg, xdg_cache) = setup_env(); let pw_file = td.path().join("pw.txt"); - fs::write(&pw_file, b"testpw\n").unwrap(); + create_password_file(&pw_file, b"testpw\n"); write_yaml_config(&xdg_cfg, &pw_file, None); - // add let mut add = Command::cargo_bin("gman").unwrap(); add.env("XDG_CONFIG_HOME", &xdg_cfg) .env("XDG_CACHE_HOME", &xdg_cache) @@ -154,7 +157,6 @@ fn cli_add_get_list_update_delete_roundtrip() { let add_out = child.wait_with_output().unwrap(); assert!(add_out.status.success()); - // get (text) let mut get = Command::cargo_bin("gman").unwrap(); get.env("XDG_CONFIG_HOME", &xdg_cfg) .env("XDG_CACHE_HOME", &xdg_cache) @@ -163,7 +165,6 @@ fn cli_add_get_list_update_delete_roundtrip() { .success() .stdout(predicate::str::contains("super_secret")); - // get as JSON let mut get_json = Command::cargo_bin("gman").unwrap(); get_json .env("XDG_CONFIG_HOME", &xdg_cfg) @@ -173,7 +174,6 @@ fn cli_add_get_list_update_delete_roundtrip() { predicate::str::contains("my_api_key").and(predicate::str::contains("super_secret")), ); - // list let mut list = Command::cargo_bin("gman").unwrap(); list.env("XDG_CONFIG_HOME", &xdg_cfg) .env("XDG_CACHE_HOME", &xdg_cache) @@ -182,7 +182,6 @@ fn cli_add_get_list_update_delete_roundtrip() { .success() .stdout(predicate::str::contains("my_api_key")); - // update let mut update = Command::cargo_bin("gman").unwrap(); update .env("XDG_CONFIG_HOME", &xdg_cfg) @@ -199,7 +198,6 @@ fn cli_add_get_list_update_delete_roundtrip() { let upd_out = child.wait_with_output().unwrap(); assert!(upd_out.status.success()); - // get again let mut get2 = Command::cargo_bin("gman").unwrap(); get2.env("XDG_CONFIG_HOME", &xdg_cfg) .env("XDG_CACHE_HOME", &xdg_cache) @@ -208,14 +206,12 @@ fn cli_add_get_list_update_delete_roundtrip() { .success() .stdout(predicate::str::contains("new_val")); - // delete let mut del = Command::cargo_bin("gman").unwrap(); del.env("XDG_CONFIG_HOME", &xdg_cfg) .env("XDG_CACHE_HOME", &xdg_cache) .args(["delete", "my_api_key"]); del.assert().success(); - // get should now fail let mut get_missing = Command::cargo_bin("gman").unwrap(); get_missing .env("XDG_CONFIG_HOME", &xdg_cfg) @@ -228,10 +224,9 @@ fn cli_add_get_list_update_delete_roundtrip() { fn cli_wrap_dry_run_env_injection() { let (td, xdg_cfg, xdg_cache) = setup_env(); let pw_file = td.path().join("pw.txt"); - fs::write(&pw_file, b"pw\n").unwrap(); + create_password_file(&pw_file, b"pw\n"); write_yaml_config(&xdg_cfg, &pw_file, Some("echo")); - // Add the secret so the profile can read it let mut add = Command::cargo_bin("gman").unwrap(); add.env("XDG_CONFIG_HOME", &xdg_cfg) .env("XDG_CACHE_HOME", &xdg_cache) @@ -243,7 +238,6 @@ fn cli_wrap_dry_run_env_injection() { let add_out = child.wait_with_output().unwrap(); assert!(add_out.status.success()); - // Dry-run wrapping: prints preview command let mut wrap = Command::cargo_bin("gman").unwrap(); wrap.env("XDG_CONFIG_HOME", &xdg_cfg) .env("XDG_CACHE_HOME", &xdg_cache) diff --git a/tests/prop_crypto.proptest-regressions b/tests/prop_crypto.proptest-regressions new file mode 100644 index 0000000..26996d7 --- /dev/null +++ b/tests/prop_crypto.proptest-regressions @@ -0,0 +1,8 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 155469a45d7311cd4003e23a3bcdaa8e55879e6222c1b6313a2b1f0b563bb195 # shrinks to password = "", msg = " " +cc 0bc9f608677234c082d10ff51b15dc39b4c194cdf920b4d87e553467c93824ed # shrinks to password = "", msg = "" diff --git a/tests/prop_crypto.rs b/tests/prop_crypto.rs index 9110a72..6cf9211 100644 --- a/tests/prop_crypto.rs +++ b/tests/prop_crypto.rs @@ -9,7 +9,7 @@ use secrecy::SecretString; proptest! { #[test] - fn prop_encrypt_decrypt_roundtrip(password in ".{0,64}", msg in ".{0,512}") { + fn prop_encrypt_decrypt_roundtrip(password in ".{1,64}", msg in ".{0,512}") { let pw = SecretString::new(password.into()); let env = encrypt_string(pw.clone(), &msg).unwrap(); let out = decrypt_string(pw, &env).unwrap(); @@ -18,10 +18,9 @@ proptest! { } #[test] - fn prop_tamper_ciphertext_detected(password in ".{0,32}", msg in ".{1,128}") { + fn prop_tamper_ciphertext_detected(password in ".{1,32}", msg in ".{1,128}") { let pw = SecretString::new(password.into()); let env = encrypt_string(pw.clone(), &msg).unwrap(); - // Flip a bit in the ct payload segment let mut parts: Vec<&str> = env.split(';').collect(); let ct_b64 = parts[6].strip_prefix("ct=").unwrap(); let mut ct = base64::engine::general_purpose::STANDARD.decode(ct_b64).unwrap();