From a03e2c985587fd036fceb7b99280fd00d79778aa Mon Sep 17 00:00:00 2001 From: ospab Date: Mon, 17 Aug 2026 16:04:38 +0300 Subject: [PATCH] fix(relay): stop rejecting every generated relay config at load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The relay is a transparent pipe now — it authenticates nothing and forwards to a fixed next hop. Both the setup wizard and the `init` template write a relay config with only listen + upstream_tcp + upstream_udp, and the relay runtime uses exactly those. But UnifiedConfig::validate still demanded a non-empty upstream_api_url — a field left from the old design where the relay authenticated clients itself and pulled the key list from the target's API. So the tool generated a config it then refused to load: every relay came up with "Relay configuration must specify upstream_api_url." That is why relay "was never finished" — it could not start from any config the tool itself produced. Validation now matches the transparent relay: require both upstream addresses (the runtime needs both carriers), and do not require the dead api_url. Leftover api_url in an old config is still tolerated, just ignored. Adds regression tests that load a config exactly as the daemon does (deserialize into the one canonical UnifiedConfig, then validate) for all three modes. This is the drift-catcher: whenever the wizard/template and the validator disagree on required fields again, a test fails instead of a user's node refusing to start. --- ostp-client/src/config.rs | 86 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 83 insertions(+), 3 deletions(-) diff --git a/ostp-client/src/config.rs b/ostp-client/src/config.rs index fc0d05a..072dd61 100644 --- a/ostp-client/src/config.rs +++ b/ostp-client/src/config.rs @@ -349,11 +349,18 @@ impl UnifiedConfig { } } AppMode::Relay(cfg) => { + // The relay forwards to a fixed next hop on both carriers, so it + // needs both upstream addresses. It does NOT need upstream_api_url: + // that field belonged to the old design where the relay + // authenticated clients itself, which it no longer does. Requiring + // it here was the bug that made every generated relay config + // (wizard and template alike write no api_url) fail to load with + // "must specify upstream_api_url" — a relay that could never start. if cfg.upstream_tcp.is_empty() { - anyhow::bail!("Relay configuration must specify upstream_tcp address."); + anyhow::bail!("Relay configuration must specify upstream_tcp (the next hop's TCP/UoT address)."); } - if cfg.upstream_api_url.is_empty() { - anyhow::bail!("Relay configuration must specify upstream_api_url."); + if cfg.upstream_udp.is_empty() { + anyhow::bail!("Relay configuration must specify upstream_udp (the next hop's UDP address)."); } } } @@ -536,3 +543,76 @@ pub struct MuxConfig { pub enabled: Option, pub sessions: Option, } + +#[cfg(test)] +mod tests { + use super::*; + + /// Loads a config.json exactly as the daemon does: parse the JSON into the + /// canonical `UnifiedConfig`, then validate. This is the real drift-catcher — + /// if the wizard/template and the validator ever disagree on required fields, + /// this fails instead of a user's relay refusing to start. + fn load(json: &str) -> Result { + let cfg: UnifiedConfig = serde_json::from_str(json)?; + cfg.validate()?; + Ok(cfg) + } + + /// Regression: the relay used to authenticate clients and so its config + /// carried `upstream_api_url`. The relay is a transparent pipe now and both + /// the wizard and the `init` template write NO api_url — yet validation kept + /// demanding it, so every generated relay config failed to load with + /// "must specify upstream_api_url". A relay that could never start. + #[test] + fn relay_config_without_api_url_loads() { + // Byte-for-byte the shape the wizard (main.rs) emits. + let json = r#"{ + "mode": "relay", + "listen": "0.0.0.0:50000", + "upstream_tcp": "203.0.113.10:50000", + "upstream_udp": "203.0.113.10:50000", + "debug": false + }"#; + load(json).expect("a transparent-relay config must load without upstream_api_url"); + } + + /// A relay still needs somewhere to forward to on both carriers, so an + /// incomplete relay config must fail loudly at load, not connect-to-empty + /// per session at runtime. + #[test] + fn relay_config_missing_upstream_udp_is_rejected() { + let json = r#"{ + "mode": "relay", + "listen": "0.0.0.0:50000", + "upstream_tcp": "203.0.113.10:50000", + "upstream_udp": "", + "debug": false + }"#; + assert!(load(json).is_err(), "a relay with no UDP upstream must be rejected"); + } + + /// A deprecated api_url left in an OLD config must not break loading — it is + /// ignored, not required and not forbidden. + #[test] + fn relay_config_with_leftover_api_url_still_loads() { + let json = r#"{ + "mode": "relay", + "listen": "0.0.0.0:50000", + "upstream_tcp": "203.0.113.10:50000", + "upstream_udp": "203.0.113.10:50000", + "upstream_api_url": "http://old.example:8080", + "debug": false + }"#; + load(json).expect("a stale api_url must be tolerated, not rejected"); + } + + /// The minimal client and server shapes the template emits must also load, + /// so this test guards all three modes against generator/validator drift. + #[test] + fn minimal_client_and_server_configs_load() { + load(r#"{"mode":"client","server":"127.0.0.1:50000","access_key":"k"}"#) + .expect("minimal client config must load"); + load(r#"{"mode":"server","listen":"0.0.0.0:50000","access_keys":["k"]}"#) + .expect("minimal server config must load"); + } +}