brew cask zap prompt when Full Disk Access is needed

[Rylan12]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

Fixes Homebrew/homebrew-cask#80835

This change adds a better error message for when brew cask zap fails due to not having Full Disk Access.

The error text can be easily changed if someone has better wording or a different message (I don't know if there are any guidelines for error messages)

Another possibility is to replace the odie error message with an opoo warning message. This will give a warning that the file wasn't able to be removed but will continue execution and remove the rest of the files that it is able to remove. A downside of this is that there would be an error message for each file that wasn't able to be removed. If this method is preferable, it wouldn't be too hard to have one error message at the end which lists all the files that weren't able to be deleted.

---

I wasn't able to find a way to detect whether Full Disk Access is enabled without trying to do something that requires it and seeing if it fails, so adding a rescue block seemed like the right option.

Currently, though, this is a catch-all and it is probably better if it only catches the error caused by not having access. I wasn't able to figure out which error to rescue so I left it as a catch-all. if anyone can figure out or guide me to figuring out which specific error should be caught I would appreciate it.

---

Lastly, tests:

I don't have any tests written for this (per the checklist). I'm not sure exactly what would be tested so I won't write any for now.

Given some guidance on what should be tested, I'd be happy to have a go at writing them.

[reitermarkus]

trying to do something that requires it and seeing if it fails

Do that in the rescue block.

[Rylan12]

@reitermarkus do you mean do something other than try to delete the files the zap is supposed to? One way that I found being used elsewhere (not in homebrew, that is) was to look at the Safari bookmarks file (I'll try to find the source for that) which would not be accessible without full disk access. That doesn't feel like the right thing to do here. Maybe it's ok, but it seems weird to have homebrew looking at safari (or other application) files. Maybe there's a better way to test for access.

Do that in the rescue block.

Do what exactly? I see the begin rescue block to be similar to python's try except, so I would try to manipulate the file in the begin and deal with not having access in the rescue. Maybe I don't understand what your point is.

Pathname.glob(resolved_path) is what fails when full disk access is not enabled, so that line is doing the "test" of whether full disk access is enabled.

[reitermarkus]

the Safari bookmarks file

In my dotfiles I am checking if ~/Library/Application Support/com.apple.TCC/TCC.db is readable.

Do what exactly?

In the rescue block, test using that file if access is enabled. If it is enable, re-raise the error, otherwise display the message.

[Rylan12]

In the rescue block, test using that file if access is enabled. If it is enable, re-raise the error, otherwise display the message.

Ahh, I see what you mean. Is that preferable to using rescue Errno::EPERM?

[reitermarkus]

Is that preferable to using rescue Errno::EPERM?

We should do this in addition to rescuing Errno::EPERM. Not every Errno::EPERM error is caused by missing Full Disk Access.

[Rylan12]

So right now I'm looking at something like:

begin
  yield path, Pathname.glob(resolved_path)
rescue Errno::EPERM
  unless File.readable?('~/Library/Application Support/com.apple.TCC/TCC.db')
    odie "Unable to remove some files. Please enable Full Disk Access for your teminal under " \
         "System Preferences → Security & Privacy → Privacy → Full Disk Access."
  else
    raise
  end
end

Is that what you're thinking?

[reitermarkus]

Is that what you're thinking?

Yep, something like that.

[reitermarkus]

I just tested locally and it seems for some reason ~/Library/Application Support/com.apple.TCC/TCC.db is now readable even without Full Disk Access but ~/Library/Application Support/com.apple.TCC still isn't, so it seems like the directory is a better option.

[Rylan12]

It seems to be working with the file for me. I can try to switch to the directory instead though

[Rylan12]

@reitermarkus Can you take another look at this?

[reitermarkus]

Thanks, @Rylan12!

[CamelKing]

Homebrew does not allow us to run it under sudo mode for security reason, and at the same time, brew cask zap requires full disk access to be opened up to the Terminal app in Mac, which practically is giving almost sudo access to all programs, not just Homebrew.

May be the message can be more specific about which directory the zap need access to? May be we can just grant access on those few directories?

[vitorgalvao]

brew cask zap requires full disk access to be opened up to the Terminal app in Mac, which practically is giving almost sudo access to all programs, not just Homebrew.

No, it is not. Disk access is not even close to sudo permissions. That restriction only appeared with Catalina; you’re claiming that before then all programs had “almost sudo access”, which is incorrect.

May be the message can be more specific about which directory the zap need access to?

It depends entirely on each app. You’re free to brew cask _stanza zap {{cask_name}} and remove the files and directories yourself.

[CamelKing]

brew cask zap requires full disk access to be opened up to the Terminal app in Mac, which practically is giving almost sudo access to all programs, not just Homebrew.

No, it is not. Disk access is not even close to sudo permissions. That restriction only appeared with Catalina; you’re claiming that before then all programs had “almost sudo access”, which is incorrect.

May be the message can be more specific about which directory the zap need access to?

It depends entirely on each app. You’re free to brew cask _stanza zap {{cask_name}} and remove the files and directories yourself.

Well, may be not super user access to the system, but giving "Full Disk Access" to "Terminal", which means I am granting the same permission to other apps (not Homebrew"). To me, that is worse than granting a temp sudo access to Homebrew. Kind of contradictory. Just my two cents.

Thanks for the suggestion on using stanza, that is great. Cheers.