fix(pack overwrite): reject revert-original on non-package targets (mirror write-path guard)

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) <noreply@anthropic.com>
This commit is contained in:
byrongamatos
2026-07-03 08:13:33 +02:00
co-authored by Claude Opus 4.8
parent 4db3db4622
commit 47085991de
2 changed files with 23 additions and 0 deletions
+10
View File
@@ -11023,6 +11023,16 @@ def post_song_revert_original(filename: str):
except ValueError: except ValueError:
pass 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: with _song_io_lock:
bak = _song_backup_path(resolved) bak = _song_backup_path(resolved)
if bak is None: if bak is None:
+13
View File
@@ -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 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): def test_preview_reports_backup_after_gap_fill(server, client):
"""Plain gap-fill (R4a) also leaves the one-time backup — the preview """Plain gap-fill (R4a) also leaves the one-time backup — the preview
surfaces it so the drawer can offer Revert.""" surfaces it so the drawer can offer Revert."""