diff --git a/Library/Homebrew/cask/artifact/moved.rb b/Library/Homebrew/cask/artifact/moved.rb index 392c6673ba..3b19f5849d 100644 --- a/Library/Homebrew/cask/artifact/moved.rb +++ b/Library/Homebrew/cask/artifact/moved.rb @@ -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 diff --git a/Library/Homebrew/cask/quarantine.rb b/Library/Homebrew/cask/quarantine.rb index 8c5436c65c..cb402e6955 100644 --- a/Library/Homebrew/cask/quarantine.rb +++ b/Library/Homebrew/cask/quarantine.rb @@ -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 diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index 978045a720..3dc19a2cc4 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,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