audit: migrate throttle list to Homebrew/core

[Rylan12]

• 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 style with your changes locally?
• Have you successfully run brew tests with your changes locally?
• Have you successfully run brew man locally and committed any changes?

---

This PR starts moving allowlists and blocklists from the Homebrew/brew repo to the Homebrew/core repo as discussed in #8980. This PR lays the groundwork for migrating all lists to their respective taps and allows exception lists to be specified for third party taps. For now, I've chosen to migrate only one list (the THROTTLED_FORMULAE list) as a proof-of-concept. Assuming this is something that is desired, migrating the rest of the lists should be trivial.

Corresponding Homebrew/core PR: Homebrew/homebrew-core#64043

---

In my opinion, the Point Of Truth for these lists should be with the formulae themselves and the brew code should not be where tap-specific rules are added.

As a summary of #8980, here are my thoughts that I expressed in #8980:

In general, the maintainers who approve/merge PRs that change the allowlists are the Homebrew/core maintainers. In my experience, it follows the typical Homebrew/core workflow (opening a PR, waiting for an approval, merging after approved) but in a different repo. For that reason, it seems like it would make more sense to have those changes be made in the Homebrew/core PR. No need to bother non-core maintainers with PRs that they won't bother looking at.

Also, shouldn't the decisions about these lists be made by the Homebrew/core mainatiners? If someone's a core maintainer but not a brew maintainer, they should still be able to approve/merge these changes as it's directly relevant to their work. Similarly, it doesn't make sense for a cask-maintainer with write access to Homebrew/brew to be able to approve/merge allowlist PRs unless they're also a core-maintainer (this is just an example and is not me pointing fingers at anyone).

It seems like having these lists in Homebrew/brew is just adding unnecessary steps/complexity.

Another maintainer expressed concerns:

I think in the "happy path" case where e.g. an item is added to a list, approved and merged the steps may be unnecessary.

The difference is: this is not what always happens. Things I've seen already happen with allow lists in Homebrew/brew that make me want to keep this process:

• a formula is added to a list by a contributor when it should not be
• a formula is added to the wrong list (provided by macOS vs. shadows macOS)
• multiple formulae of the same version are added to a list
• an exception is added to an allowlist when it could be more easily fixed in the formula
• an exception is added to an allowlist when we'd previously agreed it wouldn't be and the formula should instead be fixed/deprecated/disabled

---

CC: @MikeMcQuaid, @fxcoudert and @Homebrew/core

[dtrodrigues]

Previous PR regarding moving the throttle allowlist into Homebrew/core: #8048.

This PR is different in that it involves adding an external json allowlist within homebrew/core vs DSL, but some of the previous discussion may still be relevant. Having the allowlist in a separate place still requires a comment within the formula to be manually kept in sync with a separate list, but I don't see a clean solution to that without changing the DSL, which is already not small.

FWIW, I'm generally in favor of moving formula-specific logic (eg things like the AUTOMATIC_RESOURCE_UPDATE_BLOCKLIST in utils/pypi.rb) out of Homebrew/brew and into Homebrew/core or its respective tap in general, whether it be within the formula or separately within the tap. Overall, this would allow taps to have their own allowlists/blocklists without having to maintain a separate version of Homebrew/brew as well as mitigate other synchronization issues between the two repositories as already mentioned.

[Rylan12]

Previous PR regarding moving the throttle allowlist into Homebrew/core: #8048.

Oh, thanks for pointing that out! I didn't realize it was attempted before.

This PR is different in that it involves adding an external json allowlist within homebrew/core vs DSL, but some of the previous discussion may still be relevant. Having the allowlist in a separate place still requires a comment within the formula to be manually kept in sync with a separate list, but I don't see a clean solution to that without changing the DSL, which is already not small.

I do with Mike's point in the other PR that the source of the throttling shouldn't be in the formula itself. It makes sense to me that the formula files themselves should only contain build-information and shouldn't be cluttered with the more developer-y side of things. A comment seems sufficient to me to let curious users know why the formula hasn't been updated to the latest release.

Plus, keeping the information separate means that it's harder to accidentally/sneakily change that information. One of the valid concerns with moving this information is that contributors will be tempted to add exceptions whenever a formula fails an audit instead of fixing the problem. I think that keeping the exceptions in a separate file will be sufficient to combat this problem, though, and having it in a different repo is unnecessary (hence, this PR).

Overall, this would allow taps to have their own allowlists/blocklists without having to maintain a separate version of Homebrew/brew as well as mitigate other synchronization issues between the two repositories as already mentioned.

Agreed

[MikeMcQuaid]

This approach looks really good to me so far, nice work. Some higher-level thoughts:

• this should be robust to people deleting these files, JSON parsing failure etc. as a no-op for the formula audit and to make the tap-wide audit complain about whatever the problems are with the files
• I'm wondering putting these exceptions in a directory e.g. audit_exceptions/throttled_formulae.json or similar would be nicer than hardcoded key values in both places (and probably make merge conflicts and general Git workflows work a bit nicer)

[Rylan12]

Thanks for the review! I'm 👍 on the renaming suggestion. Will work on this more later today when I have some time.

• this should be robust to people deleting these files, JSON parsing failure etc. as a no-op for the formula audit and to make the tap-wide audit complain about whatever the problems are with the files

👍

• I'm wondering putting these exceptions in a directory e.g. audit_exceptions/throttled_formulae.json or similar would be nicer than hardcoded key values in both places (and probably make merge conflicts and general Git workflows work a bit nicer)

This still requires hard coding file names, though, right? It's probably easier to manage for users, though, so I'm 👍

I guess something similar to audit_style is probably right here; just a single audit that goes through all taps (by default or is filtered by --tap) and checks that their audits are appropriate.

Can you give a link to what you're talking about here? I'm looking at this but not seeing what I think you're describing.

Edit: ah, you're talking about this part. That looks good, I will continue to investigate.

[MikeMcQuaid]

This still requires hard coding file names, though, right? It's probably easier to manage for users, though, so I'm 👍

Yeh, it does, but it feels like it'd be easier to manage, yeh, and less likely to accidentally have a no-op from the key being typoed.

That looks good, I will continue to investigate.

Thanks!

[Rylan12]

Okay, I've made the following changes:

1. Rename audit_exceptions to tap_audit_exceptions
2. Invalid JSON is a no-op
3. brew audit --tap=<tap> will now check
   1. that all JSON files (including formula_renames.json and any others) are valid JSON
   2. that all formulae listed in the audit_exceptions tap directory exist in that tap

I've migrated the VERSIONED_HEAD_SPEC_ALLOWLIST as well to demonstrate an allowlist that is not a hash.

---

CI will fail currently because I've removed the VERSIONED_HEAD_SPEC_ALLOWLIST which means there are now two formulae that are not excluded from the audit.

Once this is ready, I think the easiest way to solve this (and similar future PRs) will be to merge Homebrew/homebrew-core#64043 first and then rerun CI here. Adding those files to homebrew/core before this PR is merged shouldn't cause any problems.

[MikeMcQuaid]

Once this is ready, I think the easiest way to solve this (and similar future PRs) will be to merge Homebrew/homebrew-core#64043 first and then rerun CI here. Adding those files to homebrew/core before this PR is merged shouldn't cause any problems.

Agreed 👍🏻

[MikeMcQuaid]

Looks good! One tiny suggested change but feel free to merge with or without it (and, if with it, to make the change and merge whenever CI is 💚). Nice work as usual @Rylan12!

[Rylan12]

Ah, need to wait for Homebrew/homebrew-core#64043 to be merged into linuxbrew-core. Will rerun and merge when that's been done.

Thanks for the reviews, @MikeMcQuaid!

[iMichka]

The changes have been merged to linuxbrew-core.

[Rylan12]

Okay, the first lists have been migrated! I'll follow up with more migrations in the coming days unless there are any problems with the migrated lists.