fix: fingerprint .env key names and provider plugins; drop a profile's models cache on delete/create

A retained per-profile snapshot is now rejected after a switch when the target
profile's .env credential set or model-provider plugins changed, and a
recreated same-name profile no longer inherits the deleted profile's catalog.
This commit is contained in:
carlotestor committed 2026-09-23 15:31:44 +00:00
1 parent 78bb366560
commit 7f2e812280
5 files changed
+334 -32

No files matched your search

+115 -20
View File
@@ -7603,25 +7603,36 @@ def _get_models_cache_path() -> Path:
``_models_cache_path`` to an isolated tmp file). ``_models_cache_path`` to an isolated tmp file).
""" """
try: try:
from api.profiles import get_active_profile_name, _is_root_profile from api.profiles import get_active_profile_name
name = (get_active_profile_name() or "").strip() return _models_cache_path_for_profile(get_active_profile_name())
if not name or _is_root_profile(name):
return _models_cache_path
# Defensive filename sanitization: the cookie-derived profile name is
# already validated by _PROFILE_ID_RE at the request boundary, but keep
# the on-disk filename safe regardless of how the name was resolved.
safe = re.sub(r"[^a-z0-9_-]", "_", name.lower())[:64]
if not safe:
return _models_cache_path
# Splice the profile into the default filename: models_cache.json →
# models_cache.<safe>.json, keeping the default's parent dir + suffix.
base = _models_cache_path
return base.with_name(f"{base.stem}.{safe}{base.suffix}")
except Exception: except Exception:
return _models_cache_path return _models_cache_path
def _models_cache_path_for_profile(name: str | None) -> Path:
"""Return the /api/models disk-cache path owned by profile *name*."""
from api.profiles import _is_root_profile
name = (name or "").strip()
if not name or _is_root_profile(name):
return _models_cache_path
# Keep the on-disk filename safe regardless of how the name was resolved.
safe = re.sub(r"[^a-z0-9_-]", "_", name.lower())[:64]
if not safe:
return _models_cache_path
base = _models_cache_path
return base.with_name(f"{base.stem}.{safe}{base.suffix}")
def delete_profile_models_cache(name: str) -> None:
"""Unlink profile *name*'s models disk snapshot (profile delete/create)."""
try:
os.unlink(str(_models_cache_path_for_profile(name)))
except (OSError, ValueError):
pass
def _get_auth_store_path() -> Path: def _get_auth_store_path() -> Path:
"""Return the auth.json path for the active Hermes profile.""" """Return the auth.json path for the active Hermes profile."""
try: try:
@@ -7892,6 +7903,88 @@ def _auth_store_semantic_fingerprint(path: Path) -> dict:
return fp return fp
def _active_profile_home() -> Path:
try:
from api.profiles import get_active_hermes_home as _gah
return _gah()
except ImportError:
return _DEFAULT_HERMES_HOME
def _models_cache_env_fingerprint(path: Path) -> dict:
"""Return the sorted names of non-empty keys in a profile ``.env``.
Credential presence decides which providers are detected; values are
never recorded, so rotating a key keeps the snapshot.
"""
p = Path(path).expanduser()
fp: dict = {"path": str(p)}
try:
text = p.read_text(encoding="utf-8")
except FileNotFoundError:
fp["missing"] = True
return fp
except OSError:
st = p.stat()
fp["mtime_ns"], fp["size"] = st.st_mtime_ns, st.st_size
return fp
keys = set()
for line in text.splitlines():
line = line.strip()
if line.startswith("export "):
line = line[len("export "):].lstrip()
if not line or line.startswith("#") or "=" not in line:
continue
k, v = line.split("=", 1)
if v.strip().strip('"').strip("'"):
keys.add(k.strip())
fp["present_keys"] = sorted(keys)
return fp
def _plugin_manifest_fields(plugin_dir: Path) -> dict | None:
for filename in ("plugin.yaml", "plugin.yml"):
manifest = plugin_dir / filename
try:
text = manifest.read_text(encoding="utf-8", errors="replace")
except OSError:
continue
fields = {}
for line in text.splitlines():
key, sep, value = line.partition(":")
if sep and not line[:1].isspace() and key.strip() in ("name", "kind", "version"):
fields[key.strip()] = value.strip().strip("\"'")
return fields
return None
def _models_cache_plugin_fingerprint(home: Path) -> list:
"""Return ``[dir, name, version]`` for the profile's model-provider plugins.
Mirrors providers' discovery: every dir under ``plugins/model-providers/``
plus flat ``plugins/<dir>`` entries whose manifest declares that kind.
"""
found = []
plugins_root = Path(home).expanduser() / "plugins"
for base, flat in ((plugins_root / "model-providers", False), (plugins_root, True)):
try:
children = sorted(base.iterdir())
except OSError:
continue
for child in children:
if not child.is_dir() or child.name.startswith(("_", ".")):
continue
if flat and child.name == "model-providers":
continue
fields = _plugin_manifest_fields(child)
if flat and (fields or {}).get("kind") != "model-provider":
continue
fields = fields or {}
found.append([str(child.relative_to(plugins_root)), fields.get("name", ""), fields.get("version", "")])
return found
def _models_cache_source_fingerprint() -> dict: def _models_cache_source_fingerprint() -> dict:
"""Return the current config/auth/catalog fingerprint for /api/models cache. """Return the current config/auth/catalog fingerprint for /api/models cache.
@@ -7903,9 +7996,12 @@ def _models_cache_source_fingerprint() -> dict:
mtime/size fingerprint because it is only rewritten on deliberate user mtime/size fingerprint because it is only rewritten on deliberate user
edits (which can change anything) and does not churn on a timer. edits (which can change anything) and does not churn on a timer.
""" """
home = _active_profile_home()
return { return {
"config_yaml": _models_cache_file_fingerprint(_get_config_path()), "config_yaml": _models_cache_file_fingerprint(_get_config_path()),
"auth_json": _auth_store_semantic_fingerprint(_get_auth_store_path()), "auth_json": _auth_store_semantic_fingerprint(_get_auth_store_path()),
"env": _models_cache_env_fingerprint(home / ".env"),
"plugins": _models_cache_plugin_fingerprint(home),
"catalog": _models_cache_catalog_fingerprint(), "catalog": _models_cache_catalog_fingerprint(),
} }
@@ -8181,12 +8277,11 @@ def invalidate_models_cache(*, delete_disk: bool = True):
cache rebuild runs. cache rebuild runs.
``delete_disk=False`` drops only the in-memory snapshot and leaves the ``delete_disk=False`` drops only the in-memory snapshot and leaves the
per-profile disk cache in place. Use it when the *sources* have not per-profile disk cache in place. Use it when the caller only needs the
changed and the caller merely needs the next request to re-resolve which next request to re-resolve which profile's catalog to serve (e.g. a
profile's catalog to serve (e.g. a per-client profile switch): the disk per-client profile switch). The snapshot is reused only while
cache is already keyed per profile and guarded by _models_cache_source_fingerprint() (config, auth, .env key names,
_models_cache_source_fingerprint(), so a stale snapshot is rejected on provider plugins, catalog) still matches.
read without paying for a full rebuild (live provider fetches).
""" """
global _cache_build_in_progress, _available_models_cache, _available_models_cache_ts global _cache_build_in_progress, _available_models_cache, _available_models_cache_ts
global _available_models_live_rebuild_ts, _available_models_cache_source_fingerprint, _cache_build_cv global _available_models_live_rebuild_ts, _available_models_cache_source_fingerprint, _cache_build_cv
+12
View File
@@ -2623,6 +2623,7 @@ def create_profile_api(name: str, clone_from: str = None,
model_provider=model_provider, model_provider=model_provider,
) )
_drop_profile_models_cache(name)
# Invalidate cached root-profile-name lookup; create_profile may have added # Invalidate cached root-profile-name lookup; create_profile may have added
# a new profile that flips is_default semantics on the agent side (#1612). # a new profile that flips is_default semantics on the agent side (#1612).
_SKILLS_STATS_CACHE.clear() _SKILLS_STATS_CACHE.clear()
@@ -2651,6 +2652,16 @@ def create_profile_api(name: str, clone_from: str = None,
} }
def _drop_profile_models_cache(name: str) -> None:
"""A deleted or new profile must never inherit a same-name models snapshot."""
try:
from api.config import delete_profile_models_cache
delete_profile_models_cache(name)
except Exception:
logger.debug("Failed to drop models cache for profile %s", name, exc_info=True)
def delete_profile_api(name: str) -> dict: def delete_profile_api(name: str) -> dict:
"""Delete a profile. Switches to default first if it's the active one. """Delete a profile. Switches to default first if it's the active one.
@@ -2684,6 +2695,7 @@ def delete_profile_api(name: str) -> dict:
else: else:
raise ValueError(f"Profile '{name}' does not exist.") raise ValueError(f"Profile '{name}' does not exist.")
_drop_profile_models_cache(name)
# Drop cached root-profile-name lookup — list_profiles_api() shape changed. # Drop cached root-profile-name lookup — list_profiles_api() shape changed.
_SKILLS_STATS_CACHE.clear() _SKILLS_STATS_CACHE.clear()
_invalidate_list_profiles_cache() _invalidate_list_profiles_cache()
+27 -9
View File
@@ -20,7 +20,7 @@ which the #7556 review flagged as an undocumented runtime contract.
`_sync_models_cache_provenance()` must run at every site that publishes or `_sync_models_cache_provenance()` must run at every site that publishes or
invalidates the snapshot, so the tuple can never tear. invalidates the snapshot, so the tuple can never tear.
## The three source axes ## The five source axes
`_models_cache_source_fingerprint()` is the single chokepoint. A cache is served `_models_cache_source_fingerprint()` is the single chokepoint. A cache is served
only when every axis matches the value recorded in the cache — the disk reader only when every axis matches the value recorded in the cache — the disk reader
@@ -31,6 +31,8 @@ the fingerprint captured at publish time.
| --- | --- | --- | | --- | --- | --- |
| `config_yaml` | stat identity: `mtime_ns` + size (`_models_cache_file_fingerprint`) | The file is rewritten only on deliberate user edits, and any edit can change the provider/model set, so the cheap conservative identity wins. | | `config_yaml` | stat identity: `mtime_ns` + size (`_models_cache_file_fingerprint`) | The file is rewritten only on deliberate user edits, and any edit can change the provider/model set, so the cheap conservative identity wins. |
| `auth_json` | content hash with a volatile-key deny-list (`_auth_store_semantic_fingerprint`, `_AUTH_FINGERPRINT_VOLATILE_KEYS`) | The credential store is rewritten roughly every 14 minutes by credential-pool / OAuth refresh; none of those rotating fields feed `detected_providers` or the returned catalog, and stat identity made the 24h cache churn on every refresh (RCA `t_d127953d` / `t_16551f61`). | | `auth_json` | content hash with a volatile-key deny-list (`_auth_store_semantic_fingerprint`, `_AUTH_FINGERPRINT_VOLATILE_KEYS`) | The credential store is rewritten roughly every 14 minutes by credential-pool / OAuth refresh; none of those rotating fields feed `detected_providers` or the returned catalog, and stat identity made the 24h cache churn on every refresh (RCA `t_d127953d` / `t_16551f61`). |
| `env` | sorted **names** of non-empty keys in the active profile's `.env` (`_models_cache_env_fingerprint`); values are never recorded | Env credentials decide `detected_providers`. Adding or removing a key changes the catalog; rotating a value does not. |
| `plugins` | `[dir, name, version]` for each model-provider plugin in the active profile home (`_models_cache_plugin_fingerprint`): every dir under `plugins/model-providers/`, plus flat `plugins/<dir>` entries whose `plugin.yaml` declares `kind: model-provider` | Plugin providers feed the catalog through `api/plugin_providers.py`. Installing, removing, or upgrading one must invalidate the catalog. The discovery rules match `providers._scan_home_layer`. |
| `catalog` | baked-in provider catalog sha256 (`_PROVIDER_MODELS` + `_PROVIDER_DISPLAY`) plus the Codex local catalog (`_codex_models_cache_fingerprint`, `_CODEX_CACHE_FINGERPRINT_VOLATILE_KEYS`) | A restart after a catalog change must not keep serving a persisted payload for up to 24h (#2443). Codex rewrites `~/.codex/models_cache.json` on its own timer, bumping `mtime_ns` and size while models, `etag`, and `client_version` stay identical, so the Codex axis hashes **content** with only the refresh timestamps (`fetched_at`, `updated_at`) removed (#7540, #7556). | | `catalog` | baked-in provider catalog sha256 (`_PROVIDER_MODELS` + `_PROVIDER_DISPLAY`) plus the Codex local catalog (`_codex_models_cache_fingerprint`, `_CODEX_CACHE_FINGERPRINT_VOLATILE_KEYS`) | A restart after a catalog change must not keep serving a persisted payload for up to 24h (#2443). Codex rewrites `~/.codex/models_cache.json` on its own timer, bumping `mtime_ns` and size while models, `etag`, and `client_version` stay identical, so the Codex axis hashes **content** with only the refresh timestamps (`fetched_at`, `updated_at`) removed (#7540, #7556). |
## Invariant: deny-lists are one-directional ## Invariant: deny-lists are one-directional
@@ -99,14 +101,21 @@ keeps this mode. `delete_disk=False` is for when the sources have **not**
changed and the caller only needs the next request to re-resolve *which* changed and the caller only needs the next request to re-resolve *which*
profile's catalog to serve. profile's catalog to serve.
`POST /api/profile/switch` uses `delete_disk=False`. The disk cache is already `POST /api/profile/switch` uses `delete_disk=False`. The disk cache is keyed
keyed per profile (`_get_models_cache_path()`), and a stale or wrong-profile per profile (`_get_models_cache_path()`), and `_is_loadable_disk_cache()`
snapshot is rejected on read by `_is_loadable_disk_cache()` via the source rejects any snapshot whose source fingerprint no longer matches. The fingerprint
fingerprint above, so deleting it on a switch bought no correctness — it only covers every profile-local input to the catalog: `config.yaml`, `auth.json`,
forced a full cold rebuild (live provider `fetch_models` calls, several seconds) the `.env` key names, and the model-provider plugins. A switch therefore reuses
on every switch. Because this mode leans entirely on the fingerprint check, it the snapshot only when none of those inputs changed, and skips the full cold
is safe only while that check stays the single gate for serving a disk snapshot rebuild (live provider `fetch_models` calls). This mode depends on the
(change-protocol items 1 and 4). fingerprint being the only gate for serving a disk snapshot (change-protocol
items 1 and 4). A new catalog input must be added as an axis before anything
can rely on this mode.
A profile's snapshot is not stored in its home, so deleting the home leaves the
file behind. `delete_profile_api()` and `create_profile_api()` both unlink
`models_cache.<name>.json` (`delete_profile_models_cache()`), so a profile that
is recreated with the same name never inherits the old catalog.
## Change protocol ## Change protocol
@@ -137,6 +146,15 @@ visibility change, any catalog field, any unknown field — still invalidate.
reloads it without a live rebuild, the default still unlinks it, and the reloads it without a live rebuild, the default still unlinks it, and the
executed `/api/profile/switch` route passes `delete_disk=False`. executed `/api/profile/switch` route passes `delete_disk=False`.
`tests/test_profile_switch_models_cache_source_axes.py` covers the safety side
of that mode. It saves a real snapshot, changes one source in the target
profile, runs the switch, and asserts a fresh rebuild for each of these cases:
adding or removing a `.env` key, installing a plugin (in either location),
removing a plugin, bumping a plugin's version, and deleting then recreating the
profile. Two tests pin the speed-up: with unchanged sources, and after rotating
a `.env` value, the switch still reuses the snapshot, and no secret value is
written to disk.
## References ## References
Issues/PRs: #2443, #7540, #7556, #7558. RCAs: `t_d127953d`, `t_16551f61`. Issues/PRs: #2443, #7540, #7556, #7558. RCAs: `t_d127953d`, `t_16551f61`.
@@ -0,0 +1,177 @@
"""A retained per-profile models disk snapshot must be rejected after a switch
when a catalog source changed in the target profile: its ``.env`` provider
credentials, its installed model-provider plugins, or the profile itself
(delete + recreate with the same name).
Each test saves a real snapshot for profile ``demo`` through
``_save_models_cache_to_disk()``, changes one source while ``default`` is
active, performs the per-client switch exactly as ``/api/profile/switch`` does
(``invalidate_models_cache(delete_disk=False)``), and asserts the first
``get_available_models()`` for ``demo`` serves the fresh rebuild.
"""
from __future__ import annotations
import os
import shutil
import sys
from types import SimpleNamespace
from pathlib import Path
import pytest
def _catalog(label: str) -> dict:
return {
"active_provider": "openai",
"default_model": label,
"configured_model_badges": {},
"groups": [
{
"provider": "OpenAI",
"provider_id": "openai",
"models": [{"id": label, "label": label, "supports_fast_tier": False}],
}
],
"aliases": {},
}
@pytest.fixture
def two_profiles(tmp_path: Path, monkeypatch):
import api.config as cfg
import api.profiles as profiles
monkeypatch.delenv("HERMES_CONFIG_PATH", raising=False)
default_home = tmp_path / ".hermes"
default_home.mkdir()
(default_home / "config.yaml").write_text("model:\n default: default-model\n", encoding="utf-8")
demo_home = default_home / "profiles" / "demo"
demo_home.mkdir(parents=True)
(demo_home / "config.yaml").write_text("model:\n default: demo-model\n", encoding="utf-8")
os.utime(demo_home / "config.yaml", (1_000_000, 1_000_000))
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", default_home)
monkeypatch.setattr(profiles, "_active_profile", "default")
monkeypatch.setattr(profiles._tls, "profile", None, raising=False)
monkeypatch.setattr(cfg, "_models_cache_path", tmp_path / "models_cache.json")
monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: None)
monkeypatch.setattr(cfg, "_invoke_models_rebuild", lambda _builder: _catalog("fresh-model"))
for attr in ("_cfg_mtime", "_cfg_path", "_cfg_fingerprint"):
monkeypatch.setattr(cfg, attr, getattr(cfg, attr), raising=False)
saved_cfg = dict(cfg._cfg_cache)
cfg.invalidate_models_cache(delete_disk=False)
cfg.reload_config()
yield SimpleNamespace(cfg=cfg, profiles=profiles, demo_home=demo_home, cache=tmp_path / "models_cache.demo.json")
profiles.clear_request_profile()
cfg.invalidate_models_cache(delete_disk=False)
cfg._cfg_cache.clear()
cfg._cfg_cache.update(saved_cfg)
def _save_demo_snapshot(env) -> None:
env.profiles.set_request_profile("demo")
try:
env.cfg._save_models_cache_to_disk(_catalog("snapshot-model"))
finally:
env.profiles.clear_request_profile()
assert env.cache.exists()
def _switch_to_demo_and_fetch(env) -> dict:
env.profiles.switch_profile("demo", process_wide=False)
env.cfg.invalidate_models_cache(delete_disk=False)
env.profiles.set_request_profile("demo")
try:
return env.cfg.get_available_models()
finally:
env.profiles.clear_request_profile()
def _write_plugin(root: Path, name: str, version: str, *, flat: bool) -> Path:
base = root / "plugins" if flat else root / "plugins" / "model-providers"
d = base / name
d.mkdir(parents=True, exist_ok=True)
(d / "plugin.yaml").write_text(
f"name: {name}\nkind: model-provider\nversion: {version}\n", encoding="utf-8"
)
return d
def test_unchanged_sources_keep_serving_the_retained_snapshot(two_profiles):
"""Control: with no source change the switch reuses the snapshot (the speed-up)."""
_save_demo_snapshot(two_profiles)
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "snapshot-model"
def test_switch_after_adding_env_key_rejects_snapshot(two_profiles):
_save_demo_snapshot(two_profiles)
(two_profiles.demo_home / ".env").write_text("DEEPSEEK_API_KEY=sk-new\n", encoding="utf-8")
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
def test_switch_after_removing_env_key_rejects_snapshot(two_profiles):
env_file = two_profiles.demo_home / ".env"
env_file.write_text("DEEPSEEK_API_KEY=sk-old\nOTHER=1\n", encoding="utf-8")
_save_demo_snapshot(two_profiles)
env_file.write_text("OTHER=1\n", encoding="utf-8")
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
def test_env_value_rotation_keeps_snapshot_and_fingerprint_has_no_secret(two_profiles):
env_file = two_profiles.demo_home / ".env"
env_file.write_text("DEEPSEEK_API_KEY=sk-secret-one\n", encoding="utf-8")
_save_demo_snapshot(two_profiles)
assert "sk-secret-one" not in two_profiles.cache.read_text(encoding="utf-8")
env_file.write_text("DEEPSEEK_API_KEY=sk-secret-two\n", encoding="utf-8")
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "snapshot-model"
@pytest.mark.parametrize("flat", [False, True], ids=["model-providers-dir", "flat-install-dir"])
def test_switch_after_installing_plugin_rejects_snapshot(two_profiles, flat):
_save_demo_snapshot(two_profiles)
_write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=flat)
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
def test_switch_after_removing_plugin_rejects_snapshot(two_profiles):
plugin = _write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=False)
_save_demo_snapshot(two_profiles)
shutil.rmtree(plugin)
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
def test_switch_after_plugin_version_change_rejects_snapshot(two_profiles):
_write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=True)
_save_demo_snapshot(two_profiles)
_write_plugin(two_profiles.demo_home, "acme", "2.0.0", flat=True)
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
def test_delete_profile_removes_its_models_cache(two_profiles, monkeypatch):
import api.profiles as profiles
monkeypatch.setattr(profiles, "_is_isolated_profile_mode", lambda: False)
monkeypatch.setitem(sys.modules, "hermes_cli.profiles", None)
_save_demo_snapshot(two_profiles)
profiles.delete_profile_api("demo")
assert not two_profiles.demo_home.exists()
assert not two_profiles.cache.exists()
def test_recreate_same_name_profile_does_not_resurrect_old_catalog(two_profiles, monkeypatch):
import api.profiles as profiles
monkeypatch.setattr(profiles, "_is_isolated_profile_mode", lambda: False)
monkeypatch.setitem(sys.modules, "hermes_cli.profiles", None)
monkeypatch.setattr(profiles, "list_profiles_api", lambda: [])
_save_demo_snapshot(two_profiles)
stale = two_profiles.cache.read_text(encoding="utf-8")
profiles.delete_profile_api("demo")
# A snapshot left behind by an older build or another process.
two_profiles.cache.write_text(stale, encoding="utf-8")
profiles.create_profile_api("demo")
assert not two_profiles.cache.exists()
(two_profiles.demo_home / "config.yaml").write_text("model:\n default: demo-model\n", encoding="utf-8")
os.utime(two_profiles.demo_home / "config.yaml", (1_000_000, 1_000_000))
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
@@ -5,9 +5,9 @@
deleted the on-disk ``models_cache.<profile>.json`` — a test-isolation deleted the on-disk ``models_cache.<profile>.json`` — a test-isolation
behaviour — which forced a full cold rebuild (live provider ``fetch_models`` behaviour — which forced a full cold rebuild (live provider ``fetch_models``
HTTPS calls, ~490k deepcopy calls) on every switch even though no source had HTTPS calls, ~490k deepcopy calls) on every switch even though no source had
changed. The disk cache is already keyed per profile and rejected on read when changed. The disk cache is keyed per profile and rejected on read when
``_models_cache_source_fingerprint()`` differs, so deleting it buys no ``_models_cache_source_fingerprint()`` differs; the source changes that must
correctness on the switch path and only costs seconds of latency. still force a rebuild are covered in test_profile_switch_models_cache_source_axes.py.
The switch route now uses ``invalidate_models_cache(delete_disk=False)``. The switch route now uses ``invalidate_models_cache(delete_disk=False)``.
""" """