search

hezral / clips

Multi format clipboard manager with extra features
GNU General Public License v3.0
39 stars 1 forks source link

Permissions seem aggressive #9

Closed danirabbit closed 3 years ago

danirabbit commented 3 years ago

While reviewing your app, I noticed that the flatpak sandbox permissions here seem pretty aggressive. Can you explain why your app needs:

all these filesystem permissions?

  - --filesystem=host
  - --filesystem=home
  - --filesystem=xdg-cache/com.github.hezral.clips
  - --filesystem=xdg-data
  - --filesystem=xdg-data/flatpak/exports/share
  - --filesystem=xdg-data/flatpak/app
  - --filesystem=/usr/share
  # - --filesystem=/usr/share/icons:ro
  - --filesystem=/usr/local/share/
  - --filesystem=/var/lib/flatpak/exports/share
  - --filesystem=/var/lib/snapd/desktop
  # needed for perfers-color-scheme

Access to the keyring?

  # keyring
  - --talk-name=org.freedesktop.secrets

GPU access?

  - --device=dri

Network access?

  - --share=network
hezral commented 3 years ago

Similar to the appstream-compose, it was based a referenced manifest.

network access for clips to retrieve basic info on URLs like below. it tries to get a page title. image

read-only access to get installed apps and app icons, since clips uses app name matching to know what app a content was copied from

keyring access for setting password for clips protection

I've updated the permissions with comments https://github.com/hezral/clips/commit/482a543e79e0f02f9face28f2212637b81768741

Confirmed working with latest build https://github.com/hezral/clips/actions/runs/1174143451

Will submit a new release

danirabbit commented 3 years ago

That makes sense for the network! Thanks for adding comments for some of the other filesystem lines, that helps a lot

You might want to double check the cache and keying permissions. I'm fairly certain that you shouldn't need access to the system for either of those features. I believe secrets and cache will be stored inside the sandbox, like gsettings

hezral commented 3 years ago

Thanks, confirmed cahce permission isn't required. Confirmed with https://github.com/hezral/clips/actions/runs/1176132494

But for the keyring it is needed and i don't see it in the .var/app// directory and when i removed it the code didn't work. I think it still uses the user's keyring session db

the keyring access is to store a password to protect contents. i didn't want to reinvent anything so i opted to use the keyring.

Screenshot from 2021-08-28 07 45 27

hezral commented 3 years ago

Closed with release in AppCenter