fix(updater,auth): atomic symlink flip, roll back on any failure, fail closed on a damaged users.json

updater:
- /opt/furtka/current is swapped via rename() instead of unlink+symlink,
  so a power cut can't leave the box without a current version
- OSError after the flip (disk full writing the Caddyfile) now rolls back
  too; rollback restores the previous Caddyfile and reloads caddy +
  daemon-reload
- the health check keeps polling on read timeouts / RemoteDisconnected
  instead of escaping and skipping the rollback
- run_update writes an 'error' stage for any failure (download, sha,
  disk full); the settings page shows it instead of spinning on
  'Downloading…'

auth:
- users.json that exists but is empty/corrupt/unreadable raises instead of
  reading as 'no admin yet' — before, a truncated file made /login an open
  create-admin form for the whole LAN. /login answers 503 with SSH recovery
  steps instead
- save_users fsyncs before the rename
- first-run admin creation is serialised (threaded server)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wq1hzUp4ZmCKHDuLKCnu73

assets/www/settings/index.html

@@ -171,6 +171,7 @@
             done: 'Update complete — reloading…',
             rolled_back: 'Update failed, rolled back to the previous version',
             rolled_back_manual: 'Rolled back manually',
+            error: 'Update failed — nothing was changed',
         };
 
         let pollHandle = null;
@@ -354,12 +355,13 @@
                 if (!r.ok) return;
                 const s = await r.json();
                 const label = STAGE_LABELS[s.stage] || `Stage: ${s.stage}`;
-                setStatus(label, s.stage === 'rolled_back');
+                const failed = s.stage === 'rolled_back' || s.stage === 'error';
+                setStatus(label, failed);
                 if (s.stage === 'done') {
                     clearInterval(pollHandle);
                     clearTimeout(fallbackReloadHandle);
                     setTimeout(() => location.reload(), 5000);
-                } else if (s.stage === 'rolled_back') {
+                } else if (failed) {
                     clearInterval(pollHandle);
                     clearTimeout(fallbackReloadHandle);
                     if (s.reason) {

furtka/api.py

@@ -1257,6 +1257,14 @@ MAX_BODY_BYTES = 1024 * 1024
 # a slow request (image pull, reconcile) runs. Requests that change state
 # still run one at a time, as they did on the single-threaded server.
 _MUTATION_LOCK = threading.Lock()
+# First-run admin creation is check-then-write; two setup POSTs racing on
+# the threaded server must not both create an admin.
+_SETUP_LOCK = threading.Lock()
+
+_USERS_FILE_BROKEN = (
+    "The login file on this box (/var/lib/furtka/users.json) is damaged. "
+    "Log in over SSH, delete that file, then reload this page to set a new admin password."
+)
 
 
 def _valid_app_name(name) -> bool:
@@ -1376,6 +1384,14 @@ class _Handler(BaseHTTPRequestHandler):
             return self._json(400, {"error": "password is required"})
         username = username.strip()
 
+        try:
+            with _SETUP_LOCK:
+                return self._login_or_setup(payload, username, password)
+        except auth.UsersFileError:
+            traceback.print_exc()
+            return self._json(503, {"error": _USERS_FILE_BROKEN})
+
+    def _login_or_setup(self, payload, username, password):
         if auth.setup_needed():
             # First-run setup path — create the admin account, then log
             # in. Require password2 so a typo doesn't lock the user out
@@ -1449,7 +1465,12 @@ class _Handler(BaseHTTPRequestHandler):
             # Already authed? Skip straight to the app list.
             if self._current_session() is not None:
                 return self._redirect("/apps")
-            return self._html(200, _render_login_html(auth.setup_needed()))
+            try:
+                setup = auth.setup_needed()
+            except auth.UsersFileError:
+                traceback.print_exc()
+                return self._html(503, f"<h1>Login unavailable</h1><p>{_USERS_FILE_BROKEN}</p>")
+            return self._html(200, _render_login_html(setup))
 
         # --- Auth guard for everything below -------------------------
         session = self._current_session()

furtka/auth.py

@@ -23,6 +23,7 @@ from __future__ import annotations
 
 import json
 import math
+import os
 import secrets
 import threading
 from dataclasses import dataclass
@@ -46,27 +47,33 @@ def verify_password(plain: str, hashed: str) -> bool:
     return _verify_password(plain, hashed)
 
 
+class UsersFileError(RuntimeError):
+    """users.json exists but can't be read or parsed."""
+
+
 def load_users() -> dict:
-    """Return the users dict, or {} if the file is missing or empty.
+    """Return the users dict, or {} if the file is missing.
 
     Missing-file is the expected state on first boot and on upgrades from
     pre-auth versions — callers treat empty-dict as "setup required".
+
+    A file that exists but is unreadable, empty or corrupt raises
+    UsersFileError instead: treating it as "no admin yet" would turn /login
+    into an open "create admin" form for anyone on the network.
     """
     path = users_file()
-    if not path.exists():
-        return {}
     try:
         raw = path.read_text()
-    except OSError:
+    except FileNotFoundError:
-        return {}
-    if not raw.strip():
         return {}
+    except (OSError, UnicodeDecodeError) as e:
+        raise UsersFileError(f"{path} is unreadable: {e}") from e
     try:
         data = json.loads(raw)
-    except json.JSONDecodeError:
+    except json.JSONDecodeError as e:
-        return {}
+        raise UsersFileError(f"{path} is corrupt: {e}") from e
     if not isinstance(data, dict):
-        return {}
+        raise UsersFileError(f"{path} is corrupt: not a JSON object")
     return data
 
 
@@ -80,7 +87,12 @@ def save_users(users: dict) -> None:
     path = users_file()
     path.parent.mkdir(parents=True, exist_ok=True)
     tmp = path.with_suffix(path.suffix + ".tmp")
-    tmp.write_text(json.dumps(users, indent=2) + "\n")
+    with open(tmp, "w") as fh:
+        fh.write(json.dumps(users, indent=2) + "\n")
+        # A power cut right after rename() can otherwise leave a zero-length
+        # users.json, which load_users() refuses — locking the owner out.
+        fh.flush()
+        os.fsync(fh.fileno())
     tmp.chmod(0o600)
     tmp.replace(path)
 

furtka/updater.py

@@ -29,6 +29,7 @@ the updater at a tmpdir.
 from __future__ import annotations
 
 import fcntl
+import http.client
 import json
 import os
 import shutil
@@ -265,9 +266,10 @@ def write_state(stage: str, **extra) -> None:
 
 def read_state() -> dict:
     try:
-        return json.loads(state_path().read_text())
+        data = json.loads(state_path().read_text())
-    except (FileNotFoundError, json.JSONDecodeError):
+    except (OSError, UnicodeDecodeError, json.JSONDecodeError):
         return {}
+    return data if isinstance(data, dict) else {}
 
 
 def acquire_lock():
@@ -318,13 +320,29 @@ def _health_check(url: str, deadline_s: float = 30.0) -> bool:
             # check. 5xx → genuinely broken, don't accept.
             if 400 <= e.code < 500:
                 return True
-        except urllib.error.URLError:
+        except (OSError, http.client.HTTPException):
-            # Connection refused / DNS / timeout → not up yet, retry.
+            # Connection refused / reset / timeout (URLError is an OSError;
+            # a read timeout or RemoteDisconnected is not wrapped by urllib)
+            # → not up yet, retry.
             pass
         time.sleep(1)
     return False
 
 
+def _swap_symlink(link: Path, target: str | Path) -> None:
+    """Point `link` at `target` atomically.
+
+    unlink() + symlink_to() leaves a window with no /opt/furtka/current at
+    all — a power cut there boots a box with no services. rename() over the
+    old link is atomic.
+    """
+    tmp = link.with_name(link.name + ".new")
+    if tmp.is_symlink() or tmp.exists():
+        tmp.unlink()
+    tmp.symlink_to(target)
+    os.replace(tmp, link)
+
+
 def prepare_update(check: UpdateCheck, download_dir: Path | None = None) -> tuple[Path, str]:
     """Download + verify the tarball. Returns (tarball_path, version)."""
     if not check.tarball_url or not check.sha256_url:
@@ -369,15 +387,11 @@ def apply_update(tarball: Path, version: str) -> None:
     target.chmod(0o755)
 
     write_state("swapping", latest=version)
-    previous = None
+    previous = os.readlink(current) if current.is_symlink() else None
-    if current.is_symlink():
+    old_caddyfile = _CADDYFILE_LIVE.read_text() if _CADDYFILE_LIVE.is_file() else None
-        previous = os.readlink(current)
-        current.unlink()
     try:
-        current.symlink_to(target)
+        _swap_symlink(current, target)
     except OSError as e:
-        if previous:
-            current.symlink_to(previous)
         raise UpdateError(f"symlink swap failed: {e}") from e
 
     write_state("restarting", latest=version)
@@ -393,27 +407,41 @@ def apply_update(tarball: Path, version: str) -> None:
         _run(["systemctl", "daemon-reload"])
         _run(["systemctl", "try-restart", "furtka-reconcile.service"])
         _run(["systemctl", "restart", "furtka-api.service"])
-    except UpdateError as e:
+    except (UpdateError, OSError) as e:
-        _rollback(previous, version, f"service restart failed: {e}")
+        # OSError: e.g. disk full or EACCES writing the Caddyfile — the new
-        raise
+        # version is already live, so this must roll back too.
+        _rollback(previous, version, f"service restart failed: {e}", old_caddyfile)
+        raise UpdateError(f"service restart failed: {e} — rolled back") from e
 
     write_state("verifying", latest=version)
     ok = _health_check("http://127.0.0.1:7000/api/apps", deadline_s=30.0)
     if not ok:
-        _rollback(previous, version, "health check failed after restart")
+        _rollback(previous, version, "health check failed after restart", old_caddyfile)
         raise UpdateError("new version failed health check — rolled back")
 
     write_state("done", version=version)
 
 
-def _rollback(previous_target: str | None, failed_version: str, reason: str) -> None:
+def _rollback(
+    previous_target: str | None,
+    failed_version: str,
+    reason: str,
+    old_caddyfile: str | None = None,
+) -> None:
     current = current_symlink()
     if previous_target:
-        if current.is_symlink():
+        _swap_symlink(current, previous_target)
-            current.unlink()
+        # The new release may have shipped Caddy routes the old code
-        current.symlink_to(previous_target)
+        # doesn't serve; put the previous Caddyfile back before reloading.
+        if old_caddyfile is not None:
+            try:
+                _CADDYFILE_LIVE.write_text(old_caddyfile)
+            except OSError:
+                pass
         # Best-effort restart on the previous target — if it fails too the
         # box is in a hard state, but we can only surface the reason.
+        subprocess.run(["systemctl", "daemon-reload"], check=False)
+        subprocess.run(["systemctl", "reload", "caddy"], check=False)
         subprocess.run(["systemctl", "restart", "furtka-api.service"], check=False)
     write_state(
         "rolled_back",
@@ -430,6 +458,7 @@ def run_update() -> UpdateCheck:
     UpdateError on any failure; the state file records the stage.
     """
     with acquire_lock():
+        try:
             check = check_update()
             if not check.update_available:
                 write_state("done", version=check.current, note="already up to date")
@@ -437,6 +466,17 @@ def run_update() -> UpdateCheck:
             tarball, version = prepare_update(check)
             apply_update(tarball, version)
             return check
+        except Exception as e:
+            # Without a terminal stage the settings page keeps showing
+            # "Downloading…" forever. Rollback already wrote its own stage.
+            if read_state().get("stage") != "rolled_back":
+                try:
+                    write_state("error", reason=str(e) or type(e).__name__)
+                except OSError:
+                    pass
+            if isinstance(e, UpdateError):
+                raise
+            raise UpdateError(str(e) or type(e).__name__) from e
 
 
 def rollback() -> str:
@@ -456,8 +496,7 @@ def rollback() -> str:
         raise UpdateError("no other version slots available to roll back to")
     target_name = candidates[0]
     target = versions_dir() / target_name
-    current.unlink()
+    _swap_symlink(current, target)
-    current.symlink_to(target)
     subprocess.run(["systemctl", "daemon-reload"], check=False)
     subprocess.run(["systemctl", "restart", "furtka-api.service"], check=False)
     write_state("rolled_back_manual", restored_to=target_name)

tests/test_api.py

@@ -1502,3 +1502,25 @@ def test_http_unexpected_exception_returns_json_500(fake_dirs, admin_session, mo
     finally:
         server.shutdown()
         server.server_close()
+
+
+def test_login_with_corrupt_users_file_is_503_not_setup(fake_dirs, tmp_path):
+    (tmp_path / "users.json").write_text("")
+    server, port = _start_server()
+    try:
+        with pytest.raises(urllib.error.HTTPError) as e:
+            urllib.request.urlopen(_request(port, "/login"))
+        assert e.value.code == 503
+        req = _request(
+            port,
+            "/login",
+            method="POST",
+            body={"username": "mallory", "password": "longenough1", "password2": "longenough1"},
+        )
+        with pytest.raises(urllib.error.HTTPError) as e:
+            urllib.request.urlopen(req)
+        assert e.value.code == 503
+        assert (tmp_path / "users.json").read_text() == ""
+    finally:
+        server.shutdown()
+        server.server_close()

tests/test_auth.py

@@ -40,14 +40,15 @@ def test_load_users_returns_empty_when_missing(tmp_users_file):
     assert auth.load_users() == {}
 
 
-def test_load_users_returns_empty_on_junk(tmp_users_file):
+@pytest.mark.parametrize("content", ["{not json", "[]", "", "\x00\x00"])
-    tmp_users_file.write_text("{not json")
+def test_load_users_fails_closed_on_corrupt_file(tmp_users_file, content):
-    assert auth.load_users() == {}
+    """A damaged users.json must not read as "no admin yet" — that would
-
+    turn /login into an open create-admin form for anyone on the LAN."""
-
+    tmp_users_file.write_text(content)
-def test_load_users_returns_empty_on_non_dict(tmp_users_file):
+    with pytest.raises(auth.UsersFileError):
-    tmp_users_file.write_text("[]")
+        auth.load_users()
-    assert auth.load_users() == {}
+    with pytest.raises(auth.UsersFileError):
+        auth.setup_needed()
 
 
 def test_save_users_atomic_and_0600(tmp_users_file):

tests/test_updater.py

@@ -463,3 +463,79 @@ def test_check_update_raises_when_no_releases_published(updater, monkeypatch):
     monkeypatch.setattr(updater, "_forgejo_api", lambda path: [])
     with pytest.raises(updater.UpdateError, match="no releases"):
         updater.check_update()
+
+
+def test_apply_update_rolls_back_on_oserror_and_restores_caddyfile(tmp_path, updater, monkeypatch):
+    """A non-UpdateError after the symlink flip (disk full writing the
+    Caddyfile) used to leave the new version live with no rollback."""
+    versions = updater.versions_dir()
+    versions.mkdir(parents=True)
+    (versions / "26.0-alpha").mkdir()
+    current = updater.current_symlink()
+    current.symlink_to(versions / "26.0-alpha")
+    caddyfile = tmp_path / "etc_caddy" / "Caddyfile"
+    caddyfile.parent.mkdir()
+    caddyfile.write_text("# old routes\n")
+
+    tar = tmp_path / "t.tar.gz"
+    _make_tarball(tar, "26.1-alpha")
+
+    def full_disk(_source):
+        caddyfile.write_text("# half-written new routes\n")
+        raise OSError(28, "No space left on device")
+
+    monkeypatch.setattr(updater, "_refresh_caddyfile", full_disk)
+    monkeypatch.setattr(updater, "_run", lambda cmd: None)
+    import subprocess
+
+    monkeypatch.setattr(subprocess, "run", lambda *a, **kw: None)
+
+    with pytest.raises(updater.UpdateError, match="rolled back"):
+        updater.apply_update(tar, "26.1-alpha")
+    assert current.resolve() == (versions / "26.0-alpha").resolve()
+    assert caddyfile.read_text() == "# old routes\n"
+    assert updater.read_state()["stage"] == "rolled_back"
+
+
+def test_swap_symlink_replaces_existing_link(tmp_path, updater):
+    a, b = tmp_path / "a", tmp_path / "b"
+    a.mkdir()
+    b.mkdir()
+    link = tmp_path / "current"
+    link.symlink_to(a)
+    updater._swap_symlink(link, b)
+    assert link.resolve() == b.resolve()
+    assert not (tmp_path / "current.new").exists()
+
+
+def test_run_update_writes_error_stage_on_download_failure(updater, monkeypatch):
+    class _Check:
+        update_available = True
+        latest = "26.1-alpha"
+        current = "26.0-alpha"
+        tarball_url = "http://x/t.tar.gz"
+        sha256_url = "http://x/t.sha256"
+
+    def fake_check():
+        return _Check()
+
+    monkeypatch.setattr(updater, "check_update", fake_check)
+
+    def boom(url, dest):
+        raise OSError(28, "No space left on device")
+
+    monkeypatch.setattr(updater, "_download", boom)
+    with pytest.raises(updater.UpdateError, match="No space left"):
+        updater.run_update()
+    state = updater.read_state()
+    assert state["stage"] == "error"
+    assert "No space left" in state["reason"]
+
+
+def test_health_check_survives_read_timeout(updater, monkeypatch):
+    def slow(url, timeout):
+        raise TimeoutError("timed out")
+
+    monkeypatch.setattr("urllib.request.urlopen", slow)
+    monkeypatch.setattr(updater.time, "sleep", lambda _s: None)
+    assert updater._health_check("http://127.0.0.1:7000/api/apps", deadline_s=0.05) is False