From f7942f3689726254f44f26dd48fc5172fbdbf807 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Sun, 19 Jul 2026 01:36:28 -0500 Subject: [PATCH] fix(gp8): confine registry asset matching to the declared directory (#1011) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `` is matched on filename stem so a format variant of the same recording can win — an `.ogg` beside the declared `.mp3` is copied out losslessly rather than transcoded. But the search spanned every directory in the archive, so an unrelated file that merely shared the stem could stand in for the declared asset: exactly the substitution the registry lookup added in #1007 exists to prevent. Candidates are now confined to the registry path's own directory. A genuinely absent asset still falls through to the legacy stem match and then the first audio asset, as documented. Found by an adversarial pass over #1007 rather than a report — no known file triggers it, since GP8 writes embedded audio to Content/Assets/ and that is the only directory scanned. It needs a hand-edited archive to reach. Both tests fail on main and pass here; their ZIP ordering is deliberate, so the fall-through target differs from the decoy (otherwise fixed and unfixed code return the same file and the tests prove nothing). Claude-Session: https://claude.ai/code/session_01SFDokqh2H6mEjk1Kgbi6JW Signed-off-by: ChrisBeWithYou Co-authored-by: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 9 +++++++ lib/gp8_audio_sync.py | 12 ++++++++-- tests/test_gp_audio_sync.py | 48 +++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 637c7cd..9f8ccfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -235,6 +235,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry). ### Fixed +- **GP8 asset resolution honours the directory the registry named.** + `` is matched on filename stem so a format variant of the + same recording can win (an `.ogg` beside the declared `.mp3` is copied out + losslessly instead of transcoded) — but the search was not restricted to the + declared directory, so an unrelated file elsewhere in the archive that merely + shared the stem could stand in for the declared asset. That is the exact + substitution the registry lookup exists to prevent. Candidates are now + confined to the registry path's own directory; a genuinely absent asset + falls through as documented. - **Guitar Pro 8: the right backing track is extracted when a file carries more than one.** `BackingTrack/AssetId` is a key into the GPIF's `` registry — `` names the exact path inside the archive — but it diff --git a/lib/gp8_audio_sync.py b/lib/gp8_audio_sync.py index 5a9aeb1..d4d14e6 100644 --- a/lib/gp8_audio_sync.py +++ b/lib/gp8_audio_sync.py @@ -165,12 +165,20 @@ def _resolve_audio_asset(zf, root=None) -> tuple[str, str | None]: # stale/edited entry must fall through, not resolve to nothing. registry_path = _asset_path_from_registry(root, declared) if registry_path: + # Matched on STEM, not the whole path, so a format variant of the + # same recording can win (see _prefer_ogg) — but constrained to the + # directory the registry actually named. Without that constraint an + # unrelated file that merely shares the stem could stand in for the + # declared asset, which is the failure the registry lookup exists + # to prevent. + declared_path = Path(registry_path) same_stem = [ n for n in audio_files - if Path(n).stem == Path(registry_path).stem + if Path(n).stem == declared_path.stem + and Path(n).parent == declared_path.parent ] if same_stem: - return Path(registry_path).stem, _prefer_ogg(same_stem) + return declared_path.stem, _prefer_ogg(same_stem) _log.warning( 'gp8_audio_sync: AssetId %r maps to %r, which is not an audio ' 'asset in the archive; falling back', diff --git a/tests/test_gp_audio_sync.py b/tests/test_gp_audio_sync.py index 20c1c01..a81fe11 100644 --- a/tests/test_gp_audio_sync.py +++ b/tests/test_gp_audio_sync.py @@ -375,3 +375,51 @@ def test_legacy_stem_match_still_works_without_a_registry(): stem, path = _resolve_audio_asset(zf) assert stem == "abc-123" assert path == "Content/Assets/abc-123.ogg" + + +def test_a_same_stem_file_in_another_directory_cannot_stand_in(): + """The registry names a PATH, not just a name. + + Resolution matches on stem so a format variant of the same recording can + win, but an unrelated file that merely shares the stem must not satisfy + the declaration — that substitution is what the registry lookup exists to + prevent. The declared asset is genuinely absent here, so the right answer + is the documented fall-through, not the decoy. + + ZIP order matters to this test: `real.ogg` is written FIRST so the + fall-through target differs from the decoy. Otherwise both the fixed and + unfixed code return the same file and the test proves nothing. + """ + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("Content/score.gpif", _gpif_bytes( + "0", {"0": "Content/Audio/track.ogg"})) + zf.writestr("Content/Assets/real.ogg", b"fake") # fall-through target + zf.writestr("Content/Assets/track.ogg", b"decoy") # shares the stem only + buf.seek(0) + _, path = _resolve_audio_asset(zipfile.ZipFile(buf)) + assert path == "Content/Assets/real.ogg", ( + "a same-stem file in a directory the registry never named must not " + "satisfy the declaration" + ) + + +def test_the_declared_directory_still_resolves_its_own_format_variants(): + """The directory constraint must not cost us the OGG preference. + + The shallower decoy is written FIRST, so unfixed code (which searches + every directory) picks it and this test fails. + """ + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("Content/score.gpif", _gpif_bytes( + "0", {"0": "Content/Assets/nested/take.mp3"})) + zf.writestr("Content/Assets/take.ogg", b"decoy-one-level-up") + zf.writestr("Content/Assets/nested/take.mp3", b"declared") + zf.writestr("Content/Assets/nested/take.ogg", b"same-take-lossless") + buf.seek(0) + stem, path = _resolve_audio_asset(zipfile.ZipFile(buf)) + assert path == "Content/Assets/nested/take.ogg", ( + "the OGG variant in the DECLARED directory wins over a shallower decoy" + ) + assert stem == "take"