diff --git a/core/archipelago/src/container/prod_orchestrator.rs b/core/archipelago/src/container/prod_orchestrator.rs index 6aea283d..0a31df14 100644 --- a/core/archipelago/src/container/prod_orchestrator.rs +++ b/core/archipelago/src/container/prod_orchestrator.rs @@ -595,10 +595,20 @@ async fn wait_for_manifest_host_ports( /// `podman inspect --format '{{json .HostConfig.PortBindings}}'` emits, e.g. /// `{"8080/tcp":[{"HostIp":"","HostPort":"18080"}]}`. Returns true only when a /// manifest container-port is positively published to a *different* host port -/// than the manifest now asks for. Absence of a binding is deliberately NOT -/// treated as drift here — that case is handled by the host-port repair/restart -/// path and by host-networked apps that publish nothing — so we never trigger a -/// destructive recreate on a false positive. +/// than the manifest now asks for — or, when the manifest DECLARES a bind +/// address, to a different host address. Absence of a binding is deliberately +/// NOT treated as drift here — that case is handled by the host-port +/// repair/restart path and by host-networked apps that publish nothing — so we +/// never trigger a destructive recreate on a false positive. +/// +/// The bind comparison is what lets a node self-heal after a catalog refresh +/// pins an app to loopback for the app gate: a legacy (pre-quadlet) container +/// still publishing `0.0.0.0:P` against a manifest that now declares +/// `bind: 127.0.0.1` is recreated to the declared state, exactly as +/// `package.update` would. An EMPTY manifest bind means "no instruction" and +/// never fires this — recreating a loopback-published container to wildcard on +/// silence is precisely the v1.7.121 incident class (Bitcoin RPC republished +/// on the LAN). fn host_port_bindings_drifted( port_bindings_json: &str, manifest_ports: &[archipelago_container::manifest::PortMapping], @@ -626,10 +636,26 @@ fn host_port_bindings_drifted( } let expected = port.host.to_string(); let matches_expected = bindings.iter().any(|b| { - b.get("HostPort") + let host_port_ok = b + .get("HostPort") .and_then(|h| h.as_str()) .map(|h| h == expected) - .unwrap_or(false) + .unwrap_or(false); + if !host_port_ok { + return false; + } + // Only a DECLARED bind participates; podman reports a wildcard + // publish as "" or "0.0.0.0". + if port.bind.is_empty() { + return true; + } + let actual_ip = b.get("HostIp").and_then(|h| h.as_str()).unwrap_or(""); + let actual = if actual_ip.is_empty() { + "0.0.0.0" + } else { + actual_ip + }; + actual == port.bind }); if !matches_expected { return true; @@ -4569,6 +4595,76 @@ mod tests { )); } + fn bound_port( + host: u16, + container: u16, + bind: &str, + ) -> archipelago_container::manifest::PortMapping { + archipelago_container::manifest::PortMapping { + bind: bind.to_string(), + ..port(host, container) + } + } + + #[test] + fn bind_drift_detected_when_declared_loopback_but_published_wildcard() { + // The legacy-container case: a pre-quadlet container still publishes + // 0.0.0.0 while the catalog-delivered manifest pins the app to + // loopback for the app gate. Must recreate, or the port stays open on + // every interface and the gate can never claim it. + for wildcard in [r#""""#, r#""0.0.0.0""#] { + let bindings = format!(r#"{{"80/tcp":[{{"HostIp":{wildcard},"HostPort":"8082"}}]}}"#); + assert!(host_port_bindings_drifted( + &bindings, + &[bound_port(8082, 80, "127.0.0.1")] + )); + } + } + + #[test] + fn no_bind_drift_when_declared_loopback_and_published_loopback() { + let bindings = r#"{"80/tcp":[{"HostIp":"127.0.0.1","HostPort":"8082"}]}"#; + assert!(!host_port_bindings_drifted( + bindings, + &[bound_port(8082, 80, "127.0.0.1")] + )); + } + + #[test] + fn no_bind_drift_on_undeclared_bind() { + // Silence is not consent (v1.7.121 incident class): an EMPTY manifest + // bind must never recreate a loopback-published container to + // wildcard — that is how Bitcoin's RPC got republished on the LAN. + let bindings = r#"{"8332/tcp":[{"HostIp":"127.0.0.1","HostPort":"8332"}]}"#; + assert!(!host_port_bindings_drifted(bindings, &[port(8332, 8332)])); + } + + #[test] + fn multi_bind_publish_satisfies_each_declared_entry() { + // Same host/container pair listed twice (loopback + archy-net + // gateway): both declared binds are present in the actual publish. + let bindings = r#"{"8332/tcp":[ + {"HostIp":"127.0.0.1","HostPort":"8332"}, + {"HostIp":"10.89.0.1","HostPort":"8332"} + ]}"#; + assert!(!host_port_bindings_drifted( + bindings, + &[ + bound_port(8332, 8332, "127.0.0.1"), + bound_port(8332, 8332, "10.89.0.1") + ] + )); + // And a wildcard-only publish drifts BOTH declared entries. + let wildcard = r#"{"8332/tcp":[{"HostIp":"","HostPort":"8332"}]}"#; + assert!(host_port_bindings_drifted( + wildcard, + &[ + bound_port(8332, 8332, "127.0.0.1"), + bound_port(8332, 8332, "10.89.0.1") + ] + )); + } + #[test] fn missing_secret_error_names_the_secret() { use archipelago_container::manifest::SecretsProvider;