Support git commit signing using OpenPGP (#1544)

* Support git commit signing using OpenPGP
* workaround for amending signed commits
* workaround for rewording signed commits
* support signing initial commit
* return both signature and signature_field value from sign

---------

Co-authored-by: Utkarsh Gupta <utkarshgupta137@gmail.com>

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Added
+* sign commits using openpgp; implement `Sign` trait to implement more methods
+
 ## [0.25.2] - 2024-03-22
 
 ### Fixes

asyncgit/src/error.rs

@@ -84,6 +84,26 @@ pub enum Error {
 	///
 	#[error("git hook error: {0}")]
 	Hooks(#[from] git2_hooks::HooksError),
+
+	///
+	#[error("sign builder error: {0}")]
+	SignBuilder(#[from] crate::sync::sign::SignBuilderError),
+
+	///
+	#[error("sign error: {0}")]
+	Sign(#[from] crate::sync::sign::SignError),
+
+	///
+	#[error("amend error: config commit.gpgsign=true detected.\ngpg signing is not supported for amending non-last commits")]
+	SignAmendNonLastCommit,
+
+	///
+	#[error("reword error: config commit.gpgsign=true detected.\ngpg signing is not supported for rewording non-last commits")]
+	SignRewordNonLastCommit,
+
+	///
+	#[error("reword error: config commit.gpgsign=true detected.\ngpg signing is not supported for rewording commits with staged changes\ntry unstaging or stashing your changes")]
+	SignRewordLastCommitStaged,
 }
 
 ///

asyncgit/src/sync/commit.rs

@@ -1,7 +1,8 @@
 //! Git Api for Commits
 use super::{CommitId, RepoPath};
+use crate::sync::sign::{SignBuilder, SignError};
 use crate::{
-	error::Result,
+	error::{Error, Result},
 	sync::{repository::repo, utils::get_head_repo},
 };
 use git2::{
@@ -18,12 +19,27 @@ pub fn amend(
 	scope_time!("amend");
 
 	let repo = repo(repo_path)?;
+	let config = repo.config()?;
+
 	let commit = repo.find_commit(id.into())?;
 
 	let mut index = repo.index()?;
 	let tree_id = index.write_tree()?;
 	let tree = repo.find_tree(tree_id)?;
 
+	if config.get_bool("commit.gpgsign").unwrap_or(false) {
+		// HACK: we undo the last commit and create a new one
+		use crate::sync::utils::undo_last_commit;
+
+		let head = get_head_repo(&repo)?;
+		if head == commit.id().into() {
+			undo_last_commit(repo_path)?;
+			return self::commit(repo_path, msg);
+		}
+
+		return Err(Error::SignAmendNonLastCommit);
+	}
+
 	let new_id = commit.amend(
 		Some("HEAD"),
 		None,
@@ -68,7 +84,7 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result<CommitId> {
 	scope_time!("commit");
 
 	let repo = repo(repo_path)?;
-
+	let config = repo.config()?;
 	let signature = signature_allow_undefined_name(&repo)?;
 	let mut index = repo.index()?;
 	let tree_id = index.write_tree()?;
@@ -82,8 +98,52 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result<CommitId> {
 
 	let parents = parents.iter().collect::<Vec<_>>();
 
-	Ok(repo
+	let commit_id = if config
-		.commit(
+		.get_bool("commit.gpgsign")
+		.unwrap_or(false)
+	{
+		use crate::sync::sign::Sign;
+
+		let buffer = repo.commit_create_buffer(
+			&signature,
+			&signature,
+			msg,
+			&tree,
+			parents.as_slice(),
+		)?;
+
+		let commit = std::str::from_utf8(&buffer).map_err(|_e| {
+			SignError::Shellout("utf8 conversion error".to_string())
+		})?;
+
+		let sign = SignBuilder::from_gitconfig(&repo, &config)?;
+		let (signature, signature_field) = sign.sign(&buffer)?;
+		let commit_id = repo.commit_signed(
+			commit,
+			&signature,
+			Some(&signature_field),
+		)?;
+
+		// manually advance to the new commit ID
+		// repo.commit does that on its own, repo.commit_signed does not
+		// if there is no head, read default branch or defaul to "master"
+		if let Ok(mut head) = repo.head() {
+			head.set_target(commit_id, msg)?;
+		} else {
+			let default_branch_name = config
+				.get_str("init.defaultBranch")
+				.unwrap_or("master");
+			repo.reference(
+				&format!("refs/heads/{default_branch_name}"),
+				commit_id,
+				true,
+				msg,
+			)?;
+		}
+
+		commit_id
+	} else {
+		repo.commit(
 			Some("HEAD"),
 			&signature,
 			&signature,
@@ -91,7 +151,9 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result<CommitId> {
 			&tree,
 			parents.as_slice(),
 		)?
-		.into())
+	};
+
+	Ok(commit_id.into())
 }
 
 /// Tag a commit.

asyncgit/src/sync/mod.rs

@@ -25,6 +25,7 @@ pub mod remotes;
 mod repository;
 mod reset;
 mod reword;
+pub mod sign;
 mod staging;
 mod stash;
 mod state;

asyncgit/src/sync/reword.rs

@@ -3,7 +3,7 @@ use git2::{Oid, RebaseOptions, Repository};
 use super::{
 	commit::signature_allow_undefined_name,
 	repo,
-	utils::{bytes2string, get_head_refname},
+	utils::{bytes2string, get_head_refname, get_head_repo},
 	CommitId, RepoPath,
 };
 use crate::error::{Error, Result};
@@ -15,6 +15,32 @@ pub fn reword(
 	message: &str,
 ) -> Result<CommitId> {
 	let repo = repo(repo_path)?;
+	let config = repo.config()?;
+
+	if config.get_bool("commit.gpgsign").unwrap_or(false) {
+		// HACK: we undo the last commit and create a new one
+		use crate::sync::utils::undo_last_commit;
+
+		let head = get_head_repo(&repo)?;
+		if head == commit {
+			// Check if there are any staged changes
+			let parent = repo.find_commit(head.into())?;
+			let tree = parent.tree()?;
+			if repo
+				.diff_tree_to_index(Some(&tree), None, None)?
+				.deltas()
+				.len() == 0
+			{
+				undo_last_commit(repo_path)?;
+				return super::commit(repo_path, message);
+			}
+
+			return Err(Error::SignRewordLastCommitStaged);
+		}
+
+		return Err(Error::SignRewordNonLastCommit);
+	}
+
 	let cur_branch_ref = get_head_refname(&repo)?;
 
 	match reword_internal(&repo, commit.get_oid(), message) {

asyncgit/src/sync/sign.rs

@@ -0,0 +1,325 @@
+//! Sign commit data.
+
+/// Error type for [`SignBuilder`], used to create [`Sign`]'s
+#[derive(thiserror::Error, Debug)]
+pub enum SignBuilderError {
+	/// The given format is invalid
+	#[error("Failed to derive a commit signing method from git configuration 'gpg.format': {0}")]
+	InvalidFormat(String),
+
+	/// The GPG signing key could
+	#[error("Failed to retrieve 'user.signingkey' from the git configuration: {0}")]
+	GPGSigningKey(String),
+
+	/// No signing signature could be built from the configuration data present
+	#[error("Failed to build signing signature: {0}")]
+	Signature(String),
+
+	/// Failure on unimplemented signing methods
+	/// to be removed once all methods have been implemented
+	#[error("Select signing method '{0}' has not been implemented")]
+	MethodNotImplemented(String),
+}
+
+/// Error type for [`Sign`], used to sign data
+#[derive(thiserror::Error, Debug)]
+pub enum SignError {
+	/// Unable to spawn process
+	#[error("Failed to spawn signing process: {0}")]
+	Spawn(String),
+
+	/// Unable to acquire the child process' standard input to write the commit data for signing
+	#[error("Failed to acquire standard input handler")]
+	Stdin,
+
+	/// Unable to write commit data to sign to standard input of the child process
+	#[error("Failed to write buffer to standard input of signing process: {0}")]
+	WriteBuffer(String),
+
+	/// Unable to retrieve the signed data from the child process
+	#[error("Failed to get output of signing process call: {0}")]
+	Output(String),
+
+	/// Failure of the child process
+	#[error("Failed to execute signing process: {0}")]
+	Shellout(String),
+}
+
+/// Sign commit data using various methods
+pub trait Sign {
+	/// Sign commit with the respective implementation.
+	///
+	/// Retrieve an implementation using [`SignBuilder::from_gitconfig`].
+	///
+	/// The `commit` buffer can be created using the following steps:
+	/// - create a buffer using [`git2::Repository::commit_create_buffer`]
+	///
+	/// The function returns a tuple of `signature` and `signature_field`.
+	/// These values can then be passed into [`git2::Repository::commit_signed`].
+	/// Finally, the repository head needs to be advanced to the resulting commit ID
+	/// using [`git2::Reference::set_target`].
+	fn sign(
+		&self,
+		commit: &[u8],
+	) -> Result<(String, String), SignError>;
+
+	#[cfg(test)]
+	fn program(&self) -> &String;
+
+	#[cfg(test)]
+	fn signing_key(&self) -> &String;
+}
+
+/// A builder to facilitate the creation of a signing method ([`Sign`]) by examining the git configuration.
+pub struct SignBuilder;
+
+impl SignBuilder {
+	/// Get a [`Sign`] from the given repository configuration to sign commit data
+	///
+	///
+	/// ```no_run
+	/// use asyncgit::sync::sign::SignBuilder;
+	/// # fn main() -> Result<(), Box<dyn std::error::Error>> {
+	///
+	/// /// Repo in a temporary directory for demonstration
+	/// let dir = std::env::temp_dir();
+	/// let repo = git2::Repository::init(dir)?;
+	///
+	/// /// Get the config from the repository
+	/// let config = repo.config()?;
+	///
+	/// /// Retrieve a `Sign` implementation
+	/// let sign = SignBuilder::from_gitconfig(&repo, &config)?;
+	/// # Ok(())
+	/// # }
+	/// ```
+	pub fn from_gitconfig(
+		repo: &git2::Repository,
+		config: &git2::Config,
+	) -> Result<impl Sign, SignBuilderError> {
+		let format = config
+			.get_string("gpg.format")
+			.unwrap_or_else(|_| "openpgp".to_string());
+
+		// Variants are described in the git config documentation
+		// https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgformat
+		match format.as_str() {
+			"openpgp" => {
+				// Try to retrieve the gpg program from the git configuration,
+				// moving from the least to the most specific config key,
+				// defaulting to "gpg" if nothing is explicitly defined (per git's implementation)
+				// https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgprogram
+				// https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgprogram
+				let program = config
+					.get_string("gpg.openpgp.program")
+					.or_else(|_| config.get_string("gpg.program"))
+					.unwrap_or_else(|_| "gpg".to_string());
+
+				// Optional signing key.
+				// If 'user.signingKey' is not set, we'll use 'user.name' and 'user.email'
+				// to build a default signature in the format 'name <email>'.
+				// https://git-scm.com/docs/git-config#Documentation/git-config.txt-usersigningKey
+				let signing_key = config
+					.get_string("user.signingKey")
+					.or_else(
+						|_| -> Result<String, SignBuilderError> {
+							Ok(crate::sync::commit::signature_allow_undefined_name(repo)
+                                .map_err(|err| {
+                                    SignBuilderError::Signature(
+                                        err.to_string(),
+                                    )
+                                })?
+                                .to_string())
+						},
+					)
+					.map_err(|err| {
+						SignBuilderError::GPGSigningKey(
+							err.to_string(),
+						)
+					})?;
+
+				Ok(GPGSign {
+					program,
+					signing_key,
+				})
+			}
+			"x509" => Err(SignBuilderError::MethodNotImplemented(
+				String::from("x509"),
+			)),
+			"ssh" => Err(SignBuilderError::MethodNotImplemented(
+				String::from("ssh"),
+			)),
+			_ => Err(SignBuilderError::InvalidFormat(format)),
+		}
+	}
+}
+
+/// Sign commit data using `OpenPGP`
+pub struct GPGSign {
+	program: String,
+	signing_key: String,
+}
+
+impl GPGSign {
+	/// Create new [`GPGSign`] using given program and signing key.
+	pub fn new(program: &str, signing_key: &str) -> Self {
+		Self {
+			program: program.to_string(),
+			signing_key: signing_key.to_string(),
+		}
+	}
+}
+
+impl Sign for GPGSign {
+	fn sign(
+		&self,
+		commit: &[u8],
+	) -> Result<(String, String), SignError> {
+		use std::io::Write;
+		use std::process::{Command, Stdio};
+
+		let mut cmd = Command::new(&self.program);
+		cmd.stdin(Stdio::piped())
+			.stdout(Stdio::piped())
+			.stderr(Stdio::piped())
+			.arg("--status-fd=2")
+			.arg("-bsau")
+			.arg(&self.signing_key);
+
+		log::trace!("signing command: {cmd:?}");
+
+		let mut child = cmd
+			.spawn()
+			.map_err(|e| SignError::Spawn(e.to_string()))?;
+
+		let mut stdin = child.stdin.take().ok_or(SignError::Stdin)?;
+
+		stdin
+			.write_all(commit)
+			.map_err(|e| SignError::WriteBuffer(e.to_string()))?;
+		drop(stdin); // close stdin to not block indefinitely
+
+		let output = child
+			.wait_with_output()
+			.map_err(|e| SignError::Output(e.to_string()))?;
+
+		if !output.status.success() {
+			return Err(SignError::Shellout(format!(
+				"failed to sign data, program '{}' exited non-zero: {}",
+				&self.program,
+				std::str::from_utf8(&output.stderr)
+					.unwrap_or("[error could not be read from stderr]")
+			)));
+		}
+
+		let stderr = std::str::from_utf8(&output.stderr)
+			.map_err(|e| SignError::Shellout(e.to_string()))?;
+
+		if !stderr.contains("\n[GNUPG:] SIG_CREATED ") {
+			return Err(SignError::Shellout(
+				format!("failed to sign data, program '{}' failed, SIG_CREATED not seen in stderr", &self.program),
+			));
+		}
+
+		let signed_commit = std::str::from_utf8(&output.stdout)
+			.map_err(|e| SignError::Shellout(e.to_string()))?;
+
+		Ok((signed_commit.to_string(), "gpgsig".to_string()))
+	}
+
+	#[cfg(test)]
+	fn program(&self) -> &String {
+		&self.program
+	}
+
+	#[cfg(test)]
+	fn signing_key(&self) -> &String {
+		&self.signing_key
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use crate::error::Result;
+	use crate::sync::tests::repo_init_empty;
+
+	#[test]
+	fn test_invalid_signing_format() -> Result<()> {
+		let (_temp_dir, repo) = repo_init_empty()?;
+
+		{
+			let mut config = repo.config()?;
+			config.set_str("gpg.format", "INVALID_SIGNING_FORMAT")?;
+		}
+
+		let sign =
+			SignBuilder::from_gitconfig(&repo, &repo.config()?);
+
+		assert!(sign.is_err());
+
+		Ok(())
+	}
+
+	#[test]
+	fn test_program_and_signing_key_defaults() -> Result<()> {
+		let (_tmp_dir, repo) = repo_init_empty()?;
+		let sign =
+			SignBuilder::from_gitconfig(&repo, &repo.config()?)?;
+
+		assert_eq!("gpg", sign.program());
+		assert_eq!("name <email>", sign.signing_key());
+
+		Ok(())
+	}
+
+	#[test]
+	fn test_gpg_program_configs() -> Result<()> {
+		let (_tmp_dir, repo) = repo_init_empty()?;
+
+		{
+			let mut config = repo.config()?;
+			config.set_str("gpg.program", "GPG_PROGRAM_TEST")?;
+		}
+
+		let sign =
+			SignBuilder::from_gitconfig(&repo, &repo.config()?)?;
+
+		// we get gpg.program, because gpg.openpgp.program is not set
+		assert_eq!("GPG_PROGRAM_TEST", sign.program());
+
+		{
+			let mut config = repo.config()?;
+			config.set_str(
+				"gpg.openpgp.program",
+				"GPG_OPENPGP_PROGRAM_TEST",
+			)?;
+		}
+
+		let sign =
+			SignBuilder::from_gitconfig(&repo, &repo.config()?)?;
+
+		// since gpg.openpgp.program is now set as well, it is more specific than
+		// gpg.program and therefore takes precedence
+		assert_eq!("GPG_OPENPGP_PROGRAM_TEST", sign.program());
+
+		Ok(())
+	}
+
+	#[test]
+	fn test_user_signingkey() -> Result<()> {
+		let (_tmp_dir, repo) = repo_init_empty()?;
+
+		{
+			let mut config = repo.config()?;
+			config.set_str("user.signingKey", "FFAA")?;
+		}
+
+		let sign =
+			SignBuilder::from_gitconfig(&repo, &repo.config()?)?;
+
+		assert_eq!("FFAA", sign.signing_key());
+
+		Ok(())
+	}
+}

src/popups/commit.rs

@@ -205,17 +205,6 @@ impl CommitPopup {
 	}
 
 	fn commit(&mut self) -> Result<()> {
-		let gpgsign =
-			get_config_string(&self.repo.borrow(), "commit.gpgsign")
-				.ok()
-				.flatten()
-				.and_then(|path| path.parse::<bool>().ok())
-				.unwrap_or_default();
-
-		if gpgsign {
-			anyhow::bail!("config commit.gpgsign=true detected.\ngpg signing not supported.\ndeactivate in your repo/gitconfig to be able to commit without signing.");
-		}
-
 		let msg = self.input.get_text().to_string();
 
 		if matches!(