Manual review of plasma-discover snap

I’d like to be able to publish the plasma-discover snap, which triggered a few problems in the automated review.

The snap name is registered to KDE, and the source is currently available at /neon/snap-packaging/discover-snap on the KDE forge. It is a snap version of the existing plasma-discover KDE application, intended for use in an Ubuntu Core context (meaning restricted to its snapd backend).

It triggered the following failures:

  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (home-snap, personal-files), needed to access the $HOME/snap directory for installed snaps, in read and write
  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (plasma-discover-notifier, snapd-control), needed to let the notifier service access snapd
  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (plasma-discover, desktop-launch), needed to launch installed snaps from the Discover UI
  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (plasma-discover, snapd-control), needed to let the Discover UI access snapd
  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (system-snap, system-files), needed to access snap-related directories in /var, /usr, /snap, and other files required for launch or install, in read-only
  • human review required due to 'deny-connection' constraint (interface attributes) declaration-snap-v2_slots_connection (dbus-kde-discover-notifier, dbus), needed to let the notifier service register on dbus
  • human review required due to 'deny-connection' constraint (interface attributes) declaration-snap-v2_slots_connection (dbus-kde-discover, dbus), needed to let the Discover UI register on dbus
2 Likes

Hey @DaSpood

Sorry for the delay. Could you please clarify why is access to $HOME/snap and following directories required?

- /snap
- /var/snap
- /var/lib/snapd
- /usr/bin/snap
- /run/user
- /etc/xdg/menus
- /usr/share/applications/mimeapps.list

Hello @jslarraz

@DaSpood is currently vacationing so I’ll answer in his stead. I don’t have all the fine details at hand but he left a few comments about this in the snapcraft.yaml here is the relevant extract:

    system-snap:
        interface: system-files
        read:
            - /snap           # $SNAP
            - /var/snap       # $SNAP_DATA and $SNAP_COMMON
            - /var/lib/snapd  # Needed to read snap dir options
            - /usr/bin/snap   # Needed to run snap commands
            - /run/user       # Needed to read the Auth file of installed snaps
            - /etc/xdg/menus  # Needed for desktop-launch
            - /usr/share/applications/mimeapps.list  # Needed for desktop-launch

To me this seems to be all related to the snapd-glib use within Discover and the way it introspects to be able to launch installed snaps.

Regards.

This was discussed, and we resorted to using dbus, which doesn’t need access to the files like the snap binary or /snap directory. I was also working on improving it, but his one was merged for some reason.

https://invent.kde.org/plasma/discover/-/merge_requests/873

The Discover patch was merged because the needed snapd changes aren’t merged yet. And we need a solution which works both in and outside Ubuntu Core which the dbus interface doesn’t provide yet.

Of course it’s the better solution once the dbus interface changes in snapd are merged, but this better solution is unfortunately not the practical “today solution” yet.

So if I understood it correctly, the requested system-files are needed to launch other snaps from the Discover. Whilst the requested interfaces seems to be needed by the snap provide its expected functionality, it is not a very common design pattern.

@pedronis does it looks appropriate to you?

Ping @pedronis - can you please provide guidance on this request for the KDE plasma-discover snap?

Apologies for the delay, I am now back.

I went and checked again if the home-snap and system-snap permissions were still fully necessary, and discover appears to be working without them. These were probably remnants from an earlier implementation of the app-launch feature, but the up-to-date dbus implementation doesn’t seem to require it anymore.

I have opened a PR to remove them from the snapcraft.yaml file (https://invent.kde.org/neon/snap-packaging/discover-snap/-/merge_requests/2).

This means from my original post, only the following failures should still be relevant:

  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (plasma-discover-notifier, snapd-control), needed to let the notifier service access snapd
  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (plasma-discover, desktop-launch), needed to launch installed snaps from the Discover UI
  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (plasma-discover, snapd-control), needed to let the Discover UI access snapd
  • human review required due to 'deny-connection' constraint (interface attributes) declaration-snap-v2_slots_connection (dbus-kde-discover-notifier, dbus), needed to let the notifier service register on dbus
  • human review required due to 'deny-connection' constraint (interface attributes) declaration-snap-v2_slots_connection (dbus-kde-discover, dbus), needed to let the Discover UI register on dbus

With the following two no longer applying:

  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (home-snap, personal-files), needed to access the $HOME/snap directory for installed snaps, in read and write
  • human review required due to 'allow-installation' constraint (bool) declaration-snap-v2_plugs_installation (system-snap, system-files), needed to access snap-related directories in /var, /usr, /snap, and other files required for launch or install, in read-only

@jslarraz @pedronis

@alexmurray would it be possible to get a second look at this after the recent change described in my previous message ?

Since the purpose of plasma-discover is to find and install applications, it is not surprising then that it requires snapd-control to install snaps and desktop-launch to then launch then (similar to snap-store). As such, +1 to me for auto-connect of these interfaces.

The dbus slots have just been granted - hopefully another @reviewer can chime in for additional votes on snapd-control and desktop-launch.

1 Like

+1 from me too for desktop-launch and snapd-control to install applications. Same reason that Alex pointed above. +2 votes for, 0 votes against. This is now live.

To close the loop on this - note snapd-control is a super-privileged interface and so any snap requiring this needs to have publisher vetting. However, in this case plasma-discover is published by the KDE Verified Accounts and so is already vetted/verified.