rubocops/urls: Ignore GitHub username and repo in binary URL check

[timhillgit]

This is a requested change from Homebrew/homebrew-core#293667 (comment) . We want to prevent binary files from being linked to but the current check is broad. It checks for suspicious strings anywhere in the URL when really, all we care about is the filename. Specifically from that PR, the current audit flags https://github.com/scriptingosx/utiluti/archive/refs/tags/v1.5.tar.gz as being a binary file because the username contains osx.

This PR fixes this issue by parsing the URL and only running the regex check against the filename part. Potentially, we could also possibly allow extensions like .gz and .zip as those are usually not used for binaries, but I don't think it's necessary.

Reproducing the bug

If you remove utiluti from not_a_binary_url_prefix_allowlist.json and run brew audit --strict utiluti you'll see the following error:

utiluti
  * line 4, col 3: https://github.com/scriptingosx/utiluti/archive/refs/tags/v1.5.tar.gz looks like a binary package, not a source archive; homebrew/core is source-only.
Error: 1 problem in 1 formula detected.

Testing

I haven't written a test yet. If we like the approach it looks like the correct location would be Library/Homebrew/test/rubocops/urls/http_spec.rb ?

---

• Have you followed our Contributing guidelines?
• Have you checked for other open Pull Requests for the same change?
• Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
• Have you explained why you'd like these changes included, not just what they do?
• For bug fixes, have you given step-by-step brew commands to reproduce the bug?
• Have you written new tests (excluding integration tests)? Here's an example.
• Have you successfully run brew lgtm (style, typechecking and tests) locally?

---

• AI was used to generate or assist with generating this PR.

---

[timhillgit]

Okay clearly I was not ready for dealing with rubocop. Consider this a bug report for now and I'll keep working.

[MikeMcQuaid]

Thanks for the PR!

I think this change as-is is too broad. You're handling the scriptingosx case: instead please omit the GitHub username and, maybe, the repository name.

This change as-is seems likely to miss cases that this cop exists to check.

[MikeMcQuaid]

just marking as "reviewed" for now

[carlocab]

TBH I don't think this class of false positives is common enough to do something about. If they're filling up the allowlist we can adjust this then.

[timhillgit]

I think I might agree with @carlocab . Parsing URLs for GitHub references isn't difficult but it does seem like overkill for less than 10 offenses. The audit_urls method just isn't really built for this. If instead of a regex it took a general purpose predicate, then that might make sense. @SMillerDev you'd requested this, thoughts on dropping it?

[MikeMcQuaid]

@timhillgit I'm happy to have this PR be dropped or take a more specific approach. It's not about the number of current offences but the cases this ends up catching to save maintainers work over the years which has (and will) be many more than 10.

[timhillgit]

The GitHub check ended up being much easier than I feared. Adding it means I believe we'll be able to remove the following formulae from not_a_binary_url_prefix_allowlist:

• flashrom
• reattach-to-user-namespace
• utiluti
• wallpaper

[MikeMcQuaid]

Thanks!