Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ssh-agent and external sign binaries for commit signing with ssh #2464

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chirpcel
Copy link

@chirpcel chirpcel commented Jan 4, 2025

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 chirpcel force-pushed the feature/external-ssh-signer branch from 14f71cc to ebd8fc1 Compare January 4, 2025 22:22
@chirpcel chirpcel marked this pull request as ready for review January 4, 2025 22:23
@chirpcel
Copy link
Author

chirpcel commented Jan 4, 2025

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
Copy link
Owner

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
Copy link
Author

chirpcel commented Jan 6, 2025

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
Copy link
Author

chirpcel commented Jan 6, 2025

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ssh signing] support custom signing program
2 participants