Skip to content

Commit 2755c1e

Browse files
committed
fix: preserve exit markers during Landlock fd hygiene (#167)
1 parent 51c935e commit 2755c1e

2 files changed

Lines changed: 116 additions & 1 deletion

File tree

crates/aft/src/cli/sandbox_launch/landlock_backend.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,12 @@ fn yama_value_same_uid_exposed(value: Option<String>) -> bool {
158158
}
159159

160160
fn close_inherited_fds() -> Result<(), String> {
161-
let result = unsafe { libc::syscall(libc::SYS_close_range, 3_u32, u32::MAX, 0_u32) };
161+
// Keep stdio (0-2) and the daemon-controlled markers (3-4) alive. The
162+
// parent deliberately remaps the exit and failure markers to 3 and 4 in
163+
// apply_marker_fd_allowlist; descriptors >= 5 already carry FD_CLOEXEC
164+
// there, so closing from 5 is defense-in-depth for a non-CLOEXEC leak,
165+
// not the primary descriptor-hygiene mechanism.
166+
let result = unsafe { libc::syscall(libc::SYS_close_range, 5_u32, u32::MAX, 0_u32) };
162167
if result == 0 {
163168
Ok(())
164169
} else {

crates/aft/tests/sandbox_launch_probe.rs

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#![cfg(unix)]
22

3+
#[cfg(target_os = "linux")]
4+
use aft::bash_background::persistence::resolve_task;
35
use aft::sandbox_profile::SandboxProfile;
46
use portable_pty::{CommandBuilder, PtySize};
57
use std::fs;
@@ -11,6 +13,13 @@ use std::process::{Command, Output};
1113
use std::time::{Duration, Instant};
1214
use tempfile::{NamedTempFile, TempDir};
1315

16+
#[cfg(target_os = "linux")]
17+
#[path = "helpers/mod.rs"]
18+
mod test_helpers;
19+
20+
#[cfg(target_os = "linux")]
21+
use test_helpers::{user_config, AftProcess};
22+
1423
const AFT_BIN: &str = env!("CARGO_BIN_EXE_aft");
1524

1625
struct ProbeFixture {
@@ -260,6 +269,107 @@ fn connect_unix_socket(fixture: &mut ProbeFixture, path: &Path) -> Output {
260269
fixture.launch_bash(&script)
261270
}
262271

272+
#[cfg(target_os = "linux")]
273+
#[test]
274+
fn native_background_exit_marker_survives_landlock_launcher() {
275+
skip_if_landlock_absent!();
276+
let fixture = tempfile::tempdir().expect("create marker fixture");
277+
let project = fixture.path().join("project");
278+
let storage = fixture.path().join("artifacts");
279+
let home = fixture.path().join("home");
280+
for directory in [&project, &storage, &home] {
281+
fs::create_dir_all(directory).expect("create marker fixture directory");
282+
}
283+
284+
let mut aft = AftProcess::spawn_with_env(&[("HOME", std::ffi::OsStr::new(&home))]);
285+
let configured = aft.send(
286+
&serde_json::json!({
287+
"id": "cfg-marker",
288+
"command": "configure",
289+
"harness": "opencode",
290+
"project_root": project.clone(),
291+
"storage_dir": storage.clone(),
292+
"bash_permissions": true,
293+
"config": user_config(serde_json::json!({
294+
"bash": { "background": true, "rewrite": true },
295+
"sandbox": { "enabled": true },
296+
})),
297+
})
298+
.to_string(),
299+
);
300+
assert_eq!(
301+
configured["success"], true,
302+
"configure failed: {configured:?}"
303+
);
304+
305+
let session = "landlock-marker-session";
306+
// Use the production background request so SpawnPlan::Launcher wires the
307+
// exit and failure markers through apply_marker_fd_allowlist onto fds 3/4
308+
// before the real sandbox-launch process applies Landlock.
309+
let launch = aft.send(
310+
&serde_json::json!({
311+
"id": "launch-marker",
312+
"method": "bash",
313+
"session_id": session,
314+
"params": {
315+
"command": "exit 37",
316+
"background": true,
317+
"permissions_requested": true,
318+
"compressed": false,
319+
},
320+
})
321+
.to_string(),
322+
);
323+
assert_eq!(
324+
launch["success"], true,
325+
"background launch failed: {launch:?}"
326+
);
327+
let task_id = launch["task_id"].as_str().expect("background task id");
328+
329+
let started = Instant::now();
330+
let terminal = loop {
331+
let status = aft.send(
332+
&serde_json::json!({
333+
"id": "status-marker",
334+
"method": "bash_status",
335+
"session_id": session,
336+
"params": { "task_id": task_id },
337+
})
338+
.to_string(),
339+
);
340+
assert_eq!(status["success"], true, "status failed: {status:?}");
341+
if matches!(
342+
status["status"].as_str(),
343+
Some("completed" | "failed" | "killed" | "timed_out")
344+
) {
345+
break status;
346+
}
347+
assert!(
348+
started.elapsed() < Duration::from_secs(10),
349+
"sandboxed task never recorded an exit marker: {status:?}"
350+
);
351+
std::thread::sleep(Duration::from_millis(50));
352+
};
353+
assert_eq!(
354+
terminal["status"], "failed",
355+
"unexpected terminal state: {terminal:?}"
356+
);
357+
assert_eq!(
358+
terminal["exit_code"], 37,
359+
"exit marker was not consumed: {terminal:?}"
360+
);
361+
362+
let paths = resolve_task(&storage, session, task_id)
363+
.expect("resolve background task layout")
364+
.paths;
365+
assert_eq!(
366+
fs::read_to_string(&paths.exit).expect("read exit marker"),
367+
"37",
368+
"production marker wire did not record the command exit code"
369+
);
370+
assert!(aft.shutdown().success());
371+
}
372+
263373
#[test]
264374
fn launcher_support_reports_first_party_backend() {
265375
skip_if_landlock_absent!();

0 commit comments

Comments
 (0)