Skip to content

Commit

Permalink
Fix bug with ssh signing for literal ssh keys
Browse files Browse the repository at this point in the history
There was a bug that was preventing Git Patch Stack from successfully
signing commits/tags when the Git config is set up with a literal public
ssh key as the signing key in the configuration. This happens to also be
the configuration that 1Password uses with Git when signing commits with
1Password.

The reason it was failing was because the temporary directory,
containing the temporary file, containing the literal ssh public key was
being deleted prior to running the signing command. This of course
caused the signing command to execute out with an error. The deletion of
the temp directory is tied to the destruction of the TempDir object.

Therefore, to address this I moved the TempDir object up to a higher
scope that would allow it to live long enough for the signing command to
happen successfully.

[changelog]
fixed: ssh signing with literal ssh public key configured

<!-- ps-id: 3abe6d7a-5cc2-4b3a-831b-15b3cee19c9d -->
  • Loading branch information
drewdeponte committed May 11, 2024
1 parent 1ad5044 commit cfc00f4
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/ps/private/git/signers/ssh_signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ fn ssh_sign_string(
) -> Result<String, SshSignStringError> {
let prog = program.unwrap_or("ssh-keygen".to_string());
let mut is_literal_key = false;
let dir = tempdir().map_err(|e| SshSignStringError::Unhandled(e.into()))?;

let ssh_key_path = match literal_ssh_key(&signing_key) {
Some(ssh_key_content) => {
is_literal_key = true;

// create a temporary directory & possibly a temporary file to hold the sigining key
// for use with the ssh-keygen command
let dir = tempdir().map_err(|e| SshSignStringError::Unhandled(e.into()))?;
let tmp_ssh_key_path = dir.path().join(".tmp_signing_key");
let mut file = File::create(tmp_ssh_key_path.as_path())
.map_err(|e| SshSignStringError::Unhandled(e.into()))?;
Expand Down

2 comments on commit cfc00f4

@alondahari
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drewdeponte oops. Should probably move the comment above as well.

@drewdeponte
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this @alondahari . I addressed it in commit 3934708.

Please sign in to comment.