From 8196d409a06eeaf2afaba910f673b80b532c8140 Mon Sep 17 00:00:00 2001 From: Christopher <210261288+Christopher-Schulze@users.noreply.github.com> Date: Fri, 14 Aug 2026 05:40:06 +0200 Subject: [PATCH] perf(tools): persist OSV malware-check verdict cache to disk The OSV preflight is a synchronous network POST to api.osv.dev (up to 10s timeout) on every MCP stdio server start. Tools like `hermes mcp test` and MCP reconnect ladders spawn the same package repeatedly, but the previous in-process cache was empty after every process restart, so each run re-queried OSV and added 5.91x variance to the connection-time span. Persist the malware-check verdict cache to `/cache/osv_check.json`. Cache expiry is stored as an absolute wall-clock timestamp, so it survives restarts and monotonic-clock skew. Loading only adds missing keys so an in-memory overwrite (e.g. a test forcing expiry) is not silently reversed by the disk copy. Writes are atomic (temp file + rename) and happen under the existing cache lock. Fixes the Hermes-owned OSV preflight variance component of #68416. Server-side `initialize` time is outside Hermes' control. - Adds `hermes_constants.get_hermes_home()` lazy import to keep `tools/osv_check.py` import-safe (stdlib + typing only at module scope). - Switches cache timestamps from `time.monotonic()` to `time.time()` for persistence compatibility. - Updates `tests/tools/test_osv_check.py` fixture to isolate disk cache per test via `HERMES_HOME` + `tmp_path`, and adds regression tests for persistence, reload, and disk format. --- tests/tools/test_osv_check.py | 58 ++++++++++++++- tools/osv_check.py | 131 ++++++++++++++++++++++++++++++---- 2 files changed, 173 insertions(+), 16 deletions(-) diff --git a/tests/tools/test_osv_check.py b/tests/tools/test_osv_check.py index 72e81058e6..695f94214c 100644 --- a/tests/tools/test_osv_check.py +++ b/tests/tools/test_osv_check.py @@ -66,14 +66,18 @@ class TestParsePackageFromArgs: class TestCheckPackageForMalware: @pytest.fixture(autouse=True) - def _fresh_cache(self): + def _fresh_cache(self, tmp_path, monkeypatch): from tools import osv_check + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) with osv_check._cache_lock: osv_check._cache.clear() + osv_check._disk_cache_loaded = False + (tmp_path / "cache" / "osv_check.json").unlink(missing_ok=True) yield with osv_check._cache_lock: osv_check._cache.clear() - + osv_check._disk_cache_loaded = False + (tmp_path / "cache" / "osv_check.json").unlink(missing_ok=True) def test_clean_package(self): """Clean package returns None (allow).""" mock_response = MagicMock() @@ -189,6 +193,56 @@ class TestCheckPackageForMalware: check_package_for_malware("uvx", ["mcp-server-fetch"]) assert mock_url.call_count == 2 + def test_disk_cache_persists_and_reloads(self, tmp_path, monkeypatch): + """A warm disk cache is reused by a fresh in-process cache.""" + from tools import osv_check + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + mock_response = MagicMock() + mock_response.read.return_value = json.dumps({"vulns": []}).encode() + mock_response.__enter__ = lambda s: s + mock_response.__exit__ = MagicMock(return_value=False) + + with patch("tools.osv_check.urllib.request.urlopen", return_value=mock_response) as mock_url: + check_package_for_malware("uvx", ["mcp-server-persist"]) + + cache_file = tmp_path / "cache" / "osv_check.json" + assert cache_file.exists(), "disk cache should be written after a warm result" + + with osv_check._cache_lock: + osv_check._cache.clear() + osv_check._disk_cache_loaded = False + + with patch("tools.osv_check.urllib.request.urlopen", return_value=mock_response) as mock_url2: + check_package_for_malware("uvx", ["mcp-server-persist"]) + + assert mock_url2.call_count == 0, "disk cache must satisfy the second call" + + def test_disk_cache_format_versioned(self, tmp_path, monkeypatch): + """Disk cache JSON has a version field and recoverable entries.""" + from tools import osv_check + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + mock_response = MagicMock() + mock_response.read.return_value = json.dumps({"vulns": []}).encode() + mock_response.__enter__ = lambda s: s + mock_response.__exit__ = MagicMock(return_value=False) + + with patch("tools.osv_check.urllib.request.urlopen", return_value=mock_response): + check_package_for_malware("uvx", ["mcp-server-format"]) + + cache_file = tmp_path / "cache" / "osv_check.json" + with open(cache_file, "r", encoding="utf-8") as f: + data = json.load(f) + assert data["version"] == osv_check._DISK_CACHE_VERSION + assert "entries" in data + key = "PyPI|mcp-server-format|" + assert key in data["entries"] + assert "expiry" in data["entries"][key] + assert data["entries"][key]["result"] is None + class TestLiveOsvQuery: """Live integration test against the real OSV API. Skipped if offline.""" diff --git a/tools/osv_check.py b/tools/osv_check.py index b543dd7f33..16dd355c56 100644 --- a/tools/osv_check.py +++ b/tools/osv_check.py @@ -9,44 +9,145 @@ Fail-open: network errors allow the package to proceed. Inspired by Block/goose's extension malware check. """ - import json import logging import os import re +import tempfile import threading import time import urllib.request +from pathlib import Path from typing import Optional, Tuple - logger = logging.getLogger(__name__) _OSV_ENDPOINT = os.getenv("OSV_ENDPOINT", "https://api.osv.dev/v1/query") _TIMEOUT = 10 # seconds -# Result cache: (ecosystem, package, version) -> (expiry_monotonic, result). -# MCP reconnect ladders, stdio recycles, and parked-server self-probes re-run -# the preflight for the SAME package on every spawn attempt. Without a cache, -# a flapping server turns into a sustained OSV query/DNS stream — the #75485 -# incident logged 779K api.osv.dev DNS queries in 16h from revival loops. -# Malware advisories don't appear or vanish on second-to-second timescales, -# so a successful verdict (clean OR blocked) is reusable. Network failures -# are NOT cached: fail-open already covers them, and caching a failure could -# mask a real advisory once connectivity returns. +# Result cache: (ecosystem, package, version) -> (expiry_timestamp, result). +# MCP reconnect ladders, stdio recycles, parked-server self-probes, and +# repeated `hermes mcp test` invocations re-run the preflight for the SAME +# package on every spawn attempt. Without a cache, a flapping server turns +# into a sustained OSV query/DNS stream — the #75485 incident logged 779K +# api.osv.dev DNS queries in 16h from revival loops. Malware advisories don't +# appear or vanish on second-to-second timescales, so a successful verdict +# (clean OR blocked) is reusable. Network failures are NOT cached: fail-open +# already covers them, and caching a failure could mask a real advisory once +# connectivity returns. +# +# The cache is also persisted to disk inside the Hermes home so that separate +# `hermes mcp test` processes (and gateway restarts) reuse a warm verdict +# instead of re-querying OSV. Expiry is stored as absolute wall-clock time so +# it survives process restarts and monotonic-clock skew. _CACHE_TTL_S = float(os.getenv("OSV_CHECK_CACHE_TTL", "3600")) _CACHE_MAX_ENTRIES = 256 _cache: dict = {} _cache_lock = threading.Lock() +_disk_cache_loaded = False +_DISK_CACHE_VERSION = 1 + + +def _disk_cache_path() -> Optional[Path]: + """Return the path for the persistent OSV verdict cache. + + Uses ``hermes_constants.get_hermes_home()`` so the cache follows the + active profile and is isolated across Hermes homes. The cache directory + is created on demand. Returns ``None`` when Hermes home cannot be + resolved, in which case only the in-process cache is used. + """ + try: + from hermes_constants import get_hermes_home + + home = get_hermes_home() + except Exception: + return None + try: + cache_dir = home / "cache" + cache_dir.mkdir(parents=True, exist_ok=True) + return cache_dir / "osv_check.json" + except Exception: + return None + + +def _load_disk_cache() -> None: + """Load persistent cache entries from disk into the in-process cache. + + Called once under ``_cache_lock`` on first use. Skips expired or + malformed entries. Uses absolute wall-clock timestamps. Only adds + missing keys so an in-memory overwrite (e.g. a test forcing expiry) + is not silently reversed by the disk copy. + """ + global _disk_cache_loaded + if _disk_cache_loaded: + return + _disk_cache_loaded = True + + path = _disk_cache_path() + if path is None: + return + + try: + with open(path, "r", encoding="utf-8") as f: + data = json.load(f) + except Exception: + return + + if not isinstance(data, dict) or data.get("version") != _DISK_CACHE_VERSION: + return + + now = time.time() + for key_str, entry in data.get("entries", {}).items(): + if not isinstance(entry, dict): + continue + expiry = entry.get("expiry") + result = entry.get("result") + if expiry is None or expiry <= now: + continue + parts = key_str.split("|", 2) + if len(parts) != 3: + continue + key = (parts[0], parts[1], parts[2] or None) + if key not in _cache: + _cache[key] = (expiry, result) + + +def _save_disk_cache() -> None: + """Persist the in-process cache to disk. + + Caller must hold ``_cache_lock`` for consistency. Writes atomically to + a sibling file then renames into place. + """ + path = _disk_cache_path() + if path is None: + return + + entries: dict = {} + for key, (expiry, result) in _cache.items(): + key_str = "|".join(str(k) if k is not None else "" for k in key) + entries[key_str] = {"expiry": expiry, "result": result} + + data = {"version": _DISK_CACHE_VERSION, "entries": entries} + + try: + tmp_fd, tmp_path = tempfile.mkstemp( + dir=path.parent, prefix=path.name + ".tmp-" + ) + with os.fdopen(tmp_fd, "w", encoding="utf-8") as f: + json.dump(data, f) + os.replace(tmp_path, path) + except Exception as exc: + logger.debug("Failed to save OSV disk cache to %s: %s", path, exc) def _cache_get(key) -> Tuple[bool, Optional[str]]: """Return (hit, result) for a fresh cache entry.""" with _cache_lock: + _load_disk_cache() entry = _cache.get(key) if entry is None: return False, None expiry, result = entry - if time.monotonic() >= expiry: + if time.time() >= expiry: del _cache[key] return False, None return True, result @@ -54,13 +155,15 @@ def _cache_get(key) -> Tuple[bool, Optional[str]]: def _cache_put(key, result: Optional[str]) -> None: with _cache_lock: + _load_disk_cache() if len(_cache) >= _CACHE_MAX_ENTRIES: - now = time.monotonic() + now = time.time() for k in [k for k, (exp, _) in _cache.items() if exp <= now]: del _cache[k] if len(_cache) >= _CACHE_MAX_ENTRIES: _cache.clear() # tiny working set in practice; safe reset - _cache[key] = (time.monotonic() + _CACHE_TTL_S, result) + _cache[key] = (time.time() + _CACHE_TTL_S, result) + _save_disk_cache() def check_package_for_malware(