Allow build-time tests to be run in environments where bwrap can't work

[smcv]

Resolves #1497.

In limited build environments, the tests can be run as:

XDP_VALIDATE_ICON_INSECURE=1 XDP_VALIDATE_SOUND_INSECURE=1 meson test -C ${builddir}

(variable names chosen to discourage users from making use of this in production to validate genuinely untrusted icons/sounds).

• xdp_validate_icon: Assign argument indices automatically

• xdp_validate_icon: Allow sandboxing of the validator to be disabled

  OS distributions often compile packages and run their build-time tests
  in a chroot environment or an unprivileged container, and bubblewrap
  cannot usually work in either of those environments.

• xdp_validate_sound: Assign argument indices automatically

• xdp_validate_sound: Allow sandboxing of the validator to be disabled

  Same reasoning as for the icon validator.

[jsparber]

We have sandboxed-image-validation and sandboxed-sound-validation as build options. IMO they should control whether the sandbox is used instead of only forcing bwrap as a required dependency.

We can just check for sandboxed-image-validation and sandboxed-sound-validation at https://github.com/flatpak/xdg-desktop-portal/blob/main/src/meson.build#L171 and only set HELPER when we actually want sandboxing.

[smcv]

We have sandboxed-image-validation and sandboxed-sound-validation as build options. IMO they should control whether the sandbox is used instead of only forcing bwrap as a required dependency.

They do, indirectly, control whether the sandbox is used (the logic might not be 100% ideal, but it's good enough, and improving it is out-of-scope for #1497 or for this PR).

But, that doesn't solve my problem. If I was willing to ship a version of x-d-p in Debian that had insecure, non-sandboxed validation, that would be fine, and I could already do that. (But that would be doing a disservice to our users, so I don't want to.)

The problem I have is that I want a version of x-d-p that by default enforces sandboxing for its validators, so that they will be secure-by-default on end-user systems; but during build-time testing I need an escape hatch that lets me turn that off, because in the distro's automated build environment it's impossible, and I'd prefer to run build-time tests with imperfect coverage rather than no build-time tests at all.

[swick]

I agree with the rationale. Would be nice if we could auto-detect this in the tests. The python tests will have the same problem but are currently not installed.

[ebassi]

I also agree that an escape hatch for build time testing is a good idea.

@smcv should the environment variables also be documented somewhere, for other downstream packagers running in the same issue?

[swick]

FWIW, we already have XDG_DESKTOP_PORTAL_TEST_APP_ID which the tests use to define the app id it should assume for the test clients.

This is something the test setup sets itself though so we should figure out if we want the test setup to detect the bwrap situation and set the env accordingly and leave it an implementation detail.

[smcv]

The python tests will have the same problem but are currently not installed

For "as-installed" tests like Debian's autopkgtest and GNOME's installed-tests concept, this is not actually such a concern: in autopkgtest we can flag certain test suites as isolation-machine, meaning "won't work in a chroot/container", and the infrastructure will either run them on a VM or bare-metal machine, or not at all. We do this for bubblewrap, flatpak and xdg-desktop-portal already.

It's specifically build-time tests via meson test that are my concern here: distros usually run their builds (and hence build-time tests) in a rather limited environment, traditionally a chroot, but increasingly often a container instead.

should the environment variables also be documented somewhere, for other downstream packagers running in the same issue?

Yeah, probably. Any suggestions for where?

Would be nice if we could auto-detect this in the tests

That would be nice, but making the escape hatch exist seems like a prerequisite for making use of it automatically, and is considerably simpler.

Auto-detection would look a lot like check_fuse_or_skip_test(), except instead of mounting a FUSE filesystem, we'd be running bwrap ... true and seeing whether it worked.

[ebassi]

Yeah, probably. Any suggestions for where?

Do we want to have a man page for the validation tools? I assume Debian will ask for one. In the meantime, we can have a README.md in the tests directory.

[smcv]

Do we want to have a man page for the validation tools? I assume Debian will ask for one.

They aren't in the PATH and are an implementation detail of x-d-p (not intended to be run directly by end users), so there's no requirement to have a man page. And, the way I implemented these environment variables in this PR doesn't actually affect the implementation of the validation tools, so it would seem inappropriate to document them in the tools' man pages: instead, it makes x-d-p avoid passing --sandbox to them. So, if anything, it would be x-d-p's own man page (which doesn't exist, but could if we wanted it to) that should document these environment variables, along with XDG_DESKTOP_PORTAL_DIR, XDG_DESKTOP_PORTAL_TEST_APP_ID, XDP_VALIDATE_ICON and XDP_VALIDATE_SOUND.

In the meantime, we can have a README.md in the tests directory.

Sure, that would make sense.

[swick]

We already have a bunch of undocumented environment variables and the changes in this PR LGTM so I'm in favor of merging this as is.

[smcv]

we can have a README.md in the tests directory

Added, no further changes (except for rebasing).