From ae482b2d41f2e047f7e79993a8d60a8801dd4023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C4=83ng=20Minh=20V=C5=A9?= <145498528+tang-vu@users.noreply.github.com> Date: Sun, 4 Oct 2026 16:48:25 +0700 Subject: [PATCH 1/2] fix: qualify PR #7213 scope on current main --- ods/bin/ods-host-agent.py | 10 ++- .../services/dashboard-api/routers/models.py | 32 ++++---- .../dashboard-api/tests/test_host_agent.py | 50 +++++++++++++ .../dashboard-api/tests/test_models.py | 75 ++++++++++++++++++- 4 files changed, 149 insertions(+), 18 deletions(-) diff --git a/ods/bin/ods-host-agent.py b/ods/bin/ods-host-agent.py index a702377966..e4a73e862d 100755 --- a/ods/bin/ods-host-agent.py +++ b/ods/bin/ods-host-agent.py @@ -791,6 +791,7 @@ def _download_huggingface_artifact( maximum=300, ) code = r''' +import os import shutil import sys from pathlib import Path @@ -812,6 +813,7 @@ def _download_huggingface_artifact( revision=revision, cache_dir=cache_dir, local_files_only=False, + token=os.environ.get("HF_TOKEN") or False, ) dest_path = Path(dest) dest_path.parent.mkdir(parents=True, exist_ok=True) @@ -829,9 +831,11 @@ def _download_huggingface_artifact( ] try: child_env = os.environ.copy() - persisted_hf_token = load_env(INSTALL_DIR / ".env").get("HF_TOKEN", "").strip() - if persisted_hf_token and not child_env.get("HF_TOKEN"): - child_env["HF_TOKEN"] = persisted_hf_token + env_path = INSTALL_DIR / ".env" + if env_path.exists(): + # Settings live-reads HF_TOKEN: rotation and removal must apply to + # downloads too, including disabling the Hub's cached login token. + child_env["HF_TOKEN"] = load_env(env_path).get("HF_TOKEN", "").strip() child_env.setdefault("HF_HUB_ETAG_TIMEOUT", str(response_timeout)) child_env.setdefault("HF_HUB_DOWNLOAD_TIMEOUT", str(response_timeout)) logger.info( diff --git a/ods/extensions/services/dashboard-api/routers/models.py b/ods/extensions/services/dashboard-api/routers/models.py index d0adafd5bc..d001bdc4a6 100644 --- a/ods/extensions/services/dashboard-api/routers/models.py +++ b/ods/extensions/services/dashboard-api/routers/models.py @@ -60,6 +60,7 @@ model_files_dir as model_files_dir, read_env_file_value, read_env_value, + read_persisted_env_value, ) from security import verify_api_key @@ -693,22 +694,18 @@ def _format_size(size_mb: int) -> str: def _hf_token() -> str: - return str( - read_env_file_value("HF_TOKEN", INSTALL_DIR) - or read_env_value("HF_TOKEN", INSTALL_DIR) - or "" - ).strip() + return read_persisted_env_value("HF_TOKEN", INSTALL_DIR).strip() -def _hf_cache_identity() -> str: +def _hf_cache_identity(token: str | None = None) -> str: """Partition metadata caches without retaining or exposing the token.""" - token = _hf_token() + token = _hf_token() if token is None else token return hashlib.sha256(token.encode("utf-8")).hexdigest() if token else "public" -def _hf_headers() -> dict[str, str]: +def _hf_headers(token: str | None = None) -> dict[str, str]: headers = {"User-Agent": "ODS-dashboard/2.5 model-library"} - token = _hf_token() + token = _hf_token() if token is None else token if token: headers["Authorization"] = f"Bearer {token}" return headers @@ -774,7 +771,8 @@ async def _hf_author_avatar_url(author: str) -> str | None: """Resolve an author's uploaded Hub avatar, caching positive and negative results.""" if not _HF_AUTHOR_RE.fullmatch(author): return None - cache_key = (author.lower(), _hf_cache_identity()) + token = _hf_token() + cache_key = (author.lower(), _hf_cache_identity(token)) now = time.monotonic() cached = _hf_avatar_cache_get(cache_key) if cached and now - cached[0] < _HF_AVATAR_CACHE_TTL_SECONDS: @@ -784,6 +782,7 @@ async def _hf_author_avatar_url(author: str) -> str | None: try: payload, _headers = await _hf_get_json( f"/api/{account_type}/{quote(author, safe='')}/overview", + token=token, ) except HTTPException as exc: if exc.status_code == 404: @@ -1010,7 +1009,11 @@ async def _hf_get_json( params: dict[str, Any] | None = None, timeout_seconds: float = 20.0, connect_timeout_seconds: float = 8.0, + token: str | None = None, ) -> tuple[Any, httpx.Headers]: + # Freeze credentials before the first await. Settings may rotate or clear + # HF_TOKEN while a request waits; retries must retain its cache identity. + headers = _hf_headers(token) timeout = httpx.Timeout( timeout_seconds, connect=min(connect_timeout_seconds, timeout_seconds), @@ -1026,7 +1029,7 @@ async def _hf_get_json( response = await client.get( f"{_HF_API_BASE}{path}", params=params, - headers=_hf_headers(), + headers=headers, ) break except httpx.TimeoutException as exc: @@ -1247,7 +1250,8 @@ async def search_huggingface_models( """Search public/authenticated Hub metadata without exposing the token.""" query = q.strip() sort_key = sort if sort in {"downloads", "likes", "lastModified"} else "downloads" - cache_key = (query.lower(), sort_key, limit, _hf_cache_identity()) + token = _hf_token() + cache_key = (query.lower(), sort_key, limit, _hf_cache_identity(token)) now = time.monotonic() cached = _hf_cache_get(cache_key) if cached and now - cached[0] < _HF_SEARCH_CACHE_TTL_SECONDS: @@ -1262,7 +1266,7 @@ async def search_huggingface_models( if query: params["search"] = query try: - payload, _headers = await _hf_get_json("/api/models", params=params) + payload, _headers = await _hf_get_json("/api/models", params=params, token=token) except HTTPException as exc: if cached and exc.status_code in {429, 502, 504}: return {**cached[1], "stale": True} @@ -1274,7 +1278,7 @@ async def search_huggingface_models( "models": models, "query": query, "sort": sort_key, - "authenticated": bool(_hf_token()), + "authenticated": bool(token), "source": "huggingface", } _hf_cache_put(cache_key, response) diff --git a/ods/extensions/services/dashboard-api/tests/test_host_agent.py b/ods/extensions/services/dashboard-api/tests/test_host_agent.py index 502e77b36b..29ea498116 100644 --- a/ods/extensions/services/dashboard-api/tests/test_host_agent.py +++ b/ods/extensions/services/dashboard-api/tests/test_host_agent.py @@ -29,6 +29,56 @@ _spec.loader.exec_module(_mod) +@pytest.mark.parametrize("saved", ["hf_rotated", "", None]) +def test_huggingface_fallback_uses_saved_token_at_sdk_boundary(tmp_path, monkeypatch, saved): + monkeypatch.setattr(_mod, "INSTALL_DIR", tmp_path) + monkeypatch.setenv("HF_TOKEN", "hf_old_process_token") + if saved is not None: + (tmp_path / ".env").write_text(f'HF_TOKEN="{saved}"\n', encoding="utf-8") + calls = [] + artifact = tmp_path / "hub-artifact.gguf" + artifact.write_bytes(b"gguf fixture") + hub = types.ModuleType("huggingface_hub") + + def download(**kwargs): + # The SDK's default uses HF_TOKEN or a cached login; an explicit False + # disables both. Observe the worker's resolved authentication choice. + token = kwargs.get("token") + effective = (os.environ.get("HF_TOKEN") or "hf_cached_login") if token is None else token + calls.append((kwargs, effective)) + return str(artifact) + + hub.hf_hub_download = download + monkeypatch.setitem(sys.modules, "huggingface_hub", hub) + + class Worker: + returncode = 0 + + def __init__(self, cmd, **kwargs): + self.cmd = cmd + self.env = kwargs["env"] + + def communicate(self, timeout): + with monkeypatch.context() as child: + child.setattr(sys, "argv", ["-c", *self.cmd[3:]]) + child.setattr(os, "environ", self.env) + exec(self.cmd[2], {"__name__": "__main__"}) + return "", "" + + monkeypatch.setattr(_mod.subprocess, "Popen", Worker) + destination = tmp_path / "download.gguf.part" + ok, error = _mod._download_huggingface_artifact( + "https://huggingface.co/org/model/resolve/main/model.gguf", + destination, threading.Event(), + ) + assert (ok, error) == (True, "") + assert destination.read_bytes() == artifact.read_bytes() + expected = saved if saved is not None else "hf_old_process_token" + assert calls[0][1] == (expected or False) + assert calls[0][0].get("token") == (expected or False) + assert _mod._model_download_proc is None + + def test_host_selection_serializes_dependency_decisions_with_cli_helper(tmp_path, monkeypatch): """The agent uses the installed host selector, including ordered stops.""" scripts = tmp_path / "scripts" diff --git a/ods/extensions/services/dashboard-api/tests/test_models.py b/ods/extensions/services/dashboard-api/tests/test_models.py index 21de3737a2..69fa32aa68 100644 --- a/ods/extensions/services/dashboard-api/tests/test_models.py +++ b/ods/extensions/services/dashboard-api/tests/test_models.py @@ -3,6 +3,7 @@ from __future__ import annotations import importlib +import hashlib import asyncio import json import os @@ -27,6 +28,78 @@ def _hf_sibling(filename: str, size: int, sha: str) -> dict: } +@pytest.mark.parametrize("saved", ["hf_rotated", "", None]) +def test_huggingface_saved_token_controls_metadata(monkeypatch, tmp_path, saved): + import routers.models as models_router + + monkeypatch.setattr(models_router, "INSTALL_DIR", str(tmp_path)) + monkeypatch.setenv("HF_TOKEN", "hf_old_process_token") + if saved is not None: + (tmp_path / ".env").write_text(f'HF_TOKEN="{saved}"\n', encoding="utf-8") + expected = saved if saved is not None else "hf_old_process_token" + assert models_router._hf_token() == expected + assert models_router._hf_headers().get("Authorization") == ( + f"Bearer {expected}" if expected else None + ) + assert models_router._hf_cache_identity() == ( + hashlib.sha256(expected.encode()).hexdigest() if expected else "public" + ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("operation", ["search", "avatar"]) +async def test_huggingface_retry_keeps_original_cache_credentials(monkeypatch, tmp_path, operation): + import routers.models as models_router + + env = tmp_path / ".env" + env.write_text("HF_TOKEN=\n", encoding="utf-8") + monkeypatch.setattr(models_router, "INSTALL_DIR", str(tmp_path)) + monkeypatch.delenv("HF_TOKEN", raising=False) + monkeypatch.setattr(models_router, "_HF_SEARCH_CACHE", {}) + monkeypatch.setattr(models_router, "_HF_AVATAR_CACHE", {}) + headers = [] + + class Client: + def __init__(self, **_kwargs): + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, *_args): + return None + + async def get(self, url, **kwargs): + authorization = kwargs["headers"].get("Authorization") + headers.append(authorization) + if len(headers) == 1: + # Settings rotates credentials while the first attempt waits. + env.write_text("HF_TOKEN=hf_new_private_token\n", encoding="utf-8") + raise httpx.ReadTimeout("first attempt timed out") + identity = "private" if authorization else "public" + payload = ([{"id": f"org/{identity}", "private": bool(authorization)}] + if operation == "search" else {"avatarUrl": f"/avatars/{identity}.png"}) + return httpx.Response(200, json=payload) + + monkeypatch.setattr(models_router.httpx, "AsyncClient", Client) + + async def lookup(): + if operation == "search": + return await models_router.search_huggingface_models(q="model", sort="downloads", limit=20) + return await models_router._hf_author_avatar_url("org") + + first = await lookup() + env.write_text("HF_TOKEN=\n", encoding="utf-8") + second = await lookup() + assert headers == [None, None] + assert second == first + if operation == "search": + assert first["authenticated"] is False + assert [model["id"] for model in first["models"]] == ["org/public"] + else: + assert first == "https://huggingface.co/avatars/public.png" + + def test_active_model_reader_preserves_unmatched_quote(monkeypatch, tmp_path): import routers.models as models_router @@ -456,7 +529,7 @@ async def profile(path, **_kwargs): }, httpx.Headers() monkeypatch.setattr(models_router, "_HF_AVATAR_CACHE", {}) - monkeypatch.setattr(models_router, "_hf_cache_identity", lambda: "public") + monkeypatch.setattr(models_router, "_hf_cache_identity", lambda *_args: "public") monkeypatch.setattr(models_router, "_hf_get_json", profile) first = await models_router._hf_author_avatar_url("unsloth") From 0d238ded4dfe2bb6d1c0d0f5f6800d8729e2932c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C4=83ng=20Minh=20V=C5=A9?= <145498528+tang-vu@users.noreply.github.com> Date: Sun, 4 Oct 2026 17:07:32 +0700 Subject: [PATCH 2/2] test(models): install SDK fixture through monkeypatch --- ods/extensions/services/dashboard-api/tests/test_host_agent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ods/extensions/services/dashboard-api/tests/test_host_agent.py b/ods/extensions/services/dashboard-api/tests/test_host_agent.py index 29ea498116..305d687755 100644 --- a/ods/extensions/services/dashboard-api/tests/test_host_agent.py +++ b/ods/extensions/services/dashboard-api/tests/test_host_agent.py @@ -48,7 +48,7 @@ def download(**kwargs): calls.append((kwargs, effective)) return str(artifact) - hub.hf_hub_download = download + monkeypatch.setattr(hub, "hf_hub_download", download, raising=False) monkeypatch.setitem(sys.modules, "huggingface_hub", hub) class Worker: