refactor(server): extract the settings routes into routers/settings.py (R3) (#863)

GET/POST /api/settings, /api/settings/reset, and the two-phase atomic
export/import bundle (/api/settings/export|import) move to lib/routers/settings.py
with their exclusive helpers (the relpath allowlist validator, the atomic writer,
the library-DB snapshot + sqlite integrity gate, the config-type validator, the
bundle schema). Bodies verbatim except @app->@router and the seam reads:
meta_db->appstate.meta_db, CONFIG_DIR->appstate.config_dir,
_running_version->appstate.running_version(), and _default_settings->
appstate.default_settings (the canonical defaults builder stays in server.py —
the scan + artist-links code share it — and is injected as a new seam callable).

server.py: 5,539 -> 4,478 (-1,061).

Verified: pyflakes clean (bar the pre-existing File/safe_join/tuning_name/ET);
route table IDENTICAL (143); full pytest 2397 passed (154 settings cases incl the
export→import round-trip + library-DB snapshot/restore + relpath-allowlist SSRF/
traversal guards, retargeted onto the settings module). eslint 0.

BEHAVIORAL — needs an on-device settings export→import round-trip sign-off before
merge (do not merge on green CI alone).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Byron Gamatos
2026-07-11 12:35:05 +02:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 73127d5416
commit 7258e1066a
9 changed files with 1178 additions and 1092 deletions
+2 -1
View File
@@ -14,6 +14,7 @@ back-compat for `.sloppak` libraries or stop accepting the new `.feedpak`:
from __future__ import annotations
import importlib
from routers import settings as settings_router
import io
import sys
import zipfile
@@ -242,7 +243,7 @@ def test_settings_dlc_count_includes_both_suffixes(tmp_path, settings_server):
(dlc / "c.FEEDPAK").write_bytes(b"") # case-insensitive (suffix.lower())
(dlc / "notes.txt").write_bytes(b"") # ignored
result = settings_server.save_settings({"dlc_dir": str(dlc)})
result = settings_router.save_settings({"dlc_dir": str(dlc)})
assert "error" not in result, result
# save_settings joins its notices into a single ``message`` string.
+5 -4
View File
@@ -10,6 +10,7 @@ shadow the config.json dlc_dir fallback.
"""
import importlib
from routers import settings as settings_router
import json
import sys
@@ -32,13 +33,13 @@ class _DirectSettingsClient:
def get(self, path):
if path != "/api/settings":
raise ValueError(f"unsupported path: {path}")
return _DirectResponse(self._server.get_settings())
return _DirectResponse(settings_router.get_settings())
def post(self, path, json):
if path == "/api/settings":
return _DirectResponse(self._server.save_settings(json))
return _DirectResponse(settings_router.save_settings(json))
if path == "/api/settings/reset":
return _DirectResponse(self._server.reset_settings(json))
return _DirectResponse(settings_router.reset_settings(json))
raise ValueError(f"unsupported path: {path}")
def close(self):
@@ -643,7 +644,7 @@ def test_achievements_enabled_persists_and_validates(api_client, tmp_path):
def test_achievements_enabled_is_resettable(server_module):
"""The flag is in the resettable allow-list so a Reset clears it to default."""
assert "achievements_enabled" in server_module._RESETTABLE_SETTINGS_KEYS
assert "achievements_enabled" in settings_router._RESETTABLE_SETTINGS_KEYS
def test_skip_startup_tasks_drives_startup_to_complete(api_client):
+5 -4
View File
@@ -11,6 +11,7 @@ is exercised separately in `test_plugins.py`.
import base64
import importlib
from routers import settings as settings_router
import json
import os
import sys
@@ -478,7 +479,7 @@ def test_normalize_export_paths_consistency(server_mod, tmp_path):
# Wraps `_validate_relpath` to assert it doesn't raise the
# hard-failure ValueErrors. _UndeclaredFile would mean the
# allowlist is wrong, not that the relpath shape is bad.
server_mod._validate_relpath(rel, cleaned, tmp_path)
settings_router._validate_relpath(rel, cleaned, tmp_path)
# ── Atomic write: unique tmp + cleanup on failure ───────────────────────────
@@ -502,7 +503,7 @@ def test_atomic_write_cleans_up_tmp_on_failure(server_mod, tmp_path, monkeypatch
for _ in range(2):
with pytest.raises(OSError):
server_mod._atomic_write_file(target, b"payload")
settings_router._atomic_write_file(target, b"payload")
# Both attempts cleaned up. No .tmp.import residue means the
# mkstemp + finally-unlink pattern held even across failures.
@@ -512,7 +513,7 @@ def test_atomic_write_cleans_up_tmp_on_failure(server_mod, tmp_path, monkeypatch
# Restoring real replace, the function should still work end-to-end.
monkeypatch.setattr(server_mod.os, "replace", real_replace)
server_mod._atomic_write_file(target, b"payload")
settings_router._atomic_write_file(target, b"payload")
assert target.read_bytes() == b"payload"
assert list(tmp_path.glob("*.tmp.import")) == []
@@ -764,7 +765,7 @@ def test_atomic_write_closes_fd_when_fdopen_fails(server_mod, tmp_path, monkeypa
monkeypatch.setattr(server_mod.os, "fdopen", boom_fdopen)
with pytest.raises(OSError, match="simulated EMFILE"):
server_mod._atomic_write_file(target, b"payload")
settings_router._atomic_write_file(target, b"payload")
# fd was closed (so it didn't leak), and the temp file mkstemp
# created was removed (so it doesn't litter / lock on Windows).
+11 -10
View File
@@ -15,6 +15,7 @@ this file pins the additive `core_server_files` section:
import base64
import importlib
from routers import settings as settings_router
import sqlite3
import sys
from pathlib import Path
@@ -118,7 +119,7 @@ def test_import_stages_db_restore_without_touching_live_db(client, server_mod, t
payload = _valid_db_bytes(tmp_path, name="incoming.db", marker="restored")
r = client.post("/api/settings/import", json={
"schema": server_mod.SETTINGS_BUNDLE_SCHEMA,
"schema": settings_router.SETTINGS_BUNDLE_SCHEMA,
"server_config": {},
"core_server_files": {
"web_library.db": {"encoding": "base64",
@@ -143,7 +144,7 @@ def test_import_rejects_corrupt_db_with_valid_magic_header(client, server_mod, t
# fail to open the bad restore.
corrupt = b"SQLite format 3\x00" + b"\xff" * 200
r = client.post("/api/settings/import", json={
"schema": server_mod.SETTINGS_BUNDLE_SCHEMA,
"schema": settings_router.SETTINGS_BUNDLE_SCHEMA,
"server_config": {},
"core_server_files": {
"web_library.db": {"encoding": "base64",
@@ -158,7 +159,7 @@ def test_import_rejects_non_sqlite_db_payload(client, server_mod, tmp_path):
# A truncated / wrong file staged as the restore would brick startup —
# reject anything lacking the SQLite magic header, before touching disk.
r = client.post("/api/settings/import", json={
"schema": server_mod.SETTINGS_BUNDLE_SCHEMA,
"schema": settings_router.SETTINGS_BUNDLE_SCHEMA,
"server_config": {},
"core_server_files": {
"web_library.db": {"encoding": "base64",
@@ -171,7 +172,7 @@ def test_import_rejects_non_sqlite_db_payload(client, server_mod, tmp_path):
def test_import_writes_custom_art_immediately(client, server_mod, tmp_path):
r = client.post("/api/settings/import", json={
"schema": server_mod.SETTINGS_BUNDLE_SCHEMA,
"schema": settings_router.SETTINGS_BUNDLE_SCHEMA,
"server_config": {},
"core_server_files": {
"playlist_covers/7.png": {"encoding": "base64",
@@ -186,7 +187,7 @@ def test_import_writes_custom_art_immediately(client, server_mod, tmp_path):
def test_import_core_path_traversal_rejected(client, server_mod, tmp_path):
secret = tmp_path.parent / "escape.txt"
r = client.post("/api/settings/import", json={
"schema": server_mod.SETTINGS_BUNDLE_SCHEMA,
"schema": settings_router.SETTINGS_BUNDLE_SCHEMA,
"server_config": {},
"core_server_files": {
"../escape.txt": {"encoding": "base64",
@@ -201,7 +202,7 @@ def test_import_core_undeclared_path_skipped_not_fatal(client, server_mod, tmp_p
# A relpath outside the core allowlist is a warn-and-skip, not a refusal —
# the rest of the bundle still applies.
r = client.post("/api/settings/import", json={
"schema": server_mod.SETTINGS_BUNDLE_SCHEMA,
"schema": settings_router.SETTINGS_BUNDLE_SCHEMA,
"server_config": {},
"core_server_files": {
"audio_cache/x.ogg": {"encoding": "base64",
@@ -289,7 +290,7 @@ def test_full_db_backup_restore_round_trip(client, server_mod, tmp_path):
def test_export_fails_hard_when_db_snapshot_unavailable(client, server_mod, monkeypatch):
# A backup that silently omits the library DB is a data-loss trap — the
# export must error rather than hand back an incomplete-looking bundle.
monkeypatch.setattr(server_mod, "_snapshot_library_db", lambda: None)
monkeypatch.setattr(settings_router, "_snapshot_library_db", lambda: None)
r = client.get("/api/settings/export")
assert r.status_code == 500
assert "library database" in r.json()["error"].lower()
@@ -299,16 +300,16 @@ def test_failed_import_disarms_staged_db_restore(client, server_mod, tmp_path, m
# If a later write in phase 2 fails, the request 500s — but a staged DB
# restore must NOT survive to swap in on the next restart.
payload = _valid_db_bytes(tmp_path, name="incoming.db")
real_write = server_mod._atomic_write_file
real_write = settings_router._atomic_write_file
def boom(target, data):
if target.name == "config.json": # last write of the commit
raise OSError("disk full")
return real_write(target, data)
monkeypatch.setattr(server_mod, "_atomic_write_file", boom)
monkeypatch.setattr(settings_router, "_atomic_write_file", boom)
r = client.post("/api/settings/import", json={
"schema": server_mod.SETTINGS_BUNDLE_SCHEMA,
"schema": settings_router.SETTINGS_BUNDLE_SCHEMA,
"server_config": {},
"core_server_files": {
"web_library.db": {"encoding": "base64",