From 35bc21a5bbf4e23fe5eee7ad62a398ff69259e53 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Thu, 15 Oct 2026 22:49:06 +0800 Subject: [PATCH] fix(civarchive): don't resolve foreign-ecosystem file IDs into CivitAI metadata CivArchive's /sha256 endpoint also indexes files from other ecosystems (e.g. civision/ModelScope) whose model_id/model_version_id belong to a different ID space. _resolve_version_from_files fed those IDs into the CivitAI-ID-based /models/{id} endpoint, which resolves modelVersionId globally, so a colliding ID silently linked the local file to an unrelated CivitAI model (hash f3a5370b... resolved to CivitAI model 119656 via version id 130022). Two guards: skip file entries whose source marks a foreign ecosystem, and require the resolved version's files to contain the queried SHA256 before accepting it. --- py/services/civarchive_client.py | 58 +++++++++++++++++- tests/services/test_civarchive_client.py | 75 +++++++++++++++++++++++- 2 files changed, 130 insertions(+), 3 deletions(-) diff --git a/py/services/civarchive_client.py b/py/services/civarchive_client.py index 11de0d5f..a70b5132 100644 --- a/py/services/civarchive_client.py +++ b/py/services/civarchive_client.py @@ -14,6 +14,15 @@ from .errors import RateLimitError logger = logging.getLogger(__name__) +#: ``source`` values on CivArchive file entries that belong to a foreign +#: ecosystem with its own model/version ID space (e.g. "civision" indexes +#: ModelScope-hosted files). Feeding those IDs into the CivitAI-ID-based +#: /models/{id} endpoint resolves to an unrelated CivitAI model, so entries +#: carrying one of these sources are skipped before any lookup happens. +#: Entries with no ``source`` stay eligible — the post-resolution hash check +#: in ``_resolve_version_from_files`` is the real correctness guard. +_FOREIGN_ECOSYSTEM_SOURCES = frozenset({"civision"}) + class CivArchiveClient: _instance = None _lock = asyncio.Lock() @@ -301,7 +310,30 @@ class CivArchiveClient: return version_copy - async def _resolve_version_from_files(self, payload: Dict[str, Any]) -> Optional[Dict[str, Any]]: + @staticmethod + def _version_contains_hash(version: Dict[str, Any], expected_hash: str) -> bool: + """Check whether any file of the resolved version matches the queried hash""" + expected = expected_hash.strip().upper() + if not expected: + return False + files = version.get("files") + if not isinstance(files, list): + return False + for file_data in files: + if not isinstance(file_data, dict): + continue + hashes = file_data.get("hashes") + if isinstance(hashes, dict): + sha256 = hashes.get("SHA256") or hashes.get("sha256") + if isinstance(sha256, str) and sha256.strip().upper() == expected: + return True + return False + + async def _resolve_version_from_files( + self, + payload: Dict[str, Any], + expected_hash: Optional[str] = None, + ) -> Optional[Dict[str, Any]]: """Fallback to fetch version data when only file metadata is available""" if not isinstance(payload, dict): return None @@ -312,6 +344,13 @@ class CivArchiveClient: for file_data in files: if not isinstance(file_data, dict): continue + source = file_data.get("source") + if isinstance(source, str) and source.strip().lower() in _FOREIGN_ECOSYSTEM_SOURCES: + logger.debug( + "Skipping CivArchive file entry from foreign ecosystem source=%s", + source, + ) + continue model_id = file_data.get("model_id") or file_data.get("modelId") version_id = file_data.get("model_version_id") or file_data.get("modelVersionId") if model_id is None or version_id is None: @@ -332,6 +371,18 @@ class CivArchiveClient: continue resolved = await self.get_model_version(model_id, version_id) if resolved: + if expected_hash and not self._version_contains_hash(resolved, expected_hash): + # The version endpoint resolves modelVersionId globally, + # ignoring the model id in the path, so foreign-ecosystem + # IDs can resolve to an unrelated CivitAI version. Only a + # hash match proves the resolved version is the right one. + logger.debug( + "Discarding CivArchive version %s/%s: resolved files do not match hash %s", + model_id, + version_id, + expected_hash[:10], + ) + continue return resolved return None @@ -352,7 +403,10 @@ class CivArchiveClient: if transformed: return transformed, None - resolved = await self._resolve_version_from_files(cast(Dict[str, Any], payload)) + resolved = await self._resolve_version_from_files( + cast(Dict[str, Any], payload), + expected_hash=model_hash, + ) if resolved: return resolved, None diff --git a/tests/services/test_civarchive_client.py b/tests/services/test_civarchive_client.py index 5eaca7bf..2c42659e 100644 --- a/tests/services/test_civarchive_client.py +++ b/tests/services/test_civarchive_client.py @@ -222,7 +222,9 @@ async def test_get_model_by_hash_uses_file_fallback(downloader, monkeypatch): client = await CivArchiveClient.get_instance() - result, error = await client.get_model_by_hash("fallback") + result, error = await client.get_model_by_hash( + "e2b7a280d6539556f23f380b3f71e4e22bc4524445c4c96526e117c6005c6ad3" + ) assert error is None assert result is not None @@ -231,6 +233,77 @@ async def test_get_model_by_hash_uses_file_fallback(downloader, monkeypatch): assert any("/models/1746460" in call["url"] for call in downloader.calls) +async def test_get_model_by_hash_skips_foreign_ecosystem_file_entries(downloader): + """File entries from a foreign ecosystem (e.g. civision) carry IDs from a + different ID space; resolving them would hit an unrelated CivitAI model.""" + file_only_payload = { + "data": { + "files": [ + { + "filename": "Kiara-Krea_c1-st4000.safetensors", + "url": "https://www.modelscope.ai/models/arbinshado/Kiara-Krea/files", + "source": "civision", + "model_id": "455108", + "model_version_id": "130022", + } + ] + } + } + + async def fake_make_request(method, url, use_auth=False, **kwargs): + downloader.calls.append({"url": url, "params": kwargs.get("params")}) + if "/sha256/" in url: + return True, copy.deepcopy(file_only_payload) + return False, "unexpected" + + downloader.make_request = fake_make_request + + client = await CivArchiveClient.get_instance() + + result, error = await client.get_model_by_hash("f3a5370b") + + assert result is None + assert error == "No version data found" + assert not any("/models/" in call["url"] for call in downloader.calls) + + +async def test_get_model_by_hash_discards_resolved_version_on_hash_mismatch(downloader): + """The version endpoint resolves modelVersionId globally, so a colliding + foreign ID can resolve to a valid-looking but wrong version. The resolved + files must contain the queried hash, otherwise the result is discarded.""" + file_only_payload = { + "data": { + "files": [ + { + "model_id": 1746460, + "model_version_id": 1976567, + } + ] + } + } + + version_payload = _base_civarchive_payload() + + async def fake_make_request(method, url, use_auth=False, **kwargs): + downloader.calls.append({"url": url, "params": kwargs.get("params")}) + if "/sha256/" in url: + return True, copy.deepcopy(file_only_payload) + if "/models/1746460" in url: + return True, copy.deepcopy(version_payload) + return False, "unexpected" + + downloader.make_request = fake_make_request + + client = await CivArchiveClient.get_instance() + + result, error = await client.get_model_by_hash( + "f3a5370b60e2a10ba8c35ddd95dc8a573b3843e032bd507a941b2cc467884955" + ) + + assert result is None + assert error == "No version data found" + + async def test_get_model_by_hash_handles_not_found(downloader): async def fake_make_request(method, url, use_auth=False, **kwargs): return False, "Resource not found"