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 `<hermes_home>/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.
This commit is contained in:
Christopher
@@ -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."""
|
||||
|
||||
+117
-14
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user