From af397c588f737c4560c607f7e32451768ffe0ed7 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Mon, 13 Feb 2023 21:26:15 +0100 Subject: [PATCH 01/13] Support git commit signing using OpenPGP --- asyncgit/src/error.rs | 7 + asyncgit/src/sync/commit.rs | 39 ++++- asyncgit/src/sync/mod.rs | 1 + asyncgit/src/sync/sign.rs | 341 ++++++++++++++++++++++++++++++++++++ src/popups/commit.rs | 11 -- 5 files changed, 384 insertions(+), 15 deletions(-) create mode 100644 asyncgit/src/sync/sign.rs diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index 6ede042230..fe1289b2b6 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -84,6 +84,13 @@ 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), } /// diff --git a/asyncgit/src/sync/commit.rs b/asyncgit/src/sync/commit.rs index b2b7a47440..6994d5c800 100644 --- a/asyncgit/src/sync/commit.rs +++ b/asyncgit/src/sync/commit.rs @@ -1,4 +1,5 @@ use super::{CommitId, RepoPath}; +use crate::sync::sign::{SignBuilder, SignError}; use crate::{ error::Result, sync::{repository::repo, utils::get_head_repo}, @@ -65,7 +66,7 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result { 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()?; @@ -79,8 +80,36 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result { let parents = parents.iter().collect::>(); - Ok(repo - .commit( + let commit_id = if config + .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 signed_commit = sign.sign(commit)?; + let commit_id = + repo.commit_signed(commit, &signed_commit, None)?; + + // manually advance to the new commit ID + // repo.commit does that on its own, repo.commit_signed does not + repo.head()?.set_target(commit_id, msg)?; + + commit_id + } else { + repo.commit( Some("HEAD"), &signature, &signature, @@ -88,7 +117,9 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result { &tree, parents.as_slice(), )? - .into()) + }; + + Ok(commit_id.into()) } /// Tag a commit. diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index c188f28c63..3fc7fa6abc 100644 --- a/asyncgit/src/sync/mod.rs +++ b/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; diff --git a/asyncgit/src/sync/sign.rs b/asyncgit/src/sync/sign.rs new file mode 100644 index 0000000000..9c18c7ee31 --- /dev/null +++ b/asyncgit/src/sync/sign.rs @@ -0,0 +1,341 @@ +//! 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` string slice can be created using the following steps: + /// - create a buffer using [`git2::Repository::commit_create_buffer`] + /// - convert the buffer using [`std::str::from_utf8`] + /// - the resulting string slice can be passed to [`Sign::sign`] now + /// + /// The returned `String` from this function 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: &str) -> Result; + + #[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 + /// + /// ``` + /// use asyncgit::sync::sign::SignBuilder; + /// # fn main() -> Result<(), Box> { + /// + /// /// 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 { + let signing_methods = config + .get_string("gitui.signing_methods") + .unwrap_or_else(|_| "shellouts".to_string()); + + match signing_methods.as_str() { + "shellouts" => { + 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 '. + // 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)), + } + } + "rust" => Err(SignBuilderError::MethodNotImplemented( + String::from(""), + )), + _ => { + Err(SignBuilderError::InvalidFormat(signing_methods)) + } + } + } +} + +/// 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: &str) -> Result { + 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)?; + + write!(stdin, "{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()) + } + + #[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 ", 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(()) + } +} diff --git a/src/popups/commit.rs b/src/popups/commit.rs index 2511d8b8ef..8398e938f7 100644 --- a/src/popups/commit.rs +++ b/src/popups/commit.rs @@ -203,17 +203,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::().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!( From 798afbc63b5ef488c481bf1edc9c003ca2107432 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Mon, 20 Feb 2023 08:32:26 +0100 Subject: [PATCH 02/13] do not run sign builder doc test --- asyncgit/src/sync/sign.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/asyncgit/src/sync/sign.rs b/asyncgit/src/sync/sign.rs index 9c18c7ee31..320a18c2c9 100644 --- a/asyncgit/src/sync/sign.rs +++ b/asyncgit/src/sync/sign.rs @@ -74,7 +74,8 @@ 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> { /// From a3f77aca1f20d5a4c2fbd2f12dc65b85d82e9982 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Tue, 21 Mar 2023 08:22:47 +0000 Subject: [PATCH 03/13] Run existing tests inside a container This is the start of an isolated test environment in order to create real GPG and SSH keys to test signing methods using the actual binaries used in the schellouts. --- .dockerignore | 1 + Dockerfile.sign-test | 4 ++++ SIGN_TEST.md | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 .dockerignore create mode 100644 Dockerfile.sign-test create mode 100644 SIGN_TEST.md diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000000..2f7896d1d1 --- /dev/null +++ b/.dockerignore @@ -0,0 +1 @@ +target/ diff --git a/Dockerfile.sign-test b/Dockerfile.sign-test new file mode 100644 index 0000000000..1735a3b69e --- /dev/null +++ b/Dockerfile.sign-test @@ -0,0 +1,4 @@ +FROM rust:slim +ENV CARGO_REGISTRIES_CRATES_IO_PROTOCOL=sparse +RUN apt-get update && apt-get install --yes python +COPY . . diff --git a/SIGN_TEST.md b/SIGN_TEST.md new file mode 100644 index 0000000000..9f79b74dc0 --- /dev/null +++ b/SIGN_TEST.md @@ -0,0 +1,34 @@ +# Git Commit Signing Tests + +Test commit signing in the isolation of a container. + +- Create GPG and SSH keys without side effects +- Call real binaries in shellouts + +## Requirements + +- Docker + +## Usage + +### Build the Test Container + +*Run in the project root:* + +```bash +docker build --tag gitui:sign-test --file Dockerfile.sign-test . +``` + +The image is around ~1GB. + +It can be pushed in CI to leverage layer caching from the registry. + +### Run the Tests + +```bash +docker run --rm gitui:sign-test cargo test --workspace +``` + +The `CARGO_HOME` can be cached in CI to a known location in order to save compile time within the container. +See [Caching the Cargo home in CI](https://doc.rust-lang.org/cargo/guide/cargo-home.html#caching-the-cargo-home-in-ci) +on the official documentation. From 374293a062e6c2eaf5742131ecb2afe7b9c44699 Mon Sep 17 00:00:00 2001 From: Utkarsh Gupta Date: Fri, 17 Mar 2023 22:13:45 +0530 Subject: [PATCH 04/13] add workaround for amending signed commits --- asyncgit/src/error.rs | 4 ++++ asyncgit/src/sync/commit.rs | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index fe1289b2b6..a3d939a475 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -91,6 +91,10 @@ pub enum Error { /// #[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, } /// diff --git a/asyncgit/src/sync/commit.rs b/asyncgit/src/sync/commit.rs index 6994d5c800..1b9c706de1 100644 --- a/asyncgit/src/sync/commit.rs +++ b/asyncgit/src/sync/commit.rs @@ -1,7 +1,7 @@ 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::{ErrorCode, ObjectType, Repository, Signature}; @@ -16,12 +16,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, From e92da3d37c581e99b1f0348a25381d98f3bf31c0 Mon Sep 17 00:00:00 2001 From: Utkarsh Gupta Date: Fri, 17 Mar 2023 22:16:43 +0530 Subject: [PATCH 05/13] add workaround for rewording signed commits --- asyncgit/src/error.rs | 8 ++++++++ asyncgit/src/sync/reword.rs | 28 +++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index a3d939a475..bad185a41a 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -95,6 +95,14 @@ pub enum Error { /// #[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, } /// diff --git a/asyncgit/src/sync/reword.rs b/asyncgit/src/sync/reword.rs index aa1786fc86..c20d252b2e 100644 --- a/asyncgit/src/sync/reword.rs +++ b/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 { 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) { From 2087128b9af632efcab9a3d3783e32bb278473bd Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Wed, 21 Feb 2024 10:25:18 +0100 Subject: [PATCH 06/13] add missing err comment --- asyncgit/src/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index bad185a41a..3dba480b6d 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -85,6 +85,7 @@ pub enum Error { #[error("git hook error: {0}")] Hooks(#[from] git2_hooks::HooksError), + /// #[error("sign builder error: {0}")] SignBuilder(#[from] crate::sync::sign::SignBuilderError), From bf81abd5ea1a8f271c277af91bf26134b2197aa4 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Wed, 21 Feb 2024 10:58:36 +0100 Subject: [PATCH 07/13] remove signing methods switch --- asyncgit/src/sync/sign.rs | 125 ++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 74 deletions(-) diff --git a/asyncgit/src/sync/sign.rs b/asyncgit/src/sync/sign.rs index 320a18c2c9..ccb52d7a8c 100644 --- a/asyncgit/src/sync/sign.rs +++ b/asyncgit/src/sync/sign.rs @@ -95,82 +95,59 @@ impl SignBuilder { repo: &git2::Repository, config: &git2::Config, ) -> Result { - let signing_methods = config - .get_string("gitui.signing_methods") - .unwrap_or_else(|_| "shellouts".to_string()); - - match signing_methods.as_str() { - "shellouts" => { - 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 '. - // 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)), - } + 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 '. + // https://git-scm.com/docs/git-config#Documentation/git-config.txt-usersigningKey + let signing_key = config + .get_string("user.signingKey") + .or_else( + |_| -> Result { + 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, + }) } - "rust" => Err(SignBuilderError::MethodNotImplemented( - String::from(""), + "x509" => Err(SignBuilderError::MethodNotImplemented( + String::from("x509"), )), - _ => { - Err(SignBuilderError::InvalidFormat(signing_methods)) - } + "ssh" => Err(SignBuilderError::MethodNotImplemented( + String::from("ssh"), + )), + _ => Err(SignBuilderError::InvalidFormat(format)), } } } From 29cea7a9d0816ca9b87e4a7b4aaab40a5a5c3c3f Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Sat, 23 Mar 2024 19:39:48 +0100 Subject: [PATCH 08/13] remove testing files these will be tracked in another issue --- .dockerignore | 1 - Dockerfile.sign-test | 4 ---- SIGN_TEST.md | 34 ---------------------------------- 3 files changed, 39 deletions(-) delete mode 100644 .dockerignore delete mode 100644 Dockerfile.sign-test delete mode 100644 SIGN_TEST.md diff --git a/.dockerignore b/.dockerignore deleted file mode 100644 index 2f7896d1d1..0000000000 --- a/.dockerignore +++ /dev/null @@ -1 +0,0 @@ -target/ diff --git a/Dockerfile.sign-test b/Dockerfile.sign-test deleted file mode 100644 index 1735a3b69e..0000000000 --- a/Dockerfile.sign-test +++ /dev/null @@ -1,4 +0,0 @@ -FROM rust:slim -ENV CARGO_REGISTRIES_CRATES_IO_PROTOCOL=sparse -RUN apt-get update && apt-get install --yes python -COPY . . diff --git a/SIGN_TEST.md b/SIGN_TEST.md deleted file mode 100644 index 9f79b74dc0..0000000000 --- a/SIGN_TEST.md +++ /dev/null @@ -1,34 +0,0 @@ -# Git Commit Signing Tests - -Test commit signing in the isolation of a container. - -- Create GPG and SSH keys without side effects -- Call real binaries in shellouts - -## Requirements - -- Docker - -## Usage - -### Build the Test Container - -*Run in the project root:* - -```bash -docker build --tag gitui:sign-test --file Dockerfile.sign-test . -``` - -The image is around ~1GB. - -It can be pushed in CI to leverage layer caching from the registry. - -### Run the Tests - -```bash -docker run --rm gitui:sign-test cargo test --workspace -``` - -The `CARGO_HOME` can be cached in CI to a known location in order to save compile time within the container. -See [Caching the Cargo home in CI](https://doc.rust-lang.org/cargo/guide/cargo-home.html#caching-the-cargo-home-in-ci) -on the official documentation. From 7d817380e14a1e52d1309c80c3ca750302c785e3 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Sat, 23 Mar 2024 20:26:19 +0100 Subject: [PATCH 09/13] sign the bytes of the commit buffer instead of utf-8 --- asyncgit/src/sync/commit.rs | 2 +- asyncgit/src/sync/sign.rs | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/asyncgit/src/sync/commit.rs b/asyncgit/src/sync/commit.rs index 3b3bfd1bc4..85a5b39db3 100644 --- a/asyncgit/src/sync/commit.rs +++ b/asyncgit/src/sync/commit.rs @@ -117,7 +117,7 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result { })?; let sign = SignBuilder::from_gitconfig(&repo, &config)?; - let signed_commit = sign.sign(commit)?; + let signed_commit = sign.sign(&buffer)?; let commit_id = repo.commit_signed(commit, &signed_commit, None)?; diff --git a/asyncgit/src/sync/sign.rs b/asyncgit/src/sync/sign.rs index ccb52d7a8c..2717df002e 100644 --- a/asyncgit/src/sync/sign.rs +++ b/asyncgit/src/sync/sign.rs @@ -51,15 +51,13 @@ pub trait Sign { /// /// Retrieve an implementation using [`SignBuilder::from_gitconfig`]. /// - /// The `commit` string slice can be created using the following steps: + /// The `commit` buffer can be created using the following steps: /// - create a buffer using [`git2::Repository::commit_create_buffer`] - /// - convert the buffer using [`std::str::from_utf8`] - /// - the resulting string slice can be passed to [`Sign::sign`] now /// /// The returned `String` from this function 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: &str) -> Result; + fn sign(&self, commit: &[u8]) -> Result; #[cfg(test)] fn program(&self) -> &String; @@ -169,7 +167,7 @@ impl GPGSign { } impl Sign for GPGSign { - fn sign(&self, commit: &str) -> Result { + fn sign(&self, commit: &[u8]) -> Result { use std::io::Write; use std::process::{Command, Stdio}; @@ -189,7 +187,7 @@ impl Sign for GPGSign { let mut stdin = child.stdin.take().ok_or(SignError::Stdin)?; - write!(stdin, "{commit}") + stdin.write_all(commit) .map_err(|e| SignError::WriteBuffer(e.to_string()))?; drop(stdin); // close stdin to not block indefinitely From 618f208b1f0ae9624caec7e43453711c950a4419 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Sat, 23 Mar 2024 20:28:30 +0100 Subject: [PATCH 10/13] changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c2a64b68b..d2a927f740 100644 --- a/CHANGELOG.md +++ b/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 From a27c7ef597923b4b5702900371b0effe018ab774 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Sat, 23 Mar 2024 20:31:10 +0100 Subject: [PATCH 11/13] format code --- asyncgit/src/sync/sign.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/asyncgit/src/sync/sign.rs b/asyncgit/src/sync/sign.rs index 2717df002e..32fc492265 100644 --- a/asyncgit/src/sync/sign.rs +++ b/asyncgit/src/sync/sign.rs @@ -187,7 +187,8 @@ impl Sign for GPGSign { let mut stdin = child.stdin.take().ok_or(SignError::Stdin)?; - stdin.write_all(commit) + stdin + .write_all(commit) .map_err(|e| SignError::WriteBuffer(e.to_string()))?; drop(stdin); // close stdin to not block indefinitely From 3a264d5ab7cf4124a2fa3229f7de34fd25cf0e63 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Sun, 24 Mar 2024 20:04:37 +0100 Subject: [PATCH 12/13] support signing initial commit --- asyncgit/src/sync/commit.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/asyncgit/src/sync/commit.rs b/asyncgit/src/sync/commit.rs index 85a5b39db3..305c129fbc 100644 --- a/asyncgit/src/sync/commit.rs +++ b/asyncgit/src/sync/commit.rs @@ -123,7 +123,20 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result { // manually advance to the new commit ID // repo.commit does that on its own, repo.commit_signed does not - repo.head()?.set_target(commit_id, msg)?; + // 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 { From a8bb4bb611bf05bd82b620769d3bfd4fffa1c5ab Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Sun, 24 Mar 2024 20:12:01 +0100 Subject: [PATCH 13/13] return both signature and signature_field value from sign --- asyncgit/src/sync/commit.rs | 9 ++++++--- asyncgit/src/sync/sign.rs | 15 +++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/asyncgit/src/sync/commit.rs b/asyncgit/src/sync/commit.rs index 305c129fbc..c19d69dd92 100644 --- a/asyncgit/src/sync/commit.rs +++ b/asyncgit/src/sync/commit.rs @@ -117,9 +117,12 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result { })?; let sign = SignBuilder::from_gitconfig(&repo, &config)?; - let signed_commit = sign.sign(&buffer)?; - let commit_id = - repo.commit_signed(commit, &signed_commit, None)?; + 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 diff --git a/asyncgit/src/sync/sign.rs b/asyncgit/src/sync/sign.rs index 32fc492265..d4aa31a62c 100644 --- a/asyncgit/src/sync/sign.rs +++ b/asyncgit/src/sync/sign.rs @@ -54,10 +54,14 @@ pub trait Sign { /// The `commit` buffer can be created using the following steps: /// - create a buffer using [`git2::Repository::commit_create_buffer`] /// - /// The returned `String` from this function can then be passed into [`git2::Repository::commit_signed`]. + /// 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; + fn sign( + &self, + commit: &[u8], + ) -> Result<(String, String), SignError>; #[cfg(test)] fn program(&self) -> &String; @@ -167,7 +171,10 @@ impl GPGSign { } impl Sign for GPGSign { - fn sign(&self, commit: &[u8]) -> Result { + fn sign( + &self, + commit: &[u8], + ) -> Result<(String, String), SignError> { use std::io::Write; use std::process::{Command, Stdio}; @@ -217,7 +224,7 @@ impl Sign for GPGSign { let signed_commit = std::str::from_utf8(&output.stdout) .map_err(|e| SignError::Shellout(e.to_string()))?; - Ok(signed_commit.to_string()) + Ok((signed_commit.to_string(), "gpgsig".to_string())) } #[cfg(test)]