From 3ee6e6be00c55bc1004592e227117ba422a58bde Mon Sep 17 00:00:00 2001 From: Tang Vu <145498528+tang-vu@users.noreply.github.com> Date: Wed, 26 Aug 2026 00:40:03 +0700 Subject: [PATCH] fix(updates): reject GitHub HTTP error payloads --- .../services/dashboard-api/routers/updates.py | 3 + .../dashboard-api/tests/test_updates.py | 103 ++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/ods/extensions/services/dashboard-api/routers/updates.py b/ods/extensions/services/dashboard-api/routers/updates.py index 1abd001ba..1bc7743f1 100644 --- a/ods/extensions/services/dashboard-api/routers/updates.py +++ b/ods/extensions/services/dashboard-api/routers/updates.py @@ -179,6 +179,7 @@ async def _refresh_release_cache() -> Optional[dict]: f"{_GITHUB_RELEASES_API}/latest", headers=_GITHUB_HEADERS, ) + response.raise_for_status() data = response.json() payload = { "latest": data.get("tag_name", "").lstrip("v"), @@ -232,6 +233,7 @@ async def get_release_manifest(): f"{_GITHUB_RELEASES_API}?per_page=5", headers=_GITHUB_HEADERS, ) + resp.raise_for_status() releases = resp.json() if not isinstance(releases, list): raise httpx.HTTPError(f"unexpected releases response: {type(releases).__name__}") @@ -298,6 +300,7 @@ async def get_update_dry_run(): f"{_GITHUB_RELEASES_API}/latest", headers=_GITHUB_HEADERS, ) + resp.raise_for_status() data = resp.json() latest = _normalize_version(data.get("tag_name")) or None changelog_url = data.get("html_url") or None diff --git a/ods/extensions/services/dashboard-api/tests/test_updates.py b/ods/extensions/services/dashboard-api/tests/test_updates.py index 5737e7b61..95a850345 100644 --- a/ods/extensions/services/dashboard-api/tests/test_updates.py +++ b/ods/extensions/services/dashboard-api/tests/test_updates.py @@ -5,6 +5,7 @@ from unittest.mock import patch, MagicMock, AsyncMock import httpx +import pytest from host_agent_client import AgentHTTPError, AgentUnavailable @@ -81,6 +82,39 @@ async def mock_get(url, **kwargs): assert data["changelog_url"] == "https://github.com/test" +@pytest.mark.asyncio +async def test_release_refresh_keeps_stale_cache_on_http_error(monkeypatch): + """A GitHub HTTP error must not replace useful stale release metadata.""" + import routers.updates as updates_mod + + stale = { + "latest": "2.7.0", + "changelog_url": "https://github.com/Osmantic/ODS/releases/tag/v2.7.0", + "checked_at": "2026-08-25T00:00:00+00:00", + } + monkeypatch.setattr( + updates_mod, + "_version_cache", + {"expires_at": 0.0, "payload": stale}, + ) + + response = httpx.Response( + 403, + request=httpx.Request("GET", updates_mod._GITHUB_RELEASES_API), + json={"message": "API rate limit exceeded"}, + ) + mock_client = AsyncMock() + mock_client.get = AsyncMock(return_value=response) + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + + with patch("routers.updates.httpx.AsyncClient", return_value=mock_client): + payload = await updates_mod._refresh_release_cache() + + assert payload == stale + assert updates_mod._version_cache["payload"] == stale + + def test_build_version_result_strips_v_prefix_from_current(): """Current versions stored with a 'v' prefix (matching the release tag convention, e.g. a .version file of 'v2.6.0') must normalize before @@ -272,6 +306,43 @@ async def mock_get(url, **kwargs): assert "error" in data +def test_releases_manifest_rejects_non_success_http_payload(test_client, tmp_path, monkeypatch): + """A JSON-shaped error body must not be published as a release manifest.""" + import routers.updates as updates_mod + + install_dir = tmp_path / "ods" + install_dir.mkdir() + (install_dir / ".version").write_text("2.6.0", encoding="utf-8") + monkeypatch.setattr(updates_mod, "INSTALL_DIR", str(install_dir)) + + response = httpx.Response( + 503, + request=httpx.Request("GET", updates_mod._GITHUB_RELEASES_API), + json=[ + { + "tag_name": "v999.0.0", + "name": "proxy error body", + "body": "not a real GitHub release", + } + ], + ) + mock_client = AsyncMock() + mock_client.get = AsyncMock(return_value=response) + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + + with patch("routers.updates.httpx.AsyncClient", return_value=mock_client): + resp = test_client.get( + "/api/releases/manifest", + headers=test_client.auth_headers, + ) + + assert resp.status_code == 200 + data = resp.json() + assert data["releases"][0]["version"] == "2.6.0" + assert data["error"] == "Could not fetch release information" + + def test_releases_manifest_github_error_fallback_reads_json_version(test_client, tmp_path, monkeypatch): """GET /api/releases/manifest reads JSON-formatted .version files.""" import routers.updates as updates_mod @@ -345,6 +416,38 @@ def test_update_dry_run_with_env_and_version(test_client, tmp_path, monkeypatch) assert "SOME_OTHER_KEY" not in data["env_keys"] +def test_update_dry_run_reports_github_http_error(test_client, tmp_path, monkeypatch): + """A rate-limit response is not a valid no-update result.""" + import routers.updates as updates_mod + + install_dir = tmp_path / "ods" + install_dir.mkdir() + (install_dir / ".env").write_text("ODS_VERSION=2.6.0\n", encoding="utf-8") + monkeypatch.setattr(updates_mod, "INSTALL_DIR", str(install_dir)) + + response = httpx.Response( + 403, + request=httpx.Request("GET", updates_mod._GITHUB_RELEASES_API), + json={"message": "API rate limit exceeded"}, + ) + mock_client = AsyncMock() + mock_client.get = AsyncMock(return_value=response) + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + + with patch("routers.updates.httpx.AsyncClient", return_value=mock_client): + resp = test_client.get( + "/api/update/dry-run", + headers=test_client.auth_headers, + ) + + assert resp.status_code == 200 + data = resp.json() + assert data["latest_version"] is None + assert data["update_available"] is False + assert "403 Forbidden" in data["version_check_error"] + + def test_update_dry_run_parses_quoted_ods_version(test_client, tmp_path, monkeypatch): """A quoted ODS_VERSION must not leak its quotes into the semver compare.""" import routers.updates as updates_mod