Fix final code review findings: docs, iface validation, test env guards

Corrects the crate docs and README to describe sip-tunnel as the static
control-plane daemon it actually is (it never inspects SIP/RTP packets),
fixes the masquerade troubleshooting note to name the Tailscale interface
instead of WAN, and marks --rtp-end-port as inclusive throughout. Adds
input validation for --wan-iface/--tailscale-iface (mirroring the existing
table-name check) since both are interpolated unquoted-adjacent into the
generated nftables ruleset. Warns on stderr when --log-level/RUST_LOG fails
to parse instead of silently falling back to info. Hardens
missing_freeswitch_ip_exits_1 and defaults_are_used_when_nothing_else_is_set
against inherited shell environment variables.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: andysnell

Cargo.toml

@@ -3,7 +3,7 @@ name = "sip-tunnel"
 version = "0.1.0"
 edition = "2024"
 rust-version = "1.96"
-description = "A userspace SIP-aware tunnel daemon that manages nftables rules for SIP/RTP traffic"
+description = "A static control-plane daemon that generates and applies nftables DNAT/masquerade rules to relay SIP/RTP traffic to FreeSWITCH over Tailscale"
 license = "MIT OR Apache-2.0"
 
 [lib]

README.md

@@ -74,7 +74,7 @@ precedence is **CLI flag > env var > default**.
 | `--sip-tcp-port` | `SIP_TUNNEL_SIP_TCP_PORT` | `5060` | SIP TCP listen port |
 | `--sip-tls-port` | `SIP_TUNNEL_SIP_TLS_PORT` | `5061` | SIP TLS listen port (TCP passthrough) |
 | `--rtp-start-port` | `SIP_TUNNEL_RTP_START_PORT` | `16384` | Lower bound (inclusive) of the managed RTP port range |
-| `--rtp-end-port` | `SIP_TUNNEL_RTP_END_PORT` | `32768` | Upper bound (exclusive) of the managed RTP port range |
+| `--rtp-end-port` | `SIP_TUNNEL_RTP_END_PORT` | `32768` | Upper bound (inclusive) of the managed RTP port range |
 | `--provider` | `SIP_TUNNEL_PROVIDER` | `none` | Upstream SIP trunking provider preset: `twilio`, `bandwidth`, `telnyx`, or `none` |
 | `--allowlist` | `SIP_TUNNEL_ALLOWLIST` | *(empty)* | Extra source CIDRs to allow, comma-delimited, in addition to any provider preset |
 | `--enable-allowlist` | `SIP_TUNNEL_ENABLE_ALLOWLIST` | `false` | Enable source-IP allowlist enforcement |
@@ -221,8 +221,8 @@ The RTP port range FreeSWITCH actually binds must match the range
   so Twilio will accept SIP from it.
 - Run `sip-tunnel` with `--provider twilio` — this bakes in Twilio's 8
   published signaling `/30` CIDRs plus its `168.86.128.0/18` media (RTP)
-  range as the allowlist, so `--enable-allowlist` works out of the box with
-  no extra `--allowlist` entries required.
+  range (UDP 10000-60000) as the allowlist, so `--enable-allowlist` works
+  out of the box with no extra `--allowlist` entries required.
 - Prefer G.711 (PCMU/PCMA) codecs; they avoid transcoding and keep RTP
   packet sizing predictable end-to-end.
 
@@ -294,7 +294,8 @@ always constructs its `HealthServer` with `probe: None`, so in production
   `sip-tunnel`'s `--rtp-start-port`/`--rtp-end-port`. A mismatch means some
   RTP ports aren't DNAT'd and those streams silently drop.
 - **Call connects but no return audio at all**: confirm the masquerade rule
-  is present and correctly targeting the WAN interface:
+  is present and correctly targeting the Tailscale interface (`oifname
+  "tailscale0" ip daddr <freeswitch-ip> masquerade`):
   `nft list table ip sip_tunnel` (substitute your `--table-name` if
   non-default) on the VPS, and check for a `masquerade` statement in the
   postrouting chain.

src/config.rs

@@ -105,7 +105,7 @@ pub struct ConfigArgs {
     )]
     pub rtp_start_port: u16,
 
-    /// Upper bound (exclusive) of the RTP port range to manage.
+    /// Upper bound (inclusive) of the RTP port range to manage.
     #[arg(
         long = "rtp-end-port",
         env = "SIP_TUNNEL_RTP_END_PORT",
@@ -229,6 +229,21 @@ pub enum ConfigError {
     /// list is empty.
     #[error("allowlist enabled but no source CIDRs configured")]
     EmptyAllowlist,
+
+    /// An interface-name field (`--wan-iface`/`--tailscale-iface`) is not a
+    /// safe Linux interface name.
+    #[error(
+        "{field} {name:?} is not a valid interface name: must be 1-15 \
+         characters with no whitespace, control characters, '\"', '/', and \
+         not \".\" or \"..\""
+    )]
+    InvalidInterfaceName {
+        /// Which field failed validation (`"wan-iface"` or
+        /// `"tailscale-iface"`).
+        field: &'static str,
+        /// The rejected interface name.
+        name: String,
+    },
 }
 
 /// The validated, effective configuration for a run of `sip-tunnel`.
@@ -248,7 +263,7 @@ pub struct Config {
     pub sip_tls_port: u16,
     /// Lower bound (inclusive) of the RTP port range to manage.
     pub rtp_start_port: u16,
-    /// Upper bound (exclusive) of the RTP port range to manage.
+    /// Upper bound (inclusive) of the RTP port range to manage.
     pub rtp_end_port: u16,
     /// Upstream SIP trunking provider preset.
     pub provider: Provider,
@@ -308,6 +323,20 @@ impl ConfigArgs {
             });
         }
 
+        if !is_valid_iface_name(&self.wan_iface) {
+            return Err(ConfigError::InvalidInterfaceName {
+                field: "wan-iface",
+                name: self.wan_iface,
+            });
+        }
+
+        if !is_valid_iface_name(&self.tailscale_iface) {
+            return Err(ConfigError::InvalidInterfaceName {
+                field: "tailscale-iface",
+                name: self.tailscale_iface,
+            });
+        }
+
         let allow_cidrs = combined_cidrs(self.provider, &self.allowlist);
         if self.enable_allowlist && allow_cidrs.is_empty() {
             return Err(ConfigError::EmptyAllowlist);
@@ -415,6 +444,22 @@ fn is_valid_table_name(name: &str) -> bool {
     chars.all(|c| c.is_ascii_alphanumeric() || c == '_')
 }
 
+/// Returns whether `name` is safe to interpolate into generated nftables
+/// syntax (inside double quotes) as an interface name: 1-15 characters
+/// (`IFNAMSIZ - 1`, matching the Linux kernel's interface name limit), no
+/// whitespace, no control characters, no `"` or `/`, and not `.` or `..`.
+fn is_valid_iface_name(name: &str) -> bool {
+    if name.is_empty() || name.chars().count() > 15 {
+        return false;
+    }
+    if name == "." || name == ".." {
+        return false;
+    }
+    !name
+        .chars()
+        .any(|c| c.is_whitespace() || c.is_control() || c == '"' || c == '/')
+}
+
 /// The `clap::ValueEnum` name for a [`Provider`] variant, used for
 /// deterministic `summary_lines` output.
 fn provider_name(provider: Provider) -> &'static str {
@@ -538,6 +583,68 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn valid_iface_names_pass() {
+        for name in ["eth0", "tailscale0", "wg-vps0"] {
+            let mut wan = valid_args();
+            wan.wan_iface = name.to_string();
+            assert!(wan.validate().is_ok(), "wan_iface {name:?} should be valid");
+
+            let mut ts = valid_args();
+            ts.tailscale_iface = name.to_string();
+            assert!(
+                ts.validate().is_ok(),
+                "tailscale_iface {name:?} should be valid"
+            );
+        }
+    }
+
+    #[test]
+    fn invalid_wan_iface_names_error() {
+        let invalid = [
+            "",
+            "sixteen_chars123",
+            "eth\"0",
+            "eth\n0",
+            "eth 0",
+            ".",
+            "..",
+        ];
+        for name in invalid {
+            let mut args = valid_args();
+            args.wan_iface = name.to_string();
+            assert!(
+                matches!(
+                    args.validate(),
+                    Err(ConfigError::InvalidInterfaceName {
+                        field: "wan-iface",
+                        ..
+                    })
+                ),
+                "wan_iface {name:?} should be rejected"
+            );
+        }
+    }
+
+    #[test]
+    fn invalid_tailscale_iface_names_error() {
+        let invalid = ["", "sixteen_chars123", "ts\"0", "ts\n0", "ts 0", ".", ".."];
+        for name in invalid {
+            let mut args = valid_args();
+            args.tailscale_iface = name.to_string();
+            assert!(
+                matches!(
+                    args.validate(),
+                    Err(ConfigError::InvalidInterfaceName {
+                        field: "tailscale-iface",
+                        ..
+                    })
+                ),
+                "tailscale_iface {name:?} should be rejected"
+            );
+        }
+    }
+
     #[test]
     fn enable_allowlist_with_no_cidrs_errors() {
         let mut args = valid_args();

src/lib.rs

@@ -1,7 +1,10 @@
-//! `sip-tunnel` is a userspace daemon that watches SIP signaling traffic and
-//! dynamically manages `nftables` rules to open and close the RTP media
-//! ports a SIP session actually negotiates, keeping the host firewall tight
-//! without hand-maintained wide-open port ranges.
+//! `sip-tunnel` is a static control-plane daemon that turns a cheap VPS into
+//! a SIP/RTP relay in front of a FreeSWITCH box reachable only over
+//! Tailscale. It generates a deterministic `nftables` DNAT+masquerade
+//! ruleset from configuration, applies it atomically via `nft -f -`,
+//! verifies it landed, serves `/healthz`, and flushes its owned table on
+//! shutdown — it never inspects SIP/RTP packets itself; the kernel forwards
+//! all signaling and media traffic.
 
 pub mod config;
 pub mod health;

src/main.rs

@@ -58,7 +58,13 @@ fn config_args(command: &Command) -> &ConfigArgs {
 /// `print-config`'s summary, `dump-ruleset`'s script) stays clean and
 /// scriptable.
 fn init_tracing(args: &ConfigArgs) {
-    let filter = EnvFilter::try_new(&args.log_level).unwrap_or_else(|_| EnvFilter::new("info"));
+    let filter = EnvFilter::try_new(&args.log_level).unwrap_or_else(|err| {
+        eprintln!(
+            "warning: invalid --log-level/RUST_LOG filter {:?} ({err}); falling back to \"info\"",
+            args.log_level
+        );
+        EnvFilter::new("info")
+    });
 
     let result = match args.log_format {
         LogFormat::Json => tracing_subscriber::fmt()

tests/cli.rs

@@ -105,5 +105,9 @@ fn huge_rtp_range_with_flag_exits_0() {
 
 #[test]
 fn missing_freeswitch_ip_exits_1() {
-    sip_tunnel().arg("check").assert().code(1);
+    sip_tunnel()
+        .arg("check")
+        .env_remove("SIP_TUNNEL_FREESWITCH_IP")
+        .assert()
+        .code(1);
 }

tests/config_precedence.rs

@@ -17,6 +17,8 @@ fn sip_tunnel() -> Command {
 fn defaults_are_used_when_nothing_else_is_set() {
     sip_tunnel()
         .args(["print-config", "--freeswitch-ip", "100.64.0.5"])
+        .env_remove("SIP_TUNNEL_TAILSCALE_IFACE")
+        .env_remove("SIP_TUNNEL_RTP_START_PORT")
         .assert()
         .success()
         .stdout(predicate::str::contains("tailscale-iface = tailscale0"))