fix(api): refuse path-like app names, validate settings before writing .env, harden the HTTP server

- app names from URL paths and JSON bodies must match APP_NAME_RE; installer
  remove()/update_env() check it too. Before, {"name": ".."} or "/" sent
  remove() into shutil.rmtree on /var/lib/furtka or the root filesystem
- manifests must carry a valid app name as well
- settings are validated in memory (placeholders, system paths, line breaks)
  before .env is written — a rejected /etc used to be saved anyway
- values containing $ are single-quoted so compose doesn't expand them
  ("pa$word" became "pa"); line breaks are refused (env injection)
- ThreadingHTTPServer so status polls keep answering during slow requests;
  state-changing POSTs still run one at a time
- 1 MiB body cap, 30 s socket timeout, bad Content-Length → 400
- POSTs with Sec-Fetch-Site other than same-origin are refused (apps on
  other ports of the same host count as same-site for SameSite=Strict)
- unexpected exceptions return a JSON 500 instead of a dropped connection
- UI pages send the browser to /login on a 401 instead of showing an empty
  app list after an API restart

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wq1hzUp4ZmCKHDuLKCnu73
This commit is contained in:
Daniel Maksymilian Syrnicki 2026-09-28 13:04:09 +02:00
parent 8bcbaec621
commit 20f6398abf
7 changed files with 293 additions and 44 deletions

View file

@ -68,6 +68,15 @@
</main>
<script>
// Sessions live in API memory, so every restart or self-update logs
// everyone out. Send the browser to the login page instead of rendering
// the 401 body as if the box had no apps.
const _fetch = window.fetch.bind(window);
window.fetch = async (...args) => {
const r = await _fetch(...args);
if (r.status === 401) { location.href = '/login'; }
return r;
};
// Revoke the cookie server-side and bounce to /login. Shared
// shape with the _HTML in furtka/api.py so the two logout
// links behave identically.

View file

@ -122,6 +122,15 @@
</main>
<script>
// Sessions live in API memory, so every restart or self-update logs
// everyone out. Send the browser to the login page instead of rendering
// the 401 body as if the box had no apps.
const _fetch = window.fetch.bind(window);
window.fetch = async (...args) => {
const r = await _fetch(...args);
if (r.status === 401) { location.href = '/login'; }
return r;
};
// Logout button in the nav — same shape as /apps and / pages.
async function doLogout(ev) {
ev.preventDefault();

View file

@ -17,12 +17,14 @@ stopgap.
import json
import re
import threading
import time
import traceback
from http.cookies import SimpleCookie
from http.server import BaseHTTPRequestHandler, HTTPServer
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
from furtka import auth, deps, dockerops, install_runner, installer, reconciler, sources
from furtka.manifest import ManifestError, load_manifest
from furtka.manifest import APP_NAME_RE, ManifestError, load_manifest
from furtka.paths import apps_dir, static_www_dir
from furtka.scanner import scan
@ -120,6 +122,15 @@ _HTML = """<!DOCTYPE html>
</div>
<script>
// Sessions live in API memory, so every restart or self-update logs
// everyone out. Send the browser to the login page instead of rendering
// the 401 body as if the box had no apps.
const _fetch = window.fetch.bind(window);
window.fetch = async (...args) => {
const r = await _fetch(...args);
if (r.status === 401) { location.href = '/login'; }
return r;
};
function esc(s) {
const d = document.createElement('div');
d.textContent = s == null ? '' : String(s);
@ -1238,7 +1249,25 @@ def _parse_settings_body(payload):
return out
# Largest request body we read. Every legitimate body is a small JSON
# object; without a cap a LAN client could make us buffer gigabytes.
MAX_BODY_BYTES = 1024 * 1024
# The server is threaded so status polls and page loads keep answering while
# 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()
def _valid_app_name(name) -> bool:
return isinstance(name, str) and APP_NAME_RE.fullmatch(name) is not None
class _Handler(BaseHTTPRequestHandler):
# Socket read timeout: a client that stops sending mid-request gets
# dropped instead of holding a worker forever.
timeout = 30
def _json(self, status, payload, extra_headers=None):
body = json.dumps(payload).encode()
self.send_response(status)
@ -1393,7 +1422,28 @@ class _Handler(BaseHTTPRequestHandler):
auth.SESSIONS.revoke(morsel.value)
return self._json(200, {"ok": True}, extra_headers=[self._clear_cookie_header()])
def _internal_error(self):
# Without this, an unexpected exception makes socketserver drop the
# connection and the UI shows "Failed to fetch" with nothing to go on.
traceback.print_exc()
try:
self._json(500, {"error": "internal error — see `journalctl -u furtka-api`"})
except OSError:
pass
def do_GET(self): # noqa: N802 — http.server convention
try:
self._route_get()
except Exception:
self._internal_error()
def do_POST(self): # noqa: N802
try:
self._route_post()
except Exception:
self._internal_error()
def _route_get(self):
# --- Public routes: login page + its assets ------------------
if self.path in ("/login", "/login/"):
# Already authed? Skip straight to the app list.
@ -1447,14 +1497,26 @@ class _Handler(BaseHTTPRequestHandler):
# /api/apps/<name>/settings
if self.path.startswith("/api/apps/") and self.path.endswith("/settings"):
name = self.path[len("/api/apps/") : -len("/settings")]
if "/" in name or not name:
if not _valid_app_name(name):
return self._json(400, {"error": "invalid app name"})
status, body = _do_get_settings(name)
return self._json(status, body)
self._json(404, {"error": "not found"})
def do_POST(self): # noqa: N802
length = int(self.headers.get("Content-Length", "0"))
def _route_post(self):
# Browsers tag every request with where it came from. Only accept
# state changes from our own pages — an app on another port of the
# same host counts as "same-site" and would pass SameSite=Strict.
# Old browsers that don't send the header fall back to the cookie.
fetch_site = self.headers.get("Sec-Fetch-Site")
if fetch_site is not None and fetch_site not in ("same-origin", "none"):
return self._json(403, {"error": "cross-site request refused"})
try:
length = int(self.headers.get("Content-Length", "0"))
except ValueError:
return self._json(400, {"error": "invalid Content-Length"})
if length < 0 or length > MAX_BODY_BYTES:
return self._json(413, {"error": "request body too large"})
raw = self.rfile.read(length) if length else b""
try:
payload = json.loads(raw.decode()) if raw else {}
@ -1473,10 +1535,15 @@ class _Handler(BaseHTTPRequestHandler):
if self._current_session() is None:
return self._json(401, {"error": "not authenticated"})
with _MUTATION_LOCK:
return self._route_authed_post(payload)
def _route_authed_post(self, payload):
# Per-app settings update: /api/apps/<name>/settings
if self.path.startswith("/api/apps/") and self.path.endswith("/settings"):
name = self.path[len("/api/apps/") : -len("/settings")]
if "/" in name or not name:
if not _valid_app_name(name):
return self._json(400, {"error": "invalid app name"})
settings = _parse_settings_body(payload)
if settings is False or settings is None:
@ -1487,7 +1554,7 @@ class _Handler(BaseHTTPRequestHandler):
# Per-app image update: /api/apps/<name>/update
if self.path.startswith("/api/apps/") and self.path.endswith("/update"):
name = self.path[len("/api/apps/") : -len("/update")]
if "/" in name or not name:
if not _valid_app_name(name):
return self._json(400, {"error": "invalid app name"})
status, body = _do_update(name)
return self._json(status, body)
@ -1519,6 +1586,8 @@ class _Handler(BaseHTTPRequestHandler):
name = payload.get("name")
if not isinstance(name, str) or not name:
return self._json(400, {"error": "missing or empty 'name' field"})
if not _valid_app_name(name):
return self._json(400, {"error": "invalid app name"})
if self.path == "/api/apps/install/plan":
status, body = _do_install_plan(name)
@ -1541,7 +1610,8 @@ class _Handler(BaseHTTPRequestHandler):
def serve(host: str = "127.0.0.1", port: int = 7000) -> None:
"""Run the API server. Blocks forever; exits on SIGINT/SIGTERM."""
server = HTTPServer((host, port), _Handler)
server = ThreadingHTTPServer((host, port), _Handler)
server.daemon_threads = True
print(f"Furtka API listening on http://{host}:{port}/")
try:
server.serve_forever()

View file

@ -2,7 +2,7 @@ import shutil
from pathlib import Path
from furtka import sources
from furtka.manifest import Manifest, ManifestError, load_manifest
from furtka.manifest import APP_NAME_RE, Manifest, ManifestError, load_manifest
from furtka.paths import apps_dir
# Values that an app's .env.example may use as obvious "fill me in" markers.
@ -35,19 +35,26 @@ class InstallError(RuntimeError):
pass
def app_dir(name: str) -> Path:
"""/var/lib/furtka/apps/<name>, refusing anything that isn't a plain app name.
Names reach us from API bodies and URL paths; without this check "..",
"." or "/" would point remove()'s rmtree at /var/lib/furtka, every app,
or the root filesystem.
"""
if not isinstance(name, str) or not APP_NAME_RE.fullmatch(name):
raise InstallError(f"{name!r} is not a valid app name")
return apps_dir() / name
def _placeholder_values(values: dict[str, str]) -> list[str]:
return [k for k, v in values.items() if v.strip() in PLACEHOLDER_SECRETS]
def _placeholder_keys(env_path: Path) -> list[str]:
if not env_path.exists():
return []
bad: list[str] = []
for raw in env_path.read_text().splitlines():
line = raw.strip()
if not line or line.startswith("#") or "=" not in line:
continue
key, _, value = line.partition("=")
value = value.strip().strip('"').strip("'")
if value in PLACEHOLDER_SECRETS:
bad.append(key.strip())
return bad
return _placeholder_values(_read_env(env_path))
def _is_denied_system_path(resolved: str) -> bool:
@ -59,16 +66,13 @@ def _is_denied_system_path(resolved: str) -> bool:
return False
def _path_setting_errors(m: Manifest, env_path: Path) -> list[str]:
def _path_setting_errors(m: Manifest, values: dict[str, str]) -> list[str]:
"""Validate the filesystem paths named by `path`-type settings.
Returns one human-readable message per offending setting. Empty values
on non-required settings are allowed — the required-field check in the
caller already refuses blanks on required fields before write.
"""
if not env_path.exists():
return []
values = _read_env(env_path)
errors: list[str] = []
for s in m.settings:
if s.type != "path":
@ -97,10 +101,35 @@ def _path_setting_errors(m: Manifest, env_path: Path) -> list[str]:
return errors
def _validate_settings(m: Manifest, values: dict[str, str]) -> None:
"""Refuse a merged settings view before it is written to .env.
Runs in memory so a rejected value (placeholder, system path, line
break) never reaches the live .env where the next reconcile would use it.
"""
for k, v in values.items():
if any(c in v for c in "\r\n\0"):
raise InstallError(f"{m.name}: setting {k!r} must not contain line breaks")
bad = _placeholder_values({s.name: values.get(s.name, "") for s in m.settings})
if bad:
raise InstallError(
f"{m.name}: please replace the example value for {', '.join(bad)} with your own"
)
path_errors = _path_setting_errors(m, values)
if path_errors:
raise InstallError(f"{m.name}: {'; '.join(path_errors)}")
def _format_env_value(v: str) -> str:
# Quote values that contain whitespace, quotes, or shell metacharacters so
# docker-compose's env substitution reads them back intact. Simple values
# stay unquoted to keep the file readable when a user SSHes in.
if any(c in v for c in "\r\n\0"):
raise InstallError("setting values must not contain line breaks")
if "$" in v and "'" not in v:
# Compose expands ${VAR} inside double quotes, so "pa$word" would
# silently turn into "pa". Single-quoted values are taken literally.
return f"'{v}'"
if v == "" or any(c in v for c in " \t\"'$`\\#"):
escaped = v.replace("\\", "\\\\").replace('"', '\\"')
return f'"{escaped}"'
@ -167,7 +196,7 @@ def install_from(src: Path, settings: dict[str, str] | None = None) -> Path:
if key not in declared:
raise InstallError(f"{m.name}: unknown setting {key!r}")
target = apps_dir() / m.name
target = app_dir(m.name)
target.mkdir(parents=True, exist_ok=True)
for item in src.iterdir():
@ -216,6 +245,7 @@ def install_from(src: Path, settings: dict[str, str] | None = None) -> Path:
for s in m.settings:
if s.required and not merged.get(s.name):
raise InstallError(f"{m.name}: setting {s.name!r} is required")
_validate_settings(m, merged)
write_env(env, merged)
elif not env.exists() and env_example.exists():
# First install with no settings and no .env shipped: bootstrap from
@ -235,7 +265,7 @@ def install_from(src: Path, settings: dict[str, str] | None = None) -> Path:
f"file and re-run `furtka app install {m.name}`."
)
path_errors = _path_setting_errors(m, env)
path_errors = _path_setting_errors(m, read_env_values(env))
if path_errors:
raise InstallError(f"{m.name}: {'; '.join(path_errors)}")
@ -256,10 +286,11 @@ def parse_env_text(text: str) -> dict[str, str]:
continue
key, _, value = line.partition("=")
value = value.strip()
if (value.startswith('"') and value.endswith('"')) or (
value.startswith("'") and value.endswith("'")
):
if len(value) >= 2 and value.startswith('"') and value.endswith('"'):
value = value[1:-1].replace('\\"', '"').replace("\\\\", "\\")
elif len(value) >= 2 and value.startswith("'") and value.endswith("'"):
# Single quotes are literal in compose's dotenv — no escapes.
value = value[1:-1]
out[key.strip()] = value
return out
@ -284,7 +315,7 @@ def update_env(name: str, settings: dict[str, str]) -> Path:
apps only. Returns the target folder; caller is expected to run
reconcile to restart the containers.
"""
target = apps_dir() / name
target = app_dir(name)
manifest_path = target / "manifest.json"
if not manifest_path.exists():
raise InstallError(f"{name!r} is not installed")
@ -316,13 +347,8 @@ def update_env(name: str, settings: dict[str, str]) -> Path:
for s in m.settings:
if s.required and not merged.get(s.name):
raise InstallError(f"{m.name}: setting {s.name!r} is required")
_validate_settings(m, merged)
write_env(env, merged)
bad = _placeholder_keys(env)
if bad:
raise InstallError(f"{m.name}: {env} still has placeholder values for {', '.join(bad)}.")
path_errors = _path_setting_errors(m, env)
if path_errors:
raise InstallError(f"{m.name}: {'; '.join(path_errors)}")
return target
@ -353,7 +379,7 @@ def remove(name: str) -> Path:
Caller is responsible for stopping the compose project first.
"""
target = apps_dir() / name
target = app_dir(name)
if not target.exists():
raise InstallError(f"{name!r} is not installed")
shutil.rmtree(target)

View file

@ -175,8 +175,10 @@ def load_manifest(path: Path, expected_name: str | None = None) -> Manifest:
raise ManifestError(f"{path}: missing required fields: {', '.join(missing)}")
name = raw["name"]
if not isinstance(name, str) or not name:
raise ManifestError(f"{path}: name must be a non-empty string")
if not isinstance(name, str) or not APP_NAME_RE.fullmatch(name):
raise ManifestError(
f"{path}: name must be lowercase letters, digits, '-' or '_' (got {name!r})"
)
if expected_name is not None and name != expected_name:
raise ManifestError(f"{path}: name {name!r} must equal {expected_name!r}")

View file

@ -407,7 +407,7 @@ def _request(port, path, cookie=None, method="GET", body=None):
def test_http_get_apps_route(fake_dirs, no_docker, admin_session):
"""Smoke test the actual HTTP server with a real socket, urllib client."""
server = api.HTTPServer(("127.0.0.1", 0), api._Handler) # port 0 → ephemeral
server = api.ThreadingHTTPServer(("127.0.0.1", 0), api._Handler) # port 0 → ephemeral
port = server.server_address[1]
t = threading.Thread(target=server.serve_forever, daemon=True)
t.start()
@ -431,7 +431,7 @@ def test_http_get_apps_route(fake_dirs, no_docker, admin_session):
def test_http_post_install_unknown_app(fake_dirs, admin_session):
server = api.HTTPServer(("127.0.0.1", 0), api._Handler)
server = api.ThreadingHTTPServer(("127.0.0.1", 0), api._Handler)
port = server.server_address[1]
t = threading.Thread(target=server.serve_forever, daemon=True)
t.start()
@ -459,7 +459,7 @@ def test_http_post_install_unknown_app(fake_dirs, admin_session):
def _start_server():
server = api.HTTPServer(("127.0.0.1", 0), api._Handler)
server = api.ThreadingHTTPServer(("127.0.0.1", 0), api._Handler)
port = server.server_address[1]
t = threading.Thread(target=server.serve_forever, daemon=True)
t.start()
@ -980,7 +980,7 @@ def test_update_settings_unknown_app(fake_dirs):
def test_http_get_settings_route(fake_dirs, no_docker, admin_session):
_, bundled = fake_dirs
_write_bundled(bundled, "fileshare", manifest=SETTINGS_MANIFEST)
server = api.HTTPServer(("127.0.0.1", 0), api._Handler)
server = api.ThreadingHTTPServer(("127.0.0.1", 0), api._Handler)
port = server.server_address[1]
t = threading.Thread(target=server.serve_forever, daemon=True)
t.start()
@ -1211,7 +1211,7 @@ def test_http_post_update_route(
api._do_install("fileshare")
update_docker_stubs["up_called"] = 0 # reset counter after install
update_docker_stubs["local"] = {"samba": "sha256:NEW"}
server = api.HTTPServer(("127.0.0.1", 0), api._Handler)
server = api.ThreadingHTTPServer(("127.0.0.1", 0), api._Handler)
port = server.server_address[1]
t = threading.Thread(target=server.serve_forever, daemon=True)
t.start()
@ -1236,7 +1236,7 @@ def test_http_post_update_route(
def test_http_post_install_with_settings(fake_dirs, no_docker, no_systemd_run, admin_session):
_, bundled = fake_dirs
_write_bundled(bundled, "fileshare", manifest=SETTINGS_MANIFEST)
server = api.HTTPServer(("127.0.0.1", 0), api._Handler)
server = api.ThreadingHTTPServer(("127.0.0.1", 0), api._Handler)
port = server.server_address[1]
t = threading.Thread(target=server.serve_forever, daemon=True)
t.start()
@ -1424,3 +1424,81 @@ def test_power_surfaces_systemd_run_missing(fake_dirs, monkeypatch):
status, body = api._do_power({"action": "reboot"})
assert status == 502
assert "systemd-run" in body["error"]
# --- hardening: names, body size, cross-site POSTs, crash handling ----------
@pytest.mark.parametrize("bad", ["/", ".", "..", "../catalog"])
def test_http_remove_rejects_path_names(fake_dirs, no_docker, admin_session, bad):
apps, _ = fake_dirs
(apps / "fileshare").mkdir()
server, port = _start_server()
try:
req = _request(
port, "/api/apps/remove", cookie=admin_session, method="POST", body={"name": bad}
)
with pytest.raises(urllib.error.HTTPError) as e:
urllib.request.urlopen(req)
assert e.value.code == 400
assert (apps / "fileshare").exists()
finally:
server.shutdown()
server.server_close()
def test_http_post_rejects_oversized_body(fake_dirs, admin_session):
server, port = _start_server()
try:
import http.client
# Announce a huge body but never send it: the server must answer
# from the header alone instead of waiting to buffer it.
conn = http.client.HTTPConnection("127.0.0.1", port, timeout=5)
conn.putrequest("POST", "/api/apps/remove")
conn.putheader("Cookie", admin_session)
conn.putheader("Content-Length", str(api.MAX_BODY_BYTES + 1))
conn.endheaders()
assert conn.getresponse().status == 413
conn.close()
finally:
server.shutdown()
server.server_close()
def test_http_post_rejects_same_site_other_origin(fake_dirs, no_docker, admin_session):
apps, _ = fake_dirs
(apps / "fileshare").mkdir()
server, port = _start_server()
try:
req = _request(
port,
"/api/apps/remove",
cookie=admin_session,
method="POST",
body={"name": "fileshare"},
)
req.add_header("Sec-Fetch-Site", "same-site")
with pytest.raises(urllib.error.HTTPError) as e:
urllib.request.urlopen(req)
assert e.value.code == 403
assert (apps / "fileshare").exists()
finally:
server.shutdown()
server.server_close()
def test_http_unexpected_exception_returns_json_500(fake_dirs, admin_session, monkeypatch):
def boom():
raise FileNotFoundError("docker")
monkeypatch.setattr(api, "_list_installed", boom)
server, port = _start_server()
try:
with pytest.raises(urllib.error.HTTPError) as e:
urllib.request.urlopen(_request(port, "/api/apps", cookie=admin_session))
assert e.value.code == 500
assert "internal error" in json.loads(e.value.read())["error"]
finally:
server.shutdown()
server.server_close()

View file

@ -462,3 +462,58 @@ def test_install_plan_skips_already_installed(tmp_path, fake_dirs, monkeypatch):
)
installer.install_plan(plan)
assert calls == ["a"]
# --- hardening: app names, validate-before-write, .env quoting --------------
@pytest.mark.parametrize("bad", ["/", ".", "..", "../catalog", "a/b", "", "UPPER"])
def test_remove_refuses_non_app_names(fake_dirs, bad):
apps, _ = fake_dirs
(apps / "fileshare").mkdir()
with pytest.raises(installer.InstallError, match="not a valid app name"):
installer.remove(bad)
assert (apps / "fileshare").exists()
assert apps.exists()
def test_update_env_refuses_non_app_names(fake_dirs):
with pytest.raises(installer.InstallError, match="not a valid app name"):
installer.update_env("..", {"X": "1"})
def test_rejected_path_setting_never_reaches_env(tmp_path, fake_dirs):
media = tmp_path / "media"
media.mkdir()
src = _write_app_source(tmp_path, "jellyfin", PATH_MANIFEST)
installer.install_from(src, settings={"MEDIA_PATH": str(media)})
env = apps_dir() / "jellyfin" / ".env"
with pytest.raises(installer.InstallError, match="system path"):
installer.update_env("jellyfin", {"MEDIA_PATH": "/etc"})
assert "/etc" not in env.read_text()
assert f"MEDIA_PATH={media}" in env.read_text()
def test_newline_in_setting_is_rejected(tmp_path, fake_dirs):
m = dict(VALID_MANIFEST, settings=[{"name": "PW", "label": "PW", "type": "password"}])
src = _write_app_source(tmp_path, "fileshare", m)
with pytest.raises(installer.InstallError, match="line breaks"):
installer.install_from(src, settings={"PW": "abc\nCOMPOSE_PROJECT_NAME=x"})
env = apps_dir() / "fileshare" / ".env"
assert not env.exists() or "COMPOSE_PROJECT_NAME" not in env.read_text()
def test_dollar_values_are_single_quoted_and_round_trip(tmp_path):
env = tmp_path / ".env"
values = {"PW": "pa$word", "B": "has space", "C": "it's $5", "D": "plain"}
installer.write_env(env, values)
text = env.read_text()
assert "PW='pa$word'" in text
assert "D=plain" in text
assert installer.read_env_values(env) == values
def test_manifest_name_must_be_app_name(tmp_path, fake_dirs):
src = _write_app_source(tmp_path, "evil", dict(VALID_MANIFEST, name="../evil"))
with pytest.raises(installer.InstallError, match="lowercase"):
installer.install_from(src)