-
Notifications
You must be signed in to change notification settings - Fork 2
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
Explicitly set the IdentityAgent path when using the issue
mode
#23
Explicitly set the IdentityAgent path when using the issue
mode
#23
Conversation
a39657e
to
13d6008
Compare
main.go
Outdated
@@ -111,6 +111,7 @@ func processCommand() int { | |||
// override default ssh-agent socket | |||
os.Setenv("SSH_AUTH_SOCK", agent.SocketFile()) | |||
log.Debugf("set SSH_AUTH_SOCK to %q\n", agent.SocketFile()) | |||
sshClient.PrependArgs([]string{"-o", fmt.Sprintf("IdentityAgent=%s", agent.SocketFile())}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think there's value in continuing to set the SSH_AUTH_SOCK
environment variable? At first glance, this would appear to complete negate the need to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I had the same though but then I remembered that -o ProxyCommand=...
exists and something might rely on it. I did not find any disadvantages to continue to set SSH_AUTH_SOCK
so I kept it just in case.
I'm fine either way to be honest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's go with the minimalist DRY approach, and separately I'll look at adding some magic to avoid prepending the additional argument when it's not required:
sshClient.PrependArgs([]string{"-o", fmt.Sprintf("IdentityAgent=%s", agent.SocketFile())}) | |
sshClient.PrependArgs([]string{"-o", "IdentityAgent=SSH_AUTH_SOCK"}) |
13d6008
to
33e4b22
Compare
This make sure the appropriate path is used even if the IdentityAgent is set in a config. Without this change a user with the following config will not be able to connect because the SSH_AUTH_SOCK env variable will be overriden: ``` Host * IdentityAgent none IdentityFile ~/.ssh/id_ecdsa_sk ```
33e4b22
to
e98397c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This make sure the appropriate path is used even if the IdentityAgent is set in a config.
Without this change a user with the following config will not be able to connect because the SSH_AUTH_SOCK env variable will be overriden: