fix(portainer): repair same-node Git routing with recoverable network migration
This commit is contained in:
@@ -1746,40 +1746,28 @@ app:
|
||||
}
|
||||
}
|
||||
exempt.sort();
|
||||
// 28 as of 2026-08-23: the 26 below plus cuprate's two exemptions —
|
||||
// 18183 (Monero p2p gossip, same reasoning as bitcoin's 8333) and
|
||||
// 18090 (host mapping for Monero's canonical 18089 restricted RPC,
|
||||
// upstream's own safe-for-public
|
||||
// subset that wallets connect to directly as a "remote node" over
|
||||
// plain HTTP JSON-RPC — same reasoning as electrumx's 50001).
|
||||
// cuprate's unrestricted RPC (full node control) stays loopback-only
|
||||
// (auth: local), not in this set.
|
||||
//
|
||||
// 26 as of 2026-08-16: the 25 below plus phoenixd 9740, a
|
||||
// loopback-only JSON API whose own generated http password
|
||||
// authenticates every request (added with the phoenixd onboarding,
|
||||
// which did not update this count — exactly the drift this test
|
||||
// exists to catch).
|
||||
//
|
||||
// 25 as of the v1.7.123 port-policy round: bitcoin p2p (8333 ×2),
|
||||
// core-lightning 9736/9835, electrumx 50001, fedimint 8173/8174,
|
||||
// fedimint-gateway 8176/9737, gitea ssh 2222, lightning-stack
|
||||
// 8091/9738/10010, lnd 9735/10009/18080, netbird 3478/8086/8087,
|
||||
// pine TLS 10381 + the three voice ports (10200/10300/10400 — the
|
||||
// disclosed known gap), router SSDP/mDNS 1900/5353. Every one is a
|
||||
// deliberate, rationale-carrying exemption; the release-gate test
|
||||
// stage timed out that cycle, so the count here lagged at 17.
|
||||
assert_eq!(
|
||||
exempt.len(),
|
||||
28,
|
||||
"unauthenticated port set changed — review before updating this count: {exempt:?}"
|
||||
);
|
||||
// Reviewed 2026-09-30: lightning-stack's three retired endpoints
|
||||
// disappeared; Cuprate restricted RPC moved from none to gate-open.
|
||||
// Compare exact endpoints, not just a count that can hide substitutions.
|
||||
let expected = [
|
||||
("bitcoin-core", 8333), ("bitcoin-knots", 8333),
|
||||
("core-lightning", 9736), ("core-lightning", 9835),
|
||||
("cuprate", 18183), ("electrumx", 50001),
|
||||
("fedimint", 8173), ("fedimint", 8174),
|
||||
("fedimint-gateway", 8176), ("fedimint-gateway", 9737),
|
||||
("gitea", 2222), ("lnd", 9735), ("lnd", 10009), ("lnd", 18080),
|
||||
("netbird", 8087), ("netbird-server", 3478), ("netbird-server", 8086),
|
||||
("phoenixd", 9740), ("pine", 10381), ("pine-openwakeword", 10400),
|
||||
("pine-piper", 10200), ("pine-whisper", 10300),
|
||||
("router", 1900), ("router", 5353),
|
||||
].into_iter().map(|(id, port)| (id.to_owned(), port)).collect::<Vec<_>>();
|
||||
assert_eq!(exempt, expected, "unauthenticated endpoint set changed; review each exemption");
|
||||
}
|
||||
|
||||
/// `auth: open` ports are served by the gate WITHOUT its login challenge,
|
||||
/// so they are the second unauthenticated-by-the-gate surface and get the
|
||||
/// same review guard as `auth: none`. Each one must be an app that
|
||||
/// enforces a real login of its own.
|
||||
/// same review guard as `auth: none`. Each must enforce its own login or
|
||||
/// have an explicitly reviewed public protocol purpose.
|
||||
#[test]
|
||||
fn gate_open_ports_are_all_accounted_for() {
|
||||
let apps = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../apps");
|
||||
@@ -1801,6 +1789,8 @@ app:
|
||||
}
|
||||
}
|
||||
open.sort();
|
||||
// Cuprate 18090 is its deliberately public restricted RPC subset;
|
||||
// unrestricted node-control RPC remains container-loopback-only.
|
||||
// Gitea 3001 (git clients speak basic-auth, not browser cookies),
|
||||
// BTCPay 23000 (checkout/invoice/webhook endpoints must be reachable
|
||||
// by anonymous payers), and — since the v1.8.7 platform round — the
|
||||
@@ -1812,11 +1802,12 @@ app:
|
||||
open,
|
||||
vec![
|
||||
("btcpay-server".to_string(), 23000u16),
|
||||
("cuprate".to_string(), 18090u16),
|
||||
("gitea".to_string(), 3001u16),
|
||||
("nginx-proxy-manager".to_string(), 8081u16),
|
||||
("tailscale".to_string(), 8240u16),
|
||||
],
|
||||
"gate-open port set changed — every entry must be an app with its own login"
|
||||
"gate-open port set changed — review login or intentional public protocol purpose"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -310,59 +310,7 @@ impl PodmanClient {
|
||||
);
|
||||
continue;
|
||||
}
|
||||
// Honour the manifest's protocol (default tcp). netbird's STUN port
|
||||
// is 3478/udp; forcing tcp here would publish the wrong protocol and
|
||||
// silently break relay discovery.
|
||||
let protocol = match port.protocol.to_ascii_lowercase().as_str() {
|
||||
"udp" => "udp",
|
||||
"sctp" => "sctp",
|
||||
_ => "tcp",
|
||||
};
|
||||
// Effective bind. A gated port with no declared bind would
|
||||
// publish 0.0.0.0 — the app would own every host address, which
|
||||
// is both the exposure itself and the reason the daemon's app
|
||||
// gate cannot bind those addresses to authenticate them. Pin it
|
||||
// to loopback so the gate can take the external addresses.
|
||||
//
|
||||
// Doing it HERE, at container creation, is the point: the pin and
|
||||
// the gate's takeover then both come from the daemon and cannot
|
||||
// disagree. The earlier attempt put this decision in manifest
|
||||
// data instead, and a node whose manifests lagged the binary
|
||||
// published Bitcoin's loopback-only RPC across the LAN
|
||||
// (test node, 2026-08-03).
|
||||
//
|
||||
// A port that already declares a bind is never overridden — that
|
||||
// is exactly what keeps `bind: 127.0.0.1` ports host-local and
|
||||
// leaves `auth: none` protocol ports (LND gRPC/REST, electrum)
|
||||
// published as they are, so remote wallets keep working.
|
||||
// NOTE: the daemon deliberately does NOT rewrite this. Pinning a
|
||||
// published port to loopback is how an app hands its external
|
||||
// addresses to the gate, but it belongs in the manifest, not in
|
||||
// daemon-side inference:
|
||||
//
|
||||
// * `bind` is already honoured by every publish path (here and
|
||||
// in package::install), so a manifest edit needs no code.
|
||||
// * inference here would cover only THIS path — proven on
|
||||
// a test node, where a recreate went through another one and
|
||||
// the pin never applied.
|
||||
// * and inferring from an ABSENT field is what republished
|
||||
// Bitcoin's loopback RPC across the LAN, and came within one
|
||||
// container-recreate of pinning LND's gRPC/REST and breaking
|
||||
// every remote wallet.
|
||||
//
|
||||
// So the migration ships as `bind: 127.0.0.1` in the signed
|
||||
// catalog. Verified 2026-08-03 that a disk-only manifest edit is
|
||||
// overridden by the catalog, which is precisely why the catalog is
|
||||
// the right and only place to carry it.
|
||||
let mut mapping = serde_json::json!({
|
||||
"container_port": port.container,
|
||||
"host_port": port.host,
|
||||
"protocol": protocol,
|
||||
});
|
||||
if !port.bind.is_empty() {
|
||||
mapping["host_ip"] = serde_json::json!(port.bind);
|
||||
}
|
||||
port_mappings.push(mapping);
|
||||
port_mappings.push(podman_publish_mapping(port));
|
||||
}
|
||||
|
||||
let mut mounts = Vec::new();
|
||||
@@ -751,6 +699,25 @@ pub fn image_uses_insecure_registry(image: &str) -> bool {
|
||||
.is_some_and(|host| INSECURE_REGISTRY_HOSTS.contains(&host))
|
||||
}
|
||||
|
||||
// Keep the explicitly declared bind and transport identical to Quadlet. The
|
||||
// app gate owns external listeners; container publication must not bypass it.
|
||||
fn podman_publish_mapping(port: &crate::manifest::PortMapping) -> serde_json::Value {
|
||||
let protocol = match port.protocol.to_ascii_lowercase().as_str() {
|
||||
"udp" => "udp",
|
||||
"sctp" => "sctp",
|
||||
_ => "tcp",
|
||||
};
|
||||
let mut mapping = serde_json::json!({
|
||||
"container_port": port.container,
|
||||
"host_port": port.host,
|
||||
"protocol": protocol,
|
||||
});
|
||||
if !port.bind.is_empty() {
|
||||
mapping["host_ip"] = serde_json::json!(port.bind);
|
||||
}
|
||||
mapping
|
||||
}
|
||||
|
||||
fn podman_network_settings(
|
||||
network: Option<&str>,
|
||||
network_policy: &str,
|
||||
@@ -1110,6 +1077,15 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn portainer_manifest_keeps_private_network_and_loopback_api_publication() {
|
||||
let m = AppManifest::parse(include_str!("../../../apps/portainer/manifest.yml")).unwrap();
|
||||
assert_eq!(podman_network_settings(m.app.container.network.as_deref(), &m.app.security.network_policy), ("slirp4netns", None));
|
||||
assert_eq!(podman_publish_mapping(&m.app.ports[0]), serde_json::json!({
|
||||
"container_port": 9000, "host_port": 9000, "protocol": "tcp", "host_ip": "127.0.0.1"
|
||||
}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn podman_network_settings_uses_networks_map_for_custom_networks() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -618,6 +618,28 @@ impl DockerRuntime {
|
||||
}
|
||||
}
|
||||
|
||||
// Docker is a development fallback. Refuse Podman-only network modes instead
|
||||
// of silently installing a different topology; still honor binds for other apps.
|
||||
fn docker_network_and_ports(manifest: &AppManifest, offset: u16) -> Result<Vec<String>> {
|
||||
let network = manifest.app.container.network.as_deref()
|
||||
.filter(|v| !v.is_empty())
|
||||
.unwrap_or(&manifest.app.security.network_policy);
|
||||
if matches!(network, "slirp4netns" | "pasta") {
|
||||
anyhow::bail!("this app requires rootless Podman networking ({network})");
|
||||
}
|
||||
let mut args = Vec::new();
|
||||
if !network.is_empty() && network != "isolated" {
|
||||
args.extend(["--network".to_owned(), network.to_owned()]);
|
||||
}
|
||||
for port in &manifest.app.ports {
|
||||
let host = port.host.checked_add(offset).context("published port offset overflow")?;
|
||||
let bind = if port.bind.is_empty() { String::new() } else { format!("{}:", port.bind) };
|
||||
let protocol = if port.protocol.is_empty() { "tcp" } else { &port.protocol };
|
||||
args.extend(["-p".to_owned(), format!("{bind}{host}:{}/{protocol}", port.container)]);
|
||||
}
|
||||
Ok(args)
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl ContainerRuntime for DockerRuntime {
|
||||
async fn pull_image(&self, image: &str, signature: Option<&str>) -> Result<()> {
|
||||
@@ -657,25 +679,7 @@ impl ContainerRuntime for DockerRuntime {
|
||||
cmd.arg("--read-only");
|
||||
}
|
||||
|
||||
match manifest.app.security.network_policy.as_str() {
|
||||
"host" => {
|
||||
cmd.arg("--network").arg("host");
|
||||
}
|
||||
"isolated" => {
|
||||
// Docker uses bridge network by default
|
||||
}
|
||||
_ => {
|
||||
cmd.arg("--network")
|
||||
.arg(&manifest.app.security.network_policy);
|
||||
}
|
||||
}
|
||||
|
||||
// Port mappings with offset
|
||||
for port in &manifest.app.ports {
|
||||
let host_port = port.host + port_offset;
|
||||
cmd.arg("-p")
|
||||
.arg(format!("{}:{}", host_port, port.container));
|
||||
}
|
||||
cmd.args(docker_network_and_ports(manifest, port_offset)?);
|
||||
|
||||
// Volumes
|
||||
for volume in &manifest.app.volumes {
|
||||
@@ -1035,6 +1039,17 @@ mod tests {
|
||||
use super::*;
|
||||
use std::collections::HashMap;
|
||||
|
||||
#[test]
|
||||
fn docker_fallback_rejects_rootless_only_topology_and_preserves_bind_protocol() {
|
||||
let mut m = AppManifest::parse(include_str!("../../../apps/portainer/manifest.yml")).unwrap();
|
||||
assert!(docker_network_and_ports(&m, 0).is_err());
|
||||
m.app.container.network = Some("bridge".into());
|
||||
m.app.ports[0].protocol = "udp".into();
|
||||
let args = docker_network_and_ports(&m, 1).unwrap();
|
||||
assert_eq!(args, vec!["--network", "bridge", "-p", "127.0.0.1:9001:9000/udp"]);
|
||||
assert!(docker_network_and_ports(&m, u16::MAX).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_container_classifier_covers_podman5_phrasings() {
|
||||
// podman 5.x `inspect` phrasing for a missing container.
|
||||
|
||||
Reference in New Issue
Block a user