mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-17 04:31:55 +00:00
fix(auth): hermes auth remove sticks for shell-exported env vars (#13418)
Removing an env-seeded credential only cleared ~/.hermes/.env and the current process's os.environ, leaving shell-exported vars (shell profile, systemd EnvironmentFile, launchd plist) to resurrect the entry on the next load_pool() call. This matched the pre-#11485 codex behaviour. Now we suppress env:<VAR> in auth.json on remove, gate _seed_from_env() behind is_source_suppressed(), clear env:* suppressions on auth add, and print a diagnostic pointing at the shell when the var lives there. Applies to every env:* seeded credential (xai, deepseek, moonshot, zai, nvidia, openrouter, anthropic, etc.), not just xai. Reported by @teknium1 from community user 'Artificial Brain' — couldn't remove their xAI key via hermes auth remove.
This commit is contained in:
parent
26abac5afd
commit
b341b19fff
3 changed files with 248 additions and 2 deletions
|
|
@ -1158,10 +1158,22 @@ def _seed_from_singletons(provider: str, entries: List[PooledCredential]) -> Tup
|
||||||
def _seed_from_env(provider: str, entries: List[PooledCredential]) -> Tuple[bool, Set[str]]:
|
def _seed_from_env(provider: str, entries: List[PooledCredential]) -> Tuple[bool, Set[str]]:
|
||||||
changed = False
|
changed = False
|
||||||
active_sources: Set[str] = set()
|
active_sources: Set[str] = set()
|
||||||
|
# Honour user suppression — `hermes auth remove <provider> <N>` for an
|
||||||
|
# env-seeded credential marks the env:<VAR> source as suppressed so it
|
||||||
|
# won't be re-seeded from the user's shell environment or ~/.hermes/.env.
|
||||||
|
# Without this gate the removal is silently undone on the next
|
||||||
|
# load_pool() call whenever the var is still exported by the shell.
|
||||||
|
try:
|
||||||
|
from hermes_cli.auth import is_source_suppressed as _is_source_suppressed
|
||||||
|
except ImportError:
|
||||||
|
def _is_source_suppressed(_p, _s): # type: ignore[misc]
|
||||||
|
return False
|
||||||
if provider == "openrouter":
|
if provider == "openrouter":
|
||||||
token = os.getenv("OPENROUTER_API_KEY", "").strip()
|
token = os.getenv("OPENROUTER_API_KEY", "").strip()
|
||||||
if token:
|
if token:
|
||||||
source = "env:OPENROUTER_API_KEY"
|
source = "env:OPENROUTER_API_KEY"
|
||||||
|
if _is_source_suppressed(provider, source):
|
||||||
|
return changed, active_sources
|
||||||
active_sources.add(source)
|
active_sources.add(source)
|
||||||
changed |= _upsert_entry(
|
changed |= _upsert_entry(
|
||||||
entries,
|
entries,
|
||||||
|
|
@ -1198,6 +1210,8 @@ def _seed_from_env(provider: str, entries: List[PooledCredential]) -> Tuple[bool
|
||||||
if not token:
|
if not token:
|
||||||
continue
|
continue
|
||||||
source = f"env:{env_var}"
|
source = f"env:{env_var}"
|
||||||
|
if _is_source_suppressed(provider, source):
|
||||||
|
continue
|
||||||
active_sources.add(source)
|
active_sources.add(source)
|
||||||
auth_type = AUTH_TYPE_OAUTH if provider == "anthropic" and not token.startswith("sk-ant-api") else AUTH_TYPE_API_KEY
|
auth_type = AUTH_TYPE_OAUTH if provider == "anthropic" and not token.startswith("sk-ant-api") else AUTH_TYPE_API_KEY
|
||||||
base_url = env_url or pconfig.inference_base_url
|
base_url = env_url or pconfig.inference_base_url
|
||||||
|
|
|
||||||
|
|
@ -152,6 +152,22 @@ def auth_add_command(args) -> None:
|
||||||
|
|
||||||
pool = load_pool(provider)
|
pool = load_pool(provider)
|
||||||
|
|
||||||
|
# Clear any env:<VAR> suppressions for this provider — re-adding a
|
||||||
|
# credential is a strong signal the user wants auth for this provider
|
||||||
|
# re-enabled. Matches the Codex device_code re-link pattern below.
|
||||||
|
if not provider.startswith(CUSTOM_POOL_PREFIX):
|
||||||
|
try:
|
||||||
|
from hermes_cli.auth import (
|
||||||
|
_load_auth_store,
|
||||||
|
unsuppress_credential_source,
|
||||||
|
)
|
||||||
|
suppressed = _load_auth_store().get("suppressed_sources", {})
|
||||||
|
for src in list(suppressed.get(provider, []) or []):
|
||||||
|
if src.startswith("env:"):
|
||||||
|
unsuppress_credential_source(provider, src)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
|
||||||
if requested_type == AUTH_TYPE_API_KEY:
|
if requested_type == AUTH_TYPE_API_KEY:
|
||||||
token = (getattr(args, "api_key", None) or "").strip()
|
token = (getattr(args, "api_key", None) or "").strip()
|
||||||
if not token:
|
if not token:
|
||||||
|
|
@ -339,14 +355,56 @@ def auth_remove_command(args) -> None:
|
||||||
print(f"Removed {provider} credential #{index} ({removed.label})")
|
print(f"Removed {provider} credential #{index} ({removed.label})")
|
||||||
|
|
||||||
# If this was an env-seeded credential, also clear the env var from .env
|
# If this was an env-seeded credential, also clear the env var from .env
|
||||||
# so it doesn't get re-seeded on the next load_pool() call.
|
# so it doesn't get re-seeded on the next load_pool() call. If the env
|
||||||
|
# var is also (or only) exported by the user's shell/systemd, .env
|
||||||
|
# cleanup alone is not enough — the next process to call load_pool()
|
||||||
|
# will re-read os.environ and resurrect the entry. Suppress the
|
||||||
|
# env:<VAR> source so _seed_from_env() skips it, and tell the user
|
||||||
|
# where the shell-level copy is still living so they can remove it.
|
||||||
if removed.source.startswith("env:"):
|
if removed.source.startswith("env:"):
|
||||||
|
import os as _os
|
||||||
env_var = removed.source[len("env:"):]
|
env_var = removed.source[len("env:"):]
|
||||||
if env_var:
|
if env_var:
|
||||||
from hermes_cli.config import remove_env_value
|
from hermes_cli.config import get_env_path, remove_env_value
|
||||||
|
from hermes_cli.auth import suppress_credential_source
|
||||||
|
|
||||||
|
# Detect whether the var lives in .env, the shell env, or both,
|
||||||
|
# BEFORE remove_env_value() mutates os.environ.
|
||||||
|
env_in_process = bool(_os.getenv(env_var))
|
||||||
|
env_in_dotenv = False
|
||||||
|
try:
|
||||||
|
env_path = get_env_path()
|
||||||
|
if env_path.exists():
|
||||||
|
env_in_dotenv = any(
|
||||||
|
line.strip().startswith(f"{env_var}=")
|
||||||
|
for line in env_path.read_text(errors="replace").splitlines()
|
||||||
|
)
|
||||||
|
except OSError:
|
||||||
|
pass
|
||||||
|
shell_exported = env_in_process and not env_in_dotenv
|
||||||
|
|
||||||
cleared = remove_env_value(env_var)
|
cleared = remove_env_value(env_var)
|
||||||
if cleared:
|
if cleared:
|
||||||
print(f"Cleared {env_var} from .env")
|
print(f"Cleared {env_var} from .env")
|
||||||
|
suppress_credential_source(provider, removed.source)
|
||||||
|
if shell_exported:
|
||||||
|
print(
|
||||||
|
f"Note: {env_var} is still set in your shell environment "
|
||||||
|
f"(not in ~/.hermes/.env)."
|
||||||
|
)
|
||||||
|
print(
|
||||||
|
" Unset it there (shell profile, systemd EnvironmentFile, "
|
||||||
|
"launchd plist, etc.) or it will keep being visible to Hermes."
|
||||||
|
)
|
||||||
|
print(
|
||||||
|
f" The pool entry is now suppressed — Hermes will ignore "
|
||||||
|
f"{env_var} until you run `hermes auth add {provider}`."
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
print(
|
||||||
|
f"Suppressed env:{env_var} — it will not be re-seeded even "
|
||||||
|
f"if the variable is re-exported later."
|
||||||
|
)
|
||||||
|
|
||||||
# If this was a singleton-seeded credential (OAuth device_code, hermes_pkce),
|
# If this was a singleton-seeded credential (OAuth device_code, hermes_pkce),
|
||||||
# clear the underlying auth store / credential file so it doesn't get
|
# clear the underlying auth store / credential file so it doesn't get
|
||||||
|
|
|
||||||
|
|
@ -1011,3 +1011,177 @@ def test_seed_from_singletons_respects_codex_suppression(tmp_path, monkeypatch):
|
||||||
# Verify the auth store was NOT modified (no auto-import happened)
|
# Verify the auth store was NOT modified (no auto-import happened)
|
||||||
after = json.loads((hermes_home / "auth.json").read_text())
|
after = json.loads((hermes_home / "auth.json").read_text())
|
||||||
assert "openai-codex" not in after.get("providers", {})
|
assert "openai-codex" not in after.get("providers", {})
|
||||||
|
|
||||||
|
|
||||||
|
def test_auth_remove_env_seeded_suppresses_shell_exported_var(tmp_path, monkeypatch, capsys):
|
||||||
|
"""`hermes auth remove xai 1` must stick even when the env var is exported
|
||||||
|
by the shell (not written into ~/.hermes/.env). Before PR for #13371 the
|
||||||
|
removal silently restored on next load_pool() because _seed_from_env()
|
||||||
|
re-read os.environ. Now env:<VAR> is suppressed in auth.json.
|
||||||
|
"""
|
||||||
|
hermes_home = tmp_path / "hermes"
|
||||||
|
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||||
|
|
||||||
|
# Simulate shell export (NOT written to .env)
|
||||||
|
monkeypatch.setenv("XAI_API_KEY", "sk-xai-shell-export")
|
||||||
|
(hermes_home / ".env").write_text("")
|
||||||
|
|
||||||
|
_write_auth_store(
|
||||||
|
tmp_path,
|
||||||
|
{
|
||||||
|
"version": 1,
|
||||||
|
"credential_pool": {
|
||||||
|
"xai": [{
|
||||||
|
"id": "env-1",
|
||||||
|
"label": "XAI_API_KEY",
|
||||||
|
"auth_type": "api_key",
|
||||||
|
"priority": 0,
|
||||||
|
"source": "env:XAI_API_KEY",
|
||||||
|
"access_token": "sk-xai-shell-export",
|
||||||
|
"base_url": "https://api.x.ai/v1",
|
||||||
|
}]
|
||||||
|
},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
from types import SimpleNamespace
|
||||||
|
from hermes_cli.auth_commands import auth_remove_command
|
||||||
|
auth_remove_command(SimpleNamespace(provider="xai", target="1"))
|
||||||
|
|
||||||
|
# Suppression marker written
|
||||||
|
after = json.loads((hermes_home / "auth.json").read_text())
|
||||||
|
assert "env:XAI_API_KEY" in after.get("suppressed_sources", {}).get("xai", [])
|
||||||
|
|
||||||
|
# Diagnostic printed pointing at the shell
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "still set in your shell environment" in out
|
||||||
|
assert "Cleared XAI_API_KEY from .env" not in out # wasn't in .env
|
||||||
|
|
||||||
|
# Fresh simulation: shell re-exports, reload pool
|
||||||
|
monkeypatch.setenv("XAI_API_KEY", "sk-xai-shell-export")
|
||||||
|
from agent.credential_pool import load_pool
|
||||||
|
pool = load_pool("xai")
|
||||||
|
assert not pool.has_credentials(), "pool must stay empty — env:XAI_API_KEY suppressed"
|
||||||
|
|
||||||
|
|
||||||
|
def test_auth_remove_env_seeded_dotenv_only_no_shell_hint(tmp_path, monkeypatch, capsys):
|
||||||
|
"""When the env var lives only in ~/.hermes/.env (not the shell), the
|
||||||
|
shell-hint should NOT be printed — avoid scaring the user about a
|
||||||
|
non-existent shell export.
|
||||||
|
"""
|
||||||
|
hermes_home = tmp_path / "hermes"
|
||||||
|
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||||
|
|
||||||
|
# Key ONLY in .env, shell must not have it
|
||||||
|
monkeypatch.delenv("DEEPSEEK_API_KEY", raising=False)
|
||||||
|
(hermes_home / ".env").write_text("DEEPSEEK_API_KEY=sk-ds-only\n")
|
||||||
|
# Mimic load_env() populating os.environ
|
||||||
|
monkeypatch.setenv("DEEPSEEK_API_KEY", "sk-ds-only")
|
||||||
|
|
||||||
|
_write_auth_store(
|
||||||
|
tmp_path,
|
||||||
|
{
|
||||||
|
"version": 1,
|
||||||
|
"credential_pool": {
|
||||||
|
"deepseek": [{
|
||||||
|
"id": "env-1",
|
||||||
|
"label": "DEEPSEEK_API_KEY",
|
||||||
|
"auth_type": "api_key",
|
||||||
|
"priority": 0,
|
||||||
|
"source": "env:DEEPSEEK_API_KEY",
|
||||||
|
"access_token": "sk-ds-only",
|
||||||
|
}]
|
||||||
|
},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
from types import SimpleNamespace
|
||||||
|
from hermes_cli.auth_commands import auth_remove_command
|
||||||
|
auth_remove_command(SimpleNamespace(provider="deepseek", target="1"))
|
||||||
|
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "Cleared DEEPSEEK_API_KEY from .env" in out
|
||||||
|
assert "still set in your shell environment" not in out
|
||||||
|
assert (hermes_home / ".env").read_text().strip() == ""
|
||||||
|
|
||||||
|
|
||||||
|
def test_auth_add_clears_env_suppression_for_provider(tmp_path, monkeypatch):
|
||||||
|
"""Re-adding a credential via `hermes auth add <provider>` clears any
|
||||||
|
env:<VAR> suppression marker — strong signal the user wants auth back.
|
||||||
|
Matches the Codex device_code re-link behaviour.
|
||||||
|
"""
|
||||||
|
hermes_home = tmp_path / "hermes"
|
||||||
|
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||||
|
monkeypatch.delenv("XAI_API_KEY", raising=False)
|
||||||
|
|
||||||
|
_write_auth_store(
|
||||||
|
tmp_path,
|
||||||
|
{
|
||||||
|
"version": 1,
|
||||||
|
"providers": {},
|
||||||
|
"suppressed_sources": {"xai": ["env:XAI_API_KEY"]},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
from types import SimpleNamespace
|
||||||
|
from hermes_cli.auth import is_source_suppressed
|
||||||
|
from hermes_cli.auth_commands import auth_add_command
|
||||||
|
|
||||||
|
assert is_source_suppressed("xai", "env:XAI_API_KEY") is True
|
||||||
|
auth_add_command(SimpleNamespace(
|
||||||
|
provider="xai", auth_type="api_key",
|
||||||
|
api_key="sk-xai-manual", label="manual",
|
||||||
|
))
|
||||||
|
assert is_source_suppressed("xai", "env:XAI_API_KEY") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_seed_from_env_respects_env_suppression(tmp_path, monkeypatch):
|
||||||
|
"""_seed_from_env() must skip env:<VAR> sources that the user suppressed
|
||||||
|
via `hermes auth remove`. This is the gate that prevents shell-exported
|
||||||
|
keys from resurrecting removed credentials.
|
||||||
|
"""
|
||||||
|
hermes_home = tmp_path / "hermes"
|
||||||
|
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||||
|
monkeypatch.setenv("XAI_API_KEY", "sk-xai-shell-export")
|
||||||
|
|
||||||
|
(hermes_home / "auth.json").write_text(json.dumps({
|
||||||
|
"version": 1,
|
||||||
|
"providers": {},
|
||||||
|
"suppressed_sources": {"xai": ["env:XAI_API_KEY"]},
|
||||||
|
}))
|
||||||
|
|
||||||
|
from agent.credential_pool import _seed_from_env
|
||||||
|
|
||||||
|
entries = []
|
||||||
|
changed, active = _seed_from_env("xai", entries)
|
||||||
|
assert changed is False
|
||||||
|
assert entries == []
|
||||||
|
assert active == set()
|
||||||
|
|
||||||
|
|
||||||
|
def test_seed_from_env_respects_openrouter_suppression(tmp_path, monkeypatch):
|
||||||
|
"""OpenRouter is the special-case branch in _seed_from_env; verify it
|
||||||
|
honours suppression too.
|
||||||
|
"""
|
||||||
|
hermes_home = tmp_path / "hermes"
|
||||||
|
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||||
|
monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-shell-export")
|
||||||
|
|
||||||
|
(hermes_home / "auth.json").write_text(json.dumps({
|
||||||
|
"version": 1,
|
||||||
|
"providers": {},
|
||||||
|
"suppressed_sources": {"openrouter": ["env:OPENROUTER_API_KEY"]},
|
||||||
|
}))
|
||||||
|
|
||||||
|
from agent.credential_pool import _seed_from_env
|
||||||
|
|
||||||
|
entries = []
|
||||||
|
changed, active = _seed_from_env("openrouter", entries)
|
||||||
|
assert changed is False
|
||||||
|
assert entries == []
|
||||||
|
assert active == set()
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue