From a616b9fb0f5940f3ded6f363b4c959a56d851645 Mon Sep 17 00:00:00 2001
From: briandevans <252620095+briandevans@users.noreply.github.com>
Date: Tue, 7 Jul 2026 04:34:53 -0700
Subject: [PATCH] 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.
---
 agent/secret_sources/onepassword.py | 13 +++++++-----
 tests/test_onepassword_secrets.py   | 31 +++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/agent/secret_sources/onepassword.py b/agent/secret_sources/onepassword.py
index 5c0ddf1ab7e..c756353311f 100644
--- a/agent/secret_sources/onepassword.py
+++ b/agent/secret_sources/onepassword.py
@@ -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_"):
diff --git a/tests/test_onepassword_secrets.py b/tests/test_onepassword_secrets.py
index 76da6b635e4..38d33c8ab42 100644
--- a/tests/test_onepassword_secrets.py
+++ b/tests/test_onepassword_secrets.py
@@ -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("")