Bug 465502 - Flatpak KCM generates broken override config
Summary: Flatpak KCM generates broken override config
Status: RESOLVED FIXED
Alias: None
Product: systemsettings
Classification: Applications
Component: kcm_flatpak (show other bugs)
Version: master
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Plasma Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-09 11:25 UTC by ratijas
Modified: 2023-04-20 13:36 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.27.5


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description ratijas 2023-02-09 11:25:04 UTC
SUMMARY

Somehow Flatpak KCM ended up generating the following file:

$HOME/.local/share/flatpak/overrides/cc.arduino.IDE2:

```
[Environment]
B=D
D
D
D
D
D
A=B
[System Bus Policy]
```

Also `flatpak list` command crashed because of it, but it has been fixed already.  Still we should not generate malformed useless overrides. See discussions at https://invent.kde.org/plasma/flatpak-kcm/-/merge_requests/35

STEPS TO REPRODUCE
1. Open Flatpak KCM
2. Open any app's page
3. Scroll down to advanced settings, expand and scroll further down to Environment.
4. Add some environment variables through the dialog. The dialog does not clean up previous values, so you may try add the same pairs multiple times, or even omit key or value.

OBSERVED RESULT
Config is broken, `flatpak list` crashes.

EXPECTED RESULT
Reasonable warning and error messages in UI, sanitizing input, not writing malformed config to file.

SOFTWARE/OS VERSIONS
Operating System: Arch Linux 
KDE Plasma Version: git-master
Qt Version: 5.15.8
Kernel Version: 6.1.9-arch1-2 (64-bit)
Graphics Platform: X11
Processors: 8 × Intel® Core™ i7-6700HQ CPU @ 2.60GHz
Memory: 15.6 GiB of RAM
Graphics Processor: NVIDIA GeForce GTX 970M/PCIe/SSE2
Manufacturer: ASUSTeK COMPUTER INC.
Product Name: G752VT
System Version: 1.0

See also: https://github.com/flatpak/flatpak/issues/5293
Comment 1 Nate Graham 2023-02-12 17:05:03 UTC
Keeping severity NOR since we've disabled the environment variable UI for now to avoid this; otherwise it would be HI or even VHI.
Comment 2 Prajna Sariputra 2023-02-17 13:20:12 UTC
(In reply to Nate Graham from comment #1)
> Keeping severity NOR since we've disabled the environment variable UI for
> now to avoid this; otherwise it would be HI or even VHI.

Unfortunately that UI still appears if a Flatpak app has at least one environment variable override added by the user (for example via Flatseal), and while neither editing an existing environment variable's value nor adding a new one works (so no damage is done in those cases) disabling/deleting environment variables do take effect, and in that case it also generates an invalid override file and prevents the affected app from launching.
Comment 3 ratijas 2023-02-22 13:36:45 UTC
> Unfortunately that UI still appears if a Flatpak app has at least one environment variable override added by the user (for example via Flatseal), and while neither editing an existing environment variable's value nor adding a new one works (so no damage is done in those cases) disabling/deleting environment variables do take effect, and in that case it also generates an invalid override file and prevents the affected app from launching.

Right… There were two functions: for loading default values, and for existing overrides. I ended up only commenting out default values.
Comment 4 ratijas 2023-03-02 22:12:31 UTC
Git commit ce407199aae085210107e57fef0fd8ed7b0b63bc by ivan tkachenko.
Committed on 02/03/2023 at 22:10.
Pushed by ratijas into branch 'master'.

FlatpakPermission: Disable loading environment from user overrides as well

M  +6    -3    flatpakpermission.cpp

https://invent.kde.org/plasma/flatpak-kcm/commit/ce407199aae085210107e57fef0fd8ed7b0b63bc
Comment 5 ratijas 2023-03-02 22:20:16 UTC
Git commit 7e45e4931ea1a84000255c54ae62bf2d2b8bc2d2 by ivan tkachenko.
Committed on 02/03/2023 at 22:17.
Pushed by ratijas into branch 'Plasma/5.27'.

FlatpakPermission: Disable loading environment from user overrides as well
(cherry picked from commit ce407199aae085210107e57fef0fd8ed7b0b63bc)

M  +6    -3    flatpakpermission.cpp

https://invent.kde.org/plasma/flatpak-kcm/commit/7e45e4931ea1a84000255c54ae62bf2d2b8bc2d2
Comment 6 Bug Janitor Service 2023-04-18 13:44:59 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/flatpak-kcm/-/merge_requests/107
Comment 7 ratijas 2023-04-18 20:19:02 UTC
Git commit 12ab644f8a3d64a35f3a3608b24caee86d48b956 by ivan tkachenko.
Committed on 18/04/2023 at 17:52.
Pushed by ratijas into branch 'master'.

FlatpakPermissionModel: Enable loading and adding environment variables

Since we have reimplemented all the reading and writing code and added
input validation to protect against empty keys (env. var. names), we can
finally safely bring them back in the UI.

M  +0    -1    autotests/flatpakpermissiontest.cpp
M  +4    -9    flatpakpermission.cpp
M  +1    -1    package/contents/ui/permissions.qml

https://invent.kde.org/plasma/flatpak-kcm/commit/12ab644f8a3d64a35f3a3608b24caee86d48b956
Comment 8 ratijas 2023-04-18 20:19:09 UTC
Git commit 29e236f97becfd699e8a38fd4101d28428e4a3dc by ivan tkachenko.
Committed on 18/04/2023 at 20:15.
Pushed by ratijas into branch 'Plasma/5.27'.

FlatpakPermissionModel: Enable loading and adding environment variables

Since we have reimplemented all the reading and writing code and added
input validation to protect against empty keys (env. var. names), we can
finally safely bring them back in the UI.
(cherry picked from commit 12ab644f8a3d64a35f3a3608b24caee86d48b956)

M  +0    -1    autotests/flatpakpermissiontest.cpp
M  +4    -9    flatpakpermission.cpp
M  +1    -1    package/contents/ui/permissions.qml

https://invent.kde.org/plasma/flatpak-kcm/commit/29e236f97becfd699e8a38fd4101d28428e4a3dc