Repository navigation
T148 — A native own-address box reaches a host exposure under the current table - #2970
norrietaylor merged 18 commits into
Conversation
The native relay admits an own-address box's flow to the host alias only at a port the client config.toml's [host-exposures] table opens, and only when the box names host.min.internal in allow_dns_hosts or covers the alias in allow_subnets. The table is read again when the file changes, and a removed exposure resets the flows the box opened to it. client-config gains the table reader, and minimald depends on client-config for it.
|
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. |
…wn-address-box-reaches-a-host-exposure
There was a problem hiding this comment.
arch-review: CONDITIONS · 4 conditions · the full ruling
The native relay gates host.min.internal by the current table and the box's half, and the placement is right. Two parts do not conform to §7.1. First, an admitted alias frame skips the box's allow_protocols. So box→host reach is wider than box→box (lines 248–249). Make the change in C1. Second, a removal cannot terminate a flow that the tracker does not hold, and the tracker frees no slot on FIN or RST (line 250). Make the change in C2. Then file the two residual issues in C3 and C4, and name them in the PR body.
Ruled at 02b26da against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
A host-alias admission skipped the box's egress verdict, so a box that declares TCP alone could reach a UDP exposure. The gate now checks the box's allow_protocols there too. The [host-exposures] reader also re-reads a file whose modification time is too close to the last read to prove it unchanged, so an edit that keeps the length inside one timestamp tick is not missed.
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The native relay applies the table and the box's egress rules correctly, and the placement conforms. One gap remains. After 1024 tracked flows, the gate admits a new flow that it does not track. When its exposure is removed, nothing ends that flow, and the host's segments still reach the box. Section 7.1 line 250 says a removed exposure's established flows are terminated. C1 refuses a flow that the gate cannot track. C2 and C3 file the two gaps that the PR body names without an issue, and list them as residuals.
Ruled at ae6537f against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
On a native Linux host, an own-address box that names Notes from the agent: Revise round 1. The host-alias admission now checks the box's allow_protocols. The table reader re-reads a file whose mtime is within 2 s of the last read and compares the text. I added a test case to native_own_ip_host_exposure_needs_the_box_to_name_the_host and a unit test an_edit_that_keeps_the_stamp_is_read_again in host_exposures.rs. The directive names 4 findings but lists 3 threads. I judged the 3 threads. I did not run just clippy-strict. |
|
/revise Deliver arch C1 in |
…wn-address-box-reaches-a-host-exposure
An untracked flow gets no reset when its exposure is removed, so the gate now refuses it. record_host_flow returns whether it tracked the flow, and the refusal log names the flow limit.
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The native gate follows the current table, and the compare refuses a flow that it cannot track. Two gaps remain. Section 7.1 line 252 says Minimal's own listeners are never exposures, decided at each flow. But host_alias_egress() admits any port that the table names, and the alias maps to 127.0.0.1, where the hostname proxy and the box-zone answerer listen. C1 fixes this. On a carved switch, a host segment from the documented literal skips the removal check, against line 250. C2 fixes this.
Ruled at b3404e9 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 2 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
On a native Linux host, an own-address box that names host.min.internal reaches a port the user's [host-exposures] table opens, and reaches it no more once the entry is removed. A box that does not name the host never reaches it. Each flow the gate admits is now one it tracks, so a removal ends every admitted flow. Notes from the agent: ## Residuals
This round: |
|
/revise Deliver arch C1 and C2 in this PR. T153 (#2981) is folded into T148, so the Minimal-listener exclusion is yours, and #2937 now covers it with its two new test lines. Remove #2981 from the |
|
/revise T102 (#2871) has merged, so this task is unblocked. Merge |
…wn-address-box-reaches-a-host-exposure
minvmd now reads the [host-exposures] table through the shared reader in client-config, so a bad entry is skipped and logged the same way on both daemons. A broken file keeps its text, so the same text is not parsed or logged again for each frame.
On a native own-address box the relay refuses a flow to host.min.internal at a port that the box-name answerer or the hostname proxy holds, ahead of the table's openings, and names the listener in the refusal. A listener held later ends the established flows to it. On a carved switch a host segment from the documented literal of a removed exposure is refused too.
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The shared reader and the native table gate conform to §7.1 lines 248-251. The new listener check on the native relay does not fully meet line 252. That line says Minimal's own listeners are never exposures, and it decides this at each flow, also for a listener that binds later. The gate builds its listener list once. The list holds the answerer at a constant port and the hostname proxy only if it served at attach time. It does not hold the published forwarders on 127.0.0.1. Make the changes in C1, C2 and C3.
Ruled at 580abde against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 3 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:
On a native Linux host, an own-address box that names host.min.internal reaches a port that the user's [host-exposures] table opens. After the entry is removed, the box reaches it no more, and its established flows end, also on a carved switch where the host answers from the documented literal. A box that does not name the host never reaches the port. An entry at a port that the box-name answerer or the hostname proxy holds opens nothing, and the refusal and the daemon log name that listener. Notes from the agent: The origin/main merge (T102) was already on the branch at the start of this round. C1: the native gate starts with the box-name answerer's port and the hostname proxy port that the box's rules are compiled with. Each flow checks these ports before the table's openings. BoxGate::hold_minimal_listener is the hook for a listener that binds later. No production code calls it yet, because the hostname proxy's later re-bind lives in crates/minimald/src/net/mod.rs and proxy.rs, which are outside the layers. The shadowed test drives the hook directly. The native daemon also has no load-time refusal with a log line. It logs once at the first flow that finds the shadowed entry. C2: the ingress filter matches the alias and the documented literal that the switch maps (documented_host_literal). Shared reader: minvmd now reads through client_config::host_exposures::HostExposuresFile. Both daemons skip a bad entry, log it and keep the other entries. A file that does not parse, or a host-exposures key that is not a table, opens no exposure. The client-config reader formerly refused the whole table for one bad entry, and that changed. Outside the layers, I added client-config as a dependency in crates/minvmd/Cargo.toml so that minvmd can use the shared reader. The PR body is written by the runner, so I did not edit it. Remove #2981 from its Residuals heading, as the /revise note asks. Extra commands I ran: cargo test -p client-config host_exposures (6 passed),…[truncated] |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The forward hold in policy.rs conforms to §7.1 line 252. The new names_host_alias() arm refuses an allow-all box at host.min.internal. Under the in-force default the gate never sees an absent allow_subnets. It sees one only for an allow-all box behind the NET-077 opt-out, and that box reaches siblings. Line 251 says an allow-all box reaches exposures as it reaches siblings. Restore is_none_or (C1) and change the test so it proves line 251 (C2).
Ruled at c061bc6 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
…wn-address-box-reaches-a-host-exposure
… once The host exposures reader now compares the change time and the device too, so a restored modification time does not hide an edit. A modification time after the read stays racy for one confirming read only, so a lookup does not read the whole file each time.
A compiled allow_subnets of None is the allow-all dimension, so it covers the host alias, as the VM host's gate decides. The test now asserts that a box with no egress rules, or no allow_subnets, reaches the exposure, and that a deny-all box does not.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change conforms to design §7.1 for NET-160 and NET-161. Commit e409c14b makes an allow-all box reach an exposure, as line 251 says. Commit 12b637d7 reads the table again after any edit, so each flow uses the current table (line 250). The gate decides Minimal's own listeners first and fails closed. A native host-address box is not affected. No conditions are open.
Ruled at e409c14 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 7 of 10 on the runtime's review findings, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
On a native Linux host, an own-address box that names host.min.internal, or that is allow-all, reaches a port the user's [host-exposures] table opens. It reaches the port no more once the entry is removed. A box whose rules do not cover the host, a deny-all box among them, never reaches it. An entry at a port that one of Minimal's own listeners holds opens nothing. Notes from the agent: Revise round 7. I ran the six test lines as one nextest invocation with the shared prefix filter native_own_ip_, because each line is a prefix of that filter. All passed. cargo clippy -p minimald -p client-config --all-targets and cargo fmt --check were clean. I did not run just clippy-strict. |
|
The runtime will not revise this pull request again on its own: the diff is +1954/-157 (2111 lines), past 3x the plan's estimate of 700 lines (est: 300 code, 400 tests, as #2937 reads now), at round 7 (the last trigger was the runtime's review findings). A person decides now. Remove |
|
/revise One change, in |
|
That What the platform saidcould not start the coder: Error: there is no container instance that can be provided to this durable object 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. |
…wn-address-box-reaches-a-host-exposure # Conflicts: # crates/minimald/src/net/switch.rs
A flow to a host exposure stayed tracked until the idle sweep, so 1024 short connections in five minutes refused every new flow with the flow limit, although the table opened the port. A reset either way now removes the flow. A FIN either way marks it closing: the last ACK still finds it, and the sweep at the bound drops closing flows first.
…wn-address-box-reaches-a-host-exposure
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare from the PASS on e409c14b conforms. The merge with #3002 keeps the native table gate on the local-socket attach and #3002's rules on the vsock attach, so the paths do not overlap. Commit 14f7dd6e frees the place of a closed flow under the per-box bound. A closing flow stays tracked, so a table removal still ends it, as section 7.1 line 250 requires. There are no conditions.
Ruled at d47e994 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 19 unresolved threads — minimal-arch[bot] crates/minimald/src/net/switch.rs, minimal-arch[bot] crates/minimald/src/net/switch.rs:2189, minimal-arch[bot] crates/minimald/src/net/switch.rs:456, minimal-arch[bot] crates/client-config/src/host_exposures.rs, minimal-arch[bot] crates/minimald/src/net/switch.rs, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 8 of 10 on a /revise note, pushed as Resolved conflicts in:
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
On a native Linux host, an own-address box that names host.min.internal reaches a port the user's table opens. It reaches it no more once the entry is removed. A box that does not name the host never reaches it, and an entry at a port that one of Minimal's own listeners holds opens nothing. A box that opens and closes many short connections to an exposed port still opens the next one. Notes from the agent: Merge: switch.rs conflicted with origin/main (T154, the VM-host relay). I kept both sides. The native gate decides the alias when a table is set. Otherwise the egress verdict now runs on main's relay_egress(), so the VM path is unchanged. main's duplicate HOST_ALIAS_RULE const and its [u8; 4] host_alias field were removed in favour of the branch's const and its Ipv4Addr field. drop_rule compares against host_alias.octets(). In the constructor, the names_host() call comes before the local names_host binding, so the local does not shadow the function. Fix: the directive asked for removal on a FIN or RST. A reset removes the entry. A FIN marks the flow closing and does not remove it, because the box's last ACK after the FINs goes through record_host_flow and would add the flow back. Removal on FIN would leak the same entry again. A closing flow stays tracked, so a removal still ends a half-closed flow. The sweep at HOST_FLOWS_PER_BOX drops every closing flow first. A bare SYN on the same ports clears the mark. |
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 d47e9940f36c 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.
There was a problem hiding this comment.
Read at d47e994. The listener registry, the hold on every forward, the allow_protocols check and the flow removal on FIN or RST are all in. An allow-all box reaches an exposure as arch ruled. Arch passes.
T148 — A native own-address box reaches a host exposure under the current table
slice: S21
kind: task
covers: NET-160, NET-161
size: M
est: 160 code, 200 tests
executor: agent
difficulty: needs care
model: standard
diagnostics:
min bugbundles carry the native daemon log, where each refused flow from an own-address box tohost.min.internalnames the box, the port and whether the table or the box's rules refused itobservability: the native relay's rate-limited refusal line names the rule
host-aliasand the box; each table reload is logged at debug with its entry countlayers:
- relay:
- crates/client-config/src/host_exposures.rs (new): parses the
[host-exposures]table of the clientconfig.toml, each entry a port and aprotothat istcpwhen the entry names none, and reads it again when the file changes- crates/client-config/src/lib.rs: exports the table reader
- crates/minimald/src/net/switch.rs: the native relay admits a flow from an own-address box to
host.min.internalonly at a port the current table opens, and only when the box'segress.allow_dns_hostsnameshost.min.internalor itsallow_subnetscovers the alias; a change to the table decides each later flow, and an exposure removed ends the established flows to it; a native host-address box is untouched (it shares the host's namespace, design §7.1 Lanes)test: cargo nextest run -p minimald native_own_ip_host_exposure_needs_the_box_to_name_the_host
test: cargo nextest run -p minimald native_own_ip_follows_the_current_host_exposures_table
test: cargo nextest run -p minimald native_own_ip_exposure_removal_ends_established_flows
produces: none
consumes: none
proof: On a native Linux host, an own-address box that names
host.min.internalreaches a port the user's table opens, reaches it no more once the entry is removed, and a box that does not name the host never reaches it.Proof
On a native Linux host, an own-address box whose egress.allow_dns_hosts names host.min.internal (or whose allow_subnets covers the alias) reaches host.min.internal at a port the [host-exposures] table of the user's config.toml opens. After the entry is removed from the file, the next flow is refused and an established flow is reset at both ends. A box that does not name the host never reaches the port. Each refusal is a rate-limited log line with rule_matched="host-alias", the box, the port and refused_by ("host-exposures table" or "box egress rules"). The three relay tests in crates/minimald/src/net/switch.rs show this.
Notes from the agent: Outside the layers: crates/minimald/Cargo.toml gains
client-config.workspace = trueand Cargo.lock gains that edge. minimald did not depend on client-config, and switch.rs needs the table reader. Gap for the next task: client_config::config::Config has #[serde(deny_unknown_fields)], so the existing config reader (and somin) rejects a config.toml that has a [host-exposures] section. The new reader reads only that table and ignores the other sections, so minimald works. The user-facing proof needs ahost_exposuresfield on Config (config.rs, not in this task's layers), most likely as part of the NET-159 task. Design choices: only a local-socket attach (attach_to_switch, the native DM2 path) sets the table on the gate. A vsock attach in a VM guest keeps the old behaviour, because minvmd applies the host's table there. The table is checked by stat (mtime, length, inode) on each frame to or from the alias, and the file is read again only when the stat changes. A table that does not parse is treated as empty (fail closed) with a warn log, and each reload is logged at debug with its entry count. A box's deny_subnets that covers the alias also refuses. allow_protocols is not checked for the alias. An established flow to a removed exposure ends at its next segment in either direction, with a reset to each end. A flow…[truncated]Tests
cargo nextest run -p minimald native_own_ip_host_exposure_needs_the_box_to_name_the_hostcargo nextest run -p minimald native_own_ip_follows_the_current_host_exposures_tablecargo nextest run -p minimald native_own_ip_exposure_removal_ends_established_flowsVerify
min task run agent-verifypassed ([tasks.verify]).Out of scope
These files are outside the task's
layers:, so this PR is a draft:crates/minimald/Cargo.tomlCloses #2937