diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 48471bb..63e8afe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,7 +25,7 @@ cargo build cargo test ``` -All 91 tests should pass (31 unit + 40 integration + 14 GPG integration + 6 cross-compatibility). They cover: +All 95 tests should pass (33 unit + 40 integration + 16 GPG integration + 6 cross-compatibility). They cover: - AES-256-CTR encryption/decryption round-trips - HMAC-SHA1 known-answer vectors - Key file TLV serialization/deserialization @@ -44,6 +44,9 @@ All 91 tests should pass (31 unit + 40 integration + 14 GPG integration + 6 cros - GPG rm-gpg-user: remove, --no-commit, user not found (GPG integration) - GPG ls-gpg-users: list, no users, named key (GPG integration) - GPG unlock roundtrip: add user, lock, unlock via GPG (GPG integration) +- GPG unlock with passphrase-protected key (pinentry/loopback) (GPG integration) +- GPG unlock clear error when no secret key matches any collaborator (GPG integration) +- GPG decrypt command builder: never passes `--batch` (suppresses pinentry) (unit) - GPG multi-user: add 2 users, remove 1, verify count (GPG integration) - Cross-tool: key exchange, encrypt/decrypt, named keys, binary files (cross-compatibility) diff --git a/README.md b/README.md index 96c3a53..f86348a 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,8 @@ gitveil unlock [...] Without arguments, attempts GPG-based unlock using keys in `.git-crypt/`. With key file arguments, uses symmetric key files. +If your GPG private key is passphrase-protected, gitveil will let `gpg-agent` invoke pinentry to prompt you (terminal or GUI), just like `git-crypt`. Only collaborator files matching a secret key in your local keyring are tried, so pinentry fires at most once. + ### `gitveil add-gpg-user` Add a GPG user as a collaborator. diff --git a/src/commands/unlock.rs b/src/commands/unlock.rs index a5f08eb..6901c05 100644 --- a/src/commands/unlock.rs +++ b/src/commands/unlock.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::io::Cursor; use std::path::PathBuf; @@ -9,7 +10,7 @@ use crate::git::config::configure_filters; use crate::git::repo::{ find_git_dir, find_repo_root, get_encrypted_files, git_crypt_dir, key_path, }; -use crate::gpg::operations::gpg_decrypt_from_file; +use crate::gpg::operations::{gpg_decrypt_from_file, gpg_list_secret_key_fingerprints}; use crate::key::key_file::KeyFile; /// Unlock the repository: load key, configure filters, and decrypt working copy. @@ -53,8 +54,19 @@ pub fn unlock(key_files: &[PathBuf], quiet: bool) -> Result<(), GitVeilError> { return Err(GitVeilError::NotInitialized); } + // Enumerate the fingerprints of secret keys in the local GPG keyring + // so we only attempt to decrypt collaborator files we actually have a + // private key for. This stops pinentry from being invoked once per + // collaborator and stops gpg from spamming stderr with "no secret + // key" for keys we obviously cannot decrypt. + let secret_fps: HashSet = gpg_list_secret_key_fingerprints()? + .into_iter() + .map(|f| f.to_ascii_lowercase()) + .collect(); + let mut unlocked_any = false; let mut last_gpg_error: Option = None; + let mut any_collaborator_found = false; // Iterate over key directories (skip symlinks) let key_dirs: Vec<_> = std::fs::read_dir(&keys_dir)? @@ -69,7 +81,22 @@ pub fn unlock(key_files: &[PathBuf], quiet: bool) -> Result<(), GitVeilError> { let key_dir = key_dir_entry.path(); let gpg_files = find_gpg_files(&key_dir); + if !gpg_files.is_empty() { + any_collaborator_found = true; + } + for gpg_file in &gpg_files { + let stem = gpg_file + .file_stem() + .map(|s| s.to_string_lossy().to_ascii_lowercase()) + .unwrap_or_default(); + + // Skip files encrypted to recipients we don't have a secret + // key for. Filenames are the recipient's primary fingerprint. + if !secret_fps.contains(&stem) { + continue; + } + match gpg_decrypt_from_file(gpg_file) { Ok(key_data) => { let mut cursor = Cursor::new(key_data.as_slice()); @@ -103,14 +130,27 @@ pub fn unlock(key_files: &[PathBuf], quiet: bool) -> Result<(), GitVeilError> { } if !unlocked_any { - let detail = last_gpg_error - .map(|e| format!(" Last error: {}", e)) - .unwrap_or_default(); - return Err(GitVeilError::Gpg(format!( - "failed to decrypt any GPG-encrypted key. \ - Do you have the right GPG private key?{}", - detail - ))); + if !any_collaborator_found { + return Err(GitVeilError::Gpg( + "no GPG-encrypted keys found in .git-crypt/keys/".into(), + )); + } + if let Some(detail) = last_gpg_error { + // A matching key was attempted but decryption failed (e.g., + // the user cancelled the pinentry prompt, or the secret key + // is no longer available to gpg-agent). + return Err(GitVeilError::Gpg(format!( + "GPG decryption failed. {}", + detail + ))); + } + // Collaborators exist but none match a secret key in our keyring. + return Err(GitVeilError::Gpg( + "no GPG secret key in your keyring matches any collaborator on this \ + repository. Make sure the right private key is imported (try \ + `gpg --list-secret-keys`)." + .into(), + )); } } diff --git a/src/gpg/operations.rs b/src/gpg/operations.rs index 3d66700..2074edd 100644 --- a/src/gpg/operations.rs +++ b/src/gpg/operations.rs @@ -1,4 +1,4 @@ -use std::io::Write; +use std::io::{Read, Write}; use std::path::Path; use std::process::{Command, Stdio}; use zeroize::Zeroizing; @@ -132,26 +132,144 @@ pub fn gpg_encrypt_to_file( Ok(()) } +/// Build the gpg command line for decrypting a single file. +/// +/// This intentionally does NOT pass `--batch`: decrypting a collaborator's +/// key requires the user's private GPG key, which is often passphrase- +/// protected. With `--batch`, gpg-agent refuses to launch pinentry and +/// decryption fails with "Inappropriate ioctl for device". Matching +/// git-crypt's behavior, the caller is responsible for connecting stdin +/// and stderr to the user's terminal so pinentry can prompt. +fn build_decrypt_command(gpg: &str, path: &Path) -> Command { + let mut cmd = Command::new(gpg); + cmd.args(["--yes", "-q", "-d"]); + cmd.arg(path); + cmd +} + /// Decrypt a GPG-encrypted file and return the plaintext. +/// +/// Mirrors git-crypt: stdin and stderr are inherited from the parent so +/// pinentry (terminal or graphical) can prompt the user for a passphrase +/// when the secret key is protected. Stdout is piped so the decrypted +/// bytes can be collected. +/// /// The returned buffer is wrapped in `Zeroizing` to ensure key material /// is scrubbed from memory when dropped. pub fn gpg_decrypt_from_file(path: &Path) -> Result>, GitVeilError> { let gpg = get_gpg_program(); + let mut cmd = build_decrypt_command(&gpg, path); + cmd.stdin(Stdio::inherit()) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()); + + let mut child = cmd + .spawn() + .map_err(|e| GitVeilError::Gpg(format!("failed to run {}: {}", gpg, e)))?; + + let mut buf = Vec::new(); + if let Some(mut stdout) = child.stdout.take() { + stdout + .read_to_end(&mut buf) + .map_err(|e| GitVeilError::Gpg(format!("failed to read gpg stdout: {}", e)))?; + } + + let status = child + .wait() + .map_err(|e| GitVeilError::Gpg(format!("failed to wait for gpg: {}", e)))?; + + if !status.success() { + return Err(GitVeilError::Gpg(format!( + "gpg decryption failed for {}", + path.display() + ))); + } + + Ok(Zeroizing::new(buf)) +} + +/// List the primary fingerprints of all secret keys in the local GPG keyring. +/// +/// Used by `unlock` to skip `.gpg` files we obviously cannot decrypt, so +/// pinentry only fires once (for our own key) and gpg doesn't spam stderr +/// with "no secret key" messages for every other collaborator. +pub fn gpg_list_secret_key_fingerprints() -> Result, GitVeilError> { + let gpg = get_gpg_program(); let output = Command::new(&gpg) - .args(["--batch", "--yes", "-q", "-d"]) - .arg(path) + .args(["--batch", "--with-colons", "--list-secret-keys"]) .output() .map_err(|e| GitVeilError::Gpg(format!("failed to run {}: {}", gpg, e)))?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); return Err(GitVeilError::Gpg(format!( - "gpg decryption failed for {}: {}", - path.display(), - stderr + "gpg --list-secret-keys failed: {}", + stderr.trim() ))); } - Ok(Zeroizing::new(output.stdout)) + let stdout = String::from_utf8_lossy(&output.stdout); + let mut fingerprints = Vec::new(); + // Colon-format primary fingerprints follow a `sec:` record; subkey + // fingerprints follow `ssb:`. We only want primaries because + // `add-gpg-user` stores collaborator files keyed by primary fingerprint. + let mut expect_primary_fpr = false; + + for line in stdout.lines() { + if line.starts_with("sec:") { + expect_primary_fpr = true; + } else if line.starts_with("ssb:") { + expect_primary_fpr = false; + } else if expect_primary_fpr && line.starts_with("fpr:") { + let parts: Vec<&str> = line.split(':').collect(); + if parts.len() > 9 { + let fp = parts[9].to_string(); + if validate_fingerprint(&fp).is_ok() { + fingerprints.push(fp); + } + } + expect_primary_fpr = false; + } + } + + Ok(fingerprints) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn decrypt_command_does_not_use_batch() { + let cmd = build_decrypt_command("gpg", Path::new("dummy.gpg")); + let args: Vec = cmd + .get_args() + .map(|a| a.to_string_lossy().to_string()) + .collect(); + assert!( + !args.iter().any(|a| a == "--batch"), + "decrypt must not use --batch (it suppresses the pinentry passphrase prompt). Args: {:?}", + args, + ); + } + + #[test] + fn decrypt_command_includes_decrypt_and_path() { + let cmd = build_decrypt_command("gpg", Path::new("some/file.gpg")); + let args: Vec = cmd + .get_args() + .map(|a| a.to_string_lossy().to_string()) + .collect(); + assert!( + args.iter().any(|a| a == "-d" || a == "--decrypt"), + "decrypt must request decryption. Args: {:?}", + args, + ); + assert!( + args.iter().any(|a| a.ends_with("file.gpg")), + "decrypt must include the file path. Args: {:?}", + args, + ); + } } diff --git a/tests/gpg_integration.rs b/tests/gpg_integration.rs index 3b9a3c7..00d6ad6 100644 --- a/tests/gpg_integration.rs +++ b/tests/gpg_integration.rs @@ -137,13 +137,45 @@ fn make_initialized_repo(gpg_home: &Path) -> tempfile::TempDir { /// Generate a GPG test key in the given GNUPGHOME. Returns the fingerprint. fn generate_test_key(gpg_home: &Path, name: &str, email: &str) -> String { - let key_spec = format!( - "%no-protection\nKey-Type: RSA\nKey-Length: 2048\nName-Real: {}\nName-Email: {}\nExpire-Date: 0\n%commit\n", - name, email - ); + generate_test_key_inner(gpg_home, name, email, None) +} + +/// Generate a passphrase-protected GPG test key. Returns the fingerprint. +fn generate_test_key_with_passphrase( + gpg_home: &Path, + name: &str, + email: &str, + passphrase: &str, +) -> String { + generate_test_key_inner(gpg_home, name, email, Some(passphrase)) +} + +fn generate_test_key_inner( + gpg_home: &Path, + name: &str, + email: &str, + passphrase: Option<&str>, +) -> String { + let key_spec = match passphrase { + None => format!( + "%no-protection\nKey-Type: RSA\nKey-Length: 2048\nName-Real: {}\nName-Email: {}\nExpire-Date: 0\n%commit\n", + name, email + ), + Some(pw) => format!( + "Key-Type: RSA\nKey-Length: 2048\nName-Real: {}\nName-Email: {}\nExpire-Date: 0\nPassphrase: {}\n%commit\n", + name, email, pw + ), + }; + + // For passphrase-protected keys, loopback mode keeps gpg from invoking + // pinentry while generating the key in CI without a TTY. + let mut args: Vec<&str> = vec!["--batch", "--gen-key"]; + if passphrase.is_some() { + args.extend_from_slice(&["--pinentry-mode", "loopback"]); + } let mut child = Command::new("gpg") - .args(["--batch", "--gen-key"]) + .args(&args) .env("GNUPGHOME", gpg_home) .stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) @@ -725,3 +757,134 @@ fn test_add_and_remove_multiple_users() { stdout ); } + +// ─── Passphrase Prompt Regression ────────────────────────────── + +/// Verifies that gitveil unlock works when the user's GPG private key is +/// passphrase-protected. The earlier implementation passed `--batch` to gpg +/// during decryption, which suppressed pinentry and failed with +/// "Inappropriate ioctl for device". Pinentry can't run in CI, so the test +/// supplies the passphrase via gpg's loopback `passphrase-file` mechanism — +/// the same code path gpg uses when pinentry is unavailable. +#[test] +fn test_gpg_unlock_with_passphrase_protected_key() { + skip_without_gpg!(); + let gpg_home = tempfile::tempdir().unwrap(); + + // Configure gpg-agent and gpg to accept a passphrase via file (no TTY). + let passphrase = "test-gitveil-passphrase"; + let passphrase_file = gpg_home.path().join("passphrase.txt"); + fs::write(&passphrase_file, passphrase).unwrap(); + fs::write( + gpg_home.path().join("gpg-agent.conf"), + "allow-loopback-pinentry\n", + ) + .unwrap(); + // Forward slashes work on every platform GnuPG runs on, including + // Windows; backslashes can be interpreted as escapes in gpg.conf. + let pw_path_gpgconf = passphrase_file.display().to_string().replace('\\', "/"); + fs::write( + gpg_home.path().join("gpg.conf"), + format!( + "pinentry-mode loopback\npassphrase-file {}\n", + pw_path_gpgconf + ), + ) + .unwrap(); + + generate_test_key_with_passphrase(gpg_home.path(), "Pat Test", "pat@gitveil.test", passphrase); + let dir = make_initialized_repo(gpg_home.path()); + + // Track an encrypted file + let gitattributes = dir.path().join(".gitattributes"); + fs::write(&gitattributes, "*.secret filter=git-crypt diff=git-crypt\n").unwrap(); + let secret_file = dir.path().join("data.secret"); + fs::write(&secret_file, "pinentry-roundtrip-secret\n").unwrap(); + assert_success(&git(dir.path(), &["add", "."]), "git add"); + assert_success( + &git(dir.path(), &["commit", "-m", "add secrets"]), + "git commit secrets", + ); + + assert_success( + &gitveil_gpg( + gpg_home.path(), + dir.path(), + &["add-gpg-user", "--trusted", "pat@gitveil.test"], + ), + "add-gpg-user", + ); + assert_success( + &gitveil_gpg(gpg_home.path(), dir.path(), &["lock", "--force"]), + "gitveil lock", + ); + + let locked = fs::read(&secret_file).unwrap(); + assert!( + locked.starts_with(b"\0GITCRYPT\0"), + "file should be encrypted after lock" + ); + + // The actual regression: unlock used to fail with "Inappropriate ioctl + // for device" against a passphrase-protected key. With --batch removed + // and stdio inherited, gpg can use the agent (loopback in this test). + let out = gitveil_gpg(gpg_home.path(), dir.path(), &["unlock"]); + assert_success(&out, "gitveil unlock (passphrase-protected key)"); + + let decrypted = fs::read_to_string(&secret_file).unwrap(); + assert_eq!(decrypted, "pinentry-roundtrip-secret\n"); +} + +// ─── Missing Secret Key ──────────────────────────────────────── + +/// When the local GPG keyring holds no secret key matching any collaborator, +/// unlock should fail with a clear, actionable error — not loop through every +/// .gpg file calling pinentry and burying the user under "no secret key" +/// noise on stderr. +#[test] +fn test_gpg_unlock_no_matching_secret_key_gives_clear_error() { + skip_without_gpg!(); + + // Alice's keyring (has the private key). + let alice_home = tempfile::tempdir().unwrap(); + generate_test_key( + alice_home.path(), + "Alice Test", + "alice-nomatch@gitveil.test", + ); + let alice_pub = alice_home.path().join("alice.asc"); + export_key_to_file(alice_home.path(), "alice-nomatch@gitveil.test", &alice_pub); + + // Bob's keyring: imports Alice's *public* key but has no private key + // matching any collaborator. + let bob_home = tempfile::tempdir().unwrap(); + let import_out = Command::new("gpg") + .args(["--batch", "--import"]) + .arg(&alice_pub) + .env("GNUPGHOME", bob_home.path()) + .output() + .expect("import alice pubkey into bob"); + assert_success(&import_out, "import alice public key into bob"); + + let dir = make_initialized_repo(bob_home.path()); + assert_success( + &gitveil_gpg( + bob_home.path(), + dir.path(), + &["add-gpg-user", "--trusted", "alice-nomatch@gitveil.test"], + ), + "add Alice as collaborator", + ); + + let out = gitveil_gpg(bob_home.path(), dir.path(), &["unlock"]); + assert!( + !out.status.success(), + "unlock should fail when no secret key matches" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("no GPG secret key") || stderr.contains("secret key"), + "error should mention missing secret key, got: {}", + stderr, + ); +}