search

dahlbyk / posh-sshell

PowerShell helpers for SSH (previously part of posh-git.)
MIT License
143 stars 8 forks source link

Allow overriding environment variable scope #15

Closed JeremySkinner closed 6 years ago

JeremySkinner commented 6 years ago

Allows changing the environment variable scope from Process to User/Machine. Also updated Stop-SshAgent and Get-SshAgent to work with win32 openssh.

JeremySkinner commented 6 years ago

Yes, I did commit the follow up but looks like I forgot to push it. Will do so when I'm back at my pc.

JeremySkinner commented 6 years ago

@dahlbyk @rkeithhill I've pushed up the missing change.

This doesn't change the environment variable scope for the calls to Get-TempEnv in the main module file, I don't think this is a big deal, but do we want to introduce a global variable to control this? My inclination would be no

dahlbyk commented 6 years ago

setenv doesn't understand your third parameter:

https://github.com/dahlbyk/posh-sshell/blob/edcb790de086f245a3a00f5fac2fc3247a0cd134/src/Utils.ps1#L3-L6

This doesn't change the environment variable scope for the calls to Get-TempEnv in the main module file, I don't think this is a big deal, but do we want to introduce a global variable to control this? My inclination would be no

Get-TempEnv is already explicitly user-scoped rather than process-scoped, and it doesn't make sense for it to be machine-scoped, so I think this is fine. In fact, it doesn't seem useful or correct to allow Start-SshAgent -Scope Machine either, as this could leak the current user's credentials to another user. Am I missing a legitimate scenario for that?

JeremySkinner commented 6 years ago

@dahlbyk Good point, I'll remove the option for machine scope and leave just process/user.

Not sure why my change to setenv isn't showing up in the PR, it shows in my local repo. I'll force push it up.

JeremySkinner commented 6 years ago

@dahlbyk force push seems to have done it, the changes are showing now and I've removed machine scope from the options list.