From 3c45a7d57bdc707f65f1abee44a4746e8c0f4e3c Mon Sep 17 00:00:00 2001 From: Byron Date: Tue, 1 Sep 2026 21:30:32 +0200 Subject: [PATCH] fix(gix): find bundled signature programs on Windows Git for Windows makes its bundled `gpg`, `gpgsm`, and `ssh-keygen` available by prepending installation directories to `PATH`. Gitoxide can run outside that prepared environment, so bare defaults may not resolve. Use `gix_path::env::installation_program()` for unconfigured defaults on Windows and retain the bare name as fallback. Explicit configuration and non-Windows behavior stay unchanged. Assisted-by: Codex Co-authored-by: GPT 5.6 --- .gitattributes | 1 + gix-object/Cargo.toml | 4 ++-- gix-object/src/signature/mod.rs | 6 ++++++ gix-object/src/signature/sign.rs | 3 ++- gix-object/src/signature/verify.rs | 4 +++- gix-path/src/env/auxiliary.rs | 13 ++++++++++++- gix/src/commit/mod.rs | 9 ++++++++- gix/tests/gix/commit.rs | 13 +++++++++---- tests/tools/src/signature.rs | 3 ++- 9 files changed, 45 insertions(+), 11 deletions(-) diff --git a/.gitattributes b/.gitattributes index 1e27c87b7f1..97d7f0b743c 100644 --- a/.gitattributes +++ b/.gitattributes @@ -3,6 +3,7 @@ # assure line feeds don't interfere with our working copy hash *.sh text eol=lf justfile text eol=lf +/tests/tools/src/signature/fixtures/ssh-* text eol=lf # have GitHub include fixture-making scripts when it counts code **/tests/fixtures/**/*.sh -linguist-vendored diff --git a/gix-object/Cargo.toml b/gix-object/Cargo.toml index df36a5e35d4..0b24abfefdc 100644 --- a/gix-object/Cargo.toml +++ b/gix-object/Cargo.toml @@ -27,7 +27,7 @@ path = "./benches/edit_tree.rs" [features] ## Enable commit and annotated-tag signing and signature verification with external programs. -signature = ["dep:gix-command", "dep:gix-tempfile"] +signature = ["dep:gix-command", "dep:gix-path", "dep:gix-tempfile"] ## Enable support for the SHA-1 hash by enabling the respective feature in the `gix-hash` crate. sha1 = ["gix-hash/sha1"] ## Enable support for the SHA-256 hash by enabling the respective feature in the `gix-hash` crate. @@ -51,6 +51,7 @@ gix-actor = { version = "^0.42.0", path = "../gix-actor" } gix-date = { version = "^0.16.0", path = "../gix-date" } gix-utils = { version = "^0.3.6", path = "../gix-utils" } gix-command = { version = "^0.10.1", path = "../gix-command", optional = true } +gix-path = { version = "^0.12.6", path = "../gix-path", optional = true } gix-tempfile = { version = "^24.0.0", path = "../gix-tempfile", optional = true } itoa = "1.0.17" @@ -71,7 +72,6 @@ gix-object = { path = ".", features = ["signature", "sha1", "sha256"] } gix-hash = { path = "../gix-hash", features = ["bstr"] } gix-testtools = { path = "../tests/tools", default-features = false } gix-odb = { path = "../gix-odb" } -gix-path = { path = "../gix-path" } termtree = "1.0.0" criterion = "0.8.2" pretty_assertions = "1.0.0" diff --git a/gix-object/src/signature/mod.rs b/gix-object/src/signature/mod.rs index 44b52fde247..6a7b4f8aaf5 100644 --- a/gix-object/src/signature/mod.rs +++ b/gix-object/src/signature/mod.rs @@ -11,6 +11,12 @@ pub mod sign; #[cfg(feature = "signature")] pub mod verify; +#[cfg(feature = "signature")] +fn ssh_path_argument(path: &std::path::Path) -> std::path::PathBuf { + // The mixed `C:/…` form works with native Windows and Git for Windows' MSYS OpenSSH. + gix_path::from_bstring(gix_path::to_unix_separators_on_windows(gix_path::into_bstr(path)).into_owned()) +} + /// A borrowed armored signature and its detected format. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct SignatureRef<'a> { diff --git a/gix-object/src/signature/sign.rs b/gix-object/src/signature/sign.rs index 80081fa36c0..e3a2a743462 100644 --- a/gix-object/src/signature/sign.rs +++ b/gix-object/src/signature/sign.rs @@ -166,6 +166,7 @@ fn sign_ssh(payload: &[u8], options: &Options) -> Result { // Unlike literal keys, resolved key paths can be passed directly to `ssh-keygen -f`. None => (options.signing_key.clone(), false), }; + let key = super::ssh_path_argument(std::path::Path::new(&key)); let mut payload_file = secure_temporary_file()?; write_temporary(&mut payload_file, payload)?; let payload_path = temporary_path(&mut payload_file)?; @@ -177,7 +178,7 @@ fn sign_ssh(payload: &[u8], options: &Options) -> Result { command = command.arg("-U"); } let output = command - .arg(&payload_path) + .arg(super::ssh_path_argument(&payload_path)) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) diff --git a/gix-object/src/signature/verify.rs b/gix-object/src/signature/verify.rs index 28b7e351af5..9bb4993028f 100644 --- a/gix-object/src/signature/verify.rs +++ b/gix-object/src/signature/verify.rs @@ -305,7 +305,9 @@ impl SignedData<'_> { .map_err(|err| Error::CommitTime(Box::new(err)))?; let verify_time = format!("-Overify-time={verify_time}"); let mut signature_file = signature_file(signature)?; - let signature_path = signature_path(&mut signature_file)?; + let signature_path = super::ssh_path_argument(&signature_path(&mut signature_file)?); + let allowed_signers = super::ssh_path_argument(&allowed_signers); + let revocation_file = revocation_file.map(|path| super::ssh_path_argument(&path)); // defensive, as we rely on English when parsing output. environment.extend([("LANG".into(), "C".into()), ("LC_ALL".into(), "C".into())]); let common = ( diff --git a/gix-path/src/env/auxiliary.rs b/gix-path/src/env/auxiliary.rs index 0ff7626e66f..571190d29f1 100644 --- a/gix-path/src/env/auxiliary.rs +++ b/gix-path/src/env/auxiliary.rs @@ -136,7 +136,18 @@ mod tests { /// /// Tests are expected to run with a full Git for Windows installation (not MinGit). const SHOULD_FIND: &[&str] = &[ - "sh", "bash", "dash", "diff", "tar", "less", "sed", "awk", "perl", "cygpath", + "sh", + "bash", + "dash", + "diff", + "tar", + "less", + "sed", + "awk", + "perl", + "cygpath", + "gpg", + "ssh-keygen", ]; /// Shouldn't find anything nonexistent, or only in PATH or in `bin`s we don't mean to search. diff --git a/gix/src/commit/mod.rs b/gix/src/commit/mod.rs index 0e6c6e74b27..5e7e8cad9a7 100644 --- a/gix/src/commit/mod.rs +++ b/gix/src/commit/mod.rs @@ -30,7 +30,14 @@ fn signature_program( Format::Ssh => (config.trusted_path(gpg::Ssh::PROGRAM)?, &gpg::Ssh::PROGRAM), }; Ok(program - .unwrap_or_else(|| gix_path::from_bstr(default.default_value_or_panic()).into_owned()) + .unwrap_or_else(|| { + let default = gix_path::from_bstr(default.default_value_or_panic()).into_owned(); + #[cfg(windows)] + if let Some(program) = default.to_str().and_then(gix_path::env::installation_program) { + return program; + } + default + }) .into_os_string()) } diff --git a/gix/tests/gix/commit.rs b/gix/tests/gix/commit.rs index 1dbf9946c46..2057c221d9e 100644 --- a/gix/tests/gix/commit.rs +++ b/gix/tests/gix/commit.rs @@ -152,9 +152,6 @@ mod signature { #[test] fn sign_write_and_verify_an_ssh_commit() -> crate::Result { - if !signature::program_available("ssh-keygen") { - return Ok(()); - } let (_key_home, key) = signature::ssh_private_key()?; let options = gix::open::Options::isolated().config_overrides([ User::NAME.validated_assignment_fmt(&"Gitoxide Signing Fixture")?, @@ -169,7 +166,15 @@ mod signature { .with_object_memory(); let mut signing_options = repo.commit_signing_options()?; assert_eq!(signing_options.format, gix::commit::sign::Format::Ssh); - assert_eq!(signing_options.program, "ssh-keygen"); + let expected_program = if cfg!(windows) { + gix::path::env::installation_program("ssh-keygen").unwrap_or_else(|| "ssh-keygen".into()) + } else { + "ssh-keygen".into() + }; + assert_eq!(signing_options.program, expected_program.into_os_string()); + if !signature::program_available(&signing_options.program) { + return Ok(()); + } assert_eq!(signing_options.signing_key, key); assert!(signing_options.program_arguments.is_empty()); signing_options.program_arguments.push("-q".into()); diff --git a/tests/tools/src/signature.rs b/tests/tools/src/signature.rs index 2105fa8b232..73700feef76 100644 --- a/tests/tools/src/signature.rs +++ b/tests/tools/src/signature.rs @@ -8,6 +8,7 @@ //! These public test identities provide no security and must never be used outside tests. use std::{ + ffi::OsStr, path::{Path, PathBuf}, process::{Command, Stdio}, }; @@ -58,7 +59,7 @@ fn msys_path(path: &str) -> String { } /// Return whether signing `program` can be launched. -pub fn program_available(program: &str) -> bool { +pub fn program_available(program: impl AsRef) -> bool { Command::new(program) .arg("--version") .stdout(Stdio::null())