brew audit: allow require_root to exist independent of run in service do blocks

[Jerry1144]

Verification

• This issue's title and/or description do not reference a single formula e.g. brew install wget. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.

Provide a detailed description of the proposed feature

Cross-post from Homebrew/homebrew-services#636. I propose that audit rules allow require_root to exist with name and without run. I.e. make such service do legal:

service do
  name macos: "daemonplist_without_suffix"
  require_root true
end

brew audit currently complains that run must be defined to use methods other than name like [:require_root].

If I removed the require_root line, I'd have to write my own Caveat, asking users to ignore the subsequent non-sudo brew services start line. How weird would that sound!

==> Caveats
Please run this command:
  sudo brew services start [formula]
And ignore the following:

To start [formula] now and restart at startup:
  brew services start [formula]

What is the motivation for the feature?

I find name+require_root perfect in "tool provides a system Daemon, and a ready-made plist to be put into /Library/LaunchDaemons". It makes brew output the right command to run in the Caveats section:

==> Caveats
To start [formula] now and restart at startup:
  sudo brew services start [formula]

and it Just Works.

How will the feature be relevant to at least 90% of Homebrew users?

Hardly. Not many formulae provide a service, and fewer a LaunchDaemon that must run as root, and even fewer a launchd.plist as part of the installation. Still, for those formulae that do, it will make life easier to write a service-do and get it merged.

What alternatives to the feature have been considered?

Manually construct the service file, filling the run, RunAtLoad, etc. in service do, just to please the auditor. Guidelines suggest I use the provided plist if a tool comes with one, and only make one myself when it doesn't.

Also, service do block written that way appends this misleading latter half of a Caveat, ugh:

Or, if you don't want/need a background service you can just run:
  #{sbin}/daemon

but I guess that's a separate issue.

[MikeMcQuaid]

@Jerry1144 Sorry, I'm not sure what's being asked for here so have a few questions.

brew audit currently complains that run must be defined to use methods other than name like [:require_root].

Why do you not want to add a run line? Is it because it is wrong/doesn't work or another reason?

Maybe it's worth providing some more examples about what:

• the current output is and why you don't like it
• what the correct output would ideally be
• anything else you feel would help

Thanks!

[Jerry1144]

@MikeMcQuaid Briefly, service do has two modes of action:

1. name only, without run. This nominates an existing launchd.plist to be handled by brew services after installation. When the software provides a plist for us.
2. with run [command to run service]. This time brew generates a launchd.plist during build, based on contents of the do block, to be handled by brew services

Audit rules currently does not allow anything other than name in mode1.

I believe the audit rules was there so that we authors don't accidentally mix the two uses - supplying run_at_load false etc for an existing plist won't change the corresponding key, so is highly likely a misuse. This is true for most variables in a service do block.

However, require_root is different: it does not influence the contents of launchd.plist even in mode2. Instead, it only makes the Caveats contain sudo, and adds a warning if anyone brew services start without sudo. Thus require_root is more like a regulatory variable, acting on brew itself, instead of specifying a key in the plist. I find it useful, and believe that allowing its use in mode1 is harmless.

---

anything else you feel would help

I feel these are off-topic, but service do sure could use even more regulatory variables:

• maybe a show_run_your_own false to suppress printing this in mode2

  Or, if you don't want/need a background service you can just run:
    [content of `run`]
  

• a really_require_root true to turn that "you can just run: [content of run]" into "you can just run: sudo [content of run]". Maybe also to turn non-sudo brew services start an error, or at least require user confirmation.

• some way to pump the contents of one service do into a plist during install, and later supply another canonical service do to be handled by brew services. Most notably, clamav could use two service files[1], but neither of which was generated by the build script, and service do only fills one hole.(maybe Ruby grammar already allows this. I saw a to_plist method in RubyDoc but don't know Ruby enough to know how to use it).
  [1]: clamav formulae has incorrect LaunchDaemon definition for MacOS homebrew-core#156684 (comment)

[MikeMcQuaid]

Audit rules currently does not allow anything other than name in mode1.

so, if I've understood correctly: you want to use an upstream-provided .plist but run it as root?

[Jerry1144]

you want to use an upstream-provided .plist but run it as root?

Yes, to instruct users that by default they should be run as root, because maybe the tool talks with SMC and those syscall only work as root. it feels awkward to paraphrase the whole plist in the formula when there's already one in place.

I understand it may pose a security risk if upstream changes plist contents someday, but the guidelines aren't very clear about the Maintainer's position on this (if run as root, always paraphrase the plist, so formula reviewers can quickly check if it's sane).

[apainintheneck]

This is reasonable and the only reason we haven't added it is because no one has mentioned until now.

[Jerry1144]

Thank you! I took the liberty to expand the Service block methods section in Formula Cookbook a little. Jerry1144@4a0e648

Should I open a PR for it? I clicked Fork and Commit, why does this commit 4a0e648 exist in this repository too 😯

[MikeMcQuaid]

Thank you! I took the liberty to expand the Service block methods section in Formula Cookbook a little. Jerry1144@4a0e648

Should I open a PR for it? I clicked Fork and Commit, why does this commit 4a0e648 exist in this repository too 😯

A PR would be great, thanks!