Page MenuHomePhabricator

Detect goofy `sudo -n` output under OSX
ClosedPublic

Authored by epriestley on Dec 23 2014, 10:21 PM.
Tags
None
Referenced Files
F13216450: D11041.id26516.diff
Sat, May 18, 12:59 AM
F13216423: D11041.id26517.diff
Sat, May 18, 12:44 AM
F13211722: D11041.diff
Fri, May 17, 6:02 AM
F13195402: D11041.diff
Sun, May 12, 10:22 PM
F13178174: D11041.diff
Wed, May 8, 8:14 PM
F13176179: D11041.diff
Wed, May 8, 11:18 AM
Unknown Object (File)
Sat, Apr 27, 3:09 AM
Unknown Object (File)
Thu, Apr 25, 1:36 AM
Subscribers

Details

Reviewers
btrahan
Commits
Restricted Diffusion Commit
rPb3394c53d8e9: Detect goofy `sudo -n` output under OSX
Summary

See rP2fedb6f941d8. We might need a more general version of this since we do some sudo stuff elsewhere, but at least on my machine sudo -n exits with code 0 when the target user exists but needs a password.

Test Plan
  • Tried to run daemons as root, with no automatic sudo to root. Got a bad result before (phd believed it had executed the daemons) and a good result afterward (phd recognized that sudo failed).
  • Tried to run daemons from root, as a non-root user. Got a good result in both cases.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Detect goofy `sudo -n` output under OSX.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
epriestley added a subscriber: fabe.

"a password is required" might be a locale specific string. (I've seen users have german text there).
Just checking for sudo: however might match some pam warnings that start the daemon anyway which might result in two daemons.. :(

Yeah, I don't see a very good way around that. Most OSX installs of Phabricator are run by developers working on it, though, so I think it's OK that this isn't perfect.

Yeah i've got no real idea there either. Might be a good idea to move the whole 'run things as another user' stuff into the ExecFuture (or have an SudoExecFuture) and try to handle it centrally in the future.

btrahan edited edge metadata.

Thanks for the code changes here.

For my own setup, should I be running some command on the _phd user so it can be su'd to? Note this now basically works in that I hit the "just run this as the active user" codepath successfully now so we can also just drop this if everyone wants. :D

This revision is now accepted and ready to land.Dec 23 2014, 10:39 PM

You can do a few things:

  • If you run phd as root, it should be able to sudo as _phd (since root can always sudo without a password, normally). You could switch to root with su, or likely run sudo phd start.
  • If you run phd as _phd, it shouldn't need to sudo.
  • If you want to be able to sudo from btrahan to _phd directly, without going through root, you'd need to edit /etc/sudoers and put some black magic in there. I don't know it offhand but you can google around, it's something like btrahan ALL=(_phd) NOPASSWD:
This revision was automatically updated to reflect the committed changes.