fix: wireguard_port identity change and check_port_open verification
Bug 1 — port not propagated to wg0.conf:
The identity update path (wireguard_port via PUT /api/config) was calling
wireguard_manager.update_config() which only saves to a JSON file via
BaseServiceManager. wg0.conf was never updated, so after a container
restart the WireGuard interface would still listen on the old port.
Fix: call apply_config() instead — it writes ListenPort into wg0.conf.
Bug 2 — check_port_open ignored configured port:
check_port_open() checked for 'listening port' in wg show output but
never compared it against the configured port. A port-mismatch (e.g.
after config change but before restart) would return True — misleading.
Fix: require 'listening port: {configured_port}' to match exactly.
Tests added:
- test_check_port_open_wrong_port_returns_false
- test_check_port_open_explicit_port_matches
- test_check_port_open_explicit_port_mismatch
- test_wireguard_port_identity_change_calls_apply_config
- test_wireguard_port_same_value_does_not_call_apply_config
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -778,7 +778,7 @@ def update_config():
|
|||||||
_wg_svc = config_manager.configs.get('wireguard', {})
|
_wg_svc = config_manager.configs.get('wireguard', {})
|
||||||
_wg_svc['port'] = new_wg
|
_wg_svc['port'] = new_wg
|
||||||
config_manager.update_service_config('wireguard', _wg_svc)
|
config_manager.update_service_config('wireguard', _wg_svc)
|
||||||
wireguard_manager.update_config({'port': new_wg})
|
wireguard_manager.apply_config({'port': new_wg})
|
||||||
port_changed_containers.add('wireguard')
|
port_changed_containers.add('wireguard')
|
||||||
port_change_messages.append(f'wireguard_port: {old_wg} → {new_wg}')
|
port_change_messages.append(f'wireguard_port: {old_wg} → {new_wg}')
|
||||||
|
|
||||||
|
|||||||
@@ -538,15 +538,16 @@ class WireGuardManager(BaseServiceManager):
|
|||||||
pass
|
pass
|
||||||
return ip
|
return ip
|
||||||
|
|
||||||
def check_port_open(self, port: int = DEFAULT_PORT) -> bool:
|
def check_port_open(self, port: int = None) -> bool:
|
||||||
"""Check if WireGuard is running and listening on the UDP port."""
|
"""Check if WireGuard is running and listening on the configured UDP port."""
|
||||||
# Primary: check if wg0 interface is up (means port IS listening)
|
configured_port = port if port is not None else self._get_configured_port()
|
||||||
|
# Primary: verify wg0 is up AND listening on the configured port
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
['docker', 'exec', 'cell-wireguard', 'wg', 'show', 'wg0'],
|
['docker', 'exec', 'cell-wireguard', 'wg', 'show', 'wg0'],
|
||||||
capture_output=True, text=True, timeout=5,
|
capture_output=True, text=True, timeout=5,
|
||||||
)
|
)
|
||||||
if result.returncode == 0 and 'listening port' in result.stdout.lower():
|
if result.returncode == 0 and f'listening port: {configured_port}' in result.stdout.lower():
|
||||||
return True
|
return True
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|||||||
@@ -227,5 +227,66 @@ class TestWireGuardEndpoints(unittest.TestCase):
|
|||||||
self.assertIn('error', json.loads(r.data))
|
self.assertIn('error', json.loads(r.data))
|
||||||
|
|
||||||
|
|
||||||
|
class TestWireGuardPortPropagation(unittest.TestCase):
|
||||||
|
"""
|
||||||
|
Test that changing wireguard_port via the identity config path calls
|
||||||
|
wireguard_manager.apply_config (writes wg0.conf), not just update_config
|
||||||
|
(which only saves a JSON file and never touches wg0.conf).
|
||||||
|
"""
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
app.config['TESTING'] = True
|
||||||
|
self.client = app.test_client()
|
||||||
|
|
||||||
|
@patch('app._set_pending_restart')
|
||||||
|
@patch('app.wireguard_manager')
|
||||||
|
@patch('app.config_manager')
|
||||||
|
def test_wireguard_port_identity_change_calls_apply_config(
|
||||||
|
self, mock_cm, mock_wg, mock_pending
|
||||||
|
):
|
||||||
|
"""wireguard_port in identity update must call apply_config, not just update_config."""
|
||||||
|
mock_cm.configs = {
|
||||||
|
'_identity': {'wireguard_port': 51820, 'ip_range': '10.0.0.0/24'},
|
||||||
|
'wireguard': {'port': 51820},
|
||||||
|
}
|
||||||
|
mock_cm.service_schemas = {}
|
||||||
|
mock_cm.update_service_config.return_value = None
|
||||||
|
mock_cm._save_all_configs.return_value = None
|
||||||
|
mock_wg.apply_config.return_value = {'restarted': [], 'warnings': []}
|
||||||
|
mock_pending.return_value = None
|
||||||
|
|
||||||
|
r = self.client.put(
|
||||||
|
'/api/config',
|
||||||
|
data=json.dumps({'wireguard_port': 51821}),
|
||||||
|
content_type='application/json',
|
||||||
|
)
|
||||||
|
self.assertEqual(r.status_code, 200)
|
||||||
|
mock_wg.apply_config.assert_called_once_with({'port': 51821})
|
||||||
|
|
||||||
|
@patch('app._set_pending_restart')
|
||||||
|
@patch('app.wireguard_manager')
|
||||||
|
@patch('app.config_manager')
|
||||||
|
def test_wireguard_port_same_value_does_not_call_apply_config(
|
||||||
|
self, mock_cm, mock_wg, mock_pending
|
||||||
|
):
|
||||||
|
"""apply_config must NOT be called when the new port equals the current port."""
|
||||||
|
mock_cm.configs = {
|
||||||
|
'_identity': {'wireguard_port': 51820, 'ip_range': '10.0.0.0/24'},
|
||||||
|
'wireguard': {'port': 51820},
|
||||||
|
}
|
||||||
|
mock_cm.service_schemas = {}
|
||||||
|
mock_cm.update_service_config.return_value = None
|
||||||
|
mock_cm._save_all_configs.return_value = None
|
||||||
|
mock_pending.return_value = None
|
||||||
|
|
||||||
|
r = self.client.put(
|
||||||
|
'/api/config',
|
||||||
|
data=json.dumps({'wireguard_port': 51820}),
|
||||||
|
content_type='application/json',
|
||||||
|
)
|
||||||
|
self.assertEqual(r.status_code, 200)
|
||||||
|
mock_wg.apply_config.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
if __name__ == '__main__':
|
if __name__ == '__main__':
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
@@ -522,6 +522,29 @@ class TestWireGuardSysctlAndPortCheck(unittest.TestCase):
|
|||||||
result = self.wg.check_port_open()
|
result = self.wg.check_port_open()
|
||||||
self.assertTrue(result)
|
self.assertTrue(result)
|
||||||
|
|
||||||
|
@patch('subprocess.run')
|
||||||
|
def test_check_port_open_wrong_port_returns_false(self, mock_run):
|
||||||
|
# wg0 is up but listening on 51820 while wg0.conf says 51821 — must return False
|
||||||
|
mock_run.return_value.returncode = 0
|
||||||
|
mock_run.return_value.stdout = 'interface: wg0\n listening port: 51820\n'
|
||||||
|
# Write wg0.conf with a different port so _get_configured_port() returns 51821
|
||||||
|
cfg_path = os.path.join(self.wg.wireguard_dir, 'wg0.conf')
|
||||||
|
with open(cfg_path, 'w') as f:
|
||||||
|
f.write('[Interface]\nListenPort = 51821\nPrivateKey = abc\n')
|
||||||
|
self.assertFalse(self.wg.check_port_open())
|
||||||
|
|
||||||
|
@patch('subprocess.run')
|
||||||
|
def test_check_port_open_explicit_port_matches(self, mock_run):
|
||||||
|
mock_run.return_value.returncode = 0
|
||||||
|
mock_run.return_value.stdout = 'interface: wg0\n listening port: 12345\n'
|
||||||
|
self.assertTrue(self.wg.check_port_open(port=12345))
|
||||||
|
|
||||||
|
@patch('subprocess.run')
|
||||||
|
def test_check_port_open_explicit_port_mismatch(self, mock_run):
|
||||||
|
mock_run.return_value.returncode = 0
|
||||||
|
mock_run.return_value.stdout = 'interface: wg0\n listening port: 51820\n'
|
||||||
|
self.assertFalse(self.wg.check_port_open(port=51821))
|
||||||
|
|
||||||
# ── get_peer_status ───────────────────────────────────────────────────────
|
# ── get_peer_status ───────────────────────────────────────────────────────
|
||||||
|
|
||||||
@patch('subprocess.run')
|
@patch('subprocess.run')
|
||||||
|
|||||||
Reference in New Issue
Block a user