From 20f6398abf7d195066721411ec3a04890c21dcb7 Mon Sep 17 00:00:00 2001
From: Daniel Maksymilian Syrnicki <info@danielmaksymilian.com>
Date: Mon, 28 Sep 2026 13:04:09 +0200
Subject: [PATCH] fix(api): refuse path-like app names, validate settings
 before writing .env, harden the HTTP server
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- 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
---
 assets/www/index.html          |  9 ++++
 assets/www/settings/index.html |  9 ++++
 furtka/api.py                  | 86 +++++++++++++++++++++++++++++---
 furtka/installer.py            | 82 ++++++++++++++++++++-----------
 furtka/manifest.py             |  6 ++-
 tests/test_api.py              | 90 +++++++++++++++++++++++++++++++---
 tests/test_installer.py        | 55 +++++++++++++++++++++
 7 files changed, 293 insertions(+), 44 deletions(-)

diff --git a/assets/www/index.html b/assets/www/index.html
index 66e5e2c..86c1559 100644
--- a/assets/www/index.html
+++ b/assets/www/index.html
@@ -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.
diff --git a/assets/www/settings/index.html b/assets/www/settings/index.html
index 46fe2ac..3966a2c 100644
--- a/assets/www/settings/index.html
+++ b/assets/www/settings/index.html
@@ -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();
diff --git a/furtka/api.py b/furtka/api.py
index d052169..1c2d20a 100644
--- a/furtka/api.py
+++ b/furtka/api.py
@@ -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()
diff --git a/furtka/installer.py b/furtka/installer.py
index f5ec1ae..e5bc6fa 100644
--- a/furtka/installer.py
+++ b/furtka/installer.py
@@ -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)
diff --git a/furtka/manifest.py b/furtka/manifest.py
index 03a4c73..432063a 100644
--- a/furtka/manifest.py
+++ b/furtka/manifest.py
@@ -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}")
 
diff --git a/tests/test_api.py b/tests/test_api.py
index 244f0a2..39f3bb4 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -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()
diff --git a/tests/test_installer.py b/tests/test_installer.py
index f784957..bc9aa11 100644
--- a/tests/test_installer.py
+++ b/tests/test_installer.py
@@ -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)