diff --git a/Library/Homebrew/cask/artifact/moved.rb b/Library/Homebrew/cask/artifact/moved.rb index 392c6673ba..d41e62cec0 100644 --- a/Library/Homebrew/cask/artifact/moved.rb +++ b/Library/Homebrew/cask/artifact/moved.rb @@ -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| diff --git a/Library/Homebrew/cask/quarantine.rb b/Library/Homebrew/cask/quarantine.rb index 752c66a3fd..dcad88d7c9 100644 --- a/Library/Homebrew/cask/quarantine.rb +++ b/Library/Homebrew/cask/quarantine.rb @@ -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 diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index 978045a720..ed94017311 100644 --- a/Library/Homebrew/test/cask/artifact/app_spec.rb +++ b/Library/Homebrew/test/cask/artifact/app_spec.rb @@ -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 diff --git a/docs/FAQ.md b/docs/FAQ.md index bc9ec012f7..143b4fecb7 100644 --- a/docs/FAQ.md +++ b/docs/FAQ.md @@ -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.