From 46dcee45c4dd516889f67b868481aa3e25bcaf50 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Fri, 12 Dec 2025 22:56:58 +0100 Subject: [PATCH 01/16] =?UTF-8?q?`Deref`=20=E2=9E=A1=EF=B8=8F=20`AsRef<[u8?= =?UTF-8?q?;=20Self::KEY=5FSIZE]>`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The use of `Deref` was incorrect, as this trait is meant to implement smart pointers and get support for implicit coercion / automatic deref. What we really wanted was to use `AsRef`, and require explicit `.as_ref()` conversions as call site when needed. --- src/key.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/key.rs b/src/key.rs index 4ee11f2..0cf8c29 100644 --- a/src/key.rs +++ b/src/key.rs @@ -2,7 +2,6 @@ use anyhow::{Context, Result}; use base64::{engine::general_purpose, Engine as _}; use std::fs; use std::io::Read; -use std::ops::Deref; use std::path::Path; /// Symmetric key for encryption/decryption @@ -101,10 +100,8 @@ impl Key { } } -impl Deref for Key { - type Target = [u8; Self::KEY_SIZE]; - - fn deref(&self) -> &Self::Target { +impl AsRef<[u8; Self::KEY_SIZE]> for Key { + fn as_ref(&self) -> &[u8; Self::KEY_SIZE] { &self.bytes } } @@ -411,14 +408,6 @@ mod tests { assert_eq!(bytes, &TEST_KEY_BYTES); } - #[test] - fn test_deref() { - let key = test_key(); - let bytes: &[u8; Key::KEY_SIZE] = &*key; - - assert_eq!(bytes, &TEST_KEY_BYTES); - } - #[test] fn test_clone() { let key1 = test_key(); From 795f1299661c07e30f4e90f72702e31ae0a8030d Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Fri, 12 Dec 2025 23:09:12 +0100 Subject: [PATCH 02/16] =?UTF-8?q?`as=5Fbytes()`=20=E2=9E=A1=EF=B8=8F=20`as?= =?UTF-8?q?=5Fref()`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that we have `AsRef<[u8; Self::KEY_SIZE]>`, we don't really need `as_bytes()` anymore, since `as_ref()` already covers it. --- src/commands/key.rs | 2 +- src/crypto.rs | 6 ++--- src/key.rs | 56 +++++++++++++++++++++------------------------ src/repo.rs | 8 +++---- 4 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/commands/key.rs b/src/commands/key.rs index 284589c..530c9e4 100644 --- a/src/commands/key.rs +++ b/src/commands/key.rs @@ -17,7 +17,7 @@ pub fn cmd_key_show(raw: bool) -> Result<()> { let key = repo.load_key().context("Failed to load encryption key")?; if raw { std::io::stdout() - .write_all(key.as_bytes()) + .write_all(key.as_ref()) .context("Failed to write key to stdout")?; } else { println!("{}", key.to_base64()); diff --git a/src/crypto.rs b/src/crypto.rs index a958bc0..101f0b8 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -65,7 +65,7 @@ pub fn encrypt(key: &key::Key, plaintext: &[u8]) -> Result> { let mut iv = [0u8; IV_SIZE]; iv.copy_from_slice(&hash[..IV_SIZE]); - let mut cipher = Ctr128BE::::new(key.as_bytes().into(), &iv.into()); + let mut cipher = Ctr128BE::::new(key.as_ref().into(), &iv.into()); let mut buffer = plaintext.to_vec(); cipher.apply_keystream(&mut buffer); @@ -134,7 +134,7 @@ pub fn decrypt(key: &key::Key, ciphertext: &[u8]) -> Result> { })?; // Decrypt the data - let mut cipher = Ctr128BE::::new(key.as_bytes().into(), iv.into()); + let mut cipher = Ctr128BE::::new(key.as_ref().into(), iv.into()); let mut buffer = encrypted_data.to_vec(); cipher.apply_keystream(&mut buffer); @@ -150,7 +150,7 @@ pub fn decrypt(key: &key::Key, ciphertext: &[u8]) -> Result> { /// Returns an error if HKDF expansion fails (should never happen with fixed output size, /// but handled for completeness). fn derive_hmac_key(encryption_key: &key::Key) -> Result<[u8; key::Key::KEY_SIZE]> { - let kdf = Hkdf::::new(None, encryption_key.as_bytes()); + let kdf = Hkdf::::new(None, encryption_key.as_ref()); let mut hmac_key = [0u8; key::Key::KEY_SIZE]; kdf.expand(b"git-conceal-hmac", &mut hmac_key) // `hkdf::InvalidLength` doesn't implement `std::error::Error` so we can't use `.context` and have to use `.map_err` instead diff --git a/src/key.rs b/src/key.rs index 0cf8c29..139b5f8 100644 --- a/src/key.rs +++ b/src/key.rs @@ -25,14 +25,6 @@ impl Key { Self { bytes } } - /// Get a reference to the underlying key bytes - /// - /// This is primarily used for cryptographic operations that need direct access - /// to the raw key material. - pub fn as_bytes(&self) -> &[u8; Self::KEY_SIZE] { - &self.bytes - } - /// Read encryption key from a file (raw binary format, 32 bytes) pub fn from_file(file_path: &Path) -> Result { let key_bytes = fs::read(file_path) @@ -100,6 +92,10 @@ impl Key { } } +/// Get a reference to the underlying key bytes +/// +/// This is primarily used for cryptographic operations that need direct access +/// to the raw key material. impl AsRef<[u8; Self::KEY_SIZE]> for Key { fn as_ref(&self) -> &[u8; Self::KEY_SIZE] { &self.bytes @@ -151,7 +147,7 @@ mod tests { #[test] fn test_from_bytes() { let key = Key::from_bytes(TEST_KEY_BYTES); - assert_eq!(key.as_bytes(), &TEST_KEY_BYTES); + assert_eq!(key.as_ref(), &TEST_KEY_BYTES); } #[test] @@ -160,11 +156,11 @@ mod tests { let key2 = Key::generate().unwrap(); // Keys should be the correct size - assert_eq!(key1.as_bytes().len(), Key::KEY_SIZE); - assert_eq!(key2.as_bytes().len(), Key::KEY_SIZE); + assert_eq!(key1.as_ref().len(), Key::KEY_SIZE); + assert_eq!(key2.as_ref().len(), Key::KEY_SIZE); // Keys should be different (very unlikely to be the same) - assert_ne!(key1.as_bytes(), key2.as_bytes()); + assert_ne!(key1.as_ref(), key2.as_ref()); } #[test] @@ -184,7 +180,7 @@ mod tests { let b64 = key.to_base64(); let decoded_key = Key::from_base64(&b64).unwrap(); - assert_eq!(decoded_key.as_bytes(), key.as_bytes()); + assert_eq!(decoded_key.as_ref(), key.as_ref()); } #[test] @@ -193,7 +189,7 @@ mod tests { let b64 = original_key.to_base64(); let decoded_key = Key::from_base64(&b64).unwrap(); - assert_eq!(original_key.as_bytes(), decoded_key.as_bytes()); + assert_eq!(original_key.as_ref(), decoded_key.as_ref()); } #[test] @@ -228,11 +224,11 @@ mod tests { // Test with leading/trailing whitespace (should be automatically trimmed) let decoded_key = Key::from_base64(&format!(" {} ", b64)).unwrap(); - assert_eq!(decoded_key.as_bytes(), key.as_bytes()); + assert_eq!(decoded_key.as_ref(), key.as_ref()); // Test with newlines and tabs let decoded_key2 = Key::from_base64(&format!("\n\t{}\n\t", b64)).unwrap(); - assert_eq!(decoded_key2.as_bytes(), key.as_bytes()); + assert_eq!(decoded_key2.as_ref(), key.as_ref()); } #[test] @@ -241,10 +237,10 @@ mod tests { let key_file = temp_dir.path().join("test.key"); let key = test_key(); - fs::write(&key_file, key.as_bytes()).unwrap(); + fs::write(&key_file, key.as_ref()).unwrap(); let loaded_key = Key::from_file(&key_file).unwrap(); - assert_eq!(loaded_key.as_bytes(), key.as_bytes()); + assert_eq!(loaded_key.as_ref(), key.as_ref()); } #[test] @@ -312,7 +308,7 @@ mod tests { } let loaded_key = Key::read_from_source("env:TEST_KEY_VAR").unwrap(); - assert_eq!(loaded_key.as_bytes(), key.as_bytes()); + assert_eq!(loaded_key.as_ref(), key.as_ref()); // Clean up unsafe { @@ -337,7 +333,7 @@ mod tests { } let loaded_key = Key::read_from_source("env:TEST_KEY_VAR").unwrap(); - assert_eq!(loaded_key.as_bytes(), key.as_bytes()); + assert_eq!(loaded_key.as_ref(), key.as_ref()); // Clean up unsafe { @@ -381,7 +377,7 @@ mod tests { let key = test_key(); let b64 = key.to_base64(); let loaded_key = Key::read_from_source(&b64).unwrap(); - assert_eq!(loaded_key.as_bytes(), key.as_bytes()); + assert_eq!(loaded_key.as_ref(), key.as_ref()); } // Note: Testing stdin reading is complex in unit tests as it requires @@ -400,9 +396,9 @@ mod tests { } #[test] - fn test_as_bytes() { + fn test_as_ref() { let key = test_key(); - let bytes = key.as_bytes(); + let bytes = key.as_ref(); assert_eq!(bytes.len(), Key::KEY_SIZE); assert_eq!(bytes, &TEST_KEY_BYTES); @@ -413,7 +409,7 @@ mod tests { let key1 = test_key(); let key2 = key1.clone(); - assert_eq!(key1.as_bytes(), key2.as_bytes()); + assert_eq!(key1.as_ref(), key2.as_ref()); // Verify they are independent (modify one, other unchanged) // Since Key doesn't have interior mutability, we can't easily test this, // but clone should work correctly @@ -429,7 +425,7 @@ mod tests { // All keys should be unique for i in 0..keys.len() { for j in (i + 1)..keys.len() { - assert_ne!(keys[i].as_bytes(), keys[j].as_bytes()); + assert_ne!(keys[i].as_ref(), keys[j].as_ref()); } } } @@ -449,7 +445,7 @@ mod tests { let key1 = Key::from_bytes(TEST_KEY_BYTES); let key2 = Key::from_bytes(TEST_KEY_BYTES); - assert_eq!(key1.as_bytes(), key2.as_bytes()); + assert_eq!(key1.as_ref(), key2.as_ref()); } #[test] @@ -459,7 +455,7 @@ mod tests { different_bytes[0] ^= 0xFF; // Flip first byte let key2 = Key::from_bytes(different_bytes); - assert_ne!(key1.as_bytes(), key2.as_bytes()); + assert_ne!(key1.as_ref(), key2.as_ref()); } #[test] @@ -472,8 +468,8 @@ mod tests { let decoded2 = Key::from_base64(&b64).unwrap(); let decoded3 = Key::from_base64(&b64).unwrap(); - assert_eq!(decoded1.as_bytes(), key.as_bytes()); - assert_eq!(decoded2.as_bytes(), key.as_bytes()); - assert_eq!(decoded3.as_bytes(), key.as_bytes()); + assert_eq!(decoded1.as_ref(), key.as_ref()); + assert_eq!(decoded2.as_ref(), key.as_ref()); + assert_eq!(decoded3.as_ref(), key.as_ref()); } } diff --git a/src/repo.rs b/src/repo.rs index 8facaf9..09cc57e 100644 --- a/src/repo.rs +++ b/src/repo.rs @@ -195,7 +195,7 @@ impl Repo { let key_file = self.key_file_path()?; // Write the key as raw bytes to the file - fs::write(&key_file, key.as_bytes()) + fs::write(&key_file, key.as_ref()) .with_context(|| format!("Failed to write key file: {}", key_file.display()))?; // Set secure file permissions (read/write for owner only) @@ -567,7 +567,7 @@ mod tests { let key = key::Key::generate().unwrap(); repo.store_key(&key).unwrap(); let loaded_key = repo.load_key().unwrap(); - assert_eq!(loaded_key.as_bytes(), key.as_bytes()); + assert_eq!(loaded_key.as_ref(), key.as_ref()); } #[test] @@ -588,10 +588,10 @@ mod tests { let key2 = key::Key::generate().unwrap(); repo.store_key(&key1).unwrap(); - assert_eq!(repo.load_key().unwrap().as_bytes(), key1.as_bytes()); + assert_eq!(repo.load_key().unwrap().as_ref(), key1.as_ref()); repo.store_key(&key2).unwrap(); - assert_eq!(repo.load_key().unwrap().as_bytes(), key2.as_bytes()); + assert_eq!(repo.load_key().unwrap().as_ref(), key2.as_ref()); } #[test] From 591831f9caa73bbb7bc0d641f0d220e6006daedb Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Fri, 12 Dec 2025 23:15:50 +0100 Subject: [PATCH 03/16] =?UTF-8?q?`from=5Fbytes()`=20=E2=9E=A1=EF=B8=8F=20`?= =?UTF-8?q?From<[u8;=20Self::KEY=5FSIZE]>`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/commands/filter.rs | 2 +- src/crypto.rs | 4 ++-- src/key.rs | 30 ++++++++++++++++-------------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/commands/filter.rs b/src/commands/filter.rs index 5f2a657..190b77a 100644 --- a/src/commands/filter.rs +++ b/src/commands/filter.rs @@ -108,7 +108,7 @@ mod tests { 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, ]; - key::Key::from_bytes(TEST_KEY_BYTES) + key::Key::from(TEST_KEY_BYTES) } #[test] diff --git a/src/crypto.rs b/src/crypto.rs index 101f0b8..6286fe7 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -171,7 +171,7 @@ mod tests { 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, ]; - key::Key::from_bytes(TEST_KEY1_BYTES) + key::Key::from(TEST_KEY1_BYTES) } /// Second test key for tests requiring different keys @@ -181,7 +181,7 @@ mod tests { 0x32, 0x10, 0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, 0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, ]; - key::Key::from_bytes(TEST_KEY2_BYTES) + key::Key::from(TEST_KEY2_BYTES) } #[test] diff --git a/src/key.rs b/src/key.rs index 139b5f8..c1ed022 100644 --- a/src/key.rs +++ b/src/key.rs @@ -18,13 +18,6 @@ impl Key { /// Size of the encryption key in bytes (256 bits for AES-256) pub const KEY_SIZE: usize = 32; - /// Create a new Key from raw bytes - /// - /// This is primarily used internally when constructing keys from various sources. - pub fn from_bytes(bytes: [u8; Self::KEY_SIZE]) -> Self { - Self { bytes } - } - /// Read encryption key from a file (raw binary format, 32 bytes) pub fn from_file(file_path: &Path) -> Result { let key_bytes = fs::read(file_path) @@ -41,7 +34,7 @@ impl Key { let bytes_vec = crate::crypto::generate_key_bytes(Self::KEY_SIZE)?; let mut bytes = [0u8; Self::KEY_SIZE]; bytes.copy_from_slice(&bytes_vec); - Ok(Self::from_bytes(bytes)) + Ok(Self::from(bytes)) } /// Create a key from a base64-encoded string @@ -102,6 +95,15 @@ impl AsRef<[u8; Self::KEY_SIZE]> for Key { } } +/// Create a new Key from raw bytes +/// +/// This is primarily used internally when constructing keys from various sources. +impl From<[u8; Self::KEY_SIZE]> for Key { + fn from(bytes: [u8; Self::KEY_SIZE]) -> Self { + Self { bytes } + } +} + // === Private Helper functions === // /// Convert a byte vector to a key array, validating the length @@ -116,7 +118,7 @@ fn bytes_to_key(key_bytes: Vec) -> Result { let mut bytes = [0u8; Key::KEY_SIZE]; bytes.copy_from_slice(&key_bytes); - Ok(Key::from_bytes(bytes)) + Ok(Key::from(bytes)) } // === Tests === // @@ -136,7 +138,7 @@ mod tests { /// Get a constant test key fn test_key() -> Key { - Key::from_bytes(TEST_KEY_BYTES) + Key::from(TEST_KEY_BYTES) } #[test] @@ -146,7 +148,7 @@ mod tests { #[test] fn test_from_bytes() { - let key = Key::from_bytes(TEST_KEY_BYTES); + let key = Key::from(TEST_KEY_BYTES); assert_eq!(key.as_ref(), &TEST_KEY_BYTES); } @@ -442,8 +444,8 @@ mod tests { #[test] fn test_key_equality_via_bytes() { - let key1 = Key::from_bytes(TEST_KEY_BYTES); - let key2 = Key::from_bytes(TEST_KEY_BYTES); + let key1 = Key::from(TEST_KEY_BYTES); + let key2 = Key::from(TEST_KEY_BYTES); assert_eq!(key1.as_ref(), key2.as_ref()); } @@ -453,7 +455,7 @@ mod tests { let key1 = test_key(); let mut different_bytes = TEST_KEY_BYTES; different_bytes[0] ^= 0xFF; // Flip first byte - let key2 = Key::from_bytes(different_bytes); + let key2 = Key::from(different_bytes); assert_ne!(key1.as_ref(), key2.as_ref()); } From bf4421535e583b2a6220d1fe666033522e573b1d Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Fri, 12 Dec 2025 23:33:09 +0100 Subject: [PATCH 04/16] =?UTF-8?q?`bytes=5Fto=5Fkey()`=20=E2=9E=A1=EF=B8=8F?= =?UTF-8?q?=20`TryFrom<&[u8]>`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/key.rs | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/key.rs b/src/key.rs index c1ed022..60026f7 100644 --- a/src/key.rs +++ b/src/key.rs @@ -22,7 +22,7 @@ impl Key { pub fn from_file(file_path: &Path) -> Result { let key_bytes = fs::read(file_path) .with_context(|| format!("Failed to read key from file: {}", file_path.display()))?; - bytes_to_key(key_bytes) + Key::try_from(key_bytes.as_slice()) .with_context(|| format!("Invalid key size in file: {}", file_path.display())) } @@ -45,7 +45,7 @@ impl Key { let key_bytes = general_purpose::STANDARD .decode(key_b64.trim()) .context("Failed to decode base64 key")?; - bytes_to_key(key_bytes).context("Invalid key size from base64") + Key::try_from(key_bytes.as_slice()).context("Invalid size for key decoded from base64") } /// Export key as base64 string @@ -68,7 +68,7 @@ impl Key { std::io::stdin() .read_to_end(&mut key_bytes) .context("Failed to read key from stdin")?; - bytes_to_key(key_bytes).context("Invalid key size from stdin") + Key::try_from(key_bytes.as_slice()) } else if let Some(env_var) = key_source.strip_prefix("env:") { // Read from environment variable (base64 encoded, format: env:VARNAME) if env_var.is_empty() { @@ -104,21 +104,19 @@ impl From<[u8; Self::KEY_SIZE]> for Key { } } -// === Private Helper functions === // - -/// Convert a byte vector to a key array, validating the length -fn bytes_to_key(key_bytes: Vec) -> Result { - if key_bytes.len() != Key::KEY_SIZE { - anyhow::bail!( - "Invalid key size: expected {} bytes, got {}", - Key::KEY_SIZE, - key_bytes.len() - ); +/// Try to convert a byte slice of arbitrary length to a Key, validating the length is correct +impl TryFrom<&[u8]> for Key { + type Error = anyhow::Error; + fn try_from(slice: &[u8]) -> Result { + let key_bytes: [u8; Key::KEY_SIZE] = slice.try_into().with_context(|| { + format!( + "Invalid key size: expected {} bytes, got {}", + Key::KEY_SIZE, + slice.len() + ) + })?; + Ok(Key::from(key_bytes)) } - - let mut bytes = [0u8; Key::KEY_SIZE]; - bytes.copy_from_slice(&key_bytes); - Ok(Key::from(bytes)) } // === Tests === // From cd57f4ce23f22a81e2d582b35f45e90e8641ead3 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Fri, 12 Dec 2025 23:22:23 +0100 Subject: [PATCH 05/16] =?UTF-8?q?`from=5Ffile()`=20=E2=9E=A1=EF=B8=8F=20`T?= =?UTF-8?q?ryFrom<&Path>`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/key.rs | 29 ++++++++++++++++------------- src/repo.rs | 2 +- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/key.rs b/src/key.rs index 60026f7..084566f 100644 --- a/src/key.rs +++ b/src/key.rs @@ -18,14 +18,6 @@ impl Key { /// Size of the encryption key in bytes (256 bits for AES-256) pub const KEY_SIZE: usize = 32; - /// Read encryption key from a file (raw binary format, 32 bytes) - pub fn from_file(file_path: &Path) -> Result { - let key_bytes = fs::read(file_path) - .with_context(|| format!("Failed to read key from file: {}", file_path.display()))?; - Key::try_from(key_bytes.as_slice()) - .with_context(|| format!("Invalid key size in file: {}", file_path.display())) - } - /// Generate a new random encryption key /// /// # Errors @@ -119,6 +111,17 @@ impl TryFrom<&[u8]> for Key { } } +/// Read encryption key from a file (raw binary format, 32 bytes) +impl TryFrom<&Path> for Key { + type Error = anyhow::Error; + fn try_from(file_path: &Path) -> Result { + let key_bytes = fs::read(file_path) + .with_context(|| format!("Failed to read key from file: {}", file_path.display()))?; + Key::try_from(key_bytes.as_slice()) + .with_context(|| format!("Invalid key size in file: {}", file_path.display())) + } +} + // === Tests === // #[cfg(test)] @@ -239,13 +242,13 @@ mod tests { let key = test_key(); fs::write(&key_file, key.as_ref()).unwrap(); - let loaded_key = Key::from_file(&key_file).unwrap(); + let loaded_key = Key::try_from(key_file.as_path()).unwrap(); assert_eq!(loaded_key.as_ref(), key.as_ref()); } #[test] fn test_from_file_nonexistent() { - let result = Key::from_file(Path::new("/nonexistent/path/key.bin")); + let result = Key::try_from(Path::new("/nonexistent/path/key.bin")); assert!(result.is_err()); assert!(result .unwrap_err() @@ -261,7 +264,7 @@ mod tests { // Write a file with wrong size fs::write(&key_file, vec![0u8; 16]).unwrap(); - let result = Key::from_file(&key_file); + let result = Key::try_from(key_file.as_path()); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("Invalid key size")); } @@ -273,7 +276,7 @@ mod tests { fs::write(&key_file, vec![]).unwrap(); - let result = Key::from_file(&key_file); + let result = Key::try_from(key_file.as_path()); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("Invalid key size")); } @@ -286,7 +289,7 @@ mod tests { // Write a file with too many bytes fs::write(&key_file, vec![0u8; 64]).unwrap(); - let result = Key::from_file(&key_file); + let result = Key::try_from(key_file.as_path()); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("Invalid key size")); } diff --git a/src/repo.rs b/src/repo.rs index 09cc57e..098f464 100644 --- a/src/repo.rs +++ b/src/repo.rs @@ -181,7 +181,7 @@ impl Repo { /// Load the encryption key from the key file in .git directory pub fn load_key(&self) -> Result { let key_file = self.key_file_path()?; - key::Key::from_file(&key_file).with_context(|| { + key::Key::try_from(key_file.as_path()).with_context(|| { format!( "Encryption key not found at {}. Run '{} unlock' first.", key_file.display(), From ed4dd42a8ec0c46c1d1de4f741f25ee76616cbc0 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Sat, 13 Dec 2025 00:14:41 +0100 Subject: [PATCH 06/16] =?UTF-8?q?`from=5Fbase64()`=20=E2=9E=A1=EF=B8=8F=20?= =?UTF-8?q?`TryFrom<&str>`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/key.rs | 49 ++++++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/key.rs b/src/key.rs index 084566f..bf2a232 100644 --- a/src/key.rs +++ b/src/key.rs @@ -29,17 +29,6 @@ impl Key { Ok(Self::from(bytes)) } - /// Create a key from a base64-encoded string - /// - /// The input string is automatically trimmed of leading and trailing whitespace - /// to handle cases where base64 strings are copied with extra whitespace. - pub fn from_base64(key_b64: &str) -> Result { - let key_bytes = general_purpose::STANDARD - .decode(key_b64.trim()) - .context("Failed to decode base64 key")?; - Key::try_from(key_bytes.as_slice()).context("Invalid size for key decoded from base64") - } - /// Export key as base64 string pub fn to_base64(&self) -> String { general_purpose::STANDARD.encode(self.bytes) @@ -69,10 +58,10 @@ impl Key { let key_b64 = std::env::var(env_var).with_context(|| { format!("Failed to read key from environment variable {}", env_var) })?; - Self::from_base64(&key_b64) + Self::try_from(key_b64.as_str()) } else { // Treat as base64-encoded key (unprefixed) - Self::from_base64(key_source) + Self::try_from(key_source) } } } @@ -122,6 +111,20 @@ impl TryFrom<&Path> for Key { } } +/// Create a key from a base64-encoded string +/// +/// The input string is automatically trimmed of leading and trailing whitespace +/// to handle cases where base64 strings are copied with extra whitespace. +impl TryFrom<&str> for Key { + type Error = anyhow::Error; + fn try_from(key_b64: &str) -> Result { + let key_bytes = general_purpose::STANDARD + .decode(key_b64.trim()) + .context("Failed to decode base64 key")?; + Key::try_from(key_bytes.as_slice()).context("Invalid size for key decoded from base64") + } +} + // === Tests === // #[cfg(test)] @@ -182,7 +185,7 @@ mod tests { let key = test_key(); let b64 = key.to_base64(); - let decoded_key = Key::from_base64(&b64).unwrap(); + let decoded_key = Key::try_from(b64.as_str()).unwrap(); assert_eq!(decoded_key.as_ref(), key.as_ref()); } @@ -190,14 +193,14 @@ mod tests { fn test_base64_roundtrip() { let original_key = test_key(); let b64 = original_key.to_base64(); - let decoded_key = Key::from_base64(&b64).unwrap(); + let decoded_key = Key::try_from(b64.as_str()).unwrap(); assert_eq!(original_key.as_ref(), decoded_key.as_ref()); } #[test] fn test_from_base64_invalid_base64() { - let result = Key::from_base64("not valid base64!!!"); + let result = Key::try_from("not valid base64!!!"); assert!(result.is_err()); assert!(result .unwrap_err() @@ -209,14 +212,14 @@ mod tests { fn test_from_base64_wrong_size() { // Base64 of 16 bytes (too short) let short_b64 = "dGVzdC1rZXktMTYtYnl0ZXM="; // "test-key-16-bytes" - let result = Key::from_base64(short_b64); + let result = Key::try_from(short_b64); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("Invalid key size")); } #[test] fn test_from_base64_empty_string() { - let result = Key::from_base64(""); + let result = Key::try_from(""); assert!(result.is_err()); } @@ -226,11 +229,11 @@ mod tests { let b64 = key.to_base64(); // Test with leading/trailing whitespace (should be automatically trimmed) - let decoded_key = Key::from_base64(&format!(" {} ", b64)).unwrap(); + let decoded_key = Key::try_from(format!(" {} ", b64).as_str()).unwrap(); assert_eq!(decoded_key.as_ref(), key.as_ref()); // Test with newlines and tabs - let decoded_key2 = Key::from_base64(&format!("\n\t{}\n\t", b64)).unwrap(); + let decoded_key2 = Key::try_from(format!("\n\t{}\n\t", b64).as_str()).unwrap(); assert_eq!(decoded_key2.as_ref(), key.as_ref()); } @@ -467,9 +470,9 @@ mod tests { let b64 = key.to_base64(); // Decode multiple times, should get same result - let decoded1 = Key::from_base64(&b64).unwrap(); - let decoded2 = Key::from_base64(&b64).unwrap(); - let decoded3 = Key::from_base64(&b64).unwrap(); + let decoded1 = Key::try_from(b64.as_str()).unwrap(); + let decoded2 = Key::try_from(b64.as_str()).unwrap(); + let decoded3 = Key::try_from(b64.as_str()).unwrap(); assert_eq!(decoded1.as_ref(), key.as_ref()); assert_eq!(decoded2.as_ref(), key.as_ref()); From 43e400b8247d5b9543f9ff285229b276e55080ca Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Sat, 13 Dec 2025 00:25:45 +0100 Subject: [PATCH 07/16] =?UTF-8?q?`to=5Fbase64()`=20=E2=9E=A1=EF=B8=8F=20`D?= =?UTF-8?q?isplay`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/commands/init.rs | 15 +++++++-------- src/commands/key.rs | 11 +++++------ src/key.rs | 33 ++++++++++++++++++--------------- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/commands/init.rs b/src/commands/init.rs index 0f262b4..848a164 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -26,30 +26,29 @@ pub fn cmd_init() -> Result<()> { repo.setup_filters() .context("Failed to set up Git filters")?; - let key_b64 = key.to_base64(); - let instructions = init_instructions(&key_b64); + let instructions = init_instructions(key); println!("{}", instructions); Ok(()) } /// Format initialization instructions for display to the user -fn init_instructions(key_b64: &str) -> String { +fn init_instructions(key: key::Key) -> String { format!( indoc! {r#" Repository initialized for {bin_name} Your encryption key (base64, save this securely!): - {key_b64} + {key} Once you share this key with users you trust, they can unlock their working copy using one of these methods: - From base64-encoded key passed directly as argument: - {bin_name} unlock "{key_b64}" + {bin_name} unlock "{key}" - From environment variable (base64): - export GIT_CONCEAL_SECRET_KEY='{key_b64}' + export GIT_CONCEAL_SECRET_KEY='{key}' {bin_name} unlock env:GIT_CONCEAL_SECRET_KEY - From stdin (raw binary, 32 bytes): - echo '{key_b64}' | base64 -d | {bin_name} unlock - + echo '{key}' | base64 -d | {bin_name} unlock - {bin_name} unlock - < /path/to/raw-binary-key.bin To start adding files to be encrypted in this repository: @@ -63,7 +62,7 @@ fn init_instructions(key_b64: &str) -> String { - Run '{bin_name} status' to validate the list of files that are encrypted. "#}, bin_name = BINARY_NAME, - key_b64 = key_b64, + key = key, filter = repo::FILTER_NAME, diff = repo::DIFF_NAME, ) diff --git a/src/commands/key.rs b/src/commands/key.rs index 530c9e4..82a7eb1 100644 --- a/src/commands/key.rs +++ b/src/commands/key.rs @@ -20,7 +20,7 @@ pub fn cmd_key_show(raw: bool) -> Result<()> { .write_all(key.as_ref()) .context("Failed to write key to stdout")?; } else { - println!("{}", key.to_base64()); + println!("{}", key); } Ok(()) @@ -49,8 +49,7 @@ pub fn cmd_key_rotate(skip_confirmation: bool) -> Result<()> { .context("Failed to re-normalize encrypted files")?; // Print follow-up instructions for the user - let new_key_b64 = new_key.to_base64(); - let instructions = rotate_instructions(&new_key_b64); + let instructions = rotate_instructions(new_key); println!("{}", instructions); Ok(()) @@ -100,14 +99,14 @@ fn rotate_confirmation_prompt() -> String { } /// Format key rotation instructions for display to the user -fn rotate_instructions(key_b64: &str) -> String { +fn rotate_instructions(key: key::Key) -> String { format!( indoc! {r#" Key rotation completed successfully Encrypted file(s) have been re-keyed and staged for commit. New encryption key (base64, save this securely and share with your team!): - {key_b64} + {key} Next steps: 1. Consider also rotating the actual secrets contained in the secret files @@ -125,6 +124,6 @@ fn rotate_instructions(key_b64: &str) -> String { Once all team members have updated to the new key, the old key can be discarded. "#}, bin_name = BINARY_NAME, - key_b64 = key_b64, + key = key, ) } diff --git a/src/key.rs b/src/key.rs index bf2a232..1d29c78 100644 --- a/src/key.rs +++ b/src/key.rs @@ -1,5 +1,6 @@ use anyhow::{Context, Result}; use base64::{engine::general_purpose, Engine as _}; +use std::fmt::Display; use std::fs; use std::io::Read; use std::path::Path; @@ -29,11 +30,6 @@ impl Key { Ok(Self::from(bytes)) } - /// Export key as base64 string - pub fn to_base64(&self) -> String { - general_purpose::STANDARD.encode(self.bytes) - } - /// Read encryption key from various sources /// /// Supports: @@ -125,6 +121,13 @@ impl TryFrom<&str> for Key { } } +/// Export key as base64 string +impl Display for Key { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", general_purpose::STANDARD.encode(self.bytes)) + } +} + // === Tests === // #[cfg(test)] @@ -172,7 +175,7 @@ mod tests { #[test] fn test_to_base64() { let key = test_key(); - let b64 = key.to_base64(); + let b64 = key.to_string(); assert!(!b64.is_empty()); assert!(b64 @@ -183,7 +186,7 @@ mod tests { #[test] fn test_from_base64() { let key = test_key(); - let b64 = key.to_base64(); + let b64 = key.to_string(); let decoded_key = Key::try_from(b64.as_str()).unwrap(); assert_eq!(decoded_key.as_ref(), key.as_ref()); @@ -192,7 +195,7 @@ mod tests { #[test] fn test_base64_roundtrip() { let original_key = test_key(); - let b64 = original_key.to_base64(); + let b64 = original_key.to_string(); let decoded_key = Key::try_from(b64.as_str()).unwrap(); assert_eq!(original_key.as_ref(), decoded_key.as_ref()); @@ -226,7 +229,7 @@ mod tests { #[test] fn test_from_base64_with_whitespace() { let key = test_key(); - let b64 = key.to_base64(); + let b64 = key.to_string(); // Test with leading/trailing whitespace (should be automatically trimmed) let decoded_key = Key::try_from(format!(" {} ", b64).as_str()).unwrap(); @@ -305,7 +308,7 @@ mod tests { #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition fn test_read_from_source_env() { let key = test_key(); - let b64 = key.to_base64(); + let b64 = key.to_string(); // Set environment variable // SAFETY: This test runs serially, so no race conditions with other tests @@ -330,7 +333,7 @@ mod tests { #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition fn test_read_from_source_env_with_whitespace() { let key = test_key(); - let b64 = key.to_base64(); + let b64 = key.to_string(); // Set environment variable with whitespace // SAFETY: This test runs serially, so no race conditions with other tests @@ -381,7 +384,7 @@ mod tests { #[test] fn test_read_from_source_base64() { let key = test_key(); - let b64 = key.to_base64(); + let b64 = key.to_string(); let loaded_key = Key::read_from_source(&b64).unwrap(); assert_eq!(loaded_key.as_ref(), key.as_ref()); } @@ -439,8 +442,8 @@ mod tests { #[test] fn test_base64_encoding_consistency() { let key = test_key(); - let b64_1 = key.to_base64(); - let b64_2 = key.to_base64(); + let b64_1 = key.to_string(); + let b64_2 = key.to_string(); // Same key should produce same base64 encoding assert_eq!(b64_1, b64_2); @@ -467,7 +470,7 @@ mod tests { #[test] fn test_multiple_base64_decodings() { let key = test_key(); - let b64 = key.to_base64(); + let b64 = key.to_string(); // Decode multiple times, should get same result let decoded1 = Key::try_from(b64.as_str()).unwrap(); From a2278a31b892bd65f002fdb27112acbebbee98e9 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Sat, 13 Dec 2025 00:37:26 +0100 Subject: [PATCH 08/16] Move `read_from_source()` from `key` to `commands::unlock` Since this method is strongly tied to reading a key from the right source (stdin, env, param) for the `cmd_unlock` command, and thus strongly tied to the command line parsing, this make more sense to put this in `commands/unlock.rs` rather than as a method on the `key::Key` model object. --- src/commands/unlock.rs | 156 ++++++++++++++++++++++++++++++++++++++++- src/key.rs | 138 +----------------------------------- 2 files changed, 155 insertions(+), 139 deletions(-) diff --git a/src/commands/unlock.rs b/src/commands/unlock.rs index 4bfe9d9..c8bcca8 100644 --- a/src/commands/unlock.rs +++ b/src/commands/unlock.rs @@ -1,6 +1,7 @@ -use crate::key; +use crate::key::Key; use crate::repo; use anyhow::{Context, Result}; +use std::io::Read; pub fn cmd_unlock(key_source: String) -> Result<()> { let repo = repo::Repo::discover()?; @@ -16,7 +17,7 @@ pub fn cmd_unlock(key_source: String) -> Result<()> { anyhow::bail!("Repository has dirty encrypted files"); } - let key = key::Key::read_from_source(&key_source)?; + let key = read_from_source(&key_source)?; // Store key in key file repo.store_key(&key).context("Failed to store key file")?; @@ -32,3 +33,154 @@ pub fn cmd_unlock(key_source: String) -> Result<()> { println!("Repository unlocked successfully"); Ok(()) } + +/// Read encryption key from various sources +/// +/// Supports: +/// - Base64-encoded key string (passed directly as argument) +/// - `"env:VARNAME"` for reading from environment variable (base64 encoded) +/// - `"-"` for reading from stdin (raw binary format, 32 bytes) +/// +/// Returns the encryption key. +fn read_from_source(key_source: &str) -> Result { + if key_source == "-" { + // Read from stdin (raw binary format) + let mut key_bytes = Vec::new(); + std::io::stdin() + .read_to_end(&mut key_bytes) + .context("Failed to read key from stdin")?; + Key::try_from(key_bytes.as_slice()) + } else if let Some(env_var) = key_source.strip_prefix("env:") { + // Read from environment variable (base64 encoded, format: env:VARNAME) + if env_var.is_empty() { + anyhow::bail!("Environment variable name cannot be empty after 'env:'"); + } + let key_b64 = std::env::var(env_var) + .with_context(|| format!("Failed to read key from environment variable {}", env_var))?; + Key::try_from(key_b64.as_str()) + } else { + // Treat as base64-encoded key (unprefixed) + Key::try_from(key_source) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Constant test key bytes for deterministic testing + const TEST_KEY_BYTES: [u8; Key::KEY_SIZE] = [ + 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, + 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, + 0xcd, 0xef, + ]; + + /// Get a constant test key + fn test_key() -> Key { + Key::from(TEST_KEY_BYTES) + } + + /// This test must run serially (not in parallel with other tests) because it modifies + /// environment variables. Environment variable modification is not thread-safe and can + /// cause race conditions when tests run in parallel. + #[serial_test::serial] + #[test] + #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition + fn test_read_from_source_env() { + let key = test_key(); + let b64 = key.to_string(); + + // Set environment variable + // SAFETY: This test runs serially, so no race conditions with other tests + unsafe { + std::env::set_var("TEST_KEY_VAR", &b64); + } + + let loaded_key = read_from_source("env:TEST_KEY_VAR").unwrap(); + assert_eq!(loaded_key.as_ref(), key.as_ref()); + + // Clean up + unsafe { + std::env::remove_var("TEST_KEY_VAR"); + } + } + + /// This test must run serially (not in parallel with other tests) because it modifies + /// environment variables. Environment variable modification is not thread-safe and can + /// cause race conditions when tests run in parallel. + #[serial_test::serial] + #[test] + #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition + fn test_read_from_source_env_with_whitespace() { + let key = test_key(); + let b64 = key.to_string(); + + // Set environment variable with whitespace + // SAFETY: This test runs serially, so no race conditions with other tests + unsafe { + std::env::set_var("TEST_KEY_VAR", &format!(" {} ", b64)); + } + + let loaded_key = read_from_source("env:TEST_KEY_VAR").unwrap(); + assert_eq!(loaded_key.as_ref(), key.as_ref()); + + // Clean up + unsafe { + std::env::remove_var("TEST_KEY_VAR"); + } + } + + /// This test must run serially (not in parallel with other tests) because it modifies + /// environment variables. Environment variable modification is not thread-safe and can + /// cause race conditions when tests run in parallel. + #[serial_test::serial] + #[test] + #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition + fn test_read_from_source_env_nonexistent() { + // Make sure the variable doesn't exist + // SAFETY: This test runs serially, so no race conditions with other tests + unsafe { + std::env::remove_var("NONEXISTENT_VAR"); + } + + let result = read_from_source("env:NONEXISTENT_VAR"); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Failed to read key from environment variable")); + } + + #[test] + fn test_read_from_source_env_empty_name() { + let result = read_from_source("env:"); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Environment variable name cannot be empty")); + } + + #[test] + fn test_read_from_source_base64() { + let key = test_key(); + let b64 = key.to_string(); + let loaded_key = read_from_source(&b64).unwrap(); + assert_eq!(loaded_key.as_ref(), key.as_ref()); + } + + // Note: Testing stdin reading is complex in unit tests as it requires + // mocking stdin or using a separate process. This would be better suited + // for integration tests. The env var and base64 cases are tested above. + + #[test] + fn test_read_from_source_invalid_base64() { + // A user accidentally passing a file path should be treated as base64 key and fail to decode + let result = read_from_source("path/to/secret-symmetric-key.bin"); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Failed to decode base64 key")); + } +} diff --git a/src/key.rs b/src/key.rs index 1d29c78..b22552a 100644 --- a/src/key.rs +++ b/src/key.rs @@ -2,7 +2,6 @@ use anyhow::{Context, Result}; use base64::{engine::general_purpose, Engine as _}; use std::fmt::Display; use std::fs; -use std::io::Read; use std::path::Path; /// Symmetric key for encryption/decryption @@ -29,37 +28,6 @@ impl Key { bytes.copy_from_slice(&bytes_vec); Ok(Self::from(bytes)) } - - /// Read encryption key from various sources - /// - /// Supports: - /// - Base64-encoded key string (passed directly as argument) - /// - `"env:VARNAME"` for reading from environment variable (base64 encoded) - /// - `"-"` for reading from stdin (raw binary format, 32 bytes) - /// - /// Returns the encryption key. - pub fn read_from_source(key_source: &str) -> Result { - if key_source == "-" { - // Read from stdin (raw binary format) - let mut key_bytes = Vec::new(); - std::io::stdin() - .read_to_end(&mut key_bytes) - .context("Failed to read key from stdin")?; - Key::try_from(key_bytes.as_slice()) - } else if let Some(env_var) = key_source.strip_prefix("env:") { - // Read from environment variable (base64 encoded, format: env:VARNAME) - if env_var.is_empty() { - anyhow::bail!("Environment variable name cannot be empty after 'env:'"); - } - let key_b64 = std::env::var(env_var).with_context(|| { - format!("Failed to read key from environment variable {}", env_var) - })?; - Self::try_from(key_b64.as_str()) - } else { - // Treat as base64-encoded key (unprefixed) - Self::try_from(key_source) - } - } } /// Get a reference to the underlying key bytes @@ -217,7 +185,7 @@ mod tests { let short_b64 = "dGVzdC1rZXktMTYtYnl0ZXM="; // "test-key-16-bytes" let result = Key::try_from(short_b64); assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("Invalid key size")); + assert_eq!(result.unwrap_err().to_string(), "Invalid size for key decoded from base64"); } #[test] @@ -300,110 +268,6 @@ mod tests { assert!(result.unwrap_err().to_string().contains("Invalid key size")); } - /// This test must run serially (not in parallel with other tests) because it modifies - /// environment variables. Environment variable modification is not thread-safe and can - /// cause race conditions when tests run in parallel. - #[serial_test::serial] - #[test] - #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition - fn test_read_from_source_env() { - let key = test_key(); - let b64 = key.to_string(); - - // Set environment variable - // SAFETY: This test runs serially, so no race conditions with other tests - unsafe { - std::env::set_var("TEST_KEY_VAR", &b64); - } - - let loaded_key = Key::read_from_source("env:TEST_KEY_VAR").unwrap(); - assert_eq!(loaded_key.as_ref(), key.as_ref()); - - // Clean up - unsafe { - std::env::remove_var("TEST_KEY_VAR"); - } - } - - /// This test must run serially (not in parallel with other tests) because it modifies - /// environment variables. Environment variable modification is not thread-safe and can - /// cause race conditions when tests run in parallel. - #[serial_test::serial] - #[test] - #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition - fn test_read_from_source_env_with_whitespace() { - let key = test_key(); - let b64 = key.to_string(); - - // Set environment variable with whitespace - // SAFETY: This test runs serially, so no race conditions with other tests - unsafe { - std::env::set_var("TEST_KEY_VAR", &format!(" {} ", b64)); - } - - let loaded_key = Key::read_from_source("env:TEST_KEY_VAR").unwrap(); - assert_eq!(loaded_key.as_ref(), key.as_ref()); - - // Clean up - unsafe { - std::env::remove_var("TEST_KEY_VAR"); - } - } - - /// This test must run serially (not in parallel with other tests) because it modifies - /// environment variables. Environment variable modification is not thread-safe and can - /// cause race conditions when tests run in parallel. - #[serial_test::serial] - #[test] - #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition - fn test_read_from_source_env_nonexistent() { - // Make sure the variable doesn't exist - // SAFETY: This test runs serially, so no race conditions with other tests - unsafe { - std::env::remove_var("NONEXISTENT_VAR"); - } - - let result = Key::read_from_source("env:NONEXISTENT_VAR"); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Failed to read key from environment variable")); - } - - #[test] - fn test_read_from_source_env_empty_name() { - let result = Key::read_from_source("env:"); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Environment variable name cannot be empty")); - } - - #[test] - fn test_read_from_source_base64() { - let key = test_key(); - let b64 = key.to_string(); - let loaded_key = Key::read_from_source(&b64).unwrap(); - assert_eq!(loaded_key.as_ref(), key.as_ref()); - } - - // Note: Testing stdin reading is complex in unit tests as it requires - // mocking stdin or using a separate process. This would be better suited - // for integration tests. The env var and base64 cases are tested above. - - #[test] - fn test_read_from_source_invalid_base64() { - // A user accidentally passing a file path should be treated as base64 key and fail to decode - let result = Key::read_from_source("path/to/secret-symmetric-key.bin"); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Failed to decode base64 key")); - } - #[test] fn test_as_ref() { let key = test_key(); From c781bd495517c3fab1da9f0ba023885ca04fa778 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Sat, 13 Dec 2025 01:06:26 +0100 Subject: [PATCH 09/16] =?UTF-8?q?`use=20crate::key`=20=E2=9E=A1=EF=B8=8F?= =?UTF-8?q?=20`use=20crate::key::Key`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To avoid having to refer to it as `key::Key` in the implementations every time --- src/commands/filter.rs | 15 +++++++-------- src/commands/init.rs | 6 +++--- src/commands/key.rs | 6 +++--- src/crypto.rs | 30 +++++++++++++++--------------- src/key.rs | 5 ++++- src/repo.rs | 22 +++++++++++----------- 6 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/commands/filter.rs b/src/commands/filter.rs index 190b77a..97f961d 100644 --- a/src/commands/filter.rs +++ b/src/commands/filter.rs @@ -1,5 +1,5 @@ use crate::crypto; -use crate::key; +use crate::key::Key; use crate::repo; use anyhow::{Context, Result}; use std::fs; @@ -68,7 +68,7 @@ fn read_stdin() -> Result> { /// Apply clean filter logic: encrypt plaintext, pass through encrypted data unchanged /// This is idempotent: clean(clean(data)) == clean(data) -fn apply_clean_filter(key: &key::Key, input: &[u8]) -> Result> { +fn apply_clean_filter(key: &Key, input: &[u8]) -> Result> { // Check if input is already encrypted using magic header if crypto::is_encrypted(input) { // Input is already encrypted, pass through unchanged @@ -82,7 +82,7 @@ fn apply_clean_filter(key: &key::Key, input: &[u8]) -> Result> { /// Apply smudge filter logic: decrypt encrypted data, pass through plaintext unchanged /// This is idempotent: smudge(smudge(data)) == smudge(data) -fn apply_smudge_filter(key: &key::Key, input: &[u8]) -> Result> { +fn apply_smudge_filter(key: &Key, input: &[u8]) -> Result> { // Check if input is encrypted using magic header if !crypto::is_encrypted(input) { // Input is already plaintext, pass through unchanged @@ -99,16 +99,15 @@ fn apply_smudge_filter(key: &key::Key, input: &[u8]) -> Result> { #[cfg(test)] mod tests { use super::*; - use crate::key; /// Constant test key for deterministic testing - fn test_key() -> key::Key { - const TEST_KEY_BYTES: [u8; key::Key::KEY_SIZE] = [ + fn test_key() -> Key { + const TEST_KEY_BYTES: [u8; Key::KEY_SIZE] = [ 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, ]; - key::Key::from(TEST_KEY_BYTES) + Key::from(TEST_KEY_BYTES) } #[test] @@ -263,7 +262,7 @@ mod tests { #[test] fn test_apply_smudge_filter_wrong_key_fails() { let key1 = test_key(); - let key2 = key::Key::generate().unwrap(); + let key2 = Key::generate().unwrap(); let plaintext = b"Secret data"; let encrypted = crypto::encrypt(&key1, plaintext).unwrap(); diff --git a/src/commands/init.rs b/src/commands/init.rs index 848a164..edd5ae8 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -1,4 +1,4 @@ -use crate::key; +use crate::key::Key; use crate::repo; use crate::BINARY_NAME; use anyhow::{Context, Result}; @@ -19,7 +19,7 @@ pub fn cmd_init() -> Result<()> { } // Generate a new key - let key = key::Key::generate().context("Failed to generate encryption key")?; + let key = Key::generate().context("Failed to generate encryption key")?; repo.store_key(&key).context("Failed to store key file")?; // Set up Git filters @@ -33,7 +33,7 @@ pub fn cmd_init() -> Result<()> { } /// Format initialization instructions for display to the user -fn init_instructions(key: key::Key) -> String { +fn init_instructions(key: Key) -> String { format!( indoc! {r#" Repository initialized for {bin_name} diff --git a/src/commands/key.rs b/src/commands/key.rs index 82a7eb1..403a48a 100644 --- a/src/commands/key.rs +++ b/src/commands/key.rs @@ -1,4 +1,4 @@ -use crate::key; +use crate::key::Key; use crate::repo; use crate::BINARY_NAME; use anyhow::{Context, Result}; @@ -39,7 +39,7 @@ pub fn cmd_key_rotate(skip_confirmation: bool) -> Result<()> { anyhow::bail!("Key rotation cancelled."); } - let new_key = key::Key::generate().context("Failed to generate new encryption key")?; + let new_key = Key::generate().context("Failed to generate new encryption key")?; repo.store_key(&new_key) .context("Failed to store new key")?; @@ -99,7 +99,7 @@ fn rotate_confirmation_prompt() -> String { } /// Format key rotation instructions for display to the user -fn rotate_instructions(key: key::Key) -> String { +fn rotate_instructions(key: Key) -> String { format!( indoc! {r#" Key rotation completed successfully diff --git a/src/crypto.rs b/src/crypto.rs index 6286fe7..39f3f3f 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -1,4 +1,4 @@ -use crate::key; +use crate::key::Key; use aes::Aes256; use anyhow::{Context, Result}; use ctr::cipher::{KeyIvInit, StreamCipher}; @@ -55,7 +55,7 @@ pub fn is_encrypted(data: &[u8]) -> bool { /// /// The HMAC is computed over: magic + version + IV + encrypted data /// and is used to verify the decryption key is correct. -pub fn encrypt(key: &key::Key, plaintext: &[u8]) -> Result> { +pub fn encrypt(key: &Key, plaintext: &[u8]) -> Result> { // Derive IV from SHA-256 hash of plaintext (like git-crypt uses HMAC-SHA1) let mut hasher = Sha256::new(); hasher.update(plaintext); @@ -89,7 +89,7 @@ pub fn encrypt(key: &key::Key, plaintext: &[u8]) -> Result> { /// Decrypt data using AES-256-CTR /// Verifies HMAC before decrypting to ensure the correct key is used. -pub fn decrypt(key: &key::Key, ciphertext: &[u8]) -> Result> { +pub fn decrypt(key: &Key, ciphertext: &[u8]) -> Result> { if ciphertext.len() < MIN_ENCRYPTED_SIZE { anyhow::bail!("Ciphertext too short to contain header, IV, and HMAC"); } @@ -149,9 +149,9 @@ pub fn decrypt(key: &key::Key, ciphertext: &[u8]) -> Result> { /// # Errors /// Returns an error if HKDF expansion fails (should never happen with fixed output size, /// but handled for completeness). -fn derive_hmac_key(encryption_key: &key::Key) -> Result<[u8; key::Key::KEY_SIZE]> { +fn derive_hmac_key(encryption_key: &Key) -> Result<[u8; Key::KEY_SIZE]> { let kdf = Hkdf::::new(None, encryption_key.as_ref()); - let mut hmac_key = [0u8; key::Key::KEY_SIZE]; + let mut hmac_key = [0u8; Key::KEY_SIZE]; kdf.expand(b"git-conceal-hmac", &mut hmac_key) // `hkdf::InvalidLength` doesn't implement `std::error::Error` so we can't use `.context` and have to use `.map_err` instead .map_err(|_| anyhow::anyhow!("HKDF expansion failed (output length mismatch)"))?; @@ -165,33 +165,33 @@ mod tests { use super::*; /// Test key for deterministic testing - fn test_key1() -> key::Key { - const TEST_KEY1_BYTES: [u8; key::Key::KEY_SIZE] = [ + fn test_key1() -> Key { + const TEST_KEY1_BYTES: [u8; Key::KEY_SIZE] = [ 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, ]; - key::Key::from(TEST_KEY1_BYTES) + Key::from(TEST_KEY1_BYTES) } /// Second test key for tests requiring different keys - fn test_key2() -> key::Key { - const TEST_KEY2_BYTES: [u8; key::Key::KEY_SIZE] = [ + fn test_key2() -> Key { + const TEST_KEY2_BYTES: [u8; Key::KEY_SIZE] = [ 0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, 0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, 0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, 0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, ]; - key::Key::from(TEST_KEY2_BYTES) + Key::from(TEST_KEY2_BYTES) } #[test] fn test_generate_key_bytes() { - let key1_bytes = generate_key_bytes(key::Key::KEY_SIZE).unwrap(); - let key2_bytes = generate_key_bytes(key::Key::KEY_SIZE).unwrap(); + let key1_bytes = generate_key_bytes(Key::KEY_SIZE).unwrap(); + let key2_bytes = generate_key_bytes(Key::KEY_SIZE).unwrap(); // Keys should be the correct size - assert_eq!(key1_bytes.len(), key::Key::KEY_SIZE); - assert_eq!(key2_bytes.len(), key::Key::KEY_SIZE); + assert_eq!(key1_bytes.len(), Key::KEY_SIZE); + assert_eq!(key2_bytes.len(), Key::KEY_SIZE); // Keys should be different (very unlikely to be the same) assert_ne!(key1_bytes, key2_bytes); diff --git a/src/key.rs b/src/key.rs index b22552a..f32d951 100644 --- a/src/key.rs +++ b/src/key.rs @@ -185,7 +185,10 @@ mod tests { let short_b64 = "dGVzdC1rZXktMTYtYnl0ZXM="; // "test-key-16-bytes" let result = Key::try_from(short_b64); assert!(result.is_err()); - assert_eq!(result.unwrap_err().to_string(), "Invalid size for key decoded from base64"); + assert_eq!( + result.unwrap_err().to_string(), + "Invalid size for key decoded from base64" + ); } #[test] diff --git a/src/repo.rs b/src/repo.rs index 098f464..9ca83e4 100644 --- a/src/repo.rs +++ b/src/repo.rs @@ -1,5 +1,5 @@ use crate::fs_helpers; -use crate::key; +use crate::key::Key; use crate::BINARY_NAME; use anyhow::{Context, Result}; use git2::Repository; @@ -179,9 +179,9 @@ impl Repo { } /// Load the encryption key from the key file in .git directory - pub fn load_key(&self) -> Result { + pub fn load_key(&self) -> Result { let key_file = self.key_file_path()?; - key::Key::try_from(key_file.as_path()).with_context(|| { + Key::try_from(key_file.as_path()).with_context(|| { format!( "Encryption key not found at {}. Run '{} unlock' first.", key_file.display(), @@ -191,7 +191,7 @@ impl Repo { } /// Store the encryption key in a file in the .git directory with secure permissions - pub fn store_key(&self, key: &key::Key) -> Result<()> { + pub fn store_key(&self, key: &Key) -> Result<()> { let key_file = self.key_file_path()?; // Write the key as raw bytes to the file @@ -564,7 +564,7 @@ mod tests { #[test] fn test_store_and_load_key() { let (_temp_dir, repo) = setup_test_repo().unwrap(); - let key = key::Key::generate().unwrap(); + let key = Key::generate().unwrap(); repo.store_key(&key).unwrap(); let loaded_key = repo.load_key().unwrap(); assert_eq!(loaded_key.as_ref(), key.as_ref()); @@ -584,8 +584,8 @@ mod tests { #[test] fn test_store_key_overwrites() { let (_temp_dir, repo) = setup_test_repo().unwrap(); - let key1 = key::Key::generate().unwrap(); - let key2 = key::Key::generate().unwrap(); + let key1 = Key::generate().unwrap(); + let key2 = Key::generate().unwrap(); repo.store_key(&key1).unwrap(); assert_eq!(repo.load_key().unwrap().as_ref(), key1.as_ref()); @@ -601,7 +601,7 @@ mod tests { // Initially should be locked assert!(!repo.is_unlocked().unwrap()); - let key = key::Key::generate().unwrap(); + let key = Key::generate().unwrap(); repo.store_key(&key).unwrap(); // Now should be unlocked @@ -611,7 +611,7 @@ mod tests { #[test] fn test_remove_key() { let (_temp_dir, repo) = setup_test_repo().unwrap(); - let key = key::Key::generate().unwrap(); + let key = Key::generate().unwrap(); // Store key repo.store_key(&key).unwrap(); @@ -870,13 +870,13 @@ mod tests { #[test] fn test_store_key_creates_file() { let (_temp_dir, repo) = setup_test_repo().unwrap(); - let key = key::Key::generate().unwrap(); + let key = Key::generate().unwrap(); repo.store_key(&key).unwrap(); let key_path = repo.key_file_path().unwrap(); assert!(key_path.exists()); - assert_eq!(fs::read(&key_path).unwrap().len(), key::Key::KEY_SIZE); + assert_eq!(fs::read(&key_path).unwrap().len(), Key::KEY_SIZE); } #[test] From 43f6cbc414f2e189edace2a543e783bda37a6f06 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Sat, 13 Dec 2025 02:06:16 +0100 Subject: [PATCH 10/16] Reorder `Key` unit tests For consistency with the order in which the methods and traits are declared in the type implementation and group them by theme --- src/key.rs | 216 ++++++++++++++++++++++++++--------------------------- 1 file changed, 107 insertions(+), 109 deletions(-) diff --git a/src/key.rs b/src/key.rs index f32d951..1a1cd67 100644 --- a/src/key.rs +++ b/src/key.rs @@ -7,7 +7,7 @@ use std::path::Path; /// Symmetric key for encryption/decryption /// /// This type wraps the raw key bytes and provides a safe API for key operations. -/// The underlying representation is only exposed when needed for cryptographic operations. +/// (The underlying raw bytes should only be needed when calling low-level cryptographic operations.) #[must_use] #[derive(Clone, Debug)] pub struct Key { @@ -30,6 +30,8 @@ impl Key { } } +// === Conversion Traits === // + /// Get a reference to the underlying key bytes /// /// This is primarily used for cryptographic operations that need direct access @@ -67,11 +69,11 @@ impl TryFrom<&[u8]> for Key { /// Read encryption key from a file (raw binary format, 32 bytes) impl TryFrom<&Path> for Key { type Error = anyhow::Error; - fn try_from(file_path: &Path) -> Result { - let key_bytes = fs::read(file_path) - .with_context(|| format!("Failed to read key from file: {}", file_path.display()))?; + fn try_from(path: &Path) -> Result { + let key_bytes = fs::read(path) + .with_context(|| format!("Failed to read key from file: {}", path.display()))?; Key::try_from(key_bytes.as_slice()) - .with_context(|| format!("Invalid key size in file: {}", file_path.display())) + .with_context(|| format!("Invalid key size in file: {}", path.display())) } } @@ -121,12 +123,6 @@ mod tests { assert_eq!(Key::KEY_SIZE, 32); } - #[test] - fn test_from_bytes() { - let key = Key::from(TEST_KEY_BYTES); - assert_eq!(key.as_ref(), &TEST_KEY_BYTES); - } - #[test] fn test_generate() { let key1 = Key::generate().unwrap(); @@ -141,78 +137,44 @@ mod tests { } #[test] - fn test_to_base64() { - let key = test_key(); - let b64 = key.to_string(); + fn test_multiple_generate_keys_unique() { + let mut keys = Vec::new(); + for _ in 0..10 { + keys.push(Key::generate().unwrap()); + } - assert!(!b64.is_empty()); - assert!(b64 - .chars() - .all(|c| c.is_alphanumeric() || c == '+' || c == '/' || c == '=')); + // All keys should be unique + for i in 0..keys.len() { + for j in (i + 1)..keys.len() { + assert_ne!(keys[i].as_ref(), keys[j].as_ref()); + } + } } #[test] - fn test_from_base64() { + fn test_as_ref() { let key = test_key(); - let b64 = key.to_string(); - - let decoded_key = Key::try_from(b64.as_str()).unwrap(); - assert_eq!(decoded_key.as_ref(), key.as_ref()); - } - - #[test] - fn test_base64_roundtrip() { - let original_key = test_key(); - let b64 = original_key.to_string(); - let decoded_key = Key::try_from(b64.as_str()).unwrap(); - - assert_eq!(original_key.as_ref(), decoded_key.as_ref()); - } - - #[test] - fn test_from_base64_invalid_base64() { - let result = Key::try_from("not valid base64!!!"); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Failed to decode base64 key")); - } + let bytes = key.as_ref(); - #[test] - fn test_from_base64_wrong_size() { - // Base64 of 16 bytes (too short) - let short_b64 = "dGVzdC1rZXktMTYtYnl0ZXM="; // "test-key-16-bytes" - let result = Key::try_from(short_b64); - assert!(result.is_err()); - assert_eq!( - result.unwrap_err().to_string(), - "Invalid size for key decoded from base64" - ); + assert_eq!(bytes.len(), Key::KEY_SIZE); + assert_eq!(bytes, &TEST_KEY_BYTES); } #[test] - fn test_from_base64_empty_string() { - let result = Key::try_from(""); - assert!(result.is_err()); + fn test_from_bytes() { + let key = Key::from(TEST_KEY_BYTES); + assert_eq!(key.as_ref(), &TEST_KEY_BYTES); } #[test] - fn test_from_base64_with_whitespace() { - let key = test_key(); - let b64 = key.to_string(); - - // Test with leading/trailing whitespace (should be automatically trimmed) - let decoded_key = Key::try_from(format!(" {} ", b64).as_str()).unwrap(); - assert_eq!(decoded_key.as_ref(), key.as_ref()); - - // Test with newlines and tabs - let decoded_key2 = Key::try_from(format!("\n\t{}\n\t", b64).as_str()).unwrap(); - assert_eq!(decoded_key2.as_ref(), key.as_ref()); + fn test_try_from_unsized_bytes() { + let unsized_bytes: &[u8] = &TEST_KEY_BYTES; + let key = Key::try_from(unsized_bytes).unwrap(); + assert_eq!(key.as_ref(), &TEST_KEY_BYTES); } #[test] - fn test_from_file() { + fn test_try_from_path() { let temp_dir = TempDir::new().unwrap(); let key_file = temp_dir.path().join("test.key"); @@ -224,7 +186,7 @@ mod tests { } #[test] - fn test_from_file_nonexistent() { + fn test_try_from_path_nonexistent() { let result = Key::try_from(Path::new("/nonexistent/path/key.bin")); assert!(result.is_err()); assert!(result @@ -234,7 +196,7 @@ mod tests { } #[test] - fn test_from_file_wrong_size() { + fn test_try_from_path_wrong_size() { let temp_dir = TempDir::new().unwrap(); let key_file = temp_dir.path().join("test.key"); @@ -247,7 +209,7 @@ mod tests { } #[test] - fn test_from_file_empty_file() { + fn test_try_from_path_empty_file() { let temp_dir = TempDir::new().unwrap(); let key_file = temp_dir.path().join("test.key"); @@ -259,7 +221,7 @@ mod tests { } #[test] - fn test_from_file_too_large() { + fn test_try_from_path_too_large() { let temp_dir = TempDir::new().unwrap(); let key_file = temp_dir.path().join("test.key"); @@ -272,66 +234,84 @@ mod tests { } #[test] - fn test_as_ref() { + fn test_try_from_string_base64() { let key = test_key(); - let bytes = key.as_ref(); + let b64 = key.to_string(); - assert_eq!(bytes.len(), Key::KEY_SIZE); - assert_eq!(bytes, &TEST_KEY_BYTES); + let decoded_key = Key::try_from(b64.as_str()).unwrap(); + assert_eq!(decoded_key.as_ref(), key.as_ref()); } #[test] - fn test_clone() { - let key1 = test_key(); - let key2 = key1.clone(); + fn test_try_from_string_base64_invalid_base64() { + let result = Key::try_from("not valid base64!!!"); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Failed to decode base64 key")); + } - assert_eq!(key1.as_ref(), key2.as_ref()); - // Verify they are independent (modify one, other unchanged) - // Since Key doesn't have interior mutability, we can't easily test this, - // but clone should work correctly + #[test] + fn test_try_from_string_base64_wrong_size() { + // Base64 of 16 bytes (too short) + let short_b64 = "dGVzdC1rZXktMTYtYnl0ZXM="; // "test-key-16-bytes" + let result = Key::try_from(short_b64); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "Invalid size for key decoded from base64" + ); } #[test] - fn test_multiple_generate_keys_unique() { - let mut keys = Vec::new(); - for _ in 0..10 { - keys.push(Key::generate().unwrap()); - } + fn test_try_from_string_base64_empty_string() { + let result = Key::try_from(""); + assert!(result.is_err()); + } - // All keys should be unique - for i in 0..keys.len() { - for j in (i + 1)..keys.len() { - assert_ne!(keys[i].as_ref(), keys[j].as_ref()); - } - } + #[test] + fn test_try_from_string_base64_with_whitespace() { + let key = test_key(); + let b64 = key.to_string(); + + // Test with leading/trailing whitespace (should be automatically trimmed) + let decoded_key = Key::try_from(format!(" {} ", b64).as_str()).unwrap(); + assert_eq!(decoded_key.as_ref(), key.as_ref()); + + // Test with newlines and tabs + let decoded_key2 = Key::try_from(format!("\n\t{}\n\t", b64).as_str()).unwrap(); + assert_eq!(decoded_key2.as_ref(), key.as_ref()); } #[test] - fn test_base64_encoding_consistency() { + fn test_to_string_base64() { let key = test_key(); - let b64_1 = key.to_string(); - let b64_2 = key.to_string(); + let b64 = key.to_string(); - // Same key should produce same base64 encoding - assert_eq!(b64_1, b64_2); + assert!(!b64.is_empty()); + assert!(b64 + .chars() + .all(|c| c.is_alphanumeric() || c == '+' || c == '/' || c == '=')); } #[test] - fn test_key_equality_via_bytes() { - let key1 = Key::from(TEST_KEY_BYTES); - let key2 = Key::from(TEST_KEY_BYTES); + fn test_to_string_base64_encoding_consistency() { + let key = test_key(); + let b64_1 = key.to_string(); + let b64_2 = key.to_string(); - assert_eq!(key1.as_ref(), key2.as_ref()); + // Same key should produce same base64 encoding + assert_eq!(b64_1, b64_2); } #[test] - fn test_key_inequality() { - let key1 = test_key(); - let mut different_bytes = TEST_KEY_BYTES; - different_bytes[0] ^= 0xFF; // Flip first byte - let key2 = Key::from(different_bytes); + fn test_base64_roundtrip() { + let original_key = test_key(); + let b64 = original_key.to_string(); + let decoded_key = Key::try_from(b64.as_str()).unwrap(); - assert_ne!(key1.as_ref(), key2.as_ref()); + assert_eq!(original_key.as_ref(), decoded_key.as_ref()); } #[test] @@ -348,4 +328,22 @@ mod tests { assert_eq!(decoded2.as_ref(), key.as_ref()); assert_eq!(decoded3.as_ref(), key.as_ref()); } + + #[test] + fn test_key_equality_via_bytes() { + let key1 = Key::from(TEST_KEY_BYTES); + let key2 = Key::from(TEST_KEY_BYTES); + + assert_eq!(key1.as_ref(), key2.as_ref()); + } + + #[test] + fn test_key_inequality() { + let key1 = test_key(); + let mut different_bytes = TEST_KEY_BYTES; + different_bytes[0] ^= 0xFF; // Flip first byte + let key2 = Key::from(different_bytes); + + assert_ne!(key1.as_ref(), key2.as_ref()); + } } From 52c583afe11161269190497bccd2ca1fb79f4b3d Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Sat, 13 Dec 2025 16:19:07 +0100 Subject: [PATCH 11/16] =?UTF-8?q?`Repo::from=5Frepository`=20=E2=9E=A1?= =?UTF-8?q?=EF=B8=8F=20`TryFrom`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/repo.rs | 75 ++++++++++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/src/repo.rs b/src/repo.rs index 9ca83e4..78fb73c 100644 --- a/src/repo.rs +++ b/src/repo.rs @@ -40,41 +40,7 @@ impl Repo { pub fn discover() -> Result { let start_path = std::env::current_dir().context("Failed to get current directory")?; let git_repo = Repository::discover(start_path).context("Not a git repository")?; - Self::from_repository(git_repo) - } - - /// Create a Repo instance from a git2 Repository - /// - /// Validates that the repository is not bare and has a working directory. - /// - /// # Errors - /// Returns an error if the repository is bare or has no working directory. - fn from_repository(git_repo: Repository) -> Result { - // Reject bare repositories - this tool needs a working directory - if git_repo.is_bare() { - anyhow::bail!( - "Bare repositories are not supported. \ - This tool encrypts/decrypts files in the working directory, \ - which bare repositories don't have. \ - Please use a non-bare repository with a checked-out working tree." - ); - } - - // Get the working directory root - let workdir = git_repo - .workdir() - .ok_or_else(|| { - anyhow::anyhow!( - "Repository has no working directory. \ - This tool requires a non-bare repository with a checked-out working tree." - ) - })? - .to_path_buf(); - - Ok(Self { - workdir, - git_repo: Rc::new(git_repo), - }) + Self::try_from(git_repo) } /// Get the repository working directory path @@ -458,6 +424,43 @@ impl Repo { } } +/// Create a Repo instance from a git2 Repository +/// +/// Validates that the repository is not bare and has a working directory. +/// +/// # Errors +/// Returns an error if the repository is bare or has no working directory. +impl TryFrom for Repo { + type Error = anyhow::Error; + + fn try_from(git_repo: Repository) -> Result { + // Reject bare repositories - this tool needs a working directory + if git_repo.is_bare() { + anyhow::bail!( + "Bare repositories are not supported. \ + This tool encrypts/decrypts files in the working directory, \ + which bare repositories don't have. \ + Please use a non-bare repository with a checked-out working tree." + ); + } + + // Get the working directory root + let workdir = git_repo + .workdir() + .ok_or_else(|| { + anyhow::anyhow!( + "Repository has no working directory. \ + This tool requires a non-bare repository with a checked-out working tree." + ) + })? + .to_path_buf(); + + Ok(Self { + workdir, + git_repo: Rc::new(git_repo), + }) + } +} // === Helper types and functions === // /// Iterator over filtered files in the git index @@ -540,7 +543,7 @@ mod tests { // Create Repo instance from the repository let repository = Repository::open(repo_path).context("Failed to open git repository")?; - let repo = Repo::from_repository(repository)?; + let repo = Repo::try_from(repository)?; Ok((temp_dir, repo)) } From b7873ba2b4b7b0fd14365d4ea7ec51f655bcf729 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Sun, 14 Dec 2025 17:01:26 +0100 Subject: [PATCH 12/16] =?UTF-8?q?`use=20crate::repo`=20=E2=9E=A1=EF=B8=8F?= =?UTF-8?q?=20`use=20crate::repo::Repo`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To avoid having to refer to it as `repo::Repo` in the implementations every time --- src/commands/filter.rs | 8 ++++---- src/commands/status.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/commands/filter.rs b/src/commands/filter.rs index 97f961d..1c30210 100644 --- a/src/commands/filter.rs +++ b/src/commands/filter.rs @@ -1,6 +1,6 @@ use crate::crypto; use crate::key::Key; -use crate::repo; +use crate::repo::Repo; use anyhow::{Context, Result}; use std::fs; use std::io::{self, Read, Write}; @@ -8,7 +8,7 @@ use std::io::{self, Read, Write}; /// Git clean filter: encrypt data from stdin and write to stdout /// This filter is idempotent: clean(clean(data)) == clean(data) /// If the input is already encrypted (has magic header), it passes through unchanged. -pub fn clean_filter(repo: &repo::Repo) -> Result<()> { +pub fn clean_filter(repo: &Repo) -> Result<()> { let key = repo.load_key().context("Failed to load encryption key")?; let input = read_stdin()?; let output = apply_clean_filter(&key, &input)?; @@ -21,7 +21,7 @@ pub fn clean_filter(repo: &repo::Repo) -> Result<()> { /// Git smudge filter: decrypt data from stdin and write to stdout /// This filter is idempotent: smudge(smudge(data)) == smudge(data) /// If the input is already plaintext (no magic header), it passes through unchanged. -pub fn smudge_filter(repo: &repo::Repo) -> Result<()> { +pub fn smudge_filter(repo: &Repo) -> Result<()> { let key = repo.load_key().context("Failed to load encryption key")?; let input = read_stdin()?; let output = apply_smudge_filter(&key, &input)?; @@ -34,7 +34,7 @@ pub fn smudge_filter(repo: &repo::Repo) -> Result<()> { /// Git diff textconv: decrypt file and write to stdout /// Used by git diff to show decrypted content of encrypted files. /// Takes a filename as argument (provided by git when using textconv). -pub fn diff_textconv(repo: &repo::Repo, filename: &str) -> Result<()> { +pub fn diff_textconv(repo: &Repo, filename: &str) -> Result<()> { let key = repo.load_key().context("Failed to load encryption key")?; // Read file diff --git a/src/commands/status.rs b/src/commands/status.rs index 08a6dcd..0433e0c 100644 --- a/src/commands/status.rs +++ b/src/commands/status.rs @@ -1,4 +1,4 @@ -use crate::repo; +use crate::repo::Repo; use anyhow::{Context, Result}; use serde::Serialize; use serde_json; @@ -6,7 +6,7 @@ use std::fmt; use std::path::PathBuf; pub fn cmd_status(files: Vec, json: bool) -> Result<()> { - let repo = repo::Repo::discover()?; + let repo = Repo::discover()?; if files.is_empty() { // Show repository status From f1c98462ab82eaa57851e51e17a2bd0f23ecf9fa Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Sun, 14 Dec 2025 17:03:38 +0100 Subject: [PATCH 13/16] Borrow parameters when more appropriate e.g. most functions that do something with a `Key` need only to borrow the `Key` (e.g. to print its value) without taking ownership of it. Same for functions that take should take `&str` instead of `String` to only borrow it --- src/commands/init.rs | 4 ++-- src/commands/key.rs | 4 ++-- src/commands/status.rs | 4 ++-- src/commands/unlock.rs | 4 ++-- src/main.rs | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/commands/init.rs b/src/commands/init.rs index edd5ae8..7f12186 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -26,14 +26,14 @@ pub fn cmd_init() -> Result<()> { repo.setup_filters() .context("Failed to set up Git filters")?; - let instructions = init_instructions(key); + let instructions = init_instructions(&key); println!("{}", instructions); Ok(()) } /// Format initialization instructions for display to the user -fn init_instructions(key: Key) -> String { +fn init_instructions(key: &Key) -> String { format!( indoc! {r#" Repository initialized for {bin_name} diff --git a/src/commands/key.rs b/src/commands/key.rs index 403a48a..f8bea8b 100644 --- a/src/commands/key.rs +++ b/src/commands/key.rs @@ -49,7 +49,7 @@ pub fn cmd_key_rotate(skip_confirmation: bool) -> Result<()> { .context("Failed to re-normalize encrypted files")?; // Print follow-up instructions for the user - let instructions = rotate_instructions(new_key); + let instructions = rotate_instructions(&new_key); println!("{}", instructions); Ok(()) @@ -99,7 +99,7 @@ fn rotate_confirmation_prompt() -> String { } /// Format key rotation instructions for display to the user -fn rotate_instructions(key: Key) -> String { +fn rotate_instructions(key: &Key) -> String { format!( indoc! {r#" Key rotation completed successfully diff --git a/src/commands/status.rs b/src/commands/status.rs index 0433e0c..dcad9f8 100644 --- a/src/commands/status.rs +++ b/src/commands/status.rs @@ -5,7 +5,7 @@ use serde_json; use std::fmt; use std::path::PathBuf; -pub fn cmd_status(files: Vec, json: bool) -> Result<()> { +pub fn cmd_status>(files: &[S], json: bool) -> Result<()> { let repo = Repo::discover()?; if files.is_empty() { @@ -40,7 +40,7 @@ pub fn cmd_status(files: Vec, json: bool) -> Result<()> { let file_statuses: Vec = files .iter() .map(|file_str| { - let file_path = std::path::Path::new(file_str); + let file_path = std::path::Path::new(file_str.as_ref()); let is_filtered = repo.is_filtered_file(file_path)?; Ok(FileStatus { file: file_path.to_path_buf(), diff --git a/src/commands/unlock.rs b/src/commands/unlock.rs index c8bcca8..76f2704 100644 --- a/src/commands/unlock.rs +++ b/src/commands/unlock.rs @@ -3,7 +3,7 @@ use crate::repo; use anyhow::{Context, Result}; use std::io::Read; -pub fn cmd_unlock(key_source: String) -> Result<()> { +pub fn cmd_unlock(key_source: &str) -> Result<()> { let repo = repo::Repo::discover()?; // Check if any filtered files have local modifications @@ -17,7 +17,7 @@ pub fn cmd_unlock(key_source: String) -> Result<()> { anyhow::bail!("Repository has dirty encrypted files"); } - let key = read_from_source(&key_source)?; + let key = read_from_source(key_source)?; // Store key in key file repo.store_key(&key).context("Failed to store key file")?; diff --git a/src/main.rs b/src/main.rs index e944ba8..110d2df 100644 --- a/src/main.rs +++ b/src/main.rs @@ -143,9 +143,9 @@ fn main() -> Result<()> { match cli.command { Commands::Init => commands::init::cmd_init(), - Commands::Unlock { key_source } => commands::unlock::cmd_unlock(key_source), + Commands::Unlock { key_source } => commands::unlock::cmd_unlock(&key_source), Commands::Lock { force } => commands::lock::cmd_lock(force), - Commands::Status { files, json } => commands::status::cmd_status(files, json), + Commands::Status { files, json } => commands::status::cmd_status(&files, json), Commands::Key { key_cmd } => match key_cmd { KeyCommands::Show { raw } => commands::key::cmd_key_show(raw), KeyCommands::Rotate { skip_confirmation } => { From 88bf3f9ce983848d9154a8b68813e4da760e79bd Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Mon, 15 Dec 2025 02:08:45 +0100 Subject: [PATCH 14/16] Make `Key` implement `PartialEq, Eq` To make tests easier to write, and because it is idiomatic and makes semantic sense for such a wrapper type anyway --- src/key.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/key.rs b/src/key.rs index 1a1cd67..ddc9369 100644 --- a/src/key.rs +++ b/src/key.rs @@ -9,7 +9,7 @@ use std::path::Path; /// This type wraps the raw key bytes and provides a safe API for key operations. /// (The underlying raw bytes should only be needed when calling low-level cryptographic operations.) #[must_use] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct Key { bytes: [u8; Self::KEY_SIZE], } @@ -182,7 +182,7 @@ mod tests { fs::write(&key_file, key.as_ref()).unwrap(); let loaded_key = Key::try_from(key_file.as_path()).unwrap(); - assert_eq!(loaded_key.as_ref(), key.as_ref()); + assert_eq!(loaded_key, key); } #[test] @@ -239,7 +239,7 @@ mod tests { let b64 = key.to_string(); let decoded_key = Key::try_from(b64.as_str()).unwrap(); - assert_eq!(decoded_key.as_ref(), key.as_ref()); + assert_eq!(decoded_key, key); } #[test] @@ -277,11 +277,11 @@ mod tests { // Test with leading/trailing whitespace (should be automatically trimmed) let decoded_key = Key::try_from(format!(" {} ", b64).as_str()).unwrap(); - assert_eq!(decoded_key.as_ref(), key.as_ref()); + assert_eq!(decoded_key, key); // Test with newlines and tabs let decoded_key2 = Key::try_from(format!("\n\t{}\n\t", b64).as_str()).unwrap(); - assert_eq!(decoded_key2.as_ref(), key.as_ref()); + assert_eq!(decoded_key2, key); } #[test] @@ -311,7 +311,7 @@ mod tests { let b64 = original_key.to_string(); let decoded_key = Key::try_from(b64.as_str()).unwrap(); - assert_eq!(original_key.as_ref(), decoded_key.as_ref()); + assert_eq!(original_key, decoded_key); } #[test] @@ -324,9 +324,9 @@ mod tests { let decoded2 = Key::try_from(b64.as_str()).unwrap(); let decoded3 = Key::try_from(b64.as_str()).unwrap(); - assert_eq!(decoded1.as_ref(), key.as_ref()); - assert_eq!(decoded2.as_ref(), key.as_ref()); - assert_eq!(decoded3.as_ref(), key.as_ref()); + assert_eq!(decoded1, key); + assert_eq!(decoded2, key); + assert_eq!(decoded3, key); } #[test] @@ -334,6 +334,7 @@ mod tests { let key1 = Key::from(TEST_KEY_BYTES); let key2 = Key::from(TEST_KEY_BYTES); + assert_eq!(key1, key2); assert_eq!(key1.as_ref(), key2.as_ref()); } @@ -344,6 +345,7 @@ mod tests { different_bytes[0] ^= 0xFF; // Flip first byte let key2 = Key::from(different_bytes); + assert_ne!(key1, key2); assert_ne!(key1.as_ref(), key2.as_ref()); } } From f406ef14056dbc9ffb8a548587d5015b52242aa8 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Mon, 15 Dec 2025 02:03:53 +0100 Subject: [PATCH 15/16] Introduce `KeySource` for `cmd_unlock` parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Allows the `#[command] Unlock`'s `#[arg] key_source: KeySource` to be more explicit in its type - Uses `impl std::str::FromStr for KeySource` to parse `"-"`/`"env:…"`/`"…"` strings into the right `KeySource` variant (which is more semantically correct than using `From<&str>` here because this is semantically more about parsing/interpreting an argument than about conversion) - Use `impl From for Key` to build/load a `Key` from a `KeySource` --- src/commands/unlock.rs | 166 +++++++++++++++++++++++++++++------------ src/main.rs | 4 +- 2 files changed, 120 insertions(+), 50 deletions(-) diff --git a/src/commands/unlock.rs b/src/commands/unlock.rs index 76f2704..8825284 100644 --- a/src/commands/unlock.rs +++ b/src/commands/unlock.rs @@ -3,7 +3,7 @@ use crate::repo; use anyhow::{Context, Result}; use std::io::Read; -pub fn cmd_unlock(key_source: &str) -> Result<()> { +pub fn cmd_unlock(key_source: KeySource) -> Result<()> { let repo = repo::Repo::discover()?; // Check if any filtered files have local modifications @@ -17,7 +17,7 @@ pub fn cmd_unlock(key_source: &str) -> Result<()> { anyhow::bail!("Repository has dirty encrypted files"); } - let key = read_from_source(key_source)?; + let key = Key::try_from(key_source)?; // Store key in key file repo.store_key(&key).context("Failed to store key file")?; @@ -34,33 +34,57 @@ pub fn cmd_unlock(key_source: &str) -> Result<()> { Ok(()) } -/// Read encryption key from various sources -/// -/// Supports: -/// - Base64-encoded key string (passed directly as argument) -/// - `"env:VARNAME"` for reading from environment variable (base64 encoded) -/// - `"-"` for reading from stdin (raw binary format, 32 bytes) -/// -/// Returns the encryption key. -fn read_from_source(key_source: &str) -> Result { - if key_source == "-" { - // Read from stdin (raw binary format) - let mut key_bytes = Vec::new(); - std::io::stdin() - .read_to_end(&mut key_bytes) - .context("Failed to read key from stdin")?; - Key::try_from(key_bytes.as_slice()) - } else if let Some(env_var) = key_source.strip_prefix("env:") { - // Read from environment variable (base64 encoded, format: env:VARNAME) - if env_var.is_empty() { - anyhow::bail!("Environment variable name cannot be empty after 'env:'"); +/// Possible sources to read the encryption key from +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum KeySource { + /// Base64-encoded key string passed directly as argument + ArgumentValue(String), + /// Environment variable containing the base64-encoded key + EnvVar(String), + /// Raw binary key read from stdin + Stdin, +} + +/// Convert the argument passed to the `unlock` command into a `KeySource`: +impl std::str::FromStr for KeySource { + type Err = anyhow::Error; + fn from_str(arg_value: &str) -> Result { + if arg_value == "-" { + Ok(Self::Stdin) + } else if let Some(env_var) = arg_value.strip_prefix("env:") { + if env_var.is_empty() { + anyhow::bail!("Environment variable name cannot be empty after 'env:'"); + } + Ok(Self::EnvVar(env_var.to_string())) + } else { + Ok(Self::ArgumentValue(arg_value.to_string())) + } + } +} + +/// Read encryption key from various possible sources +impl TryFrom for Key { + type Error = anyhow::Error; + fn try_from(key_source: KeySource) -> Result { + match key_source { + KeySource::ArgumentValue(arg_value) => Key::try_from(arg_value.as_str()), + KeySource::EnvVar(env_var) => { + let key_b64 = std::env::var(env_var.as_str()).with_context(|| { + format!( + "Failed to read key from environment variable {}", + env_var.as_str() + ) + })?; + Key::try_from(key_b64.as_str()) + } + KeySource::Stdin => { + let mut key_bytes = Vec::new(); + std::io::stdin() + .read_to_end(&mut key_bytes) + .context("Failed to read key from stdin")?; + Key::try_from(key_bytes.as_slice()) + } } - let key_b64 = std::env::var(env_var) - .with_context(|| format!("Failed to read key from environment variable {}", env_var))?; - Key::try_from(key_b64.as_str()) - } else { - // Treat as base64-encoded key (unprefixed) - Key::try_from(key_source) } } @@ -83,10 +107,10 @@ mod tests { /// This test must run serially (not in parallel with other tests) because it modifies /// environment variables. Environment variable modification is not thread-safe and can /// cause race conditions when tests run in parallel. - #[serial_test::serial] #[test] + #[serial_test::serial] #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition - fn test_read_from_source_env() { + fn test_key_source_from_env() { let key = test_key(); let b64 = key.to_string(); @@ -96,7 +120,10 @@ mod tests { std::env::set_var("TEST_KEY_VAR", &b64); } - let loaded_key = read_from_source("env:TEST_KEY_VAR").unwrap(); + let key_source: KeySource = "env:TEST_KEY_VAR".parse().unwrap(); + assert_eq!(key_source, KeySource::EnvVar("TEST_KEY_VAR".to_string())); + + let loaded_key = Key::try_from(key_source).unwrap(); assert_eq!(loaded_key.as_ref(), key.as_ref()); // Clean up @@ -108,10 +135,10 @@ mod tests { /// This test must run serially (not in parallel with other tests) because it modifies /// environment variables. Environment variable modification is not thread-safe and can /// cause race conditions when tests run in parallel. - #[serial_test::serial] #[test] + #[serial_test::serial] #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition - fn test_read_from_source_env_with_whitespace() { + fn test_key_source_from_env_with_whitespace() { let key = test_key(); let b64 = key.to_string(); @@ -121,7 +148,10 @@ mod tests { std::env::set_var("TEST_KEY_VAR", &format!(" {} ", b64)); } - let loaded_key = read_from_source("env:TEST_KEY_VAR").unwrap(); + let key_source: KeySource = "env:TEST_KEY_VAR".parse().unwrap(); + assert_eq!(key_source, KeySource::EnvVar("TEST_KEY_VAR".to_string())); + + let loaded_key = Key::try_from(key_source).unwrap(); assert_eq!(loaded_key.as_ref(), key.as_ref()); // Clean up @@ -133,17 +163,20 @@ mod tests { /// This test must run serially (not in parallel with other tests) because it modifies /// environment variables. Environment variable modification is not thread-safe and can /// cause race conditions when tests run in parallel. - #[serial_test::serial] #[test] + #[serial_test::serial] #[allow(unsafe_code)] // Required for std::env::set_var/remove_var in Rust 2024 Edition - fn test_read_from_source_env_nonexistent() { + fn test_key_source_from_env_nonexistent() { // Make sure the variable doesn't exist // SAFETY: This test runs serially, so no race conditions with other tests unsafe { std::env::remove_var("NONEXISTENT_VAR"); } - let result = read_from_source("env:NONEXISTENT_VAR"); + let key_source: KeySource = "env:NONEXISTENT_VAR".parse().unwrap(); + assert_eq!(key_source, KeySource::EnvVar("NONEXISTENT_VAR".to_string())); + + let result = Key::try_from(key_source); assert!(result.is_err()); assert!(result .unwrap_err() @@ -152,8 +185,9 @@ mod tests { } #[test] - fn test_read_from_source_env_empty_name() { - let result = read_from_source("env:"); + fn test_key_source_from_env_empty_name() { + let result: Result = "env:".parse(); + assert!(result.is_err()); assert!(result .unwrap_err() @@ -162,25 +196,61 @@ mod tests { } #[test] - fn test_read_from_source_base64() { + fn test_key_source_from_base64_argument() { let key = test_key(); let b64 = key.to_string(); - let loaded_key = read_from_source(&b64).unwrap(); + let key_source: KeySource = b64.parse().unwrap(); + assert_eq!(key_source, KeySource::ArgumentValue(b64)); + + let loaded_key = Key::try_from(key_source).unwrap(); assert_eq!(loaded_key.as_ref(), key.as_ref()); } - // Note: Testing stdin reading is complex in unit tests as it requires - // mocking stdin or using a separate process. This would be better suited - // for integration tests. The env var and base64 cases are tested above. - #[test] - fn test_read_from_source_invalid_base64() { - // A user accidentally passing a file path should be treated as base64 key and fail to decode - let result = read_from_source("path/to/secret-symmetric-key.bin"); + fn test_key_source_from_invalid_base64_argument() { + // A user accidentally passing a file path as a base64 key + // This path happens to not be valid base64 (due to the `.` and `-` characters in it) + let path_looking_string = "path/to/secret-symmetric-key.bin"; + let key_source: KeySource = path_looking_string.parse().unwrap(); + assert_eq!( + key_source, + KeySource::ArgumentValue(path_looking_string.to_string()) + ); + + let result = Key::try_from(key_source); assert!(result.is_err()); assert!(result .unwrap_err() .to_string() .contains("Failed to decode base64 key")); } + + #[test] + fn test_key_source_from_base64_argument_invalid_key_length() { + // A user accidentally passing a file path as a base64 key + // This path happens to be valid base64, but the wrong length + let path_looking_string = "path/to/the/secret/symmetric/key"; + let key_source: KeySource = path_looking_string.parse().unwrap(); + assert_eq!( + key_source, + KeySource::ArgumentValue(path_looking_string.to_string()) + ); + + let result = Key::try_from(key_source); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Invalid size for key decoded from base64")); + } + + #[test] + fn test_key_source_from_stdin_argument() { + let key_source: KeySource = "-".parse().unwrap(); + assert_eq!(key_source, KeySource::Stdin); + + // Note: Testing stdin reading is complex in unit tests as it requires mocking stdin or using a separate process. + // Hence why we're only testing the parsing of "-" as KeySource::Stdin but not the Key::try_from(KeySource::Stdin) itself. + // (If we really wanted to test this case of reading from stdin itself, an integration test would be better suited.) + } } diff --git a/src/main.rs b/src/main.rs index 110d2df..7cc8baf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -55,7 +55,7 @@ enum Commands { - 'env:VARNAME': read the base64-encoded key from the given environment variable (recommended on CI)\n\ - '-': read the raw binary key from stdin (expects raw binary, 32 bytes as input)" )] - key_source: String, + key_source: commands::unlock::KeySource, }, // Lock #[command( @@ -143,7 +143,7 @@ fn main() -> Result<()> { match cli.command { Commands::Init => commands::init::cmd_init(), - Commands::Unlock { key_source } => commands::unlock::cmd_unlock(&key_source), + Commands::Unlock { key_source } => commands::unlock::cmd_unlock(key_source), Commands::Lock { force } => commands::lock::cmd_lock(force), Commands::Status { files, json } => commands::status::cmd_status(&files, json), Commands::Key { key_cmd } => match key_cmd { From c1c2a28779fd7ae5b6b95e5e8467e556fafdd3ca Mon Sep 17 00:00:00 2001 From: "Ian G. Maia" Date: Mon, 15 Dec 2025 20:31:07 +0100 Subject: [PATCH 16/16] Add line break --- src/repo.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/repo.rs b/src/repo.rs index 78fb73c..cab5c09 100644 --- a/src/repo.rs +++ b/src/repo.rs @@ -461,6 +461,7 @@ impl TryFrom for Repo { }) } } + // === Helper types and functions === // /// Iterator over filtered files in the git index