Merge pull request #16160 from jawshooah/audit-enhancements

Audit: add checks for name and appcast stanzas

lib/hbc/audit.rb

@@ -2,22 +2,21 @@ require 'hbc/checkable'
 require 'hbc/download'
 
 class Hbc::Audit
-  attr_reader :cask
-
   include Hbc::Checkable
 
+  attr_reader :cask
+
   def initialize(cask)
     @cask = cask
   end
 
   def run!(download = false)
-    _check_required_stanzas
-    _check_no_string_version_latest
-    _check_sha256_no_check_if_latest
-    _check_sha256_actually_256
-    _check_sha256_invalid
-    _check_sourceforge_download_url_format
-    _check_download(download) if download
+    check_required_stanzas
+    check_no_string_version_latest
+    check_sha256
+    check_appcast
+    check_sourceforge_download_url_format
+    check_download(download) if download
     return !(errors? or warnings?)
   end
 
@@ -25,51 +24,74 @@ class Hbc::Audit
     "audit for #{cask}"
   end
 
+  private
 
-  def _check_required_stanzas
+  def check_required_stanzas
     odebug "Auditing required stanzas"
     %i{version sha256 url homepage}.each do |sym|
       add_error "a #{sym} stanza is required" unless cask.send(sym)
     end
     add_error 'a license value is required (:unknown is OK)' unless cask.license
+    add_error 'at least one name stanza is required' if cask.full_name.empty?
     # todo: specific DSL knowledge should not be spread around in various files like this
     # todo: nested_container should not still be a pseudo-artifact at this point
     installable_artifacts = cask.artifacts.reject{ |k,v| [:uninstall, :zap, :nested_container].include?(k)}
     add_error 'at least one activatable artifact stanza is required' unless installable_artifacts.size > 0
   end
 
-  def _check_no_string_version_latest
+  def check_no_string_version_latest
     odebug "Verifying version :latest does not appear as a string ('latest')"
     if (cask.version == 'latest')
       add_error "you should use version :latest instead of version 'latest'"
     end
   end
 
-  def _check_sha256_no_check_if_latest
+  def check_sha256
+    return unless cask.sha256
+    check_sha256_no_check_if_latest
+    check_sha256_actually_256
+    check_sha256_invalid
+  end
+
+  def check_sha256_no_check_if_latest
     odebug "Verifying sha256 :no_check with version :latest"
     if cask.version == :latest and cask.sha256 != :no_check
       add_error "you should use sha256 :no_check when version is :latest"
     end
   end
 
-  def _check_sha256_actually_256
-    odebug "Verifying sha256 string is a legal SHA-256 digest"
-    if cask.sha256.kind_of?(String)
-      unless cask.sha256.length == 64 && cask.sha256[/^[0-9a-f]+$/i]
+  def check_sha256_actually_256(sha256: cask.sha256, stanza: 'sha256')
+    odebug "Verifying #{stanza} string is a legal SHA-256 digest"
+    if sha256.kind_of?(String)
+      unless sha256.length == 64 && sha256[/^[0-9a-f]+$/i]
         add_error "sha256 string must be of 64 hexadecimal characters"
       end
     end
   end
 
-  def _check_sha256_invalid
-    odebug "Verifying sha256 is not a known invalid value"
+  def check_sha256_invalid(sha256: cask.sha256, stanza: 'sha256')
+    odebug "Verifying #{stanza} is not a known invalid value"
     empty_sha256 = 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'
-    if cask.sha256 == empty_sha256
+    if sha256 == empty_sha256
       add_error "cannot use the sha256 for an empty string: #{empty_sha256}"
     end
   end
 
-  def _check_download(download)
+  def check_appcast
+    return unless cask.appcast
+    odebug 'Auditing appcast'
+    check_appcast_has_sha256
+    return unless cask.appcast.sha256
+    check_sha256_actually_256(sha256: cask.appcast.sha256, stanza: 'appcast :sha256')
+    check_sha256_invalid(sha256: cask.appcast.sha256, stanza: 'appcast :sha256')
+  end
+
+  def check_appcast_has_sha256
+    odebug 'Verifying appcast has :sha256 key'
+    add_error 'a sha256 is required for appcast' unless cask.appcast.sha256
+  end
+
+  def check_download(download)
     odebug "Auditing download"
     downloaded_path = download.perform
     Hbc::Verify.all(cask, downloaded_path)
@@ -77,14 +99,14 @@ class Hbc::Audit
     add_error "download not possible: #{e.message}"
   end
 
-  def _check_sourceforge_download_url_format
+  def check_sourceforge_download_url_format
     odebug "Auditing URL format"
-    if _bad_sourceforge_url?
+    if bad_sourceforge_url?
       add_warning "SourceForge URL format incorrect. See https://github.com/caskroom/homebrew-cask/blob/master/CONTRIBUTING.md#sourceforge-urls"
     end
   end
 
-  def _bad_sourceforge_url?
+  def bad_sourceforge_url?
     return false unless cask.url.to_s =~ /sourceforge/
     valid_url_formats = [
       %r{\Ahttps?://sourceforge\.net/projects/[^/]+/files/latest/download\Z},

spec/cask/audit_spec.rb

@@ -23,28 +23,64 @@ describe Hbc::Audit do
 
   describe "run!" do
     describe "required fields" do
-      it "adds an error if url is missing" do
-        audit = Hbc::Audit.new(Hbc.load('missing-url'))
-        audit.run!
-        expect(audit.errors).to include('a url stanza is required')
+      %w[version sha256 url homepage].each do |stanza|
+        it "adds an error if #{stanza} is missing" do
+          audit = Hbc::Audit.new(Hbc.load("missing-#{stanza}"))
+          audit.run!
+          expect(audit.errors).to include("a #{stanza} stanza is required")
+        end
       end
 
-      it "adds an error if version is missing" do
-        audit = Hbc::Audit.new(Hbc.load('missing-version'))
+      it "adds an error if license is missing" do
+        audit = Hbc::Audit.new(Hbc.load('missing-license'))
         audit.run!
-        expect(audit.errors).to include('a version stanza is required')
+        expect(audit.errors).to include('a license value is required (:unknown is OK)')
       end
 
-      it "adds an error if homepage is missing" do
-        audit = Hbc::Audit.new(Hbc.load('missing-homepage'))
+      it "adds an error if name is missing" do
+        audit = Hbc::Audit.new(Hbc.load('missing-name'))
         audit.run!
-        expect(audit.errors).to include('a homepage stanza is required')
+        expect(audit.errors).to include('at least one name stanza is required')
       end
+    end
 
-      it "adds an error if version is latest and using sha256" do
+    describe "sha256 checks" do
+      it "adds an error if version is :latest and sha256 is not :no_check" do
         audit = Hbc::Audit.new(Hbc.load('version-latest-with-checksum'))
         audit.run!
-        expect(audit.errors).to include(%q{you should use sha256 :no_check when version is :latest})
+        expect(audit.errors).to include('you should use sha256 :no_check when version is :latest')
+      end
+
+      it "adds an error if sha256 is not a legal SHA-256 digest" do
+        audit = Hbc::Audit.new(Hbc.load('invalid-sha256'))
+        audit.run!
+        expect(audit.errors).to include('sha256 string must be of 64 hexadecimal characters')
+      end
+
+      it "adds an error if sha256 is sha256 for empty string" do
+        audit = Hbc::Audit.new(Hbc.load('sha256-for-empty-string'))
+        audit.run!
+        expect(audit.errors).to include('cannot use the sha256 for an empty string: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855')
+      end
+    end
+
+    describe "appcast checks" do
+      it "adds an error if appcast has no sha256" do
+        audit = Hbc::Audit.new(Hbc.load('appcast-missing-sha256'))
+        audit.run!
+        expect(audit.errors).to include('a sha256 is required for appcast')
+      end
+
+      it "adds an error if appcast sha256 is not a string of 64 hexadecimal characters" do
+        audit = Hbc::Audit.new(Hbc.load('appcast-invalid-sha256'))
+        audit.run!
+        expect(audit.errors).to include('sha256 string must be of 64 hexadecimal characters')
+      end
+
+      it "adds an error if appcast sha256 is sha256 for empty string" do
+        audit = Hbc::Audit.new(Hbc.load('appcast-sha256-for-empty-string'))
+        audit.run!
+        expect(audit.errors).to include('cannot use the sha256 for an empty string: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855')
       end
     end
 

spec/support/Casks/appcast-invalid-sha256.rb

@@ -0,0 +1,4 @@
+test_cask 'appcast-invalid-sha256' do
+  appcast 'http://localhost/appcast.xml',
+          :sha256 => 'not a valid shasum'
+end

spec/support/Casks/appcast-missing-sha256.rb

@@ -0,0 +1,3 @@
+test_cask 'appcast-missing-sha256' do
+  appcast 'http://localhost/appcast.xml'
+end

spec/support/Casks/appcast-sha256-for-empty-string.rb

@@ -0,0 +1,4 @@
+test_cask 'appcast-sha256-for-empty-string' do
+  appcast 'http://localhost/appcast.xml',
+          :sha256 => 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'
+end

spec/support/Casks/invalid-sha256.rb

@@ -0,0 +1,4 @@
+test_cask 'invalid-sha256' do
+  version '1.2.3'
+  sha256 'not a valid shasum'
+end

spec/support/Casks/missing-license.rb

@@ -0,0 +1,4 @@
+test_cask 'missing-license' do
+  version '1.2.3'
+  url 'http://localhost/something.dmg'
+end

spec/support/Casks/missing-name.rb

@@ -0,0 +1,4 @@
+test_cask 'missing-name' do
+  version '1.2.3'
+  url 'http://localhost/something.dmg'
+end

spec/support/Casks/missing-sha256.rb

@@ -0,0 +1,4 @@
+test_cask 'missing-sha256' do
+  version '1.2.3'
+  url 'http://localhost/something.dmg'
+end

spec/support/Casks/sha256-for-empty-string.rb

@@ -0,0 +1,4 @@
+test_cask 'sha256-for-empty-string' do
+  version '1.2.3'
+  sha256 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'
+end

test/support/Casks/with-suite.rb

@@ -3,6 +3,7 @@ cask 'with-suite' do
   sha256 'd1302a0dc25aff72ad395ed01a830468b92253ffd28269574f3ac0b5eb8aad54'
 
   url TestHelper.local_binary_url('caffeine_suite.zip')
+  name 'Caffeine'
   homepage 'http://example.com/with-suite'
   license :unknown