Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cask_language_reference/stanzas/url.md: refine comment rules #18051

Merged
merged 1 commit into from
Mar 4, 2016
Merged

cask_language_reference/stanzas/url.md: refine comment rules #18051

merged 1 commit into from
Mar 4, 2016

Conversation

vitorgalvao
Copy link
Member

Note: In this context, I’m referring to audit as the process of manually checking a cask for correctness, not brew cask audit.

While working on #17498 and #17499, I came to the realisation I was working on the wrong solution to the right problem. The right solution involves a slight rethink of the previous comment rule.

Our current method of informing a url was verified (by pointing out only the host) is insufficient and flawed, as it does not account for maintainer inattention. While for cases like world-of-tanks it is pretty straightforward, since the company owns both domains, the rule is particularly ineffective for popular hosts like amazonwas.com and cloudfront.net, where the host isn’t sufficiently identifiable to any application.

# bitbucket.org is the official download host per the vendor homepage isn’t enough to prove maintainers kept the comment up to date with url changes (what if at a certain point a contribution changed the bitbucket account to a malicious one and a mistake was made in merging?). # bitbucket.org/luisangelsm/yacreader was verified as official when first introduced to the cask is, however, clear in this regard, as is # rink.hockeyapp.net/api/2/apps/6ab08fb043a94f51c9109c216e295a50 was verified as official when first introduced to the cask. In those examples, discrepancies between the comment and url would be catchable to a user auditing the cask, signalling a mistake in need of correction. github.io links would also no longer be exempt from the rule.

With that in mind, the rule template for the comment ought then to be:

# URL_SECTION was verified as official when first introduced to the cask

Where URL_SECTION is the smallest possible portion of the URL, including and following the top domain host, that uniquely identifies the app or vendor.


So what about appcast comments (the whole point of #17499 and half of #17498)? While working on the aforementioned issues, I also realised appcast hosts are irrelevant, since they do not pose a threat to users, only malicious urls do. In theory, even a third-party appcast (which is somewhat what github releases.atom appcasts are) can be used just as effectively with no harm.

This could only become a potential issue where url is extrapolated from appcast, but as long as the maintainer actually verified the url as being trustworthy, which must happen anyway, the issue no longer exists. Furthermore, this can be eliminated (almost?) completely. The reason to use urls from the appcast feed is versioning (when the website download is unversioned), but now that we’re close to having in place the system to check appcasts for updates, we can switch those to the official unversioned URLs and still have version and sha256.

Things that need to be done, in order:

@vitorgalvao
Copy link
Member Author

Merging now so the rule is officially in place.

vitorgalvao added a commit that referenced this pull request Mar 4, 2016
cask_language_reference/stanzas/url.md: refine comment rules
@vitorgalvao vitorgalvao merged commit 8327e2a into Homebrew:master Mar 4, 2016
@vitorgalvao vitorgalvao deleted the url-comment branch March 4, 2016 12:26
@vitorgalvao vitorgalvao mentioned this pull request Mar 4, 2016
1 task
@miccal miccal removed documentation Issue regarding documentation. meta labels Dec 23, 2016
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants