Fix non-MB sources writing IDs to MusicBrainz tags

Spotify and Deezer IDs were being written into MusicBrainz tag fields
(mb_albumid, mb_trackid, etc.) because the generic album_id/track_id
fields are mapped to those tags in MEDIA_FIELD_MAP. This caused
third-party players to reject files with errors like
'Invalid MusicBrainz identifier: 158760742'.

Fix by validating that values destined for mb_* fields match the
standard MusicBrainz UUID format before writing them.

Fixes #6519

Author: benstev1

beets/autotag/hooks.py

@@ -16,6 +16,7 @@
 
 from __future__ import annotations
 
+import re
 from copy import deepcopy
 from dataclasses import dataclass, field
 from functools import cached_property
@@ -44,6 +45,38 @@
 
 log = logging.getLogger("beets")
 
+# MusicBrainz UUIDs standard 8-4-4-4-12 hex format.
+_MB_UUID_RE = re.compile(
+    r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$",
+    re.IGNORECASE,
+)
+
+# Fields in item_data that must contain valid MusicBrainz UUIDs.
+# Values that don't match are dropped
+# cannot accidentally populate MusicBrainz tags with their own numeric/opaque IDs.
+_MB_ID_FIELDS = frozenset(
+    {
+        "mb_albumid",
+        "mb_albumartistid",
+        "mb_albumartistids",
+        "mb_artistid",
+        "mb_artistids",
+        "mb_releasegroupid",
+        "mb_releasetrackid",
+        "mb_trackid",
+    }
+)
+
+
+def _is_valid_mb_id(value: Any) -> bool:
+    """Return True if *value* is a valid MusicBrainz UUID (or a list of them)."""
+    if isinstance(value, list):
+        return bool(value) and all(
+            _MB_UUID_RE.match(str(v)) for v in value if v is not None
+        )
+    return bool(value and _MB_UUID_RE.match(str(value)))
+
+
 SYNCHRONISED_LIST_FIELDS = {
     ("albumtype", "albumtypes"),
     ("artist", "artists"),
@@ -201,6 +234,11 @@ def item_data(self) -> JSONDict:
         ):
             data[media_field] = data.pop(info_field)
 
+        # Drop any MusicBrainz ID field whose value is not a valid MB UUID.
+        for mb_field in _MB_ID_FIELDS:
+            if mb_field in data and not _is_valid_mb_id(data[mb_field]):
+                del data[mb_field]
+
         return data
 
     def __init__(

docs/changelog.rst

@@ -38,6 +38,7 @@ New features
 Bug fixes
 ~~~~~~~~~
 
+<<<<<<< HEAD
 - :ref:`import-cmd` Automatically remux WAV files containing MP3 streams
   (``WAVE_FORMAT_MPEGLAYER3``) to proper MP3 files during import, instead of
   silently importing them with incorrect metadata. :bug:`6455`
@@ -49,6 +50,13 @@ Bug fixes
   registry instead of unconditionally instantiating its own private instance,
   which also restores compatibility with :doc:`plugins/mbpseudo` for
   chroma-triggered lookups. :bug:`6212` :bug:`6441`
+=======
+- Correctly handle semicolon-delimited genre values from externally-tagged
+  files. :bug:`6450`
+- Fix non-MusicBrainz sources (e.g. Spotify, Deezer) incorrectly writing
+  their numeric or opaque IDs into MusicBrainz tag fields, which caused
+  third-party players to reject the files. :bug:`6519`
+>>>>>>> e157fd245 (Fix non-MB sources writing IDs to MusicBrainz tags)
 
 For plugin developers
 ~~~~~~~~~~~~~~~~~~~~~

test/autotag/test_hooks.py

@@ -25,6 +25,7 @@
     AlbumMatch,
     TrackInfo,
     TrackMatch,
+    _is_valid_mb_id,
     correct_list_fields,
 )
 from beets.library import Item
@@ -426,3 +427,146 @@ def test_correct_list_fields(
     data = correct_list_fields(input_data)
 
     assert (data[single_field], data[list_field]) == expected_values
+
+
+# Tests for issue #6519: non-MB sources must not pollute MusicBrainz ID tags
+# Real IDs from the issue report (Deezer numeric) and a typical Spotify ID.
+_SPOTIFY_ALBUM_ID = "4aawyAB9vmqN3uQ7FjRGTy"
+_SPOTIFY_TRACK_ID = "6rqhFgbbKwnb9MLmUQDhG6"
+_SPOTIFY_ARTIST_ID = "4Z8W4fKeB5YxbusRsdQVPb"
+_DEEZER_ALBUM_ID = "158760742"
+_DEEZER_TRACK_ID = "930705291"
+_DEEZER_ARTIST_ID = "3876259001"
+
+_VALID_MB_ALBUM_UUID = "7edb51cb-77d6-4416-a23c-3a8c2994a2c7"
+_VALID_MB_TRACK_UUID = "dfa939ec-118c-4d0f-84a0-60f3d1e6522c"
+_VALID_MB_ARTIST_UUID = "a6623d39-2d8e-4f70-8242-0a9553b91e50"
+
+
+@pytest.mark.parametrize(
+    "value, expected",
+    [
+        # Valid UUIDs
+        (_VALID_MB_ALBUM_UUID, True),
+        (_VALID_MB_TRACK_UUID, True),
+        # Valid UUID list
+        ([_VALID_MB_ALBUM_UUID, _VALID_MB_ARTIST_UUID], True),
+        # Spotify base-62 IDs
+        (_SPOTIFY_ALBUM_ID, False),
+        (_SPOTIFY_TRACK_ID, False),
+        # Deezer numeric IDs
+        (_DEEZER_ALBUM_ID, False),
+        (_DEEZER_TRACK_ID, False),
+        # Edge cases
+        (None, False),
+        ("", False),
+        ([], False),
+    ],
+)
+def test_is_valid_mb_id(value, expected):
+    assert _is_valid_mb_id(value) == expected
+
+
+# Regression tests for https://github.com/beetbox/beets/issues/6519:
+# non-MB sources must not write their IDs into MusicBrainz tags.
+
+
+@pytest.mark.parametrize(
+    "album_id, artist_id, track_id",
+    [
+        (_SPOTIFY_ALBUM_ID, _SPOTIFY_ARTIST_ID, _SPOTIFY_TRACK_ID),
+        (_DEEZER_ALBUM_ID, _DEEZER_ARTIST_ID, _DEEZER_TRACK_ID),
+    ],
+    ids=["spotify", "deezer"],
+)
+def test_non_mb_album_ids_not_in_mb_fields(
+    config, album_id, artist_id, track_id
+):
+    """Non-MB album/artist IDs must not appear in MusicBrainz tag fields."""
+    info = AlbumInfo(
+        tracks=[
+            TrackInfo(
+                title="Track",
+                track_id=track_id,
+                artist_id=artist_id,
+                medium=1,
+                medium_index=1,
+                medium_total=1,
+                index=1,
+            )
+        ],
+        album="Album",
+        album_id=album_id,
+        artist="Artist",
+        artist_id=artist_id,
+        year=2024,
+    )
+    data = info.item_data
+    assert "mb_albumid" not in data
+    assert "mb_albumartistid" not in data
+    assert "mb_albumartistids" not in data
+
+
+@pytest.mark.parametrize(
+    "track_id, artist_id",
+    [
+        (_SPOTIFY_TRACK_ID, _SPOTIFY_ARTIST_ID),
+        (_DEEZER_TRACK_ID, _DEEZER_ARTIST_ID),
+    ],
+    ids=["spotify", "deezer"],
+)
+def test_non_mb_track_ids_not_in_mb_fields(config, track_id, artist_id):
+    """Non-MB track/artist IDs must not appear in MusicBrainz tag fields."""
+    track = TrackInfo(
+        title="Track",
+        track_id=track_id,
+        artist_id=artist_id,
+        medium=1,
+        medium_index=1,
+        index=1,
+    )
+    data = track.item_data
+    assert "mb_trackid" not in data
+    assert "mb_artistid" not in data
+    assert "mb_artistids" not in data
+
+
+def test_valid_mb_album_uuids_are_written(config):
+    """Regression: valid MusicBrainz UUIDs must still be written to MB tags."""
+    info = AlbumInfo(
+        tracks=[
+            TrackInfo(
+                title="Track",
+                track_id=_VALID_MB_TRACK_UUID,
+                artist_id=_VALID_MB_ARTIST_UUID,
+                medium=1,
+                medium_index=1,
+                medium_total=1,
+                index=1,
+            )
+        ],
+        album="Album",
+        album_id=_VALID_MB_ALBUM_UUID,
+        artist="Artist",
+        artist_id=_VALID_MB_ARTIST_UUID,
+        data_source="MusicBrainz",
+        year=2024,
+    )
+    data = info.item_data
+    assert data["mb_albumid"] == _VALID_MB_ALBUM_UUID
+    assert data["mb_albumartistid"] == _VALID_MB_ARTIST_UUID
+
+
+def test_valid_mb_track_uuid_is_written(config):
+    """Regression: valid MusicBrainz UUID in track_id must still be written."""
+    track = TrackInfo(
+        title="Track",
+        track_id=_VALID_MB_TRACK_UUID,
+        artist_id=_VALID_MB_ARTIST_UUID,
+        medium=1,
+        medium_index=1,
+        index=1,
+    )
+    data = track.item_data
+    assert data["mb_trackid"] == _VALID_MB_TRACK_UUID
+    assert data["mb_artistid"] == _VALID_MB_ARTIST_UUID