From e93357a38537c8da94568f97ca4882982a7ba813 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 7 Jun 2026 19:42:30 -0700 Subject: [PATCH] Lyrics retag: fetch query from a read-only lyrics_meta, never db_data (no tag pollution) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lock-in pass caught a real bug in 1051ef24: the retag lyrics path stuffed the library title/artist into a plan's db_data to feed the lyrics query — but db_data is exactly what write_tags_to_file writes ("only writes fields that have DB values"). So an UNMATCHED track (one with no source match, meant to get art/lyrics only) would have had its title/artist tags overwritten from the library values — an unintended tag write on a track we never verified. Fix: each plan now carries a separate READ-ONLY lyrics_meta ({title, artist, album}) sourced from the library track + album scope, kept entirely out of db_data. apply_track_plans reads lyrics_meta for the query (db_data fallback for older plans); unmatched plans keep db_data={} so no tags are written. _fix_library_retag threads lyrics_meta through the manual-apply path too. Tests: +1 regression pinning that an unmatched lyrics plan calls write_tags_to_file with EMPTY db_data (no title/artist leak) while still fetching lyrics. 70 lyrics/retag/repair tests pass. --- core/repair_jobs/library_retag.py | 53 ++++++++++++++++++++----------- core/repair_worker.py | 2 ++ tests/test_missing_lyrics_job.py | 22 ++++++++++++- 3 files changed, 57 insertions(+), 20 deletions(-) diff --git a/core/repair_jobs/library_retag.py b/core/repair_jobs/library_retag.py index 1aa497e7..9743993f 100644 --- a/core/repair_jobs/library_retag.py +++ b/core/repair_jobs/library_retag.py @@ -126,19 +126,25 @@ def apply_track_plans(track_plans, cover_action=None, cover_url=None, full=False # Lyrics: fetch/refresh the .lrc for this track (independent of tag write # success — a track with no tag changes may still be missing lyrics). + # Query metadata comes from the plan's READ-only lyrics_meta (never + # db_data, so nothing here can leak into a tag write). Falls back to + # db_data for plans that predate lyrics_meta. if _lyrics_client: - try: - dur = db_data.get('duration') or db_data.get('duration_seconds') - if lyrics_client_wrote := _lyrics_client.create_lrc_file( - fp, - db_data.get('title') or db_data.get('track_title') or '', - db_data.get('artist') or db_data.get('artist_name') or '', - album_name=db_data.get('album') or db_data.get('album_title'), - duration_seconds=int(dur) if dur else None, - ): - result['lyrics_written'] += 1 - except Exception as e: - logger.debug("retag lyrics fetch failed for %s: %s", fp, e) + lm = tp.get('lyrics_meta') or {} + title = lm.get('title') or db_data.get('title') or '' + artist = lm.get('artist') or db_data.get('artist') or '' + if title: + try: + dur = lm.get('duration') or db_data.get('duration') + wrote = _lyrics_client.create_lrc_file( + fp, title, artist, + album_name=lm.get('album') or db_data.get('album'), + duration_seconds=int(dur) if dur else None, + ) + if wrote: + result['lyrics_written'] += 1 + except Exception as e: + logger.debug("retag lyrics fetch failed for %s: %s", fp, e) if cover_action and cover_data and last_dir: try: @@ -425,18 +431,22 @@ class LibraryRetagJob(RepairJob): unmatched.append(lib['title'] or os.path.basename(lib['file_path'])) # No source match means no re-tag — but album cover art and/or # lyrics still apply to the file, so those modes include an - # art/lyrics-only plan (empty db_data → apply writes no tags). + # art/lyrics-only plan (empty db_data → apply writes NO tags). if cover_action or lyrics_action: - track_plans.append({ + plan_row = { 'file_path': rp, 'track_id': lib['id'], 'title': lib['title'], 'changes': [], - # Carry the library title/artist so lyrics fetch has a query - # even when there's no source match to build db_data from. - 'db_data': ({'title': lib.get('title'), 'artist': artist_name} - if lyrics_action else {}), - }) + 'db_data': {}, # never write tags for an unmatched track + } + if lyrics_action: + # READ-only metadata for the lyrics query — kept OUT of + # db_data so it can never be written as tags. + plan_row['lyrics_meta'] = { + 'title': lib.get('title'), 'artist': artist_name, + 'album': album_title} + track_plans.append(plan_row) continue current = _read_current_tags(rp) plan = plan_track(current, src, album_meta, mode=mode) @@ -453,6 +463,11 @@ class LibraryRetagJob(RepairJob): 'changes': plan['changes'], 'db_data': db_data, } + if lyrics_action: + # READ-only lyrics query metadata (never written as tags). + tp['lyrics_meta'] = { + 'title': lib.get('title'), 'artist': artist_name, + 'album': album_title} if depth == 'full': tp['full_meta'] = _build_full_meta( db_data, src, album_title, artist_name, lib['title']) diff --git a/core/repair_worker.py b/core/repair_worker.py index 4f5392eb..887c6efa 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -1465,6 +1465,8 @@ class RepairWorker: plan = {'file_path': rp, 'db_data': t.get('db_data') or {}} if t.get('full_meta'): plan['full_meta'] = t['full_meta'] + if t.get('lyrics_meta'): + plan['lyrics_meta'] = t['lyrics_meta'] # read-only lyrics query metadata resolved_plans.append(plan) from core.repair_jobs.library_retag import apply_track_plans diff --git a/tests/test_missing_lyrics_job.py b/tests/test_missing_lyrics_job.py index a3b1db92..fe7038e2 100644 --- a/tests/test_missing_lyrics_job.py +++ b/tests/test_missing_lyrics_job.py @@ -185,11 +185,31 @@ def test_apply_track_plans_lyrics_action(tmp_path, monkeypatch): seen.update(title=title) or True) monkeypatch.setattr("core.lyrics_client.lyrics_client", fake_client) - plans = [{"file_path": str(audio), "db_data": {"title": "Song", "artist": "Artist"}}] + plans = [{"file_path": str(audio), "db_data": {}, + "lyrics_meta": {"title": "Song", "artist": "Artist", "album": "Album"}}] res = library_retag.apply_track_plans(plans, lyrics_action=True) assert res["lyrics_written"] == 1 and seen["title"] == "Song" +def test_apply_track_plans_lyrics_never_writes_tags(tmp_path, monkeypatch): + # The lyrics query must come from lyrics_meta, NOT db_data — so an + # unmatched track (db_data={}) gets lyrics fetched but NO tags written. + from core.repair_jobs import library_retag + audio = tmp_path / "t.flac"; audio.write_bytes(b"x") + written = [] + monkeypatch.setattr("core.tag_writer.write_tags_to_file", + lambda fp, db_data, **k: written.append(db_data) or {"success": True}) + monkeypatch.setattr("core.lyrics_client.lyrics_client", + SimpleNamespace(create_lrc_file=lambda *a, **k: True)) + + plans = [{"file_path": str(audio), "db_data": {}, + "lyrics_meta": {"title": "Song", "artist": "Artist", "album": "Al"}}] + res = library_retag.apply_track_plans(plans, lyrics_action=True) + assert res["lyrics_written"] == 1 + # write_tags_to_file was called with an EMPTY db_data — no title/artist leaked in. + assert written == [{}] + + def test_apply_track_plans_no_lyrics_when_disabled(tmp_path, monkeypatch): from core.repair_jobs import library_retag audio = tmp_path / "t.flac"; audio.write_bytes(b"x")