admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

Merge pull request #15483 from JBYoshi/cask-prompt-permissions

Browse Source 
Check for App Management permissions before updating apps.
This commit is contained in:
Mike McQuaid 2023-05-29 09:15:22 +01:00 committed by GitHub
commit 89dfe4fc5b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 135 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
Library/Homebrew/cask/artifact/moved.rb
View File

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

64
Library/Homebrew/cask/quarantine.rb
View File

@ -189,5 +189,69 @@ module Cask
sudo: !to.writable?,
)
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)).returns(T::Boolean) }
def self.app_management_permissions_granted?(app:, command:)
return true 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 true
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 true
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
opoo <<~EOF
Your terminal does not have App Management permissions, so Homebrew will delete and reinstall the app.
This may result in some configurations (like notification settings or location in the Dock/Launchpad) being lost.
To fix this, go to Settings > Security and Privacy > App Management and turn on the switch for your terminal.
EOF
false
end
end
end

80
Library/Homebrew/test/cask/artifact/app_spec.rb
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,68 @@ 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 "uninstalls and reinstalls the app" 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
app.uninstall_phase(command: command, force: force, successor: cask)
expect(target_path).not_to exist
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.install_phase(command: command, adopt: adopt, force: force, predecessor: cask)
expect(target_contents_path).to exist
end
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
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.install_phase(command: command, adopt: adopt, force: force, predecessor: cask)
expect(target_contents_path).to exist
it "reuses the same directory" do
source_contents_path = source_path.join("Contents")
target_contents_path = target_path.join("Contents")
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 "uninstalls and reinstalls the app" 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: []))
app.uninstall_phase(command: command, force: force, successor: cask)
expect(target_path).not_to exist
app.install_phase(command: command, adopt: adopt, force: force, predecessor: cask)
expect(target_contents_path).to exist
end
end
end
end
end

6
docs/FAQ.md
View File

@ -220,3 +220,9 @@ Or use the `--greedy` flag:
brew upgrade --greedy
Refer to the `upgrade` section of the [`brew` manual page](Manpage.md) for more details.
## Why do my cask apps lose their Dock position / Launchpad position / permission settings when I run `brew upgrade`?
Homebrew has two possible strategies to update cask apps: uninstalling the old version and reinstalling the new one, or replacing the contents of the app with the new contents. With the uninstall/reinstall strategy, [macOS thinks the app is being deleted without any intent to reinstall it](https://github.com/Homebrew/brew/pull/15138), and so it removes some internal metadata for the old app, including where it appears in the Dock and Launchpad and which permissions it's been granted. The content replacement strategy works around this by treating the upgrade as an in-place upgrade. However, starting in macOS Ventura, these in-place upgrades are only allowed when the updater application (in this case, the terminal running Homebrew) has [certain permissions granted](https://github.com/Homebrew/brew/pull/15483). Either the "App Management" or "Full Disk Access" permission will suffice.
Homebrew defaults to in-place upgrades when it has the necessary permissions. Otherwise, it will use the uninstall/reinstall strategy.