RetroArch
RetroArch copied to clipboard
libretro
Android: Remove external storage permission
Description
The android client requests blanket access to internal storage, even though it only needs to access the self-contained /sdcard/RetroArch folder.
By getting rid of this permission, we can improve sandboxing and show respect to the user's privacy. Also, this issue is what's holding up play store updates.
We do this by moving the RetroArch folder to app-specific storage, /sdcard/Android/data/com.retroarch.aarch64/files/RetroArch. A DocumentsProvider is added to allow the user to easily access it. And a migration flow is added to prompt existing users to copy their RetroArch folder to the new location.
Consequences are that users will have to copy their content into app-specific storage before they play them, and that some automated backup workflows will be disrupted.
It wasn't practical to accomplish this by leaving the folder in place and accessing it through DocumentsContract, since the files couldn't be accessed through the POSIX apis that the rest of the codebase relies on, and because it has a performance penalty.
Related Issues
This issue is discussed here: https://github.com/libretro/RetroArch/issues/12181
Related Pull Requests
[Any other PRs from related repositories that might be needed for this pull request to work]
Reviewers
[If possible @mention all the people that should review your pull request]
if this will allow us to update the Play Store version, this is very exciting stuff! We very much appreciate the contribution!
I'm not very familiar with Android at all, but fairly familiar with iOS. I took a look at this PR and it seems reasonable and straightforward to me, but I did have a couple concerns.
@cwyc , could you please describe in more detail how the migration affects:
- paths in retroarch.cfg and playlists
- roms that may not exist in the retroarch directory?
For the paths in the .cfg and .lpl files, on iOS if a file exists in the sandbox, the filename is prefixed/truncated to indicate it should be searched for in the sandbox. Does the same thing happen on Android? If so I'd expect it's probably not an issue except for any the files the user has created themselves outside the sandbox, which is basically the same question as 2.
@warmenhoven Thank you for pointing this out, I hadn't considered path references in config files.
Yes, retroarch.cfg and playlists do carry absolute paths. We will need to update them when we copy content over. This is complicated by the fact that the app gives users the option to individually relocate the content directories, so they might need to be selected and copied in separate operations.
As for roms outside of the RetroArch folder, they won't be copied over, and playlists referring to them will be broken. But for roms that are copied, we can update the paths in content*_history.lpl and other playlist files we find on a best-effort basis.
The current migration flow assumes all this exists in ~/RetroArch. Instead, perhaps it needs to begin by searching config and playlist files for external references, giving users the option to select and copy each to standard locations in app-specific storage, and updating the paths. It will also have to be recursive, as files in the content directories might reference others. ROMs can go in a RetroArch/roms folder.
Still, we might not be able to catch and update all the references, and some people's setups might get messed up. I'd be open to other approaches.
@cwyc Ok so in my opinion:
- getting it on the play store again is a big important thing,
- upgrades/migrations are always an place where people are expecting weirdness/brokenness,
- and people who are putting retroarch-related stuff outside the retroarch folder are already more likely to be equipped to be able to fix the brokenness manually;
- so my opinion is that worst case this is fine to merge as is. Still, there should be best effort made to do something better if at all easy/possible. If not, at least you tried, and people will get over it.
Given that, I think it's safest not to try to get too smart/tricky with it. Update the paths in the .cfg and .lpl files that you are already directly affecting (the most common cases), and everything else the user is going to have to sort out for themselves.
I don't know if there's an easy way to give people an option to migrate or not. If it were me as an end-user, I'd rather just start over fresh if weird breakage were a possibility.
We already have the welcome message that shows on first launch, so we could duplicate that function to make another message that shows only when the other message is disabled (i.e., already been seen) and refers them to a docs page that lays all this out in more detail.
I'm just wary, in general, of automagic stuff.
But whatever, I'm fine with it either way. Any breakage/discomfort is going to be transitory anyway, and, as warmenhoven stated, just getting the Play Store version updated is worth pretty much any amount of temporary inconvenience.
If it will make only possible to play ROMS only from folder “/sdcard/Android/data/com.retroarch.aarch64/files/RetroArch” than no, just delete google play support or make sure that APK version from site sill have external storage permission support. I anyway use APK from site because google play version is limited. Google don’t care about emulation scene anyway and you should not care about their paranoid security stuff that mostly makes no sense. Also deleting retroarch will delete also your saves and savestates. So, if you implement google play support, make sure it’s still possible to use retroarch in convenient way for APK from site. I will not use google play version anyway
RetroArch, either the site build or the playstore build is essentially useless for big collection of roms until it supports reading roms or making the playlists read roms from SAF (storage access framework).
The reason is that SAF middleware like RSAF is the only way to access localhost or remote NAS, like webdav, nfs, samba, etc. I can run (even if it's annoying to lose the nice playlist interface, because only dolphin actually bothers to iterate the dirs when looking for roms) remote roms in android PPSSPP, dolphin, etc using RSAF and a Linux webdav server. I can't in RA.
Ideally I'd be able to permit access to a webdav directory, with the usual portable playlist directory change and leave it at that, although I doubt it will ever happen.
@hizzlekizzle I've added some code that just does a find-and-replace in .cfg and .lpl files, replacing the old default path with the new location. For good measure it makes a backup before it does so. Agree with @i30817 integrating the file picker with SAF should be a priority, but I don't have experience with that codebase.
Playlist path auto changing has some danger because the manual playlists portable playlist relocation feature already autochanges paths in some circumstances. Please test what happens with portable playlists.
The trigger for the portable playlist change is explained in this post and the next one: https://github.com/libretro/RetroArch/issues/9163#issuecomment-1834096083
I think it's a bit of a obscure feature. Useful though.
About SAF, I don't really get the holdup. It's been like this for years and it's not like people are asking for implementations of what they want to use SAF for (webdav, samba). Other people did that, exactly so emulator devs won't have to care.
There are examples of c++ apps asking for a android SAF permission for a single dir and operating in the resulting path in scummvm (they scan the resulting path for games, recursively), so it's not even a question of only a single dir being available.
In fact, I wouldn't be surprised if the scummvm core allowed RSAF to access remotes from its internal gui, which would be extremely ironic. Since the core is just upstream now, plus a few tweaks and a gui to create a playlist.
I should try it out actually. Edit: didn't work unfortunately, the
@i30817 I couldn't get the playlist relocation behaviour to work through the upgrade. At least with content_history.lpl, it looks in the old locations if we don't otherwise patch the paths. Any idea why?
But maybe we need to switch our focus to SAF in native code. I'll look into how scummvm does it. And if we achieve that, those who prefer it can leave the folder in sdcard.
It's possible that the portable playlist feature was left half baked when jdgleaver left, and content history never changes paths even if you change rgui_browser_directory and it's different from base_content_directory (that exists, ie it's not the first time you enable portable playlists with those files).
There is no reason for that be because base_content_directory actually exists in that playlist if you enable portable playlists, I have it.
It can also be that you have to restart RetroArch before these changes get reflected on the playlists, I kind of recall something like that. That is in itself a flaw, if so.
Edit: to get in the same page, I think RA portable playlists work by saving rgui_browser_directory in each playlist as base_content_directory if it doesn't exist yet, and every time a RetroArch instance starts up, and portable playlists are on and for each playlist base_content_directory that is different from rgui_browser_directory, replace the paths parts with the old base_content_directory by the current gui_browser_directory and change the directory seperator of the rest of the path (if necessary), then replace base_content_directory by rgui_browser_directory again, then write out the playlist.
This sounds like something a first iteration would do when loading the playlists into memory the first time, instead of each time rgui_browser_directory changes.
I don't understand how that algorithm interacts with a forced path change in between enabling portable playlists and they getting written out if rgui_browser_directory changes. Maybe you only have to also delete base_content_directory from each playlist and force turn off portable playlist (to force the user to choose a base_content_path inside the RetroArch apk sandbox when and if they turn it on again).
(Edited out rant about useless dirs in the android RA filechooser start)
(Edited out rant about how the users will have to move the roms anyway so automagic is all useless really, and portable playlists can change the paths anyway - too cynical, and I think it's pretty clear that the average user has no clue how to use portable playlists 😂 if even devs don't know about it)
Edit: thinking some further, we obviously want a runtime mechanism like scummvm to ask android to ask the user for read\write permissions on the rom directory and copy, but not change playlists at all. Normally that path would be authorized during the scan, but on this transition or using portable playlists, it would be when changing rgui_browser_directory \ running the new version the first time.
RetroArch even has a very similar place to scummvm to put this in on ozone at least, where the file browser starts with memorized shortcut paths (scummvm allows adding authorized paths to its own shortcut paths by going up to the root and triggering <add directory> option, they just don't start already there but in the last dir used). Most of the paths already there are some variant of useless in android though (no permissions to read)
Every other path in the retroarch.cfg and playlists that might need to be changed should be user visible warnings at least, if those files are copied over (off the top of my head, the assets file, rgui_browser_directory, base_content_dir in playlists (may not correspond to the path a user added as the rom scan paths), may be forgetting some.
But honestly, changing locations of the base RetroArch dir is a minefield, it's the one thing that breaks a lot of ideas of using the same configs between installs or platforms. I do not recommend saving anything but the playlists, and even then you should probably warn about paths outside allowed dirs (but not change them IMO, except, see the next paragraph).
If at the same time, this allows RSAF to work, you get external SD card and webdav\NAS storage, and play store update, that's worth a few settings nuked, even portable playlists if necessary (base_content_directory in playlists might not be authorized, even if the rom dir was - imagine it's a parent of the authorized rom dir - I imagine this situation might confuse the update code, although rgui_broswer_directory would be null if retroarch.cfg was reset, so nothing might happen at first - even if you only copy the playlists during the transition as expect the user to authorize the rom dir for them to work, it might be worth it to remove base_content_directory, if it's not readable).
Edit: there is a catch 22 where to save the playlists into the new sandbox the new RetroArch has to ask permission to the user to find the playlists in the expected place (outside the sandbox). You can't even check if the playlists exist in /storage/emulated/0/RetroArch/playlists without asking for the permission first I think, making the idea not very friendly unless newer versions stop trying eventually, and assume anyone that wanted to preserve playlists already did.
@cwyc hey man, just so you don't think we've forgotten about this/you: we're hoping to get another stable release out very soon and then we'll be digging into this immediately after. We very much appreciate the work and are excited for the possibilities this opens up.
The biggest issue with this is going to be rom storage. Moving them over to the data directory with everything else will result in their deletion if RetroArch is uninstalled. I would definitely recommend making sure users can still select alternative locations. Or consider using the same way Seleco does it with Mame4Droid 2024, and place the files in the /sdcard/Android/media folder instead of /sdcard/Android/data.
SAF doesn't require roms to be in the program sandbox, just that the directory be allowed access (in the android file chooser\files app). The user visible part of the local filesystem (the so called internal storage) is still accessible.
Along, and this is the important part, other mount points in the file app, like NAS drives, webdav, sdcards etc, which are added with 3rd party apps that slot in to the files app as a permission manager. Thing is, I doubt very much it will be simple to implement considering how heavily RetroArch integrated a file manager, because android wants NOTHING else than the Files app giving access to files to programs.
@LibretroAdmin Why was this PR merged? I don’t believe we’ve come to consensus about how to handle this issue. We were talking about directly accessing files through DocumentsProviders.
@cwyc Oh, I thought this was ready to go, at least to see if it makes it through Google's approval gauntlet. Is this salvageable as-is or do we need to revert? I (at least; can't speak for others) am okay with some temporary breakage on the nightly build, which is why we waited to get a stable release out before doing anything here.
@hizzlekizzleI mean, it’s a big change that will mess up people’s workflows, and I think we can find a better solution that works in a different way. I think it would be best to revert it.
I was asked to merge it. Maybe there was some misunderstanding or miscommunication.
Anyway, I will revert it now then.
How do we proceed from here?
Yeah, my bad.
So, what still needs to be resolved is how people are going to get ROMs into RetroArch's reachable locations without putting them at risk of deletion during app uninstallation, right? And some stuff about playlists?
As you may have inferred, I'm of the opinion that getting the Play Store package up-to-date is of the utmost importance, even if the transition is bumpy.
Yeah, my bad.
So, what still needs to be resolved is how people are going to get ROMs into RetroArch's reachable locations without putting them at risk of deletion during app uninstallation, right? And some stuff about playlists?
Not just the roms. Anything that goes into the 3 SAF folders (data, media and obb) would suffer that same fate - cloudsync would solve that for playlist, saves and states though.
It's funny, Apple opens up to emulation (within reason) but Google seems to want to make it as difficult as possible (I doubt they are singling out emulation with all their changes, it's more of a side effect).
Hi! Unless I've completely misunderstood this PR, implementing it would be very problematic. It would make all external frontends (ES-DE, Daijishou, Dig etc.) unusable with RetroArch, and like mentioned above, clearing the storage for the RetroArch app or uninstalling it would also delete all ROMs. In addition to this a lot of people mix RetroArch with standalone emulators using the same ROM directories and those emulators would also not be able to access the games any longer.
The best approach would be to add SAF support, or to give RetroArch MANAGE_EXTERNAL_STORAGE permissions. However the latter would likely get rejected by Google so a Play release would be impossible.
But you could argue whether the Play store is a lost cause anyway, the crazy restrictions that Google has introduced to their store have now reached a point where it makes it close to impossible to ship complex applications that actually work as intended. It's a usability nightmare to deal with all the SAF setup and Google is likely going to make it even worse with future OS releases and app store policy changes. It's probably best to drop Play store support and focus on publishing via your website, as you are already doing actually. The alternative stores like Samsung Galaxy Store, Huawei AppGallery and Amazon Appstore also don't have these restrictions so MANAGE_EXTERNAL_STORAGE works fine when publishing there.
@LibretroAdmin Just to confirm, you've reverted this PR? As mentioned it would be incredibly disruptive and make a lot of setups unusable. Thanks! :)
Well, I wouldn't say 'misunderstood' but this PR is to give SAF access. Which then creates the issue of where to place roms and other support files so they don't get accidentally deleted by an uninstall or a bad update.
Removing external storage permission is important not only for play store access, but also because it’s an enormous sandboxing hole. You’re giving a program, which regularly executes arbitrary code often from questionable sources, access to all of your photos and files. But as others have said, a less disruptive solution would be to directly access DocumentsProvider files.
Thanks for your comments, well as long as it's thoroughly tested and doesn't break frontend support for instance then I guess improvements to the storage and security model are always appreciated!
Being able to choose a folder like PPSSPP (/sdcard/PSP for example) when starting is the least, imagine deleting RetroArch and losing all your saves that you invested a lot of time, this way it would still be completely safe and whenever you reinstall it you would keep the data as it is now
I've looked a bit more at this PR and my understanding is that it may be very disruptive. Moving directories to application-internal storage, i.e. under Android/data will make them inaccessible by other apps on recent Android versions (version 13 and later?). I'm not sure how the DocumentsProvider will help here, as someone may just want to access the savestates or whatnot using a file manager, or syncing data between various devices using a third party app and so on.
In addition to this, launching games from external applications like frontends will probably be broken too. Take for example the following am command:
am start -n com.retroarch.aarch64/com.retroarch.browser.retroactivity.RetroActivityFuture -e CONFIGFILE /storage/emulated/0/Android/data/com.retroarch.aarch64/files/retroarch.cfg -e LIBRETRO cap32_libretro_android.so -e ROM /storage/emulated/0/ROMs/amstradcpc/3D_Fight.zip
This is basically how frontends do it, although for example ES-DE is of course not using the am command directly but is natively utilizing the Intent API. It's still the same effect though, if removing storage access permissions from RetroArch it would not be able to load the ROM file. And many people also want to relocate their ROMs to external storage such as an SD card, which requires that RetroArch has access to this external storage device.
And as mentioned in a previous comment many people mix emulators and use RetroArch and standalone emulators interchangeably with the same ROMs directory. Adding to the above there's the problem of someone uninstalling RetroArch or clearing its internal storage from the Android app settings, which would wipe all saves, savestates, ROMs and so on.
Again, please let me know if I misunderstood the approach taken here as I've not tested the code in the PR. I'm all for modernizing RetroArch as far as Android is concerned but it really needs to be done in a way that is non-disruptive so that users retain the functionality they have today and that third party apps like frontends can continue to function as they do today.
Thanks! :)
So, was this reverted completely? Any chance it will be implemented again in a future? It's weird to see the app is on iOS main app store and not in Android main app store.
Other emulators have already implemented workarounds, and in most of them you are able to choose a ROM folder, and a separate folder to store/access saves, etc, or in emulators like Dolphin, you are even able to export/import a ZIP file with all your save content for backup purposes. Why this cannot be implemented into RetroArch?
