drewdeponte / git-ps-rs

Official git-ps Rust implementation - the future of git-ps
https://git-ps.sh
MIT License
78 stars 8 forks source link

Issue with 1password SSH commit signing #290

Closed alondahari closed 6 months ago

alondahari commented 7 months ago

After setting up SSH signing with 1password, trying to integrate or request a review on a patch results in an error: CleanShot 2024-01-26 at 15 20 19

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.

alondahari commented 7 months ago

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

drewdeponte commented 7 months ago

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

edgarjs commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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

CleanShot 2024-04-25 at 16 02 13@2x

drewdeponte commented 4 months ago

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