From 3e428f76762a7e76b81cd502addabfd855cc3fe6 Mon Sep 17 00:00:00 2001 From: Michael Cho Date: Tue, 19 Dec 2023 13:19:40 -0500 Subject: [PATCH] formula_auditor: split out checksum check --- Library/Homebrew/formula_auditor.rb | 37 +++-- Library/Homebrew/test/dev-cmd/audit_spec.rb | 150 ++++++++++++-------- 2 files changed, 115 insertions(+), 72 deletions(-) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index da319e10937f6..2e64090dda3ed 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -806,24 +806,11 @@ def audit_revision_and_version_scheme return if formula.stable.blank? current_version = formula.stable.version - current_checksum = formula.stable.checksum current_version_scheme = formula.version_scheme current_revision = formula.revision - current_url = formula.stable.url previous_committed, newest_committed = committed_version_info - if current_version == newest_committed[:version] && - current_url == newest_committed[:url] && - current_checksum != newest_committed[:checksum] && - current_checksum.present? && newest_committed[:checksum].present? - problem( - "stable sha256 changed without the url/version also changing; " \ - "please create an issue upstream to rule out malicious " \ - "circumstances and to find out why the file changed.", - ) - end - unless previous_committed[:version_scheme].nil? if current_version_scheme < previous_committed[:version_scheme] problem "version_scheme should not decrease (from #{previous_committed[:version_scheme]} " \ @@ -849,6 +836,30 @@ def audit_revision_and_version_scheme end end + def audit_unconfirmed_checksum_change + return unless @git + return unless formula.tap # skip formula not from core or any taps + return unless formula.tap.git? # git log is required + return if formula.stable.blank? + + current_version = formula.stable.version + current_checksum = formula.stable.checksum + current_url = formula.stable.url + + _, newest_committed = committed_version_info + + if current_version == newest_committed[:version] && + current_url == newest_committed[:url] && + current_checksum != newest_committed[:checksum] && + current_checksum.present? && newest_committed[:checksum].present? + problem( + "stable sha256 changed without the url/version also changing; " \ + "please create an issue upstream to rule out malicious " \ + "circumstances and to find out why the file changed.", + ) + end + end + def audit_text bin_names = Set.new bin_names << formula.name diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb index b7d5ae260541c..cf0d39c095119 100644 --- a/Library/Homebrew/test/dev-cmd/audit_spec.rb +++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb @@ -1009,65 +1009,6 @@ class Foo < Formula end end - describe "checksums" do - describe "should not change with the same version" do - before do - formula_gsub( - 'sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - ) - end - - it { is_expected.to match("stable sha256 changed without the url/version also changing") } - end - - describe "should not change with the same version when not the first commit" do - before do - formula_gsub_origin_commit( - 'sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - ) - formula_gsub_origin_commit "revision 2" - formula_gsub_origin_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" - formula_gsub( - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - 'sha256 "e048c5e6144f5932d8672c2fade81d9073d5b3ca1517b84df006de3d25414fc1"', - ) - end - - it { is_expected.to match("stable sha256 changed without the url/version also changing") } - end - - describe "can change with the different version" do - before do - formula_gsub_origin_commit( - 'sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - ) - formula_gsub "foo-1.0.tar.gz", "foo-1.1.tar.gz" - formula_gsub_origin_commit( - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - 'sha256 "e048c5e6144f5932d8672c2fade81d9073d5b3ca1517b84df006de3d25414fc1"', - ) - end - - it { is_expected.to be_nil } - end - - describe "can be removed when switching schemes" do - before do - formula_gsub_origin_commit( - 'url "https://brew.sh/foo-1.0.tar.gz"', - 'url "https://foo.com/brew/bar.git", tag: "1.0", revision: "f5e00e485e7aa4c5baa20355b27e3b84a6912790"', - ) - formula_gsub_origin_commit('sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', - "") - end - - it { is_expected.to be_nil } - end - end - describe "revisions" do describe "should not be removed when first committed above 0" do it { is_expected.to be_nil } @@ -1173,6 +1114,97 @@ class Foo < Formula end end + describe "#audit_unconfirmed_checksum_change" do + subject do + fa = described_class.new(Formulary.factory(formula_path), git: true) + fa.audit_unconfirmed_checksum_change + fa.problems.first&.fetch(:message) + end + + before do + origin_formula_path.dirname.mkpath + origin_formula_path.write <<~RUBY + class Foo#{foo_version} < Formula + url "https://brew.sh/foo-1.0.tar.gz" + sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e" + revision 2 + version_scheme 1 + end + RUBY + + origin_tap_path.mkpath + origin_tap_path.cd do + system "git", "init" + system "git", "add", "--all" + system "git", "commit", "-m", "init" + end + + tap_path.mkpath + tap_path.cd do + system "git", "clone", origin_tap_path, "." + end + end + + describe "checksums" do + describe "should not change with the same version" do + before do + formula_gsub( + 'sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', + 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', + ) + end + + it { is_expected.to match("stable sha256 changed without the url/version also changing") } + end + + describe "should not change with the same version when not the first commit" do + before do + formula_gsub_origin_commit( + 'sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', + 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', + ) + formula_gsub_origin_commit "revision 2" + formula_gsub_origin_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" + formula_gsub( + 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', + 'sha256 "e048c5e6144f5932d8672c2fade81d9073d5b3ca1517b84df006de3d25414fc1"', + ) + end + + it { is_expected.to match("stable sha256 changed without the url/version also changing") } + end + + describe "can change with the different version" do + before do + formula_gsub_origin_commit( + 'sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', + 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', + ) + formula_gsub "foo-1.0.tar.gz", "foo-1.1.tar.gz" + formula_gsub_origin_commit( + 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', + 'sha256 "e048c5e6144f5932d8672c2fade81d9073d5b3ca1517b84df006de3d25414fc1"', + ) + end + + it { is_expected.to be_nil } + end + + describe "can be removed when switching schemes" do + before do + formula_gsub_origin_commit( + 'url "https://brew.sh/foo-1.0.tar.gz"', + 'url "https://foo.com/brew/bar.git", tag: "1.0", revision: "f5e00e485e7aa4c5baa20355b27e3b84a6912790"', + ) + formula_gsub_origin_commit('sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', + "") + end + + it { is_expected.to be_nil } + end + end + end + describe "#audit_versioned_keg_only" do specify "it warns when a versioned formula is not `keg_only`" do fa = formula_auditor "foo@1.1", <<~RUBY, core_tap: true