Change filesystem access to xdg-music only

[Eonfge]

Not functional at the moment, requires some upstream attention

[flathubbot]

Started test build 30130

[Eonfge]

@bellegarde-c In line with Flatpak specs, I made a special Lollypop branch that has the sandbox more restrictive. I hope you can take a look at it, because at the moment there are two issues:

• The player crashes when you try to access an existing catalog directly
• The folder-selector is very rough, and doesn't seem to integrate with the GtkFileChooserNative

I have no intention of releasing this in its current state, but I do want you and others to be able to test it.

[flathubbot]

Build 30130 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/29016/org.gnome.Lollypop.flatpakref

[bellegarde-c]

flatpak install --user https://dl.flathub.org/build-repo/29016/org.gnome.Lollypop.flatpakref

Done this but looks like Lollypop is not sandboxed here.

[bellegarde-c]

[Eonfge]

Weird. I previously had issues opening the portal browser. Well, I'll run it through a few more tests in that case.

The primary problem right now... is that this change is not backwards compatible. Users will have to re-select their music folders (other then dxg-music) to reimport their libraries. Is this worth the hassle?

Do you foresee any issues when we tighten up the sandbox permissions?

@bilelmoussaoui do you have any comments concerning the sandboxing of Lollypop?

[flathubbot]

Started test build 30498

[flathubbot]

Build 30498 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/29378/org.gnome.Lollypop.flatpakref

[bilelmoussaoui]

As lollypop allows the user to select their music folder/add new ones, the change in the permissions won't work for everyone. Although the portal does support selecting folders now, it won't work for users with older version :/ So I would prefer to wait a bit before merging this.

[Eonfge]

I actually ran into that myself. My version of Lollypop had a direct link to a non-xdg location. So over time, people will have to re-configure lollypop for the paths to properly migrate? I'll test that a few times, and we might have to hold off on it for a year or so, making sure that the fallout is not to big.

[flathubbot]

Started test build 30912

[flathubbot]

Build 30912 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/29778/org.gnome.Lollypop.flatpakref

[flathubbot]

Started test build 30979

[flathubbot]

Build 30979 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/29844/org.gnome.Lollypop.flatpakref

[Eonfge]

@bilelmoussaoui I've done some testing. As of right now, it will cause issues when we deploy this update, even for users who only downloaded Lollypop yesterday.

What I've tested:

• Clean environment (Fedora 33, GNOME Boxes)
• Install lollypop flathub master
• Add custom folder with music (~/CustomMusicFolder/)
• Import, refresh, play for testing purposes
• Status: All good so far
• Install lollypop flathub NoMoreHost
• import, refresh
• Status: Files can't be found

I had to re-select my music library and I had to reimport it. The current version of Lollypop and GtkFileChooserNetive, don't use a a /var/run/ mount when selecting files, if they don't have to. As such, when updating lollypop it will be unable to access the files in ~/CustomMusicFolder/

So, what's the plan? I do want to improve the security in the future

[bellegarde-c]

@Eonfge Can I do something from Lollypop POV ?

[bilelmoussaoui]

Nothing can be done regarding the migration as the files/directories you grant an application the permission to open/read/write are stored in the flatpak documents store and are added once you open a file using the portal (the native file dialog). The directories has to be re-added so that Lollypop has actually the permission granted to open/read those files instead of "having permissions to everything"

[bilelmoussaoui]

@Eonfge Can I do something from Lollypop POV ?

what would be nice is to migrate all file choosers to native ones as a first step

[Eonfge]

@bellegarde-c I was thinking about that. Would it be possible to add a -DFlatpak=true flag to meson?

You can then show a message that informs users about their folders and permissions when they start Lollypop and a folder is missing. If a folder is missing, you can tell users like like "Sandboxing good. Sandboxing WIP. Please re-import".

The comment of @bilelmoussaoui is certainly true, making sure all file choosers are up-to-date is important, but I think we can't escape the realization that enforcing sandboxing does require a minor action from the user.

[Eonfge]

@bellegarde-c Would it be possible to provide some user-notification when they start the Flatpak version first?

If you could include such a message, I can combine it with this change. Then, when users update Lollypop to a more sandboxed version, they'll be informed about the change. The flag and notification could be temporary, because as time progresses all people will migrate. After a few months, we could remove it.

[flathubbot]

Started test build 32608

[flathubbot]

Build 32608 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/31417/org.gnome.Lollypop.flatpakref

[flathubbot]

Started test build 32676

[flathubbot]

Build 32676 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/31475/org.gnome.Lollypop.flatpakref

[bilelmoussaoui]

@bilelmoussaoui What I want is detecting configured folders in Lollypop with access denied by sandbox.

You can query the documents portal and see which permissions your app has for a specific folder/file

[Eonfge]

• The pop-up about 'Automatically download metadata?' doesn't actually enable network access, thus being non-functional

This one is working here. Do you mean that lollypop is not downloading artwork or that the setting is not updated ?

It would be the setting then. This is what I see after I click agree:

[bellegarde-c]

It modifies the background data, it's another issue :D

[bellegarde-c]

Updated Lollypop to handle migration only if needed.

[flathubbot]

Started test build 35501

[flathubbot]

Build 35501 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/34162/org.gnome.Lollypop.flatpakref

[Eonfge]

I've successfully tested the following scenarios:

• Do a fresh installation, select custom folder
• Do a fresh installation, use xdg-music
• Have an existing library, have music in a custom folder, re-import folder required
• Have an existing library, have music in xdg-music and a custom folder, xdg-folder is instantly available, custom folder must be re-imported
• Have an existing library, have music in xdg-music only, no re-import required

Bugs

• UI Bug: in scenario 2, the dxg-folder is not visible when I open the preferences menu for the first time. It only becomes visible if I then clos it and re-open it.
• Underwater Bug: In all scenarios, I get a message that Last FM and Listen Brains can't find the appropriate files.
  log.txt

In summary

I think we have this fully working 😄 I would invite others to try it, but I haven't found any issues.

[bellegarde-c]

UI bug was a Lollypop bug, fixed ;)

[flathubbot]

Started test build 35514

[flathubbot]

Build 35514 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/34174/org.gnome.Lollypop.flatpakref

[Eonfge]

Well, all seems well. Let me know when there is a new Lollypop release and I'll release it with the improved sandboxing.

[flathubbot]

Started test build 35572

[flathubbot]

Build 35572 failed

[flathubbot]

Started test build 35573

[flathubbot]

Started test build 35574

[flathubbot]

Build 35573 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/34228/org.gnome.Lollypop.flatpakref

[flathubbot]

Build 35574 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/34229/org.gnome.Lollypop.flatpakref

[bellegarde-c]

Time to merge this, 1.4.8 is out.

[Eonfge]

Closing this branch in favour of #122 so we don't have a whole series of surplus comments on Git