From 91f2d6304c057095d6d57a9a4589b1c6140bafa6 Mon Sep 17 00:00:00 2001 From: Robert Syrnicki Date: Mon, 24 Aug 2026 14:57:11 +0200 Subject: [PATCH] Fix wireguard.status()/set_tunnel_enabled() crash on unreadable conf path Found by actually running furtka serve and clicking through the new Settings UI: /api/gateway/status crashed the whole request with a raw PermissionError traceback because conf_path.is_file() doesn't swallow every OSError (confirmed directly, not assumed) when the parent directory isn't accessible. A read-only status check must degrade to "not configured" instead of 500ing; set_tunnel_enabled() now raises a clean WireguardError instead of leaking an unhandled exception. Also gives wireguard.py an env-var-overridable default path (FURTKA_WIREGUARD_CONF / default_conf_path()), matching every other runtime path in the codebase, instead of a single hardcoded constant. --- furtka/wireguard.py | 47 +++++++++++++++++++++++++++++------------ tests/test_wireguard.py | 36 +++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 13 deletions(-) diff --git a/furtka/wireguard.py b/furtka/wireguard.py index 1b8bae3..55208ac 100644 --- a/furtka/wireguard.py +++ b/furtka/wireguard.py @@ -16,13 +16,18 @@ that's unavoidable with stock wg-easy) and hands it to set_tunnel_enabled() here. """ +import os import subprocess from pathlib import Path -CONF_PATH = Path("/etc/wireguard/furtka-gw.conf") +DEFAULT_CONF_PATH = Path("/etc/wireguard/furtka-gw.conf") INTERFACE = "furtka-gw" +def default_conf_path() -> Path: + return Path(os.environ.get("FURTKA_WIREGUARD_CONF", DEFAULT_CONF_PATH)) + + class WireguardError(Exception): """Recoverable failure from set_tunnel_enabled — the caller should 5xx.""" @@ -60,7 +65,7 @@ def _default_reload(action: str) -> None: def set_tunnel_enabled( enabled: bool, wg: dict | None = None, - conf_path: Path = CONF_PATH, + conf_path: Path | None = None, reload=_default_reload, ) -> bool: """Bring the tunnel up (writing conf_path from `wg`) or down. @@ -72,16 +77,21 @@ def set_tunnel_enabled( """ if enabled and wg is None: raise ValueError("wg config is required to enable the tunnel") + if conf_path is None: + conf_path = default_conf_path() - conf_path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) - had_conf = conf_path.is_file() - previous = conf_path.read_text() if had_conf else None + try: + conf_path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + had_conf = conf_path.is_file() + previous = conf_path.read_text() if had_conf else None - if enabled: - conf_path.write_text(_conf_content(wg)) - conf_path.chmod(0o600) - elif had_conf: - conf_path.unlink() + if enabled: + conf_path.write_text(_conf_content(wg)) + conf_path.chmod(0o600) + elif had_conf: + conf_path.unlink() + except OSError as e: + raise WireguardError(f"could not access {conf_path}: {e}") from e try: reload("enable" if enabled else "disable") @@ -115,11 +125,22 @@ def _default_wg_show() -> str: return result.stdout.strip() -def status(conf_path: Path = CONF_PATH, wg_show=_default_wg_show) -> dict: +def status(conf_path: Path | None = None, wg_show=_default_wg_show) -> dict: """Whether the tunnel is configured, and its latest handshake if `wg` tooling is available (None if not — e.g. wg-tools missing, or the - interface isn't actually up yet right after enabling).""" - configured = conf_path.is_file() + interface isn't actually up yet right after enabling). + + Read-only and best-effort: any OSError stat-ing conf_path (missing + parent dir, an unexpected permission situation) is treated the same as + "not configured" rather than raised — a status check must never 500 + the whole request over something this innocuous. + """ + if conf_path is None: + conf_path = default_conf_path() + try: + configured = conf_path.is_file() + except OSError: + configured = False latest_handshake = None if configured: try: diff --git a/tests/test_wireguard.py b/tests/test_wireguard.py index bb35139..486cb01 100644 --- a/tests/test_wireguard.py +++ b/tests/test_wireguard.py @@ -37,6 +37,26 @@ def test_enable_requires_wg_config(tmp_path): wireguard.set_tunnel_enabled(True, conf_path=tmp_path / "furtka-gw.conf") +def test_enable_surfaces_clean_error_on_unreadable_conf_dir(tmp_path): + restricted = tmp_path / "restricted" + restricted.mkdir(mode=0o000) + try: + with pytest.raises(wireguard.WireguardError, match="could not access"): + wireguard.set_tunnel_enabled(True, wg=WG, conf_path=restricted / "furtka-gw.conf") + finally: + restricted.chmod(0o700) + + +def test_default_conf_path_uses_env_override(monkeypatch, tmp_path): + monkeypatch.setenv("FURTKA_WIREGUARD_CONF", str(tmp_path / "custom.conf")) + assert wireguard.default_conf_path() == tmp_path / "custom.conf" + + +def test_default_conf_path_default_when_unset(monkeypatch): + monkeypatch.delenv("FURTKA_WIREGUARD_CONF", raising=False) + assert wireguard.default_conf_path() == wireguard.DEFAULT_CONF_PATH + + def test_enable_writes_conf_and_calls_reload(tmp_path): conf_path = tmp_path / "furtka-gw.conf" calls = [] @@ -115,6 +135,22 @@ def test_status_not_configured(tmp_path): assert s == {"configured": False, "latest_handshake": None} +def test_status_survives_unreadable_conf_path(tmp_path): + """Regression: an inaccessible parent directory raises PermissionError + even for a path that doesn't exist yet — confirmed by actually hitting + this against a real box's /etc/wireguard/ as a non-root user, which + crashed the whole /api/gateway/status request with a raw traceback. + A read-only status check must degrade to "not configured", never 500. + """ + restricted = tmp_path / "restricted" + restricted.mkdir(mode=0o000) + try: + s = wireguard.status(conf_path=restricted / "furtka-gw.conf") + assert s == {"configured": False, "latest_handshake": None} + finally: + restricted.chmod(0o700) # so tmp_path cleanup can remove it + + def test_status_configured_reports_handshake(tmp_path): conf_path = tmp_path / "furtka-gw.conf" conf_path.write_text(wireguard._conf_content(WG))