mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(secrets): fold OP_CONNECT_HOST/OP_CONNECT_TOKEN into 1Password auth cache-key
_auth_fingerprint() built the 1Password secret cache-key from the service-account token, OP_ACCOUNT, and OP_SESSION_* vars but omitted OP_CONNECT_HOST/OP_CONNECT_TOKEN, which are in _OP_ENV_ALLOWLIST and are forwarded to the op child (the Connect-server auth path). Rotating OP_CONNECT_TOKEN or re-pointing OP_CONNECT_HOST at a different Connect identity left the fingerprint unchanged, so both the in-process and disk caches kept serving secrets resolved under the old Connect credentials for the full TTL (default 300s, disk-persisted across invocations). This contradicts the function's own docstring invariant that a value cached under a previous identity is never served under a new one; it closes the gap for the Connect path, matching the OP_SESSION_*/service-account paths that are already protected.
This commit is contained in:
parent
7db521a697
commit
a616b9fb0f
2 changed files with 39 additions and 5 deletions
|
|
@ -172,16 +172,19 @@ def _validate_references(
|
|||
def _auth_fingerprint(token_env: str) -> str:
|
||||
"""SHA-256 prefix over the auth material `op` would use.
|
||||
|
||||
Folds in the service-account token, ``OP_ACCOUNT``, and *all*
|
||||
``OP_SESSION_*`` vars (the names `op` actually exports for interactive
|
||||
sessions — ``OP_SESSION_<account_shorthand>``). Signing out and into a
|
||||
different identity therefore changes the cache key, so a value cached under
|
||||
a previous identity is never served under a new one. Never logged or
|
||||
Folds in the service-account token, ``OP_ACCOUNT``, the 1Password Connect
|
||||
``OP_CONNECT_HOST``/``OP_CONNECT_TOKEN``, and *all* ``OP_SESSION_*`` vars
|
||||
(the names `op` actually exports for interactive sessions —
|
||||
``OP_SESSION_<account_shorthand>``). Signing out and into a different
|
||||
identity therefore changes the cache key, so a value cached under a
|
||||
previous identity is never served under a new one. Never logged or
|
||||
displayed; the raw token never leaves this hash.
|
||||
"""
|
||||
parts: List[str] = [
|
||||
f"token={os.environ.get(token_env, '')}",
|
||||
f"account={os.environ.get('OP_ACCOUNT', '')}",
|
||||
f"connect_host={os.environ.get('OP_CONNECT_HOST', '')}",
|
||||
f"connect_token={os.environ.get('OP_CONNECT_TOKEN', '')}",
|
||||
]
|
||||
for key in sorted(os.environ):
|
||||
if key.startswith("OP_SESSION_"):
|
||||
|
|
|
|||
|
|
@ -41,6 +41,8 @@ def _clean_op_env(monkeypatch):
|
|||
monkeypatch.delenv(key, raising=False)
|
||||
monkeypatch.delenv("OP_SERVICE_ACCOUNT_TOKEN", raising=False)
|
||||
monkeypatch.delenv("OP_ACCOUNT", raising=False)
|
||||
monkeypatch.delenv("OP_CONNECT_HOST", raising=False)
|
||||
monkeypatch.delenv("OP_CONNECT_TOKEN", raising=False)
|
||||
yield
|
||||
|
||||
|
||||
|
|
@ -329,6 +331,35 @@ def test_session_change_invalidates_cache(monkeypatch, tmp_path):
|
|||
assert calls["n"] == 2 # cache key changed → refetch
|
||||
|
||||
|
||||
def test_connect_credential_change_invalidates_cache(monkeypatch, tmp_path):
|
||||
"""A different 1Password Connect identity must not reuse a cached value."""
|
||||
fake_op = tmp_path / "op"
|
||||
fake_op.write_text("")
|
||||
calls = {"n": 0}
|
||||
|
||||
def fake_run(*a, **k):
|
||||
calls["n"] += 1
|
||||
return _ok("v")
|
||||
|
||||
monkeypatch.setattr(op.subprocess, "run", fake_run)
|
||||
op._reset_cache_for_tests(tmp_path)
|
||||
|
||||
monkeypatch.setenv("OP_CONNECT_HOST", "https://connect.example.com")
|
||||
monkeypatch.setenv("OP_CONNECT_TOKEN", "tokenA")
|
||||
op.fetch_onepassword_secrets(
|
||||
references={"K": "op://V/I/F"}, cache_ttl_seconds=300,
|
||||
binary=fake_op, home_path=tmp_path,
|
||||
)
|
||||
# Rotate the Connect token → new identity.
|
||||
monkeypatch.setenv("OP_CONNECT_TOKEN", "tokenB")
|
||||
op._CACHE.clear()
|
||||
op.fetch_onepassword_secrets(
|
||||
references={"K": "op://V/I/F"}, cache_ttl_seconds=300,
|
||||
binary=fake_op, home_path=tmp_path,
|
||||
)
|
||||
assert calls["n"] == 2 # cache key changed → refetch
|
||||
|
||||
|
||||
def test_partial_failure_not_cached(monkeypatch, tmp_path):
|
||||
fake_op = tmp_path / "op"
|
||||
fake_op.write_text("")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue