Homebrew-cask: Add `verified` parameter to `url`

Created on 21 Mar 2020  Â·  6Comments  Â·  Source: Homebrew/homebrew-cask

We have a magic comment required when url and homepage hostnames differ.

In addition to it being imperfect in select cases where “the smallest possible portion of the URL that uniquely identifies the app or vendor” includes a URL part that is interpolated, we shouldn’t hide functionality in comments.

The option is important for user security, and a great visual cue for maintainers reviewing a cask—if it’s there it’s an extra line; if it’s not it’s an audit warning. So we should not get rid of the rule, but instead make it a proper option.

Instead of (from activitywatch):

# github.com/ActivityWatch/activitywatch was verified as official when first introduced to the cask
url "https://github.com/ActivityWatch/activitywatch/releases/download/v#{version}/activitywatch-v#{version}-macos-x86_64.zip"

do

url "https://github.com/ActivityWatch/activitywatch/releases/download/v#{version}/activitywatch-v#{version}-macos-x86_64.zip",
    verified: 'https://github.com/ActivityWatch/activitywatch'

This could be a simple string comparison with the start of url.

Ping @Homebrew/cask.

core good first issue ready to implement

All 6 comments

According to the documentation, the primary goal is:

so a user auditing the cask knows the URL was verified by the Homebrew Cask team as the one provided by the vendor, even though it may look unofficial or suspicious.

I like the idea of a user casually inspecting a cask without having to resort to the stanza documentation.

I’d love to collect a few alternatives so we can decide on a solution that preserves as much of the spirit of that goal as possible.

Option 2

An attempt to make this unambiguous:

url "https://github.com/ActivityWatch/activitywatch/releases/download/v#{version}/activitywatch-v#{version}-macos-x86_64.zip"

confirmed url_starts_with: 'https://github.com/ActivityWatch/activitywatch',
          because: '79028#issuecomment-602017687'

By allowing multiple copies, the confirmed stanza would be open for future uses:

url "https://github.com/ActivityWatch/activitywatch/releases/download/v#{version}/activitywatch-v#{version}-macos-x86_64.zip"

confirmed url_starts_with: 'https://github.com/ActivityWatch/activitywatch',
          because: '79028#issuecomment-602017687'

confirmed false_alarm: 'https://virustotal.com/gui/file/12345',
          because: '56223#issuecomment-123456789'       

I like the idea of a user casually inspecting a cask without having to resort to the stanza documentation.

So do I, but as someone who had a big hand in shaping this rule, I doubt it happens with any meaningful recurrence.

If you care about this verification, you’ll read the documentation anyway because you won’t trust a comment; you’ll want to know exactly what kind of verification was done. I’ll go so far as saying we want people to read the documentation, because it has the very important point of “[t]he comment doesn’t mean you should trust the source blindly”.

decide on a solution that preserves as much of the spirit of that goal as possible.

In practice, the rule is useful for maintainers because it’s a visual cue to pay attention (if it’s there it’s an extra line; if it’s not it’s an audit warning) first and users a far second.

Option 2

A new confirmed stanza seems unnecessary. There’s nothing else that require confirmation in casks, other than url. We can name the parameter confirmed_starting_with instead, but that might make it seem it was checked every time, while verified only implies it was done at some point.

The because is also unnecessary, because there’s seldom one. The comment is added by the user, the maintainer verifies it, and if it’s correct it’s included. The because would only point to the initial issue where it was introduced, which has no information.

I doubt it happens with any meaningful recurrence.

Makes sense.

In practice, the rule is useful _for maintainers_ because it’s a visual cue to pay attention (if it’s there it’s an extra line; if it’s not it’s an audit warning) first and users a far second.

Alright, I wasn’t aware of this.

In that case, 👍 on your suggestion. (Though maybe we want to reflect in the docs that our goals have somewhat shifted.)

Alright, I wasn’t aware of this.

That would’ve been relevant information for me to stick in the top post. I’ll edit it.

(Though maybe we want to reflect in the docs that our goals have somewhat shifted.)

Sure. Though to be fair to the documentation (I was looking into it now to see how to change the wording), while we do specify the comment is there for users to audit the cask, we don’t say they should be able to do it without referencing the documentation.

@vitorgalvao @claui
I have added the parameter according to your discussion.
Feel free to give me any advice.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

RJHsiao picture RJHsiao  Â·  3Comments

dentarg picture dentarg  Â·  3Comments

vasigorc picture vasigorc  Â·  3Comments

CoolTomatos picture CoolTomatos  Â·  3Comments

GeekOnGadgets picture GeekOnGadgets  Â·  3Comments