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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if SSH_AUTH_SOCK env var is set before trying to use the SSH agent. #987

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lapfelix
Copy link
Contributor

Fixes the cloning test through ssh (GCSingleCommitRepositoryTests's testClone) on computers that don't have a correctly setup ssh agent, like mine 馃ゴ (as the keys in ~/.ssh will be used instead, and that's going to work as long as they're not protected by a passphrase).

If SSH_AUTH_SOCK isn't there, it will fail later on anyway when libgit2 uses libssh2.

Should also make ssh agent workarounds like this one unnecessary: #161 (comment)

I AGREE TO THE GITUP CONTRIBUTOR LICENSE AGREEMENT

Fixes the ssh cloning test on computers that don't have a correctly setup ssh agent (as the keys in `~/.ssh` will be used instead)
@lapfelix lapfelix changed the title Check if SSH_AUTH_SOCK is set before trying to use the SSH agent. Check if SSH_AUTH_SOCK env var is set before trying to use the SSH agent. Apr 13, 2024
@lucasderraugh
Copy link
Collaborator

@lapfelix Being relatively unfamiliar with this portion of the codebase, does libssh2 not support accessing passphrased sah keys?

@lapfelix
Copy link
Contributor Author

hmm it would definitely be possible to support passphrased ssh keys, and GitUp could save the passphrase in the keychain.

(In theory though the ssh agent should "just work" but in practice, I'm not sure why it's not super reliable and why it sometimes needs to be kickstarted after a reboot)

@lucasderraugh
Copy link
Collaborator

I guess I'm trying to figure out what this particular PR is trying to improve. Wouldn't this environment variable be set as long as ssh-agent was running? Is this just meant to short circuit the case where it isn't?

@lapfelix
Copy link
Contributor Author

Yes basically I didn't find any straightforward way to retry calling the _CredentialsCallback, but if the ssh-agent isn't running, the environment variable won't be set, so we don't even need to try the ssh-agent, we know it'll fail.

This PR helps cases where the ssh agent isn't running, but maybe there's a more "correct" solution to the problem. Like I wonder when line 455 ever executed, for instance

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

Successfully merging this pull request may close these issues.

None yet

2 participants