Manual review of plasma-desktop-session snap

hello @cav Likewise we did a few adjustments to this one. It’ll require a new manual approval. The changes are mainly:

  • more system files to be able to read to validate users through pam (screenlock to do its work properly)
  • screen-inhibit-control now being needed as a slot

Thanks in advance!

hi @kottens - just for extra clarity, the additional system-files interface files you’re looking for auto-connection are apart of shell-config-files which are:

  • /etc/alsa
  • /etc/xdg
  • /etc/writable/default/im-config
  • /etc/shadow
  • /etc/pam.d
  • /etc/login.defs
  • /var/lib/extrausers

does this look correct?

Hello @cav

Yes, this is correct. Thanks!

+1 from me for both updates to shell-config-files and for screen-inhibit-control . what do other @reviewers think?

hi @kottens - I was also double checking the list of files in shell-config-files and found that /var/lib/extrausers may already be given read access by default as seen in the default template in the defaultOtherBaseTemplateRules section.

hi @cav hm… this is a bit surprising, apparmor would clearly flag the violation for me without this line. isn’t this template bit mainly for non-core systems? this snap is for use on a core system.

ah yes - in that case I stand corrected - my bad!

+1 from me for granting plasma-desktop-session read access to the requested files.

+2 for, 0 against granting read access following files via system-files interface. This is now live.

  • /etc/alsa
  • /etc/xdg
  • /etc/writable/default/im-config
  • /etc/shadow
  • /etc/pam.d
  • /etc/login.defs
  • /var/lib/extrausers

Regarding screen-inhibit-control, I think it was designed to be sloted by base snaps. @jamesh is it now expected to be used with session snaps?

Thanks

This is the PR making the interface usable on Core systems:

https://github.com/canonical/snapd/pull/14134

It is not an implicit slot on core because the services the application would talk to are running as part of a desktop session snap, as is being done here.

Thanks so much for the clarification @jamesh.

@jamesh @kottens just to be sure, is auto-connection desired for screen-inhibit-control? If I’m right, In that case all snaps will auto-connect to plasma-desktop-session:screen-inhibit-control

The existing behaviour of the implicit slot on classic seems to be to let it auto-connect, so having it auto-connect on core seems fine.

Sounds good.

Then, +1 from me for granting plasma-desktop-session slot auto-connection on the screen-inhibit-control.

It seems that the store dashboard does not allow reviewers to grant declarations for screen-inhibit-control, as it still says “This is a rule for a system only interface. No other snap can have such slots.”. I’ll ping the store team to get this fixed

1 Like

It should be live now. Please let me know if it works as expected

Thanks

1 Like

Hello @cav & @jslarraz

I did a new upload of this package for the switch to core24. It got flagged again for systemd-user-control interface and now we also need the upcoming pipewire interface as a plug.

Could this be let through allowing auto-connections please?

Thanks in advance.

1 Like

hi @kottens

Given the context of the snap being a desktop session, +1 for auto-connecting pipewire.

I also saw that there are updates to shell-session-locale-files and shell-config-files. Could you describe the updates to help with voting.

Finally, I saw the shutdown interface is requested. Is manual or auto-connection desired?

hi @cav

Oh yes, I forgot about shutdown! This one should auto-connect too please.

As for shell-config-files, we added /etc/security to the list. Somehow the lock screen becomes literally crazy without being able to list it when on core24.

shell-session-locale-files shouldn’t have changed since the last iteration.

Hello,

Any chance of having someone looking into this soonish?

This would be appreciated.

Hey @kottens

Could you please put your requests together in a topic? It will give the request visibility and will be easier to review

Thanks