mirror of
https://github.com/instructkr/claw-code.git
synced 2026-08-25 00:00:38 +08:00
fix(sandbox): probe the full launcher shape, not just the mapping
The startup probe validated only the mapping flags against the trivial program `true`, but the real launcher always adds --mount --ipc --pid --uts --fork. On environments where the user namespace is created but mount propagation inside it is restricted (e.g. AppArmor-restricted CI runners), the fallback mapping passed the probe and the sandbox activated, yet every sandboxed command died with "cannot change root filesystem propagation: Permission denied", silently returning empty tool output and breaking the mock parity suite. The candidates now define the complete static launcher shape (mapping flags + namespace flags), so probe success implies launch success; the launcher reuses the candidate instead of re-appending the namespace flags, keeping probe and launch as one source of truth. The order-guarding test asserts the namespace flags are present in every candidate. Co-authored-by: linkst <2024023709@m.scnu.edu.cn>
This commit is contained in:
@@ -225,13 +225,9 @@ pub fn build_linux_sandbox_command(
|
|||||||
.iter()
|
.iter()
|
||||||
.map(|arg| arg.to_string())
|
.map(|arg| arg.to_string())
|
||||||
.collect();
|
.collect();
|
||||||
args.extend([
|
// The candidates already carry the namespace flags, so the probe and the
|
||||||
"--mount".to_string(),
|
// launcher share a single argument shape; only the opt-in `--net` is
|
||||||
"--ipc".to_string(),
|
// added here.
|
||||||
"--pid".to_string(),
|
|
||||||
"--uts".to_string(),
|
|
||||||
"--fork".to_string(),
|
|
||||||
]);
|
|
||||||
if status.network_active {
|
if status.network_active {
|
||||||
args.push("--net".to_string());
|
args.push("--net".to_string());
|
||||||
}
|
}
|
||||||
@@ -287,18 +283,51 @@ fn command_exists(command: &str) -> bool {
|
|||||||
|
|
||||||
/// Candidate `unshare` user-namespace mapping options, in preference order.
|
/// Candidate `unshare` user-namespace mapping options, in preference order.
|
||||||
///
|
///
|
||||||
/// Most systems accept `--map-root-user` alone. On kernels or containers that
|
/// Most systems accept `--map-root-user` alone. Some hardened containers and
|
||||||
/// block unprivileged writes to `/proc/self/uid_map` (e.g. GitHub Actions,
|
/// seccomp profiles block unprivileged writes to `/proc/self/uid_map`; there,
|
||||||
/// restricted AppArmor profiles), util-linux instead delegates to the setuid
|
/// util-linux delegates to the setuid `newuidmap`/`newgidmap` helpers when
|
||||||
/// `newuidmap`/`newgidmap` helpers when `--map-auto` is also present.
|
/// `--map-auto` is also present.
|
||||||
///
|
///
|
||||||
/// That fallback therefore depends on the setuid helpers (the `uidmap`
|
/// That fallback therefore depends on the setuid helpers (the `uidmap`
|
||||||
/// package on Debian/Ubuntu) and on the current user having a range in
|
/// package on Debian/Ubuntu) and on the current user having a range in
|
||||||
/// `/etc/subuid` and `/etc/subgid`. When either is missing, `--map-auto`
|
/// `/etc/subuid` and `/etc/subgid`. When either is missing, `--map-auto`
|
||||||
/// fails and the startup probe rejects the candidate, keeping the plain form.
|
/// 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]] = &[
|
const UNSHARE_MAPPING_CANDIDATES: &[&[&str]] = &[
|
||||||
&["--user", "--map-root-user"],
|
&[
|
||||||
&["--user", "--map-root-user", "--map-auto"],
|
"--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.
|
/// Probe a candidate `unshare` mapping invocation with a trivial program.
|
||||||
@@ -403,14 +432,45 @@ mod tests {
|
|||||||
fn mapping_candidates_prefer_plain_root_mapping() {
|
fn mapping_candidates_prefer_plain_root_mapping() {
|
||||||
assert!(!super::UNSHARE_MAPPING_CANDIDATES.is_empty());
|
assert!(!super::UNSHARE_MAPPING_CANDIDATES.is_empty());
|
||||||
for candidate in super::UNSHARE_MAPPING_CANDIDATES {
|
for candidate in super::UNSHARE_MAPPING_CANDIDATES {
|
||||||
|
// Mapping flags.
|
||||||
assert!(candidate.contains(&"--user"));
|
assert!(candidate.contains(&"--user"));
|
||||||
assert!(candidate.contains(&"--map-root-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
|
// The plain form must be tried first; `--map-auto` is only a fallback
|
||||||
// for kernels/containers that block unprivileged uid_map writes.
|
// for kernels/containers that block unprivileged uid_map writes.
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
super::UNSHARE_MAPPING_CANDIDATES[0],
|
super::UNSHARE_MAPPING_CANDIDATES[0],
|
||||||
&["--user", "--map-root-user"]
|
&[
|
||||||
|
"--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",
|
||||||
|
]
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user