From 39818468aa0ada57820d0aee2b1b427513bd0ae6 Mon Sep 17 00:00:00 2001 From: Repin Agent Date: Fri, 17 Jul 2026 04:07:24 -0600 Subject: [PATCH 1/2] fix(cli): single-UDP-transport startup guard (M6 CP6) Two transport-wiring passes disagree if more than one runs: the channel driver keeps the LAST transport (set_transport is last-wins) while the global event handler keeps the FIRST (set_global_event_handler is a first-wins OnceLock), so two UDP transports would bind different sockets and race (inbound handled on one, outbound sent on the other). Add a startup guard, single_udp_transport(), that asserts exactly one UDP bind and fails startup fast (Err from startup_sequence) rather than binding two. Two identical binds deduped to one still pass; two distinct binds are rejected. Tests: single_udp_transport accepts exactly one, rejects two distinct, rejects empty. RED: make the guard return Ok for >1 -> the reject-two test fails (accepts 0.0.0.0:5060). --- crates/rustisk-cli/src/main.rs | 66 +++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/crates/rustisk-cli/src/main.rs b/crates/rustisk-cli/src/main.rs index 2efbdec..2a3e12a 100644 --- a/crates/rustisk-cli/src/main.rs +++ b/crates/rustisk-cli/src/main.rs @@ -1928,6 +1928,34 @@ fn load_pjsip_notify_config(content: &str, config: &asterisk_sip::notify::Notify } } +/// Single-UDP-transport startup guard (M6 CP6). +/// +/// rustisk wires the SIP transport through two passes that DISAGREE if more than +/// one runs: the channel driver keeps the LAST transport it is handed +/// (`set_transport` is last-wins) while the global event handler keeps the FIRST +/// (`set_global_event_handler` is a first-wins `OnceLock`). With two transports, +/// inbound would be handled on the first socket while outbound is sent on the +/// second — a silent split-brain / race. rustisk supports exactly ONE UDP SIP +/// transport, so assert that at startup and fail fast rather than binding two. +/// +/// Returns the single bind address, or an `Err` describing the misconfiguration. +fn single_udp_transport(addrs: &[SocketAddr]) -> Result { + match addrs { + [one] => Ok(*one), + [] => Err("no UDP SIP transport to bind (empty transport set)".to_string()), + many => Err(format!( + "{} UDP SIP transports configured ({}); rustisk supports exactly one — the channel \ + driver keeps the last and the event handler the first, so two would race. Configure a \ + single udp transport.", + many.len(), + many.iter() + .map(|a| a.to_string()) + .collect::>() + .join(", ") + )), + } +} + /// Perform the startup sequence. async fn startup_sequence(config_dir: &str, dirs: &AsteriskDirs) -> Result<(), String> { info!("Loading configuration from: {}", config_dir); @@ -2137,7 +2165,12 @@ async fn startup_sequence(config_dir: &str, dirs: &AsteriskDirs) -> Result<(), S } } - for bind_addr in &unique_addrs { + // Single-UDP-transport startup guard (M6 CP6): reject a config that would + // bind two racing transports. Two DISTINCT udp binds fail fast here; two + // identical ones deduped to a single addr above pass. + let sip_bind_addr = single_udp_transport(&unique_addrs)?; + + for bind_addr in std::slice::from_ref(&sip_bind_addr) { match asterisk_sip::stack::SipStack::new(*bind_addr).await { Ok(mut sip_stack) => { info!("SIP stack bound to {}", bind_addr); @@ -2625,6 +2658,37 @@ async fn main() { mod tests { use super::*; + fn sa(s: &str) -> SocketAddr { + s.parse().unwrap() + } + + /// M6 CP6: exactly one UDP transport is accepted; the single bind is + /// returned unchanged. + #[test] + fn single_udp_transport_accepts_exactly_one() { + assert_eq!( + single_udp_transport(&[sa("0.0.0.0:5060")]).unwrap(), + sa("0.0.0.0:5060") + ); + } + + /// M6 CP6: two DISTINCT UDP transports (what a two-`type=transport-udp` + /// config produces after de-dup) are REJECTED at startup — the guard fails + /// fast rather than binding two racing transports. RED: remove the guard + /// (or make it return `Ok` for `>1`) and this assertion fails. + #[test] + fn single_udp_transport_rejects_two() { + let err = single_udp_transport(&[sa("0.0.0.0:5060"), sa("0.0.0.0:5062")]) + .expect_err("two distinct UDP transports must be rejected"); + assert!(err.contains("exactly one"), "error must explain the single-transport rule: {err}"); + } + + /// M6 CP6: an empty transport set is rejected (nothing to bind). + #[test] + fn single_udp_transport_rejects_empty() { + assert!(single_udp_transport(&[]).is_err()); + } + /// Create a unique empty temp directory for a test. fn test_dir(tag: &str) -> std::path::PathBuf { let dir = std::env::temp_dir().join(format!( From 4ba563b414fe7d2a6351fea7e9542c53f940120a Mon Sep 17 00:00:00 2001 From: Repin Agent Date: Fri, 17 Jul 2026 04:15:07 -0600 Subject: [PATCH 2/2] test(cli): pin the CP6 guard through the startup entry point (codex P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract udp_transport_binds (collect + de-dup) and resolve_single_udp_bind (binds + guard), and route startup through resolve_single_udp_bind — the single path startup uses. Config-level tests now exercise that exact entry point: two DISTINCT udp transports are rejected, two IDENTICAL dedup to one and are accepted (pinning dedup-before-guard), and no-config falls back to the default. So the tests fail RED if the guard is removed from the startup path or dedup is reordered after it, not only if the helper itself is changed. --- crates/rustisk-cli/src/main.rs | 159 +++++++++++++++++++++++++-------- 1 file changed, 121 insertions(+), 38 deletions(-) diff --git a/crates/rustisk-cli/src/main.rs b/crates/rustisk-cli/src/main.rs index 2a3e12a..dde1743 100644 --- a/crates/rustisk-cli/src/main.rs +++ b/crates/rustisk-cli/src/main.rs @@ -1928,6 +1928,57 @@ fn load_pjsip_notify_config(content: &str, config: &asterisk_sip::notify::Notify } } +/// Collect the de-duplicated set of UDP SIP bind addresses from the pjsip +/// config, falling back to `default_bind` when no transports are configured. +/// Non-UDP transports (tls/tcp/ws) are not yet supported and are skipped. +/// +/// Extracted so the collection + de-dup + [`single_udp_transport`] guard chain +/// is testable at the config level (codex CP6 P2): two DISTINCT udp sections +/// yield two binds (rejected by the guard), while two IDENTICAL sections dedup +/// to one (accepted) — behaviour a direct-helper test alone would not pin. +fn udp_transport_binds( + pjsip_config: Option<&asterisk_sip::pjsip_config::PjsipConfig>, + default_bind: SocketAddr, +) -> Vec { + let collected: Vec = match pjsip_config { + Some(cfg) if !cfg.transports.is_empty() => { + let mut addrs = Vec::new(); + for t in &cfg.transports { + if t.protocol.as_str() == "udp" { + addrs.push(t.bind); + } else { + info!("Transport {} ({}) not yet supported, skipping", t.name, t.protocol); + } + } + if addrs.is_empty() { + addrs.push(default_bind); + } + addrs + } + _ => vec![default_bind], + }; + // De-duplicate, preserving order. + let mut unique = Vec::new(); + for addr in collected { + if !unique.contains(&addr) { + unique.push(addr); + } + } + unique +} + +/// Resolve the ONE UDP SIP bind to start (M6 CP6): collect + de-dup the +/// configured UDP transports, then enforce the single-transport guard. This is +/// the single entry point `startup_sequence` calls, so a test on it pins the +/// whole chain — removing the guard here (the only path startup uses) fails the +/// rejection tests RED. +fn resolve_single_udp_bind( + pjsip_config: Option<&asterisk_sip::pjsip_config::PjsipConfig>, + default_bind: SocketAddr, +) -> Result { + single_udp_transport(&udp_transport_binds(pjsip_config, default_bind)) +} + /// Single-UDP-transport startup guard (M6 CP6). /// /// rustisk wires the SIP transport through two passes that DISAGREE if more than @@ -2131,44 +2182,11 @@ async fn startup_sequence(config_dir: &str, dirs: &AsteriskDirs) -> Result<(), S TECH_REGISTRY.list() ); - // Start a SIP stack for each configured transport (or the default UDP) - let transports_to_start: Vec = if let Some(ref cfg) = pjsip_config { - if cfg.transports.is_empty() { - vec![sip_bind] - } else { - let mut addrs = Vec::new(); - for t in &cfg.transports { - match t.protocol.as_str() { - "udp" => addrs.push(t.bind), - other => { - info!( - "Transport {} ({}) not yet supported, skipping", - t.name, other - ); - } - } - } - if addrs.is_empty() { - addrs.push(sip_bind); - } - addrs - } - } else { - vec![sip_bind] - }; - - // De-duplicate bind addresses - let mut unique_addrs: Vec = Vec::new(); - for addr in &transports_to_start { - if !unique_addrs.contains(addr) { - unique_addrs.push(*addr); - } - } - - // Single-UDP-transport startup guard (M6 CP6): reject a config that would - // bind two racing transports. Two DISTINCT udp binds fail fast here; two - // identical ones deduped to a single addr above pass. - let sip_bind_addr = single_udp_transport(&unique_addrs)?; + // Single-UDP-transport startup guard (M6 CP6): collect + de-dup the + // configured UDP binds and reject a config that would bind two racing + // transports. Two DISTINCT udp binds fail fast here; two identical ones + // dedup to a single addr and pass. + let sip_bind_addr = resolve_single_udp_bind(pjsip_config.as_ref(), sip_bind)?; for bind_addr in std::slice::from_ref(&sip_bind_addr) { match asterisk_sip::stack::SipStack::new(*bind_addr).await { @@ -2689,6 +2707,71 @@ mod tests { assert!(single_udp_transport(&[]).is_err()); } + fn udp_transport(name: &str, bind: &str) -> asterisk_sip::pjsip_config::TransportConfig { + asterisk_sip::pjsip_config::TransportConfig { + name: name.to_string(), + protocol: "udp".to_string(), + bind: bind.parse().unwrap(), + external_media_address: None, + external_signaling_address: None, + external_signaling_port: None, + cert_file: None, + priv_key_file: None, + local_net: vec![], + } + } + + fn cfg_with(transports: Vec) -> asterisk_sip::pjsip_config::PjsipConfig { + asterisk_sip::pjsip_config::PjsipConfig { + transports, + ..Default::default() + } + } + + /// M6 CP6 (codex P2): the FULL config chain — collect + de-dup + guard — + /// rejects a config with TWO DISTINCT udp transports. This pins the + /// production wiring (dedup before guard), not just the helper: it fails RED + /// if the guard call is removed from startup OR if dedup is moved after the + /// guard. + #[test] + fn two_distinct_udp_transports_config_is_rejected() { + let cfg = cfg_with(vec![ + udp_transport("t1", "0.0.0.0:5060"), + udp_transport("t2", "0.0.0.0:5062"), + ]); + // Through the exact entry point startup uses, so this fails RED if the + // guard is removed from that path OR dedup is reordered after it. + assert!( + resolve_single_udp_bind(Some(&cfg), sa("0.0.0.0:5060")).is_err(), + "a two-udp-transport config must be rejected at startup" + ); + } + + /// M6 CP6 (codex P2): two IDENTICAL udp transports dedup to one bind and are + /// ACCEPTED — the guard must not reject a redundant-but-consistent config + /// (pins that de-dup runs BEFORE the guard). + #[test] + fn two_identical_udp_transports_config_is_accepted() { + let cfg = cfg_with(vec![ + udp_transport("t1", "0.0.0.0:5060"), + udp_transport("t2", "0.0.0.0:5060"), + ]); + assert_eq!( + resolve_single_udp_bind(Some(&cfg), sa("0.0.0.0:5062")).unwrap(), + sa("0.0.0.0:5060") + ); + } + + /// M6 CP6: no config (and no transports) falls back to the default bind — + /// exactly one, accepted. + #[test] + fn no_config_uses_default_bind() { + assert_eq!( + resolve_single_udp_bind(None, sa("0.0.0.0:5060")).unwrap(), + sa("0.0.0.0:5060") + ); + } + /// Create a unique empty temp directory for a test. fn test_dir(tag: &str) -> std::path::PathBuf { let dir = std::env::temp_dir().join(format!(