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:

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