Repository navigation
T131 — A native host-address box's probe and launch refusal read the box's own state - #2965
Conversation
The start-up probe now runs once the control socket is bound and the daemon stands in its own leaf, and logs its outcome on every host. unenforceable_rules no longer names an empty destination list that the in-force default filled in, so a refusal names only declared flags.
A host-address create re-reads the classifier fact when the stored one says the probe could not read the table, and logs whether it re-read.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Approved: every review thread resolved at @norrietaylor resolved the last thread, which releases the approval this review had withheld. Nothing to do. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The design placement and the fail-closed defaults conform. The change breaks one NET-079 proof. allow_list_host_address_declaration_refused_on_vm_host still expects AllowSubnets for an empty allow_subnets beside a named host, and the PR's verify run fails on it. Update that expectation (C1). Then state the new naming in the unenforceable_rules doc comment, the test comment at line 4311, and the spec 18 rationale at line 707 (C2).
Ruled at 3062f33 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
…ost-address-box-s-probe-and-launch-ref
Update allow_list_host_address_declaration_refused_on_vm_host and the unenforceable_rules comments to the predicate this branch ships.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 2 unresolved threads — minimal-arch[bot] crates/minimald/src/net/classifier.rs:185, minimal-arch[bot] crates/minimald/src/net/classifier.rs:184. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change conforms to §4.1 and §7.4. The startup probe now reads the fact after the bind. A host-address create re-reads only a stale ProbeUnreadable fact, through the launch's own box_runtime helper. The refusal still refuses every declaration it refused before. It now names only the lists that grant reach, so create and launch agree. The task's proof sentence about an --allow-subnets box that runs its commands does not match the NET-079 refusal sub-requirement. Correct that sentence in the plan. The code follows the spec.
Ruled at da49cb4 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 1 of 10 on a failed check suite, pushed as
Verify Review threads the coder judged:
On a native Linux host with the classifier installed, a deny-all host_ip box runs with per-box enforcement and reaches nothing. A host_ip box started with --allow-subnets runs its commands. This round the minimald test suite passes again: the VM-host test now expects a refusal that names only allow_dns_hosts and allow_protocols for a box that declared hosts and protocols. Notes from the agent: Revise round 1. The failing check was allow_list_host_address_declaration_refused_on_vm_host, which still expected AllowSubnets for an empty allow_subnets list. The full minimald suite now passes. The spec 18 rationale (docs/specs/18-spec-box-networking/18-spec-box-networking.md, the NET-079 comment near line 707) still says that any allow list short of every allow list present and empty is a rule. That file is outside this task's layers, so I did not change it. A follow-up docs change must say that an empty destination list beside a non-empty one names no rule. |
|
Round 2 of 10 on a failed check suite: the coder changed nothing. Noop round. The failed checks are infrastructure, not code. ci-macos-success and ci-linux-kvm-success are aggregators that run 'exit 1' because e2e and test-kvm failed. The excerpts for e2e (exit 100 after the reap and install-action steps) and test-kvm show only the echo lines of the workflow scripts and the cleanup. They show no compile error and no test failure from this diff. Re-run those lanes. Both earlier threads (C1, C2) are already fixed at the branch head: the doc comment of unenforceable_rules says that an empty destination list beside a non-empty one names no rule. I also ran the 94 minimald tests that match host_address, host_ip, unenforceable or classifier, and all 94 passed. I did not run e2e or test-kvm, because this box has no KVM and no macOS runner. |
|
/revise The test-kvm and e2e failures are real, and this diff causes them (runs 41749060110 and 41749060006). |
…ost-address-box-s-probe-and-launch-ref
The per-launch reader in guest_classifier_integration takes every line with host_ip_enforcement= as a launch decision. The create lines now carry the fact under host_fact, so that reader sees launch lines only.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This push renames one log field on the host-address create lines, from host_ip_enforcement to host_fact. The change moves no check and changes no default. Now only the per-launch record and the refusal lines carry host_ip_enforcement, which is the attribute that design §7.4 line 280 names. The rest of the change is the same as at da49cb44, which passed. The ruling stays PASS.
Ruled at 4b44872 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 3 of 10 on a /revise note, pushed as
Verify On a native Linux host with the classifier installed, a deny-all host_ip box is created over the re-read per_box fact and reaches nothing, and a host_ip box started with --allow-subnets alone is never refused naming allow_dns_hosts. The create's log lines no longer carry the host_ip_enforcement= field, so the guest per-launch check reads only the launch's own classifier lines. Notes from the agent: Revise round 3: the two host-address create log lines in box_manager.rs (answered from the stored fact, and re-read) now carry the fact as host_fact= in place of host_ip_enforcement=. guest_classifier_integration::guest_host_ip_enforcement_per_box_after_load collects every line with host_ip_enforcement= and the box name, so it now reads only the per-launch lines. The test is not edited. I did not run the test-kvm or e2e lanes on this box, because they need libkrun and KVM. The fix for those lanes is unverified until CI runs them. The test that pins the create log reads only reread=true/false, so it did not change. cargo clippy -p minimald --all-targets is clean. |
|
/revise Fix the LOW thread at |
…ost-address-box-s-probe-and-launch-ref
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare since the last PASS is a clean merge of main and one change to a doc comment. The new comment above the create gate states what create_host_ip_fact does. No code that the PASS on 4b44872e ruled on changed. NET-079 conforms to §4.1 line 54 and §7.4 line 280.
Ruled at 8159b64 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 4 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
On a native Linux host with the classifier installed, a deny-all host_ip box runs with per-box enforcement and reaches nothing, and a host_ip box started with --allow-subnets runs its commands. This round changes only the comment above the create gate, so it now says that the gate reads the fact from create_host_ip_fact, which re-reads the host when the start-up probe could not read the table. Notes from the agent: Revise round 4 changes only a comment. C1 and C2 are already fixed at this head. The test allow_list_host_address_declaration_refused_on_vm_host expects AllowDnsHosts. The doc comment of unenforceable_rules and the test comment near line 4313 say that an empty destination list beside a non-empty one names no rule. The agent cannot resolve threads, so the reviewer must resolve them. |
|
/revise Fix the LOW thread at |
…ost-address-box-s-probe-and-launch-ref
A failed assert no longer leaks the tree stand-in, the reading stand-in or the node fact into a later test.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare from 8159b64b adds no production code. The one branch commit moves the test's reset of the stand-ins into a Drop guard. The guard runs before the fact lock is released, and no assertion changed. The merge brings only #2985 from main. NET-079 still conforms to §4.1 line 54 and §7.4 line 280, as the earlier PASS ruled.
Ruled at ea2d44b against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 3 unresolved threads — minimal-arch[bot] crates/minimald/src/net/classifier.rs:185, minimal-arch[bot] crates/minimald/src/net/classifier.rs:184, minimal-foundry[bot] crates/minimald/src/box_manager/tests.rs. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 5 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
On a native Linux host with the classifier installed, a deny-all host_ip box runs with per-box enforcement and reaches nothing, and a host_ip box started with --allow-subnets runs its commands. This round changes only the test: it now resets its process-global stand-ins and the node fact on every exit. Notes from the agent: This round addresses the LOW thread at tests.rs:5227. A local drop guard now resets CREATE_CLASSIFIER_TREE_STANDIN, the reading stand-in and the host-address fact, and then drops the temp tree. The guard is declared after the PROBE_TEST_MUTEX guard, so the reset runs before the lock is released. The two test lines ran together in one nextest command. cargo clippy -p minimald --all-targets printed no warnings. The note asks me to resolve the three threads. I did not resolve them, because resolution is the reviewer's decision. The two classifier.rs threads were already answered valid in an earlier round, and this round does not change that verdict. |
There was a problem hiding this comment.
Reviewed ea2d44b0 against T131's plan block. A host-address create re-reads the classifier fact only when the stored fact says the probe could not read the table, and keeps the stored fact if the re-read fails. The create lines now carry host_fact, which clears the launch-record test that failed on both lanes. Arch-review passes on this head.
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @norrietaylor closed the last one. The review of ea2d44b0af53 withheld this approval on the threads open then. Posted as a comment: the App cannot approve its own pull request; the check and the label carry it.
T131 — A native host-address box's probe and launch refusal read the box's own state
slice: S16
kind: task
covers: NET-079
size: M
est: 90 code, 190 tests
executor: agent
difficulty: needs care
model: standard
diagnostics: what a person reads is the launch outcome: a deny-all host-address box on a host whose table decides per 8000 box is refused with no note saying the box runs unenforced,
min session policyshowsper-box enforcement per_boxfor it, and a box started with only--allow-subnetsis never refused naming a rule it did not setobservability: the daemon's start-up classifier line is written after the control socket binds and names the probe's outcome, and each host-address create logs whether it re-read the host's fact
layers:
- start:
- crates/minimald/src/main.rs: the start-up classifier read runs after the control socket is bound and the daemon has entered its node-plane leaf, so the probe no longer reads an unplaced daemon as an unreadable table
- crates/minimald/src/net/classifier.rs:
unenforceable_rulesreads an emptyallow_dns_hostslist the in-force default filled in as no rule, so the launch gate and the create gate decide the same declaration, and a refusal names only the flags the box was started withtest: cargo nextest run -p minimald launch_refusal_names_only_declared_rules
test: cargo nextest run -p minimald per_box_host_refuses_unenforceable_host_ip_declaration
- create:
- crates/minimald/src/box_manager.rs: a host-address create re-reads the host's classifier fact when the stored one says the probe was unreadable, instead of answering from the stale start-up read
- crates/minimald/src/box_manager/tests.rs: the stale-fact re-read
test: cargo nextest run -p minimald host_ip_create_rereads_a_stale_unreadable_fact
produces: none
consumes: none
depends on:
- assumes OQ1 (one host DNS user per machine, the current behaviour; host setup refuses on a host whose installed service names another user)
- assumes OQ2 (the spec's interim holds: a native non-deny-all host-address box resolves through the host's resolver and its allow list is decided by address rules only)
proof: On a native Linux host with the classifier installed, a deny-all
host_ipbox runs with per-box enforcement and reaches nothing, and ahost_ipbox started with--allow-subnetsruns its commands.Proof
On a native Linux host with the classifier installed, the daemon reads the classifier fact only after its control socket binds and it stands in its own leaf. A create that still finds the stale 'probe could not read the table' fact re-reads the host. So a deny-all host_ip box is created and launched with per-box enforcement (min session policy shows per_box) and reaches nothing. A host_ip box with --allow-subnets alone is no longer refused for allow_dns_hosts, a rule it never set. On a host that decides per box, it is still refused for allow_subnets alone, as the NET-079 refusal sub-requirement requires.
Notes from the agent: Proof gap: the task proof says a host_ip box started with --allow-subnets 'runs its commands'. The covered NET-079 sub-requirement (per_box_host_refuses_unenforceable_host_ip_declaration, which still passes unchanged) refuses any non-empty allow list on a host that decides per box. This change makes that box run only on a host that cannot decide per box (unenforced, as before). On a per-box host the refusal now names only allow_subnets. A person must decide whether the proof sentence or the sub-requirement is the intent. The box runs on a per-box host only if the spec is changed. unenforceable_rules: an empty destination list next to a non-empty one names no rule. This applies to allow_dns_hosts (the case the task names) and symmetrically to allow_subnets, so a box with only --allow-dns-hosts is not refused for allow_subnets. A lone empty list with the other list absent still names its rule, as the existing tests pin. main.rs: the start-up read moved into read_host_ip_enforcement_fact, which both the UDS and the vsock branches call right after the bind and before Server::run. It now logs an info line on a host that decides too (probe=decided). box_manager.rs: a test-only CREATE_CLASSIFIER_TREE_STANDIN lets the test drive the create re-read over a stand-in tree. The re-read uses the same re_read_classifier_fact the launch uses. The …[truncated]
Tests
cargo nextest run -p minimald launch_refusal_names_only_declared_rulescargo nextest run -p minimald per_box_host_refuses_unenforceable_host_ip_declarationcargo nextest run -p minimald host_ip_create_rereads_a_stale_unreadable_factVerify
min task run agent-verifyexited 100 under rustc 1.99.0, which the repository does not pin (1.99.1 in rust-toolchain.toml); the PR is not a draft on that alone — CI, which honours the pin, is the gate.Closes #2928