sparkle-project / Sparkle

A software update framework for macOS
https://sparkle-project.org
Other
7.28k stars 1.04k forks source link

App modification prompt when new downloaded update overrides NSUpdateSecurityPolicy #2591

Closed danielpunkass closed 2 weeks ago

danielpunkass commented 3 weeks ago

Summary

By invoking the renamex_np function to perform an atomic swap of the existing and updated versions of an app, Sparkle causes a user-facing notification asserting that the host app has been prevented from "modifying apps on your Mac."

Screenshot 2024-06-25 at 1 39 25 PM

This is an alarming message to have conveyed to users, so I think that Sparkle should avoid any update mechanism that will cause it to appear.

To reproduce, first reset the system TCC permissions for the pertinent app's "SystemPolicyAppBundles" permission. Then attempt to conduct an update with the app from an older version to a new one. In the absence of another test app, you could use my app MarsEdit to reproduce the issue:

  1. Download and install MarsEdit 5.2: http://redsweater.com/marsedit/MarsEdit5.2.zip
  2. Reset MarsEdit's pertinent TCC permissions: tccutil reset SystemPolicyAppBundles com.red-sweater.marsedit5
  3. Launch MarsEdit 5.2 and Check for Updates
  4. Accept the update 5.2.1, and follow it through to completion.

Although the upgrade works, the notification about modifying other apps appears. This reflects Sparkle's updater attempting to call renamex_np to swap the two apps, and then falling back to the older method of copying to a temporary folder to swap.

Possible Fix

I am not sure if the alerts will appear for any app, or only apps with particular mixes of sandboxing, hardened runtime, etc. If it can be determined that the alert (and associated API failure) occur only for some kinds of apps, I propose that the condition of those apps be detected and the attempt to invoke the renamex_np function be avoided.

Version

default branch running macOS Sonoma 14.5

zorgiepoo commented 3 weeks ago

As investigated in Slack together, the issue is caused by a combination of #2505 + a bug with renamex_np + the new downloaded app update having a "stricter than default" NSUpdateSecurityPolicy which disallows another process with the same team ID to modify the app.

Keeping this issue to see if we can detect this and opt sparkle into the worse "less-atomic" path in this case to prevent a prompt (we currently only do this if the team ID from Autoupdate and the new app differs, but not account for an overridden stricter update policy).

In mean time I suggest removing NSUpdateSecurityPolicy in new updates especially if the allowed processes (e.g. Setapp) don't need to be allowed for Sparkle builds (updating from an old version should still work without issue as long as the new downloaded update doesn't have the strict NSUpdateSecurityPolicy). It should be possible to add the allowed team ID / signing ID to this policy too, but need to investigate the complexity of suggesting that.

zorgiepoo commented 2 weeks ago

Opting apps that use a custom update security policy out of atomic swap API call in https://github.com/sparkle-project/Sparkle/pull/2593 , also generating a warning if such a policy is added.

It’s technically possible to support a custom update policy that allows Sparkles Autoupdate helper but it’s “hard” at the moment (since the signing ID of Autoupdate is not currently guaranteed to be fixed). And anyway this is just a workaround. For the vast majority it should be fine to not add a custom update security policy. Sparkle does not encourage it.

Out of curiousity from apps shipping on setapp I took a random survey and found out like 10 / 50 apps kept the setapp update security policy in their direct builds from their websites. Of these 10 apps they weren’t using an impacted Sparkle build (yet). Surprisingly I only found one or two Electron based apps on setapp.

zorgiepoo commented 2 weeks ago

Published 2.6.4 release for those using package managers with this fix.

For the record, the order of arguments in renamex_np does not matter.