Skip to content

Commit 5542165

Browse files
MikeMcQuaidMikeMcQuaid
authored
Merge pull request #23032 from Homebrew/install-step-cert-trust
Add install step keychain cleanup
2 parents 2554430 + 29861c0 commit 5542165

7 files changed

Lines changed: 159 additions & 18 deletions

File tree

‎Library/Homebrew/install_steps.rb‎

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ def self.normalise_step_value(key, obj)
102102
when Hash
103103
::T.cast(obj.to_h { |key, value| [key.to_s, value&.to_s] }.compact_blank, PathSpec)
104104
when String, Pathname
105-
%w[path source target].include?(key) ? { "path" => obj.to_s } : obj.to_s
105+
%w[path source target matching_certificate].include?(key) ? { "path" => obj.to_s } : obj.to_s
106106
else
107107
obj
108108
end
@@ -305,6 +305,20 @@ def update_desktop_database
305305
add_rebuild_action("update_desktop_database", "share/applications")
306306
end
307307

308+
sig {
309+
params(
310+
name: ::String,
311+
matching_certificate: ::T.nilable(::T.any(::String, ::Pathname)),
312+
base: ::T.nilable(::T.any(::String, ::Symbol)),
313+
).void
314+
}
315+
def delete_keychain_certificate(name, matching_certificate: nil, base: nil)
316+
add_step("delete_keychain_certificate",
317+
"name" => name,
318+
"matching_certificate" => (path_spec(matching_certificate, base:, default_base: nil) if
319+
matching_certificate))
320+
end
321+
308322
private
309323

310324
sig { params(type: ::String, fields: ::T.nilable(StepValue)).void }
@@ -457,6 +471,38 @@ def run_install_step(step)
457471
run_formula_tool("shared-mime-info", "update-mime-database", resolve_path(step_path(step, "path")))
458472
when "update_desktop_database"
459473
run_formula_tool("desktop-file-utils", "update-desktop-database", resolve_path(step_path(step, "path")))
474+
when "delete_keychain_certificate"
475+
certificate_hash = nil
476+
if step.key?("matching_certificate")
477+
certificate = resolve_path(step_path(step, "matching_certificate"))
478+
return unless certificate.exist?
479+
480+
certificate_hash = run_command_output("/usr/bin/openssl", "x509", "-fingerprint", "-sha256", "-noout",
481+
"-in", certificate)
482+
.lines
483+
.first
484+
.to_s
485+
.split("=", 2)[1]
486+
.to_s
487+
.delete(":")
488+
.strip
489+
.upcase
490+
return if certificate_hash.blank?
491+
end
492+
493+
certificate_hashes = run_command_output(
494+
"/usr/bin/security", "find-certificate", "-a", "-c", step_string(step, "name"), "-Z",
495+
sudo: true
496+
).lines.filter_map { |line| line[/\ASHA-256 hash:\s*(\S+)/, 1]&.upcase }
497+
498+
if certificate_hash
499+
run_command "/usr/bin/security", "delete-certificate", "-Z", certificate_hash, sudo: true if
500+
certificate_hashes.include?(certificate_hash)
501+
else
502+
certificate_hashes.each do |matching_certificate_hash|
503+
run_command "/usr/bin/security", "delete-certificate", "-Z", matching_certificate_hash, sudo: true
504+
end
505+
end
460506
else
461507
raise ArgumentError, "unknown install step: #{step.fetch("type")}"
462508
end
@@ -647,9 +693,14 @@ def context_config_value(method)
647693
config.public_send(method) if config.respond_to?(method)
648694
end
649695

650-
sig { params(command: SystemCommandArg, args: SystemCommandArg).void }
651-
def run_command(command, *args)
652-
system_command!(command, args: args, print_stdout: true, print_stderr: true, reset_uid: true)
696+
sig { params(command: SystemCommandArg, args: SystemCommandArg, sudo: T::Boolean).void }
697+
def run_command(command, *args, sudo: false)
698+
system_command!(command, args: args, sudo:, print_stdout: true, print_stderr: true, reset_uid: true)
699+
end
700+
701+
sig { params(command: SystemCommandArg, args: SystemCommandArg, sudo: T::Boolean).returns(String) }
702+
def run_command_output(command, *args, sudo: false)
703+
system_command!(command, args: args, sudo:, print_stderr: true, reset_uid: true).stdout
653704
end
654705
end
655706
end

‎Library/Homebrew/json_api_postinstall_preflight_postflight_plan.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -408,13 +408,19 @@ is stripped during metadata serialisation.
408408
`./bin/brew style homebrew/core`, targeted `./bin/brew audit --strict
409409
--online --formula ...` for the changed formulae and `./bin/brew readall
410410
homebrew/core`.
411-
- [ ] PR 7, certificate and trust store actions.
411+
- [x] PR 7, certificate and trust store actions.
412+
Commit: `Add install step keychain cleanup`.
412413
Estimated existing formulae/casks affected: about `17` formulae update
413414
certificate/trust state and `8` cask flight blocks invoke
414415
`/usr/bin/security` for keychain certificate cleanup.
415-
Notes for implementation: separate formula-owned symlinked certificate
416-
actions from keychain mutations; keychain work likely counts as non-Homebrew
417-
code and should be prepared for sandbox policy decisions.
416+
Scope: cask `delete_keychain_certificate` step, runner execution through
417+
fixed `/usr/bin/security find-certificate` and `delete-certificate` calls,
418+
optional local certificate fingerprint matching for selective deletion,
419+
cask step block allow-list entries and docs. Formula-owned `cert.pem`
420+
symlinks use the existing `ln_sf` step with `source_formula` and
421+
`source_base: :formula_pkgetc`; specialised trust store generation such as
422+
`ca-certificates` bundle regeneration and Mono `cert-sync` stays legacy Ruby
423+
because current repeated usage is below the named-variant threshold.
418424
- [ ] PR 8, cask permission and ownership actions.
419425
Estimated existing casks affected: about `21` casks change permissions and
420426
`36` change ownership.

‎Library/Homebrew/rubocops/shared/install_steps_helper.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,14 @@ module InstallStepsHelper
1212
REBUILD_ACTION_STEP_METHODS =
1313
[:compile_gsettings_schemas, :gio_querymodules, :gdk_pixbuf_query_loaders, :gtk_update_icon_cache,
1414
:update_mime_database, :update_desktop_database].freeze
15+
KEYCHAIN_STEP_METHODS = [:delete_keychain_certificate].freeze
1516
ALLOWED_STEP_METHODS = T.let(
1617
[*FILE_PREPARATION_STEP_METHODS, *LINK_STEP_METHODS, *CONFIG_WRITE_STEP_METHODS, *SERVICE_DATA_STEP_METHODS,
1718
*REBUILD_ACTION_STEP_METHODS].freeze,
1819
T::Array[Symbol],
1920
)
2021
CASK_ALLOWED_STEP_METHODS = T.let(
21-
[*FILE_PREPARATION_STEP_METHODS, *CONFIG_WRITE_STEP_METHODS].freeze,
22+
[*FILE_PREPARATION_STEP_METHODS, *CONFIG_WRITE_STEP_METHODS, *KEYCHAIN_STEP_METHODS].freeze,
2223
T::Array[Symbol],
2324
)
2425

‎Library/Homebrew/test/install_steps_spec.rb‎

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,27 @@
8686
path: "nested/example",
8787
},
8888
},
89+
{
90+
type: :delete_keychain_certificate,
91+
name: "NodeMITMProxyCA",
92+
matching_certificate: "~/Library/Application Support/betwixt/ssl/certs/ca.pem",
93+
},
8994
]
9095

9196
expect(Homebrew::InstallSteps::DSL.normalise_steps(steps)).to contain_exactly(
92-
"type" => "mkdir_p",
93-
"path" => {
94-
"base" => "var",
95-
"path" => "nested/example",
97+
{
98+
"type" => "mkdir_p",
99+
"path" => {
100+
"base" => "var",
101+
"path" => "nested/example",
102+
},
103+
},
104+
{
105+
"type" => "delete_keychain_certificate",
106+
"name" => "NodeMITMProxyCA",
107+
"matching_certificate" => {
108+
"path" => "~/Library/Application Support/betwixt/ssl/certs/ca.pem",
109+
},
96110
},
97111
)
98112
end
@@ -344,6 +358,63 @@
344358
end
345359
end
346360

361+
specify "deletes matching keychain certificates by SHA-256 hash" do
362+
steps = Homebrew::InstallSteps::DSL.build do
363+
delete_keychain_certificate "Charles"
364+
end
365+
366+
runner = Homebrew::InstallSteps::Runner.new(context:)
367+
expect(runner).to receive(:run_command_output)
368+
.with("/usr/bin/security", "find-certificate", "-a", "-c", "Charles", "-Z", sudo: true)
369+
.and_return(<<~EOS)
370+
SHA-256 hash: ABC123
371+
SHA-256 hash: DEF456
372+
EOS
373+
expect(runner).to receive(:run_command)
374+
.with("/usr/bin/security", "delete-certificate", "-Z", "ABC123", sudo: true).ordered
375+
expect(runner).to receive(:run_command)
376+
.with("/usr/bin/security", "delete-certificate", "-Z", "DEF456", sudo: true).ordered
377+
378+
runner.run(steps)
379+
end
380+
381+
specify "only deletes the keychain certificate matching a local certificate" do
382+
certificate = root/"home/Library/Application Support/betwixt/ssl/certs/ca.pem"
383+
certificate.dirname.mkpath
384+
certificate.write "certificate"
385+
steps = Homebrew::InstallSteps::DSL.build do
386+
delete_keychain_certificate "NodeMITMProxyCA", matching_certificate: certificate
387+
end
388+
389+
runner = Homebrew::InstallSteps::Runner.new(context:)
390+
expect(runner).to receive(:run_command_output)
391+
.with("/usr/bin/openssl", "x509", "-fingerprint", "-sha256", "-noout", "-in", certificate)
392+
.and_return("sha256 Fingerprint=AB:CD:EF\n")
393+
expect(runner).to receive(:run_command_output)
394+
.with("/usr/bin/security", "find-certificate", "-a", "-c", "NodeMITMProxyCA", "-Z", sudo: true)
395+
.and_return(<<~EOS)
396+
SHA-256 hash: ABCDEF
397+
SHA-256 hash: FEDCBA
398+
EOS
399+
expect(runner).to receive(:run_command)
400+
.with("/usr/bin/security", "delete-certificate", "-Z", "ABCDEF", sudo: true)
401+
402+
runner.run(steps)
403+
end
404+
405+
specify "skips keychain certificate deletion when a local certificate is missing" do
406+
certificate = root/"missing.pem"
407+
steps = Homebrew::InstallSteps::DSL.build do
408+
delete_keychain_certificate "NodeMITMProxyCA", matching_certificate: certificate
409+
end
410+
411+
runner = Homebrew::InstallSteps::Runner.new(context:)
412+
expect(runner).not_to receive(:run_command_output)
413+
expect(runner).not_to receive(:run_command)
414+
415+
runner.run(steps)
416+
end
417+
347418
specify "does not add the default base to home paths" do
348419
steps = Homebrew::InstallSteps::DSL.build(default_base: :var) do
349420
mkdir_p "~/example"

‎Library/Homebrew/test/rubocops/cask/install_steps_spec.rb‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
3131
preflight_steps do
3232
system "true"
33-
^^^^^^^^^^^^^ Steps blocks may only contain install step DSL calls: `mkdir`, `mkdir_p`, `touch`, `move`, `mv`, `move_children`, `symlink`, `ln_s`, `ln_sf`, `write`.
33+
^^^^^^^^^^^^^ Steps blocks may only contain install step DSL calls: `mkdir`, `mkdir_p`, `touch`, `move`, `mv`, `move_children`, `symlink`, `ln_s`, `ln_sf`, `write`, `delete_keychain_certificate`.
3434
end
3535
end
3636
CASK
@@ -44,7 +44,7 @@
4444
4545
preflight_steps do
4646
update_desktop_database
47-
^^^^^^^^^^^^^^^^^^^^^^^ Steps blocks may only contain install step DSL calls: `mkdir`, `mkdir_p`, `touch`, `move`, `mv`, `move_children`, `symlink`, `ln_s`, `ln_sf`, `write`.
47+
^^^^^^^^^^^^^^^^^^^^^^^ Steps blocks may only contain install step DSL calls: `mkdir`, `mkdir_p`, `touch`, `move`, `mv`, `move_children`, `symlink`, `ln_s`, `ln_sf`, `write`, `delete_keychain_certificate`.
4848
end
4949
end
5050
CASK
@@ -63,6 +63,8 @@
6363
move_children "source", "target"
6464
ln_sf "source", "target", source_base: :relative, uninstall: true
6565
write "foo.conf", "key = value\n"
66+
delete_keychain_certificate "Charles"
67+
delete_keychain_certificate "NodeMITMProxyCA", matching_certificate: "~/Library/Application Support/betwixt/ssl/certs/ca.pem"
6668
end
6769
end
6870
CASK

‎docs/Cask-Cookbook.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,12 @@ postflight_steps do
575575
mv "payload", "Shared/payload"
576576
ln_s "Shared/payload", "Payload", source_base: :relative
577577
end
578+
579+
uninstall_postflight_steps do
580+
delete_keychain_certificate "Charles"
581+
delete_keychain_certificate "NodeMITMProxyCA",
582+
matching_certificate: "~/Library/Application Support/betwixt/ssl/certs/ca.pem"
583+
end
578584
```
579585

580586
A steps block may only contain supported step calls with literal arguments; it cannot call the wider cask DSL or arbitrary Ruby code. Each phase may define either its Ruby flight block or its matching steps block, not both.
@@ -595,6 +601,8 @@ Relative paths default to `staged_path` for `base:`, `source_base:` and `target_
595601
* `ln_s`: alias for `symlink`; example: `ln_s "Shared/payload", "Payload", source_base: :relative`.
596602
* `ln_sf`: create or replace a symlink; example: `ln_sf "Shared/payload", "Payload", source_base: :relative, uninstall: true`.
597603
* `write`: write literal content to a file unless it already exists; example: `write "Shared/foo.conf", "key = value"`. Pass `overwrite: true` to always replace the file. A trailing newline is appended unless the content already ends with one. Content may use the `{{staged_path}}`, `{{appdir}}` and `{{version}}` tokens, which are expanded at install time; any other `{{...}}` is left verbatim.
604+
* `delete_keychain_certificate`: delete macOS keychain certificates whose common name matches the argument; example: `delete_keychain_certificate "Charles"`. Pass `matching_certificate:` with a local certificate path to delete only the matching SHA-256 fingerprint; example:
605+
`delete_keychain_certificate "NodeMITMProxyCA", matching_certificate: "~/Library/Application Support/betwixt/ssl/certs/ca.pem"`.
598606

599607
{% endraw %}
600608

‎docs/Formula-Cookbook.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,9 +1259,11 @@ class Foo < Formula
12591259
# ...
12601260
url "https://example.com/foo-1.0.tar.gz"
12611261

1262-
def post_install
1263-
rm pkgetc/"cert.pem" if File.exist?(pkgetc/"cert.pem")
1264-
pkgetc.install_symlink Formula["ca-certificates"].pkgetc/"cert.pem"
1262+
post_install_steps do
1263+
ln_sf "cert.pem", "cert.pem",
1264+
source_formula: "ca-certificates",
1265+
source_base: :formula_pkgetc,
1266+
target_base: :pkgetc
12651267
end
12661268
# ...
12671269
end

0 commit comments

Comments
 (0)