8000 feat(minimald): Unit 3 DNS — B5 host-side egress proxy + .min.internal hostname registry by gominimal-aw-bot[bot] · Pull Request #546 · gominimal/minimal · GitHub
[go: up one dir, main page]

Skip to content
Merged
24 changes: 12 additions & 12 deletions crates/minimald-rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,23 +233,23 @@ impl OneshotSshRpc for DestroySession {
}

// ---------------------------------------------------------------------------
// Session-creation flow (composition with client-side policy gating).
// Session-creation flow (multi-round contribution composition).
//
// Distinct from the simpler [`CreateSession`] above: that one takes a
// fully-formed [`sessions::Record`]; the flow below composes the record
// by walking client contributions plus daemon-side project/package
// closures, with one round-trip for client-side policy gating in
// between.
// by walking client contributions and daemon-side closures across one or
// more rounds. Each call returns a [`SessionStep`]: either the next round
// of pending items or a protocol-level fault.
//
// TODO: these three RPCs are the building blocks for what is eventually
// going to subsume `CreateSession` — once the flow lands on the daemon
// the final `SubmitVerdict` reply will assemble a `sessions::Record` and
// the single-shot `CreateSession` becomes redundant. Keeping them
// separate for now so existing `CreateSession` callers stay working
// going to subsume `CreateSession` — once the multi-round flow lands on
// the daemon, the terminal `SessionStep` will assemble a `sessions::Record`
// and the single-shot `CreateSession` becomes redundant. Keeping them
// separate for now so the existing `CreateSession` callers stay working
// while the contribution flow is built out.

/// An RPC to open a new session and receive the items the client must
/// gate via user policy.
/// An RPC to open a new session and receive the first round of items
/// the client must resolve.
pub struct SessionCreate;

impl OneshotSshRpc for SessionCreate {
Expand All @@ -258,8 +258,8 @@ impl OneshotSshRpc for SessionCreate {
type Response = Errorable<sessions::wire::request::SessionStep>;
}

/// An RPC to submit the client's verdicts on the pending items. The
/// daemon assembles the final session on receipt.
/// An RPC to submit the client's verdicts for one round and receive the
/// next round (or a `complete` signal in [`sessions::wire::request::ContributionResponse`]).
pub struct SubmitVerdict;

impl OneshotSshRpc for SubmitVerdict {
Expand Down
8 changes: 8 additions & 0 deletions crates/minimald/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,14 @@ async fn async_main() -> Result<(), MainError> {
// If we got this far we need to launch minimald.
if !cli.listen_args().unwrap().vsock {
// standard path, listening on UDS socket

// DM2 (native-Linux host): bind the B5 host-side egress proxy listener
// as a startup reachability check. PTask `*.min.internal` hostnames (Unit 3)
// are resolved host-side and routed by `Host:` header through this proxy;
// the host resolver is never consulted. A bind failure warns with a
// remedy (this supersedes the former R3.4 systemd-resolved probe).
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

MEDIUM — Correctness / Spec R3.3

The egress proxy is never started. bind_listener binds the TCP socket to test reachability, but let _ = ... drops the returned Option<TcpListener> immediately at the semicolon (let _ is not a binding in Rust — it drops on the spot). serve() is never called anywhere, so the proxy never accepts a connection and no *.min.internal hostname is routable at runtime.

R3.3 requires a browser on the same host to reach a PTask via hostname through the host-side proxy. That cannot happen without a running serve() loop.

Wiring serve() would also need an architectural change: Manager owns hostnames: HostnameRegistry as a plain field with exclusive &mut self access, but Router::new() requires Arc<T: HostRoute>. The test suite bridges this with the Shared(RwLock<HostnameRegistry>) wrapper in proxy.rs, but nothing equivalent exists in the production path. Completing R3.3 therefore needs both the serve() call and a registry-sharing refactor.

If serve() wiring is intentionally deferred to #502 (the B8 mTLS extension that picks up this routing core), the PR description should name that as a known-deferred item alongside the #542 and Unit 4 deferrals, so a reviewer can confirm the gap is tracked.

let _ = minimald::net::proxy::bind_listener(minimald::net::proxy::DEFAULT_PROXY_ADDR).await;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

MEDIUM · Correctness — The TcpListener returned by bind_listener is immediately dropped via let _ = ..., so port 7654 is released and serve() is never called. Meanwhile, inside bind_listener, the success path emits status = "listening" — which appears in daemon logs as if the egress proxy is actively serving. It is not: HTTP(S)_PROXY clients pointing at 127.0.0.1:7654 will get connection-refused.

If this is intentionally a pure reachability probe (check-and-release), the status = "listening" label in bind_listener's success log is misleading. Consider status = "probed" or similar, or add a note in bind_listener's doc that the caller may drop the returned listener for probe-only use.

If the intent is to wire the proxy into the daemon's event loop here, the TcpListener should be retained and serve(listener, router) called (spawned onto the runtime). The routing core (Router, serve, handle_connection) is fully implemented in proxy.rs; only the wiring in main.rs is missing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

MEDIUM — Correctness / Spec compliance (R3.3)

bind_listener binds the proxy listener then immediately discards it — serve() is never called anywhere in the daemon startup path. PTask *.localhost hostnames are therefore not routable in production: any HTTP_PROXY-configured client will receive connection-refused rather than a routed response.

R3.3 requires that "a browser on the same host shall reach a webserver inside an OwnIp PTask by its hostname"; that end-to-end path depends on the proxy actually serving. The serve() function is implemented and covered by tests in proxy.rs, but 8000 is not wired in.

The task body for #500 explicitly scopes main.rs to a "reachability check" (bind and warn on failure), so this appears to be intentional scoping with the actual serve wiring deferred. If that's correct, the bind_listener comment (reachable rather than listening) is accurate, but R3.3 is not delivered by this PR and the deferred wiring should be tracked explicitly.


if let Err(e) = std::fs::remove_file(cli.listen_on())
&& e.kind() != std::io::ErrorKind::NotFound
{
Expand Down
258 changes: 258 additions & 0 deletions crates/minimald/src/net/dns.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,258 @@
//! In-memory PTask hostname registry: the host-side routing table for the B5
//! egress proxy (Unit 3, UC2a) on the native-Linux (DM2) path.
//!
//! ## `*.min.internal` + host-side egress proxy
//!
//! Open Question 1 of the networking spec is settled in favour of the spec's B5
//! model (re-scoped 2026-06-23, superseding spike #485's systemd-resolved
//! finding): a PTask hostname takes the form `<session>.<host-id>.min.internal`,
//! and both resolution and routing stay **host-side**. The host resolver is
//! never consulted and `minimald` writes nothing to it — `*.min.internal` (the TLD
//! is an opaque label) is mapped internally by the host-side egress proxy
//! ([`super::proxy`]), which routes each incoming request to the right PTask by
//! its `Host:` header. This registry is the lookup table that proxy consults —
//! [`HostnameRegistry::resolve`] is the routing decision for a `Host:` header.
//! Because resolution is host-side, the no-systemd sandbox (hakoniwa) and
//! microVM (libkrun) runtimes never resolve anything, and the TLD choice is
//! irrelevant to correctness.
//!
//! `HostNet` PTasks register to `127.0.0.1` (R3.6); `OwnIp` PTasks register to
//! their gvproxy switch IP (R3.1), which the proxy reaches through the switch
//! relay. [`HostnameRegistry::register`] takes the target address, so both modes
//! are the same registry call with a different target.
//!
//! Covers R3.5 (structured register/deregister tracing) and R3.6 (`HostNet`
//! registration). The former systemd-resolved startup probe (R3.4) is removed by
//! the re-scope; an egress-proxy reachability check
//! ([`super::proxy::bind_listener`]) replaces it.

use std::collections::HashMap;
use std::fmt;
use std::net::{IpAddr, Ipv4Addr};

use sessions::SessionId;

/// The DNS suffix every PTask hostname carries (see the module docs).
pub const HOSTNAME_SUFFIX: &str = "min.internal";

/// Default `<host-id>`: a stable short name for this `minimald` instance. The
/// host-id is configurable; this is the value used when none is configured.
pub const DEFAULT_HOST_ID: &str = "local";

/// The loopback address a local-only (non-DM5) PTask hostname routes to (R3.6).
const LOOPBACK: IpAddr = IpAddr::V4(Ipv4Addr::LOCALHOST);

/// A registered PTask hostname of the form `<session>.<host-id>.min.internal`.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct Hostname(String);

impl Hostname {
/// Builds the hostname for a PTask. DNS names are case-insensitive, so the
/// rendered form is lower-cased to keep lookups stable.
fn for_ptask(session_name: &str, host_id: &str) -> Self {
Self(format!("{session_name}.{host_id}.{HOSTNAME_SUFFIX}").to_ascii_lowercase())
}

/// The hostname as a string slice.
#[must_use]
pub fn as_str(&self) -> &str {
&self.0
}
}

impl fmt::Display for Hostname {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(&self.0)
}
}

/// A live registration: the hostname minted for a session, plus the stable
/// `SessionId` carried in its R3.5 tracing events. The id is captured at
/// registration so `deregister` (keyed by the mutable session name) emits the
/// same stable identifier the `registered` event did.
#[derive(Debug, Clone)]
struct Registration {
id: SessionId,
hostname: Hostname,
8BD0 }

/// An in-memory registry of live PTask hostnames, owned by the sessions manager
/// (one per `minimald`). It maps each live PTask's hostname to the address a
/// host-side proxy routes its requests to, and tracks the reverse (session →
/// hostname) so a hostname is withdrawn when its session exits.
#[derive(Debug)]
pub struct HostnameRegistry {
/// The `<host-id>` component shared by every hostname this registry mints.
host_id: String,
/// hostname → the address a host-side proxy routes its requests to.
by_host: HashMap<Hostname, IpAddr>,
/// session name → its live registration, for withdrawal on exit.
by_session: HashMap<String, Registration>,
}

impl HostnameRegistry {
/// Creates an empty registry whose hostnames use the given `<host-id>`.
#[must_use]
pub fn new(host_id: impl Into<String>) -> Self {
Self {
host_id: host_id.into(),
by_host: HashMap::new(),
by_session: HashMap::new(),
}
}

/// Registers `session_name`'s hostname, routing it to `target`, and returns
/// the hostname. Emits the R3.5 `registered` tracing event. `session_id` is
/// the stable, unique identifier carried in that event for log correlation;
/// `session_name` is the registry key (mutable, and reusable after the
/// session exits), so both are emitted.
pub fn register(
&mut self,
session_id: SessionId,
session_name: &str,
target: IpAddr,
) -> Hostname {
let hostname = Hostname::for_ptask(session_name, &self.host_id);
self.by_session.insert(
session_name.to_string(),
Registration {
id: session_id,
hostname: hostname.clone(),
},
);
self.by_host.insert(hostname.clone(), target);
tracing::info!(
session_id = %session_id,
session_name,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

MEDIUM — Spec compliance (R3.5): session_name field does not match the required session_id

R3.5 explicitly lists the expected structured-tracing fields:

session_id, hostname, ip, action (registered / deregistered)

Both register (here, line 102) and deregister (line 124) emit session_name instead of session_id. The PR notes acknowledge this deviation and explain the registry is keyed by name, but the field name still diverges from the spec. This matters for log-aggregation pipelines, alerting rules, or operator tooling that queries the canonical session_id field — they will miss these events.

Consider either:

  • Aliasing the field at the tracing call site: session_id = session_name (keeps the local variable name, satisfies the spec field name), or
  • Amending R3.5 in the spec to formally rename the field to session_name if the registry-by-name architecture is the intended long-term shape.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

MEDIUM · Spec compliance · R3.5

R3.5 (spec text unchanged in this PR) requires the tracing event field to be named session_id:

"...shall emit a structured tracing event with fields: session_id, hostname, ip, action ..."

The implementation emits session_name instead. The same deviation is present in the deregister event (line ~125).

This is more than a renaming: SessionId is a UUID (stable, unique, unchangeable for the lifetime of a session record), while session_name is a human-readable string that is optional (falling back to the project dir basename), mutable via RenameSession, and potentially reused after destruction. A log-pipeline or monitoring consumer correlating events by the R3.5-specified session_id field will find nothing in these events.

The PR body notes "the registry is keyed by name per the architecture doc" — that is a valid reason for the registry’s internal key, but R3.5 governs the tracing event field, not the registry key. sessions::Record carries both id: SessionId and name: Option<String>, so emitting session_id alongside session_name is not blocked architecturally; the call sites in sessions.rs have access to the full record.

hostname = %hostname,
ip = %target,
action = "registered",
"registered PTask hostname"
);
hostname
}

/// Registers a `HostNet` PTask, routing its hostname to `127.0.0.1` (R3.6).
pub fn register_host_net(&mut self, session_id: SessionId, session_name: &str) -> Hostname {
self.register(session_id, session_name, LOOPBACK)
}

/// Withdraws `session_name`'s hostname, returning it if one was registered.
/// Emits the R3.5 `deregistered` tracing event only when an entry is
/// actually removed, so calling it for an unregistered session is a silent
/// no-op. The event carries the same stable `session_id` the matching
/// `registered` event did, and formats `ip` with `Display` to match it.
pub fn deregister(&mut self, session_name: &str) -> Option<Hostname> {
let Registration { id, hostname } = self.by_session.remove(session_name)?;
let ip = self
.by_host
.remove(&hostname)
.expect("by_host is kept in sync with by_session by register");
tracing::info!(
session_id = %id,
session_name,
hostname = %hostname,
ip = %ip,
action = "deregistered",
"deregistered PTask hostname"
);
Some(hostname)
}

/// Resolves a `Host:` header to the address a host-side proxy routes the
/// request to, or `None` if no live PTask owns that hostname.
///
/// This is the registry/proxy contract: every `*.min.internal` name resolves to
/// loopback in the DNS layer, so the per-PTask routing decision is made here,
/// by hostname. The header's optional `:port` suffix is ignored and matching
/// is case-insensitive, matching how a real `Host:` header arrives.
#[must_use]
pub fn resolve(&self, host_header: &str) -> Option<IpAddr> {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LOW (Correctness) — split(':').next() strips the port by splitting on the first colon, which gives an incorrect host component for IPv6 Host: header values. For [::1]:8080 it yields [ rather than [::1]; for a bare [::1] it yields [ (the whole string is the correct result).

Currently harmless: the registry only holds *.localhost names and a proxy routing to a PTask will never receive an IPv6 literal as the Host: value. But the parsing is latently wrong and would silently return None instead of a potential match if this code path is ever reused or extended.

The standard approach for HTTP Host-header port stripping handles the bracket-enclosed IPv6 form:

let host = if host_header.starts_with('[') {
    // IPv6 literal — strip the optional ":port" after the closing "]"
    host_header
        .find(']')
        .map(|i| &host_header[..=i])
        .unwrap_or(host_header)
} else {
    host_header.split(':').next().unwrap_or(host_header)
};

At minimum, a comment documenting that IPv6 Host: headers are intentionally unsupported here would prevent a future contributor from extending the code unaware of the edge case.

let host = host_component(host_header);
self.by_host
.get(&Hostname(host.to_ascii_lowercase()))
.copied()
}
}

/// Extracts the host of a `Host:` header, stripping an optional `:port` suffix.
///
/// Handles both the common `name:port` form and the bracketed IPv6 literal
/// form (`[::1]:8080`), where the port follows the closing bracket rather than
/// the first colon. The registry only ever holds `*.min.internal` names, so an
/// IPv6 literal never routes; parsing it correctly keeps a naive split-on-first-
/// colon from silently truncating `[::1]` to `[`. Shared with [`super::proxy`],
/// which splits the same authority into host and port.
pub(crate) fn host_component(host_header: &str) -> &str {
if host_header.starts_with('[') {
// IPv6 literal: the host is everything up to and including the closing
// bracket; any `:port` follows it.
host_header
.find(']')
.map_or(host_header, |close| &host_header[..=close])
} else {
// `name` or `name:port`: the host ends at the first colon.
host_header.split(':').next().unwrap_or(host_header)
}
}

#[cfg(test)]
mod tests {
use super::*;

/// Proof artifact 1 (registry/proxy contract): registering a `HostNet`
/// PTask makes the host-side proxy route its `Host:` header to `127.0.0.1`;
/// deregistering withdraws it so the proxy no longer routes it. `*.min.internal`
/// is synthesized to loopback statically by the resolver, so this asserts the
/// registry/proxy routing contract, not a `getaddrinfo` lifecycle.
#[test]
fn host_net_registration_routes_by_host_header_then_withdraws() {
let mut reg = HostnameRegistry::new("dev");

let hostname = reg.register_host_net(SessionId::nil(), "myservice");
assert_eq!(hostname.as_str(), "myservice.dev.min.internal");

// The host-side proxy routes a request by its `Host:` header to the PTask
// — with or without the `:port` a real header carries.
let loopback = IpAddr::V4(Ipv4Addr::LOCALHOST);
assert_eq!(reg.resolve("myservice.dev.min.internal"), Some(loopback));
assert_eq!(
reg.resolve("myservice.dev.min.internal:8080"),
Some(loopback)
);

// After the session exits the entry is gone and the proxy no longer
// routes it.
let removed = reg
.deregister("myservice")
.expect("hostname was registered");
assert_eq!(removed.as_str(), "myservice.dev.min.internal");
assert_eq!(reg.resolve("myservice.dev.min.internal"), None);
}

/// Port stripping handles both the common `name:port` form and the
/// bracketed IPv6 literal form, where the port follows the closing bracket.
/// The registry never holds an IPv6 literal, so this guards the parse from
/// silently truncating `[::1]` to `[` at the first colon.
#[test]
fn host_component_strips_port_including_bracketed_ipv6() {
assert_eq!(
host_component("svc.dev.min.internal"),
"svc.dev.min.internal"
);
assert_eq!(
host_component("svc.dev.min.internal:8080"),
"svc.dev.min.internal"
);
assert_eq!(host_component("[::1]:8080"), "[::1]");
assert_eq!(host_component("[::1]"), "[::1]");
}

/// Deregistering a session that was never registered is a silent no-op, so
/// the manager can call it unconditionally on session teardown.
#[test]
fn deregister_unknown_session_is_a_noop() {
let mut reg = HostnameRegistry::new("dev");
assert_eq!(reg.deregister("ghost"), None);
}
}
2 changes: 2 additions & 0 deletions crates/minimald/src/net/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
//! Covers R1.4 (gvproxy child lifecycle), R1.6 (per-host IP allocation with no
//! reuse), and R1.8 (structured tracing for every switch lifecycle event).

pub mod dns;
pub mod proxy;
pub mod switch;

use std::fmt;
Expand Down
Loading
Loading
0