From bcca4cdf0e1fc1322f350873d6800e7153aeb2f1 Mon Sep 17 00:00:00 2001 From: Rafa Cardenas <253999660+rafa-stacks@users.noreply.github.com> Date: Sat, 15 Aug 2026 12:10:22 -0600 Subject: [PATCH 1/2] feat: start preflight guards (port bind test, project-name collision) + external-node event-port warnings --- README.md | 6 +++ src/config/mod.rs | 27 +++++++++- src/utils/docker.rs | 116 ++++++++++++++++++++++++++++++++++++++++++ src/utils/services.rs | 31 +++++++++++ 4 files changed, 178 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 29590ea..31fab6b 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,12 @@ name, so `stacksup` commands (and bare `docker compose -f` runs) are always scoped to the deployment whose directory you're in — `stop`, `logs`, and `chainstate wipe` can't touch a neighbour. +`stacksup start` refuses to run before doing damage when it detects a +collision: it test-binds every port it is about to publish (pointing at +`port_offset` when one is taken) and rejects a `name` already in use by a +stack rendered from a different directory (which compose would otherwise +silently adopt). + ## Secrets Credentials never live in `stacks.toml` — the tool rejects them there. They go diff --git a/src/config/mod.rs b/src/config/mod.rs index 1359616..2e0f74e 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -399,7 +399,10 @@ impl Deployment { ServiceMode::External => { warnings.push( "signer is managed but the node is external: apply `rendered/apply-to-your-node.toml` \ - to your node (stacker = true, matching auth_password, signer events_observer)" + to your node (stacker = true, matching auth_password, signer events_observer). \ + Note: the signer's event endpoint (port 30000) is not published on this host — \ + an off-host node cannot push to it; run the node on this machine or expose the \ + port yourself (compose override)" .into(), ); } @@ -414,7 +417,9 @@ impl Deployment { { warnings.push( "stacks-api is managed but the node is external: add the [[events_observer]] block \ - from `rendered/apply-to-your-node.toml` to your node config, then verify with `stacksup config check`" + from `rendered/apply-to-your-node.toml` to your node config, then verify with `stacksup config check`. \ + Note: the API's event port (3700) is not published on this host — an off-host node \ + cannot push to it; run the node on this machine or expose the port yourself (compose override)" .into(), ); } @@ -676,6 +681,24 @@ mod tests { assert!(w.iter().any(|m| m.contains("apply-to-your-node"))); } + #[test] + fn external_node_with_managed_receivers_warns_about_unpublished_ports() { + let w = warnings( + "network = \"testnet\"\n[stacks-node]\nmode = \"external\"\nrpc_host = \"h\"\n[stacks-api]\nmode = \"enabled\"\n[postgres]\nmode = \"enabled\"", + ); + assert!( + w.iter().any(|m| m.contains("event port (3700)")), + "got: {w:?}" + ); + let w = warnings( + "network = \"testnet\"\n[stacks-node]\nmode = \"external\"\nrpc_host = \"h\"\n[stacks-signer]\nmode = \"enabled\"", + ); + assert!( + w.iter().any(|m| m.contains("event endpoint (port 30000)")), + "got: {w:?}" + ); + } + #[test] fn deployment_name_and_offset_are_validated() { let e = errors("name = \"Bad_Name\"\nnetwork = \"testnet\""); diff --git a/src/utils/docker.rs b/src/utils/docker.rs index 068abf7..d090f62 100644 --- a/src/utils/docker.rs +++ b/src/utils/docker.rs @@ -60,6 +60,80 @@ fn ensure_docker() -> Result<()> { Ok(()) } +/// Refuse to start when this deployment's name is already in use by a compose +/// project rendered from a DIFFERENT directory — otherwise compose silently +/// adopts (and replaces) the other deployment's containers. +fn guard_project_collision(deployment: &Deployment, data_dir: &Path) -> Result<()> { + let out = Command::new("docker") + .args(["compose", "ls", "--all", "--format", "json"]) + .output(); + let Ok(out) = out else { return Ok(()) }; // best effort + let listing = String::from_utf8_lossy(&out.stdout); + if let Some(other) = project_conflict(&listing, deployment.project(), &compose_file(data_dir)) { + bail!( + "deployment name `{}` is already in use by a stack rendered from {other} — set a \ + distinct `name` in stacks.toml (compose would otherwise adopt that stack's containers)", + deployment.project() + ); + } + Ok(()) +} + +/// Pure half of the collision check: does `ls_json` (docker compose ls +/// output) contain `project` with a config file other than ours? +fn project_conflict(ls_json: &str, project: &str, our_compose_file: &Path) -> Option { + let ours = our_compose_file + .canonicalize() + .unwrap_or_else(|_| our_compose_file.to_path_buf()); + let entries: Vec = serde_json::from_str(ls_json).ok()?; + for e in entries { + if e["Name"].as_str() != Some(project) { + continue; + } + // ConfigFiles is comma-separated; any path matching ours means it's us. + let files = e["ConfigFiles"].as_str().unwrap_or_default(); + let is_ours = files.split(',').map(str::trim).any(|f| { + Path::new(f) + .canonicalize() + .map(|p| p == ours) + .unwrap_or(f == ours.to_string_lossy()) + }); + if !is_ours && !files.is_empty() { + return Some(files.to_string()); + } + } + None +} + +/// Test-bind every host port the deployment is about to publish, skipping +/// services that are already running (their ports are legitimately ours). +/// Catches port squatting BEFORE compose creates half a stack. +fn guard_published_ports( + deployment: &Deployment, + data_dir: &Path, + only_service: Option<&str>, +) -> Result<()> { + let running = running_services(data_dir).unwrap_or_default(); + for (service, port) in crate::utils::services::published_ports(deployment) { + if running.iter().any(|r| r == service) { + continue; + } + if let Some(only) = only_service + && only != service + { + continue; + } + if std::net::TcpListener::bind(("0.0.0.0", port)).is_err() { + bail!( + "host port {port} (published by {service}) is already in use — another deployment \ + or process owns it; set or raise `port_offset` in stacks.toml, or stop \ + whatever holds the port" + ); + } + } + Ok(()) +} + /// Names of this stack's currently running compose services. Best effort: /// `None` when docker or the rendered compose file is unavailable. pub fn running_services(data_dir: &Path) -> Option> { @@ -165,6 +239,9 @@ pub fn start(deployment: &Deployment, data_dir: &Path, service: Option<&str>) -> ); } + guard_project_collision(deployment, data_dir)?; + guard_published_ports(deployment, data_dir, service)?; + if let Some(name) = service { ensure_enabled(deployment, name)?; println!("Starting {name} (and its dependencies)..."); @@ -366,6 +443,45 @@ mod tests { assert!(args[2].starts_with("/data")); } + #[test] + fn project_conflict_flags_other_directories_only() { + let ours = Path::new("/data/rendered/docker-compose.yml"); + let ls = r#"[ + {"Name": "stacks", "Status": "running(2)", "ConfigFiles": "/other/rendered/docker-compose.yml"}, + {"Name": "testnet-b", "Status": "running(1)", "ConfigFiles": "/data/rendered/docker-compose.yml"} + ]"#; + // same name, different directory -> conflict + assert_eq!( + project_conflict(ls, "stacks", ours).as_deref(), + Some("/other/rendered/docker-compose.yml") + ); + // same name, same file -> that's us, no conflict + assert!(project_conflict(ls, "testnet-b", ours).is_none()); + // name not present -> no conflict + assert!(project_conflict(ls, "mainnet-c", ours).is_none()); + // garbage json -> best effort, no conflict + assert!(project_conflict("not json", "stacks", ours).is_none()); + } + + #[test] + fn guard_published_ports_reports_taken_port() { + // hold a port, then ask the guard about a deployment that publishes it + let listener = std::net::TcpListener::bind(("0.0.0.0", 0)).unwrap(); + let taken = listener.local_addr().unwrap().port(); + let mut d = deployment("network = \"testnet\"\n[postgres]\nmode = \"enabled\""); + // shift postgres (5432) onto the taken port + d.port_offset = taken - crate::utils::services::POSTGRES_PORT; + // data_dir without a compose file -> no services considered running + let err = guard_published_ports(&d, Path::new("/nonexistent"), None) + .unwrap_err() + .to_string(); + assert!(err.contains(&format!("host port {taken}")), "got: {err}"); + assert!(err.contains("port_offset")); + // a free port passes + drop(listener); + assert!(guard_published_ports(&d, Path::new("/nonexistent"), None).is_ok()); + } + #[test] fn ensure_enabled_accepts_enabled_services() { let s = deployment("network = \"testnet\"\n[postgres]\nmode = \"enabled\""); diff --git a/src/utils/services.rs b/src/utils/services.rs index a83db88..8abe82f 100644 --- a/src/utils/services.rs +++ b/src/utils/services.rs @@ -142,6 +142,37 @@ pub fn roster(deployment: &Deployment) -> Vec<(&'static str, ServiceMode)> { ] } +/// Every (service, host port) this deployment publishes when started — +/// the base ports shifted by `port_offset`. Used by the start preflight to +/// test-bind before compose does. +pub fn published_ports(deployment: &Deployment) -> Vec<(&'static str, u16)> { + let mut ports = Vec::new(); + if deployment.bitcoind.mode == ServiceMode::Enabled { + ports.push(( + "bitcoind", + deployment.published(bitcoind_rpc_port(deployment)), + )); + ports.push(( + "bitcoind", + deployment.published(bitcoind_p2p_port(deployment)), + )); + } + if deployment.stacks_node.mode == ServiceMode::Enabled { + ports.push(("stacks-node", deployment.published(NODE_RPC_PORT))); + ports.push(("stacks-node", deployment.published(NODE_P2P_PORT))); + } + if deployment.stacks_api.mode == ServiceMode::Enabled { + ports.push(("stacks-api", deployment.published(API_PORT))); + } + if deployment.stacks_mesh_api.mode == ServiceMode::Enabled { + ports.push(("stacks-mesh-api", deployment.published(MESH_API_PORT))); + } + if deployment.postgres.mode == ServiceMode::Enabled { + ports.push(("postgres", deployment.published(POSTGRES_PORT))); + } + ports +} + #[cfg(test)] mod tests { use super::*; From 1e02299c42566f4c1e68a874e058886ab9ad3653 Mon Sep 17 00:00:00 2001 From: Rafa Cardenas <253999660+rafa-stacks@users.noreply.github.com> Date: Sat, 15 Aug 2026 12:21:59 -0600 Subject: [PATCH 2/2] review: dual-stack port check, typed compose ls parse, underflow-proof test --- src/utils/docker.rs | 50 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/src/utils/docker.rs b/src/utils/docker.rs index d090f62..727d204 100644 --- a/src/utils/docker.rs +++ b/src/utils/docker.rs @@ -79,32 +79,53 @@ fn guard_project_collision(deployment: &Deployment, data_dir: &Path) -> Result<( Ok(()) } +/// One row of `docker compose ls --format json`. +#[derive(serde::Deserialize)] +struct ComposeLsEntry { + #[serde(rename = "Name")] + name: String, + /// Comma-separated compose file paths. + #[serde(rename = "ConfigFiles", default)] + config_files: String, +} + /// Pure half of the collision check: does `ls_json` (docker compose ls /// output) contain `project` with a config file other than ours? fn project_conflict(ls_json: &str, project: &str, our_compose_file: &Path) -> Option { let ours = our_compose_file .canonicalize() .unwrap_or_else(|_| our_compose_file.to_path_buf()); - let entries: Vec = serde_json::from_str(ls_json).ok()?; + let entries: Vec = serde_json::from_str(ls_json).ok()?; for e in entries { - if e["Name"].as_str() != Some(project) { + if e.name != project { continue; } - // ConfigFiles is comma-separated; any path matching ours means it's us. - let files = e["ConfigFiles"].as_str().unwrap_or_default(); - let is_ours = files.split(',').map(str::trim).any(|f| { + // Any path matching ours means it's us. + let is_ours = e.config_files.split(',').map(str::trim).any(|f| { Path::new(f) .canonicalize() .map(|p| p == ours) .unwrap_or(f == ours.to_string_lossy()) }); - if !is_ours && !files.is_empty() { - return Some(files.to_string()); + if !is_ours && !e.config_files.is_empty() { + return Some(e.config_files); } } None } +/// A port conflicts only when a bind fails with AddrInUse on SOME family — +/// an unsupported family (e.g. no IPv6) or a permission error is not a +/// conflict. Mirrors how docker publishes on both stacks. +fn port_taken(port: u16) -> bool { + ["0.0.0.0", "::"] + .iter() + .any(|ip| match std::net::TcpListener::bind((*ip, port)) { + Ok(_) => false, + Err(e) => e.kind() == std::io::ErrorKind::AddrInUse, + }) +} + /// Test-bind every host port the deployment is about to publish, skipping /// services that are already running (their ports are legitimately ours). /// Catches port squatting BEFORE compose creates half a stack. @@ -123,7 +144,7 @@ fn guard_published_ports( { continue; } - if std::net::TcpListener::bind(("0.0.0.0", port)).is_err() { + if port_taken(port) { bail!( "host port {port} (published by {service}) is already in use — another deployment \ or process owns it; set or raise `port_offset` in stacks.toml, or stop \ @@ -465,9 +486,16 @@ mod tests { #[test] fn guard_published_ports_reports_taken_port() { - // hold a port, then ask the guard about a deployment that publishes it - let listener = std::net::TcpListener::bind(("0.0.0.0", 0)).unwrap(); - let taken = listener.local_addr().unwrap().port(); + // hold a port, then ask the guard about a deployment that publishes + // it. Ephemeral ports are virtually always >= 32768, but re-roll to + // guarantee `taken - POSTGRES_PORT` can't underflow anywhere. + let (listener, taken) = loop { + let l = std::net::TcpListener::bind(("0.0.0.0", 0)).unwrap(); + let p = l.local_addr().unwrap().port(); + if p >= crate::utils::services::POSTGRES_PORT { + break (l, p); + } + }; let mut d = deployment("network = \"testnet\"\n[postgres]\nmode = \"enabled\""); // shift postgres (5432) onto the taken port d.port_offset = taken - crate::utils::services::POSTGRES_PORT;