From 3c37303088c7c6056421d28fd13cbf1c326e2c7e Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Mon, 12 Oct 2026 01:02:15 +1100 Subject: [PATCH] cask/audit: check signing of more artifact types - `audit_signing` only saw `app`, `pkg`, `binary` and manual installers, so suites, plugins, prefpanes, screen savers and other bundles were never Gatekeeper checked. - Scan bundle artifacts with `gktool scan`, as for `app`. - Walk `suite` and generic `artifact` folders, scanning each bundle and checking loose Mach-O executables for notarisation; `gktool` rejects any plain folder so the folder itself is not scanned. - Name the failing path in the error, as `gktool` output does not. - Check Mach-O `installer script:` executables for notarisation. - Select these through a new opt-in `include_all_signable_artifacts:` so the other `extract_artifacts` audits are unchanged and font casks still skip the download. --- Library/Homebrew/cask/audit.rb | 83 ++++++++++---- Library/Homebrew/test/cask/audit_spec.rb | 132 +++++++++++++++++++++-- 2 files changed, 190 insertions(+), 25 deletions(-) diff --git a/Library/Homebrew/cask/audit.rb b/Library/Homebrew/cask/audit.rb index 4ab6b203d8fb1..8ea8012729c7c 100644 --- a/Library/Homebrew/cask/audit.rb +++ b/Library/Homebrew/cask/audit.rb @@ -30,6 +30,12 @@ class Audit DOWNLOAD_TIMEOUT = 600 + GATEKEEPER_BUNDLE_ARTIFACTS = T.let([ + Artifact::App, Artifact::AudioUnitPlugin, Artifact::Colorpicker, Artifact::InputMethod, + Artifact::InternetPlugin, Artifact::Mdimporter, Artifact::Prefpane, Artifact::ScreenSaver, + Artifact::Service, Artifact::Vst3Plugin, Artifact::VstPlugin + ].freeze, T::Array[T.class_of(Artifact::Moved)]) + Error = T.type_alias do { message: T.nilable(String), @@ -168,14 +174,15 @@ def summary sig { params( - include_manual_installers: T::Boolean, - _block: T.nilable(T.proc.params( + include_manual_installers: T::Boolean, + include_all_signable_artifacts: T::Boolean, + _block: T.nilable(T.proc.params( arg0: T::Array[T.any(Artifact::Installer, Artifact::Pkg, Artifact::Relocated)], arg1: Pathname, ).void), ).void } - def extract_artifacts(include_manual_installers: false, &_block) + def extract_artifacts(include_manual_installers: false, include_all_signable_artifacts: false, &_block) return unless online? return if download.nil? || @download_failed @@ -186,7 +193,10 @@ def extract_artifacts(include_manual_installers: false, &_block) (include_manual_installers && artifact.is_a?(Artifact::Installer) && artifact.manual_install && - [".app", ".pkg"].include?(artifact.path.extname.downcase)) + [".app", ".pkg"].include?(artifact.path.extname.downcase)) || + (include_all_signable_artifacts && + [Artifact::Installer, Artifact::Suite, Artifact::Artifact, *GATEKEEPER_BUNDLE_ARTIFACTS] + .any? { artifact.is_a?(it) }) end if @artifacts_extracted && @tmpdir @@ -621,7 +631,7 @@ def audit_signing Homebrew::SimulateSystem.current_arch.to_s) || cask.tap&.audit_exception(:signing_audit_skiplist, cask.token, "all") - extract_artifacts(include_manual_installers: true) do |artifacts, tmpdir| + extract_artifacts(include_all_signable_artifacts: true) do |artifacts, tmpdir| is_container = artifacts.any? do |artifact| artifact.is_a?(Artifact::App) || artifact.is_a?(Artifact::Pkg) || (artifact.is_a?(Artifact::Installer) && [".app", ".pkg"].include?(artifact.path.extname.downcase)) @@ -647,18 +657,14 @@ def audit_signing result = case artifact when Artifact::Pkg - system_command("spctl", args: ["--assess", "--type", "install", path], print_stderr: false) - when Artifact::App - next opoo "gktool not found, skipping app signing audit" unless which("gktool") - - system_command("gktool", args: ["scan", path], print_stderr: false) + package_assessment(path) when Artifact::Installer if artifact.path.extname.downcase == ".app" - next opoo "gktool not found, skipping app signing audit" unless which("gktool") - - system_command("gktool", args: ["scan", path], print_stderr: false) + gatekeeper_scan(path) elsif artifact.path.extname.downcase == ".pkg" - system_command("spctl", args: ["--assess", "--type", "install", path], print_stderr: false) + package_assessment(path) + elsif !artifact.manual_install && BinaryPathname.wrap(path).binary_executable? + notarization_check(path) else next false end @@ -666,19 +672,39 @@ def audit_signing # Shell scripts cannot be signed, so we skip them next false if ::Utils::Path.text_executable?(path) - system_command("codesign", args: ["--verify", "-R=notarized", "--check-notarization", path], - print_stderr: false) + notarization_check(path) + when Artifact::Suite, Artifact::Artifact + # `gktool scan` fails any plain folder, so check the bundles and executables inside instead. + failed_path, failed_result = path.find do |found| + next if found.symlink? + + bundle = found.directory? && %w[ + .app .bundle .colorpicker .component .mdimporter .plugin .prefpane .saver .service .vst .vst3 + ].include?(found.extname.downcase) + found_result = if bundle + gatekeeper_scan(found) + elsif found.file? && found.executable? && BinaryPathname.wrap(found).binary_executable? + notarization_check(found) + end + break found, found_result if found_result&.success? == false + + Find.prune if bundle + end + path = failed_path if failed_path + failed_result + when *GATEKEEPER_BUNDLE_ARTIFACTS + gatekeeper_scan(path) else add_error "Unknown artifact type: #{artifact.class}", location: url.location next end - next false if result.success? + next false if result.nil? || result.success? next true if gatekeeper_failure_expected next true if is_in_skiplist signing_failure_message = <<~EOS - Signature verification failed: + Signature verification failed for #{path.relative_path_from(tmpdir)}: #{result.merged_output} EOS @@ -1219,6 +1245,27 @@ def audit_deprecate_disable private + sig { params(path: Pathname).returns(T.nilable(SystemCommand::Result)) } + def gatekeeper_scan(path) + unless which("gktool") + opoo "gktool not found, skipping bundle signing audit" + return + end + + system_command("gktool", args: ["scan", path], print_stderr: false) + end + + sig { params(path: Pathname).returns(SystemCommand::Result) } + def package_assessment(path) + system_command("spctl", args: ["--assess", "--type", "install", path], print_stderr: false) + end + + sig { params(path: Pathname).returns(SystemCommand::Result) } + def notarization_check(path) + system_command("codesign", args: ["--verify", "-R=notarized", "--check-notarization", path], + print_stderr: false) + end + sig { returns(T.nilable(Pathname)) } def fetch_download return if @download_failed diff --git a/Library/Homebrew/test/cask/audit_spec.rb b/Library/Homebrew/test/cask/audit_spec.rb index ed4191a05fb9a..974a4d9d55efd 100644 --- a/Library/Homebrew/test/cask/audit_spec.rb +++ b/Library/Homebrew/test/cask/audit_spec.rb @@ -811,21 +811,44 @@ def tmp_cask(name, text) allow(audit).to receive_messages(download: download_double, signing?: true) end - context "when cask is not using a signed artifact" do + test_each(["font 'Audit.ttf'", "qlplugin 'Audit.qlgenerator'"]) do |stanza| + context "when cask only has `#{stanza}`" do + let(:cask) do + tmp_cask "signing-cask-test", <<~RUBY + cask 'signing-cask-test' do + version '1.0' + url "https://brew.sh/index.html" + #{stanza} + end + RUBY + end + + it "does not fail" do + expect(download_double).not_to receive(:fetch) + expect(UnpackStrategy).not_to receive(:detect) + expect(run).not_to error_with(/Audit\.app/) + end + end + end + + context "when cask only has a suite" do + let(:online) { true } let(:cask) do tmp_cask "signing-cask-test", <<~RUBY cask 'signing-cask-test' do version '1.0' - url "https://brew.sh/index.html" - artifact "example.pdf", target: "/Library/Application Support/example" + url "https://brew.sh/" + suite 'Audit Suite' end RUBY end - it "does not fail" do - expect(download_double).not_to receive(:fetch) - expect(UnpackStrategy).not_to receive(:detect) - expect(run).not_to error_with(/Audit\.app/) + it "extracts it" do + allow(Cask::Quarantine).to receive(:available?).and_return(true) + allow(ObjectSpace).to receive(:define_finalizer) + allow(download_double).to receive(:fetch).and_return(Pathname.new("/tmp/test.zip")) + expect(UnpackStrategy).to receive(:detect).and_return(nil) + run end end @@ -920,6 +943,101 @@ def tmp_cask(name, text) run end end + + context "when cask has other signable artifacts" do + let(:tmpdir) { mktmpdir } + let(:passed_result) { instance_double(SystemCommand::Result, success?: true) } + let(:failed_result) do + instance_double(SystemCommand::Result, success?: false, merged_output: "rejected") + end + let(:cask_artifact) { "suite 'Audit Suite'" } + let(:cask) do + tmp_cask "signing-cask-test", <<~RUBY + cask 'signing-cask-test' do + version '1.0' + url "https://brew.sh/" + #{cask_artifact} + end + RUBY + end + let(:suite) { tmpdir/"Audit Suite" } + + def stub_check(command, *args, path) + allow(audit).to receive(:system_command) + .with(command, args: [*args, path], print_stderr: false).and_return(failed_result) + end + + before do + allow(cask).to receive(:tap).and_return(tap) + allow(Cask::Quarantine).to receive_messages(available?: true, detect: true) + allow(audit).to receive(:which).with("gktool").and_return(Pathname("/usr/bin/gktool")) + allow(audit).to receive(:system_command).and_return(passed_result) + allow(audit).to receive(:extract_artifacts).and_yield(cask.artifacts.to_a, tmpdir) + end + + test_each_hash({ + audio_unit_plugin: "Audit.component", + colorpicker: "Audit.colorPicker", + input_method: "Audit.app", + internet_plugin: "Audit.plugin", + mdimporter: "Audit.mdimporter", + prefpane: "Audit.prefPane", + screen_saver: "Audit.saver", + service: "Audit.service", + vst3_plugin: "Audit.vst3", + vst_plugin: "Audit.vst", + }) do |stanza, bundle| + context "when the #{stanza} fails Gatekeeper" do + let(:cask_artifact) { "#{stanza} '#{bundle}'" } + + it "errors" do + stub_check("gktool", "scan", tmpdir/bundle) + expect(run).to error_with(/Signature verification failed/) + end + end + end + + context "when a generic artifact bundle fails Gatekeeper" do + let(:cask_artifact) { "artifact 'Audit.plugin', target: '~/Library/Audit/Audit.plugin'" } + + it "errors" do + (tmpdir/"Audit.plugin").mkpath + stub_check("gktool", "scan", tmpdir/"Audit.plugin") + expect(run).to error_with(/Signature verification failed/) + end + end + + it "errors when an app in the suite fails Gatekeeper" do + (suite/"Good.app").mkpath + (suite/"Bad.app").mkpath + stub_check("gktool", "scan", suite/"Bad.app") + expect(run).to error_with(%r{Signature verification failed for Audit Suite/Bad\.app:}) + end + + it "does not check a suite without code" do + (suite/"lib").mkpath + FileUtils.touch suite/"lib/audit.jar" + expect(audit).not_to receive(:system_command) + run + end + + it "errors when a loose executable in the suite is not notarized", :needs_macos do + suite.mkpath + FileUtils.install TEST_FIXTURE_DIR/"mach/a.out", suite/"audit", mode: 0755 + stub_check("codesign", "--verify", "-R=notarized", "--check-notarization", suite/"audit") + expect(run).to error_with(%r{Signature verification failed for Audit Suite/audit:}) + end + + context "when an installer script is not notarized", :needs_macos do + let(:cask_artifact) { "installer script: { executable: 'audit-installer' }" } + + it "errors" do + FileUtils.install TEST_FIXTURE_DIR/"mach/a.out", tmpdir/"audit-installer", mode: 0755 + stub_check("codesign", "--verify", "-R=notarized", "--check-notarization", tmpdir/"audit-installer") + expect(run).to error_with(/Signature verification failed/) + end + end + end end describe "homepage domain age" do