Support ssh-agent and external sign binaries for commit signing with ssh by chirpcel · Pull Request #2464 · gitui-org/gitui

chirpcel · 2025-01-04T00:21:36Z

This Pull Request fixes/closes #2188.

It changes the following:

Respect configuration for alternative signing binary (for example 1Password ssh agent)
Using ssh-keygen for ssh signing operation to support ssh-agent
- additional: sharing same implementation for default & configured binary

I followed the checklist:

I added unittests
I ran make check without errors
I tested the overall application
I added an appropriate item to the changelog

chirpcel · 2025-01-04T22:28:26Z

There's one drawback. Like before, we don't support encrypted private keys which are not loaded to agent. The error message isn't as clear as before, but from a functionality point of view it isn't different. I would make a new issue for this behavior and take a look into supporting encrypted private keys.

extrawurst · 2025-01-05T21:08:35Z

The error message isn't as clear as before

cant we make it as clear as before and then followup

supporting encrypted private keys.

chirpcel · 2025-01-06T07:15:40Z

Actually I didn't figured out how to capture the passphrase prompt, as it's spawned directly on the tty.
But I just have an Idea, maybe we could provide an empty passphrase as arg to skip the password prompt and map the "wrong passphrase" error to the previous error message.
I'll take a look at it today.

chirpcel · 2025-01-06T15:14:49Z

@extrawurst Worked, we now have a meaningful error message. I tested with 1Password SSH-Agent, Default SSH-Agent, non encrypted keys on disk and encrypted keys on disk.

DaRacci · 2025-02-22T03:27:28Z

Any movement on this?

This would allow me to use gitui without needing to fallback to shell to actually commit.

chirpcel · 2025-02-22T19:49:35Z

@DaRacci Think maintainer is currently busy as asking for co-maintainers and migrating his repo to an org for better support by more people.
For now you can compile yourself with applied pr as I do or wait.

naseschwarz

Hey @chirpcel. Thank you for this implementation. I support this fully and agree that this should be the way commit signing is implemented at the moment. It is almost in compliance with git commit's implementation. Where changes will be required for currently unsupported git commit signing features, I have remarked them in this review. I wouldn't say that these are blockers, though.

I also have minor questions and nitpicks, mostly about corner cases, here and there and some FYIs.

On which platforms have you tested this? I have tested it on Linux and it works as promised. It would be great if someone tested this on Windows and Mac, too, though. (A test should consist of at least signing a commit and checking whether that commit's signature can be validated.)

CHANGELOG.md

asyncgit/src/sync/sign.rs

chirpcel · 2025-03-29T23:10:54Z

Thanks @naseschwarz for your cr! I commited your suggestions and commented on questions. I'll take a look at the remaining annotations of edge cases/optimizations. I'll update the pr soon.

I've tested the implementation on linux and macOS. Windows currently missing, maybe I can test it in a VM. Currently don't know about license/activation. Keep you updated.

hazzuk · 2025-11-18T00:26:49Z

Would simply like to add that I've been been running a compiled version of gitui with this pull request since August. So far I've had no issues on my Linux setup.

For me, ssh-agent support was critical, so I'd also like to say thank you for this work. I hope we see this merged one day.

chirpcel · 2025-11-19T18:24:00Z

@hazzuk Me too and I totally forgot that I have this PR open 😶‍🌫️
You just bring back my attention to it. I'll sort out the last open notes and the we should get it merged.

chirpcel · 2025-11-19T20:05:39Z

Open notes should now been solved.
I'll test tomorrow on some machines with different configs and then request an follow-up review.

chirpcel · 2025-11-20T20:23:21Z

I didn't encounter problems on my machines. Tested on linux with openssh agent and pgp based ssh agent.

asgeirn · 2025-11-21T07:03:10Z

Tested on MacOS using 1Password signing, worked perfectly!

chirpcel · 2025-11-21T18:16:16Z

@extrawurst
As this initial PR is some months old and it happened much since them, who is currently able to do a review on changes?

cruessler · 2025-11-22T18:13:00Z

I’ll have a look!

cruessler · 2025-11-28T07:20:16Z

(Just wanted to let you know that I haven’t forgotten this PR. I hope to be getting to it over the weekend.)

cruessler

First of all, sorry for the long delay! (I wanted to make sure I had the time and focus needed for an actual review, and that took some time, unfortunately.)

Overall, this looks good to me, as far as I can tell. I just have a few questions.

cruessler · 2025-12-04T13:56:05Z

asyncgit/src/sync/sign.rs

-				Ok(signer)
+					.map(|value| {
+						if value.starts_with("key::") {
+							value.replacen("key::", "", 1)


What do you think about value.strip_prefix("key::") instead (seems more idiomatic)?

cruessler · 2025-12-27T16:06:45Z

asyncgit/src/sync/sign.rs

+						named_temp_file.path().display()
+					);
+					pub_key_temp_file = Some(named_temp_file);
+				}


What do you think about changing to something like the following:

let (signing_key, pub_key_temp_file) = { … };

This could make the data flow easier to follow.

cruessler · 2025-12-27T16:07:25Z

asyncgit/src/sync/sign.rs

+					.get_string("gpg.ssh.program")
+					.unwrap_or_else(|_| "ssh-keygen".to_string());
+
+				let mut signing_key = config


What do you think about renaming to signing_key_path since that’s the field name in SSHSign?

cruessler · 2025-12-27T16:08:43Z

asyncgit/src/sync/sign.rs

 		}
 	}
+
+	fn signing_key_into_temp_file(


What do you think about prefixing this with a verb so that it becomes write_signing_key_to_temp_file (or something similar)?

cruessler · 2025-12-27T16:09:43Z

asyncgit/src/sync/sign.rs

-		}
-	}
+	signing_key_path: String,
+	_pub_key_temp_file: Option<NamedTempFile>,


Why is this prefixed with _? Does this warrant a comment?

chirpcel mentioned this pull request Jan 4, 2025

[ssh signing] support custom signing program #2188

Open

chirpcel force-pushed the feature/external-ssh-signer branch from 14f71cc to ebd8fc1 Compare January 4, 2025 22:22

chirpcel marked this pull request as ready for review January 4, 2025 22:23

chirpcel force-pushed the feature/external-ssh-signer branch 2 times, most recently from 19f0891 to 004b135 Compare January 15, 2025 20:17

naseschwarz self-requested a review March 20, 2025 15:39

naseschwarz suggested changes Mar 22, 2025

View reviewed changes

naseschwarz added the needs_work label Mar 22, 2025

feat(ssh-agent-signing): implement commit signing with ssh-agent

fdb461b

chirpcel force-pushed the feature/external-ssh-signer branch from cb6934e to fdb461b Compare November 19, 2025 19:07

chirpcel added 3 commits November 19, 2025 20:44

chore(tmp): use NamedTempfile to handle lifetime

80960bd

fix(git): handle current key literal prefix

16a186a

chore(cleanup): switch to documented ssh-keygen agent forcing

7537147

chirpcel requested a review from naseschwarz November 20, 2025 20:21

extrawurst requested review from cruessler and removed request for naseschwarz November 21, 2025 18:25

cruessler requested changes Dec 27, 2025

View reviewed changes

Uh oh!

Comments

Conversation

chirpcel commented Jan 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chirpcel commented Jan 4, 2025

Uh oh!

extrawurst commented Jan 5, 2025

Uh oh!

chirpcel commented Jan 6, 2025

Uh oh!

chirpcel commented Jan 6, 2025

Uh oh!

DaRacci commented Feb 22, 2025

Uh oh!

chirpcel commented Feb 22, 2025

Uh oh!

naseschwarz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chirpcel commented Mar 29, 2025

Uh oh!

hazzuk commented Nov 18, 2025

Uh oh!

chirpcel commented Nov 19, 2025

Uh oh!

chirpcel commented Nov 19, 2025

Uh oh!

chirpcel commented Nov 20, 2025

Uh oh!

asgeirn commented Nov 21, 2025

Uh oh!

chirpcel commented Nov 21, 2025

Uh oh!

cruessler commented Nov 22, 2025

Uh oh!

cruessler commented Nov 28, 2025

Uh oh!

cruessler left a comment

Choose a reason for hiding this comment

Uh oh!

cruessler Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

cruessler Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

cruessler Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

cruessler Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

cruessler Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

chirpcel commented Jan 4, 2025 •

edited

Loading

naseschwarz left a comment •

edited

Loading