baldurk / renderdoc

RenderDoc is a stand-alone graphics debugging tool.
https://renderdoc.org
MIT License
8.6k stars 1.3k forks source link

Grant MANAGE_EXTERNAL_STORAGE permission via adb shell appops #3327

Closed w-pearson closed 1 month ago

w-pearson commented 1 month ago

Description

This removes the need to manually grant RenderDoc this permission via the (automatically-opened) settings screen. As far as I can tell, appops does not require root access and has existed since before the MANAGE_EXTERNAL_STORAGE permission was added in Android 10.

baldurk commented 1 month ago

Unfortunately I don't want to make this change. Adding new adb calls in general and especially to new adb commands that haven't been used before I see as quite risky, since there is no guarantee of how adb behaves and it could vary significantly across devices, all the way up to crashing/hanging/disconnecting the connection. Any commands like this which aren't strictly required to work at all are avoided - we pass -g at install time but if that fails we rely on granting the permission at runtime.

w-pearson commented 1 month ago

That's reasonable. I'm going to give some more context, but ultimately it's fine if you don't want to include this.

I wouldn't expect commands ran via adb shell to cause a disconnect, as they run on the device itself and can be ran interactively if you run adb shell on its own. Though there are some exceptions, such as reboot, and there probably are commands that affect USB or networking. Other non-shell adb commands seem like a higher risk, such as adb root which needs to restart adbd. But I could totally see there being some devices that misbehave in very stupid ways, so that is still valid risk.

adb install -g doesn't grant MANAGE_EXTERNAL_STORAGE. adb help and adb shell pm help both say that -g: grant all runtime permissions. The documentation says runtime permissions are marked as the dangerous protection level in android.Manifest.permission, and one example of that is the older WRITE_EXTERNAL_STORAGE permission (which no longer works as of API level 30/Android 11). But MANAGE_EXTERNAL_STORAGE is marked as signature|appop|preinstalled, and the documentation says that appop corresponds to special permissions instead, which have their own requesting process (which RenderDoc does already implement). It's a bit silly that there are multiple different kinds of permissions with different ways of requesting them, but that's just how it is unfortunately.

The documentation on MANAGE_EXTERNAL_STORAGE does directly mention this appops command,

Regular end-users probably don't have much of a reason to care about the MANAGE_EXTERNAL_STORAGE popup since they probably are using stable releases and would only need to re-grant the permission when a new stable release comes out. It only becomes a nuisance when RenderDoc needs to be reinstalled frequently. This happens if using devices that are frequently re-flashed to an image without RenderDoc installed, and also when developing RenderDoc itself. Both of these are pretty niche, though.