Skip to content

feat(tests): add end-to-end integration tests for sandbox execution strategies#1213

Open
mvanhorn wants to merge 4 commits into
always-further:mainfrom
mvanhorn:feat/1211-integration-tests-execution-strategy
Open

feat(tests): add end-to-end integration tests for sandbox execution strategies#1213
mvanhorn wants to merge 4 commits into
always-further:mainfrom
mvanhorn:feat/1211-integration-tests-execution-strategy

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Summary

  • Add execution_strategy_run.rs: end-to-end integration tests that spawn the nono binary with inline hermetic profiles and verify that the Direct and Supervised sandbox execution strategies enforce filesystem grant boundaries. Tests cover the positive case (allowed path exits zero), the negative case (denied path returns non-zero with no content leak), and a platform-cfg-gated Linux variant for the Supervised strategy.
  • Add rollback_run.rs: integration tests that exercise the baseline-snapshot → write → rollback flow using nono run --rollback, nono rollback list, and nono rollback cleanup --dry-run as subprocesses. Rollback state is isolated to a temp directory via --rollback-dest and XDG_STATE_HOME.

Both files follow the env!("CARGO_BIN_EXE_nono") + tempfile pattern used by the existing deny_overlap_run.rs and config_flag.rs tests. No extends dependencies — profiles are inline JSON so tests are hermetic.

Why this matters

Closes #1211. The issue identified near-zero integration coverage of execution_runtime.rs, supervised_runtime.rs, and rollback_runtime.rs — the three modules where the two recent regressions (af_unix deadlock and BPF jt-offset bypass) would have been caught by tests like these. The new tests give the sandbox enforcement path a direct exercise path without requiring a full CI environment: non-integration assertions (positive control, rollback list/cleanup) run everywhere; the Linux-specific sandbox denial tests are gated #[cfg(target_os = "linux")].

Testing

Tests compile and pass cargo test --no-run on macOS (Rust 1.95). The Linux-gated sandbox denial tests (direct_strategy_denies_path_outside_grant, supervised_strategy_denies_path_outside_grant, filesystem_deny_boundary_produces_diagnostic_footer) require a Linux host with Landlock-capable kernel to execute. The macOS-gated Seatbelt test and the platform-neutral tests (allowed_path_exits_zero, rollback cleanup dry-run) run on any OS.

CI: tests compile; full sandbox enforcement tests need Linux.

Fixes #1211

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +565
Lines removed -0
Total changed 565
Classification Large (> 300 lines)

Affected crates

  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds end-to-end integration tests for the core sandbox execution strategies and the rollback snapshot feature. The feedback points out that two of the rollback tests are incomplete: they set up the scenarios but stop short of actually executing the restore and dry-run restore commands to assert on the expected outcomes. Complete implementations are provided to fully exercise and verify the rollback functionality.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/nono-cli/tests/rollback_run.rs Outdated
Comment thread crates/nono-cli/tests/rollback_run.rs Outdated
@SequeI

SequeI commented Jun 19, 2026

Copy link
Copy Markdown
Member

This is amazing and very much needed! Thanks for this PR so soon after the issue opened, always good to get more resiliency into the project. I basically agree with the gemini comments, + if we could use less of the ASCII banners for test grouping and be more concise; would be amazing.

After addressed, I shall do a full review :) + DCO

…trategies (always-further#1211)

Add execution_strategy_run.rs and rollback_run.rs integration tests that spawn
the real nono binary as a subprocess with hermetic inline profiles, exercising
the Direct and Supervised sandbox strategies and the rollback snapshot/restore
flow end-to-end. All tests are gated by cfg(target_os) where appropriate.

Signed-off-by: Matt Van Horn <mvanhorn@gmail.com>
@mvanhorn mvanhorn force-pushed the feat/1211-integration-tests-execution-strategy branch from 79734cb to 41cacb6 Compare June 19, 2026 15:31
The macOS Direct-strategy denial test read /etc/passwd, but Seatbelt grants
the sandboxed process a set of system read paths so it can exec and load dyld,
and /etc/passwd is a world-readable system file inside that allowed set. The
read therefore succeeded and the test failed. Read a secret file created in the
temp root (a sibling of the granted workspace, neither granted nor a system
path) so the assertion exercises a genuine out-of-grant denial, mirroring the
Linux variant's intent without depending on a system path.

Signed-off-by: mvanhorn <mvanhorn@gmail.com>
@mvanhorn

Copy link
Copy Markdown
Contributor Author

The macOS test failure was a test-premise bug, not a sandbox regression. direct_strategy_denies_path_outside_grant_macos read /etc/passwd, but Seatbelt grants the sandboxed process a set of system read paths (so it can exec and load dyld), and /etc/passwd is a world-readable system file inside that set, so the read succeeds and the assertion fails. Reproduced locally: the capability footer shows the workspace grant plus "34 system/group paths", and /etc/passwd printed.

Pushed 2636505e: the test now reads a secret file created in the temp root (a sibling of the granted workspace, neither granted nor a system path), so it exercises a genuine out-of-grant denial and mirrors the Linux variant's intent without depending on a system path.

@lukehinds

Copy link
Copy Markdown
Contributor

from what I know /etc/passwd is just a relic not even used anymore by the later versions of mac, I think its just a placeholder to stop other systems tripping up over its absence. PR looks good! will review shortly, thanks for contributing.

@lukehinds lukehinds requested review from SequeI and lukehinds June 21, 2026 16:56
Signed-off-by: Aleksy Siek <aleksy@alwaysfurther.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

integration: add end-to-end execution tests for core sandbox features

3 participants