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

fly: remove removal of quarantine #98525

Merged
merged 1 commit into from
Jan 23, 2021
Merged

fly: remove removal of quarantine #98525

merged 1 commit into from
Jan 23, 2021

Conversation

vitorgalvao
Copy link
Member

We (HBC) are the ones who add the quarantine, so we should never be the ones removing it. That breaks expectations. Removing quarantine ourselves in one cask means users who rely on it being active have to check every cask installation to see if it’s there.

We explain how to remove quarantine when needed and have --no-quarantine, both of those need to be explicitly called by users thus indicating explicit consent. That’s the way it should be. Either that or we remove quarantining altogether, but we should never say we do something and then do the reverse (bugs excluded).

Refs #98458.

cc @miccal @todd-a-jacobs.

@todd-a-jacobs
Copy link
Contributor

todd-a-jacobs commented Jan 22, 2021

@vitorgalvao Requiring this to be done through the GUI is very much an anti-pattern. While I respect the decision to avoid automatically untainting it, the xattr instructions should be provided as caveats during install, rather than forcing the user to use the GUI.

My problem isn't with the security considerations here, it's with enforcing a multi-click, GUI approach to solving a known issue with a specific binary when a command-line alternative with no root privileges required can be presented to the user during cask installation.

@vitorgalvao
Copy link
Member Author

Requiring this to be done through the GUI is very much an anti-pattern.

Nobody is requiring you to go through the GUI. I’ve pointed out above how to it at install time: brew install --no-quarantine fly.

@todd-a-jacobs
Copy link
Contributor

Requiring this to be done through the GUI is very much an anti-pattern.

Nobody is requiring you to go through the GUI. I’ve pointed out above how to it at install time: brew install --no-quarantine fly.

The point is that it's non-intuitive for users who may not realize that they need to install it with different arguments. That's why I'm suggesting putting the solution into the caveats displayed to the user, rather than being something they need to know before attempting the installation.

@vitorgalvao
Copy link
Member Author

vitorgalvao commented Jan 22, 2021

The point is that it's non-intuitive for users who may not realize that they need to install it with different arguments.

Which is the case for literally every cask with signed software. There’s nothing special about this one. You’re arguing for it because you use it.

@miccal
Copy link
Member

miccal commented Jan 22, 2021

Apologies for all the trouble I caused with this @vitorgalvao.

@vitorgalvao
Copy link
Member Author

Apologies for all the trouble I caused with this @vitorgalvao.

Not at all! It’s two minor casks; no big deal in the slightest. By now I’ve said it multiple times and it continues to hold: in my book, you’re a fantastic maintainer.

Copy link
Member

@BrewTestBot BrewTestBot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-approved by @vitorgalvao.

@BrewTestBot BrewTestBot merged commit 899378c into master Jan 23, 2021
@BrewTestBot BrewTestBot deleted the fly-quarantine branch January 23, 2021 02:59
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Feb 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants