rubygems / bundler

Manage your Ruby application's gem dependencies
https://bundler.io
MIT License
4.88k stars 2k forks source link

sudo check has, er, issues #929

Closed erikh closed 11 years ago

erikh commented 13 years ago

Your sudo check depends on two things:

Simply put, if I perform my sudo before I launch bundler (like, in a capistrano+rvm setup where I sudo to alternate users manually to perform different tasks), bundler politely prompts me and hangs my deployment, even though it has full write access to the bundle directory because they're owned by the user running bundler. Setting --path disables this, which is not very intuitive.

In this same vein, I would really like to see a "I know what I'm doing, don't run sudo for me" option.

ghost commented 11 years ago

Or, more appropriately, make a '--use-sudo[=optionalusername]' option, and by default DON'T USE SUDO. This is especially important since the command you call is "cp -R ". This makes limiting sudo access... difficult. Limiting to /bin/cp isn't sufficient as that would allow a malicious developer to wreak havoc in, say, /etc or /boot

indirect commented 11 years ago

The sudo option exists specifically to make it easy of developers who can't install gems without sudo (like people using system ruby on OS X). It is not intended for use on production machines, and we strongly recommend that you use the --deployment flag there, which installs a standalone set of gems for the single application involved.

Knowing that, what do you think should be different, and why? Your issue is a bit unclear to me on both counts.

On Nov 16, 2012, at 4:04 PM, mushuweasel notifications@github.com wrote:

Or, more appropriately, make a '--use-sudo[=optionalusername]' option. This is especially important since the command you call is "cp -R ". This makes limiting sudo access... difficult. Limiting to /bin/cp isn't sufficient as that would allow a malicious developer to wreak havoc in, say, /etc or /boot

\ Reply to this email directly or view it on GitHub.

ghost commented 11 years ago

I'm tasked with developing a sandbox server. Multiple developers working with customers on api features on a single stack with customer specific seed data. So it's a hybrid, part dev part deployment. Standard IT user policies: any sudo root access for non-admins must be reasonably limited, standard username has primary group matching their username and dev group as their secondary.

My choices with bundler as I see it are

1) use bundle install --path, which bypasses sudo, but requires monkeypatching a perm fix to prevent collisions among developers running bundle

2) use bundle install --deployment, which requires the same monkeypatch, and makes for dev speedbumps

3) give all developers sudo. Even limited to /bin/cp is unacceptable.

If there's an option to use an env variable (vendor/bundle/$USER, $HOME/somepath) in bundle_path, that would work. If adding group write perms to the system ruby gems dir were sufficient to avoid sudo, that would work. If there's a way to make gem respect umask and setgid directories, that would make option 1 viable.

My main issue is that as written, sudo isn't an 'option'. Making root access a requirement for doing dev work is simply an incorrect approach.

On Nov 16, 2012 6:09 PM, "André Arko" notifications@github.com wrote:

The sudo option exists specifically to make it easy of developers who can't install gems without sudo (like people using system ruby on OS X). It is not intended for use on production machines, and we strongly recommend that you use the --deployment flag there, which installs a standalone set of gems for the single application involved.

Knowing that, what do you think should be different, and why? Your issue is a bit unclear to me on both counts.

On Nov 16, 2012, at 4:04 PM, mushuweasel notifications@github.com wrote:

Or, more appropriately, make a '--use-sudo[=optionalusername]' option. This is especially important since the command you call is "cp -R ". This makes limiting sudo access... difficult. Limiting to /bin/cp isn't sufficient as that would allow a malicious developer to wreak havoc in, say, /etc or /boot

\ Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/carlhuda/bundler/issues/929#issuecomment-10469380.

indirect commented 11 years ago

Please tell me why you believe that root access is required for development. The sudo code in Bundler is only activated if it would be impossible to install the gems without it. Since you seem to want to use bundle install --system, just give your developers read and write access to $GEM_HOME, and bob's your uncle.

ghost commented 11 years ago

Have done, and it still executed sudo. afaict, with a default GEM_HOME, in our case /opt/ruby/lib/.../gems, one would have to make / writable in order to not trigger sudo. That cannot be what is intended. Perhaps a relatively defined GEM_HOME var is the answer, but at the least that should be something documented in your section about sudo.

On Nov 16, 2012 9:55 PM, "André Arko" notifications@github.com wrote:

Please tell me why you believe that root access is required for development. The sudo code in Bundler is only activated if it would be impossible to install the gems without it. Since you seem to want to use bundle install --system, just give your developers read and write access to $GEM_HOME, and bob's your uncle.

— Reply to this email directly or view it on GitHubhttps://github.com/carlhuda/bundler/issues/929#issuecomment-10471037.

ghost commented 11 years ago

And just on a 'what one expects' level, a default behavior of triggering an sudo internally is much less intuitive than throwing an error about permissions and suggesting using a --sudo command line option.

I see this keeps coming back around in the tickets here. Perhaps 'works as designed' is not the right answer. Please consider it.

On Nov 16, 2012 10:40 PM, "Benjamin Abbott-Scott" benjamin@abbott-scott.net wrote:

Have done, and it still executed sudo. afaict, with a default GEM_HOME, in our case /opt/ruby/lib/.../gems, one would have to make / writable in order to not trigger sudo. That cannot be what is intended. Perhaps a relatively defined GEM_HOME var is the answer, but at the least that should be something documented in your section about sudo. On Nov 16, 2012 9:55 PM, "André Arko" notifications@github.com wrote:

Please tell me why you believe that root access is required for development. The sudo code in Bundler is only activated if it would be impossible to install the gems without it. Since you seem to want to use bundle install --system, just give your developers read and write access to $GEM_HOME, and bob's your uncle.

— Reply to this email directly or view it on GitHubhttps://github.com/carlhuda/bundler/issues/929#issuecomment-10471037.

indirect commented 11 years ago

Whether the GEM_HOME is "default" or not doesn't matter at all. Bundler simply checks to see if it's possible to a) install the gems, and b) create executable files for the gems. On OS X, for example, it is (on a fresh install) possible to install the gems, but not create the executable files, because /usr/bin is the hardcoded location for gem executables on OS X. I don't know anything about your setup, and you're not telling me very much, but Bundler should not try to invoke sudo unless there is a permissions error when it tries to install.

If you're seeing a bug in the sudo behaviour, please check out IISSUES for troubleshooting steps.

I think erroring out and suggesting the sudo option is a better UX in general, but I completely fail to see how that would help your stated use case.

mfischer-zd commented 10 years ago

I'd like to reopen this issue.

Basically, what we need is the ability to declare a policy on certain hosts that Bundler is not permitted to install system Gems, even if the user has sudo privileges on said hosts for other reasons. Currently the implicit-sudo behavior gives us no way to do that. So we occasionally and irritatingly have to clean up the damage afterwards when someone forgets to supply the --path or --development option and then enters a password without fully understanding the consequences of doing so.

Admittedly, it would be better to enumerate all the permitted uses of sudo on a system, but that could be unwieldy for new admins.

indirect commented 10 years ago

@mfischer-zd that sounds like you should either blacklist bundle in sudoers, or create your own wrapper for bundle that throws an error if the user is root. I'm not willing to maintain code in Bundler forever to help you out with your particular sysadmin problem, sorry. :)

mfischer-zd commented 10 years ago

If bundler is just re-execing itself under sudo, then that's a viable option -- is that in fact what it is actually doing?

The wrapper suggestion isn't really viable, since it's trivially bypassable and burdensome to maintain.

It must be said, however, that no other mainstream program that I'm aware of casually attempts to escalate its own privileges when it can't do what it wants to do. I realize and respect that bundler is trying to be helpful, but by doing so, it causes another set of problems that were likely not anticipated. It's not playing nice with multi-user systems, which Unix is at heart. You can't assume that every system is a single-user system. Consequently, I don't think it's unreasonable in this case to be able to configure Bundler to disable this behavior.

indirect commented 10 years ago

Yes, bundler is just re-execing itself under sudo.

I’m not sure what situation you’re in where a two-line script is burdensome to maintain, but that sounds like a different problem.

The reason that the feature exists is that Bundler is primarily a development tool, and developers use machines where gems are only installable by root (eg system Ruby on OS X). On production multi-user Unix machines, the expectation is that Bundler will be run by a deployment script that can’t help but add the —path or —deployment option. It’s not ideal, and I would love to remove the sudo escalation code, but right now I don’t have any other way to keep the application working after the gems have been installed on machines that need sudo.

On Dec 23, 2013, at 6:12 PM, Michael S. Fischer notifications@github.com wrote:

If bundler is just re-execing itself under sudo, then that's a viable option -- is that in fact what it is actually doing?

The wrapper suggestion isn't really viable, since it's trivially bypassable and burdensome to maintain.

It must be said, however, that no other mainstream program that I'm aware of casually attempts to escalate its own privileges when it can't do what it wants to do. I realize and respect that bundler is trying to be helpful, but by doing so, it causes another set of problems that were likely not anticipated. It's not playing nice with multi-user systems, which Unix is at heart. You can't assume that every system is a single-user system. Consequently, I don't think it's unreasonable in this case to be able to configure Bundler to disable this behavior.

— Reply to this email directly or view it on GitHub.

mfischer-zd commented 10 years ago

The issue with the script is not its length, as you seem to suggest. The issue is its maintenance.

Consider a multi-user system with multiple Rubies installed, and where Bundler is installed in each one. We'd either need to ensure that the directory that the wrapper is in has precedence in every user's PATH, which we don't necessarily have control over (users are allowed to install their own shell profiles; and this wouldn't work with rvm anyway, because it prepends to PATH). If the user gets it wrong, we lose our protection. And if we can't control PATH, are we supposed to rename the bundler script in every Ruby and put our wrapper in its place? That would be incredibly burdensome.

The fact that Bundler is "primarily" a development tool is irrelevant (not to mention an inaccurate characterization in practice; it is very useful for production systems, especially those running multiple Ruby-based services that have incompatible Gem requirements). The hosts we're concerned with are hosts primarily used by developers, though not their primary workstations (specifically, they are used to stage deployments). We just don't think Bundler's behavior is appropriate for all use cases; others clearly believe so as well; and we're apparently struggling to convince you of this.

In our view, you're unfairly shifting the burden from the programmer to the user to compensate for this (otherwise unusual) behavior. I don't know why, but you seem unreasonably steadfast in your refusal to allow an escape from this behavior for those of us having standard configurations for multi-user systems, where users aren't supposed to be mucking around with system Gems.

It seems entirely reasonable to me that if a file exists such as /etc/bundle_nosudo, Bundler can simply fail with EPERM, putting it on equal footing with other software. This would be trivial to implement and wouldn't require the wrapper-juggling discussed above.

indirect commented 10 years ago

Your replies are to a ticket that has been resolved, so I have been explaining the reasoning behind the ticket being closed.

Please move your feature proposal to http://github.com/bundler/bundler-features/issues so that we can see if your idea has support from the community of Bundler users.

On Dec 23, 2013, at 8:14 PM, Michael S. Fischer notifications@github.com wrote:

The issue with the script is not its length, as you seem to suggest. The issue is its maintenance.

Consider a multi-user system with multiple Rubies installed, and where Bundler is installed in each one. We'd either need to ensure that the directory that the wrapper is in has precedence in every user's PATH, which we don't necessarily have control over (users are allowed to install their own shell profiles; and this wouldn't work with rvm anyway, because it prepends to PATH). If the user gets it wrong, we lose our protection. And if we can't control PATH, are we supposed to rename the bundler script in every Ruby and put our wrapper in its place? That would be incredibly burdensome.

The fact that Bundler is "primarily" a development tool is irrelevant (not to mention an inaccurate characterization in practice; it is very useful for production systems, especially those running multiple Ruby-based services that have incompatible Gem requirements). The hosts we're concerned with are hosts primarily used by developers, though not their primary workstations (specifically, they are used to stage deployments). We just don't think Bundler's behavior is appropriate for all use cases; others clearly believe so as well; and we're apparently struggling to convince you of this.

In our view, you're unfairly shifting the burden from the programmer to the user to compensate for this (otherwise unusual) behavior. I don't know why, but you seem unreasonably steadfast in your refusal to allow an escape from this behavior for those of us having standard configurations for multi-user systems, where users aren't supposed to be mucking around with system Gems.

It seems entirely reasonable to me that if a file exists such as /etc/bundle_nosudo, Bundler can simply fail with EPERM, putting it on equal footing with other software. This would be trivial to implement and wouldn't require the wrapper-juggling discussed above.

— Reply to this email directly or view it on GitHub.