Issue with 1password SSH commit signing #290

alondahari · 2024-01-26T15:31:15Z

After setting up SSH signing with 1password, trying to integrate or request a review on a patch results in an error:

Seems like the issue comes from the fact that the 1password setup agent is adding the public key directly to user.signingkey, not the path to the key. What's more, I think we need to use the 1password binary to sign the commit, which they set on gpg.ssh.program.

We should also fix the unwrap in cherry_picking.rs to bubble up the issue instead of causing a panic if anything goes wrong with the signing process. We should probably print out a warning message and proceed without signing instead.

The text was updated successfully, but these errors were encountered:

alondahari · 2024-01-26T15:41:00Z

Happy to take this on if you verify this issue @drewdeponte

The program option in git config can live under [`gpg.<format>.program`](https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgltformatgtprogram) in gitconfig, so we would need to respect that. I moved fetching the program option to inside the format option match, so we can use that to namespace the call to get the program configuration, falling back to the legacy `gpg.program` config. This small refactor is done since the ssh branch should also have an optional custom program to run (the 1Password binary for example) and we would want to follow the same general path with both methods. #290

When a signing program is defined in the gitconfig, respect that and use it instead of openssh. This will make using 1password ssh integration possible, since it's using a custom binary. #290

drewdeponte · 2024-02-08T19:25:22Z

@alondahari the proper thing for us to do here is to switch to external execution of whatever the configured command is. The same way that git proper does. So we may need to reference git proper.

This will allow us to remove the ssh signing dependencies which is good but will also give us the flexibility to behave the same way that Git proper does.

The program option in git config can live under [`gpg.<format>.program`](https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgltformatgtprogram) in gitconfig, so we would need to respect that. I moved fetching the program option to inside the format option match, so we can use that to namespace the call to get the program configuration, falling back to the legacy `gpg.program` config. This small refactor is done since the ssh branch should also have an optional custom program to run (the 1Password binary for example) and we would want to follow the same general path with both methods. #290

If the user's config specifies a program to use to sign the commits, use that over the default openssh. This allows users to use custom binaries even when the desired format is SSH. This is necessary to allow the 1Password SSH integration. #290

The program option in git config can live under [`gpg.<format>.program`](https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgltformatgtprogram) in gitconfig, so we would need to respect that. I moved fetching the program option to inside the format option match, so we can use that to namespace the call to get the program configuration, falling back to the legacy `gpg.program` config. This small refactor is done since the ssh branch should also have an optional custom program to run (the 1Password binary for example) and we would want to follow the same general path with both methods. #290

Add a function that returns a signing key if it is a literal value in gitconfig, or None if it's not (would be a path). This is done since with SSH signing, the user.signingKey in gitconfig can either be a path to a file with the key, or a literal key (like with gpg). See: https://git-scm.com/docs/git-config#Documentation/git-config.txt-usersigningKey #290

Update the ssh signer methods to support both literal and path signing strings in gitconfig. #290

We will use the tempfile crate to create a tempfile to be used when signing commits with SSH, so we will need to use this dependency outside of the test utils. #290

The program option in git config can live under [`gpg.<format>.program`](https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgltformatgtprogram) in gitconfig, so we would need to respect that. I moved fetching the program option to inside the format option match, so we can use that to namespace the call to get the program configuration, falling back to the legacy `gpg.program` config. This small refactor is done since the ssh branch should also have an optional custom program to run (the 1Password binary for example) and we would want to follow the same general path with both methods. #290

Getting the signing key from the config should be identical no matter what the format is, so take it out of the match. This is done in a general effort to improve the readability and maintainability of this piece of code. #290

When signing commits with SSH, we need to use the program in gitconfig if specified. Otherwise, fallback to ssh-keygen. This aligns with how git proper is doing it. #290

Add a function that returns a signing key if it is a literal value in gitconfig, or None if it's not (would be a path). This is done since with SSH signing, the user.signingKey in gitconfig can either be a path to a file with the key, or a literal key (like with gpg). See: https://git-scm.com/docs/git-config#Documentation/git-config.txt-usersigningKey #290

Add a function to create a temp file containing a ssh key if one is supplied literally in the gitconfig. The function accepts a path, since it will be in a temp dir that we need to live long enough to complete the signing. #290

We will use the tempfile crate to create a tempfile to be used when signing commits with SSH, so we will need to use this dependency outside of the test utils. #290 [changelog] added: tempfile crate dependency

The program option in git config can live under [`gpg.<format>.program`](https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgltformatgtprogram) in gitconfig, so we would need to respect that. I moved fetching the program option to inside the format option match, so we can use that to namespace the call to get the program configuration, falling back to the legacy `gpg.program` config. This small refactor is done since the ssh branch should also have an optional custom program to run (the 1Password binary for example) and we would want to follow the same general path with both methods. #290

Getting the signing key from the config should be identical no matter what the format is, so take it out of the match. This is done in a general effort to improve the readability and maintainability of this piece of code. #290

When signing commits with SSH, we need to use the program in gitconfig if specified. Otherwise, fallback to ssh-keygen. This aligns with how git proper is doing it. #290 [changelog] updated: ssh commit signing respects literal keys in config updated: ssh commit signing respects custom program updated: default ssh commit signing uses ssh-keygen

The ss-key dependency is no longer used, since ssh-keygen is used instead. #290 [changelog] removed: ssh-key crate dependency

Add a function that returns a signing key if it is a literal value in gitconfig, or None if it's not (would be a path). This is done since with SSH signing, the user.signingKey in gitconfig can either be a path to a file with the key, or a literal key (like with gpg). See: https://git-scm.com/docs/git-config#Documentation/git-config.txt-usersigningKey #290

Add a function to create a temp file containing a ssh key if one is supplied literally in the gitconfig. The function accepts a path, since it will be in a temp dir that we need to live long enough to complete the signing. #290

We will use the tempfile crate to create a tempfile to be used when signing commits with SSH, so we will need to use this dependency outside of the test utils. #290 [changelog] added: tempfile crate dependency

The program option in git config can live under [`gpg.<format>.program`](https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgltformatgtprogram) in gitconfig, so we would need to respect that. I moved fetching the program option to inside the format option match, so we can use that to namespace the call to get the program configuration, falling back to the legacy `gpg.program` config. This small refactor is done since the ssh branch should also have an optional custom program to run (the 1Password binary for example) and we would want to follow the same general path with both methods. #290

Getting the signing key from the config should be identical no matter what the format is, so take it out of the match. This is done in a general effort to improve the readability and maintainability of this piece of code. #290

When signing commits with SSH, we need to use the program in gitconfig if specified. Otherwise, fallback to ssh-keygen. This aligns with how git proper is doing it. #290 [changelog] updated: ssh commit signing respects literal keys in config updated: ssh commit signing respects custom program updated: default ssh commit signing uses ssh-keygen

The ss-key dependency is no longer used, since ssh-keygen is used instead. #290 [changelog] removed: ssh-key crate dependency

Following how git does things, we will default to openpgp if the signing format is not defined in gitconfig #290

edgarjs · 2024-04-25T20:37:14Z

Hey! Hope you're doing well. I was having this same issue after switching to 1Password. Is there any ETA on the next release that includes the fix?

drewdeponte · 2024-04-25T21:36:26Z

@edgarjs if you build from source what is in the main branch currently has the fix. I was waiting for people to do more testing. But, that doesn't seem to be happening.

So, I will try and cut a release tonight or tomorrow and we can just go from there.

edgarjs · 2024-04-25T23:52:13Z

@drewdeponte I tried that, but I get this error now (the blurred part is an array of numbers):

drewdeponte · 2024-05-11T04:09:00Z

@edgarjs @alondahari this issue has been actually resolved now in the latest release, v7.1.1.

alondahari added a commit that referenced this issue Feb 9, 2024

Update ssh signer

b56a103

Update the ssh signer methods to support both literal and path signing strings in gitconfig. #290

alondahari added a commit that referenced this issue Feb 9, 2024

Remove ssh crate dependency

e7be5ce

The ss-key dependency is no longer used, since ssh-keygen is used instead. #290 [changelog] removed: ssh-key crate dependency

alondahari mentioned this issue Feb 9, 2024

Fix op ssh signing #292

Merged

alondahari added a commit that referenced this issue Feb 12, 2024

Remove ssh crate dependency

936c746

The ss-key dependency is no longer used, since ssh-keygen is used instead. #290 [changelog] removed: ssh-key crate dependency

alondahari added a commit that referenced this issue Feb 12, 2024

Default to openpgp format

19a85af

Following how git does things, we will default to openpgp if the signing format is not defined in gitconfig #290

alondahari closed this as completed Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue with 1password SSH commit signing #290

Issue with 1password SSH commit signing #290

alondahari commented Jan 26, 2024

alondahari commented Jan 26, 2024

drewdeponte commented Feb 8, 2024

edgarjs commented Apr 25, 2024

drewdeponte commented Apr 25, 2024

edgarjs commented Apr 25, 2024

drewdeponte commented May 11, 2024

Issue with 1password SSH commit signing #290

Issue with 1password SSH commit signing #290

Comments

alondahari commented Jan 26, 2024

alondahari commented Jan 26, 2024

drewdeponte commented Feb 8, 2024

edgarjs commented Apr 25, 2024

drewdeponte commented Apr 25, 2024

edgarjs commented Apr 25, 2024

drewdeponte commented May 11, 2024