flatpak/xdg-desktop-portal

org.freedesktop.impl.portal.PermissionStore overpacking the data GVariant

A6GibKm opened this issue · 4 comments

Operating System

Fedora 40 Silverblue

XDG Desktop Portal version

1.18

XDG Desktop Portal version (Other)

No response

Desktop Environment

GNOME

Desktop Environment (Other)

No response

Expected Behavior

Running

flatpak permission-set devices camera yes --data=0 org.gnome.Snapshot.Devel && flatpak permission-show org.gnome.Snapshot.Devel

should return

Table   Object App                      Permissions Data
devices camera org.gnome.Snapshot.Devel no          0

and then Calling org.freedesktop.impl.portal.PermissionStore.Lookup with arguments (devices, camera) should return

({'org.gnome.Snapshot.Devel': ['no'], 'yes': ['org.gnome.Snapshot.Devel']}, 0)

Current Behavior

Lookup returns

({'org.gnome.Snapshot.Devel': ['no'], 'yes': ['org.gnome.Snapshot.Devel']}, <0>)

Note the <0>.

Steps to Reproduce

See above.

Anything else we should know?

See https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/2982. Software just reads the GVariant from lookup and sets it as it, on every iteration the GVariant gets more GVariant'y.

I don't think it should return 0, because that'd be a direct uint32. The permission store stores generic data, and the way to do that is by packing it in a variant, so it's expected that it returns <0>.

smcv commented

({'org.gnome.Snapshot.Devel': ['no'], 'yes': ['org.gnome.Snapshot.Devel']}, <0>)

This is correct for the documentation in https://docs.flatpak.org/en/latest/portal-api-reference.html#gdbus-method-org-freedesktop-impl-portal-PermissionStore.Lookup, which says this method returns (a{sas}v).

({'org.gnome.Snapshot.Devel': ['no'], 'yes': ['org.gnome.Snapshot.Devel']}, 0)

This would be incorrect, because the type of that data structure is (a{sas}i) which doesn't match the required signature.

Software just reads the GVariant from lookup and sets it as it, on every iteration the GVariant gets more GVariant'y.

If that's what other software does, then it's the other software (e.g. gnome-control-center) that will need to be adjusted.

In your example, if g-c-c wants to write back the same permissions unchanged, then it should be doing this (pseudocode):

Set({'org.gnome.Snapshot.Devel': ['no'], 'yes': ['org.gnome.Snapshot.Devel']}, <0>)

For example, if using GDBus, it might do this:

g_dbus_connection_call (
    ...,
    g_variant_new_parsed ("({'org.gnome.Snapshot.Devel': ['no'], 'yes': ['org.gnome.Snapshot.Devel']}, <0>)"),
    ...
);
smcv commented

'yes': ['org.gnome.Snapshot.Devel']

As a side note, if the documentation is accurate, then this seems wrong to me: the documentation says the dictionary is a map from app ID to array of permissions, and yes is not a valid app-ID. I would expect it to look something like this:

{
    'org.gnome.Snapshot.Devel': ['no'],
    'com.example.Foo': ['one', 'two', 'three'],
    ...
}

I been playing around with this since I opened it and the correct way to fix the borked settings would be to call (from d-spy)

('devices', true, 'camera', {'org.gnome.Snapshot.Devel': ['yes']}, <byte 0x00>)

In any case, I opened a MR against g-c-c at https://gitlab.gnome.org/GNOME/gnome-control-center/-/merge_requests/2506.