Skip to content

refactor(suidhelper): move jailer exec mechanics behind a bon builder#66

Merged
markovejnovic merged 1 commit into
mainfrom
refactor/cleaner-cstr
Jul 9, 2026
Merged

refactor(suidhelper): move jailer exec mechanics behind a bon builder#66
markovejnovic merged 1 commit into
mainfrom
refactor/cleaner-cstr

Conversation

@markovejnovic

Copy link
Copy Markdown
Contributor

What

Splits the jailer subcommand along the tools=glue / util=laws line:

  • src/util/jailer.rs (new) — the validated newtypes (VmId, CgroupSetting, JailSock), validate_id_number, their refusal-contract docs, and the become-root/execve mechanics now live here as a Jailer struct built with bon::Builder. Trusted-config fields (jailer, firecracker, jail_base, parent_cgroup) and caller-validated fields (vm, uid, gid, cgroups, api_sock) are documented as distinct trust classes; invoke() consumes the struct, enforces the cgroup cap, builds argv, raises privilege, closes inherited fds, and execves (Ok arm is Infallible).
  • src/util/path.rs (new)ToCStringExt extension trait (to_cstring() on Path and str) replaces the ad-hoc cstr_path/cstr_str/cstr_bytes helpers. impl TryFrom<&Path> for CString would be the natural spelling but is orphan-rule-forbidden (both types are std's).
  • src/tools/jailer.rs — shrinks to the clap surface + trusted-config resolution; re-exports the validators so tests/tools/jailer.rs and main.rs are untouched.

Why

Same shape as the rest of the crate's typed-proof style: named builder setters instead of a 9-positional-arg build_argv, compile-time-required fields via bon's typestate, and the security core (refusal contracts + execve) in one audited util module instead of interleaved with CLI glue.

Adds bon = 3.9 as a dependency.

Testing

  • cargo fmt --check, cargo clippy --all-targets (0 warnings), cargo nextest run — 121/121 pass
  • cargo check --all-targets --features insecure_test_seams clean
  • No behavior change intended: argv layout, refusal contracts, empty-envp execve, and fd hygiene are byte-identical; property suite in tests/tools/jailer.rs unchanged and green

Split the jailer subcommand along the tools=glue/util=laws line:
the validated newtypes (VmId, CgroupSetting, JailSock), their refusal
contracts, and the become-root/execve mechanics move to util::jailer
as a bon-built Jailer struct; tools::jailer keeps only the clap
surface, trusted-config resolution, and re-exports. Ad-hoc cstr_*
helpers become a ToCStringExt extension trait in util::path
(TryFrom<&Path> for CString is orphan-rule-forbidden).
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.05556% with 10 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
native/suidhelper/src/util/jailer.rs 91.93% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

Test Results

  4 files  ±0  193 suites  ±0   2m 16s ⏱️ - 1m 6s
557 tests ±0  557 ✅ ±0    0 💤 ±0  0 ❌ ±0 
942 runs  ±0  557 ✅ ±0  385 💤 ±0  0 ❌ ±0 

Results for commit f3d0aa6. ± Comparison against base commit 252a05f.

@markovejnovic markovejnovic merged commit a00cb6e into main Jul 9, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant