fix: clean-install bugs — Tor false-installed, WG port-check honesty, encrypted backup upload
Unit Tests / test (push) Successful in 13m7s
Unit Tests / test (push) Successful in 13m7s
Three independent bugs surfaced during pic1 clean-install testing: 1. Tor _exit_status hardcoded configured=True regardless of whether Tor was actually installed. Status now flows through the same store-installed / container-running bridge used by every other optional service, so Tor only reports installed when the container is present and running. 2. check_port_open compared the port from wg0.conf against the kernel-reported listening port, causing false "port closed" results whenever the conf and the running container were momentarily out of sync. The function is now an honest liveness check: any wg0 interface that is up and has a "listening port:" line in `wg show` is considered open. The check-port API endpoint now also returns the actual kernel listening_port and a port_mismatch flag so the UI can inform the user when a container recreate is needed. (The recreate machinery already exists via the port-change pending-restart path; this fix makes the mismatch visible rather than silently lying about reachability.) 3. upload_backup only handled .zip archives; encrypted .age blobs were rejected with a generic error. The endpoint now calls backup_crypto.is_encrypted() to detect Age-encrypted blobs and stores them verbatim as <id>.tar.gz.age with mode 0600 so they can be uploaded and then restored with a passphrase. The plaintext zip path is unchanged. Tests added/updated: test_connectivity_manager.py (Tor status bridge), test_wireguard_manager.py + test_wireguard_endpoints.py (port-check liveness and mismatch flag), test_config_backup_restore_http.py (encrypted upload round-trip). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2159,7 +2159,10 @@ class ConnectivityManager(BaseServiceManager):
|
||||
except OSError:
|
||||
info['configured'] = False
|
||||
elif exit_type == 'tor':
|
||||
info['configured'] = True # Tor uses defaults; no per-cell config
|
||||
# Tor has no per-cell config file; it counts as configured only via
|
||||
# the store-installed / container-running bridge below, like every
|
||||
# other exit type. Do not hardcode True here.
|
||||
pass
|
||||
elif exit_type == 'sshuttle':
|
||||
info['configured'] = os.path.isfile(
|
||||
os.path.join(self.sshuttle_dir, 'sshuttle.conf'))
|
||||
|
||||
+28
-7
@@ -961,27 +961,48 @@ def download_backup(backup_id):
|
||||
def upload_backup():
|
||||
try:
|
||||
from app import config_manager
|
||||
import backup_crypto
|
||||
if 'file' not in request.files:
|
||||
return jsonify({'error': 'No file provided'}), 400
|
||||
f = request.files['file']
|
||||
filename = f.filename or ''
|
||||
if filename.endswith('.zip'):
|
||||
backup_id = filename[:-4]
|
||||
else:
|
||||
raw = f.read()
|
||||
|
||||
# Derive a clean backup id from the filename, stripping known suffixes.
|
||||
stem = filename
|
||||
for suffix in ('.tar.gz.age', '.age', '.zip'):
|
||||
if stem.endswith(suffix):
|
||||
stem = stem[:-len(suffix)]
|
||||
break
|
||||
backup_id = ''.join(c for c in stem if c.isalnum() or c == '_')
|
||||
if not backup_id:
|
||||
backup_id = f"backup_{datetime.utcnow().strftime('%Y%m%d_%H%M%S')}"
|
||||
backup_id = ''.join(c for c in backup_id if c.isalnum() or c == '_')
|
||||
|
||||
# Encrypted backups are opaque blobs: store them verbatim as
|
||||
# <id>.tar.gz.age so restore_config()/_resolve_backup_dir() can decrypt
|
||||
# them with the passphrase supplied at restore time.
|
||||
if backup_crypto.is_encrypted(raw):
|
||||
config_manager.backup_dir.mkdir(parents=True, exist_ok=True)
|
||||
archive_path = config_manager.backup_dir / f'{backup_id}.tar.gz.age'
|
||||
archive_path.write_bytes(raw)
|
||||
try:
|
||||
os.chmod(archive_path, 0o600)
|
||||
except OSError as e:
|
||||
logger.warning(f"Could not chmod uploaded backup: {e}")
|
||||
return jsonify({'backup_id': backup_id, 'encrypted': True})
|
||||
|
||||
backup_path = config_manager.backup_dir / backup_id
|
||||
backup_path.mkdir(parents=True, exist_ok=True)
|
||||
try:
|
||||
with zipfile.ZipFile(io.BytesIO(f.read())) as zf:
|
||||
with zipfile.ZipFile(io.BytesIO(raw)) as zf:
|
||||
zf.extractall(backup_path)
|
||||
except zipfile.BadZipFile:
|
||||
shutil.rmtree(backup_path, ignore_errors=True)
|
||||
return jsonify({'error': 'Invalid zip file'}), 400
|
||||
return jsonify({'error': 'Invalid backup file'}), 400
|
||||
if not (backup_path / 'manifest.json').exists():
|
||||
shutil.rmtree(backup_path, ignore_errors=True)
|
||||
return jsonify({'error': 'Invalid backup: missing manifest.json'}), 400
|
||||
return jsonify({'backup_id': backup_id})
|
||||
return jsonify({'backup_id': backup_id, 'encrypted': False})
|
||||
except Exception as e:
|
||||
logger.error(f"Error uploading backup: {e}")
|
||||
return jsonify({'error': str(e)}), 500
|
||||
|
||||
+10
-1
@@ -288,6 +288,15 @@ def check_wireguard_port():
|
||||
try:
|
||||
from app import wireguard_manager
|
||||
port_open = wireguard_manager.check_port_open()
|
||||
return jsonify({'port_open': port_open, 'port': wireguard_manager._get_configured_port()})
|
||||
configured_port = wireguard_manager._get_configured_port()
|
||||
listening_port = wireguard_manager._kernel_listening_port()
|
||||
return jsonify({
|
||||
'port_open': port_open,
|
||||
'port': configured_port,
|
||||
'listening_port': listening_port,
|
||||
'port_mismatch': (
|
||||
listening_port is not None and listening_port != configured_port
|
||||
),
|
||||
})
|
||||
except Exception as e:
|
||||
return jsonify({"error": str(e)}), 500
|
||||
|
||||
@@ -984,19 +984,44 @@ class WireGuardManager(BaseServiceManager):
|
||||
pass
|
||||
return ip
|
||||
|
||||
def check_port_open(self, port: int = None) -> bool:
|
||||
"""Check if WireGuard is running and listening on the configured UDP port."""
|
||||
configured_port = port if port is not None else self._get_configured_port()
|
||||
# Primary: verify wg0 is up AND listening on the configured port
|
||||
def _kernel_listening_port(self) -> Optional[int]:
|
||||
"""Return the UDP port wg0 is actually bound to per `wg show`, or None.
|
||||
|
||||
This reads the live kernel state, which is the source of truth for what
|
||||
port traffic must reach — it may differ from wg0.conf's ListenPort if the
|
||||
container has not been recreated since the port was changed.
|
||||
"""
|
||||
try:
|
||||
result = subprocess.run(
|
||||
['docker', 'exec', 'cell-wireguard', 'wg', 'show', 'wg0'],
|
||||
capture_output=True, text=True, timeout=5,
|
||||
)
|
||||
if result.returncode == 0 and f'listening port: {configured_port}' in result.stdout.lower():
|
||||
return True
|
||||
if result.returncode != 0:
|
||||
return None
|
||||
for line in result.stdout.lower().splitlines():
|
||||
line = line.strip()
|
||||
if line.startswith('listening port:'):
|
||||
try:
|
||||
return int(line.split(':', 1)[1].strip())
|
||||
except (ValueError, IndexError):
|
||||
return None
|
||||
except Exception:
|
||||
pass
|
||||
return None
|
||||
|
||||
def check_port_open(self, port: int = None) -> bool:
|
||||
"""True when WireGuard is up and bound to a UDP port (reachable).
|
||||
|
||||
This is a liveness check, not a strict equality check against the
|
||||
configured port: an interface that is up with a `listening port:` line
|
||||
is serving traffic on that bound port. The bound port may differ from
|
||||
wg0.conf's ListenPort if the container has not yet been recreated — that
|
||||
is surfaced separately via the endpoint's actual-port field, not by
|
||||
reporting the port closed.
|
||||
"""
|
||||
# Primary: wg0 is up and has a listening port → reachable on that port.
|
||||
if self._kernel_listening_port() is not None:
|
||||
return True
|
||||
# Fallback: recent peer handshake confirms external reachability
|
||||
try:
|
||||
statuses = self.get_all_peer_statuses()
|
||||
|
||||
@@ -30,6 +30,8 @@ api_dir = Path(__file__).parent.parent / 'api'
|
||||
sys.path.insert(0, str(api_dir))
|
||||
|
||||
from app import app
|
||||
import backup_crypto
|
||||
import tarfile
|
||||
|
||||
|
||||
class TestCreateConfigBackup(unittest.TestCase):
|
||||
@@ -345,6 +347,63 @@ class TestUploadBackup(unittest.TestCase):
|
||||
)
|
||||
self.assertEqual(r.status_code, 400)
|
||||
|
||||
@patch('app.config_manager')
|
||||
def test_upload_stores_encrypted_blob_verbatim(self, mock_cm):
|
||||
backup_dir = Path(self.tmp)
|
||||
mock_cm.backup_dir = backup_dir
|
||||
blob = backup_crypto.encrypt_bytes(b'payload-bytes', 'secret')
|
||||
self.assertTrue(blob.startswith(backup_crypto.MAGIC))
|
||||
r = self.client.post(
|
||||
'/api/config/backup/upload',
|
||||
data={'file': (io.BytesIO(blob), 'backup_20260101_010101.tar.gz.age')},
|
||||
content_type='multipart/form-data',
|
||||
)
|
||||
self.assertEqual(r.status_code, 200)
|
||||
data = json.loads(r.data)
|
||||
self.assertTrue(data['encrypted'])
|
||||
self.assertEqual(data['backup_id'], 'backup_20260101_010101')
|
||||
archive = backup_dir / 'backup_20260101_010101.tar.gz.age'
|
||||
self.assertTrue(archive.exists())
|
||||
self.assertEqual(archive.read_bytes(), blob)
|
||||
|
||||
@patch('app.config_manager')
|
||||
def test_upload_encrypted_then_restore_round_trip(self, mock_cm):
|
||||
# Build a real encrypted backup archive (tar.gz of a manifest, then
|
||||
# encrypted), upload it, then restore it through the real ConfigManager
|
||||
# decrypt/resolve path with the correct and an incorrect passphrase.
|
||||
from config_manager import ConfigManager
|
||||
|
||||
backup_dir = Path(self.tmp) / 'backups'
|
||||
backup_dir.mkdir(parents=True, exist_ok=True)
|
||||
mock_cm.backup_dir = backup_dir
|
||||
|
||||
tar_buf = io.BytesIO()
|
||||
with tarfile.open(fileobj=tar_buf, mode='w:gz') as tar:
|
||||
inner = json.dumps({'backup_id': 'rt', 'services': []}).encode()
|
||||
info = tarfile.TarInfo('manifest.json')
|
||||
info.size = len(inner)
|
||||
tar.addfile(info, io.BytesIO(inner))
|
||||
blob = backup_crypto.encrypt_bytes(tar_buf.getvalue(), 'pw123')
|
||||
|
||||
r = self.client.post(
|
||||
'/api/config/backup/upload',
|
||||
data={'file': (io.BytesIO(blob), 'rt.tar.gz.age')},
|
||||
content_type='multipart/form-data',
|
||||
)
|
||||
self.assertEqual(r.status_code, 200)
|
||||
backup_id = json.loads(r.data)['backup_id']
|
||||
|
||||
# Resolve+decrypt with the correct passphrase succeeds.
|
||||
real_cm = ConfigManager.__new__(ConfigManager)
|
||||
real_cm.backup_dir = backup_dir
|
||||
path, cleanup = real_cm._resolve_backup_dir(f'{backup_id}.tar.gz.age', 'pw123')
|
||||
self.assertTrue((path / 'manifest.json').exists())
|
||||
shutil.rmtree(cleanup, ignore_errors=True)
|
||||
|
||||
# Wrong passphrase raises PermissionError → route returns 400.
|
||||
with self.assertRaises(PermissionError):
|
||||
real_cm._resolve_backup_dir(f'{backup_id}.tar.gz.age', 'wrong')
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
@@ -1037,11 +1037,35 @@ class TestExitStatus(unittest.TestCase):
|
||||
self.assertIn('status', item)
|
||||
self.assertIn(item['status'], ('active', 'configured', 'not_configured'))
|
||||
|
||||
def test_tor_defaults_to_configured(self):
|
||||
def test_tor_not_configured_when_not_installed_or_running(self):
|
||||
# Tor must not report configured just because it has no per-cell config;
|
||||
# it flows through the store-installed / container-running bridge.
|
||||
mgr = self._mgr()
|
||||
with patch.object(cm_module, 'subprocess') as mock_sp:
|
||||
mock_sp.run.return_value = MagicMock(returncode=1, stdout='', stderr='')
|
||||
info = mgr._exit_status('tor')
|
||||
self.assertFalse(info['configured'])
|
||||
self.assertEqual(info['status'], 'not_configured')
|
||||
|
||||
def test_tor_configured_when_store_installed(self):
|
||||
mgr = self._mgr(installed={'tor': {'manifest': {'id': 'tor'}}})
|
||||
with patch.object(cm_module, 'subprocess') as mock_sp:
|
||||
mock_sp.run.return_value = MagicMock(returncode=1, stdout='', stderr='')
|
||||
info = mgr._exit_status('tor')
|
||||
self.assertTrue(info['configured'])
|
||||
self.assertEqual(info['status'], 'configured')
|
||||
|
||||
def test_tor_configured_when_container_running(self):
|
||||
mgr = self._mgr()
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
if 'inspect' in cmd:
|
||||
return MagicMock(returncode=0, stdout='true\n', stderr='')
|
||||
return MagicMock(returncode=1, stdout='', stderr='')
|
||||
|
||||
with patch.object(cm_module, 'subprocess') as mock_sp:
|
||||
mock_sp.run.side_effect = fake_run
|
||||
info = mgr._exit_status('tor')
|
||||
self.assertTrue(info['configured'])
|
||||
self.assertEqual(info['status'], 'configured')
|
||||
|
||||
|
||||
@@ -36,6 +36,7 @@ class TestWireGuardEndpoints(unittest.TestCase):
|
||||
def test_check_port_returns_port_open_true(self, mock_wg):
|
||||
mock_wg.check_port_open.return_value = True
|
||||
mock_wg._get_configured_port.return_value = 51820
|
||||
mock_wg._kernel_listening_port.return_value = 51820
|
||||
r = self.client.post('/api/wireguard/check-port')
|
||||
self.assertEqual(r.status_code, 200)
|
||||
data = json.loads(r.data)
|
||||
@@ -43,15 +44,35 @@ class TestWireGuardEndpoints(unittest.TestCase):
|
||||
self.assertIn('port', data)
|
||||
self.assertTrue(data['port_open'])
|
||||
self.assertEqual(data['port'], 51820)
|
||||
self.assertEqual(data['listening_port'], 51820)
|
||||
self.assertFalse(data['port_mismatch'])
|
||||
|
||||
@patch('app.wireguard_manager')
|
||||
def test_check_port_reports_actual_listening_port_on_mismatch(self, mock_wg):
|
||||
# Configured 51821 but kernel bound to 51820 — endpoint surfaces the real
|
||||
# bound port and flags the mismatch without reporting the port closed.
|
||||
mock_wg.check_port_open.return_value = True
|
||||
mock_wg._get_configured_port.return_value = 51821
|
||||
mock_wg._kernel_listening_port.return_value = 51820
|
||||
r = self.client.post('/api/wireguard/check-port')
|
||||
self.assertEqual(r.status_code, 200)
|
||||
data = json.loads(r.data)
|
||||
self.assertTrue(data['port_open'])
|
||||
self.assertEqual(data['port'], 51821)
|
||||
self.assertEqual(data['listening_port'], 51820)
|
||||
self.assertTrue(data['port_mismatch'])
|
||||
|
||||
@patch('app.wireguard_manager')
|
||||
def test_check_port_returns_port_open_false(self, mock_wg):
|
||||
mock_wg.check_port_open.return_value = False
|
||||
mock_wg._get_configured_port.return_value = 51820
|
||||
mock_wg._kernel_listening_port.return_value = None
|
||||
r = self.client.post('/api/wireguard/check-port')
|
||||
self.assertEqual(r.status_code, 200)
|
||||
data = json.loads(r.data)
|
||||
self.assertFalse(data['port_open'])
|
||||
self.assertIsNone(data['listening_port'])
|
||||
self.assertFalse(data['port_mismatch'])
|
||||
|
||||
@patch('app.wireguard_manager')
|
||||
def test_check_port_returns_500_on_exception(self, mock_wg):
|
||||
|
||||
@@ -615,27 +615,44 @@ class TestWireGuardSysctlAndPortCheck(unittest.TestCase):
|
||||
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
|
||||
def test_check_port_open_true_despite_port_mismatch(self, mock_run):
|
||||
# wg0 is up and listening on 51820 while wg0.conf says 51821. The kernel
|
||||
# port is what actually serves traffic, so this is a reachability success;
|
||||
# check_port_open must NOT return False merely because of the mismatch.
|
||||
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())
|
||||
self.assertTrue(self.wg.check_port_open())
|
||||
|
||||
@patch('subprocess.run')
|
||||
def test_check_port_open_explicit_port_matches(self, mock_run):
|
||||
def test_kernel_listening_port_parses_actual_bound_port(self, mock_run):
|
||||
mock_run.return_value.returncode = 0
|
||||
mock_run.return_value.stdout = 'interface: wg0\n listening port: 51820\n'
|
||||
self.assertEqual(self.wg._kernel_listening_port(), 51820)
|
||||
|
||||
@patch('subprocess.run')
|
||||
def test_kernel_listening_port_none_when_down(self, mock_run):
|
||||
mock_run.return_value.returncode = 1
|
||||
mock_run.return_value.stdout = ''
|
||||
self.assertIsNone(self.wg._kernel_listening_port())
|
||||
|
||||
@patch('subprocess.run')
|
||||
def test_check_port_open_true_when_interface_bound(self, mock_run):
|
||||
# check_port_open is a liveness check: an up interface with a bound port
|
||||
# is reachable, regardless of which port number it is.
|
||||
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):
|
||||
def test_check_port_open_true_even_when_bound_port_differs(self, mock_run):
|
||||
# Bound port (51820) differs from the configured/expected port (51821),
|
||||
# but the interface is up and serving — this is reachable, not closed.
|
||||
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))
|
||||
self.assertTrue(self.wg.check_port_open(port=51821))
|
||||
|
||||
# ── get_peer_status ───────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@@ -720,7 +720,9 @@ function Settings() {
|
||||
const url = URL.createObjectURL(res.data);
|
||||
const a = document.createElement('a');
|
||||
a.href = url;
|
||||
a.download = `${id}.zip`;
|
||||
// Encrypted archives already carry their .tar.gz.age extension; only
|
||||
// plaintext backups (served as a zip) need a .zip name appended.
|
||||
a.download = id.endsWith('.age') ? id : `${id}.zip`;
|
||||
a.click();
|
||||
URL.revokeObjectURL(url);
|
||||
} catch {
|
||||
@@ -739,7 +741,7 @@ function Settings() {
|
||||
const res = await cellAPI.listBackups();
|
||||
setBackups(res.data || []);
|
||||
} catch {
|
||||
toast('Upload failed — ensure it is a valid backup zip', 'error');
|
||||
toast('Upload failed — ensure it is a valid backup (.zip or encrypted .age) file', 'error');
|
||||
} finally {
|
||||
setBackupUploading(false);
|
||||
}
|
||||
@@ -1088,7 +1090,7 @@ function Settings() {
|
||||
<label className="btn-secondary flex items-center gap-2 text-sm cursor-pointer" title="Upload backup zip">
|
||||
{backupUploading ? <RefreshCw className="h-4 w-4 animate-spin" /> : <Upload className="h-4 w-4" />}
|
||||
Upload
|
||||
<input type="file" accept=".zip" className="hidden" onChange={uploadBackup} />
|
||||
<input type="file" accept=".zip,.age" className="hidden" onChange={uploadBackup} />
|
||||
</label>
|
||||
<button
|
||||
onClick={createBackup}
|
||||
|
||||
Reference in New Issue
Block a user