Skip to content
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

Disallow running brew cask as root. #1476

Merged
merged 1 commit into from
Nov 10, 2016

Conversation

reitermarkus
Copy link
Member

@reitermarkus reitermarkus commented Nov 10, 2016

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew tests with your changes locally?

As discussed in #1452.

@reitermarkus reitermarkus added in progress Maintainers are working on this cask Homebrew Cask labels Nov 10, 2016
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once tests are 💚 🚢

@reitermarkus reitermarkus merged commit 9a9ab92 into Homebrew:master Nov 10, 2016
@reitermarkus reitermarkus deleted the rootless branch November 10, 2016 14:57
@reitermarkus reitermarkus removed the in progress Maintainers are working on this label Nov 10, 2016
@bcg62
Copy link

bcg62 commented Nov 10, 2016

This breaks a lot of CM tools like chef/puppet that function with out having a tty available for a user to enter a password.

Theres been a lot of back and fourth about this it seems and I'm still not sure what the correct solution is, could someone point me in the right direction?

Homebrew/homebrew-cask#5667
Homebrew/homebrew-cask#19180 (comment)

@reitermarkus
Copy link
Member Author

reitermarkus commented Nov 10, 2016

@bcg62, see #1452 (comment). Basically there are two “solutions”: Either run a sudo keep-alive loop or add the current user to /etc/sudoers for the duration of the script.

@bcg62
Copy link

bcg62 commented Nov 10, 2016

thanks @reitermarkus

This seems fine for some simple bash scripting but when trying to do something similar in a real CM tool its very intrusive and dangerous to modify sudo on the fly to preform an action, or do something like a keep alive in the background. Its also a broader discussion for the tool responsible referenced above.

@reitermarkus
Copy link
Member Author

reitermarkus commented Nov 10, 2016

Ok, I just read through the two discussions you linked above, and it seems like adding support for SUDO_ASKPASS is pretty trivial, so this could be a third option.

@bcg62
Copy link

bcg62 commented Nov 28, 2016

Thanks, would you be able to share your example?

@reitermarkus
Copy link
Member Author

First you will have to save your askpass-script to /path/to/script.

Then you can either do this

SUDO_ASKPASS="/path/to/script" brew cask install cask-which-requires-sudo

or this

export SUDO_ASKPASS="/path/to/script"
brew cask install cask-which-requires-sudo
brew cask install another-cask-which-requires-sudo

@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cask Homebrew Cask
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants