Merge pull request #17302 from Homebrew/ww/redact-env

This commit is contained in:
Patrick Linnane 2024-05-14 14:01:26 -07:00 committed by GitHub
commit 1f603d381b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 85 additions and 66 deletions

View File

@ -5,9 +5,12 @@ require "date"
require "json" require "json"
require "utils/popen" require "utils/popen"
require "exceptions" require "exceptions"
require "system_command"
module Homebrew module Homebrew
module Attestation module Attestation
extend SystemCommand::Mixin
# @api private # @api private
HOMEBREW_CORE_REPO = "Homebrew/homebrew-core" HOMEBREW_CORE_REPO = "Homebrew/homebrew-core"
# @api private # @api private
@ -71,7 +74,7 @@ module Homebrew
signing_workflow: T.nilable(String), subject: T.nilable(String)).returns(T::Hash[T.untyped, T.untyped]) signing_workflow: T.nilable(String), subject: T.nilable(String)).returns(T::Hash[T.untyped, T.untyped])
} }
def self.check_attestation(bottle, signing_repo, signing_workflow = nil, subject = nil) def self.check_attestation(bottle, signing_repo, signing_workflow = nil, subject = nil)
cmd = [gh_executable, "attestation", "verify", bottle.cached_download, "--repo", signing_repo, "--format", cmd = ["attestation", "verify", bottle.cached_download, "--repo", signing_repo, "--format",
"json"] "json"]
cmd += ["--cert-identity", signing_workflow] if signing_workflow.present? cmd += ["--cert-identity", signing_workflow] if signing_workflow.present?
@ -83,7 +86,8 @@ module Homebrew
raise GhAuthNeeded, "missing credentials" if credentials.blank? raise GhAuthNeeded, "missing credentials" if credentials.blank?
begin begin
output = Utils.safe_popen_read({ "GH_TOKEN" => credentials }, *cmd) result = system_command!(gh_executable, args: cmd, env: { "GH_TOKEN" => credentials },
secrets: [credentials])
rescue ErrorDuringExecution => e rescue ErrorDuringExecution => e
# Even if we have credentials, they may be invalid or malformed. # Even if we have credentials, they may be invalid or malformed.
raise GhAuthNeeded, "invalid credentials" if e.status.exitstatus == 4 raise GhAuthNeeded, "invalid credentials" if e.status.exitstatus == 4
@ -92,7 +96,7 @@ module Homebrew
end end
begin begin
attestations = JSON.parse(output) attestations = JSON.parse(result.stdout)
rescue JSON::ParserError rescue JSON::ParserError
raise InvalidAttestationError, "attestation verification returned malformed JSON" raise InvalidAttestationError, "attestation verification returned malformed JSON"
end end

View File

@ -13,39 +13,45 @@ RSpec.describe Homebrew::Attestation do
let(:fake_bottle) do let(:fake_bottle) do
instance_double(Bottle, cached_download:, filename: fake_bottle_filename, url: fake_bottle_url) instance_double(Bottle, cached_download:, filename: fake_bottle_filename, url: fake_bottle_url)
end end
let(:fake_json_resp) do let(:fake_result_invalid_json) { instance_double(SystemCommand::Result, stdout: "\"invalid JSON") }
JSON.dump([ let(:fake_result_json_resp) do
{ verificationResult: { instance_double(SystemCommand::Result,
verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }], stdout: JSON.dump([
statement: { subject: [{ name: fake_bottle_filename.to_s }] }, { verificationResult: {
} }, verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }],
]) statement: { subject: [{ name: fake_bottle_filename.to_s }] },
} },
]))
end end
let(:fake_json_resp_backfill) do let(:fake_result_json_resp_backfill) do
JSON.dump([ digest = Digest::SHA256.hexdigest(fake_bottle_url)
{ verificationResult: { instance_double(SystemCommand::Result,
verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }], stdout: JSON.dump([
statement: { { verificationResult: {
subject: [{ name: "#{Digest::SHA256.hexdigest(fake_bottle_url)}--#{fake_bottle_filename}" }], verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }],
}, statement: {
} }, subject: [{ name: "#{digest}--#{fake_bottle_filename}" }],
]) },
} },
]))
end end
let(:fake_json_resp_too_new) do let(:fake_result_json_resp_too_new) do
JSON.dump([ instance_double(SystemCommand::Result,
{ verificationResult: { stdout: JSON.dump([
verifiedTimestamps: [{ timestamp: "2024-03-15T00:00:00Z" }], { verificationResult: {
statement: { subject: [{ name: fake_bottle_filename.to_s }] }, verifiedTimestamps: [{ timestamp: "2024-03-15T00:00:00Z" }],
} }, statement: { subject: [{ name: fake_bottle_filename.to_s }] },
]) } },
]))
end end
let(:fake_json_resp_wrong_sub) do let(:fake_json_resp_wrong_sub) do
JSON.dump([ instance_double(SystemCommand::Result,
{ verificationResult: { stdout: JSON.dump([
verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }], { verificationResult: {
statement: { subject: [{ name: "wrong-subject.tar.gz" }] }, verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }],
} }, statement: { subject: [{ name: "wrong-subject.tar.gz" }] },
]) } },
]))
end end
describe "::gh_executable" do describe "::gh_executable" do
@ -78,9 +84,10 @@ RSpec.describe Homebrew::Attestation do
expect(GitHub::API).to receive(:credentials) expect(GitHub::API).to receive(:credentials)
.and_return(fake_gh_creds) .and_return(fake_gh_creds)
expect(Utils).to receive(:safe_popen_read) expect(described_class).to receive(:system_command!)
.with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::HOMEBREW_CORE_REPO, "--format", "json") described_class::HOMEBREW_CORE_REPO, "--format", "json"],
env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds])
.and_raise(ErrorDuringExecution.new(["foo"], status: fake_error_status)) .and_raise(ErrorDuringExecution.new(["foo"], status: fake_error_status))
expect do expect do
@ -93,9 +100,10 @@ RSpec.describe Homebrew::Attestation do
expect(GitHub::API).to receive(:credentials) expect(GitHub::API).to receive(:credentials)
.and_return(fake_gh_creds) .and_return(fake_gh_creds)
expect(Utils).to receive(:safe_popen_read) expect(described_class).to receive(:system_command!)
.with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::HOMEBREW_CORE_REPO, "--format", "json") described_class::HOMEBREW_CORE_REPO, "--format", "json"],
env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds])
.and_raise(ErrorDuringExecution.new(["foo"], status: fake_auth_status)) .and_raise(ErrorDuringExecution.new(["foo"], status: fake_auth_status))
expect do expect do
@ -108,10 +116,11 @@ RSpec.describe Homebrew::Attestation do
expect(GitHub::API).to receive(:credentials) expect(GitHub::API).to receive(:credentials)
.and_return(fake_gh_creds) .and_return(fake_gh_creds)
expect(Utils).to receive(:safe_popen_read) expect(described_class).to receive(:system_command!)
.with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::HOMEBREW_CORE_REPO, "--format", "json") described_class::HOMEBREW_CORE_REPO, "--format", "json"],
.and_return("\"invalid json") env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds])
.and_return(fake_result_invalid_json)
expect do expect do
described_class.check_attestation fake_bottle, described_class.check_attestation fake_bottle,
@ -123,9 +132,10 @@ RSpec.describe Homebrew::Attestation do
expect(GitHub::API).to receive(:credentials) expect(GitHub::API).to receive(:credentials)
.and_return(fake_gh_creds) .and_return(fake_gh_creds)
expect(Utils).to receive(:safe_popen_read) expect(described_class).to receive(:system_command!)
.with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::HOMEBREW_CORE_REPO, "--format", "json") described_class::HOMEBREW_CORE_REPO, "--format", "json"],
env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds])
.and_return(fake_json_resp_wrong_sub) .and_return(fake_json_resp_wrong_sub)
expect do expect do
@ -145,43 +155,48 @@ RSpec.describe Homebrew::Attestation do
end end
it "calls gh with args for homebrew-core" do it "calls gh with args for homebrew-core" do
expect(Utils).to receive(:safe_popen_read) expect(described_class).to receive(:system_command!)
.with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity", described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity",
described_class::HOMEBREW_CORE_CI_URI) described_class::HOMEBREW_CORE_CI_URI],
.and_return(fake_json_resp) env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds])
.and_return(fake_result_json_resp)
described_class.check_core_attestation fake_bottle described_class.check_core_attestation fake_bottle
end end
it "calls gh with args for backfill when homebrew-core fails" do it "calls gh with args for backfill when homebrew-core fails" do
expect(Utils).to receive(:safe_popen_read) expect(described_class).to receive(:system_command!)
.with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity", described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity",
described_class::HOMEBREW_CORE_CI_URI) described_class::HOMEBREW_CORE_CI_URI],
env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds])
.once .once
.and_raise(described_class::InvalidAttestationError) .and_raise(described_class::InvalidAttestationError)
expect(Utils).to receive(:safe_popen_read) expect(described_class).to receive(:system_command!)
.with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::BACKFILL_REPO, "--format", "json") described_class::BACKFILL_REPO, "--format", "json"],
.and_return(fake_json_resp_backfill) env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds])
.and_return(fake_result_json_resp_backfill)
described_class.check_core_attestation fake_bottle described_class.check_core_attestation fake_bottle
end end
it "raises when the backfilled attestation is too new" do it "raises when the backfilled attestation is too new" do
expect(Utils).to receive(:safe_popen_read) expect(described_class).to receive(:system_command!)
.with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity", described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity",
described_class::HOMEBREW_CORE_CI_URI) described_class::HOMEBREW_CORE_CI_URI],
env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds])
.once .once
.and_raise(described_class::InvalidAttestationError) .and_raise(described_class::InvalidAttestationError)
expect(Utils).to receive(:safe_popen_read) expect(described_class).to receive(:system_command!)
.with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", .with(fake_gh, args: ["attestation", "verify", cached_download, "--repo",
described_class::BACKFILL_REPO, "--format", "json") described_class::BACKFILL_REPO, "--format", "json"],
.and_return(fake_json_resp_too_new) env: { "GH_TOKEN" => fake_gh_creds }, secrets: [fake_gh_creds])
.and_return(fake_result_json_resp_too_new)
expect do expect do
described_class.check_core_attestation fake_bottle described_class.check_core_attestation fake_bottle