Android: Automaticaly add MANAGE_EXTERNAL_STORAGE permission to apps

[sabae-valve]

Description

This prevents the user from needing to click through a permission granting screen on their device before the agent is able to run on the device. Presumably, if you have adb access to the device, we can assume it's alright to add this permission automatically. This is only relevant on API >= 30, as the MANAGE_EXTERNAL_STORAGE permission is not available on earlier Android versions [0].

[0] https://developer.android.com/reference/android/Manifest.permission#MANAGE_EXTERNAL_STORAGE

[baldurk]

Aside from the code formatting issue, I'm not sure about this change in general. Unfortunately Android is a completely broken and unpredictable hellscape of nightmares so adding any new command usage like this even seemingly innocuous has the chance to completely break things on some devices because there's no way to predict or have any confidence that these will work universally. We do use this shell pm command in other places but not for the grant command which means that some devices may not have grant and the failure case for Android can be catastrophic in terms of completely breaking the ability for anything to work.

Although this would be an improvement, the tradeoff is a benefit of convenience vs a very real risk of complete non-functioning. Generally speaking I don't add any new Android commands unless they are absolutely necessary and there's no alternative due to how completely and utterly terrible the platform is.

[sabae-valve]

no way to predict or have any confidence that these will work universally

pm grant has been standard in Android since 6.0 [0]. If a permission is given to pm grant that it does not understand, the pm utility exits with an error code after taking no other action. I am ignoring the return code of adbExecCommand so if it fails execution can continue, and I am also only doing this on API version 30+, where this permission (and pm grant) are guaranteed to be understood. Automatically running pm grant is a standard installation flow for developer tools (for instance, Unreal Engine uses it in its development deploy scripts) so it should be extremely low-risk to include.

[0] https://developer.android.com/about/versions/marshmallow/android-6.0-testing

[baldurk]

I don't have that kind of confidence I'm afraid. I've seen time and time again that things which you would expect are standard and reliable in Android end up missing on some devices - since Android devices can basically be customised to any degree possible by the vendors. It's almost impossible to overstate or exaggerate just how fundamentally broken Android is as a platform.

There's no guarantee or requirement that anything available in upstream Android will still be present in a customised version. Similarly error handling that might be present upstream isn't necessarily present downstream so what might be silently ignored upstream could cause show-stopping breakage in a different Android device.

The result is I'm very conservative with what changes are allowed to minimise this risk and only take it when necessary - in this case it's a minor annoyance to have to click past this dialog when a new version is installed. It would be nice to fix it, and on a sane platform it would be fixable, but Android is not sane.

[sabae-valve]

I have updated this to only run the commands if there is already a property set on-device, which should make this very conservative. We already check properties in other areas, so this should be quite safe.