From e5d47f85c98471128af3917a34119b3e00d2fe4b Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 5 Dec 2024 14:11:00 -0500 Subject: [PATCH 1/3] attestation: handle multiple subjects This should fix the behavior observed in https://github.com/Homebrew/homebrew-core/issues/177384#issuecomment-2521141910 and below. Signed-off-by: William Woodruff --- Library/Homebrew/attestation.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/attestation.rb b/Library/Homebrew/attestation.rb index d7479057ebea5..20c3b4aea6459 100644 --- a/Library/Homebrew/attestation.rb +++ b/Library/Homebrew/attestation.rb @@ -164,7 +164,10 @@ def self.check_attestation(bottle, signing_repo, signing_workflow = nil, subject # `gh attestation verify` returns a JSON array of one or more results, # for all attestations that match the input's digest. We want to additionally - # filter these down to just the attestation whose subject matches the bottle's name. + # filter these down to just the attestation whose subject(s) contain the bottle's name. + # As of 2024-12-04 GitHub's Artifact Attestation feature can put multiple subjects + # in a single attestation, so we check every subject in each attestation + # and select the first attestation with a matching subject. subject = bottle.filename.to_s if subject.blank? attestation = if bottle.tag.to_sym == :all @@ -175,12 +178,15 @@ def self.check_attestation(bottle, signing_repo, signing_workflow = nil, subject # This is sound insofar as the signature has already been verified. However, # longer term, we should also directly attest to `:all`-tagged bottles. attestations.find do |a| - actual_subject = a.dig("verificationResult", "statement", "subject", 0, "name") - actual_subject.start_with? "#{bottle.filename.name}--#{bottle.filename.version}" + candidate_subjects = a.dig("verificationResult", "statement", "subject") + candidate_subjects.any? do |candidate| + candidate["name"].start_with? "#{bottle.filename.name}--#{bottle.filename.version}" + end end else attestations.find do |a| - a.dig("verificationResult", "statement", "subject", 0, "name") == subject + candidate_subjects = a.dig("verificationResult", "statement", "subject") + candidate_subjects.any? { |candidate| candidate["name"] == subject } end end From 9e82563a3aea1bcddfcc803eb76fe527a13cb23c Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 5 Dec 2024 14:16:30 -0500 Subject: [PATCH 2/3] document the version that causes problems here Signed-off-by: William Woodruff --- Library/Homebrew/attestation.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Library/Homebrew/attestation.rb b/Library/Homebrew/attestation.rb index 20c3b4aea6459..00efbcad29ed0 100644 --- a/Library/Homebrew/attestation.rb +++ b/Library/Homebrew/attestation.rb @@ -168,6 +168,8 @@ def self.check_attestation(bottle, signing_repo, signing_workflow = nil, subject # As of 2024-12-04 GitHub's Artifact Attestation feature can put multiple subjects # in a single attestation, so we check every subject in each attestation # and select the first attestation with a matching subject. + # In particular, this happens with v2.0.0 and later of the + # `actions/attest-build-provenance` action. subject = bottle.filename.to_s if subject.blank? attestation = if bottle.tag.to_sym == :all From 057a6c5f9b6d1cc5a240925e13f21f535827288e Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 5 Dec 2024 14:26:30 -0500 Subject: [PATCH 3/3] add multi subject test Signed-off-by: William Woodruff --- Library/Homebrew/test/attestation_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Library/Homebrew/test/attestation_spec.rb b/Library/Homebrew/test/attestation_spec.rb index 48d85577178a6..298e8d3a0a3e2 100644 --- a/Library/Homebrew/test/attestation_spec.rb +++ b/Library/Homebrew/test/attestation_spec.rb @@ -34,6 +34,15 @@ } }, ])) end + let(:fake_result_json_resp_multi_subject) do + instance_double(SystemCommand::Result, + stdout: JSON.dump([ + { verificationResult: { + verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }], + statement: { subject: [{ name: "nonsense" }, { name: fake_bottle_filename.to_s }] }, + } }, + ])) + end let(:fake_result_json_resp_backfill) do digest = Digest::SHA256.hexdigest(fake_bottle_url) instance_double(SystemCommand::Result, @@ -234,6 +243,17 @@ described_class.check_core_attestation fake_bottle end + it "calls gh with args for homebrew-core and handles a multi-subject attestation" do + expect(described_class).to receive(:system_command!) + .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo", + described_class::HOMEBREW_CORE_REPO, "--format", "json"], + env: { "GH_TOKEN" => fake_gh_creds, "GH_HOST" => "github.com" }, secrets: [fake_gh_creds], + print_stderr: false, chdir: HOMEBREW_TEMP) + .and_return(fake_result_json_resp_multi_subject) + + described_class.check_core_attestation fake_bottle + end + it "calls gh with args for backfill when homebrew-core attestation is missing" do expect(described_class).to receive(:system_command!) .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",