theseal / ssh-askpass

ssh-askpass for macOS
183 stars 37 forks source link

Should it's launchd plist really be loaded as root? #22

Closed drew1kun closed 5 years ago

drew1kun commented 6 years ago

Not an issue, but a question.

The Homebrew postinstall notes suggest the following:

...
To have launchd start ssh-askpass now and restart at startup:
  sudo brew services start ssh-askpass
...

While Homebrew states that brew command should never be run as root.

Could you please clarify this?

Thank you.

simmel commented 6 years ago

That is obviously an error on our part. Want to send a pull request for great fame and imaginary internet points? Thanks for noticing!

On Sat, 19 May 2018, at 22:59, drew-kun wrote:

Not an issue, but a question.

The Homebrew postinstall notes suggest the following:

... To have launchd start ssh-askpass now and restart at startup: sudo brew services start ssh-askpass ...

While Homebrew states that brew command should never be used with sudo.> Could you please clarify this?

Thank you.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub[1], or mute the thread[2].

Links:

  1. https://github.com/theseal/ssh-askpass/issues/22
  2. https://github.com/notifications/unsubscribe-auth/AALqfJFTr12v6nPxuF5yEuqXj1VEir4Dks5t0IepgaJpZM4UF2on
theseal commented 5 years ago

The output is from brew itself, we have no references to sudo any more. Not sure why doe…

simmel commented 5 years ago

We're closing this since this is no longer the case. Thanks for reporting it!

simmel commented 5 years ago

Uh, brew still says we should use sudo apparently (thanks @theseal!) Are we using the correct way in our formula? Otherwise we should raise a ticket with Homebrew?

simmel commented 5 years ago

https://www.rubydoc.info/github/Homebrew/brew/master/Formula#plist-instance_method looks like we're using the correct method.

https://duckduckgo.com/?q="plist"+site%3Ahttps%3A%2F%2Fdocs.brew.sh doesn't mention plist anywhere else really.

simmel commented 5 years ago

Uh, this is weird: https://github.com/Homebrew/brew/blob/a75a8c69b39aa9584c299d37826a1700256e8ab8/Library/Homebrew/extend/os/mac/caveats.rb#L14-L20 https://www.rubydoc.info/github/Homebrew/brew/master/Formula#plist_options-class_method Feels like startup is not a really good term for it. Seems like it has been like this since the introduction (I cba to check that carefully tbh).

simmel commented 5 years ago

@theseal I guess we should just remove startup? Can you try it and then commit?

theseal commented 5 years ago

You are right! startup is a strange name for the option.

Did local modification on the Formula and then reinstall, sudo is no longer suggested. Good find!

$ brew edit ssh-askpass
Editing /usr/local/Homebrew/Library/Taps/theseal/homebrew-ssh-askpass/Formula/ssh-askpass.rb
MacBook-Esc:homebrew-ssh-askpass jocar$ brew reinstall ssh-askpass
==> Reinstalling theseal/ssh-askpass/ssh-askpass
==> Downloading https://github.com/theseal/ssh-askpass/archive/v1.2.1.tar.gz
Already downloaded: /Users/jocar/Library/Caches/Homebrew/downloads/024aac11f52566fc5369709220a927b9bf43767ed6d80ab6e1e48c5994e51ab0--ssh-askpass-1.2.1.tar.gz
==> Caveats
NOTE: After you have started the launchd service (read below) you need to log out and in (reboot might be easiest) before you can add keys to the agent with `ssh-add -c`.

To have launchd start theseal/ssh-askpass/ssh-askpass now and restart at login:
  brew services start theseal/ssh-askpass/ssh-askpass
==> Summary
🍺  /usr/local/Cellar/ssh-askpass/1.2.1: 5 files, 6.7KB, built in 4 seconds

Will update the Tap in a minute!