[#583] Auditing of cask download and checksums

closes #688

CONTRIBUTING.md

@@ -83,7 +83,9 @@ Did it install? If something went wrong, `brew cask uninstall my-new-cask` and
 edit your Cask to fix it.
 
 If everything looks good, you'll also want to make sure you cask passes audit
-with `brew cask audit my-new-cask`
+with
+
+`brew cask audit my-new-cask --download`
 
 If your application and homebrew-cask do not work well together, feel free to
 [file an issue](https://github.com/phinze/homebrew-cask/issues) after checking

Gemfile

@@ -4,5 +4,5 @@ group :test do
   gem 'rake'
   gem 'minitest', '4.7.0'
   gem 'minitest-colorize'
-  gem 'mocha', '0.13.3'
+  gem 'mocha', '0.14.0'
 end

Gemfile.lock

@@ -5,7 +5,7 @@ GEM
     minitest (4.7.0)
     minitest-colorize (0.0.5)
       minitest (>= 2.0)
-    mocha (0.13.3)
+    mocha (0.14.0)
       metaclass (~> 0.0.1)
     rake (10.0.4)
 
@@ -15,5 +15,5 @@ PLATFORMS
 DEPENDENCIES
   minitest (= 4.7.0)
   minitest-colorize
-  mocha (= 0.13.3)
+  mocha (= 0.14.0)
   rake

lib/cask/audit.rb

@@ -1,4 +1,5 @@
 require 'cask/checkable'
+require 'cask/download'
 
 class Cask::Audit
   attr_reader :cask
@@ -9,17 +10,18 @@ class Cask::Audit
     @cask = cask
   end
 
-  def run!
+  def run!(download = false)
     _check_required_fields
     _check_checksums
     _check_sourceforge_download_url_format
-    return if errors?
+    _check_download(download) if download
   end
 
   def summary_header
     "audit for #{cask}"
   end
 
+
   def _check_required_fields
     add_error "url is required" unless cask.url
     add_error "version is required" unless cask.version
@@ -31,6 +33,12 @@ class Cask::Audit
     add_error "could not find checksum or no_checksum" unless cask.sums.is_a?(Array) && cask.sums.length > 0
   end
 
+  def _check_download(download)
+    download.perform
+  rescue => e
+    add_error "download not possible: #{e.message}"
+  end
+
   def _check_sourceforge_download_url_format
     if cask.url.to_s.match(/\S*sourceforge\.\S*\/\S*/i)
       unless cask.url.to_s.match(/\S*downloads.sourceforge.net\/\S+/i)

lib/cask/auditor.rb

@@ -1,7 +1,13 @@
 class Cask::Auditor
-  def self.audit(cask)
+  def self.audit(cask, options = {})
     audit = Cask::Audit.new(cask)
-    audit.run!
+
+    if options.fetch(:audit_download, false)
+      audit.run!(Cask::Download.new(cask))
+    else
+      audit.run!
+    end
+
     puts audit.summary
   end
 end

lib/cask/cli/audit.rb

@@ -1,12 +1,38 @@
 class Cask::CLI::Audit
+  def self.help
+    'verifies installability of casks'
+  end
+
   def self.run(*args)
-    casks_to_audit = args.empty? ? Cask.all : args.map { |arg| Cask.load(arg) }
-    casks_to_audit.each do |cask|
-      Cask::Auditor.audit(cask)
+    new(args, Cask::Auditor).run
+  end
+
+  def initialize(args, auditor)
+    @args = args
+    @auditor = auditor
+  end
+
+  def run
+    casks_to_audit.each { |cask| audit(cask) }
+  end
+
+  def audit(cask)
+    @auditor.audit(cask, :audit_download => audit_download?)
+  end
+
+  def audit_download?
+    @args.include?('--download')
+  end
+
+  def casks_to_audit
+    if cask_list.empty?
+      Cask.all
+    else
+      cask_list.map { |arg| Cask.load(arg) }
     end
   end
 
-  def self.help
-    "verifies installability of casks"
+  def cask_list
+    @cask_list ||= @args.reject { |a| a == '--download' }
   end
 end

lib/cask/download.rb

@@ -0,0 +1,30 @@
+class Cask::Download
+  attr_reader :cask
+
+  def initialize(cask)
+    @cask = cask
+  end
+
+  def perform
+    require 'formula_support'
+    software_spec = SoftwareSpec.new(cask.url.to_s, cask.version)
+    downloader = CurlDownloadStrategy.new(cask.title, software_spec)
+    downloaded_path = downloader.fetch
+
+    _check_sums(downloaded_path, cask.sums) unless cask.sums === 0
+    downloaded_path
+  end
+
+  private
+  def _check_sums(path, sums)
+    has_sum = false
+    sums.each do |sum|
+      unless sum.empty?
+        computed = Checksum.new(sum.hash_type, Digest.const_get(sum.hash_type.to_s.upcase).file(path).hexdigest)
+        raise ChecksumMismatchError.new(sum, computed) unless sum == computed
+        has_sum = true
+      end
+    end
+    raise ChecksumMissingError.new("Checksum required. SHA1: '#{Digest::SHA1.file(path).hexdigest}'") unless has_sum
+  end
+end

lib/cask/installer.rb

@@ -1,4 +1,5 @@
 require 'digest'
+require 'cask/download'
 
 class Cask::Installer
   class << self
@@ -7,12 +8,8 @@ class Cask::Installer
         raise CaskAlreadyInstalledError.new(cask)
       end
 
-      require 'formula_support'
-      software_spec = SoftwareSpec.new(cask.url.to_s, cask.version)
-      downloader = CurlDownloadStrategy.new(cask.title, software_spec)
-      downloaded_path = downloader.fetch
-
-      _check_sums(downloaded_path, cask.sums) unless cask.sums === 0
+      download = Cask::Download.new(cask)
+      downloaded_path = download.perform
 
       FileUtils.mkdir_p cask.destination_path
 
@@ -31,18 +28,6 @@ class Cask::Installer
       cask.destination_path.rmtree
     end
 
-    def _check_sums(path, sums)
-      has_sum = false
-      sums.each do |sum|
-        unless sum.empty?
-          computed = Checksum.new(sum.hash_type, Digest.const_get(sum.hash_type.to_s.upcase).file(path).hexdigest)
-          raise ChecksumMismatchError.new(sum, computed) unless sum == computed
-          has_sum = true
-        end
-      end
-      raise ChecksumMissingError.new("Checksum required. SHA1: '#{Digest::SHA1.file(path).hexdigest}'") unless has_sum
-    end
-
     def _with_extracted_mountpoints(path)
       if _dmg?(path)
         File.open(path) do |dmg|

test/cask/audit_test.rb

@@ -68,6 +68,7 @@ describe Cask::Audit do
         audit.errors.must_include 'homepage is required'
       end
     end
+
     describe "preferred download URL formats" do
       it "adds a warning if SourceForge doesn't use download subdomain" do
         warning_msg = 'SourceForge URL format incorrect. See https://github.com/phinze/homebrew-cask/pull/225#issuecomment-16536889 for details'
@@ -81,5 +82,17 @@ describe Cask::Audit do
         audit.warnings.wont_include warning_msg
       end
     end
+
+    describe "audit of downloads" do
+      it "creates an error if the download fails" do
+        error_message = "Download Failed"
+        download = mock()
+        download.expects(:perform).raises(StandardError.new(error_message))
+
+        audit = Cask::Audit.new(TestHelper.test_cask)
+        audit.run!(download)
+        audit.errors.first.must_match(/#{error_message}/)
+      end
+    end
   end
 end

test/cli/audit_test.rb

@@ -0,0 +1,45 @@
+require 'test_helper'
+
+describe Cask::CLI::Audit do
+  let(:auditor) { mock() }
+  let(:cask) { mock() }
+
+  describe 'selection of casks to audit' do
+    it 'audits all casks if no names are given' do
+      Cask.stubs(:all => [cask, cask])
+      auditor.expects(:audit).times(2)
+
+      run_audit([], auditor)
+    end
+
+    it 'audits specified casks if names are given' do
+      cask_name = 'nice-app'
+      Cask.expects(:load).with(cask_name).returns(cask)
+      auditor.expects(:audit).with(cask, :audit_download => false)
+
+      run_audit([cask_name], auditor)
+    end
+  end
+
+  describe 'rules for downloading a cask' do
+    it 'does not download the cask per default' do
+      Cask.stubs(:load => cask)
+
+      auditor.expects(:audit).with(cask, :audit_download => false)
+
+      run_audit(['caskname'], auditor)
+    end
+
+    it 'download a cask if --download flag is set' do
+      Cask.stubs(:load => cask)
+
+      auditor.expects(:audit).with(cask, :audit_download => true)
+
+      run_audit(['caskname', '--download'], auditor)
+    end
+  end
+
+  def run_audit(args, auditor)
+    Cask::CLI::Audit.new(args, auditor).run
+  end
+end

test/test_helper.rb

@@ -23,6 +23,11 @@ HOMEBREW_CACHE.mkpath
 require 'minitest/autorun'
 require 'minitest-colorize'
 
+# Force mocha to patch MiniTest since we have both loaded thanks to homebrew's testing_env
+require 'mocha/api'
+require 'mocha/integration/mini_test'
+Mocha::Integration::MiniTest.activate
+
 # our baby
 require 'cask'