refactor(config): one canonical config.json schema for client/server/relay

All three on-disk config.json shapes (client, server, relay) used to be declared locally inside ostp/src/main.rs, invisible to any other consumer — which is exactly how ostp_client::migrate ended up matching against loosely typed serde_json::Value instead of a real schema, with no guarantee its hand-built output actually matched what the CLI parser expected. Moved every one of those definitions (AppMode, UnifiedConfig, ServerConfig, RelayServerConfig, ClientFileConfig, TunConfig, ExcludeConfig, MuxConfig, TransportConfigRaw, ApiConfig, FallbackCfg, ListenConfig, UserConfig) into ostp_client::config — the same file that already held the runtime ClientConfig/OstpConfig/etc. main.rs now imports them instead of re-declaring them (`ClientFileConfig as ClientConfig` to avoid colliding with the runtime ClientConfig, which stays separate on purpose: it's the engine's internal shape — handshake/io timeouts and the like a user never sets in config.json — built FROM one of these via the mapping in run_client_directly, not the same thing). ServerConfig.dns is now Option<serde_json::Value> rather than Option<ostp_server::dns::DnsConfig> — ostp-client doesn't (and shouldn't) depend on ostp-server just to name that type. main.rs, which already depends on both crates, deserializes it right before handing it to run_server(). cmd_migrate now proves its output against this schema before ever writing to disk (serde_json::from_value::<UnifiedConfig>(migrated)) — a migrator/ schema drift is now a hard error at migrate time, not a corrupted config.json discovered later. Added a matching unit test (every_migrated_output_matches_the_canonical_schema) that exercises this same check on all three migration paths (modular, legacy-flat, server).
2026-07-08 18:58:34 +03:00 · 2026-07-08 18:58:34 +03:00 · d822f48891
parent 26665a826f
commit d822f48891
3 changed files with 315 additions and 223 deletions
--- a/ostp-client/src/config.rs
+++ b/ostp-client/src/config.rs
@ -293,3 +293,249 @@ impl ClientConfig {
        })
    }
 }
+
+// ═══════════════════════════════════════════════════════════════════════
+// On-disk config.json shapes — client, server, and relay.
+//
+// This is the ONE place these are defined. They used to be declared locally
+// inside ostp/src/main.rs (the CLI binary) with no other consumer able to
+// see them, which is exactly how ostp-client::migrate ended up working
+// against loosely-typed serde_json::Value instead of a real schema, and how
+// the CLI, the migrator, and this crate's own hot-reload path could each
+// silently drift out of sync with what a config.json actually looks like.
+// main.rs now imports these instead of re-declaring them (see the `use
+// ostp_client::config::{...}` at its top).
+//
+// These are DELIBERATELY separate from ClientConfig/OstpConfig/etc. above:
+// this section is the friendly, minimal shape a user actually edits by
+// hand; the types above are what the running engine needs internally
+// (handshake/io timeouts, resolved addresses, ...) and are built FROM one
+// of these via the mapping in ostp/src/main.rs::run_client_directly. Only
+// `ClientConfig` collides by name with the runtime type above, so the
+// on-disk one is `ClientFileConfig` — everything else keeps its natural name.
+// ═══════════════════════════════════════════════════════════════════════
+
+#[derive(Debug, Deserialize, Serialize)]
+#[serde(tag = "mode", rename_all = "lowercase")]
+pub enum AppMode {
+    Server(ServerConfig),
+    Client(ClientFileConfig),
+    Relay(RelayServerConfig),
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct UnifiedConfig {
+    #[serde(flatten)]
+    pub mode: AppMode,
+    pub log_level: Option<String>,
+}
+
+impl UnifiedConfig {
+    pub fn validate(&self) -> Result<()> {
+        match &self.mode {
+            AppMode::Server(cfg) => {
+                if cfg.access_keys.is_empty() {
+                    anyhow::bail!("Server configuration must contain at least one access_key.");
+                }
+                if let Some(outbound) = &cfg.outbound {
+                    if outbound.enabled {
+                        let action = outbound.default_action.as_deref().unwrap_or("direct");
+                        if action == "direct" && outbound.rules.is_empty() {
+                            println!("\n[WARNING] Server outbound proxy is ENABLED, but default_action is 'direct' and there are no rules!");
+                            println!("          This means ALL traffic will bypass the proxy and go out directly from the server IP.");
+                            println!("          If you want all traffic to be proxied, change 'default_action' to 'proxy'.\n");
+                        }
+                    }
+                }
+            }
+            AppMode::Client(cfg) => {
+                if cfg.access_key.is_empty() {
+                    anyhow::bail!("Client configuration must contain an access_key.");
+                }
+            }
+            AppMode::Relay(cfg) => {
+                if cfg.upstream_tcp.is_empty() {
+                    anyhow::bail!("Relay configuration must specify upstream_tcp address.");
+                }
+                if cfg.upstream_api_url.is_empty() {
+                    anyhow::bail!("Relay configuration must specify upstream_api_url.");
+                }
+            }
+        }
+        Ok(())
+    }
+}
+
+#[derive(Debug, Deserialize, Serialize, Clone)]
+#[serde(untagged)]
+pub enum UserConfig {
+    Detailed {
+        access_key: String,
+        name: Option<String>,
+        limit_bytes: Option<u64>,
+    },
+    KeyOnly(String),
+}
+
+impl UserConfig {
+    pub fn key(&self) -> String {
+        match self {
+            UserConfig::KeyOnly(k) => k.clone(),
+            UserConfig::Detailed { access_key, .. } => access_key.clone(),
+        }
+    }
+    pub fn name(&self) -> Option<String> {
+        match self {
+            UserConfig::KeyOnly(_) => None,
+            UserConfig::Detailed { name, .. } => name.clone(),
+        }
+    }
+    pub fn limit(&self) -> Option<u64> {
+        match self {
+            UserConfig::KeyOnly(_) => None,
+            UserConfig::Detailed { limit_bytes, .. } => *limit_bytes,
+        }
+    }
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct ServerConfig {
+    pub listen: ListenConfig,
+    pub access_keys: Vec<UserConfig>,
+    pub debug: Option<bool>,
+    pub outbound: Option<OutboundConfig>,
+    pub api: Option<ApiConfig>,
+    pub fallback: Option<FallbackCfg>,
+    pub transport: Option<TransportConfigRaw>,
+    // Left untyped: ostp-client does not (and should not) depend on
+    // ostp-server just to name its DnsConfig type. The CLI binary — which
+    // already depends on both crates — deserializes this into
+    // ostp_server::dns::DnsConfig right before handing it to run_server().
+    pub dns: Option<serde_json::Value>,
+}
+
+/// Relay-node config.json shape.
+#[derive(Debug, Deserialize, Serialize)]
+pub struct RelayServerConfig {
+    /// Listen address(es) (UDP + TCP UoT)
+    pub listen: ListenConfig,
+    /// Upstream address for TCP (UoT) traffic
+    pub upstream_tcp: String,
+    /// Upstream address for UDP traffic
+    pub upstream_udp: String,
+    /// Target server's API URL, for key sync
+    pub upstream_api_url: String,
+    /// Bearer token for the target server's API
+    #[serde(default)]
+    pub upstream_api_token: String,
+    /// Key sync interval in seconds (default 30)
+    #[serde(default = "default_sync_interval")]
+    pub sync_interval_secs: u64,
+    pub debug: Option<bool>,
+}
+
+fn default_sync_interval() -> u64 { 30 }
+
+/// Supports both a single string "0.0.0.0:50000" and an array
+/// ["0.0.0.0:50000", "[::]:50000"].
+#[derive(Debug, Deserialize, Serialize, Clone)]
+#[serde(untagged)]
+pub enum ListenConfig {
+    Single(String),
+    Multiple(Vec<String>),
+}
+
+impl ListenConfig {
+    pub fn addresses(&self) -> Vec<String> {
+        match self {
+            ListenConfig::Single(s) => vec![s.clone()],
+            ListenConfig::Multiple(v) => v.clone(),
+        }
+    }
+
+    pub fn primary(&self) -> String {
+        match self {
+            ListenConfig::Single(s) => s.clone(),
+            ListenConfig::Multiple(v) => v.first().cloned().unwrap_or_default(),
+        }
+    }
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct ApiConfig {
+    pub enabled: Option<bool>,
+    pub bind: Option<String>,
+    pub token: Option<String>,
+    pub webpath: Option<String>,
+    pub username: Option<String>,
+    pub password_hash: Option<String>,
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct FallbackCfg {
+    pub enabled: Option<bool>,
+    pub listen: Option<String>,
+    pub target: Option<String>,
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct ClientFileConfig {
+    pub server: String,
+    pub access_key: String,
+    pub mtu: Option<usize>,
+    pub socks5_bind: Option<String>,
+    pub tun: Option<TunConfig>,
+    pub debug: Option<bool>,
+    pub exclude: Option<ExcludeConfig>,
+    pub mux: Option<MuxConfig>,
+    pub transport: Option<TransportConfigRaw>,
+    pub gui: Option<serde_json::Value>,
+}
+
+#[derive(Debug, Deserialize, Serialize, Clone)]
+pub struct TransportConfigRaw {
+    pub mode: Option<String>,
+    pub stealth_sni: Option<String>,
+    pub tcp_fragmentation: Option<bool>,
+}
+
+#[derive(Debug, Deserialize, Serialize, Clone)]
+pub struct TunConfig {
+    pub enable: bool,
+    pub wintun_path: Option<String>,
+    pub ipv4_address: Option<String>,
+    pub dns: Option<String>,
+    pub kill_switch: Option<bool>,
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct OutboundConfig {
+    pub enabled: bool,
+    pub protocol: String,
+    pub address: String,
+    pub port: u16,
+    #[serde(default)]
+    pub rules: Vec<OutboundRule>,
+    pub default_action: Option<String>,
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct OutboundRule {
+    pub domain_suffix: Option<Vec<String>>,
+    pub ip_cidr: Option<Vec<String>>,
+    pub protocol: Option<String>,
+    pub action: Option<String>,
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct ExcludeConfig {
+    pub domains: Option<Vec<String>>,
+    pub ips: Option<Vec<String>>,
+    pub processes: Option<Vec<String>>,
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct MuxConfig {
+    pub enabled: Option<bool>,
+    pub sessions: Option<usize>,
+}
--- a/ostp-client/src/migrate.rs
+++ b/ostp-client/src/migrate.rs
@ -483,6 +483,40 @@ mod tests {
        assert_eq!(new, current);
    }

+    /// Every migrated output must actually deserialize into the ONE
+    /// canonical schema (`crate::config`) — this is the same check
+    /// `cmd_migrate` runs at runtime before ever touching a user's file,
+    /// exercised here directly so a schema/migrator drift fails a fast unit
+    /// test instead of surfacing as "your migrated config won't load".
+    #[test]
+    fn every_migrated_output_matches_the_canonical_schema() {
+        let modular = json!({
+            "inbounds": [{ "type": "tun", "tag": "tun-in", "mtu": 1350 }],
+            "outbounds": [
+                { "type": "ostp", "tag": "proxy", "server": "1.2.3.4", "port": 50000, "access_key": "k" },
+                { "type": "direct", "tag": "direct" }
+            ],
+            "routing": { "rules": [], "default_outbound": "proxy" }
+        });
+        let (new, _) = migrate_client_json(modular);
+        serde_json::from_value::<crate::config::ClientFileConfig>(new)
+            .expect("modular->flat migration output must match ClientFileConfig");
+
+        let legacy_flat = json!({
+            "server": "1.2.3.4:50000",
+            "access_key": "k",
+            "tun": { "enable": true, "wintun_path": "x", "ipv4_address": "y" }
+        });
+        let (new, _) = migrate_client_json(legacy_flat);
+        serde_json::from_value::<crate::config::ClientFileConfig>(new)
+            .expect("legacy-flat migration output must match ClientFileConfig");
+
+        let server = json!({ "listen": "0.0.0.0:50000", "access_keys": ["k"] });
+        let (new, _) = migrate_server_json(server);
+        serde_json::from_value::<crate::config::ServerConfig>(new)
+            .expect("server migration output must match ServerConfig");
+    }
+
    #[test]
    fn server_config_backfills_api_defaults_and_drops_legacy_token() {
        let old = json!({
--- a/ostp/src/main.rs
+++ b/ostp/src/main.rs
@ -1,6 +1,5 @@
 use anyhow::{anyhow, Result};
 use clap::Parser;
-use serde::{Deserialize, Serialize};
 use std::fs;
 use std::path::PathBuf;
 use colored::Colorize;
@ -231,226 +230,19 @@ fn parse_outbound_action(value: Option<String>) -> ostp_server::OutboundAction {
    }
 }

-#[derive(Debug, Deserialize, Serialize)]
-#[serde(tag = "mode", rename_all = "lowercase")]
-enum AppMode {
-    Server(ServerConfig),
-    Client(ClientConfig),
-    Relay(RelayServerConfig),
-}
-
-#[derive(Debug, Deserialize, Serialize)]
-struct UnifiedConfig {
-    #[serde(flatten)]
-    mode: AppMode,
-    log_level: Option<String>,
-}
-
-impl UnifiedConfig {
-    fn validate(&self) -> Result<()> {
-        match &self.mode {
-            AppMode::Server(cfg) => {
-                if cfg.access_keys.is_empty() {
-                    anyhow::bail!("Server configuration must contain at least one access_key.");
-                }
-                if let Some(outbound) = &cfg.outbound {
-                    if outbound.enabled {
-                        let action = outbound.default_action.as_deref().unwrap_or("direct");
-                        if action == "direct" && outbound.rules.is_empty() {
-                            println!("\n[WARNING] Server outbound proxy is ENABLED, but default_action is 'direct' and there are no rules!");
-                            println!("          This means ALL traffic will bypass the proxy and go out directly from the server IP.");
-                            println!("          If you want all traffic to be proxied, change 'default_action' to 'proxy'.\n");
-                        }
-                    }
-                }
-            }
-            AppMode::Client(cfg) => {
-                if cfg.access_key.is_empty() {
-                    anyhow::bail!("Client configuration must contain an access_key.");
-                }
-            }
-            AppMode::Relay(cfg) => {
-                if cfg.upstream_tcp.is_empty() {
-                    anyhow::bail!("Relay configuration must specify upstream_tcp address.");
-                }
-                if cfg.upstream_api_url.is_empty() {
-                    anyhow::bail!("Relay configuration must specify upstream_api_url.");
-                }
-            }
-        }
-        Ok(())
-    }
-}
-
-#[derive(Debug, Deserialize, Serialize, Clone)]
-#[serde(untagged)]
-pub enum UserConfig {
-    Detailed {
-        access_key: String,
-        name: Option<String>,
-        limit_bytes: Option<u64>,
-    },
-    KeyOnly(String),
-}
-
-impl UserConfig {
-    pub fn key(&self) -> String {
-        match self {
-            UserConfig::KeyOnly(k) => k.clone(),
-            UserConfig::Detailed { access_key, .. } => access_key.clone(),
-        }
-    }
-    pub fn name(&self) -> Option<String> {
-        match self {
-            UserConfig::KeyOnly(_) => None,
-            UserConfig::Detailed { name, .. } => name.clone(),
-        }
-    }
-    pub fn limit(&self) -> Option<u64> {
-        match self {
-            UserConfig::KeyOnly(_) => None,
-            UserConfig::Detailed { limit_bytes, .. } => limit_bytes.clone(),
-        }
-    }
-}
-
-#[derive(Debug, Deserialize, Serialize)]
-struct ServerConfig {
-    listen: ListenConfig,
-    access_keys: Vec<UserConfig>,
-    debug: Option<bool>,
-    outbound: Option<OutboundConfig>,
-    api: Option<ApiConfig>,
-    fallback: Option<FallbackCfg>,
-    transport: Option<TransportConfigRaw>,
-    dns: Option<ostp_server::dns::DnsConfig>,
-}
-
-/// Конфигурация Relay-узла в config.json
-#[derive(Debug, Deserialize, Serialize)]
-struct RelayServerConfig {
-    /// Адрес(а) прослушивания (UDP + TCP UoT)
-    listen: ListenConfig,
-    /// Адрес upstream для TCP (UoT) трафика
-    upstream_tcp: String,
-    /// Адрес upstream для UDP трафика
-    upstream_udp: String,
-    /// URL API целевого сервера для синхронизации ключей
-    upstream_api_url: String,
-    /// Bearer-токен для API целевого сервера
-    #[serde(default)]
-    upstream_api_token: String,
-    /// Интервал синхронизации ключей в секундах (по умолчанию 30)
-    #[serde(default = "default_sync_interval")]
-    sync_interval_secs: u64,
-    debug: Option<bool>,
-}
-
-fn default_sync_interval() -> u64 { 30 }
-
-/// Supports both single string "0.0.0.0:50000" and array ["0.0.0.0:50000", "[::]:50000"]
-#[derive(Debug, Deserialize, Serialize, Clone)]
-#[serde(untagged)]
-enum ListenConfig {
-    Single(String),
-    Multiple(Vec<String>),
-}
-
-impl ListenConfig {
-    fn addresses(&self) -> Vec<String> {
-        match self {
-            ListenConfig::Single(s) => vec![s.clone()],
-            ListenConfig::Multiple(v) => v.clone(),
-        }
-    }
-
-    fn primary(&self) -> String {
-        match self {
-            ListenConfig::Single(s) => s.clone(),
-            ListenConfig::Multiple(v) => v.first().cloned().unwrap_or_default(),
-        }
-    }
-}
-
-#[derive(Debug, Deserialize, Serialize)]
-struct ApiConfig {
-    enabled: Option<bool>,
-    bind: Option<String>,
-    token: Option<String>,
-    webpath: Option<String>,
-    username: Option<String>,
-    password_hash: Option<String>,
-}
-
-#[derive(Debug, Deserialize, Serialize)]
-struct FallbackCfg {
-    enabled: Option<bool>,
-    listen: Option<String>,
-    target: Option<String>,
-}
-
-#[derive(Debug, Deserialize, Serialize)]
-struct ClientConfig {
-    server: String,
-    access_key: String,
-    mtu: Option<usize>,
-    socks5_bind: Option<String>,
-    tun: Option<TunConfig>,
-    debug: Option<bool>,
-    exclude: Option<ExcludeConfig>,
-    mux: Option<MuxConfig>,
-    transport: Option<TransportConfigRaw>,
-    gui: Option<serde_json::Value>,
-}
-
-#[derive(Debug, Deserialize, Serialize, Clone)]
-struct TransportConfigRaw {
-    mode: Option<String>,
-    stealth_sni: Option<String>,
-    tcp_fragmentation: Option<bool>,
-}
-
-#[derive(Debug, Deserialize, Serialize, Clone)]
-struct TunConfig {
-    enable: bool,
-    wintun_path: Option<String>,
-    ipv4_address: Option<String>,
-    dns: Option<String>,
-    kill_switch: Option<bool>,
-}
-
-
-#[derive(Debug, Deserialize, Serialize)]
-struct OutboundConfig {
-    enabled: bool,
-    protocol: String,
-    address: String,
-    port: u16,
-    #[serde(default)]
-    rules: Vec<OutboundRule>,
-    default_action: Option<String>,
-}
-
-#[derive(Debug, Deserialize, Serialize)]
-struct OutboundRule {
-    domain_suffix: Option<Vec<String>>,
-    ip_cidr: Option<Vec<String>>,
-    protocol: Option<String>,
-    action: Option<String>,
-}
-
-#[derive(Debug, Deserialize, Serialize)]
-struct ExcludeConfig {
-    domains: Option<Vec<String>>,
-    ips: Option<Vec<String>>,
-    processes: Option<Vec<String>>,
-}
-
-#[derive(Debug, Deserialize, Serialize)]
-struct MuxConfig {
-    enabled: Option<bool>,
-    sessions: Option<usize>,
-}
+// The on-disk config.json shapes (client/server/relay + all nested types)
+// live in ostp_client::config now — this used to be ~220 lines of struct
+// definitions duplicated here with no other consumer able to see them,
+// which is exactly why ostp_client::migrate had to work against loosely
+// typed JSON instead of a real schema. `ClientFileConfig` is aliased back to
+// the bare `ClientConfig` name used throughout the rest of this file, so it
+// doesn't collide with `ostp_client::config::ClientConfig` (the RUNTIME
+// shape the engine actually uses — a different thing on purpose; see the
+// doc comment on that struct).
+use ostp_client::config::{
+    AppMode, ClientFileConfig as ClientConfig, MuxConfig, TransportConfigRaw, TunConfig,
+    UnifiedConfig,
+};

 #[tokio::main]
 async fn main() -> Result<()> {
@ -1570,8 +1362,16 @@ async fn run_app() -> Result<()> {
                })
            }).collect::<Vec<_>>();
            let host = get_or_ask_public_ip(&args.config);
-            // Build DNS config and set owndns flag in subscribe links if DNS enabled
-            let dns_cfg = server_cfg.dns;
+            // Build DNS config and set owndns flag in subscribe links if DNS enabled.
+            // Kept untyped (serde_json::Value) in the shared ServerConfig so
+            // ostp-client doesn't need a dependency on ostp-server just to
+            // name this type — deserialize it here instead, where both
+            // crates are already in scope.
+            let dns_cfg: Option<ostp_server::dns::DnsConfig> = server_cfg
+                .dns
+                .map(serde_json::from_value)
+                .transpose()
+                .map_err(|e| anyhow!("Invalid 'dns' section in server config: {e}"))?;
            // Pass all listen addresses for multi-listener support
            ostp_server::run_server(listen_addrs, Some(host), access_keys_meta, outbound, api_config, fallback_config, debug, dns_cfg, Some(args.config)).await?;
        }
@ -1739,6 +1539,18 @@ fn cmd_migrate(config_path: &std::path::Path) -> Result<()> {
        return Ok(());
    }

+    // Prove the migrator's output actually matches the ONE canonical schema
+    // (ostp_client::config) before ever touching the user's file — this is
+    // what makes "single source of truth" a guarantee instead of just an
+    // intention: if migrate.rs's hand-built JSON ever drifts from what
+    // UnifiedConfig actually expects, this catches it here, not as a
+    // corrupted config.json on someone's server.
+    serde_json::from_value::<ostp_client::config::UnifiedConfig>(migrated.clone())
+        .map_err(|e| anyhow!(
+            "Internal error: the migrated config does not match the current schema ({e}). \
+             Nothing was written — this is a bug in the migrator, please report it."
+        ))?;
+
    let backup_path = config_path.with_extension("json.bak");
    fs::copy(config_path, &backup_path)?;
    println!("{} Original config backed up to {:?}", "[ostp]".cyan().bold(), backup_path);