-
Notifications
You must be signed in to change notification settings - Fork 10
Add macos_verify_code_signing action
#757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
624d39e
004b7b6
419d130
090fe89
898632d
22b0599
eb24b50
0316198
a734d64
70ca8d8
679af96
fd9fe78
70e8a2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'fastlane/action' | ||
| require 'fastlane_core/ui/ui' | ||
|
|
||
| module Fastlane | ||
| module Actions | ||
| class MacosVerifyCodeSigningAction < Action | ||
| def self.run(params) | ||
| paths = params[:artifact_path] | ||
| UI.user_error!('No artifact to verify: `artifact_path` is empty') if paths.empty? | ||
|
|
||
| paths.each do |path| | ||
| UI.user_error!("There is no artifact at #{path}") unless File.exist?(path) | ||
|
|
||
| UI.message("Verifying #{path}") | ||
|
|
||
| case File.extname(path).downcase | ||
| when '.app' | ||
| verify_app_bundle(path: path, expected_authority: params[:expected_authority], verify_notarization: params[:verify_notarization]) | ||
| when '.dmg' | ||
| verify_disk_image(path: path, expected_authority: params[:expected_authority], verify_notarization: params[:verify_notarization]) | ||
| else | ||
| UI.user_error!("Don't know how to verify #{path}. Supported artifacts are `.app` bundles and `.dmg` disk images") | ||
| end | ||
|
Comment on lines
+18
to
+25
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Should we also support I know you named that action I know that's not a super strong argument, but that would also avoid the strange case of if we don't rename that action but later need to support iOS/ Anyway, probably not that important, but I still figured I'd leave this as food for thoughts.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the one hand, Apple code signing is rather similar between platforms, so it makes sense to have a generic action that can handle any mix bag of Apple binaries. On the other, the presence of notarization in macOS represent a substantial difference. An API that allows to check code signing for an If we'll feel the need for an iOS counterpart, I think we could address the code duplication concern by extracting helper objects.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WFM 👍 |
||
| end | ||
|
|
||
| UI.success("Verified #{paths.length} artifact(s)") | ||
| end | ||
|
|
||
| def self.verify_app_bundle(path:, expected_authority:, verify_notarization:) | ||
| UI.user_error!("#{path} is not signed at all") unless signed?(path, deep: true) | ||
| verify_authority!(path: path, expected_authority: expected_authority) unless expected_authority.nil? | ||
|
|
||
| return unless verify_notarization | ||
|
|
||
| verify!("#{path} was rejected by Gatekeeper", 'spctl', '--assess', '--type', 'execute', '--verbose=2', path) | ||
| verify!("#{path} has no notarization ticket stapled to it", 'xcrun', 'stapler', 'validate', path) | ||
| end | ||
|
|
||
| # Unlike an app bundle, a disk image is often not signed at all — `electron-builder`, for one, | ||
| # only signs the app inside it. Gatekeeper then rejects the image itself with `no usable | ||
| # signature` even when it carries a notarization ticket, so the stapled ticket is the only | ||
| # check that means anything for an unsigned image. | ||
| # | ||
| def self.verify_disk_image(path:, expected_authority:, verify_notarization:) | ||
| # Said up front because `sh` logs a non-zero exit in red regardless of it being handled, | ||
| # which reads as a broken build in the CI log of an otherwise passing job. | ||
| UI.message("Checking whether #{path} is signed. Disk images usually aren't, so a `codesign` failure due to the image being unsigned is expected and tolerated (other signature failures will still fail).") | ||
|
|
||
| if signed?(path) | ||
| verify_authority!(path: path, expected_authority: expected_authority) unless expected_authority.nil? | ||
| verify!("#{path} was rejected by Gatekeeper", 'spctl', '--assess', '--type', 'open', '--context', 'context:primary-signature', '--verbose=2', path) if verify_notarization | ||
| else | ||
| UI.important("#{path} is not signed — skipping its signature checks. We expect that the app it contains is the one carrying the signature.") | ||
| end | ||
|
|
||
| verify!("#{path} has no notarization ticket stapled to it", 'xcrun', 'stapler', 'validate', path) if verify_notarization | ||
| end | ||
|
|
||
| # Distinguishes an artifact that carries no signature at all from one whose signature is | ||
| # broken: the former is expected for a disk image, the latter always a failure. | ||
| # | ||
| # @param deep [Boolean] Whether to also verify the nested code an app bundle embeds. | ||
| # | ||
| def self.signed?(path, deep: false) | ||
| command = ['codesign', '--verify'] | ||
| command << '--deep' if deep | ||
| command += ['--strict', '--verbose=2', path] | ||
|
|
||
| exitstatus, output = sh(*command) { |status, result, _| [status.exitstatus, result] } | ||
|
|
||
| return true if exitstatus.zero? | ||
| return false if output.include?('not signed at all') | ||
|
|
||
| UI.user_error!("The code signature of #{path} is not valid:\n#{output}") | ||
| end | ||
|
|
||
| def self.verify!(error_message, *command) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💄 Pure style nitpick: I think it's a combination of having those So instead I think one of those alternatives would read better: verify!('spctl', '--assess', '--type', 'execute', '--verbose=2', path, error_message: "#{path} was rejected by Gatekeeper")
verify!("#{path} was rejected by Gatekeeper", command: ['spctl', '--assess', '--type', 'execute', '--verbose=2', path])
verify!(
command: ['spctl', '--assess', '--type', 'execute', '--verbose=2', path],
error_message: "#{path} was rejected by Gatekeeper"
)wdyt?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. The error message up front followed by all the arguments felt iffy to me too... "verify" as a verb for what the method does is confusing, also. In the end, it's just a convenience wrapper for sh(*command, error_callback: ->(_) { UI.user_error!(error_message) })I think maybe a combination of your suggestion to explicitly pass Given this is an internal refactor and that I'm keep to ship the action to then use it officially in Automattic/wp-calypso#112767, I'm going to punt working on this in a follow up, though. Thanks!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Opus 4.8 input on the naming: Thread context read.
Option 3, drop generic helper entirely is attractive because a method named can be clearer than an array of command parameters. It doesn't solve the repetition of the |
||
| sh(*command, error_callback: ->(_) { UI.user_error!(error_message) }) | ||
| end | ||
|
|
||
| def self.verify_authority!(path:, expected_authority:) | ||
| details = sh('codesign', '--display', '--verbose=2', path) | ||
| return if details.include?("Authority=#{expected_authority}") | ||
|
|
||
| UI.user_error!("#{path} is not signed by '#{expected_authority}':\n#{details}") | ||
| end | ||
|
|
||
| ##################################################### | ||
| # @!group Documentation | ||
| ##################################################### | ||
|
|
||
| def self.description | ||
| 'Verify that macOS artifacts are properly code signed and notarized' | ||
| end | ||
|
|
||
| def self.details | ||
| <<~DETAILS | ||
| Verify that the given macOS artifacts are signed, and optionally notarized. | ||
|
|
||
| The checks that apply are picked from the artifact's extension: | ||
|
|
||
| - `.app` — the signature is valid and satisfies its designated requirement, Gatekeeper accepts | ||
| the bundle for execution, and a notarization ticket is stapled to it. | ||
| - `.dmg` — if the image is signed, the signature is valid (and can be checked against the expected authority) and Gatekeeper accepts opening it; when `verify_notarization` is true, a notarization ticket is stapled to the image. | ||
| DETAILS | ||
| end | ||
|
|
||
| def self.available_options | ||
| [ | ||
| FastlaneCore::ConfigItem.new( | ||
| key: :artifact_path, | ||
| description: 'The path, or list of paths, to the `.app` bundle(s) or `.dmg` disk image(s) to verify', | ||
| type: Array, | ||
| verify_block: proc do |value| | ||
| UI.user_error!('`artifact_path` must be a String or an Array of Strings') unless value.all?(String) | ||
| end | ||
| ), | ||
| FastlaneCore::ConfigItem.new( | ||
| key: :expected_authority, | ||
| description: 'The signing authority the artifact is expected to be signed by, e.g. `Developer ID Application: ACME, Inc. (ABCDE12345)`. ' \ | ||
| + 'When omitted, any valid signature is accepted', | ||
|
Comment on lines
+122
to
+123
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 It would be nice to provide some guidance in that |
||
| type: String, | ||
| optional: true, | ||
| default_value: nil | ||
| ), | ||
| FastlaneCore::ConfigItem.new( | ||
| key: :verify_notarization, | ||
| description: 'Whether to also assert that the artifact is accepted by Gatekeeper and has a notarization ticket stapled to it', | ||
| type: Boolean, | ||
| default_value: true | ||
| ), | ||
| ] | ||
| end | ||
|
|
||
| def self.authors | ||
| ['Automattic'] | ||
| end | ||
|
|
||
| def self.is_supported?(platform) | ||
| platform == :mac | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,239 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'spec_helper' | ||
|
|
||
| describe Fastlane::Actions::MacosVerifyCodeSigningAction do | ||
| let(:authority) { 'Developer ID Application: Automattic, Inc. (ABCDE12345)' } | ||
|
|
||
| # Runs the action against artifacts that exist on disk, so that the existence | ||
| # check doesn't get in the way of asserting on the commands the action runs. | ||
| # | ||
| def with_artifacts(*names) | ||
| in_tmp_dir do |tmp_dir| | ||
| paths = names.map do |name| | ||
| path = File.join(tmp_dir, name) | ||
| # A `.app` is a directory and a `.dmg` a file, but the action only calls | ||
| # `File.exist?`, so either satisfies it. | ||
| FileUtils.mkdir_p(path) | ||
| path | ||
| end | ||
| yield(paths) | ||
| end | ||
| end | ||
|
|
||
| def expect_codesign_verify_deep(path, exitstatus: 0, output: '') | ||
| expect_shell_command('codesign', '--verify', '--deep', '--strict', '--verbose=2', path, exitstatus: exitstatus, output: output) | ||
| end | ||
|
|
||
| def expect_codesign_verify(path, exitstatus: 0, output: '') | ||
| expect_shell_command('codesign', '--verify', '--strict', '--verbose=2', path, exitstatus: exitstatus, output: output) | ||
| end | ||
|
|
||
| def expect_codesign_display(path, authority:) | ||
| expect_shell_command('codesign', '--display', '--verbose=2', path, output: "Executable=#{path}\nAuthority=#{authority}\n") | ||
| end | ||
|
|
||
| def expect_gatekeeper_assess_execute(path, exitstatus: 0) | ||
| expect_shell_command('spctl', '--assess', '--type', 'execute', '--verbose=2', path, exitstatus: exitstatus) | ||
| end | ||
|
|
||
| def expect_gatekeeper_assess_open(path, exitstatus: 0) | ||
| expect_shell_command('spctl', '--assess', '--type', 'open', '--context', 'context:primary-signature', '--verbose=2', path, exitstatus: exitstatus) | ||
| end | ||
|
|
||
| def expect_stapler_validate(path, exitstatus: 0) | ||
| expect_shell_command('xcrun', 'stapler', 'validate', path, exitstatus: exitstatus) | ||
| end | ||
|
|
||
| before do | ||
| allow_fastlane_action_sh | ||
| end | ||
|
|
||
| describe 'app bundles' do | ||
| it 'verifies the signature, Gatekeeper acceptance and stapled ticket' do | ||
| with_artifacts('Test.app') do |(path)| | ||
| expect_codesign_verify_deep(path) | ||
| expect_gatekeeper_assess_execute(path) | ||
| expect_stapler_validate(path) | ||
|
|
||
| run_described_fastlane_action(artifact_path: path) | ||
| end | ||
| end | ||
|
|
||
| it 'skips the notarization checks when `verify_notarization` is `false`' do | ||
| with_artifacts('Test.app') do |(path)| | ||
| expect_codesign_verify_deep(path) | ||
| expect(Open3).not_to receive(:popen2e).with('spctl', any_args) | ||
| expect(Open3).not_to receive(:popen2e).with('xcrun', any_args) | ||
|
|
||
| run_described_fastlane_action(artifact_path: path, verify_notarization: false) | ||
| end | ||
| end | ||
|
|
||
| it 'passes when signed by the expected authority' do | ||
| with_artifacts('Test.app') do |(path)| | ||
| expect_codesign_verify_deep(path) | ||
| expect_codesign_display(path, authority: authority) | ||
| expect_gatekeeper_assess_execute(path) | ||
| expect_stapler_validate(path) | ||
|
|
||
| run_described_fastlane_action(artifact_path: path, expected_authority: authority) | ||
| end | ||
| end | ||
|
|
||
| it 'fails when signed by a different authority' do | ||
| with_artifacts('Test.app') do |(path)| | ||
| expect_codesign_verify_deep(path) | ||
| expect_codesign_display(path, authority: 'Apple Development: Someone Else (ZZZZZ99999)') | ||
|
|
||
| expect { run_described_fastlane_action(artifact_path: path, expected_authority: authority) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /is not signed by '#{Regexp.escape(authority)}'/) | ||
| end | ||
| end | ||
|
|
||
| it 'fails when the signature is not valid' do | ||
| with_artifacts('Test.app') do |(path)| | ||
| expect_codesign_verify_deep(path, exitstatus: 1, output: "#{path}: invalid signature (code or signature have been modified)\n") | ||
|
|
||
| expect { run_described_fastlane_action(artifact_path: path) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /The code signature of .*Test\.app is not valid/) | ||
| end | ||
| end | ||
|
|
||
| # Unlike a disk image, an app bundle without a signature is always a failure. | ||
| it 'fails when it is not signed at all' do | ||
| with_artifacts('Test.app') do |(path)| | ||
| expect_codesign_verify_deep(path, exitstatus: 1, output: "#{path}: code object is not signed at all\n") | ||
|
|
||
| expect { run_described_fastlane_action(artifact_path: path) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /Test\.app is not signed at all/) | ||
| end | ||
| end | ||
|
|
||
| it 'fails when Gatekeeper rejects it' do | ||
| with_artifacts('Test.app') do |(path)| | ||
| expect_codesign_verify_deep(path) | ||
| expect_gatekeeper_assess_execute(path, exitstatus: 3) | ||
|
|
||
| expect { run_described_fastlane_action(artifact_path: path) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /Test\.app was rejected by Gatekeeper/) | ||
| end | ||
| end | ||
|
|
||
| it 'fails when it has no notarization ticket stapled' do | ||
| with_artifacts('Test.app') do |(path)| | ||
| expect_codesign_verify_deep(path) | ||
| expect_gatekeeper_assess_execute(path) | ||
| expect_stapler_validate(path, exitstatus: 65) | ||
|
|
||
| expect { run_described_fastlane_action(artifact_path: path) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /Test\.app has no notarization ticket stapled to it/) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| describe 'disk images' do | ||
| # `electron-builder` signs the app inside the image but not the image itself, | ||
| # which is what our own `.dmg` artifacts look like. | ||
| it 'checks only the stapled ticket when the image is not signed' do | ||
| with_artifacts('Test.dmg') do |(path)| | ||
| expect_codesign_verify(path, exitstatus: 1, output: "#{path}: code object is not signed at all\n") | ||
| expect_stapler_validate(path) | ||
| expect(Open3).not_to receive(:popen2e).with('spctl', any_args) | ||
| expect(FastlaneCore::UI).to receive(:important).with(/is not signed — skipping its signature checks/) | ||
|
|
||
| run_described_fastlane_action(artifact_path: path) | ||
| end | ||
| end | ||
|
|
||
| it 'does not assert the authority of an unsigned image' do | ||
| with_artifacts('Test.dmg') do |(path)| | ||
| expect_codesign_verify(path, exitstatus: 1, output: "#{path}: code object is not signed at all\n") | ||
| expect_stapler_validate(path) | ||
| expect(Open3).not_to receive(:popen2e).with('codesign', '--display', any_args) | ||
|
|
||
| run_described_fastlane_action(artifact_path: path, expected_authority: authority) | ||
| end | ||
| end | ||
|
|
||
| it 'checks Gatekeeper and the authority when the image is signed' do | ||
| with_artifacts('Test.dmg') do |(path)| | ||
| expect_codesign_verify(path) | ||
| expect_codesign_display(path, authority: authority) | ||
| expect_gatekeeper_assess_open(path) | ||
| expect_stapler_validate(path) | ||
|
|
||
| run_described_fastlane_action(artifact_path: path, expected_authority: authority) | ||
| end | ||
| end | ||
|
|
||
| it 'fails when the image carries a broken signature' do | ||
| with_artifacts('Test.dmg') do |(path)| | ||
| expect_codesign_verify(path, exitstatus: 1, output: "#{path}: invalid signature (code or signature have been modified)\n") | ||
|
|
||
| expect { run_described_fastlane_action(artifact_path: path) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /The code signature of .*Test\.dmg is not valid/) | ||
| end | ||
| end | ||
|
|
||
| it 'fails when it has no notarization ticket stapled' do | ||
| with_artifacts('Test.dmg') do |(path)| | ||
| expect_codesign_verify(path, exitstatus: 1, output: "#{path}: code object is not signed at all\n") | ||
| expect_stapler_validate(path, exitstatus: 65) | ||
|
|
||
| expect { run_described_fastlane_action(artifact_path: path) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /Test\.dmg has no notarization ticket stapled to it/) | ||
| end | ||
| end | ||
|
|
||
| it 'skips every check but the signature when `verify_notarization` is `false`' do | ||
| with_artifacts('Test.dmg') do |(path)| | ||
| expect_codesign_verify(path) | ||
| expect(Open3).not_to receive(:popen2e).with('spctl', any_args) | ||
| expect(Open3).not_to receive(:popen2e).with('xcrun', any_args) | ||
|
|
||
| run_described_fastlane_action(artifact_path: path, verify_notarization: false) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| it 'verifies every artifact when given a list of paths' do | ||
| with_artifacts('One.app', 'Two.dmg') do |(app_path, dmg_path)| | ||
| expect_codesign_verify_deep(app_path) | ||
| expect_gatekeeper_assess_execute(app_path) | ||
| expect_stapler_validate(app_path) | ||
|
|
||
| expect_codesign_verify(dmg_path, exitstatus: 1, output: "#{dmg_path}: code object is not signed at all\n") | ||
| expect_stapler_validate(dmg_path) | ||
|
|
||
| run_described_fastlane_action(artifact_path: [app_path, dmg_path]) | ||
| end | ||
| end | ||
|
|
||
| it 'fails on an artifact it does not know how to verify' do | ||
| with_artifacts('Test.zip') do |(path)| | ||
| expect { run_described_fastlane_action(artifact_path: path) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /Don't know how to verify .*Test\.zip/) | ||
| end | ||
| end | ||
|
|
||
| it 'fails when there is no artifact at the given path' do | ||
| expect { run_described_fastlane_action(artifact_path: '/path/to/Missing.app') } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, %r{There is no artifact at /path/to/Missing\.app}) | ||
| end | ||
|
|
||
| it 'fails when given an empty list of paths' do | ||
| expect { run_described_fastlane_action(artifact_path: []) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /`artifact_path` is empty/) | ||
| end | ||
|
|
||
| it 'fails when `artifact_path` is neither a String nor an Array' do | ||
| expect { run_described_fastlane_action(artifact_path: 42) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /'artifact_path' value must be either `Array` or `comma-separated String`!/) | ||
| end | ||
|
|
||
| it 'fails when `artifact_path` is an Array of something other than Strings' do | ||
| expect { run_described_fastlane_action(artifact_path: [42]) } | ||
| .to raise_error(FastlaneCore::Interface::FastlaneError, /`artifact_path` must be a String or an Array of Strings/) | ||
| end | ||
| end |
Uh oh!
There was an error while loading. Please reload this page.