diff --git a/api/app.py b/api/app.py index 9cc4710..bb47520 100644 --- a/api/app.py +++ b/api/app.py @@ -438,6 +438,22 @@ def update_config(): identity_keys = {'cell_name', 'domain', 'ip_range', 'wireguard_port'} identity_updates = {k: v for k, v in data.items() if k in identity_keys} + # Validate cell_name — must be non-empty and at most 255 characters (DNS limit) + if 'cell_name' in identity_updates: + v = str(identity_updates['cell_name']) + if len(v) > 255: + return jsonify({'error': 'cell_name must be 255 characters or fewer'}), 400 + if not v: + return jsonify({'error': 'cell_name cannot be empty'}), 400 + + # Validate domain — must be non-empty and at most 255 characters (DNS limit) + if 'domain' in identity_updates: + v = str(identity_updates['domain']) + if len(v) > 255: + return jsonify({'error': 'domain must be 255 characters or fewer'}), 400 + if not v: + return jsonify({'error': 'domain cannot be empty'}), 400 + # Validate ip_range — must be a valid CIDR within an RFC-1918 range if 'ip_range' in identity_updates: import ipaddress as _ipa diff --git a/tests/integration/test_apply_propagation.py b/tests/integration/test_apply_propagation.py new file mode 100644 index 0000000..4de60a0 --- /dev/null +++ b/tests/integration/test_apply_propagation.py @@ -0,0 +1,350 @@ +""" +Apply-propagation integration tests. + +Verifies the full save → pending → apply → verify lifecycle: + + 1. GET /api/config/pending — reports needs_restart + 2. PUT /api/config — a port change marks pending + 3. DELETE /api/config/pending — discard clears the flag + 4. POST /api/config/apply — apply returns 200, clears pending, + restarts only the affected container + 5. After restart, GET /api/config reflects the saved change + +Routes used (confirmed in api/app.py): + GET /api/config/pending — {needs_restart, changed_at, changes, containers} + DELETE /api/config/pending — discard without restart + POST /api/config/apply — trigger restart; returns {message, restart_in_progress} + GET /health — {status: "healthy"} (used to poll for recovery) + +Why calendar.port? +------------------ +Changing calendar.port is the safest apply-test trigger because: + - It falls under the _PORT_CHANGE_MAP in app.py and therefore sets + needs_restart=true pointing only at the ['radicale'] container. + - The API container itself is NOT in that list, so the Flask process + stays up during apply — no connection gap to handle. + - cell_name / domain changes are applied immediately to DNS and do NOT + set needs_restart, so they cannot be used to exercise the pending path. + +Run with: pytest tests/integration/test_apply_propagation.py -v +""" +import time +import sys +import os + +import pytest +import requests +from requests.exceptions import ConnectionError, Timeout + +sys.path.insert(0, os.path.dirname(__file__)) +from conftest import API_BASE + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +_POLL_INTERVAL = 2 # seconds between health polls +_HEALTH_TIMEOUT = 90 # max seconds to wait for healthy after apply + +# Two distinct valid calendar ports used as before/after values. +# Neither conflicts with any other default service port. +_CAL_PORT_A = 5232 # the standard Radicale default +_CAL_PORT_B = 5233 # an alternate safe value used as the "changed" state + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def get(path, **kw): + return requests.get(f"{API_BASE}{path}", **kw) + + +def put(path, **kw): + return requests.put(f"{API_BASE}{path}", **kw) + + +def post(path, **kw): + return requests.post(f"{API_BASE}{path}", **kw) + + +def delete(path, **kw): + return requests.delete(f"{API_BASE}{path}", **kw) + + +def wait_for_healthy(timeout: int = _HEALTH_TIMEOUT) -> bool: + """ + Poll GET /health until it returns {"status": "healthy"} or timeout expires. + Connection errors are swallowed so the loop survives a container restart. + Returns True if healthy within timeout, False otherwise. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + try: + r = requests.get(f"{API_BASE}/health", timeout=5) + if r.status_code == 200 and r.json().get("status") == "healthy": + return True + except (ConnectionError, Timeout): + pass # API may be momentarily unreachable — keep trying + time.sleep(_POLL_INTERVAL) + return False + + +def pending_state() -> dict: + """Return the current /api/config/pending response body.""" + return get("/api/config/pending").json() + + +def current_calendar_port() -> int: + """Read calendar.port from the live config.""" + cfg = get("/api/config").json() + svc = cfg.get("service_configs", {}).get("calendar", {}) + return int(svc.get("port", _CAL_PORT_A)) + + +def set_calendar_port(port: int) -> requests.Response: + """PUT calendar.port and return the response.""" + return put("/api/config", json={"calendar": {"port": port}}) + + +# --------------------------------------------------------------------------- +# TestPendingState (no container restarts) +# --------------------------------------------------------------------------- + +class TestPendingState: + """Tests that verify pending-state semantics without triggering an apply.""" + + def test_pending_starts_false(self): + """ + After discarding any stale pending changes, GET /api/config/pending + must return needs_restart=false. + """ + delete("/api/config/pending") + data = pending_state() + assert data["needs_restart"] is False, ( + f"Expected needs_restart=false at baseline, got: {data}" + ) + + def test_save_config_sets_pending(self): + """ + PUT /api/config with a changed calendar.port must flip needs_restart + to true. The change is discarded (not applied) so no restart occurs. + """ + original_port = current_calendar_port() + # Pick whichever alternate port is not currently in use. + new_port = _CAL_PORT_B if original_port == _CAL_PORT_A else _CAL_PORT_A + + # Start from a clean state. + delete("/api/config/pending") + assert pending_state()["needs_restart"] is False, "Could not clear pending state" + + try: + r = set_calendar_port(new_port) + assert r.status_code == 200, f"PUT /api/config failed: {r.text}" + + data = pending_state() + assert data["needs_restart"] is True, ( + f"Expected needs_restart=true after port change, got: {data}" + ) + assert isinstance(data["changes"], list) + assert len(data["changes"]) > 0, ( + "changes list is empty even though needs_restart is true" + ) + # The pending restart should be scoped to the radicale container. + assert "radicale" in data.get("containers", []) or data.get("containers") == ["*"], ( + f"Expected 'radicale' in pending containers, got: {data.get('containers')}" + ) + finally: + set_calendar_port(original_port) + delete("/api/config/pending") + + def test_discard_clears_pending(self): + """ + DELETE /api/config/pending after a config change must reset + needs_restart to false and empty the changes list without + restarting any containers. + """ + original_port = current_calendar_port() + new_port = _CAL_PORT_B if original_port == _CAL_PORT_A else _CAL_PORT_A + + delete("/api/config/pending") # start clean + + try: + r = set_calendar_port(new_port) + assert r.status_code == 200, f"PUT /api/config failed: {r.text}" + assert pending_state()["needs_restart"] is True, ( + "needs_restart not set after port change — cannot test discard" + ) + + dr = delete("/api/config/pending") + assert dr.status_code == 200, ( + f"DELETE /api/config/pending returned {dr.status_code}: {dr.text}" + ) + body = dr.json() + assert "message" in body, f"Discard response missing 'message': {body}" + + data = pending_state() + assert data["needs_restart"] is False, ( + f"needs_restart still true after discard: {data}" + ) + assert data["changes"] == [], ( + f"changes list not empty after discard: {data}" + ) + finally: + set_calendar_port(original_port) + delete("/api/config/pending") + + +# --------------------------------------------------------------------------- +# TestApplyAndVerify (triggers actual container restart) +# --------------------------------------------------------------------------- + +class TestApplyAndVerify: + """ + Tests that call POST /api/config/apply. + + Because a calendar.port change only restarts the 'radicale' container + (not the API container), the API stays up throughout. wait_for_healthy() + is still called after each apply to confirm full readiness before making + assertions. + + Every test restores the original calendar port in a finally block. + """ + + def test_apply_endpoint_exists(self): + """ + POST /api/config/apply must return 200 with a 'message' key even + when there is nothing pending (documented no-op behaviour). + """ + delete("/api/config/pending") + + r = post("/api/config/apply") + assert r.status_code == 200, ( + f"Expected 200 from POST /api/config/apply (no-op), " + f"got {r.status_code}: {r.text}" + ) + body = r.json() + assert "message" in body, f"Response body missing 'message' key: {body}" + + def test_apply_clears_pending(self): + """ + After saving a config change and calling POST /api/config/apply, + GET /api/config/pending must return needs_restart=false. + + app.py clears the pending flag synchronously before spawning the + restart thread, so the flag is cleared as soon as the apply HTTP + response is received — regardless of when containers finish starting. + """ + original_port = current_calendar_port() + new_port = _CAL_PORT_B if original_port == _CAL_PORT_A else _CAL_PORT_A + + delete("/api/config/pending") + + try: + r = set_calendar_port(new_port) + assert r.status_code == 200, f"PUT /api/config failed: {r.text}" + assert pending_state()["needs_restart"] is True, ( + "needs_restart not set — cannot verify that apply clears it" + ) + + ar = post("/api/config/apply") + assert ar.status_code == 200, ( + f"POST /api/config/apply returned {ar.status_code}: {ar.text}" + ) + apply_body = ar.json() + assert "message" in apply_body, ( + f"Apply response missing 'message': {apply_body}" + ) + + # Wait for the API to confirm healthy (radicale restart in background). + recovered = wait_for_healthy() + assert recovered, ( + f"API did not return healthy within {_HEALTH_TIMEOUT}s after apply" + ) + + data = pending_state() + assert data["needs_restart"] is False, ( + f"needs_restart still true after apply + recovery: {data}" + ) + + finally: + set_calendar_port(original_port) + delete("/api/config/pending") + wait_for_healthy(_HEALTH_TIMEOUT) + + def test_cell_name_change_persists_after_apply(self): + """ + Full lifecycle: change calendar.port → apply → wait for healthy → + GET /api/config must return the new port value. + + This confirms that the configuration written to disk before apply + survives the container restart cycle and is not rolled back. + cell_name is used as a secondary check: because cell_name changes + are applied immediately to DNS (not via pending), we verify it was + not inadvertently cleared by the apply path. + """ + original_port = current_calendar_port() + original_cfg = get("/api/config").json() + original_name = original_cfg["cell_name"] + + new_port = _CAL_PORT_B if original_port == _CAL_PORT_A else _CAL_PORT_A + + delete("/api/config/pending") + + try: + # 1. Save a new calendar port. + r = set_calendar_port(new_port) + assert r.status_code == 200, f"PUT /api/config failed: {r.text}" + + # 2. Confirm config is saved before apply. + saved_port = current_calendar_port() + assert saved_port == new_port, ( + f"calendar.port not saved before apply: got {saved_port}" + ) + + # 3. Confirm pending is set. + assert pending_state()["needs_restart"] is True, ( + "needs_restart not set after calendar.port change" + ) + + # 4. Apply. + ar = post("/api/config/apply") + assert ar.status_code == 200, ( + f"POST /api/config/apply returned {ar.status_code}: {ar.text}" + ) + apply_body = ar.json() + assert "message" in apply_body, ( + f"Apply response missing 'message': {apply_body}" + ) + + # 5. Wait for healthy. + recovered = wait_for_healthy() + assert recovered, ( + f"API did not return healthy within {_HEALTH_TIMEOUT}s after apply" + ) + + # 6. Verify the port change persists in the running config. + post_apply_port = current_calendar_port() + assert post_apply_port == new_port, ( + f"calendar.port reverted after apply: " + f"expected {new_port}, got {post_apply_port}" + ) + + # 7. Confirm that cell_name was not inadvertently cleared. + post_apply_cfg = get("/api/config").json() + assert post_apply_cfg["cell_name"] == original_name, ( + f"cell_name changed unexpectedly during apply: " + f"expected {original_name!r}, got {post_apply_cfg['cell_name']!r}" + ) + + # 8. Confirm pending is cleared. + assert pending_state()["needs_restart"] is False, ( + "needs_restart still true after apply + recovery" + ) + + finally: + # Restore the original calendar port. + set_calendar_port(original_port) + post("/api/config/apply") + wait_for_healthy(_HEALTH_TIMEOUT) + delete("/api/config/pending") diff --git a/tests/test_identity_validation.py b/tests/test_identity_validation.py new file mode 100644 index 0000000..529a8a0 --- /dev/null +++ b/tests/test_identity_validation.py @@ -0,0 +1,94 @@ +""" +Unit tests for cell_name and domain length validation in update_config(). +""" + +import sys +import os +import json + +# Ensure api/ is on the path +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', 'api')) + +import pytest +from app import app + + +@pytest.fixture +def client(): + app.config['TESTING'] = True + with app.test_client() as c: + yield c + + +def put_config(client, payload): + return client.put( + '/api/config', + data=json.dumps(payload), + content_type='application/json', + ) + + +# --------------------------------------------------------------------------- +# cell_name validation +# --------------------------------------------------------------------------- + +def test_cell_name_too_long_returns_400(client): + """cell_name > 255 characters must be rejected with 400.""" + resp = put_config(client, {'cell_name': 'a' * 256}) + assert resp.status_code == 400 + body = json.loads(resp.data) + assert 'cell_name' in body['error'] + assert '255' in body['error'] + + +def test_cell_name_exactly_255_returns_200(client): + """cell_name of exactly 255 characters must be accepted.""" + resp = put_config(client, {'cell_name': 'a' * 255}) + assert resp.status_code == 200 + + +def test_cell_name_empty_string_returns_400(client): + """Empty cell_name must be rejected with 400.""" + resp = put_config(client, {'cell_name': ''}) + assert resp.status_code == 400 + body = json.loads(resp.data) + assert 'cell_name' in body['error'] + + +def test_cell_name_valid_returns_200(client): + """A short, valid cell_name must be accepted.""" + resp = put_config(client, {'cell_name': 'mycell'}) + assert resp.status_code == 200 + + +# --------------------------------------------------------------------------- +# domain validation +# --------------------------------------------------------------------------- + +def test_domain_too_long_returns_400(client): + """domain > 255 characters must be rejected with 400.""" + resp = put_config(client, {'domain': 'b' * 256}) + assert resp.status_code == 400 + body = json.loads(resp.data) + assert 'domain' in body['error'] + assert '255' in body['error'] + + +def test_domain_exactly_255_returns_200(client): + """domain of exactly 255 characters must be accepted.""" + resp = put_config(client, {'domain': 'b' * 255}) + assert resp.status_code == 200 + + +def test_domain_empty_string_returns_400(client): + """Empty domain must be rejected with 400.""" + resp = put_config(client, {'domain': ''}) + assert resp.status_code == 400 + body = json.loads(resp.data) + assert 'domain' in body['error'] + + +def test_domain_valid_returns_200(client): + """A short, valid domain must be accepted.""" + resp = put_config(client, {'domain': 'cell.local'}) + assert resp.status_code == 200 diff --git a/webui/src/App.jsx b/webui/src/App.jsx index d0d3f35..1deb2c9 100644 --- a/webui/src/App.jsx +++ b/webui/src/App.jsx @@ -129,7 +129,9 @@ function PendingRestartBanner({ pending, onApply, onCancel }) { ); } -function App() { +// AppCore is the real application — it consumes DraftConfigContext and must +// be rendered inside DraftConfigProvider (see App below). +function AppCore() { const [isOnline, setIsOnline] = useState(false); const [isLoading, setIsLoading] = useState(true); const [pending, setPending] = useState({ needs_restart: false, changes: [] }); @@ -243,7 +245,6 @@ function App() { } return ( -
@@ -326,6 +327,13 @@ function App() {
+ ); +} + +function App() { + return ( + + ); } diff --git a/webui/src/pages/CellNetwork.jsx b/webui/src/pages/CellNetwork.jsx index 271f144..ba0e43b 100644 --- a/webui/src/pages/CellNetwork.jsx +++ b/webui/src/pages/CellNetwork.jsx @@ -175,21 +175,21 @@ export default function CellNetwork() { ) : invite ? (
-
- Cell - {invite.cell_name} +
+ Cell + {invite.cell_name}
-
- Domain - {invite.domain} +
+ Domain + {invite.domain}
-
- Endpoint - {invite.endpoint || '(no external IP)'} +
+ Endpoint + {invite.endpoint || '(no external IP)'}
-
- VPN subnet - {invite.vpn_subnet} +
+ VPN subnet + {invite.vpn_subnet}
@@ -288,14 +288,14 @@ export default function CellNetwork() { {connections.map(conn => (
-
+
-
-
- {conn.cell_name} - .{conn.domain} +
+
+ {conn.cell_name} + .{conn.domain}
-
+
Subnet: {conn.vpn_subnet} Endpoint: {conn.endpoint || '—'} {conn.last_handshake && ( diff --git a/webui/src/pages/Dashboard.jsx b/webui/src/pages/Dashboard.jsx index c39d64c..77da25d 100644 --- a/webui/src/pages/Dashboard.jsx +++ b/webui/src/pages/Dashboard.jsx @@ -244,9 +244,9 @@ function Dashboard({ isOnline }) {
-
+

Cell Name

-

{cellStatus.cell_name}

+

{cellStatus.cell_name}

diff --git a/webui/src/pages/Settings.jsx b/webui/src/pages/Settings.jsx index 8401473..ed192ed 100644 --- a/webui/src/pages/Settings.jsx +++ b/webui/src/pages/Settings.jsx @@ -457,8 +457,16 @@ function Settings() { ? 'Must be within an RFC-1918 range: 10.0.0.0/8, 172.16.0.0/12, or 192.168.0.0/16' : null; + const cellNameError = identity.cell_name && identity.cell_name.length > 255 + ? 'Cell name must be 255 characters or fewer' + : (!identity.cell_name ? 'Cell name is required' : null); + + const domainError = identity.domain && identity.domain.length > 255 + ? 'Domain must be 255 characters or fewer' + : (!identity.domain ? 'Domain is required' : null); + const saveIdentity = async () => { - if (ipRangeError) return; + if (ipRangeError || cellNameError || domainError) return; setIdentitySaving(true); try { const res = await cellAPI.updateConfig(identity); @@ -622,18 +630,20 @@ function Settings() { {/* Cell Identity */}
- + { setIdentity((i) => ({ ...i, cell_name: v })); setIdentityDirty(true); draftConfig?.setDirty('identity', true); }} placeholder="mycell" + maxLength={255} /> - + { setIdentity((i) => ({ ...i, domain: v })); setIdentityDirty(true); draftConfig?.setDirty('identity', true); }} placeholder="cell.local" + maxLength={255} /> @@ -647,7 +657,7 @@ function Settings() {