From 47085991de55e73bbb5f454cd4c3eeb4d4189ee7 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Fri, 3 Jul 2026 08:13:33 +0200 Subject: [PATCH] fix(pack overwrite): reject revert-original on non-package targets (mirror write-path guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The POST /api/song/{fn}/revert-original handler restored a .bak over its target after only checking that a backup existed — unlike the write path (gap-fill/overwrite), which refuses non-sloppak targets via is_sloppak. A non-package path under DLC_DIR with a sibling .bak (or a plain directory carrying manifest.yaml.bak) could therefore be mutated by a feature meant only for song packages. Add the same is_sloppak guard after path resolution and before any restore or DB resync, returning the 404 the art/source endpoints use for a non-package target. Existing behaviors (404 when no backup, .bak preserved after a successful revert, demo-block, path-traversal safety) are intact. Regression test: a non-package file under the DLC dir with a sibling .bak is refused (404) and left byte-for-byte unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- server.py | 10 ++++++++++ tests/test_pack_overwrite.py | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/server.py b/server.py index 63b9bca..e8771d1 100644 --- a/server.py +++ b/server.py @@ -11023,6 +11023,16 @@ def post_song_revert_original(filename: str): except ValueError: pass + # Confine revert to actual song packages, mirroring the write path's guard + # (_gap_fill_proposals / _overwrite_proposals both refuse non-sloppak + # targets). Without this, a non-package path under DLC_DIR that happens to + # have a sibling `.bak` (or a plain directory carrying a manifest.yaml.bak) + # would get its backup copied over it and the DB re-synced — mutating a file + # this feature was never meant to touch. Same 404 the art/source endpoints + # return for a non-sloppak target. + if resolved is None or not resolved.exists() or not sloppak_mod.is_sloppak(resolved): + return JSONResponse({"error": "not found"}, 404) + with _song_io_lock: bak = _song_backup_path(resolved) if bak is None: diff --git a/tests/test_pack_overwrite.py b/tests/test_pack_overwrite.py index 7b8a930..fb135bc 100644 --- a/tests/test_pack_overwrite.py +++ b/tests/test_pack_overwrite.py @@ -317,6 +317,19 @@ def test_revert_without_backup_is_404(server, client): assert client.post("/api/song/a.sloppak/revert-original").status_code == 404 +def test_revert_refuses_non_package_target_with_sibling_bak(server, client): + """A non-package file under DLC_DIR with a sibling `.bak` must NOT be + reverted — revert mirrors the write path's is_sloppak guard, so it never + restores a stray backup over a file the feature was not meant to touch.""" + target = server.DLC_DIR / "notes.txt" + target.write_bytes(b"user notes, not a pack") + (server.DLC_DIR / "notes.txt.bak").write_bytes(b"stray backup") + r = client.post("/api/song/notes.txt/revert-original") + assert r.status_code == 404 + # The target is left byte-for-byte untouched. + assert target.read_bytes() == b"user notes, not a pack" + + def test_preview_reports_backup_after_gap_fill(server, client): """Plain gap-fill (R4a) also leaves the one-time backup — the preview surfaces it so the drawer can offer Revert."""