[Bug]: Subsandbox should base its environment on the environment of the original flatpak run

[smcv]

Checklist

• I agree to follow the Code of Conduct that this project adheres to.
• I have searched the issue tracker for a bug that matches the one I want to file, without success.
• If this is an issue with a particular app, I have tried filing it in the appropriate issue tracker for the app (e.g. under https://github.com/flathub/) and determined that it is an issue with Flatpak itself.
• This issue is not a report of a security vulnerability (see here if you need to report a security issue).

Flatpak version

1.14.1

What Linux distribution are you using?

Other (specify below)

Linux distribution version

any

What architecture are you using?

x86_64

How to reproduce

Scenario 1: variables that are passed through unaltered by Flatpak

1. In one terminal: SOME_VARIABLE=portal /usr/libexec/flatpak-portal -vvr
2. In another terminal: SOME_VARIABLE=run flatpak run --command=bash org.gnome.Recipes (any app will do)
3. In the org.gnome.Recipes shell: flatpak-spawn -- sh -c 'env|grep SOME_'
4. Expected result: SOME_VARIABLE=run is inherited from flatpak run
5. Actual result: SOME_VARIABLE=portal is inherited from flatpak-portal

Scenario 2: variables that are used internally by Flatpak

1. weston --socket temp-wayland-1 --backend=wayland-backend.so & (or use Xephyr :11 on X11 systems)
2. weston --socket temp-wayland-2 --backend=wayland-backend.so & (or use Xephyr :22 on X11 systems)
3. Arrange the Weston or Xephyr windows so you know which one is which
4. In one terminal: WAYLAND_DISPLAY=temp-wayland-1 /usr/libexec/flatpak-portal -vvr (or use DISPLAY=:11 on X11 systems)
5. In another terminal: WAYLAND_DISPLAY=temp-wayland-2 flatpak run --command=bash org.gnome.Recipes (any Wayland GUI app will do; or use DISPLAY=:22 on X11 systems)
6. In the org.gnome.Recipes shell: gnome-recipes
7. Close the window
8. In the org.gnome.Recipes shell: flatpak-spawn -- gnome-recipes
9. Expected result: both gnome-recipes windows appear inside the second Weston or Xephyr
10. Actual result: the first gnome-recipes window inherits WAYLAND_DISPLAY or DISPLAY from flatpak run and correctly appears inside the second Weston or Xephyr, but the second gnome-recipes window unexpectedly inherits WAYLAND_DISPLAY or DISPLAY from flatpak-portal and appears inside the first Weston or Xephyr.

Expected Behavior

(see above)

Actual Behavior

(see above)

Additional Information

In these illustrative examples I'm running flatpak run and flatpak-portal manually, with obviously-different environment variables. The Wayland or X11 display is a highly-visible example which is convenient for testing, but this can happen for any environment variable.

In real life, flatpak run would inherit its environment from the desktop environment (GNOME Shell or equivalent), or from a terminal where the user ran flatpak run.

Similarly, in real life, flatpak-portal would inherit its environment from systemd --user on systemd systems, or from dbus-daemon --session on non-systemd.

#4539 is closely related to this. If this issue was resolved, then that would solve 95% of #4539: the only thing remaining to be solved separately on #4539 would be to keep using the same specific commit of each extension as in the original app launch, even if it was no longer the latest.

#5271 is also closely related to this.

[smcv]

Here is one possible way to solve this:

• During startup, the original flatpak run serializes its environment variables (not the sandbox's, but its own!) in env -0 format, and saves it into a file per instance, probably in the $XDG_RUNTIME_DIR
• When flatpak-portal runs flatpak run for a subsandbox on behalf of a running Flatpak instance, it loads that file and uses it to populate the environment that flatpak run will inherit, instead of letting flatpak run inherit the environment from flatpak-portal
• Any modifications to the environment that are made by flatpak run (for example removing the TMPDIR, overwriting the XDG_RUNTIME_DIR, etc.) proceed as usual from there

Implementation notes:

• The file with the serialized environment might contain secrets, so it must have 0600 or 0400 permissions
• We should probably not allow the sandboxed app to read it (we can mount a read-only empty file over the top, or something like that)

[smcv]

flatpak-portal cannot just read the /proc/*/environ from the original flatpak run, because the original flatpak run used execve() to replace itself with bwrap.

It also cannot just read the /proc/*/environ from the bwrap process that replaced the original flatpak run, because flatpak run runs bwrap with an empty environment.

If we changed flatpak run to let bwrap inherit the environment from flatpak run (which is somewhat desirable anyway in case bwrap needs to be run with a LD_LIBRARY_PATH or something), and used bwrap --clearenv to get the desired empty environment instead, then we could use the /proc/*/environ of the bwrap process (which is conveniently in env -0 format already); but that might not work on legacy systems where bwrap is setuid. So saving the environment to a file is probably more robust.

[doraskayo]

@smcv, have you had a chance to work on this? This issue breaks user expectation and typical workflows with apps that started relying on subsandboxes (Steam, Chromium-based, etc.). It's quite a nuisance.

[doraskayo]

@smcv, a few questions:

1. Do you consider mounting a read-only empty file over the file containing the environment variables to be the best approach here?
   • Alternative ideas, without actually testing that they are feasible:
     1. Create a separate directory tree for "private" files of each app instance that wouldn't be exposed to the sandbox. This may end up quite involved with separate locking and garbage collection.
     2. Introduce another directory level in the current app instance directory so it contains two directories: "public" (?) and "private". The contents of the current app instance directory would be placed in the "public" directory, which would still be mounted in the sandbox but in its old location for backwards compatibility. The "private" directory would not be mounted in the sandbox and could be used to keep the environment file.
        • is the content of the app instance directories outside the sandbox considered some kind of "stable API"?
2. Is it acceptable to not inherit any environment variable from flarpak-portal when it executes flatpak run for the subsandbox? It does make sense to me, but I just want to confirm.
   • What would you do with pressure-vessel?

[doraskayo]

@smcv, do you have an opinion on the above?

[smcv]

If you're interested in working on this, please do!

@smcv, have you had a chance to work on this?

If you have not seen replies from me, please assume "no". I have a lot of things on my to-do list, some of them are time-sensitive, and everyone wants their favourite issue to be my top priority.

Do you consider mounting a read-only empty file over the file containing the environment variables to be the best approach here?

If I had a chance to work on this, one of the pieces of work I would have been doing would be answering this question; but because I haven't, sorry, I don't have a definitive answer.

is the content of the app instance directories outside the sandbox considered some kind of "stable API"?

Yes, I believe other components like xdg-desktop-portal rely on ${XDG_RUNTIME_DIR}/.flatpak/${instance} having its current contents, both inside and outside the sandbox, so please don't move that around.

We could potentially have some sort of ${XDG_RUNTIME_DIR}/.flatpak/${instance}-private or something like that, which is available outside the sandbox but not shared with the sandboxed app? That seems like its cleanup would have the same requirements as ${XDG_RUNTIME_DIR}/.flatpak/${instance}.

Or mounting an empty file over the top for the container "view" of the file containing the environment would be another option.

Is it acceptable to not inherit any environment variable from flarpak-portal when it executes flatpak run for the subsandbox? It does make sense to me, but I just want to confirm.

I think as long as we inherit the environment from a host process, it doesn't have to be specifically flatpak-portal, and it would be OK (and internally consistent) for it to inherit a copy of the environment of the flatpak run that started the "parent" sandbox. (The equivalent of env -i A=b X=y... flatpak run ... where A=b, X=y, ... are the environment of the original flatpak run, except we'd want flatpak-portal to use something more like the implementation of --env-fd.)

What would you do with pressure-vessel?

pressure-vessel already sets about a million environment variables inside the subsandbox ("below" flatpak run, but "above" the game process), so it inherits very little from flatpak run. In the examples in my original issue report here, we are probably already overwriting SOME_VARIABLE.

The place where this really matters is when flatpak run looks up environment variables from the host so that it can set up things like GL, X11 and Wayland, like the WAYLAND_DISPLAY in my original issue report.

I would want to test the proposed PR with pressure-vessel to make sure there isn't something unexpected going on, because pressure-vessel is a heavy and specialized user of the subsandbox API, but I think what we've described here would be fine for it.