From 2461ac3c70b274644eb27ad6c3666ef8859189dc Mon Sep 17 00:00:00 2001
From: tegwick <bernd.worsch@gmail.com>
Date: Wed, 19 Aug 2026 19:31:19 +0200
Subject: [PATCH] Reject duplicate local_port among local forwards; document
 the two meanings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

local_port means different things per direction, and conflating them is easy:
a `direction: local` tunnel binds it here (-L local_port:remote_host:remote_port),
while a reverse tunnel — the default — uses it as the *destination* on the
workstation (-R remote_port:remote_host:local_port) and binds on the remote.

Two local forwards on one port do not fail loudly: whichever binds first wins
and the losers reconnect forever, so the map reads healthy while the port
answers from a different machine than the one asked for. Reject that at config
load. Reverse tunnels stay exempt by design — every state-hub-* reverse tunnel
targets 8000 on purpose, so each remote box reaches this hub at its own 18000.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/bridge/config.py | 38 ++++++++++++++++++++++++++++++++++++-
 tests/test_config.py | 45 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/src/bridge/config.py b/src/bridge/config.py
index a7fac7b..35cf60e 100644
--- a/src/bridge/config.py
+++ b/src/bridge/config.py
@@ -5,7 +5,7 @@ import os
 import warnings
 from dataclasses import dataclass
 from pathlib import Path
-from typing import Dict, Optional
+from typing import Dict, List, Optional
 
 import yaml
 
@@ -59,9 +59,45 @@ def _parse_tunnels(raw: dict) -> Dict[str, TunnelConfig]:
         if not isinstance(data, dict):
             raise ConfigError(f"Tunnel '{name}' must be a mapping")
         tunnels[name] = _parse_tunnel(name, data)
+    _reject_duplicate_local_ports(tunnels)
     return tunnels
 
 
+def _reject_duplicate_local_ports(tunnels: Dict[str, TunnelConfig]) -> None:
+    """Two `direction: local` tunnels must never share a local_port.
+
+    Only local forwards bind a port on this machine
+    (``-L local_port:remote_host:remote_port``). If two claim the same one,
+    whichever binds first wins and the losers sit in an endless reconnect loop —
+    the map reads healthy while the port answers from a different machine than
+    the one you asked for. Silent, and worse than an outage. Fail at load
+    instead (BRIDGE-WP-0007).
+
+    Reverse tunnels are deliberately exempt. There, local_port is the
+    *destination* on this workstation
+    (``-R remote_port:remote_host:local_port``) and the listener is on the
+    remote, so sharing is the intended design: every ``state-hub-*`` reverse
+    tunnel targets 8000 so each remote box reaches this hub at its own 18000.
+    Rejecting that would be rejecting the point of the tool.
+    """
+    by_port: Dict[int, List[str]] = {}
+    for name, tunnel in tunnels.items():
+        if tunnel.direction != "local":
+            continue
+        by_port.setdefault(tunnel.local_port, []).append(name)
+    collisions = {port: names for port, names in by_port.items() if len(names) > 1}
+    if not collisions:
+        return
+    detail = "; ".join(
+        f"port {port}: {', '.join(sorted(names))}" for port, names in sorted(collisions.items())
+    )
+    raise ConfigError(
+        f"Duplicate local_port among 'direction: local' tunnels ({detail}). "
+        f"Each local forward binds that port here, so they would race for it — "
+        f"give the non-canonical one a different port."
+    )
+
+
 def _parse_tunnel(name: str, data: dict) -> TunnelConfig:
     required = ["host", "remote_port", "local_port", "ssh_user", "ssh_key", "actor"]
     for field in required:
diff --git a/tests/test_config.py b/tests/test_config.py
index 1d453f7..b6ed135 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -297,3 +297,48 @@ class TestCertCommandConfig:
         monkeypatch.setenv("BRIDGE_CONFIG", str(config_file))
         cfg = load_config()
         assert cfg.tunnels["state-hub-coulombcore"].cert_command is None
+
+
+def test_duplicate_local_ports_are_rejected():
+    """A local-forward port collision is silent at runtime; fail at load."""
+    from bridge.config import ConfigError, _parse_tunnels
+
+    base = {
+        "host": "192.0.2.1",
+        "remote_port": 18000,
+        "ssh_user": "tegwick",
+        "ssh_key": "~/.ssh/id_ops",
+        "actor": "agt-claude-railiance01",
+        "direction": "local",
+    }
+    with pytest.raises(ConfigError, match="Duplicate local_port"):
+        _parse_tunnels(
+            {
+                "k3s-a": {**base, "local_port": 16443},
+                "k3s-b": {**base, "local_port": 16443},
+            }
+        )
+
+
+def test_reverse_tunnels_may_share_a_local_port():
+    """local_port is the destination on this workstation for a reverse tunnel.
+
+    Every state-hub-* reverse tunnel targets 8000 on purpose, so each remote box
+    reaches this hub at its own 18000. Rejecting that would break the tool.
+    """
+    from bridge.config import _parse_tunnels
+
+    base = {
+        "host": "192.0.2.1",
+        "remote_port": 18000,
+        "ssh_user": "tegwick",
+        "ssh_key": "~/.ssh/id_ops",
+        "actor": "agt-claude-railiance01",
+    }
+    tunnels = _parse_tunnels(
+        {
+            "state-hub-a": {**base, "local_port": 8000},
+            "state-hub-b": {**base, "local_port": 8000},
+        }
+    )
+    assert {t.local_port for t in tunnels.values()} == {8000}