fix(sandbox): the devbox /data write-deny makes bubblewrap mandatory
A devbox-based profile on a host with /data warned and fell back to Landlock when bubblewrap was missing or failed to exec, and Landlock grants the workspace under /data. requires_data_write_deny (devbox inheritance and an existing /data; a stat error other than NotFound counts as present) now joins requires_read_deny in the fail-closed startup decision, and the /data ro-bind is added only under that predicate, so a missing /data no longer makes bwrap fail to exec. Ported from upstream (grok-build sandbox/lib.rs requires_data_write_deny). Claude-Session: https://claude.ai/code/session_01RUqV8HVGXcAETyYbZ2tPQ4
This commit is contained in:
1 parent
edce333620
commit
09867915ef
2 files changed
+80
-11
No files matched your search
@@ -333,6 +333,27 @@ fn is_devbox_based(profile: &ProfileName, config: &SandboxConfig) -> bool {
|
|||||||
_ => false,
|
_ => false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
/// Whether an existing `/data` needs the devbox bwrap read-only bind; a filesystem error other than NotFound conservatively requires it.
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
pub fn requires_data_write_deny(profile: &ProfileName, workspace: &Path) -> bool {
|
||||||
|
requires_data_write_deny_for(
|
||||||
|
profile,
|
||||||
|
&profiles::load_sandbox_config(workspace),
|
||||||
|
data_path_requires_bind(Path::new("/data")),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
fn requires_data_write_deny_for(
|
||||||
|
profile: &ProfileName,
|
||||||
|
config: &SandboxConfig,
|
||||||
|
is_data_present: bool,
|
||||||
|
) -> bool {
|
||||||
|
is_devbox_based(profile, config) && is_data_present
|
||||||
|
}
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
fn data_path_requires_bind(path: &Path) -> bool {
|
||||||
|
path.try_exists().unwrap_or(true)
|
||||||
|
}
|
||||||
/// Whether kernel read-deny enforcement is required. The single source of truth
|
/// Whether kernel read-deny enforcement is required. The single source of truth
|
||||||
/// for this classification so callers (e.g. the shell's fail-closed startup path)
|
/// for this classification so callers (e.g. the shell's fail-closed startup path)
|
||||||
/// cannot drift and silently fail open.
|
/// cannot drift and silently fail open.
|
||||||
@@ -399,7 +420,11 @@ struct BwrapDenyPlan {
|
|||||||
#[cfg(all(feature = "enforce", target_os = "linux"))]
|
#[cfg(all(feature = "enforce", target_os = "linux"))]
|
||||||
fn bwrap_deny_plan(profile: &ProfileName, workspace: &Path) -> Option<BwrapDenyPlan> {
|
fn bwrap_deny_plan(profile: &ProfileName, workspace: &Path) -> Option<BwrapDenyPlan> {
|
||||||
let config = profiles::load_sandbox_config(workspace);
|
let config = profiles::load_sandbox_config(workspace);
|
||||||
let deny_write: Vec<String> = if is_devbox_based(profile, &config) {
|
let deny_write: Vec<String> = if requires_data_write_deny_for(
|
||||||
|
profile,
|
||||||
|
&config,
|
||||||
|
data_path_requires_bind(Path::new("/data")),
|
||||||
|
) {
|
||||||
vec!["/data".to_string()]
|
vec!["/data".to_string()]
|
||||||
} else {
|
} else {
|
||||||
Vec::new()
|
Vec::new()
|
||||||
@@ -441,7 +466,11 @@ fn bwrap_deny_plan(profile: &ProfileName, workspace: &Path) -> Option<BwrapDenyP
|
|||||||
#[cfg(all(not(feature = "enforce"), target_os = "linux"))]
|
#[cfg(all(not(feature = "enforce"), target_os = "linux"))]
|
||||||
fn bwrap_deny_plan(profile: &ProfileName, workspace: &Path) -> Option<BwrapDenyPlan> {
|
fn bwrap_deny_plan(profile: &ProfileName, workspace: &Path) -> Option<BwrapDenyPlan> {
|
||||||
let config = profiles::load_sandbox_config(workspace);
|
let config = profiles::load_sandbox_config(workspace);
|
||||||
let deny_write: Vec<String> = if is_devbox_based(profile, &config) {
|
let deny_write: Vec<String> = if requires_data_write_deny_for(
|
||||||
|
profile,
|
||||||
|
&config,
|
||||||
|
data_path_requires_bind(Path::new("/data")),
|
||||||
|
) {
|
||||||
vec!["/data".to_string()]
|
vec!["/data".to_string()]
|
||||||
} else {
|
} else {
|
||||||
Vec::new()
|
Vec::new()
|
||||||
@@ -612,6 +641,43 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
#[test]
|
#[test]
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
fn data_write_deny_requires_devbox_and_a_present_data_dir() {
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
std::fs::create_dir_all(tmp.path().join(".kigi")).unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
tmp.path().join(".kigi/sandbox.toml"),
|
||||||
|
"[profiles.devext]\nextends = \"devbox\"\n\n[profiles.plain]\nextends = \"workspace\"\n",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
let config = profiles::load_sandbox_config(tmp.path());
|
||||||
|
assert!(requires_data_write_deny_for(
|
||||||
|
&ProfileName::Devbox,
|
||||||
|
&config,
|
||||||
|
true
|
||||||
|
));
|
||||||
|
assert!(!requires_data_write_deny_for(
|
||||||
|
&ProfileName::Devbox,
|
||||||
|
&config,
|
||||||
|
false
|
||||||
|
));
|
||||||
|
assert!(requires_data_write_deny_for(
|
||||||
|
&ProfileName::Custom("devext".into()),
|
||||||
|
&config,
|
||||||
|
true
|
||||||
|
));
|
||||||
|
assert!(!requires_data_write_deny_for(
|
||||||
|
&ProfileName::Custom("plain".into()),
|
||||||
|
&config,
|
||||||
|
true
|
||||||
|
));
|
||||||
|
assert!(!requires_data_write_deny_for(
|
||||||
|
&ProfileName::Workspace,
|
||||||
|
&config,
|
||||||
|
true
|
||||||
|
));
|
||||||
|
}
|
||||||
|
#[test]
|
||||||
fn configured_profile_is_recorded() {
|
fn configured_profile_is_recorded() {
|
||||||
set_configured_profile("read-only");
|
set_configured_profile("read-only");
|
||||||
assert_eq!(configured_profile_name(), Some("read-only"));
|
assert_eq!(configured_profile_name(), Some("read-only"));
|
||||||
@@ -717,10 +783,11 @@ mod tests {
|
|||||||
"devbox-empty",
|
"devbox-empty",
|
||||||
"[profiles.devempty]\nextends = \"devbox\"\n",
|
"[profiles.devempty]\nextends = \"devbox\"\n",
|
||||||
);
|
);
|
||||||
assert!(
|
assert_eq!(
|
||||||
bwrap_reexec_for_profile(&ProfileName::Custom("devempty".to_string()), &ws_empty)
|
bwrap_reexec_for_profile(&ProfileName::Custom("devempty".to_string()), &ws_empty)
|
||||||
.is_some(),
|
.is_some(),
|
||||||
"devbox-extending custom must compose the /data write-deny re-exec"
|
Path::new("/data").exists(),
|
||||||
|
"devbox-extending custom composes the /data write-deny re-exec exactly when /data exists"
|
||||||
);
|
);
|
||||||
let _ = std::fs::remove_dir_all(&ws_empty);
|
let _ = std::fs::remove_dir_all(&ws_empty);
|
||||||
let ws_ws = temp_workspace_with_sandbox_toml(
|
let ws_ws = temp_workspace_with_sandbox_toml(
|
||||||
|
|||||||
@@ -1043,16 +1043,18 @@ pub fn apply_sandbox(
|
|||||||
kigi_tools::implementations::kigi::grep::ripgrep::rg_path();
|
kigi_tools::implementations::kigi::grep::ripgrep::rg_path();
|
||||||
crate::builtin::extract_bundled_files(&crate::util::kigi_home::kigi_home());
|
crate::builtin::extract_bundled_files(&crate::util::kigi_home::kigi_home());
|
||||||
}
|
}
|
||||||
|
// The devbox `/data` write-deny is a bwrap mount; Landlock would grant the workspace under `/data`.
|
||||||
#[cfg(target_os = "linux")]
|
#[cfg(target_os = "linux")]
|
||||||
let requires_read_deny = kigi_sandbox::requires_read_deny(&sandbox_profile, &workspace);
|
let bwrap_required = kigi_sandbox::requires_read_deny(&sandbox_profile, &workspace)
|
||||||
|
|| kigi_sandbox::requires_data_write_deny(&sandbox_profile, &workspace);
|
||||||
#[cfg(target_os = "linux")]
|
#[cfg(target_os = "linux")]
|
||||||
{
|
{
|
||||||
let refuse_unprotected = |detail: &str| {
|
let refuse_unprotected = |detail: &str| {
|
||||||
eprintln!(
|
eprintln!(
|
||||||
"error: this sandbox could not enforce its read-deny set on Linux \
|
"error: this sandbox could not enforce its read-deny set or the devbox \
|
||||||
(bubblewrap missing/unusable, or a deny glob exceeded its expansion \
|
`/data` write-deny on Linux (bubblewrap missing/unusable, or a deny glob \
|
||||||
limit — see any message above). Install bubblewrap with \
|
exceeded its expansion limit — see any message above). Install bubblewrap \
|
||||||
`apt install -y bubblewrap` if needed. Refusing to start with denied \
|
with `apt install -y bubblewrap` if needed. Refusing to start with denied \
|
||||||
paths unprotected.{detail}"
|
paths unprotected.{detail}"
|
||||||
);
|
);
|
||||||
};
|
};
|
||||||
@@ -1060,7 +1062,7 @@ pub fn apply_sandbox(
|
|||||||
Some(mut cmd) => {
|
Some(mut cmd) => {
|
||||||
use std::os::unix::process::CommandExt;
|
use std::os::unix::process::CommandExt;
|
||||||
let err = cmd.exec();
|
let err = cmd.exec();
|
||||||
if requires_read_deny {
|
if bwrap_required {
|
||||||
refuse_unprotected(&format!(" (bwrap exec failed: {err})"));
|
refuse_unprotected(&format!(" (bwrap exec failed: {err})"));
|
||||||
std::process::exit(1);
|
std::process::exit(1);
|
||||||
}
|
}
|
||||||
@@ -1070,7 +1072,7 @@ pub fn apply_sandbox(
|
|||||||
Install bubblewrap: apt install -y bubblewrap"
|
Install bubblewrap: apt install -y bubblewrap"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
None if requires_read_deny && !kigi_sandbox::is_inside_bwrap() => {
|
None if bwrap_required && !kigi_sandbox::is_inside_bwrap() => {
|
||||||
refuse_unprotected("");
|
refuse_unprotected("");
|
||||||
std::process::exit(1);
|
std::process::exit(1);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user