Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 131 additions & 16 deletions rust/crates/runtime/src/sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,14 @@ pub fn build_linux_sandbox_command(
return None;
}

let mut args = vec![
"--user".to_string(),
"--map-root-user".to_string(),
"--mount".to_string(),
"--ipc".to_string(),
"--pid".to_string(),
"--uts".to_string(),
"--fork".to_string(),
];
let mut args: Vec<String> = working_unshare_mapping()
.unwrap_or(UNSHARE_MAPPING_CANDIDATES[0])
.iter()
.map(|arg| arg.to_string())
.collect();
// The candidates already carry the namespace flags, so the probe and the
// launcher share a single argument shape; only the opt-in `--net` is
// added here.
if status.network_active {
args.push("--net".to_string());
}
Expand Down Expand Up @@ -282,6 +281,82 @@ fn command_exists(command: &str) -> bool {
.is_some_and(|paths| env::split_paths(&paths).any(|path| path.join(command).exists()))
}

/// Candidate `unshare` user-namespace mapping options, in preference order.
///
/// Most systems accept `--map-root-user` alone. Some hardened containers and
/// seccomp profiles block unprivileged writes to `/proc/self/uid_map`; there,
/// util-linux delegates to the setuid `newuidmap`/`newgidmap` helpers when
/// `--map-auto` is also present.
///
/// That fallback therefore depends on the setuid helpers (the `uidmap`
/// package on Debian/Ubuntu) and on the current user having a range in
/// `/etc/subuid` and `/etc/subgid`. When either is missing, `--map-auto`
/// fails and the startup probe rejects the candidate, keeping the plain form.
///
/// Each candidate is the **complete** static argument shape the launcher
/// uses (see `build_linux_sandbox_command`): mapping flags followed by the
/// namespace flags `--mount --ipc --pid --uts --fork`. The startup probe
/// runs each candidate verbatim (plus a trivial program), so probe success
/// implies launch success: on systems where the mapping works but the
/// namespace flags are denied (e.g. AppArmor-restricted CI runners that
/// block mount propagation in user namespaces), the probe fails and the
/// sandbox stays disabled instead of activating a launcher that always
/// errors.
///
/// `--net` is intentionally absent: it is appended only when network
/// isolation is active (the non-default path), and probing with it would
/// disable the sandbox on hosts that block network-namespace creation (e.g.
/// Docker's default seccomp profile) even when network isolation is never
/// requested.
const UNSHARE_MAPPING_CANDIDATES: &[&[&str]] = &[

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The "e.g. GitHub Actions" example is inaccurate: on ubuntu-latest the fallback passes this mapping-only probe but the full launcher (with --mount --ipc --pid --uts --fork) still fails (cannot change root filesystem propagation: Permission denied), so the sandbox stays broken/disabled there either way. Suggest citing an environment that blocks direct uid_map writes but permits the full launch (e.g. certain hardened containers/seccomp profiles), so the docs match reality.

&[
"--user",
"--map-root-user",
"--mount",
"--ipc",
"--pid",
"--uts",
"--fork",
],
&[
"--user",
"--map-root-user",
"--map-auto",
"--mount",
"--ipc",
"--pid",
"--uts",
"--fork",
],
];

/// Probe a candidate `unshare` mapping invocation with a trivial program.
fn unshare_probe(args: &[&str]) -> bool {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This probe is the regression. It validates only the mapping flags against the trivial program true, but the real launcher always adds --mount --ipc --pid --uts --fork — and on GitHub ubuntu-latest, AppArmor blocks mount propagation inside the user namespace. Empirically on the runner: unshare --user --map-root-user --map-auto true exits 0 (probe passes → sandbox activates), but unshare --user --map-root-user --map-auto --mount --ipc --pid --uts --fork sh -lc "echo alpha" exits 1 with cannot change root filesystem propagation: Permission denied. Result: CI's mock_parity_harness bash roundtrip returns empty stdout and fails (main control run passes). Please probe the full launcher argument shape — candidates should carry --mount --ipc --pid --uts --fork alongside the mapping flags — so probe success implies launch success.

std::process::Command::new("unshare")
.args(args)
.arg("true")
.stdin(std::process::Stdio::null())
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.status()
.is_ok_and(|status| status.success())
}

/// The first mapping option set that works on this machine, if any.
///
/// Probes are cached for the process lifetime; a missing `unshare` binary or a
/// kernel that refuses every mapping yields `None`.
fn working_unshare_mapping() -> Option<&'static [&'static str]> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The OnceLock<Option<&'static [&'static str]>> caching is exactly right — including caching the "nothing works" case so a restricted host isn't re-probed on every call. Nice.

use std::sync::OnceLock;
static MAPPING: OnceLock<Option<&'static [&'static str]>> = OnceLock::new();
*MAPPING.get_or_init(|| {
UNSHARE_MAPPING_CANDIDATES
.iter()
.copied()
.find(|args| unshare_probe(args))
})
}

/// Check whether `unshare --user` actually works on this system.
/// On some CI environments (e.g. GitHub Actions), the binary exists but
/// user namespaces are restricted, causing silent failures.
Expand All @@ -292,13 +367,7 @@ fn unshare_user_namespace_works() -> bool {
if !command_exists("unshare") {
return false;
}
std::process::Command::new("unshare")
.args(["--user", "--map-root-user", "true"])
.stdin(std::process::Stdio::null())
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.status()
.is_ok_and(|status| status.success())
working_unshare_mapping().is_some()
})
}

Expand Down Expand Up @@ -359,6 +428,52 @@ mod tests {
assert_eq!(request.allowed_mounts, vec!["tmp"]);
}

#[test]
fn mapping_candidates_prefer_plain_root_mapping() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Once the candidates carry the full launcher flag set, please extend this test to assert each candidate contains the namespace flags (--mount, --ipc, --pid, --uts, --fork) in addition to the mapping flags and ordering — so a future edit can't silently shrink the probe shape back to mapping-only. (I prototyped this: pure string assertions, runs fine on macOS.)

assert!(!super::UNSHARE_MAPPING_CANDIDATES.is_empty());
for candidate in super::UNSHARE_MAPPING_CANDIDATES {
// Mapping flags.
assert!(candidate.contains(&"--user"));
assert!(candidate.contains(&"--map-root-user"));
// Namespace flags the real launcher appends — the probe must
// exercise the full invocation shape, not just mapping flags.
assert!(candidate.contains(&"--mount"));
assert!(candidate.contains(&"--ipc"));
assert!(candidate.contains(&"--pid"));
assert!(candidate.contains(&"--uts"));
assert!(candidate.contains(&"--fork"));
}
// The plain form must be tried first; `--map-auto` is only a fallback
// for kernels/containers that block unprivileged uid_map writes.
assert_eq!(
super::UNSHARE_MAPPING_CANDIDATES[0],
&[
"--user",
"--map-root-user",
"--mount",
"--ipc",
"--pid",
"--uts",
"--fork",
]
);
// The second candidate inserts `--map-auto` in the position util-linux
// expects (after `--map-root-user`, before the namespace flags).
assert_eq!(
super::UNSHARE_MAPPING_CANDIDATES[1],
&[
"--user",
"--map-root-user",
"--map-auto",
"--mount",
"--ipc",
"--pid",
"--uts",
"--fork",
]
);
}

#[test]
fn builds_linux_launcher_with_network_flag_when_requested() {
let config = SandboxConfig::default();
Expand Down
Loading