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

Enable quarantining of Homebrew-Cask's downloads #4656

Merged
merged 1 commit into from
Aug 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 2 additions & 5 deletions Library/Homebrew/cask/lib/hbc/artifact/moved.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,8 @@ def move_back(skip: false, force: false, command: nil, **options)
ohai "Backing #{self.class.english_name} '#{target.basename}' up to '#{source}'."
source.dirname.mkpath

if target.parent.writable?
FileUtils.cp_r(target, source)
else
command.run!("/bin/cp", args: ["-r", target, source], sudo: true)
end
# We need to preserve extended attributes between copies.
command.run!("/bin/cp", args: ["-pR", target, source], sudo: !target.parent.writable?)

delete(target, force: force, command: command, **options)
end
Expand Down
13 changes: 9 additions & 4 deletions Library/Homebrew/cask/lib/hbc/auditor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@

module Hbc
class Auditor
def self.audit(cask, audit_download: false, check_token_conflicts: false, commit_range: nil)
new(cask, audit_download, check_token_conflicts, commit_range).audit
def self.audit(cask, audit_download: false, check_token_conflicts: false, quarantine: true, commit_range: nil)
new(cask, audit_download, check_token_conflicts, quarantine, commit_range).audit
end

attr_reader :cask, :commit_range

def initialize(cask, audit_download, check_token_conflicts, commit_range)
def initialize(cask, audit_download, check_token_conflicts, quarantine, commit_range)
@cask = cask
@audit_download = audit_download
@quarantine = quarantine
@commit_range = commit_range
@check_token_conflicts = check_token_conflicts
end
Expand All @@ -19,6 +20,10 @@ def audit_download?
@audit_download
end

def quarantine?
@quarantine
end

def check_token_conflicts?
@check_token_conflicts
end
Expand Down Expand Up @@ -52,7 +57,7 @@ def audit_languages(languages)
end

def audit_cask_instance(cask)
download = audit_download? && Download.new(cask)
download = audit_download? && Download.new(cask, quarantine: quarantine?)
audit = Audit.new(cask, download: download,
check_token_conflicts: check_token_conflicts?,
commit_range: commit_range)
Expand Down
11 changes: 6 additions & 5 deletions Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ class AbstractCommand
include Options
include Homebrew::Search

option "--[no-]binaries", :binaries, true
option "--debug", :debug, false
option "--verbose", :verbose, false
option "--outdated", :outdated_only, false
option "--require-sha", :require_sha, false
option "--[no-]binaries", :binaries, true
option "--debug", :debug, false
option "--verbose", :verbose, false
option "--outdated", :outdated_only, false
option "--require-sha", :require_sha, false
option "--[no-]quarantine", :quarantine, true
Copy link
Member

Choose a reason for hiding this comment

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

When would you not want to quarantine?

Copy link
Member

Choose a reason for hiding this comment

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

When would you not want to quarantine?

When you quarantine but want to bypass it for a single single app, the way to do it is to right-click the app and select “Open”.

I’m sure there are users that have the following method when installing with HBC:

  1. brew cask install {whatever}.
  2. Invoke app launcher (Alfred, Spotlight).
  3. Call app.

In this situation you’ll be greeted with a “this app can’t be opened” and will have to manually go into /Applications and perform the procedure. --no-quarantine eases that step for apps that you know are not signed but trust.

Realistically, I predict quarantining will be one of those features we will get complaints about (think dev shops that have automated setups for their machines) and will get feature requests to add the option to not quarantine, so might as well do it preemptively.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, seems reasonable, thanks for explaining 👍

amyspark marked this conversation as resolved.
Show resolved Hide resolved

def self.command_name
@command_name ||= name.sub(/^.*:/, "").gsub(/(.)([A-Z])/, '\1_\2').downcase
Expand Down
4 changes: 3 additions & 1 deletion Library/Homebrew/cask/lib/hbc/cli/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ def run

def audit(cask)
odebug "Auditing Cask #{cask}"
Auditor.audit(cask, audit_download: download?, check_token_conflicts: token_conflicts?)
Auditor.audit(cask, audit_download: download?,
check_token_conflicts: token_conflicts?,
quarantine: quarantine?)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/cli/fetch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def run
casks.each do |cask|
Installer.print_caveats(cask)
ohai "Downloading external files for Cask #{cask}"
downloaded_path = Download.new(cask, force: force?).perform
downloaded_path = Download.new(cask, force: force?, quarantine: quarantine?).perform
Verify.all(cask, downloaded_path)
ohai "Success! Downloaded to -> #{downloaded_path}"
end
Expand Down
3 changes: 2 additions & 1 deletion Library/Homebrew/cask/lib/hbc/cli/install.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ def run
verbose: verbose?,
force: force?,
skip_cask_deps: skip_cask_deps?,
require_sha: require_sha?).install
require_sha: require_sha?,
quarantine: quarantine?).install
rescue CaskAlreadyInstalledError => e
opoo e.message
end
Expand Down
3 changes: 2 additions & 1 deletion Library/Homebrew/cask/lib/hbc/cli/reinstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ def run
verbose: verbose?,
force: force?,
skip_cask_deps: skip_cask_deps?,
require_sha: require_sha?).reinstall
require_sha: require_sha?,
quarantine: quarantine?).reinstall
end
end

Expand Down
9 changes: 7 additions & 2 deletions Library/Homebrew/cask/lib/hbc/cli/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ def run

old_cask = CaskLoader.load(old_cask.installed_caskfile)

old_cask_installer = Installer.new(old_cask, binaries: binaries?, verbose: verbose?, force: force?, upgrade: true)
old_cask_installer =
Installer.new(old_cask, binaries: binaries?,
verbose: verbose?,
force: force?,
upgrade: true)

new_cask = CaskLoader.load(old_cask.to_s)

Expand All @@ -46,7 +50,8 @@ def run
force: force?,
skip_cask_deps: skip_cask_deps?,
require_sha: require_sha?,
upgrade: true)
upgrade: true,
quarantine: quarantine?)

started_upgrade = false
new_artifacts_installed = false
Expand Down
13 changes: 12 additions & 1 deletion Library/Homebrew/cask/lib/hbc/download.rb
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
require "fileutils"
require "hbc/quarantine"
require "hbc/verify"

module Hbc
class Download
attr_reader :cask

def initialize(cask, force: false)
def initialize(cask, force: false, quarantine: true)
@cask = cask
@force = force
@quarantine = quarantine
end

def perform
clear_cache
fetch
quarantine
downloaded_path
end

Expand All @@ -38,5 +41,13 @@ def fetch
rescue StandardError => e
raise CaskError, "Download failed on Cask '#{cask}' with message: #{e}"
end

def quarantine
return unless @quarantine
return unless Quarantine.available?
return if Quarantine.detect(@downloaded_path)

Quarantine.cask(cask: @cask, download_path: @downloaded_path)
end
end
end
43 changes: 39 additions & 4 deletions Library/Homebrew/cask/lib/hbc/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ def to_s

class CaskUnavailableError < AbstractCaskErrorWithToken
def to_s
"Cask '#{token}' is unavailable" << (reason.empty? ? "." : ": #{reason}")
"Cask '#{token}' is unavailable#{reason.empty? ? "." : ": #{reason}"}"
end
end

class CaskUnreadableError < CaskUnavailableError
def to_s
"Cask '#{token}' is unreadable" << (reason.empty? ? "." : ": #{reason}")
"Cask '#{token}' is unreadable#{reason.empty? ? "." : ": #{reason}"}"
end
end

Expand Down Expand Up @@ -73,7 +73,7 @@ def to_s

class CaskCyclicDependencyError < AbstractCaskErrorWithToken
def to_s
"Cask '#{token}' includes cyclic dependencies on other Casks" << (reason.empty? ? "." : ": #{reason}")
"Cask '#{token}' includes cyclic dependencies on other Casks#{reason.empty? ? "." : ": #{reason}"}"
end
end

Expand All @@ -91,7 +91,7 @@ def to_s

class CaskInvalidError < AbstractCaskErrorWithToken
def to_s
"Cask '#{token}' definition is invalid" << (reason.empty? ? "." : ": #{reason}")
"Cask '#{token}' definition is invalid#{reason.empty? ? "." : ": #{reason}"}"
end
end

Expand Down Expand Up @@ -149,4 +149,39 @@ def to_s
EOS
end
end

class CaskQuarantineError < CaskError
attr_reader :path, :reason

def initialize(path, reason)
@path = path
@reason = reason
end

def to_s
s = "Failed to quarantine #{path}."

unless reason.empty?
s << " Here's the reason:\n"
s << Formatter.error(reason)
s << "\n" unless reason.end_with?("\n")
end

s
end
end

class CaskQuarantinePropagationError < CaskQuarantineError
def to_s
s = "Failed to quarantine one or more files within #{path}."

unless reason.empty?
s << " Here's the reason:\n"
s << Formatter.error(reason)
s << "\n" unless reason.end_with?("\n")
end

s
end
end
end
19 changes: 16 additions & 3 deletions Library/Homebrew/cask/lib/hbc/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require "hbc/download"
require "hbc/staged"
require "hbc/verify"
require "hbc/quarantine"

require "cgi"

Expand All @@ -23,7 +24,10 @@ class Installer

PERSISTENT_METADATA_SUBDIRS = ["gpg"].freeze

def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false, binaries: true, verbose: false, require_sha: false, upgrade: false, installed_as_dependency: false)
def initialize(cask, command: SystemCommand, force: false,
skip_cask_deps: false, binaries: true, verbose: false,
require_sha: false, upgrade: false,
installed_as_dependency: false, quarantine: true)
@cask = cask
@command = command
@force = force
Expand All @@ -34,9 +38,12 @@ def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false
@reinstall = false
@upgrade = upgrade
@installed_as_dependency = installed_as_dependency
@quarantine = quarantine
end

attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?, :upgrade?, :verbose?, :installed_as_dependency?
attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?,
:upgrade?, :verbose?, :installed_as_dependency?,
:quarantine?

def self.print_caveats(cask)
odebug "Printing caveats"
Expand Down Expand Up @@ -86,6 +93,7 @@ def install
uninstall_existing_cask if @reinstall

oh1 "Installing Cask #{Formatter.identifier(@cask)}"
opoo "macOS's Gatekeeper has been disabled for this Cask" unless quarantine?
stage
install_artifacts
enable_accessibility_access
Expand Down Expand Up @@ -137,7 +145,7 @@ def summary

def download
odebug "Downloading"
@downloaded_path = Download.new(@cask, force: false).perform
@downloaded_path = Download.new(@cask, force: false, quarantine: quarantine?).perform
odebug "Downloaded to -> #{@downloaded_path}"
@downloaded_path
end
Expand Down Expand Up @@ -176,6 +184,11 @@ def extract_primary_container
else
primary_container.extract_nestedly(to: @cask.staged_path, basename: basename, verbose: verbose?)
end

return unless quarantine?
return unless Quarantine.available?

Quarantine.propagate(from: @downloaded_path, to: @cask.staged_path, command: @command)
end

def install_artifacts
Expand Down
80 changes: 80 additions & 0 deletions Library/Homebrew/cask/lib/hbc/quarantine.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
require "development_tools"
module Hbc
module Quarantine
module_function

QUARANTINE_ATTRIBUTE = "com.apple.quarantine".freeze

QUARANTINE_SCRIPT = (HOMEBREW_LIBRARY_PATH/"cask/lib/hbc/utils/quarantine.swift").freeze

# @private
def swift
@swift ||= DevelopmentTools.locate("swift")
end

def available?
status = !swift.nil?
odebug "Quarantine is #{status ? "available" : "not available"}."
status
end

def detect(file)
return if file.nil?

odebug "Verifying Gatekeeper status of #{file}"

quarantine_status = !status(file).empty?

odebug "#{file} is #{quarantine_status ? "quarantined" : "not quarantined"}"

quarantine_status
end

def status(file, command: SystemCommand)
command.run("/usr/bin/xattr",
args: ["-p", QUARANTINE_ATTRIBUTE, file],
print_stderr: false).stdout.rstrip
end

def cask(cask: nil, download_path: nil, command: SystemCommand)
return if cask.nil? || download_path.nil?

odebug "Quarantining #{download_path}"

quarantiner = command.run(swift,
args: [
QUARANTINE_SCRIPT,
download_path,
cask.url.to_s,
cask.homepage.to_s,
])

return if quarantiner.success?

case quarantiner.exit_status
when 2
raise CaskQuarantineError.new(download_path, "Insufficient parameters")
else
raise CaskQuarantineError.new(download_path, quarantiner.stderr)
end
end

def propagate(from: nil, to: nil, command: SystemCommand)
return if from.nil? || to.nil?

raise CaskError, "#{from} was not quarantined properly." unless detect(from)

odebug "Propagating quarantine from #{from} to #{to}"

quarantine_status = status(from, command: command)

quarantiner = command.run("/usr/bin/xattr",
args: ["-w", "-r", QUARANTINE_ATTRIBUTE, quarantine_status, to],
print_stderr: false)

return if quarantiner.success?

raise CaskQuarantinePropagationError.new(to, quarantiner.stderr)
end
end
end