Repository navigation
feat(minimald): run a box's package builds under that box's egress verdict - #2720
norrietaylor merged 33 commits into
Conversation
A spec build and a patched build take an optional classifier leaf. When set, the build's sandbox joins that leaf, so the egress verdict of the box the build runs for decides its connections (NET-155).
A context made for a box carries the box's classifier leaf, and the local orchestrator hands it to every spec build it schedules. The client-side and command-line patched builds pass no leaf (NET-155).
A box's build side-op and its in-box patched build run in the box's classifier leaf, so the box's egress verdict decides them. A build that a deny-all box's rules refuse now names the destination and the box, and each build start is logged with the box, package and leaf.
…s-package-builds-under-that-box-s-egre
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Approved: no findings at Nothing to do. Verdict APPROVE, posted as a comment: the App cannot review its own pull request; the check and the label carry it. Clobber: none seen. |
…s-package-builds-under-that-box-s-egre
A box whose launch recorded a classifier leaf of its own no longer builds in the daemon's cgroup when that leaf is missing. The build is refused, so it never runs outside the box's egress verdict.
There was a problem hiding this comment.
arch-review: CONDITIONS · 5 conditions · the full ruling
The change puts a placed box's package builds in the box's own classifier leaf. That follows §4.1 line 54, and a missing leaf now refuses the build. Five items remain. The refusal text blames the box's rules for in-zone names, which §5.3 line 152 exempts (C1). The start log names the box's leaf when the build runs in the daemon's cgroup (C2). Builds for own_ip and unplaced boxes stay node-plane, and no issue is filed (C3). NET-155 is not in the spec (C4), and no root-lane proof runs a placed build (C5).
Ruled at 3e4b720 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
A deny-all box that asks the daemon to build a package which downloads during its build sees the build fail with the destination and the box named, and a box allowed that destination builds it. This round adds one more case: a box whose launch placed it under its egress rules, and whose placement is now gone, has its build refused. The build does not run with the daemon's open network. Notes from the agent: Revise round 1. box_build_leaf now takes whether the box record says host_ip_enforcement = PerBox. When it does and no leaf directory exists, start_build returns a PermissionDenied error with the text "the build for box did not start: box runs under egress rules, and this host cannot apply them to the build" and logs a warning. That text gives no next step, because I found no command that I could verify restores the placement of a running box. A box never placed in a leaf still builds in the daemon's cgroup, as before. Ran cargo clippy -p minimald --all-targets with no warnings. |
|
/revise Fix arch-review conditions C1, C2, C4 and C5 at 3e4b720 (arch_rulings 510). Change nothing else.
Generated by Claude Code |
…s-package-builds-under-that-box-s-egre
A failure to reach a min.internal name is not a refusal by the box's egress rules. A build with no classifier leaf now logs that it runs in the daemon's own cgroup as node-plane traffic.
A root-lane proof runs a real spec build in a deny leaf and in an allow leaf against one local peer.
A package build that minimald runs for a box with a classifier leaf runs in that leaf. A placed box whose leaf is missing refuses the build.
Spec lint
Advisory. None of this blocks the merge; the review decides. Advisory, and never a merge gate. Linter: |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change now conforms to section 4.1 line 54 of the networking design. A box's package build runs in the box's own classifier leaf on both build paths. A placed box whose leaf is gone gets a refusal, and the build does not run in the daemon's cgroup. The refusal text does not apply to min.internal names, as the resolver carve-out requires. NET-155 in spec 18 states what the code does. No conditions are open.
Ruled at a400c75 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 2 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
A deny-all box that asks the daemon to build a package that downloads during its build sees the build fail with the destination and the box named. A box that is allowed that destination builds the package. A failure to reach a min.internal name is not reported as a refusal by the box's rules. The new root-lane proof package_build_in_a_box_leaf_follows_its_verdict_over_a_loaded_table shows a real spec build that cannot connect from a deny leaf and connects from an allow leaf. Notes from the agent: Revise round 2 addresses C1, C2, C4 and C5. C1: egress_refusal returns None when the host is min.internal (boxes::core::zone_answer::ZONE_APEX) or ends with .min.internal, and the refusal test pins web.min.internal and host.min.internal:8080. C2: BuildTap::observe and the in-box helper log a separate line, "starting a package build for a box in the daemon's own cgroup, as node-plane traffic", with leaf = "none" when the box has no leaf. Outside the layers, as the directive asked: docs/specs/18-spec-box-networking/18-spec-box-networking.md (NET-155 added after NET-080, with the scope boundary for boxes with no leaf) and crates/minimald/tests/classifier_root_integration.rs (the new #[ignore]d proof, run by just test-root-integration). Not run: the new root proof. This box has no sudo and does not run as root. It compiles and passes clippy. It needs gcc -static, nft and the delegated cgroup2 tree, the same as the other proofs in that binary. Not run: just lint-prose, because Vale is not installed here. PR body correction for note 3: after 3e4b720, a placed box whose leaf is gone refuses the build. So a stopped placed box does not build in the daemon's cgroup. Only a box with no classifier leaf (every own_ip box, and a host_ip box with host_ip_enforcement = none) builds in the daemon's cgroup. I cannot edit the PR body, so the runner or a person must carry this. C3 is not addressed and no issue was filed, as the note asked. |
|
/revise Fix the new root-lane proof at a400c75 (job 122790590953,
Generated by Claude Code |
|
That Nothing to type again: the command gets a rocket when its round starts, or a refusal that names the reason. No round was counted for it yet. |
Cache::at_dir makes its subdirectories but not its base, so the root-lane build proof failed before the build ran.
|
Round 3 of 10 on a failed check suite, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. A deny-all box that asks the daemon to build a package which downloads during its build sees the build fail with the destination and the box named, and a box allowed that destination builds it. The root-lane proof package_build_in_a_box_leaf_follows_its_verdict_over_a_loaded_table now gets past its cache setup and runs the real build in the deny leaf and in the allow leaf. Notes from the agent: Round 3 addresses the failed minimald-root-integration check. The proof package_build_in_a_box_leaf_follows_its_verdict_over_a_loaded_table called lcache::Cache::at_dir on a directory that did not exist. Cache::at_dir makes its subdirectories but not its base, so the proof failed with NotFound before the build started. The fix creates the directory first. classifier_root_integration.rs is outside the layers. An earlier round added the proof for review thread C5, and this round only fixes it. I could not run the root lane here: this box has no cgroup2 mount, no nft and no root. To check the build half, I used a temporary op test (not committed) with no leaf. It reproduced the NotFound. With the directory created, the same probe build ran in the sandbox and printed 'connected to 127.0.0.1 port '. So the leaf half (deny refuses with EHOSTUNREACH, allow connects) is still proved only by CI. The sandbox remount EPERM that I saw with a base under /tmp comes from this box's nodev /tmp. It does not apply to the CI runner. cargo-deny fails on a RustSec advisory ('Exponential event growth with ENABLE_WIKILINKS') in a dependency. This branch does not change Cargo.lock or deny.toml, so the advisory is outside this task. The 'unit' check excerpt ends at 'error: recipe |
|
Round 4 of 10 on a /revise note: the coder changed nothing. The fix that the /revise note asks for is already at the branch head, in commit 265663e. That commit creates the cache directory with create_dir_all before Cache::at_dir in run_probe_build. exec_base needs no change: sandbox::config::build calls fs::create_dir_all on its base directory (crates/sandbox/src/config.rs:852), so op creates it. This round has no new commit for that reason. The deny-leaf and allow-leaf legs are unchanged. I did not run just test-root-integration because this box is not root (uid 1000). The root lane must prove it. The minimald test binaries, the root-integration binary among them, compiled during the nextest runs. The note says to change nothing else, so I did not fix the two open threads, although both findings hold. They need a follow-up round or an issue. The macOS minvmd timeouts and cargo-deny are left alone, as the note says. |
|
The runtime will not revise this pull request again on its own: the diff is +2069/-58 (2127 lines), past 3x the plan's estimate of 580 lines (est: 320 code, 260 tests, as #2691 reads now), at round 1 (the last trigger was a conflict with the base branch). A person decides now. Remove |
|
Round 1 of 10 on a failed check suite: the coder changed nothing. No change this round. ci-macos-success is the lane aggregator, and it failed only because the macOS e2e job failed. The e2e job failed in one case, fresh_install_range_lands_on_macos (scripts/session-e2e.sh, added on main by T98 #2713). Its GET to http://e2e-fi-range.min.internal:8080/ got no response. Earlier steps of the same case passed: the activation of the own-IP box, both session execs, the in-box responder check, and the zone answer of an address in 127.0.64.0/24. So the failure is in the host-to-box ingress forward at the range address, after min finalize-install applied the range to lo0 on a running VM. This branch does not touch that path: the ingress forwards, the range walk, the zone answerer and the switch registration of the box stay as they were. The only own-IP change on this branch is own_address_build_network. It attaches a network only when a package build needs DNS or the internet. If an activation build used it in this case, the in-VM daemon would refu…[truncated] |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The ruling from 834ee39 holds for this head. The new commit merges main and does not change the PR's own diff. Section 4.1 line 54 is unchanged in the new arch checkout. The D9 telemetry section does not govern this change. NET-155 conforms.
Ruled at 0f22720 against gominimal/arch@42d214dc8ac8 by claude-opus-5-5.
There was a problem hiding this comment.
Own review at 0f22720. I compared this head with the one I last reviewed, 9b80461, using git range-diff 354548eb..9b804617 aa6964d5..0f227202. All 19 of the PR's own commits are identical. The head adds only merges of main, and the base-merge round resolved a conflict in crates/minimald/box_actor/tests.rs that changes only context lines. My earlier review holds. The arch review passes at this head (ruling 598), and every lane is green.
Generated by Claude Code
|
/revise Close security thread 4401069049 for the check path, by the owner's ruling. Change nothing else.
Generated by Claude Code |
…s-package-builds-under-that-box-s-egre
A check's package builds follow the box's egress verdict, as a task's do.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The new commit refuses min check for a placed box whose classifier leaf is gone. It uses the same refusal as a task. Section 4.1 line 54 says that the box's own cgroup decides a deny-all box's verdict. So a check build must not run in the daemon's cgroup, and now it does not. The merge of main does not change the NET-155 code. The ruling on 0f22720 stands for the rest of the change.
Ruled at b0dcac4 against gominimal/arch@42d214dc8ac8 by claude-opus-5-5.
|
Round 2 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. A deny-all box that asks the daemon to build a package which downloads during its build sees the build fail with the destination and the box named, and a box allowed that destination builds it. This round also closes the check path: Notes from the agent: Revise round 2: addressed the /revise note for security thread 4401069049. BoxActor::start_check now reads the box record after self.context(false) and calls exec::task_build_refusal with the name fallback that task_producer uses (record.name, else the id). Two new tests in box_actor/tests.rs: box_package_build_under_box_egress_verdict_refuses_check_of_placed_box_without_leaf (PerBox record, no leaf, PermissionDenied with placed_box_build_refusal) and box_package_build_under_box_egress_verdict_starts_check_of_box_never_placed. They go through a real TestServer box and its handle. The directive listed no unresolved threads, so threads is empty. Nothing else changed. |
|
The runtime will not revise this pull request again on its own: the diff is +2133/-58 (2191 lines), past 3x the plan's estimate of 580 lines (est: 320 code, 260 tests, as #2691 reads now), at round 2 (the last trigger was a /revise note). A person decides now. Remove |
There was a problem hiding this comment.
Approve at b0dcac42.
- The
/reviseround adds the NET-155 refusal toBoxActor::start_check. Aftercontext(false)it reads the box record and callsexec::task_build_refusal. Somin checkrefuses a placed box whose leaf is gone, with the sameplaced_box_build_refusaltext thatmin task runprints. Both callers ofstart_checkservemin check, and no relaunch asks for a check, so the refusal cannot block a launch. - The two new tests in
box_actor/tests.rspin the refusal and the never-placed path. - The merge
641b255cofmainis clean. Its hunks insetup.rsandenv.rsare port probes and expose replies, not NET-155 code. - arch-review rules PASS at this head, and every lane is green.
Generated by Claude Code
…dict (#2720) T104, NET-155. A box's package builds run in the box's classifier leaf, or for an own-address box on a task network of the box's own, so the box's egress verdict decides their connections. That covers build and materialize side operations, launch builds, in-box installs, patched builds and task runs. A box whose launch placed it in a leaf refuses a build, task or check when the leaf is gone, so a build never runs outside the verdict. A build the box's rules refuse names the destination and the box, and each build start logs the box, package and leaf. A box with no leaf and no own address builds in the daemon's cgroup as node-plane traffic, as before, and logs that it does. Backport of #2720. Adapted to the line: - box_actor/setup.rs hunks land in session.rs, box_ops.rs is session_sop.rs, and the tests land in session/tests.rs, with super::setup:: read as super::, a Record literal of the line's fields, and local rewrite_record and read_record helpers. - boxes, sandbox and box_manager read sessions, sandbox2 and sessions, and crate::traced becomes tokio::spawn and tokio::task::spawn_blocking. - mctx keeps the line's build_graph body, without main's dangling-hit replan, and only gains the local_orchestrator call that carries the leaf. Dropped: main's session_id on the env channel, which T104 only carried as context, the dangling-hit test in op/src/patched.rs, and the test for #2234 in op/src/specs.rs, which comes with #2234 below. (cherry picked from commit a26f89b)
…dict (#2720) T104, NET-155. A box's package builds run in the box's classifier leaf, or for an own-address box on a task network of the box's own, so the box's egress verdict decides their connections. That covers build and materialize side operations, launch builds, in-box installs, patched builds and task runs. A box whose launch placed it in a leaf refuses a build, task or check when the leaf is gone, so a build never runs outside the verdict. A build the box's rules refuse names the destination and the box, and each build start logs the box, package and leaf. A box with no leaf and no own address builds in the daemon's cgroup as node-plane traffic, as before, and logs that it does. Backport of #2720. Adapted to the line: - box_actor/setup.rs hunks land in session.rs, box_ops.rs is session_sop.rs, and the tests land in session/tests.rs, with super::setup:: read as super::, a Record literal of the line's fields, and local rewrite_record and read_record helpers. - boxes, sandbox and box_manager read sessions, sandbox2 and sessions, and crate::traced becomes tokio::spawn and tokio::task::spawn_blocking. - mctx keeps the line's build_graph body, without main's dangling-hit replan, and only gains the local_orchestrator call that carries the leaf. Dropped: main's session_id on the env channel, which T104 only carried as context, the dangling-hit test in op/src/patched.rs, and the test for #2234 in op/src/specs.rs, which comes with #2234 below. (cherry picked from commit a26f89b)
T104 — Run a box's package builds under that box's egress verdict
slice: S16
kind: task
covers: NET-155
size: L
est: 320 code, 260 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: when a box's rules refuse a destination its package build needs, the build fails and its error names the destination and the box, in words like "the build of could not reach : the egress rules of box do not allow it", printed where the build's errors already print and pinned by box_package_build_refusal_names_destination_and_box
observability: the daemon logs one line per package build it starts for a box, naming the box, the package and the classifier leaf the build runs in
layers:
- op:
- crates/op/src/specs.rs: a spec build placed for a box runs its sandbox in that box's classifier leaf and not in the daemon's leaf, and the dependency builds it starts carry the same placement
- crates/op/src/patched.rs: a patched build carries its placement to the spec build it runs
test: cargo nextest run -p op spec_build_runs_in_the_given_classifier_leaf
- build-path:
- crates/orchestrator/src/local_backend.rs: the local backend carries a box's placement to every spec build it schedules
- crates/mctx/src/lib.rs: a context made for a box carries the box's placement into the orchestrator it builds with
- crates/mctx/src/env.rs: the client-side patched build passes no placement, so it stays the caller's own traffic
- crates/mip/src/cmd_pkg_patched_build.rs: the command-line patched build passes no placement
test: cargo nextest run -p mctx box_context_carries_its_classifier_leaf
- daemon:
- crates/minimald/src/box_actor/setup.rs: the context a box's build runs with carries that box's classifier leaf
- crates/minimald/src/box_ops.rs: the build side operation starts its build with the box's placement
- crates/minimald/src/env.rs: the in-box build helper runs its patched build in the box's leaf, and a build refused a destination reports the destination and the box
- crates/minimald/src/box_actor/tests.rs: the build placement and refusal tests
test: cargo nextest run -p minimald box_package_build_under_box_egress_verdict
test: cargo nextest run -p minimald box_package_build_refusal_names_destination_and_box
produces:
- crates/op/src/specs.rs
consumes: none
proof: A deny-all box that asks the daemon to build a package which downloads during its build sees the build fail with the destination and the box named, and a box allowed that destination builds it.
Proof
A box that runs
min buildormin patched-pkgnow gets its package builds placed in the box's own classifier leaf, not in the daemon's leaf. For a deny-all box, a build that cannot reach its download fails with a line such as "the build of hello could not reach example.com:443: the egress rules of box my-box do not allow it", before the build's own error. A box in the allow subtree builds in its allow leaf, and its failures are never reported as refused by its rules. The tests show the placement, the wording and the destination parsing. No test in this session runs a real enforced build.Notes from the agent: 1. The spec excerpt (.implement/spec.md) does not contain NET-155. It stops at NET-143. I worked from the task fields (layers, diagnostics, observability, proof) only.
2. A real end-to-end proof needs root and an installed classifier tree, so the tests do not run one: a deny-all box whose build is refused by the kernel. The tests pin these parts: the sandbox config carries the leaf, the context and orchestrator carry it, the daemon finds the box's leaf, and the refusal wording.
3. The daemon finds the box's leaf as the directory that the launch created under /boxes/deny or /allow. Only host-address boxes that are running have a leaf. Other boxes build in the daemon's own cgroup, as before. A build of a stopped box also builds there. Creating a leaf for that build was out of scope.
4. The destination comes from the build's last output lines, so the error names it only when the build tool prints it. The parser knows the failure lines of curl and wget, and of git over HTTPS (curl's text). When no line names a destination, the build's own error is printed unchanged. The refusal is reported only for a leaf in the deny subtree, because that verdict refuses every destination.
5. Printed text: I adjusted the task's suggested wording to 'the build of could not reach : the egress rules of box do not allow it'. Through `m…[truncated]
Layer exception
Two files are outside T104's layers. The arch review at 3e4b720 (arch_rulings 510) asked for both:
docs/specs/18-spec-box-networking/18-spec-box-networking.md: condition C4. The code cites NET-155, and the spec stopped at NET-143. This PR adds NET-155 as an EARS requirement with the leaf-missing refusal and the no-leaf scope boundary.crates/minimald/tests/classifier_root_integration.rs: condition C5. It adds the#[ignore]d root-lane proofpackage_build_in_a_box_leaf_follows_its_verdict_over_a_loaded_table, run byjust test-root-integration. A real spec build fails to connect from a deny leaf and connects from an allow leaf. It passes inminimald-root-integrationon 0a16760.Tests
cargo nextest run -p op spec_build_runs_in_the_given_classifier_leafcargo nextest run -p mctx box_context_carries_its_classifier_leafcargo nextest run -p minimald box_package_build_under_box_egress_verdictcargo nextest run -p minimald box_package_build_refusal_names_destination_and_boxVerify
min task run agent-verifypassed ([tasks.verify]).Base
The base branch moved while the coder worked; it was merged in before the tests ran.
Closes #2691
Generated by Claude Code