From 41a801ad7eeffdab3be3bae8f89e8f81ab1d2256 Mon Sep 17 00:00:00 2001 From: Federico Guerinoni Date: Tue, 26 May 2026 15:02:58 +0200 Subject: [PATCH] feat(reword): support gpg signing for non-HEAD commits Rewording any commit older than HEAD used to fail with SignRewordNonLastCommit as soon as commit.gpgsign was enabled, because git2's rebase API has no way to sign the commits it produces. The behaviour matched neither git CLI nor user expectation, so anyone relying on signing had to drop down to the shell to fix a typo a few commits back. --- CHANGELOG.md | 1 + asyncgit/src/sync/mod.rs | 2 + asyncgit/src/sync/reword.rs | 193 +++++++++++++++++++++++++++++++++++- 3 files changed, 195 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6db667675c..6a7b330f1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * use [tombi](https://github.com/tombi-toml/tombi) for all toml file formatting * open the external editor from the status diff view [[@WaterWhisperer](https://github.com/WaterWhisperer)] ([#2805](https://github.com/gitui-org/gitui/issues/2805)) * automatically convert spaces to dashes when creating or renaming a branch [[@pbouillon]](https//pbouillon.github.io)] ([#2916](https://github.com/gitui-org/gitui/pull/2916)) +* support rewording non-HEAD commits when `commit.gpgsign` is enabled, by rebuilding the chain manually and signing each rewritten commit ### Fixes * crash when opening submodule ([#2895](https://github.com/gitui-org/gitui/issues/2895)) diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index 2a5f413e8f..2cd358d065 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -274,6 +274,8 @@ pub mod tests { pub fn repo_init_bare() -> Result<(TempDir, Repository)> { init_log(); + sandbox_config_files(); + let tmp_repo_dir = TempDir::new()?; let bare_repo = Repository::init_bare(tmp_repo_dir.path())?; Ok((tmp_repo_dir, bare_repo)) diff --git a/asyncgit/src/sync/reword.rs b/asyncgit/src/sync/reword.rs index a503686321..352bea1c10 100644 --- a/asyncgit/src/sync/reword.rs +++ b/asyncgit/src/sync/reword.rs @@ -3,6 +3,7 @@ use git2::{Oid, RebaseOptions, Repository}; use super::{ commit::signature_allow_undefined_name, repo, + sign::SignBuilder, utils::{bytes2string, get_head_refname, get_head_repo}, CommitId, RepoPath, }; @@ -38,7 +39,7 @@ pub fn reword( return Err(Error::SignRewordLastCommitStaged); } - return Err(Error::SignRewordNonLastCommit); + return reword_signed(&repo, commit.get_oid(), message); } let cur_branch_ref = get_head_refname(&repo)?; @@ -147,6 +148,116 @@ fn reword_internal( Err(Error::NoBranch) } +/// Reword a non-HEAD commit while honoring `commit.gpgsign`. +/// +/// `git2`'s rebase API cannot sign commits, so we rebuild the chain +/// from `target` up to HEAD manually and sign each rewritten commit with `commit_signed`. +/// A pure reword does not alter trees, so the original tree of every commit in the chain is reused, +/// only parents, committer and (for `target`) the message are rewritten. +fn reword_signed( + repo: &Repository, + target: Oid, + new_message: &str, +) -> Result { + let config = repo.config()?; + let signer = SignBuilder::from_gitconfig(repo, &config)?; + let committer = signature_allow_undefined_name(repo)?; + + let cur_branch_ref = get_head_refname(repo)?; + let head_oid = repo.head()?.peel_to_commit()?.id(); + + // collect commits from HEAD down to (and including) target. + let mut chain = Vec::new(); + let mut cur = repo.find_commit(head_oid)?; + loop { + let id = cur.id(); + let parent_count = cur.parent_count(); + chain.push(cur); + if id == target { + break; + } + if parent_count != 1 { + return Err(Error::Generic( + "reword across merge commits is not supported" + .to_string(), + )); + } + cur = repo.find_commit(id)?.parent(0)?; + } + + // target first, then its descendants up to HEAD. + chain.reverse(); + + let (target_commit, descendants) = + chain.split_first().ok_or_else(|| { + Error::Generic("empty reword chain".to_string()) + })?; + + let target_parent = + target_commit.parent(0).map_err(|_| Error::NoParent)?; + let parents = [&target_parent]; + let new_target_oid = create_signed_commit( + repo, + signer.as_ref(), + &target_commit.author(), + &committer, + new_message, + &target_commit.tree()?, + &parents, + )?; + + let mut last_new_oid = new_target_oid; + for original in descendants { + let new_parent = repo.find_commit(last_new_oid)?; + let parents = [&new_parent]; + let msg = original.message_raw().unwrap_or(""); + last_new_oid = create_signed_commit( + repo, + signer.as_ref(), + &original.author(), + &committer, + msg, + &original.tree()?, + &parents, + )?; + } + + // move the branch to the rewritten tip and refresh the worktree. + let mut branch_ref = repo.find_reference(&cur_branch_ref)?; + branch_ref.set_target(last_new_oid, "gitui: reword (signed)")?; + repo.set_head(&cur_branch_ref)?; + repo.checkout_head(None)?; + + Ok(new_target_oid.into()) +} + +fn create_signed_commit( + repo: &Repository, + signer: &dyn crate::sync::sign::Sign, + author: &git2::Signature<'_>, + committer: &git2::Signature<'_>, + message: &str, + tree: &git2::Tree<'_>, + parents: &[&git2::Commit<'_>], +) -> Result { + use crate::sync::sign::SignError; + + let buffer = repo.commit_create_buffer( + author, committer, message, tree, parents, + )?; + let contents = std::str::from_utf8(&buffer).map_err(|_| { + Error::Sign(SignError::Shellout( + "utf8 conversion error".to_string(), + )) + })?; + let (signature, signature_field) = signer.sign(&buffer)?; + Ok(repo.commit_signed( + contents, + &signature, + signature_field.as_deref(), + )?) +} + #[cfg(test)] mod tests { use super::*; @@ -192,4 +303,84 @@ mod tests { get_commit_info(repo_path, &reworded).unwrap().message ); } + + #[cfg(unix)] + #[test] + fn test_reword_signed_non_head() { + use std::os::unix::fs::PermissionsExt; + + let (td, repo) = repo_init_empty().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path: &RepoPath = + &root.as_os_str().to_str().unwrap().into(); + + // fake gpg program: drain stdin, emit a fixed PGP block on stdout and the GNUPG status line gitui's signer expects on stderr. + let script_path = td.path().join("fake_gpg.sh"); + let script = "#!/bin/sh\n\ + cat > /dev/null\n\ + printf -- '-----BEGIN PGP SIGNATURE-----\\n\\nfake\\n-----END PGP SIGNATURE-----\\n'\n\ + printf '[GNUPG:] BEGIN_SIGNING\\n[GNUPG:] SIG_CREATED fake\\n' 1>&2\n"; + std::fs::write(&script_path, script).unwrap(); + std::fs::set_permissions( + &script_path, + std::fs::Permissions::from_mode(0o755), + ) + .unwrap(); + + { + let mut cfg = repo.config().unwrap(); + cfg.set_bool("commit.gpgsign", true).unwrap(); + cfg.set_str("gpg.format", "openpgp").unwrap(); + cfg.set_str("gpg.program", script_path.to_str().unwrap()) + .unwrap(); + cfg.set_str("user.signingKey", "TEST_KEY").unwrap(); + } + + // build a 3-commit history with signing disabled so we can use + // `write_commit_file`, then enable signing for the reword. + repo.config() + .unwrap() + .set_bool("commit.gpgsign", false) + .unwrap(); + write_commit_file(&repo, "foo", "a", "commit1"); + let oid2 = write_commit_file(&repo, "foo", "ab", "commit2"); + write_commit_file(&repo, "foo", "abc", "commit3"); + repo.config() + .unwrap() + .set_bool("commit.gpgsign", true) + .unwrap(); + + let reworded = + reword(repo_path, oid2, "RewordedMiddle").unwrap(); + + // reworded commit carries the new message. + assert_eq!( + get_commit_info(repo_path, &reworded).unwrap().message, + "RewordedMiddle" + ); + + // HEAD still points to the descendant ("commit3") and the chain length is unchanged. + let head = repo.head().unwrap().peel_to_commit().unwrap(); + assert_eq!(head.message().unwrap().trim_end(), "commit3"); + assert_eq!(head.parent_count(), 1); + let middle = head.parent(0).unwrap(); + assert_eq!(middle.id(), reworded.get_oid()); + let first = middle.parent(0).unwrap(); + assert_eq!(first.message().unwrap().trim_end(), "commit1"); + assert!(first.parent(0).is_err()); + + // both rewritten commits carry the fake PGP signature. + let (sig_middle, _) = + repo.extract_signature(&middle.id(), None).unwrap(); + assert!(sig_middle + .as_str() + .unwrap() + .contains("BEGIN PGP SIGNATURE")); + let (sig_head, _) = + repo.extract_signature(&head.id(), None).unwrap(); + assert!(sig_head + .as_str() + .unwrap() + .contains("BEGIN PGP SIGNATURE")); + } }