Valet asks for password on macOS High Sierra fresh Install

[hedii]

On a fresh install of macOS High Sierra, Valet always asks for password.

Steps to reproduce:

• Fresh MacOS High Sierra installation
• As a normal user (no root)
• Install homebrew (https://brew.sh)
• brew install homebrew/php/php71
• brew install mariadb
• Install composer (https://getcomposer.org/download/)
• Make composer available globally (https://getcomposer.org/doc/00-intro.md#globally)
• Include composer bin to path (export PATH=$HOME/bin:/usr/local/bin:~/.composer/vendor/bin:$PATH)
• composer global require laravel/valet
• valet install asks for password
• valet (no args) asks for password

I was not experiencing this on previous fresh installation of macOS.

[M165437]

I've got the same problem and from looking at the source of laravel/valet/valet I'm wondering why this hasn't always been the case. The script immediately checks if the user has root privileges and if not prompts for the password.

if [[ "$EUID" -ne 0 ]]
then
    sudo $SOURCE "$@"
    exit
fi

[M165437]

The issue #198 is related. At least the open command works as expected since pull request #396 but still not without a sudo for me. Seems to be a problem only for some users. How can we pinpoint this down?

[M165437]

@taylorotwell Could you explain the thought behind if [[ "$EUID" -ne 0 ]] in valet/valet? When I run valet in the terminal my EUID will always be different to 0 hence I'll always be prompted for the sudo password (the first time I use it per terminal session). Is this the expected behavior?

[M165437]

I think I know what it is. I checked my old system where I had a valet file in /etc/sudoers.d

Cmnd_Alias VALET = /usr/local/bin/valet *
%admin ALL=(root) NOPASSWD: VALET

After a clean install of macOS Sierra and setting everything back up (like @hedii did) this file isn't there. I copied it over from my old system and now it works.

Is this file still part of valet? When and how is it created? I can't find anything in valet's source.

[M165437]

If anyone's interested, this was changed in March when the pull request #334 was merged that removed the code that would create two files in /etc/sudoers.d (valet and brew). People with older versions kept these though and are noticing the change now upon clean installing their OS.

So the new intended behavior is to prompt for sudo once on a new terminal session on any valet command?

[drbyte]

So the new intended behavior is to prompt for sudo once on a new terminal session on any valet command?

Yes that's correct.

And as mentioned in the comments in #334, if someone wants to override that, they can update their own local machine's sudoers configuration.

[M165437]

I honestly ask myself what difference it makes having or not having these sudoers.d files (besides convenience). Adam commented

Theoretically if some other script you downloaded and ran went and modified your copy of the Valet source to do something malicious, Valet would be able run that malicious code with root access without a password prompt.

Well, now there is a password prompt but a maliciously modified copy of the Valet source would still run with root access.

To be fair, Adam also comments

… it's probably safer for that to be an opt-in thing that they can add themselves. … There's risks to anything being able to run as sudo without a password, and right now we've been opting people into that risk without them necessarily being aware.

[adamwathan]

I don't remember all of the history re: the root stuff but it does seem like we shouldn't have to ask for a password for every single command. I remember one of the design goals of adding the Valet bash script entry point was making it so the end user didn't have to know if a command needed to be run as sudo or not.

Not sure when or why that changed, there's either a good reason or it's a side effect of something else and we can still fix it.

[adamwathan]

Seems like it happened here:

fd595b5#diff-99ddf9af6c9558d5ae1319df38ab8cd4

I'm guessing this was done to make things easier/more consistent inside Valet itself. Since certain things need to be run as root and certain things don't, this was probably the easiest way to at least always know the current user was the same for every command so running things not as root could be done consistently.

I'm sure there's a way we could rework the design to avoid that but probably significant amount of work.

[M165437]

How about optionally bringing back the sudoers.d files upon installing Valet and explaining the reason behind them?

[ptasker]

Yeah the sudo issues wreaks all sorts of havoc on High Sierra (I haven't tested on Sierra), but it's annoying as homebrew shows nginx as running even when it's not. brew services list also lies, re: Homebrew/homebrew-services#102

[mikew1]

I can't tell from this thread whether I am safe to do valet install (giving root password as it requires), on a system recently migrated to High Sierra with apple's migration assistant, which has never had valet installed before.

So, is valet broken on High Sierra? If root is needed, shouldn't software be clear about what it is going to do with root permissions rather than requiring blind trust?

My experience was: OK, it needs root password, so, what is it going to do to my system?
The docs are silent about need for root password.
A laracasts video of January 2017 demonstrates valet being installed without root password.

My assumption now has to be that it is broken so not installing it. Pity. Sorry for bad news.

Update: I just checked my El Capitan system, which never had it installed, and valet also asks for root in just the same way.

[M165437]

@mikew1 Yes, you are as safe as you have always been. In an older version of Valet, it would create those sudoers.d files that would basically give Valet sudo privileges. They are not created anymore, so now you opt-in everytime you use it.

[mikew1]

Thanks. I'm reading the source now, via cd ~/.composer/vendor/laravel/valet && subl . to answer the question "what's this going to do to my system". First query answered is it going to install nginx via brew, if not already present. (The wording in the docs doesn't make it clear at all that that is what is going to happen.)