Make Homebrew check for App Management permissions before updating apps.

This commit is contained in:
JBYoshi 2023-05-24 21:53:00 -05:00
parent 129f434894
commit 40352cb2da
No known key found for this signature in database
GPG Key ID: AE4430116622D05D
3 changed files with 118 additions and 19 deletions

View File

@ -85,6 +85,7 @@ module Cask
Utils.gain_permissions_mkpath(target.dirname, command: command) unless target.dirname.exist?
if target.directory?
Quarantine.ensure_app_management_permissions_granted(app: target, command: command)
if target.writable?
source.children.each { |child| FileUtils.move(child, target/child.basename) }
else
@ -156,6 +157,7 @@ module Cask
if target.directory? && matching_artifact?(successor)
# If an app folder is deleted, macOS considers the app uninstalled and removes some data.
# Remove only the contents to handle this case.
Quarantine.ensure_app_management_permissions_granted(app: target, command: command)
target.children.each do |child|
if target.writable? && !force
child.rmtree

View File

@ -188,5 +188,66 @@ module Cask
],
)
end
# Ensures that Homebrew has permission to update apps on macOS Ventura.
# This may be granted either through the App Management toggle or the Full Disk Access toggle.
# The system will only show a prompt for App Management, so we ask the user to grant that.
sig { params(app: Pathname, command: T.class_of(SystemCommand)).void }
def self.ensure_app_management_permissions_granted(app:, command:)
return unless app.directory?
# To get macOS to prompt the user for permissions, we need to actually attempt to
# modify a file in the app.
test_file = app / ".homebrew-write-test"
# We can't use app.writable? here because that conflates several access checks,
# including both file ownership and whether system permissions are granted.
# Here we just want to check whether sudo would be needed.
looks_writable_without_sudo = if app.owned?
(app.lstat.mode & 0200) != 0
elsif app.grpowned?
(app.lstat.mode & 0020) != 0
else
(app.lstat.mode & 0002) != 0
end
if looks_writable_without_sudo
begin
File.write(test_file, "")
test_file.delete
return
rescue Errno::EACCES
# Using error handler below
end
else
begin
command.run!(
"touch",
args: [
test_file,
],
print_stderr: false,
sudo: true,
)
command.run!(
"rm",
args: [
test_file,
],
print_stderr: false,
sudo: true,
)
return
rescue ErrorDuringExecution => e
# We only want to handle "touch" errors here; propagate "sudo" errors up
raise e unless e.stderr.include?("touch: #{test_file}: Operation not permitted")
end
end
odie <<~EOF
Your terminal needs permission to update apps.
Go to Settings > Security and Privacy > App Management, or look for a notification saying your terminal was prevented from modifying apps.
EOF
end
end
end

View File

@ -303,10 +303,12 @@ describe Cask::Artifact::App, :cask do
end
describe "upgrade" do
before do
install_phase
end
# Fix for https://github.com/Homebrew/homebrew-cask/issues/102721
it "reuses the same directory" do
install_phase
contents_path = target_path.join("Contents/Info.plist")
expect(target_path).to exist
@ -326,28 +328,62 @@ describe Cask::Artifact::App, :cask do
expect(contents_path).to exist
end
it "properly handles non-writable directories" do
install_phase
describe "when the system blocks modifying apps" do
it "does not delete files" do
target_contents_path = target_path.join("Contents")
source_contents_path = source_path.join("Contents")
target_contents_path = target_path.join("Contents")
expect(File).to receive(:write).with(target_path / ".homebrew-write-test",
instance_of(String)).and_raise(Errno::EACCES)
allow(app.target).to receive(:writable?).and_return false
allow(command).to receive(:run!).with(any_args).and_call_original
expect { app.uninstall_phase(command: command, force: force, successor: cask) }.to raise_error(SystemExit)
expect(target_contents_path).to exist
end
end
expect(command).to receive(:run!)
.with("/bin/cp", args: ["-pR", source_contents_path, target_path],
sudo: true)
.and_call_original
expect(FileUtils).not_to receive(:move).with(source_contents_path, an_instance_of(Pathname))
describe "when the directory is owned by root" do
before do
allow(app.target).to receive(:writable?).and_return false
allow(app.target).to receive(:owned?).and_return false
end
app.uninstall_phase(command: command, force: force, successor: cask)
expect(target_contents_path).not_to exist
expect(target_path).to exist
expect(source_contents_path).to exist
it "reuses the same directory" do
source_contents_path = source_path.join("Contents")
target_contents_path = target_path.join("Contents")
app.install_phase(command: command, adopt: adopt, force: force, predecessor: cask)
expect(target_contents_path).to exist
allow(command).to receive(:run!).with(any_args).and_call_original
expect(command).to receive(:run!)
.with("/bin/cp", args: ["-pR", source_contents_path, target_path],
sudo: true)
.and_call_original
expect(FileUtils).not_to receive(:move).with(source_contents_path, an_instance_of(Pathname))
app.uninstall_phase(command: command, force: force, successor: cask)
expect(target_contents_path).not_to exist
expect(target_path).to exist
expect(source_contents_path).to exist
app.install_phase(command: command, adopt: adopt, force: force, predecessor: cask)
expect(target_contents_path).to exist
end
describe "when the system blocks modifying apps" do
it "does not delete files" do
target_contents_path = target_path.join("Contents")
allow(command).to receive(:run!).with(any_args).and_call_original
expect(command).to receive(:run!)
.with("touch", args: [target_path / ".homebrew-write-test"],
print_stderr: false,
sudo: true)
.and_raise(ErrorDuringExecution.new([], status: 1,
output: [[:stderr, "touch: #{target_path}/.homebrew-write-test: Operation not permitted\n"]], secrets: []))
expect { app.uninstall_phase(command: command, force: force, successor: cask) }.to raise_error(SystemExit)
expect(target_contents_path).to exist
end
end
end
end
end