fix(jailer): replace hardcoded FD limits with dynamic closure strategies

[lilongen]

Summary

• Replace hardcoded FD upper bounds (1024 on Linux, 4096 on macOS) with dynamic limits from getrlimit(RLIMIT_NOFILE), fixing FD leakage on systems with raised ulimit -n
• Add /proc/self/fd enumeration via raw getdents64 syscall as an efficient middle strategy on Linux (zero heap allocation, async-signal-safe)
• Restructure close_fds_from() into a 3-strategy cascade: close_range → /proc/self/fd → brute-force with dynamic limit

Problem

The FD cleanup in the pre_exec hook used hardcoded upper bounds:

• Linux: for fd in first_fd..1024
• macOS: for fd in first_fd..4096

On production systems with ulimit -n 65536 or higher, any FDs above these limits leaked into the jailed process, potentially exposing credentials, database connections, or network sockets inherited from the parent.

Solution

Linux — 3-strategy cascade

Strategy	Condition	Complexity	Description
1. close_range	Linux 5.9+	O(1)	Kernel closes all FDs in range (already existed)
2. /proc/self/fd	/proc mounted	O(open FDs)	Enumerate via raw getdents64, close only open FDs
3. Brute-force	Fallback	O(ulimit)	Close first_fd..getrlimit(RLIMIT_NOFILE)

macOS

Brute-force close with dynamic limit from getrlimit(RLIMIT_NOFILE) (replaces hardcoded 4096).

Key constraints

All operations are async-signal-safe (required for pre_exec context):

• No heap allocation — stack buffer for getdents64, byte literals for paths
• No io::Error, String, Vec, CString — raw syscalls only
• getrlimit is POSIX async-signal-safe
• RLIM_INFINITY handled by capping to 1,048,576 (Linux nr_open default)

Test plan

• test_close_high_numbered_fd — creates FD 2000 (above old limits), verifies closure
• test_get_max_fd_returns_positive — validates dynamic limit query
• test_parse_fd_from_name (Linux) — covers valid FDs, ./.., empty, i32 overflow
• All 4 existing tests continue to pass
• cargo clippy -D warnings — zero warnings on both platforms
• cargo fmt --check — clean
• Tested on macOS ARM64 (6/6 passed) and Linux ARM64 via Lima (7/7 passed)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Updates the jailer’s pre-exec FD cleanup to avoid leaking inherited file descriptors on hosts with high ulimit -n, while maintaining the async-signal-safe constraint required by CommandExt::pre_exec.

Changes:

• Replace hardcoded FD upper bounds with a dynamic limit from getrlimit(RLIMIT_NOFILE).
• Add a Linux-only middle strategy that enumerates /proc/self/fd via raw getdents64 to close only actually-open FDs.
• Add tests covering high-numbered FD closure and Linux FD-name parsing.

---

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

[Copilot]

close_fds_via_proc currently returns true even if getdents64 fails (e.g., syscall returns -1 for EINTR/EBADF). That would skip the brute-force fallback and can leave FDs open. Consider treating nread < 0 (and also malformed d_reclen conditions) as a failure: close dir_fd and return false so brute_force_close_fds() runs.

[Copilot]

get_max_fd() uses if rlim.rlim_cur < i32::MAX as u64 which incorrectly treats an exact i32::MAX soft limit as “too large” and caps it to 1,048,576. This looks like an off-by-one comparison; use <= if the intent is “fits in i32”.

[Copilot]

If getrlimit(RLIMIT_NOFILE) fails (or returns 0), get_max_fd() falls back to 1024, which can reintroduce the original FD-leak problem on hosts with a higher limit (since brute-force will stop at 1024). Consider a fallback that still errs on the side of closing more FDs (e.g., a higher cap like the existing 1,048,576) to preserve the security invariant even when getrlimit is blocked or fails.

Suggested change

	// getrlimit failed or returned 0; safe default per POSIX OPEN_MAX
	1024
	// getrlimit failed or returned 0; use the same conservative cap to
	// ensure we still attempt to close a wide range of FDs.
	1_048_576

[DorianZheng]

Same getdents64 bug as #407: in fd.rs:71-118, nread < 0 is treated as success so close_fds_via_proc() returns true on failure and the getrlimit-based fallback never kicks in. Fix is to return false when getdents64 fails. Would also be good to add a test that exercises this error path.

[lilongen]

Addressed all review feedback in commit f90c941:

@copilot + @DorianZheng — close_fds_via_proc returns false on getdents64 failure

• Split nread <= 0 into two checks: nread < 0 (error → return false, triggers brute-force fallback) and nread == 0 (normal EOF)

@copilot — Off-by-one in get_max_fd()

• Changed < i32::MAX to <= i32::MAX so exact i32::MAX soft limit is handled correctly

@copilot — Fallback too low when getrlimit fails

• Raised from 1024 to 1_048_576 to preserve security invariant

@DorianZheng — Added test for fallback path

• New test_brute_force_fallback_closes_fds exercises brute_force_close_fds() directly

All tests pass, clippy clean.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

---

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

[Copilot]

child_close_high_numbered_fd returns 0 (passing the test) when setrlimit(RLIMIT_NOFILE, 4096) fails, which can make the test vacuously succeed on systems where the hard limit is low and the regression would otherwise go untested. Consider deriving a high_fd from the current hard limit (e.g., raise rlim_cur up to rlim_max, then pick high_fd = min(2000, rlim_cur - 1) and only skip if rlim_cur can’t exceed the old hardcoded bound). This keeps the test meaningful across more CI environments.

Suggested change

	// Raise the soft FD limit so we can dup2 to a high FD number.
	// Some systems default to ulimit -n 1024, which would prevent dup2 to FD 2000.
	let new_limit = libc::rlimit {
	rlim_cur: 4096,
	rlim_max: 4096,
	};
	// SAFETY: setrlimit is async-signal-safe. Raising soft limit within hard limit is allowed.
	if unsafe { libc::setrlimit(libc::RLIMIT_NOFILE, &new_limit) } != 0 {
	// Can't raise limit (hard limit too low) — skip test gracefully
	return 0;
	}
	// dup2 stdout to a high FD number that exceeds the old hardcoded limits.
	// Use 2000 on all platforms — above old Linux limit (1024) and reasonable.
	let high_fd: i32 = 2000;
	let result = unsafe { libc::dup2(STDOUT_FD, high_fd) };
	if result != high_fd {
	// dup2 failed — unexpected after raising ulimit
	return 1;
	}
	// Verify the high FD is open
	if unsafe { libc::fcntl(high_fd, libc::F_GETFD) } < 0 {
	return 2;
	}
	// Close all inherited FDs
	if close_inherited_fds_raw().is_err() {
	return 3;
	}
	// The high FD should now be closed
	if unsafe { libc::fcntl(high_fd, libc::F_GETFD) } != -1 {
	return 4;
	const OLD_HARDCODED_LOWER_BOUND: i32 = 1024;
	const TARGET_HIGH_FD: i32 = 2000;
	let mut rlim = libc::rlimit {
	rlim_cur: 0,
	rlim_max: 0,
	};
	// SAFETY: getrlimit is async-signal-safe and rlim points to valid stack storage.
	if unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, &mut rlim) } != 0 {
	return 1;
	}
	// Raise the soft limit as far as the current hard limit allows so we can
	// test a high-numbered FD even on systems where 4096 is not permitted.
	if rlim.rlim_cur < rlim.rlim_max {
	let raised_limit = libc::rlimit {
	rlim_cur: rlim.rlim_max,
	rlim_max: rlim.rlim_max,
	};
	// SAFETY: setrlimit is async-signal-safe. Raising rlim_cur up to rlim_max is valid.
	if unsafe { libc::setrlimit(libc::RLIMIT_NOFILE, &raised_limit) } == 0 {
	rlim = raised_limit;
	} else {
	// Re-read the effective limit in case another policy or prior state changed it.
	// SAFETY: getrlimit is async-signal-safe and rlim points to valid stack storage.
	if unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, &mut rlim) } != 0 {
	return 2;
	}
	}
	}
	let effective_limit = if rlim.rlim_cur > i32::MAX as libc::rlim_t {
	i32::MAX
	} else {
	rlim.rlim_cur as i32
	};
	if effective_limit <= OLD_HARDCODED_LOWER_BOUND {
	// Can't exercise the regression meaningfully on this system.
	return 0;
	}
	let high_fd = std::cmp::min(TARGET_HIGH_FD, effective_limit - 1);
	// dup2 stdout to a high FD number that exceeds the old hardcoded limits.
	let result = unsafe { libc::dup2(STDOUT_FD, high_fd) };
	if result != high_fd {
	return 3;
	}
	// Verify the high FD is open
	if unsafe { libc::fcntl(high_fd, libc::F_GETFD) } < 0 {
	return 4;
	}
	// Close all inherited FDs
	if close_inherited_fds_raw().is_err() {
	return 5;
	}
	// The high FD should now be closed
	if unsafe { libc::fcntl(high_fd, libc::F_GETFD) } != -1 {
	return 6;

[Copilot]

In close_fds_via_proc, if d_reclen is 0 or would run past nread, the code breaks out of the inner loop but still leaves success as true and continues the outer loop. That can cause the function to return true (skipping the brute-force fallback) even though directory parsing was incomplete, potentially leaving some FDs unclosed. Consider treating this as a hard failure: set success = false and break out of the outer read loop when the record looks malformed/inconsistent, so callers reliably fall back to the brute-force strategy.

[Copilot]

getdents64 can return -1 with EINTR; currently that is treated as a failure and triggers the brute-force fallback. That keeps correctness, but can cause a large (and avoidable) performance hit in pre_exec if an interrupt occurs. Consider checking errno and retrying the getdents64 call on EINTR, only falling back on other errors.

Suggested change

	// getdents64 failed (e.g., EINTR, EBADF); signal failure so
	// the brute-force fallback runs instead.
	// SAFETY: errno is thread-local and may be read immediately after
	// the failing syscall to distinguish EINTR from real failures.
	let errno = unsafe { *libc::__errno_location() };
	if errno == libc::EINTR {
	continue;
	}
	// getdents64 failed for a non-retryable reason (e.g., EBADF);
	// signal failure so the brute-force fallback runs instead.