secure urls

[vszakats]

• Have you followed the guidelines for contributing?
• Have you checked that there aren't other open pull requests for the same formula update/change?
• Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
• Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

---

[MikeMcQuaid]

@vszakats Can you make an audit check for this?

[vszakats]

@MikeMcQuaid audit done.

[MikeMcQuaid]

@vszakats Can you make an audit check for these?

[vszakats]

Audit check continues to fail to run on my system. It tries to touch some internal files only accessible with an admin account. This started a few month ago.

[MikeMcQuaid]

@vszakats Please file an issue about that and let's get to the bottom of it.

[MikeMcQuaid]

@vszakats I'm honestly still a bit disappointed these are being fixed up manually but at the very least we need to add audits where possible.

[vszakats]

Here is the error:

$ brew audit --strict --online ./podiff.rb 
error: could not lock config file /usr/local/Homebrew/.git/config: Permission denied
Error: Failure while executing: git config --file=/usr/local/Homebrew/.git/config --replace-all homebrew.devcmdrun true

[MikeMcQuaid]

@vszakats Does that file exist? If so, you should chown -R $USER /usr/local/Homebrew.

[vszakats]

The config file does exist. The user account I'm using Homebrew with (and running audit from) doesn't have write access to it though, nor is that account on the sudo-ers list, so a chown fails as well.

[MikeMcQuaid]

Ok. Can you run the same command with --debug --verbose? Thanks!

[vszakats]

Opened an Issue for this: Homebrew/brew#1462

[vszakats]

@MikeMcQuaid Posting further results there, if you agree.

[ilovezfs]

@vszakats you can put a separate installation of brew somewhere else in order to test audit. It works based off of the path to the brew command that you're executing to decide which tree should be used: https://github.com/Homebrew/brew/blob/master/docs/Installation.md#alternative-installs

[vszakats]

Using HOMEBREW_DEVELOPER=1 setting, I could proceed with the audits. Finished them fine, with an unrelated nit reported and fixed along the way.

[ilovezfs]

@vszakats the idea is for you to open a PR to https://github.com/Homebrew/brew automating the detection of insecure URLs that match the domains you're fixing here. For instance https://github.com/Homebrew/brew/blob/master/Library/Homebrew/dev-cmd/audit.rb#L1194-L1236

[vszakats]

@ilovezfs I know — but, my script is a convoluted one, written in an obscure programming language (POSIX shell was not enough) and it still requires manual intervention from time to time (especially speaking of cask formulae).

[vszakats]

What would help in automatising, is a parser that'd return all the URLs found in a formula (including comments, and possibly non-active/alternate branches (found in some casks), but excluding __END__ sections), with variables expanded, and tagged with the stanza context (url, homepage, appcast, comment), along with original byte position/length inside the formula.

With these information the "only" task left is to do the detection/discovery and replace original URLs with upgraded ones, if they hit a certain confidence level. And list the rest for manual verification. This would also involve keeping a short blacklist of URLs not to touch and balancing the confidence levels/threshold.

[MikeMcQuaid]

Merged this, thanks @vszakats.

[MikeMcQuaid]

@vszakats It's not the language that's the issue, it's that it needs to live inside audit.rb e.g. this logic be in Ruby and called on every URL: https://github.com/vszakats/https-check/blob/6584e44a064be85bd50c251555f6b50ed92f31ce/chkhttps.sh#L44-L54

It could live in https://github.com/Homebrew/brew/blob/1fb7d0fa5702f21d35ed3831419daa6f3ea24473/Library/Homebrew/dev-cmd/audit.rb#L726 or https://github.com/Homebrew/brew/blob/1fb7d0fa5702f21d35ed3831419daa6f3ea24473/Library/Homebrew/dev-cmd/audit.rb#L766 and be matched by a regex. The main goal is avoiding false negatives rather than false positives. Would be amazing if you could try to open a PR with something similar (even if it doesn't work quite right) and we can help you get over the finish line.

[vszakats]

Thanks for the pointers.

By "language problem" I meant that I'm currently using a different script, not written in .sh but in a strange different one (too embarrassed to post it). It's a fragile mess with formula parsing intermixed with HTTPS discovery logic. Large portion of the mess resulted from adding cask support. With human oversight, it's rather useful though.

The only reasonable way I can see moving along with this is separating the parsing and discovery logic. But that's not something I'll be capable of doing in the near future in Ruby and in a clean way. It's non-trivial to parse source code, and the tools I've tried (f.e. ruby_parser) miss comments and make finding out context and retrofitting changes into the source even more complicated.

The main goal is avoiding false negatives rather than false positives.

That's what I have been aiming for. Any ambiguous/heuristic detection is flagged as a "hint".

[MikeMcQuaid]

@vszakats I think it's sufficient to just have a regex for all URLs that are then checked with curl to see if there's a https with identical content to http. One (if not both) of the above methods will handle avoiding patches for you; full Ruby parsing is definitely not necessary.