diff --git a/Library/Homebrew/cask/audit.rb b/Library/Homebrew/cask/audit.rb index 6f80d2f585..b095905211 100644 --- a/Library/Homebrew/cask/audit.rb +++ b/Library/Homebrew/cask/audit.rb @@ -6,6 +6,7 @@ require "cask/download" require "digest" require "utils/curl" require "utils/git" +require "utils/notability" module Cask class Audit @@ -14,22 +15,22 @@ module Cask attr_reader :cask, :commit_range, :download - attr_predicate :check_appcast? + attr_predicate :appcast? - def initialize(cask, check_appcast: false, download: false, check_token_conflicts: false, - commit_range: nil, command: SystemCommand) + def initialize(cask, appcast: false, download: false, + token_conflicts: false, online: false, strict: false, + new_cask: false, commit_range: nil, command: SystemCommand) @cask = cask - @check_appcast = check_appcast + @appcast = appcast @download = download + @online = online + @strict = strict + @new_cask = new_cask @commit_range = commit_range - @check_token_conflicts = check_token_conflicts + @token_conflicts = token_conflicts @command = command end - def check_token_conflicts? - @check_token_conflicts - end - def run! check_blacklist check_required_stanzas @@ -48,6 +49,9 @@ module Cask check_latest_with_auto_updates check_stanza_requires_uninstall check_appcast_contains_version + check_github_repository + check_gitlab_repository + check_bitbucket_repository self rescue => e odebug "#{e.message}\n#{e.backtrace.join("\n")}" @@ -272,7 +276,7 @@ module Cask end def check_token_conflicts - return unless check_token_conflicts? + return unless @token_conflicts return unless core_formula_names.include?(cask.token) add_warning "possible duplicate, cask token conflicts with Homebrew core formula: #{core_formula_url}" @@ -301,7 +305,7 @@ module Cask end def check_appcast_contains_version - return unless check_appcast? + return unless appcast? return if cask.appcast.to_s.empty? return if cask.appcast.configuration == :no_check @@ -322,6 +326,50 @@ module Cask add_error "appcast at URL '#{appcast_stanza}' offline or looping" end + def check_github_repository + user, repo = get_repo_data(%r{https?://github\.com/([^/]+)/([^/]+)/?.*}) + return if user.nil? + + odebug "Auditing GitHub repo" + + error = SharedAudits.github(user, repo) + add_error error if error + end + + def check_gitlab_repository + user, repo = get_repo_data(%r{https?://gitlab\.com/([^/]+)/([^/]+)/?.*}) + return if user.nil? + + odebug "Auditing GitLab repo" + + error = SharedAudits.gitlab(user, repo) + add_error error if error + end + + def check_bitbucket_repository + user, repo = get_repo_data(%r{https?://bitbucket\.org/([^/]+)/([^/]+)/?.*}) + return if user.nil? + + odebug "Auditing Bitbucket repo" + + error = SharedAudits.bitbucket(user, repo) + add_error error if error + end + + def get_repo_data(regex) + return unless @online + return unless @new_cask + + _, user, repo = *regex.match(cask.url.to_s) + _, user, repo = *regex.match(cask.homepage) unless user + _, user, repo = *regex.match(cask.appcast.to_s) unless user + return if !user || !repo + + repo.gsub!(/.git$/, "") + + [user, repo] + end + def check_blacklist return if cask.tap&.user != "Homebrew" return unless reason = Blacklist.blacklisted_reason(cask.token) diff --git a/Library/Homebrew/cask/auditor.rb b/Library/Homebrew/cask/auditor.rb index db3cfa7317..4f209120dc 100644 --- a/Library/Homebrew/cask/auditor.rb +++ b/Library/Homebrew/cask/auditor.rb @@ -8,34 +8,37 @@ module Cask extend Predicable def self.audit(cask, audit_download: false, audit_appcast: false, - check_token_conflicts: false, quarantine: true, commit_range: nil) + audit_online: false, audit_strict: false, + audit_token_conflicts: false, audit_new_cask: false, + quarantine: true, commit_range: nil) new(cask, audit_download: audit_download, audit_appcast: audit_appcast, - check_token_conflicts: check_token_conflicts, + audit_online: audit_online, + audit_new_cask: audit_new_cask, + audit_strict: audit_strict, + audit_token_conflicts: audit_token_conflicts, quarantine: quarantine, commit_range: commit_range).audit end attr_reader :cask, :commit_range def initialize(cask, audit_download: false, audit_appcast: false, - check_token_conflicts: false, quarantine: true, commit_range: nil) + audit_online: false, audit_strict: false, + audit_token_conflicts: false, audit_new_cask: false, + quarantine: true, commit_range: nil) @cask = cask @audit_download = audit_download @audit_appcast = audit_appcast + @audit_online = audit_online + @audit_strict = audit_strict + @audit_new_cask = audit_new_cask @quarantine = quarantine @commit_range = commit_range - @check_token_conflicts = check_token_conflicts + @audit_token_conflicts = audit_token_conflicts end - def audit_download? - @audit_download - end - - attr_predicate :audit_appcast?, :quarantine? - - def check_token_conflicts? - @check_token_conflicts - end + attr_predicate :audit_appcast?, :audit_download?, :audit_online?, + :audit_strict?, :audit_new_cask?, :audit_token_conflicts?, :quarantine? def audit if !Homebrew.args.value("language") && language_blocks @@ -64,10 +67,13 @@ module Cask def audit_cask_instance(cask) download = audit_download? && Download.new(cask, quarantine: quarantine?) - audit = Audit.new(cask, check_appcast: audit_appcast?, - download: download, - check_token_conflicts: check_token_conflicts?, - commit_range: commit_range) + audit = Audit.new(cask, appcast: audit_appcast?, + online: audit_online?, + strict: audit_strict?, + new_cask: audit_new_cask?, + token_conflicts: audit_token_conflicts?, + download: download, + commit_range: commit_range) audit.run! puts audit.summary audit.success? diff --git a/Library/Homebrew/cask/cmd/audit.rb b/Library/Homebrew/cask/cmd/audit.rb index f046b91df4..c2b94dba98 100644 --- a/Library/Homebrew/cask/cmd/audit.rb +++ b/Library/Homebrew/cask/cmd/audit.rb @@ -1,32 +1,66 @@ # frozen_string_literal: true +require "cli/parser" + module Cask class Cmd class Audit < AbstractCommand - option "--download", :download, false - option "--appcast", :appcast, false - option "--token-conflicts", :token_conflicts, false + option "--download", :download_arg, false + option "--appcast", :appcast_arg, false + option "--token-conflicts", :token_conflicts_arg, false + option "--strict", :strict_arg, false + option "--online", :online_arg, false + option "--new-cask", :new_cask_arg, false + + def self.usage + <<~EOS + `cask audit` [] [] + + --strict - Run additional, stricter style checks. + --online - Run additional, slower style checks that require a network connection. + --new-cask - Run various additional style checks to determine if a new cask is eligible + for Homebrew. This should be used when creating new casks and implies + `--strict` and `--online`. + --download - Audit the downloaded file + --appcast - Audit the appcast + --token-conflicts - Audit for token conflicts + + Check for Homebrew coding style violations. This should be run before + submitting a new cask. If no are provided, check all locally + available casks. Will exit with a non-zero status if any errors are + found, which can be useful for implementing pre-commit hooks. + EOS + end def self.help "verifies installability of Casks" end def run + Homebrew.auditing = true + strict = new_cask_arg? || strict_arg? + token_conflicts = strict || token_conflicts_arg? + + online = new_cask_arg? || online_arg? + download = online || download_arg? + appcast = online || appcast_arg? + failed_casks = casks(alternative: -> { Cask.to_a }) - .reject { |cask| audit(cask) } + .reject do |cask| + odebug "Auditing Cask #{cask}" + Auditor.audit(cask, audit_download: download, + audit_appcast: appcast, + audit_online: online, + audit_strict: strict, + audit_new_cask: new_cask_arg?, + audit_token_conflicts: token_conflicts, + quarantine: quarantine?) + end return if failed_casks.empty? raise CaskError, "audit failed for casks: #{failed_casks.join(" ")}" end - - def audit(cask) - odebug "Auditing Cask #{cask}" - Auditor.audit(cask, audit_download: download?, - audit_appcast: appcast?, - check_token_conflicts: token_conflicts?, - quarantine: quarantine?) - end end end end diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index cd08395aa7..b72a88f6e3 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -3,6 +3,7 @@ require "formula" require "formula_versions" require "utils/curl" +require "utils/notability" require "extend/ENV" require "formula_cellar_checks" require "cmd/search" @@ -510,79 +511,30 @@ module Homebrew user, repo = get_repo_data(%r{https?://github\.com/([^/]+)/([^/]+)/?.*}) return if user.nil? - begin - metadata = GitHub.repository(user, repo) - rescue GitHub::HTTPNotFoundError - return - end + warning = SharedAudits.github(user, repo) + return if warning.nil? - return if metadata.nil? - - new_formula_problem "GitHub fork (not canonical repository)" if metadata["fork"] - if (metadata["forks_count"] < 30) && (metadata["subscribers_count"] < 30) && - (metadata["stargazers_count"] < 75) - new_formula_problem "GitHub repository not notable enough (<30 forks, <30 watchers and <75 stars)" - end - - return if Date.parse(metadata["created_at"]) <= (Date.today - 30) - - new_formula_problem "GitHub repository too new (<30 days old)" + new_formula_problem warning end def audit_gitlab_repository user, repo = get_repo_data(%r{https?://gitlab\.com/([^/]+)/([^/]+)/?.*}) return if user.nil? - out, _, status= curl_output("--request", "GET", "https://gitlab.com/api/v4/projects/#{user}%2F#{repo}") - return unless status.success? + warning = SharedAudits.gitlab(user, repo) + return if warning.nil? - metadata = JSON.parse(out) - return if metadata.nil? - - new_formula_problem "GitLab fork (not canonical repository)" if metadata["fork"] - if (metadata["forks_count"] < 30) && (metadata["star_count"] < 75) - new_formula_problem "GitLab repository not notable enough (<30 forks and <75 stars)" - end - - return if Date.parse(metadata["created_at"]) <= (Date.today - 30) - - new_formula_problem "GitLab repository too new (<30 days old)" + new_formula_problem warning end def audit_bitbucket_repository user, repo = get_repo_data(%r{https?://bitbucket\.org/([^/]+)/([^/]+)/?.*}) return if user.nil? - api_url = "https://api.bitbucket.org/2.0/repositories/#{user}/#{repo}" - out, _, status= curl_output("--request", "GET", api_url) - return unless status.success? + warning = SharedAudits.bitbucket(user, repo) + return if warning.nil? - metadata = JSON.parse(out) - return if metadata.nil? - - new_formula_problem "Uses deprecated mercurial support in Bitbucket" if metadata["scm"] == "hg" - - new_formula_problem "Bitbucket fork (not canonical repository)" unless metadata["parent"].nil? - - if Date.parse(metadata["created_on"]) >= (Date.today - 30) - new_formula_problem "Bitbucket repository too new (<30 days old)" - end - - forks_out, _, forks_status= curl_output("--request", "GET", "#{api_url}/forks") - return unless forks_status.success? - - watcher_out, _, watcher_status= curl_output("--request", "GET", "#{api_url}/watchers") - return unless watcher_status.success? - - forks_metadata = JSON.parse(forks_out) - return if forks_metadata.nil? - - watcher_metadata = JSON.parse(watcher_out) - return if watcher_metadata.nil? - - return if (forks_metadata["size"] < 30) && (watcher_metadata["size"] < 75) - - new_formula_problem "Bitbucket repository not notable enough (<30 forks and <75 watchers)" + new_formula_problem warning end def get_repo_data(regex) diff --git a/Library/Homebrew/test/cask/audit_spec.rb b/Library/Homebrew/test/cask/audit_spec.rb index 48e984e476..4533bf825c 100644 --- a/Library/Homebrew/test/cask/audit_spec.rb +++ b/Library/Homebrew/test/cask/audit_spec.rb @@ -39,12 +39,12 @@ describe Cask::Audit, :cask do let(:cask) { instance_double(Cask::Cask) } let(:download) { false } - let(:check_token_conflicts) { false } + let(:token_conflicts) { false } let(:fake_system_command) { class_double(SystemCommand) } let(:audit) { - described_class.new(cask, download: download, - check_token_conflicts: check_token_conflicts, - command: fake_system_command) + described_class.new(cask, download: download, + token_conflicts: token_conflicts, + command: fake_system_command) } describe "#result" do @@ -517,7 +517,7 @@ describe Cask::Audit, :cask do describe "token conflicts" do let(:cask_token) { "with-binary" } - let(:check_token_conflicts) { true } + let(:token_conflicts) { true } context "when cask token conflicts with a core formula" do let(:formula_names) { %w[with-binary other-formula] } diff --git a/Library/Homebrew/test/cask/cmd/audit_spec.rb b/Library/Homebrew/test/cask/cmd/audit_spec.rb index cc83e7b10d..0411531760 100644 --- a/Library/Homebrew/test/cask/cmd/audit_spec.rb +++ b/Library/Homebrew/test/cask/cmd/audit_spec.rb @@ -21,7 +21,13 @@ describe Cask::Cmd::Audit, :cask do expect(Cask::CaskLoader).to receive(:load).with(cask_token).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_download: false, audit_appcast: false, check_token_conflicts: false, quarantine: true) + .with(cask, audit_download: false, + audit_appcast: false, + audit_token_conflicts: false, + audit_new_cask: false, + audit_online: false, + audit_strict: false, + quarantine: true) .and_return(true) described_class.run(cask_token) @@ -32,7 +38,13 @@ describe Cask::Cmd::Audit, :cask do it "does not download the Cask per default" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_download: false, audit_appcast: false, check_token_conflicts: false, quarantine: true) + .with(cask, audit_download: false, + audit_appcast: false, + audit_token_conflicts: false, + audit_new_cask: false, + audit_online: false, + audit_strict: false, + quarantine: true) .and_return(true) described_class.run("casktoken") @@ -41,7 +53,13 @@ describe Cask::Cmd::Audit, :cask do it "download a Cask if --download flag is set" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_download: true, audit_appcast: false, check_token_conflicts: false, quarantine: true) + .with(cask, audit_download: true, + audit_appcast: false, + audit_token_conflicts: false, + audit_new_cask: false, + audit_online: false, + audit_strict: false, + quarantine: true) .and_return(true) described_class.run("casktoken", "--download") @@ -52,7 +70,13 @@ describe Cask::Cmd::Audit, :cask do it "does not check for token conflicts per default" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_download: false, audit_appcast: false, check_token_conflicts: false, quarantine: true) + .with(cask, audit_download: false, + audit_appcast: false, + audit_token_conflicts: false, + audit_new_cask: false, + audit_online: false, + audit_strict: false, + quarantine: true) .and_return(true) described_class.run("casktoken") @@ -61,10 +85,112 @@ describe Cask::Cmd::Audit, :cask do it "checks for token conflicts if --token-conflicts flag is set" do allow(Cask::CaskLoader).to receive(:load).and_return(cask) expect(Cask::Auditor).to receive(:audit) - .with(cask, audit_download: false, audit_appcast: false, check_token_conflicts: true, quarantine: true) + .with(cask, audit_download: false, + audit_appcast: false, + audit_token_conflicts: true, + audit_new_cask: false, + audit_online: false, + audit_strict: false, + quarantine: true) .and_return(true) described_class.run("casktoken", "--token-conflicts") end end + + describe "rules for checking strictly" do + it "does not check strictly per default" do + allow(Cask::CaskLoader).to receive(:load).and_return(cask) + expect(Cask::Auditor).to receive(:audit) + .with(cask, audit_download: false, + audit_appcast: false, + audit_token_conflicts: false, + audit_new_cask: false, + audit_online: false, + audit_strict: false, + quarantine: true) + .and_return(true) + + described_class.run("casktoken") + end + + it "checks strictly if --strict flag is set" do + allow(Cask::CaskLoader).to receive(:load).and_return(cask) + expect(Cask::Auditor).to receive(:audit) + .with(cask, audit_download: false, + audit_appcast: false, + audit_token_conflicts: true, + audit_new_cask: false, + audit_online: false, + audit_strict: true, + quarantine: true) + .and_return(true) + + described_class.run("casktoken", "--strict") + end + end + + describe "rules for checking online" do + it "does not check online per default" do + allow(Cask::CaskLoader).to receive(:load).and_return(cask) + expect(Cask::Auditor).to receive(:audit) + .with(cask, audit_download: false, + audit_appcast: false, + audit_token_conflicts: false, + audit_new_cask: false, + audit_online: false, + audit_strict: false, + quarantine: true) + .and_return(true) + + described_class.run("casktoken") + end + + it "checks online if --online flag is set" do + allow(Cask::CaskLoader).to receive(:load).and_return(cask) + expect(Cask::Auditor).to receive(:audit) + .with(cask, audit_download: true, + audit_appcast: true, + audit_token_conflicts: false, + audit_new_cask: false, + audit_online: true, + audit_strict: false, + quarantine: true) + .and_return(true) + + described_class.run("casktoken", "--online") + end + end + + describe "rules for checking new casks" do + it "does not check new casks per default" do + allow(Cask::CaskLoader).to receive(:load).and_return(cask) + expect(Cask::Auditor).to receive(:audit) + .with(cask, audit_download: false, + audit_appcast: false, + audit_token_conflicts: false, + audit_new_cask: false, + audit_online: false, + audit_strict: false, + quarantine: true) + .and_return(true) + + described_class.run("casktoken") + end + + it "checks new casks if --new-cask flag is set" do + allow(Cask::CaskLoader).to receive(:load).and_return(cask) + expect(Cask::Auditor).to receive(:audit) + .with(cask, audit_download: true, + audit_appcast: true, + audit_token_conflicts: true, + audit_new_cask: true, + audit_online: true, + audit_strict: true, + quarantine: true) + .and_return(true) + + described_class.run("casktoken", "--new-cask") + end + end end diff --git a/Library/Homebrew/utils/notability.rb b/Library/Homebrew/utils/notability.rb new file mode 100644 index 0000000000..ef6099844d --- /dev/null +++ b/Library/Homebrew/utils/notability.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require "utils/curl" + +module SharedAudits + module_function + + def github(user, repo) + begin + metadata = GitHub.repository(user, repo) + rescue GitHub::HTTPNotFoundError + return + end + + return if metadata.nil? + + return "GitHub fork (not canonical repository)" if metadata["fork"] + if (metadata["forks_count"] < 30) && (metadata["subscribers_count"] < 30) && + (metadata["stargazers_count"] < 75) + return "GitHub repository not notable enough (<30 forks, <30 watchers and <75 stars)" + end + + return if Date.parse(metadata["created_at"]) <= (Date.today - 30) + + "GitHub repository too new (<30 days old)" + end + + def gitlab(user, repo) + out, _, status= curl_output("--request", "GET", "https://gitlab.com/api/v4/projects/#{user}%2F#{repo}") + return unless status.success? + + metadata = JSON.parse(out) + return if metadata.nil? + + return "GitLab fork (not canonical repository)" if metadata["fork"] + if (metadata["forks_count"] < 30) && (metadata["star_count"] < 75) + return "GitLab repository not notable enough (<30 forks and <75 stars)" + end + + return if Date.parse(metadata["created_at"]) <= (Date.today - 30) + + "GitLab repository too new (<30 days old)" + end + + def bitbucket(user, repo) + api_url = "https://api.bitbucket.org/2.0/repositories/#{user}/#{repo}" + out, _, status= curl_output("--request", "GET", api_url) + return unless status.success? + + metadata = JSON.parse(out) + return if metadata.nil? + + return "Uses deprecated mercurial support in Bitbucket" if metadata["scm"] == "hg" + + return "Bitbucket fork (not canonical repository)" unless metadata["parent"].nil? + + return "Bitbucket repository too new (<30 days old)" if Date.parse(metadata["created_on"]) >= (Date.today - 30) + + forks_out, _, forks_status= curl_output("--request", "GET", "#{api_url}/forks") + return unless forks_status.success? + + watcher_out, _, watcher_status= curl_output("--request", "GET", "#{api_url}/watchers") + return unless watcher_status.success? + + forks_metadata = JSON.parse(forks_out) + return if forks_metadata.nil? + + watcher_metadata = JSON.parse(watcher_out) + return if watcher_metadata.nil? + + return if (forks_metadata["size"] < 30) && (watcher_metadata["size"] < 75) + + "Bitbucket repository not notable enough (<30 forks and <75 watchers)" + end +end