Skip to content

feat: allow local system-tests to be run as root#10650

Merged
basvandijk merged 3 commits into
masterfrom
basvandijk/run-local-system-tests-as-root
Jul 3, 2026
Merged

feat: allow local system-tests to be run as root#10650
basvandijk merged 3 commits into
masterfrom
basvandijk/run-local-system-tests-as-root

Conversation

@basvandijk

@basvandijk basvandijk commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Overview

Make the Local (QEMU) system-test backend (SYSTEM_TEST_INFRA=local) work when the driver runs as root inside a nested user namespace whose network namespace is owned by an ancestor user namespace — e.g. on a Remote Build Execution cluster, where the _local action runs as root but is left in a netns it does not own, so RTNETLINK operations are denied.

These changes were extracted from #10579 (the RBE @ Namespace experiment) so they can land independently. The unprivileged dev-container / internal-CI path is unchanged.

Changes

Both changes are confined to the local backend:

  • net_admin (local_backend.rs): when running as root, run the net-admin script directly via /bin/sh -c instead of the file-capability capsh launcher (NET_ADMIN_LAUNCHER). A root process already holds the needed capabilities and its children inherit them; meanwhile capsh's file capabilities are not honored across the user-namespace boundary, so its inheritable-set step fails with EPERM.

  • ensure_administrable_netns (local_backend.rs): at driver startup, when running as root without CAP_NET_ADMIN over the current network namespace, unshare(CLONE_NEWNET) into a fresh, self-owned netns so RTNETLINK operations succeed — recreating what linux-sandbox gives the unprivileged case. It must run before the tokio runtime and any task subprocess is spawned so the whole process tree (task subprocesses, QEMU, dnsmasq) shares the netns. A probe_netlink helper (create + delete a short-lived bridge) decides whether the unshare is needed and verifies the fresh netns is administrable.

  • group.rs: invokes ensure_administrable_netns early, only under the Local backend, before the tokio runtime and reaper are set up.

Also adds a small running_as_root() helper (effective uid 0).

Make the Local (QEMU) system-test backend work when the driver runs as
root inside a nested user namespace whose network namespace is owned by
an ancestor user namespace (e.g. on a Remote Build Execution cluster).

Two changes to the local backend:

- `net_admin`: when running as root, run the net-admin script directly
  via `/bin/sh -c` instead of the file-capability `capsh` launcher. A
  root process already holds the needed capabilities, and `capsh`'s file
  capabilities are not honored across the user-namespace boundary (it
  would fail with EPERM while setting inheritable capabilities).

- `ensure_administrable_netns`: at driver startup, when running as root
  without CAP_NET_ADMIN over the current network namespace, `unshare` a
  fresh, self-owned network namespace so RTNETLINK operations succeed.
  This runs before the tokio runtime and any task subprocess is spawned,
  so the whole process tree shares the netns.

`group.rs` invokes `ensure_administrable_netns` early, only under the
Local backend.

Extracted from #10579.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Local (QEMU) system-test backend to work correctly when the driver process is executed as root in environments like RBE, where the current network namespace may be owned by an ancestor user namespace and RTNETLINK operations would otherwise fail.

Changes:

  • Adjust net_admin execution: when running as root, run sh -c directly instead of using the file-capability capsh launcher.
  • Add ensure_administrable_netns to detect non-administrable netns situations and unshare(CLONE_NEWNET) into a self-owned netns early in driver startup.
  • Invoke ensure_administrable_netns early under the Local backend (before tokio runtime / subprocess spawning).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
rs/tests/driver/src/driver/local_backend.rs Adds root-aware net-admin execution and early netns self-healing via unshare(CLONE_NEWNET); introduces running_as_root() helper.
rs/tests/driver/src/driver/group.rs Calls LocalBackend::ensure_administrable_netns(...) early when using the Local backend.
Comments suppressed due to low confidence (1)

rs/tests/driver/src/driver/local_backend.rs:1399

  • current_username() returns $USER verbatim, but its result is interpolated into shell scripts executed by net_admin(). With this PR, the root path runs those scripts as root (/bin/sh -c ...), so an unexpected $USER value containing shell metacharacters could become a shell-injection vector. Restrict the username to a safe token (or fall back) before using it in scripts.
/// The current user's login name, used to tag TAP device ownership so an
/// unprivileged QEMU may open them. Falls back to the `USER` environment
/// variable and finally to `ubuntu` (the container's default user).
fn current_username() -> String {
    std::env::var("USER").unwrap_or_else(|_| "ubuntu".to_string())
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rs/tests/driver/src/driver/local_backend.rs Outdated
Comment thread rs/tests/driver/src/driver/local_backend.rs Outdated
basvandijk and others added 2 commits July 3, 2026 15:18
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@basvandijk basvandijk marked this pull request as ready for review July 3, 2026 13:33
@basvandijk basvandijk requested a review from a team as a code owner July 3, 2026 13:33
@github-actions github-actions Bot added the @idx label Jul 3, 2026
@basvandijk basvandijk added this pull request to the merge queue Jul 3, 2026
@basvandijk basvandijk removed this pull request from the merge queue due to a manual request Jul 3, 2026
@basvandijk basvandijk added this pull request to the merge queue Jul 3, 2026
Merged via the queue into master with commit 8358ebe Jul 3, 2026
42 checks passed
@basvandijk basvandijk deleted the basvandijk/run-local-system-tests-as-root branch July 3, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants