-
Notifications
You must be signed in to change notification settings - Fork 248
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
Security: move shims to end of PATH #247
Comments
Hmm, this goes counter to how all the *env tools work, and yeah there are conflicts with |
Translating pyenv to goenv gave us a relatively proven *env for Go, but it's pretty clear at this point that the way pyenv works has some issues.
For these reasons, it's worth thinking through how goenv should actually work vs pyenv. They are different use cases. @huyz In my own case, I removed /usr/bin/go when I started using goenv years ago -- too many opportunities for confusion if it's left in place. Removing the system Go would be my recommendation on Linux -- would that work on Mac as well? |
At least for me, I don't think that removing other So if I understand you right, the security risk of third parties dropping dangerously-named executables applies to all *env tools, but even more so with goenv because of how I think for me the solution is to keep the |
I agree it's not easy to override system tools unless they are later in the $PATH.
The general solution would be to go with a system of isolation such as containers or NixOS. Traditional operating systems, whether from commercial vendors or from open-source distributions, tend to make the assumption that the package manager manages all executables. If the vendor or distribution doesn't provide an isolation method, then any *env tool you add will always be a workaround and will always be fighting the package manager in some way.
More recent versions of
I mentioned in #249 that I wound up doing something similar -- I have a |
In the case of pyenv, python packages are less likely to install executables in the $PATH, and if they do it's ordinarily one file with some sort of unique name that is less likely to conflict with a well-known executable. I think this is to some extent cultural, but is also reinforced by the fact that setup.py has to explicitly name each executable -- it's not likely that a python package author will accidentally install an executable on others' machines. If a python package installs an executable on purpose it's easy to preview what will happen by looking at the contents of setup.py. |
Minor note: I just realized that sometimes there already is a |
I wonder if something that |
I happen to face the issue that after the suggested fix was merged and updated my goenv, when switching from project A which has go version 1.18.x to project B with version 1.19, then goenv would not detect the new go shim. If I moved the path to the beginning of the PATH, as it was already mentioned done by other *env tools, it works fine switching versions. |
@alexsapran That does not sound related to this change -- both 1.18 and 1.19 should be equally reachable via the shims. It sounds like you have a non-goenv-managed Go version somewhere in your PATH, or a .go-version file in project B's directory, or were working in a shell that had |
I thought that it was a non-goenv-managed indeed and made sure my env was clean, removed it. |
For any future folks who find this thread:
Which would you rather do:
You really, really want to do (B) -- installing |
I am just reporting my experience with trying to use goenv. I feel this discussion is not very productive. Happy to stop it here. |
@alexsapran My last message was not aimed at you -- I did say "future folks". I think you provided a fine example, and a good followup of your own resolution. Thank you! I am hoping to head off more duplicate issues being spawned like #279 and #289. I have been feeling compelled to respond to each of these incidents because even though I am not this project's owner, I wrote the PR that fixed the vulnerability. |
I'm not sure what's the take away here, don't use goenv? Also, is the problem that |
I don't know about other folks, but my takeaway in zsh is: # one time
mkdir -p $GOENV_ROOT/bin
ln -s ../shims/go{,fmt} $GOENV_ROOT/bin
# in zsh RC files
typeset -gU PATH
PATH=$GOENV_ROOT/bin:$PATH |
@mimoo There are two takeaways:
Goenv is fixed as far as (2) is concerned -- it no longer inserts itself at the head of PATH as of #248, and this issue was closed. The conversation since then is around how to manage things when you still have a system or hand-installed version of Go, despite (1). The suggestion from @huyz isn't bad and might work for others. If anyone wants to tackle automating that in goenv, I'd encourage you to submit a new issue and PR. |
Ah yeah I thought about it today, so removing golang ( |
Life must have happened and I dropped this and never ran into the problem myself -- we still have a vulnerability similar to #99. As @mattes mentions in #99:
Here's what that allows, for instance:
The offending code appears to be in these two places in goenv-init:
The text was updated successfully, but these errors were encountered: