What I did
Repository rules / “don’t edit” areas
From CONTRIBUTING.md and subprojects/README.md, subprojects/ contains vendored/submodule/copylib code (bubblewrap, libglnx, dbus-proxy, variant-schema-compiler). I treated subprojects/ as third-party and excluded it from typo fixing.
You already skip po/ (translations) and node_modules/, and I kept those exclusions.
Typos fixed (project-owned files only)
I ran codespell with write mode and exclusions, and fixed the reported typos across:
NEWS
app/…
common/…
doc/…
tests/…
session-helper/…
portal/…
data/…
Then I handled the remaining items individually:
NEWS: thse -> these
common/flatpak-utils-private.h: Thse -> These
app/flatpak-polkit-agent-text-listener.c: identies -> identities
tests/test-auth.sh: Propertly -> Properly
tests/testlibrary.c: remore -> remote
common/flatpak-transaction.c: improved wording to avoid the xwindows typo (X11 window ID)
Added .codespellrc
Created .codespellrc:
skip: node_modules,po,subprojects
ignore-regex: .*(ratatui|Affinitized|affinitized).*
ignore-words-list: nd,ot,THUR,IST,fo,hel,bu
(these were confirmed as legitimate tokens/abbreviations/namespace prefix/test strings in this repo, so they should not be “fixed”)
Verification:
codespell --config .codespellrc . now exits clean.
Signed-off-by: rezky_nightky <with.rezky@gmail.com>
Only G_FILE_ATTRIBUTE_STANDARD_NAME is necessary here for
g_file_info_get_name() and g_file_enumerator_get_child() purpose. This
avoids various thumbnailing house-keeping that GIO tries to do when any
thumbnailing file attribute is requested (fairly visible when run with
strace), in addition to just making more sense that way.
A few years ago there was a very painful attempt at porting from
libsoup2 to libsoup3. Flatpak libsoup3 support never landed and it seems
like a large amount of distros have switched over to libcurl instead.
This commit removes libsoup2 support completely instead of growing
libsoup3 support.
Closes#5915Closes#4582
Instead of clearing the environment that we spawn the flatpak executable
with, we use the new --clear-env. For environment variables that we
inherit from the calling instance, we append them as arguments after the
--clear-env so it starts from a clean slate.
Closes: https://github.com/flatpak/flatpak/issues/5271
Most access to the `client_pid_data_hash` hash table are unsafe due to
threading.
One approach to solve this would be to protect the hash table with a
mutex, but as per a deeper analysis, nothing in these callbacks is
slow or heavy enough to justify the need for separate threads.
Make method invocations run in the main thread.
Closes: https://github.com/flatpak/flatpak/issues/5605
The original intention was to add 'sandbox-a11y-own-names', which would
match the prefix of other arguments, and it's what was documented in the
D-Bus XML and in flatpak-spawn too.
Fixes 8ec21a28f25cc7c40cc9e30555ad4283cec0ed94
In context of the previous commit, this allows Flatpak apps to spawn
subsandboxes with `--a11y-own-name=DBUS_NAME`, where `DBUS_NAME` must
have the app id as prefix.
For example, `org.webkitgtk.MiniBrowser` would be able to spawn a Web
process using the Flatpak portal, and by passing
`org.webkitgtk.MiniBrowser.Sandboxed.WebProcess0`, this Web process
would be able to own this name in the a11y bus. This allows the Web
process and the main WebKit process to connect their a11y trees across
sandboxes.
With older GLib, it's provided by libglnx, but with newer GLib, we need
to include the correct header.
Fixes: 7b1cd206 "Replace flatpak_close_fds_workaround() with g_fdwalk_set_cloexec()"
Signed-off-by: Simon McVittie <smcv@collabora.com>
As discussed in #5695, I think we're reaching a point where removing
Autotools is preferable to fixing it.
1.14.x continues to use Autotools, so platforms whose Meson version is
too old can stay on that branch until it becomes unsupported. We have
a very conservative Meson dependency (Ubuntu 20.04).
Signed-off-by: Simon McVittie <smcv@collabora.com>
flatpak_close_fds_workaround() wasn't technically async-signal-safe,
because the requirement for sysconf() to be async-signal-safe was
removed in POSIX.1-2008.
It could also leave high fds open in some cases: in practice
sysconf(_SC_OPEN_MAX) returns the soft resource limit, but if our
resource limit has been reduced by an ancestor process, we could
conceivably still have fds open and inherited above that number.
We can fix this by using g_fdwalk_set_cloexec() with GLib >= 2.79.2,
or the backport in libglnx with older GLib. This uses close_range()
if possible, falling back to rummaging in /proc with async-signal-safe
syscalls.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Otherwise, the child process will inherit GIO_USE_VFS=local, breaking
its ability to use GVfs and other GIO plugin interfaces.
Resolves: https://github.com/flatpak/flatpak/issues/5567
Signed-off-by: Simon McVittie <smcv@collabora.com>
Instead of inheriting the portal's environment when spawning a
subsandbox using flatpak-run, inherit the environment in which
flatpak-run was originally executed for the parent instance.
This means that environment variables that affect the sandbox setup
of the parent instance now also propagate to the setup of
subsandboxes, including "FLATPAK_GL_DRIVERS".
Closes: https://github.com/flatpak/flatpak/issues/5278
Now that we are logging `flatpak -v` messages with log level INFO,
and printing INFO messages in the same way as DEBUG, we can reserve
log level DEBUG for `flatpak -v -v` messages. This means we no longer
need a weird secondary debug domain.
There is a very small behaviour change here: G_MESSAGES_DEBUG=flatpak
is now similar to `flatpak -v -v` (previously `flatpak -v`), and
G_MESSAGES_DEBUG=flatpak2 no longer has any effect. This seems more in
line with what would be expected from a GLib-based application.
In flatpak(1) and the system helper, this does not change behaviour
other than that: the same messages are logged by `-v` and by `-v -v`
as before.
In daemons that do not implement `-v -v` (the OCI authenticator, portal
and session helper), it continues to be necessary to use
G_MESSAGES_DEBUG to see flatpak_debug2() messages.
Signed-off-by: Simon McVittie <smcv@collabora.com>
To make indentation work with less effort. The modeline was copied from
libostree with minor modification and the .editorconfig from GLib.
The advantage of having both a modeline and an editorconfig is we can
work out of the box on more editor setups, and the modeline allows us to
specify the style with a lot more fine grained control.
Recent Meson versions have warnings if you add the subprojects
directory as an include path, because the way Meson wants to consume
subprojects is by the subproject's build system producing a Meson
dependency object that encapsulates its include directory. Flatpak
doesn't have a Meson build system yet, but I'm working on that.
libglnx seems to be set up to have the libglnx directory be its include
path instead: for example, ostree (by the author of libglnx) already
uses "libglnx.h" or <libglnx.h> everywhere. Do the same here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
During unit testing we don't have a complete Flatpak app or runtime
available, and `flatpak run` is not necessarily in FLATPAK_BINDIR yet;
but we can run the portal with this environment variable set, to
specify a mock implementation of Flatpak.
This helps to reproduce #4286.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Just because we can allocate a new, unused fd in the portal's fd space,
that doesn't mean that fd number is going to be unused in the child
process's fd space: we might need to remap it.
Resolves: flatpak/flatpak#4286
Fixes: aeb6a7ab "portal: Convert --env in extra-args into --env-fd"
Signed-off-by: Simon McVittie <smcv@collabora.com>
This will allow us to add additional mapping entries for fds to be
used internally by `flatpak run`, in particular --env-fd.
Defer the second pass through the fd array until the last possible
moment, so that any extra fds we want to add (like the --env-fd) have
already been added by then.
Helps: flatpak/flatpak#4286
Signed-off-by: Simon McVittie <smcv@collabora.com>
Otherwise we'll run out of file descriptors eventually, when starting
a sufficiently large number of subsandboxes.
Resolves: flatpak/flatpak#4285
Fixes: aeb6a7ab "portal: Convert --env in extra-args into --env-fd"
Signed-off-by: Simon McVittie <smcv@collabora.com>
In D-Bus, handles are defined to be unsigned, but in GVariant, for some
reason they're signed. Make sure they aren't negative, which could
result in a NULL dereference for fds.
A handle used in the conventional way will never legitimately be
negative (in GVariant's interpretation) or have its high bit set
(in D-Bus' interpretation), because file descriptors are signed 32-bit
integers, so an array of distinct file descriptors can never be long
enough for the distinction between signed and unsigned to matter.
In practice fds are limited by the kernel to several orders of
magnitude fewer than that anyway.
Fixes: 3ebf371f "run: Allow caller to replace /app and/or /usr"
Signed-off-by: Simon McVittie <smcv@collabora.com>
The pressure-vessel container tool in Steam will want to use this, to
replace /usr with a Steam Runtime container supplied by the Steam CDN,
instead of using the same Flatpak runtime that is used to run the Steam
client and non-containerized games.
If a custom /usr is used, the "official" Flatpak runtime is still the
one reflected in the metadata. It is also mounted at /run/parent,
with all its extensions, so that pressure-vessel has the option of using
its graphics drivers (by populating the custom /usr with symlinks into
/run/parent and/or /run/host).
When doing this, we need to put an empty directory on /app, because
the real /app expects to be run on top of the real runtime. It would
also be reasonable to substitute a custom replacement for /app, so
I've included support for that too.
Partially addresses #3797.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Running Flatpak Chromium on NixOS fails with the following error:
> Error calling Spawn(): org.freedesktop.DBus.Error.FileNotFound: Failed to start command: Failed to execute child process “flatpak” (No such file or directory)
Presumably, Chromium calls portal’s `Spawn` method with `FLATPAK_SPAWN_FLAGS_CLEAR_ENV` flag, which also removes `PATH`.
Since NixOS does not install programs to global `/usr/bin` and relies solely on `PATH`, this is probably what prevents `flatpak` command itself from being found.
There is a relevant TODO note in the code about `LD_LIBRARY_PATH` but at least for `PATH`, we can solve the issue by hardcoding the path to the binary.
This is really just syntactic sugar for running `env -u VAR ... COMMAND`,
but env(1) is inconvenient when the form of the COMMAND is not known:
if the COMMAND might contain an equals sign, you end up having to run
`env -u VAR sh -c 'exec "$@"' sh COMMAND`. Let's make this simpler.
This follows up from GHSA-4ppf-fxf6-vxg2 to fix an issue that I noticed
while resolving that vulnerability, but is not required for fixing the
vulnerability.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Previously, if you launched a subsandbox while specifying environment
variable overrides, any environment variable overrides that existed
in the parent Flatpak app would take precedence:
host$ flatpak run --env=FOO=1 --command=bash example.app
[📦 example.app ~]$ env | grep FOO
FOO=1
[📦 example.app ~]$ flatpak-spawn --env=FOO=x sh -c 'env | grep FOO'
FOO=1
This does not seem like least-astonishment, and in particular will
cause problems if the app wants to override LD_LIBRARY_PATH in the
subsandbox. Change the precedence so that the environment variables
set by flatpak-spawn will "win":
host$ flatpak run --env=FOO1=1 --env=FOO2=2 --command=bash example.app
[📦 example.app ~]$ env | grep FOO
FOO1=1
FOO2=2
[📦 example.app ~]$ flatpak-spawn --env=FOO1=x sh -c 'env | grep FOO'
FOO1=x
FOO2=2
This follows up from GHSA-4ppf-fxf6-vxg2 to fix an issue that I noticed
while resolving that vulnerability, but is not required for fixing the
vulnerability.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If the caller specifies a variable that can be used to inject arbitrary
code into processes, we must not allow it to enter the environment
block used to run `flatpak run`, which runs unsandboxed.
This change requires the previous commit "context: Add --env-fd option",
which adds infrastructure used here.
To be secure, this change also requires the previous commit
"run: Convert all environment variables into bwrap arguments", which
protects a non-setuid bwrap(1) from the same attack.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
This hides overridden variables from the command-line, which means
processes running under other uids can't see them in /proc/*/cmdline,
which might be important if they contain secrets.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
As with flatpak run --parent-expose-pids, this will only work if we have
a working, non-setuid bwrap. Systems where user namespace creation is
restricted and bwrap needs to be setuid (Debian 10, RHEL/CentOS 7,
Arch Linux linux-hardened kernel) will have degraded functionality.
This option is similar to --expose-pids, except that instead of making
the subsandbox use a nested pid namespace inside the parent's, it makes
the subsandbox share the parent's pid namespace as-is, so that process
IDs in the parent and the subsandbox are interchangeable. This will
be useful if the parent and the subsandbox communicate via protocols
that assume a global view of the process ID namespace, for example
passing process IDs across an AF_UNIX socket or in shared memory.
In particular, this will be useful for Steam's pressure-vessel container
tool: the IPC between the Steam client and the "game overlay" loaded into
Steam games uses process IDs, and becomes confused if they don't match up.
This weakens the security boundary between a subsandbox and the parent,
but that's OK in some cases, especially if the subsandbox is being used
as a way to get a different runtime /usr (flatpak-spawn --latest-version
or #4018) rather than as a security boundary.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Previously, we'd silently ignore remapped or sandbox-exposed fds that
were not included with the D-Bus message, which seems unlikely to
work as intended.
Signed-off-by: Simon McVittie <smcv@collabora.com>
