Repository navigation
fix(minimald): admit allow-exposed and ask-yes ports at the box's relay gate - #2028
Merged
Merged
Conversation
…ay gate A port published with min net expose under dynamic_ingress = allow, or an ask answered yes, was bound and listed but refused at the box's relay gate: the gate admitted only declared and listen-published ports. The gate now keeps a third, explicitly exposed set. The publish admits the port once its forward stands, the box's stop withdraws it, and a listener closing never touches it (NET-044, NET-017). The expose reply and the live-ingress rows no longer carry the 'not yet reachable' caveat, the spec 18 annotations name exposed ports in the admitted set, and the session e2e requires the host to reach the allow-exposed port instead of tolerating a miss.
Contributor
|
Warning Review limit reached
This review includes 12 billable files and costs up to $3.00.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 7 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (12)
Comment |
Contributor
Spec lint
Advisory. None of this blocks the merge; the review decides. Advisory, and never a merge gate. Linter: |
Contributor
|
Approved: no findings at Nothing to do. Clobber: none: |
…al port Revert the NET-138 annotation to main's text; a separate spec PR rewrites it and already covers exposed ports. The NET-044 annotation edit stays. Assert at the admit site that the admitted port is the forward's internal port, the one the box's stop withdraws, so a host-port remap cannot split them. Add a gate test: revoking an expose on a port a declared mapping also holds keeps the declaration's admission and its connection, and returns 0. Mark LiveMapping::pending deprecated in its doc comment.
# Conflicts: # crates/minimald/src/rpc.rs
jtnkminimal
approved these changes
Oct 7, 2026
norrietaylor
enabled auto-merge
October 7, 2026 02:36
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
A port published from inside a box with
min net exposeunderdynamic_ingress = allowwas bound on the host and listed bymin session policy, but a connection to it never reached the box. The live row read(pending; not yet reachable)and the expose reply ended; not yet reachable. Anaskanswered yes had the same shape.Cause
The box's relay gate (
SessionGate::admits_tcpincrates/minimald/src/net/switch.rs) admitted only declared ports and listen-published ports. Only the listen watcher filled the listen-published set. The expose path, forallowand for an answeredask, bound the forward and never admitted the port, so the gate refused the SYN. When a listener later appeared on an expose-held port, the watcher settled it without admitting it either.Fix
exposed, beside the declared and listen-published sets.admits_tcpand the inbound SYN check read all three.publish_exposed_port(shared byallowand an answeredask) admits the port at the box's live gate once the forward stands and the mapping is recorded, in the same actor turn.published port N at ADDR.GetLiveIngressserves the rows as recorded (pending: false). The wire field stays so a client can still read an older daemon's rows, and the CLI's rendering ofpending: trueis kept for that case only.allowand a yes-answeredask. The NET-138 annotation is left as onmain: #1898 rewrites it and covers exposed ports there.debug_assert_eq!) that the admitted port is the forward's internal port, the one the stop withdraws, so a future host-port remap cannot split the admission from its withdrawal.minimald-rpc: the doc comment onLiveMapping::pendingmarks the field deprecated. Daemons from this change onward always sendSome(false); the field can go when the support window for older daemons ends.scripts/session-e2e.sh: inproof_expose_from_inside_box, the VM lane's host probe was carried as a known gap and is now a hard pass. The native lane's allow leg used to require the host probe to fail and the row to read pending. It now requires the probe to reach the in-box server and the row to carry no caveat. Every expose reply check now rejects the old caveat.Requirement
NET-044: WHEN a dynamic ingress request is decided
allowTHE SYSTEM SHALL publish the port. In spec 18, "publish" means a request to the port returns the in-box server's response (NET-040). NET-047 covers only rejected requests and does not allow a pending window.Verification
net::switch::tests::gate_admits_an_exposed_port_until_the_expose_is_revoked: the exposed port is admitted and passes the SYN gate, an unadmitted port stays refused, a listener withdrawal leaves it admitted, and the revocation withdraws it.session::tests::expose_allow_admits_the_port_at_the_box_gate_until_stop: anallowexpose admits the port at the box's registered gate, an unpublished port stays refused, and the stop withdraws the admission.session_host::tests::expose_ask_yes_admits_the_port_at_the_box_gate: anaskanswered yes through a real attach admits the port, and an unpublished port stays refused.net::switch::tests::withdrawing_an_exposed_port_a_declaration_holds_keeps_its_connections: revoking an expose on a port a declared mapping also holds returns 0, keeps the declaration's admission, and leaves its connection's reply-flow record in place.net::listeners::tests::a_port_the_runtime_expose_published_is_never_bound_or_withdrawnnow also asserts that the watcher leaves the exposed admission in place after the listener closes.expose_allow_publishes_and_lists, the env channel reply tests, and the live-ingress listings.just test-cross(musl in Docker): clippy with-D warningsis clean. Because cargo stops at the first failing binary, theminimald,minimalandminimald-rpctests were then run throughcross testwith the recipe's flags plus--no-fail-fast. Every new and updated test passes. The failures left in that run (13 inminimaldlib: relay, hooks and nsenter tests; 6 in theminimallib: resolver, stopped-VM timing and a stderr hint; 2 innetns_root_integration) are environmental under emulation. The same 13minimaldtests fail the same way onmainin this container.mainand adding the review follow-ups:cross clippy -p minimald -p minimald-rpc --all-targets -- -D warningsis clean, andcross test -p minimald --libfiltered to the expose, withdraw and gate tests ran 59: 58 passed, including the newwithdrawing_an_exposed_port_a_declaration_holds_keeps_its_connections. The one failure,expose_ask_shed_mid_dialog_is_the_daemons_refusal(frommain, untouched here), hit its 30 s timeout under emulation while the batch ran and passed when rerun alone.just lint-shellpasses.just lint-prosereports no alerts on the changed lines.Not verified here: the
scripts/session-e2e.shchanges (the VM-lane reach check, now a hard pass, and the native allow leg's reach) need a real VM or KVM lane. The CI e2e lanes prove them. Runningjust e2ewas out of scope on this host.Merge-order note
#2027 (bounded audit log, fail closed) adds
withdraw_unauditedincrates/minimald/src/session.rs. Whichever of #2027 and this PR merges second must:withdraw_unauditedcalllive_gate(switch_address).withdraw_exposed(port)before the unexpose, andexpose_unrecordable_allow_is_refused_and_deny_still_refusesthat a connection opened to the exposed port before the withdrawal is reset.#2027 was not on
mainwhen this branch last merged it, so this PR does not carry that wiring.