rubygems icon indicating copy to clipboard operation
rubygems copied to clipboard

Add a way to disable "auto-sudo"

Open deivid-rodriguez opened this issue 4 years ago • 32 comments

Describe the problem as clearly as you can

Currently bundler detects if it's lacking write permissions on the default GEM_HOME and automatically "upgrades privileges" if needed.

It turns out the main users running into these issues (OS ruby package users) don't actually want this behaviour. Instead, they usually prefer "downgrading" to a folder with less permissions than upgrading the running user's privileges.

Also, although this shouldn't influence what we do here and it should just be fixed, there's quite a few issues with the current "auto-sudo" implementation, as can be seen by searching existing issues.

So, we should consider providing some enhancements to this. Some ideas:

  • Automatically respect --user-install from the bundler side if configured.
  • Provide a way to opting in to the alternative behaviour of "downgrading the folder rather than upgrading the user".
  • Ask user when this problem first appear about the preferred behaviour and save it.

Note that rubygems currently does not try to "upgrade to sudo" and provides a way out by configuring gem: --user-install in your ~/.gemrc file. Also, note that we're considering making this behaviour the default if the default folder is not writable in #2847.

I'm particularly interested in @voxik, @terceiro, and @duckinator opionions.

deivid-rodriguez avatar Oct 23 '20 12:10 deivid-rodriguez

Agreed, I think using sudo for installing gems is almost always a mistake. I think most users want to just install wherever it's possible with current permissions, or they actually want system packages provided by the package manager.

I actually thought Bundler would install under $HOME if it cannot write to the GEM_DIR, but seems not.

A good alternative for Bundler is to bundle config set --local path vendor/bundle (which is unfortunately quite verbose, longer than the deprecated bundle install --path vendor/bundle). That won't share gems between applications though, which might or not be wanted.

eregon avatar Oct 23 '20 13:10 eregon

IMO installing to $HOME should be the default if the user has no write permissions to the global GEM_DIR. i.e. gem install or bundle install should by default install to $HOME if the user is not root. There are use cases for installing gems globally, and if you want that, just run gem install or bundle install as root.

There is one problem with user install, though: programs provided by the installed gems do not end up in $PATH without explicit user intervention (.bashrc).

terceiro avatar Oct 23 '20 15:10 terceiro

I realized that my opinion above is perhaps too strong, and that may be off-topic in the context of the current discussion (providing a way of opting out of "auto sudo"). I think making bundler respect the rubygems --user-install configuration is a good way of doing it.

terceiro avatar Oct 23 '20 15:10 terceiro

I almost exclusively use OS-provided ruby packages, and have definitely had problems with this.

I think having Bundler respect --user-install could go a long way towards making the out-of-the-box experience more user-friendly.


@terceiro in PR #2847 we're considering having RubyGems do --user-install if $GEM_HOME isn't writable. I also suggested adding a warning like pip does when the bin directory isn't in the PATH, an.

If that's all implemented and Bundler starts respecting RubyGems being configured to use --user-install, I believe it handles everything you mentioned in the least-frustrating way the RubyGems+Bundler team is capable of.

duckinator avatar Oct 23 '20 19:10 duckinator

@duckinator I think rubygems already warns when using --user-install and the bin directory isn't in the PATH?

It also makes sense to me that we start respecting rubygems' --user-install in bundler. But I guess it also make sense to implement the same thing as #2847 in bundler regardless of the --user-install setting/flag.

In addition to that, I was thinking of whether we should promote --user-install to a global configuration instead of a per-command flag? I think once you want --user-install for a command, you usually want it everywhere, so configurations like install: --user-install, don't make much sense since you'll also want it for gem uninstall and any other command that uses the default location of gems?

deivid-rodriguez avatar Oct 26 '20 12:10 deivid-rodriguez

  1. Bundler should always respect to RubyGems settings.
  2. If there was need to configured anything, then operating_system.rb should be the place. This is the place where distributions modifies the RubyGems settings. ~/.gemrc is out of OS control, there is not allowed to touch anything in home via package manager. Env variable is not easy to setup globally and ensure they are applied.
  3. I really don't want users to use sudo ever. Using sudo implies modification of system owned directories by third party tool and that is not good idea, unless one knows what (s)he is doing. There is quite a lot of companies, where you don't have root permissions on the system. No exceptions.

It turns out the main users running into these issues (OS ruby package users) don't actually want this behaviour. Instead, they usually prefer "downgrading" to a folder with less permissions than upgrading the running user's privileges.

OS ruby package users don't want to downgrade neither be asked for privileges. They want to install into location which is accessible. Please note that there is for example Fedora Silverblue, where the OS is image based and you essentially cannot write anywhere except your home.

* Ask user when this problem first appear about the preferred behavior and save it.

I don't think user should be queried. From OS POV, it just complicates everything. It is more code which might be in way to implement the directory layout we need.

And frankly, this probably address just very small part of Ruby users. Complete beginners would be confused anyway, for advanced users, it would be annoying.

voxik avatar Oct 26 '20 16:10 voxik

  1. It seems that all people here agree on respecting --user-install, so let's do it.
  2. My idea of an environment variable is that it can be used by OS packagers to customize the behaviour to best fit the users of "rubygems packaged by OS".
  3. Sure, this is what I want to provide with this ticket.

OS ruby package users don't want to downgrade neither be asked for privileges. They want to install into location which is accessible. Please note that there is for example Fedora Silverblue, where the OS is image based and you essentially cannot write anywhere except your home.

This is exactly what I'm suggesting and what I mean by "dowgrading". Really unfortunate term choice, but I mean, fallback to a folder where the user has write privileges if the default location is not writable.

I don't think user should be queried. From OS POV, it just complicates everything. It is more code which might be in way to implement the directory layout we need.

Ok, this was just me brainstorming, let's forget about that one.

deivid-rodriguez avatar Oct 26 '20 17:10 deivid-rodriguez

Personally I much prefer --user-install whenever possible. It makes it easier for me to figure out where downloaded gems end up, on different linux distributions (debian in particular confuses me in this regard, e. g. when /usr/local/ is suddenly used or /var/, whereas the ruby hierarchy is in /usr/; this is why I prefer stuff in the home directory usually, thus --user-install).

rubyFeedback avatar Oct 31 '20 01:10 rubyFeedback

You didn't read the original issue #4027, did you?

There's no way to make bundler respect --user-install unless it's with hacks, like:

--- a/bundler/lib/bundler/rubygems_integration.rb
+++ b/bundler/lib/bundler/rubygems_integration.rb
@@ -163,6 +163,8 @@ def sources
     end
 
     def gem_dir
+      user_install = Gem.configuration[:gem]&.split(' ')&.include?('--user-install')
+      return Gem.user_dir if user_install
       Gem.dir
     end

This is only for gem the same would have to be done for install, at least.

It would be much better to have a global configuration for this, but at what point are you going to load it, and what is it going to override?

I have tried all these approaches. There's no straightforward solution except what I proposed in pull #4028: an environment variable GEM_USER_INSTALL that turns on the desired behavior. It was closed with no discussion due to tone policing.

Has anyone actually tried to write a patch?

felipec avatar Nov 26 '20 20:11 felipec

It sounds to me like the solution is to have a single setting which can be overridden by operating_system.rb and an environment variable. The former would allow OSes to configure it, and the environment variable overriding operating_system.rb means users can still change their system however they want. If this uses Bundler's typical approach of also letting you set them in ~/.bundler/config, it should work in pretty much every scenario I can think of.

Does this sound right, @voxik @terceiro @deivid-rodriguez?

duckinator avatar Nov 27 '20 21:11 duckinator

This general issue is quite old, considering I found https://github.com/rubygems/bundler/issues/3006, which I found after discovering that bundler has been doing things on my system with sudo without asking. :angry:

kenyon avatar Dec 05 '20 08:12 kenyon

@kenyon yeah, this is an old problem, and definitely needs to be fixed. Separately from that — and I know this is unsolicited advice, so feel free to disregard it — I recommend disabling passwordless sudo, since it means any program or script can acquire root access. (EDIT, 2021-05-21: As kenyon mentioned, this suggestion doesn't actually solve the problem.)

@voxic @terceiro @deivid-rodriguez when y'all get a chance, could you provide input on my suggested approach? :slightly_smiling_face:

duckinator avatar Dec 07 '20 04:12 duckinator

@duckinator the problem would also occur with the default sudo configuration where the timeout is five minutes, if you happened to use sudo in the same session within the last five minutes. The command sudo should not be called in this codebase at all, except maybe in tests and documentation. In twenty years of system administration, this is the only program I've used that calls sudo like this. There is no need to try to be helpful, everybody knows you need to escalate privileges to do systemwide things. Just rip it out, please, that is the only possible fix.

Compare:

  • https://github.com/rubygems/rubygems/search?q=sudo
  • https://github.com/pypa/pip/search?q=sudo
  • https://github.com/npm/cli/search?q=sudo
  • https://salsa.debian.org/search?search=sudo&project_id=228&group_id=2051
  • https://github.com/rpm-software-management/dnf/search?q=sudo
  • https://github.com/rust-lang/cargo/search?q=sudo
  • https://github.com/composer/composer/search?q=sudo
  • https://github.com/haskell/cabal/search?q=sudo

kenyon avatar Dec 07 '20 05:12 kenyon

@duckinator one can already override the default installation directory in operating_system.rb by overriding Gem.default_dir; I just tested and that also makes bundler install to that location.

terceiro avatar Dec 07 '20 19:12 terceiro

I'm yet another victim of this bug. I ran "bundler install" using a regular account, expecting that it will install missing dependencies to a local folder or will fail to install into a system folder.

Instead, bunlder acted as a malicious software, escalated its privileges using passwordless (in my env) sudo quietly and installed a bunch of files including software with higher versions into my system folders next to (or overwriting?) existing packages. That broke my system ruby/rails setup.

Please do not do it by default!

blshkv avatar Jun 02 '21 02:06 blshkv

@deivid-rodriguez I think it's long past time to have Bundler stop using sudo. It's becoming pretty clear that while we try to figure out the best approach, we're just pissing people off because Bundler's romping through /usr making a mess of things. I think it's time we just make a decision and go with it.

My understanding of the situation is that this combination should solve most if not all of the sudo-related problems:

  1. Have RubyGems automatically use --user-install if the default gems directory is not writable (see #2847 "Make --user-install as default"))
  2. Have Bundler use RubyGems' installation directory unless explicitly overridden.
  3. As part of No.2, rip out all uses of sudo.

Does that seem correct to you? If so, I think the first step is to land #2847.

duckinator avatar Jun 02 '21 06:06 duckinator

Yeah, I tend to have the same impression. Nobody seems to want this feature, and it causes issues for people.

deivid-rodriguez avatar Jun 02 '21 08:06 deivid-rodriguez

All the people using it are on macOS, and they never mention it because they don't know it exists—it means Bundler just works for them and they don't open issues (anymore). We could probably limit this feature to macOS and everyone would be happy, though.

indirect avatar Jun 02 '21 08:06 indirect

That's very interesting, so MacOS comes with a sudo-less setup by default? I think by solving 1 and 2 mentioned by @duckinator wouldn't affect anyone new yet alleviate the issue for most people currently affected.

I'm really curious about what's going on with MacOS because we do get a constant flow of MacOS specific issues that I'm not able to address without a MacOS machine :disappointed:. Basically every time anybody opens an issue reading "Unfortunately, an unexpected error occurred, and Bundler cannot continue", it's a MacOS permissions specific issue, and it happens once every one or two weeks.

deivid-rodriguez avatar Jun 02 '21 08:06 deivid-rodriguez

I would think even on macOS installing gems in /usr is almost never (say 99.9%) what one wants. (If one wants to install in system dirs, they can of course sudo bundler install)

My feeling on macOS is:

  • Don't use the system Ruby, it's broken in several ways (e.g., universal binary itself but only compiles C extensions on only one of both archs), and it's hard to install gems for it. #2847 would help though by installing gems under the home and not globally (which is basically guaranteed mess and confusion).
  • If you build and installed Ruby, then you never want gems for it to be in non-user-writable places (e.g., /usr).

I think @duckinator's plan sounds great.

eregon avatar Jun 02 '21 09:06 eregon

I don't see how it would depend on OS unless you use macOS like a Windows where you download software from random untrusted locations and install it manually to whatever location. But it will get messy very quickly if you mix two package managers like homebrew and bundler (maybe bad example, just guessing) if they install software into the same location (/usr/lib64, /usr/bin)

blshkv avatar Jun 02 '21 09:06 blshkv

@indirect I feel like I might be missing some context — can you elaborate on why macOS is a special case, here? Wouldn't installing it in a user-writable location work the same as it does for anyone on Linux or *BSD?

duckinator avatar Jun 02 '21 14:06 duckinator

@duckinator sorry, I lost track and should have included that context!

macOS includes a ruby install with it. Gems for that ruby have historically been installed to a special location (iirc /System/Library/Frameworks/Ruby or something like that) with binstubs in (iirc) /usr/bin.

Many end users need a tool that happens to be written in ruby, but they are not ruby users or developers themselves. A good example of this is CocoaPods, for Apple developers who don’t want to know anything about ruby, just use the pod command from the gem. Or metsploit, a tool for security research that doesn’t require any ruby coding knowledge.

It has not historically been feasible to expect non-ruby developers to understand how to use a ruby version manager to install a ruby to install a gem they need.

As a result, many of those types of projects actively support running on the “system” ruby, which needs sudo to install gems. If Bundler doesn’t sudo, it causes an error and many support tickets. IIRC even if users run sudo bundle install the files end up with unusable permissions, and Bundler gets more support tickets.

The options I can think of are:

  • limit auto-sudo to macos
  • ask users on macos to install homebrew and homebrew ruby instead since that doesn’t need sudo
  • give up entirely because Apple started saying this year that system ruby is deprecated and will be removed someday
  • make the permissions error a lot more explanatory

indirect avatar Jun 05 '21 23:06 indirect

@indirect if this is in fact needed for macOS' system Ruby to behave properly, I'm wondering if a multi-stage approach implementing everything you said might make sense?:

  1. Limit auto-sudo to the macOS system Ruby install and recommend using homebrew Ruby instead for that reason.
  2. Make the permissions error a lot better.
  3. Whenever Apple stops shipping a system Ruby, completely drop it.

What're your thoughts, @indirect @deivid-rodriguez?

duckinator avatar Jun 18 '21 19:06 duckinator

That seems ideal to me 👍🏻

indirect avatar Jun 20 '21 05:06 indirect

Automatically using sudo as part of installing local dependencies when attempting to compile an arbitrary cloned git repository is such an utterly horrendous idea. Why om earth is this not reverted yet when it is clear that this bad practice is causing problems for many users and clearly is not what they want?

If Apple has a broken ruby system that it is not possible to work with, then the solution is

    if OS.mac? and is_using_system_ruby_binary
        puts "Apple has a broken system ruby installation that it is not possible to work with."
        puts "To use bundle you need to install ruby using homebrew."
        puts "(If you really want to attempt to use the system ruby anyway,"
        puts "you can set the environmental variable FIX_SYMPTOM_BY_INSTALLING_WITH_SUDO"
        puts "to 1 and re-run this command.)"
        exit(1)
    end

Even ignoring the terrible security aspects of running sudo behind the user's back, there is also the principle that you should never surprise your users.

Please watch the excellent talk How to Design a Good API and Why it Matters by Joshua Blosh where he points out

Don't Violate the Principle of Least Astonishment
• User of API should not be surprised by behavior
─ It's worth extra implementation effort
─ It's even worth reduced performance

And just because the context he is talking about is programming language APIs, that does not mean "Ah, but bundle is a command line program, so in that case it is acceptable to surprise users". It is not.

hlovdal avatar Jan 27 '22 20:01 hlovdal

@hlovdal thank you for the gentle nudge here. This issue is, frankly, intimidating and seems to get put off for a lot because of that. I agree this functionality should be removed, and multiple other maintainers have also explicitly voiced support for this change earlier in this very issue. You don't need to convince us it's a problem — it's just been hard to make the change happen, for various reasons.

For some context: the auto-sudo functionality predates most, if not all, of the current development team. It's not "reverting" a minor change, it's removing a piece of functionality that's existed for at least 12 years and only become more entrenched over time.

If you're curious about what the current plan is, please see these two comments earlier in this issue: https://github.com/rubygems/rubygems/issues/4031#issuecomment-852773654 and https://github.com/rubygems/rubygems/issues/4031#issuecomment-864224803.

A lot of the work has already been figured out in bits and pieces but never pulled together and properly resolved. I'm going to spend today trying to get everything to the point that it just needs another maintainer to review and merge a few PRs. I'll post an update here later tonight with my progress.

duckinator avatar Jan 31 '22 22:01 duckinator

Hi, sorry, I forgot to post the update here like I said. I did start work over on #5327 since it seems to be the first step of this process. Will attempt to wrap it up next week.

duckinator avatar Feb 04 '22 06:02 duckinator

Some unexpected things came up on my end the last few weeks, but I'm finally digging back into this again!

I started by rebasing #5327 off master, and am now trying to figure out why some tests can't find the bundle executable.

duckinator avatar Feb 26 '22 23:02 duckinator

As mentioned in #5327, I'm having trouble making progress on that PR because there's CI problems I can't reproduce locally.

duckinator avatar Feb 28 '22 01:02 duckinator