fix(picker): scope Vertex explicit-config to Hermes signals, not ambient ADC
Address review feedback: the gate reused has_vertex_credentials(), which also returns True for an ambient GOOGLE_APPLICATION_CREDENTIALS path. That var is commonly set globally for unrelated GCP work, so a user who never configured Hermes for Vertex would see it in the explicit-only picker and could spend against those credentials — weakening the explicit-configuration guarantee the gate documents (mirrors the existing _IMPLICIT_ENV_VARS carve-out). Add has_explicit_vertex_config() in agent/vertex_adapter.py that checks only Hermes-scoped signals — VERTEX_PROJECT_ID / vertex.project_id (project override) or a resolvable VERTEX_CREDENTIALS_PATH — and NOT GOOGLE_APPLICATION_CREDENTIALS. Route the auth gate through it. Adds a regression test asserting an ambient GOOGLE_APPLICATION_CREDENTIALS path alone does not mark Vertex explicit, and updates the existing test to drive the real config signal instead of mocking has_vertex_credentials().
This commit is contained in:
@@ -226,3 +226,26 @@ def has_vertex_credentials() -> bool:
|
||||
if _resolve_project_override():
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def has_explicit_vertex_config() -> bool:
|
||||
"""True only when the user deliberately pointed Hermes at Vertex.
|
||||
|
||||
Stricter than :func:`has_vertex_credentials`, which also returns True for
|
||||
an ambient ``GOOGLE_APPLICATION_CREDENTIALS`` path — a var commonly set
|
||||
globally for unrelated GCP work. That ambient signal must NOT mark Vertex
|
||||
"explicitly configured" for the model-picker gate, or a user who never set
|
||||
Hermes up for Vertex would suddenly see it (and could spend against those
|
||||
credentials). So this checks only Hermes-scoped signals:
|
||||
|
||||
* ``VERTEX_PROJECT_ID`` env or ``vertex.project_id`` in config.yaml
|
||||
(``_resolve_project_override``), or
|
||||
* a resolvable ``VERTEX_CREDENTIALS_PATH`` service-account file
|
||||
(the Hermes-specific path var — NOT ``GOOGLE_APPLICATION_CREDENTIALS``).
|
||||
"""
|
||||
if _resolve_project_override():
|
||||
return True
|
||||
sa_path = _get_secret("VERTEX_CREDENTIALS_PATH")
|
||||
if sa_path and os.path.exists(sa_path):
|
||||
return True
|
||||
return False
|
||||
|
||||
@@ -2009,6 +2009,39 @@ def is_provider_explicitly_configured(provider_id: str) -> bool:
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# 5. OAuth-token / cloud-SDK providers (Vertex AI, Bedrock) have NO API-key
|
||||
# env var to detect in step 3 and mint short-lived tokens from ADC / a
|
||||
# service account / the AWS SDK chain. The user "explicitly configures"
|
||||
# them by writing non-secret routing settings into config.yaml
|
||||
# (``vertex.project_id`` / a credentials path, ``bedrock.region``) rather
|
||||
# than by pasting a key — so without this branch such a provider is only
|
||||
# ever "explicitly configured" while it is the *current* provider, and it
|
||||
# silently vanishes from explicit-only pickers (desktop chat model menu)
|
||||
# otherwise. Treat the presence of that deliberate config as explicit.
|
||||
#
|
||||
# NOTE: this uses has_explicit_vertex_config(), NOT has_vertex_credentials()
|
||||
# — the latter also counts an ambient GOOGLE_APPLICATION_CREDENTIALS path
|
||||
# (commonly set globally for unrelated GCP work), which would mark Vertex
|
||||
# explicit for users who never set Hermes up for it. Only Hermes-scoped
|
||||
# signals (VERTEX_PROJECT_ID / vertex.project_id / VERTEX_CREDENTIALS_PATH)
|
||||
# count here.
|
||||
try:
|
||||
if normalized in ("vertex", "google-vertex", "vertex-ai", "gcp-vertex", "vertexai"):
|
||||
from agent.vertex_adapter import has_explicit_vertex_config
|
||||
|
||||
if has_explicit_vertex_config():
|
||||
return True
|
||||
elif normalized == "bedrock":
|
||||
from hermes_cli.config import load_config as _load_cfg
|
||||
|
||||
bedrock_cfg = _load_cfg().get("bedrock")
|
||||
if isinstance(bedrock_cfg, dict) and str(
|
||||
bedrock_cfg.get("region") or ""
|
||||
).strip():
|
||||
return True
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return False
|
||||
|
||||
|
||||
|
||||
@@ -52,6 +52,70 @@ def test_ambient_pool_source_does_not_count_as_explicit(tmp_path, monkeypatch):
|
||||
assert is_provider_explicitly_configured("copilot") is False
|
||||
|
||||
|
||||
def test_vertex_adc_counts_as_explicit_when_config_present(tmp_path, monkeypatch):
|
||||
"""A keyless Vertex provider is explicitly configured when the user pointed
|
||||
Hermes at it (VERTEX_PROJECT_ID / vertex.project_id / VERTEX_CREDENTIALS_PATH),
|
||||
even when it is NOT the current provider — otherwise it silently vanishes
|
||||
from explicit-only pickers (desktop chat model menu) unless already selected."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||
for var in ("VERTEX_PROJECT_ID", "VERTEX_CREDENTIALS_PATH", "GOOGLE_APPLICATION_CREDENTIALS"):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
_write_auth_store(tmp_path, {"version": 1, "providers": {}, "active_provider": None})
|
||||
|
||||
from hermes_cli.auth import is_provider_explicitly_configured
|
||||
|
||||
# vertex.project_id in config.yaml is a deliberate, Hermes-scoped signal.
|
||||
_write_config(tmp_path, {
|
||||
"model": {"provider": "anthropic", "default": "claude-opus-4-8"},
|
||||
"vertex": {"project_id": "my-gcp-project"},
|
||||
})
|
||||
assert is_provider_explicitly_configured("vertex") is True
|
||||
|
||||
# No Hermes-scoped Vertex config at all → stays hidden.
|
||||
_write_config(tmp_path, {"model": {"provider": "anthropic", "default": "claude-opus-4-8"}})
|
||||
assert is_provider_explicitly_configured("vertex") is False
|
||||
|
||||
|
||||
def test_vertex_ambient_google_creds_env_does_not_count_as_explicit(tmp_path, monkeypatch):
|
||||
"""An ambient GOOGLE_APPLICATION_CREDENTIALS path (commonly set globally for
|
||||
unrelated GCP work) must NOT mark Vertex explicit — only Hermes-scoped
|
||||
signals do. Regression guard for the picker gate (PR review feedback)."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||
monkeypatch.delenv("VERTEX_PROJECT_ID", raising=False)
|
||||
monkeypatch.delenv("VERTEX_CREDENTIALS_PATH", raising=False)
|
||||
_write_config(tmp_path, {"model": {"provider": "anthropic", "default": "claude-opus-4-8"}})
|
||||
_write_auth_store(tmp_path, {"version": 1, "providers": {}, "active_provider": None})
|
||||
|
||||
# A real, existing SA file pointed to ONLY by the ambient Google var.
|
||||
sa = tmp_path / "adc.json"
|
||||
sa.write_text("{}")
|
||||
monkeypatch.setenv("GOOGLE_APPLICATION_CREDENTIALS", str(sa))
|
||||
|
||||
from hermes_cli.auth import is_provider_explicitly_configured
|
||||
assert is_provider_explicitly_configured("vertex") is False
|
||||
|
||||
|
||||
def test_bedrock_region_counts_as_explicit(tmp_path, monkeypatch):
|
||||
"""Bedrock (AWS SDK auth, no API key) is explicitly configured once the
|
||||
user pins a region in config.yaml, mirroring the Vertex keyless case."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||
_write_auth_store(tmp_path, {"version": 1, "providers": {}, "active_provider": None})
|
||||
|
||||
from hermes_cli.auth import is_provider_explicitly_configured
|
||||
|
||||
_write_config(tmp_path, {
|
||||
"model": {"provider": "anthropic", "default": "claude-opus-4-8"},
|
||||
"bedrock": {"region": "us-east-1"},
|
||||
})
|
||||
assert is_provider_explicitly_configured("bedrock") is True
|
||||
|
||||
_write_config(tmp_path, {
|
||||
"model": {"provider": "anthropic", "default": "claude-opus-4-8"},
|
||||
"bedrock": {"region": ""},
|
||||
})
|
||||
assert is_provider_explicitly_configured("bedrock") is False
|
||||
|
||||
|
||||
def test_returns_true_when_moa_reference_slot_uses_provider(tmp_path, monkeypatch):
|
||||
"""MoA advisor slots are explicit provider selections for auth gating."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||
|
||||
Reference in New Issue
Block a user