mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
feat(secrets): adapt 1Password onto the SecretSource interface
Follow-up on the cherry-picked #36896 commits, wiring 1Password into the new registry as the reference *mapped* source: - OnePasswordSource adapter (shape=mapped, scheme=op): fetch-only — precedence, override semantics, conflict warnings, and env writes move to the orchestrator; apply_onepassword_secrets kept as legacy shim like Bitwarden's. - Registered in _ensure_builtin_sources; mapped op:// bindings now outrank bulk Bitwarden project dumps on contested vars. - _cache.py FetchResult/is_valid_env_name re-exported from base so there is exactly one canonical definition; bitwarden.py re-adapted onto the contributor's DiskCache substrate. - ErrorKind classification for op failures (auth/binary/empty/network). - Registry + conformance coverage for OnePasswordSource, incl. the headline multi-source test: both vaults claim the same var, mapped 1Password wins, conflict surfaced, provenance correct. - env_loader tests migrated off the legacy apply_* mocks onto the fetch layer; AUTHOR_MAP entry for @hwrdprkns.
This commit is contained in:
teknium1
Teknium
parent
8a76de962f
commit
8235f484c9
5 changed files with 312 additions and 15 deletions
|
|
@ -55,6 +55,7 @@ from agent.secret_sources._cache import (
|
|||
FetchResult,
|
||||
is_valid_env_name,
|
||||
)
|
||||
from agent.secret_sources.base import ErrorKind, SecretSource
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -477,6 +478,156 @@ def apply_onepassword_secrets(
|
|||
return result
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SecretSource adapter — the registry-facing wrapper around this module.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class OnePasswordSource(SecretSource):
|
||||
"""1Password as a registered secret source.
|
||||
|
||||
Thin adapter over the module's fetch machinery. ``fetch()`` only
|
||||
*fetches* — precedence, override semantics, conflict warnings, and
|
||||
the ``os.environ`` writes are the orchestrator's job
|
||||
(see ``agent.secret_sources.registry.apply_all``).
|
||||
|
||||
1Password is a **mapped** source: the user explicitly binds each env
|
||||
var to an ``op://`` reference under ``secrets.onepassword.env``, so
|
||||
its claims outrank bulk sources (e.g. a Bitwarden project dump) on
|
||||
contested vars.
|
||||
"""
|
||||
|
||||
name = "onepassword"
|
||||
label = "1Password"
|
||||
shape = "mapped"
|
||||
scheme = "op"
|
||||
|
||||
def override_existing(self, cfg: dict) -> bool:
|
||||
# Default True: an explicit VAR→op:// binding is the strongest
|
||||
# user intent there is — leaving a stale .env line in place
|
||||
# should not silently defeat it (same rotation rationale as
|
||||
# Bitwarden).
|
||||
return bool(isinstance(cfg, dict) and cfg.get("override_existing", True))
|
||||
|
||||
def protected_env_vars(self, cfg: dict):
|
||||
token_env = _DEFAULT_TOKEN_ENV
|
||||
if isinstance(cfg, dict):
|
||||
token_env = str(cfg.get("service_account_token_env") or token_env)
|
||||
return frozenset({token_env})
|
||||
|
||||
def config_schema(self) -> dict:
|
||||
return {
|
||||
"enabled": {"description": "Master switch", "default": False},
|
||||
"env": {
|
||||
"description": "Map of ENV_VAR -> op://vault/item/field reference",
|
||||
"default": {},
|
||||
},
|
||||
"account": {
|
||||
"description": "op --account shorthand (empty = default account)",
|
||||
"default": "",
|
||||
},
|
||||
"service_account_token_env": {
|
||||
"description": "Env var holding the service-account token "
|
||||
"(unset = desktop/interactive session)",
|
||||
"default": _DEFAULT_TOKEN_ENV,
|
||||
},
|
||||
"binary_path": {
|
||||
"description": "Pin the op binary (empty = resolve via PATH)",
|
||||
"default": "",
|
||||
},
|
||||
"cache_ttl_seconds": {
|
||||
"description": "Disk+memory cache TTL; 0 disables",
|
||||
"default": 300,
|
||||
},
|
||||
"override_existing": {
|
||||
"description": "Resolved values overwrite .env/shell values",
|
||||
"default": True,
|
||||
},
|
||||
}
|
||||
|
||||
def fetch(self, cfg: dict, home_path: Path) -> FetchResult:
|
||||
cfg = cfg if isinstance(cfg, dict) else {}
|
||||
result = FetchResult()
|
||||
|
||||
env_map = cfg.get("env")
|
||||
valid, warnings = _validate_references(
|
||||
env_map if isinstance(env_map, dict) else None
|
||||
)
|
||||
result.warnings.extend(warnings)
|
||||
if not valid:
|
||||
if not warnings:
|
||||
result.error = (
|
||||
"secrets.onepassword.enabled is true but the env: map is "
|
||||
"empty. Add ENV_VAR: op://vault/item/field entries."
|
||||
)
|
||||
result.error_kind = ErrorKind.NOT_CONFIGURED
|
||||
return result
|
||||
|
||||
binary_path = str(cfg.get("binary_path") or "")
|
||||
binary = find_op(binary_path)
|
||||
result.binary_path = binary
|
||||
if binary is None:
|
||||
if binary_path:
|
||||
result.error = (
|
||||
f"secrets.onepassword.binary_path ({binary_path!r}) is "
|
||||
"not an executable op binary."
|
||||
)
|
||||
else:
|
||||
result.error = (
|
||||
"secrets.onepassword.enabled is true but the op CLI was "
|
||||
"not found on PATH. Install it "
|
||||
"(https://developer.1password.com/docs/cli/get-started/) "
|
||||
"or set secrets.onepassword.binary_path."
|
||||
)
|
||||
result.error_kind = ErrorKind.BINARY_MISSING
|
||||
return result
|
||||
|
||||
try:
|
||||
ttl = float(cfg.get("cache_ttl_seconds", 300))
|
||||
except (TypeError, ValueError):
|
||||
ttl = 300.0
|
||||
|
||||
try:
|
||||
secrets, fetch_warnings = fetch_onepassword_secrets(
|
||||
references=valid,
|
||||
account=str(cfg.get("account") or ""),
|
||||
token_env=str(
|
||||
cfg.get("service_account_token_env") or _DEFAULT_TOKEN_ENV
|
||||
),
|
||||
binary=binary,
|
||||
cache_ttl_seconds=ttl,
|
||||
home_path=home_path,
|
||||
)
|
||||
except RuntimeError as exc:
|
||||
result.error = str(exc)
|
||||
result.error_kind = _classify_op_error(str(exc))
|
||||
return result
|
||||
|
||||
result.secrets = secrets
|
||||
result.warnings.extend(fetch_warnings)
|
||||
return result
|
||||
|
||||
|
||||
def _classify_op_error(message: str) -> ErrorKind:
|
||||
"""Best-effort mapping of op failure text onto the shared taxonomy."""
|
||||
lowered = message.lower()
|
||||
if "timed out" in lowered:
|
||||
return ErrorKind.TIMEOUT
|
||||
if "not found on path" in lowered or "not an executable" in lowered \
|
||||
or "failed to invoke" in lowered:
|
||||
return ErrorKind.BINARY_MISSING
|
||||
if any(tok in lowered for tok in ("unauthorized", "not signed in",
|
||||
"session expired", "authentication",
|
||||
"401", "403")):
|
||||
return ErrorKind.AUTH_FAILED
|
||||
if "empty value" in lowered:
|
||||
return ErrorKind.EMPTY_VALUE
|
||||
if any(tok in lowered for tok in ("network", "connection", "resolve host",
|
||||
"dns")):
|
||||
return ErrorKind.NETWORK
|
||||
return ErrorKind.INTERNAL
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Test hook — used by hermetic tests to flush the cache between cases.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -167,6 +167,13 @@ def _ensure_builtin_sources() -> None:
|
|||
except Exception: # noqa: BLE001 — never block startup
|
||||
logger.warning("Failed to register bundled Bitwarden secret source",
|
||||
exc_info=True)
|
||||
try:
|
||||
from agent.secret_sources.onepassword import OnePasswordSource
|
||||
|
||||
register_source(OnePasswordSource())
|
||||
except Exception: # noqa: BLE001 — never block startup
|
||||
logger.warning("Failed to register bundled 1Password secret source",
|
||||
exc_info=True)
|
||||
|
||||
|
||||
def _reset_registry_for_tests() -> None:
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json"
|
|||
|
||||
# Auto-extracted from noreply emails + manual overrides
|
||||
AUTHOR_MAP = {
|
||||
"taylorhp@gmail.com": "hwrdprkns", # PR #36896 salvage (secrets: 1Password op:// secret source + shared _cache substrate, adapted onto the SecretSource interface)
|
||||
"ishengeqi@163.com": "isheng-eqi", # PR #59428 salvage (cron: reject past one-shot timestamps in update_job fallback + resume_job; #59395). Also PR #59446 salvage (cron: advance one-shot next_run_at before dispatch so concurrent gateway+desktop schedulers can't double-execute; #59229).
|
||||
"derek2000139@qq.com": "derek2000139", # PR #57838 salvage (desktop/windows: pre-write update marker before quit dwell so the renderer's waitForUpdateToFinish gate parks instead of respawning a backend that re-locks venv .pyd files mid-update)
|
||||
"AndreasHiltner@users.noreply.github.com": "AndreasHiltner", # PR #56854 salvage (gateway: route multiplex profile responses through the profile's own adapter — 53-site _adapter_for_source sweep)
|
||||
|
|
|
|||
|
|
@ -466,3 +466,135 @@ class TestBitwardenConformance(SecretSourceConformance):
|
|||
monkeypatch.setattr(bw, "find_bws", lambda **kw: None)
|
||||
monkeypatch.delenv("BWS_ACCESS_TOKEN", raising=False)
|
||||
return BitwardenSource()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 1Password adapter
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestOnePasswordSource:
|
||||
def test_identity(self):
|
||||
from agent.secret_sources.onepassword import OnePasswordSource
|
||||
|
||||
src = OnePasswordSource()
|
||||
assert src.name == "onepassword"
|
||||
assert src.shape == "mapped"
|
||||
assert src.scheme == "op"
|
||||
|
||||
def test_override_existing_defaults_true(self):
|
||||
from agent.secret_sources.onepassword import OnePasswordSource
|
||||
|
||||
src = OnePasswordSource()
|
||||
assert src.override_existing({}) is True
|
||||
assert src.override_existing({"override_existing": False}) is False
|
||||
|
||||
def test_protected_vars_track_token_env(self):
|
||||
from agent.secret_sources.onepassword import OnePasswordSource
|
||||
|
||||
src = OnePasswordSource()
|
||||
assert src.protected_env_vars({}) == frozenset(
|
||||
{"OP_SERVICE_ACCOUNT_TOKEN"}
|
||||
)
|
||||
assert src.protected_env_vars(
|
||||
{"service_account_token_env": "MY_OP_TOKEN"}
|
||||
) == frozenset({"MY_OP_TOKEN"})
|
||||
|
||||
def test_fetch_empty_map_not_configured(self, tmp_path):
|
||||
from agent.secret_sources.onepassword import OnePasswordSource
|
||||
|
||||
result = OnePasswordSource().fetch({"enabled": True}, tmp_path)
|
||||
assert result.error_kind is ErrorKind.NOT_CONFIGURED
|
||||
|
||||
def test_fetch_missing_binary(self, tmp_path, monkeypatch):
|
||||
import agent.secret_sources.onepassword as op
|
||||
|
||||
monkeypatch.setattr(op, "find_op", lambda *_a, **_kw: None)
|
||||
result = op.OnePasswordSource().fetch(
|
||||
{"enabled": True, "env": {"K": "op://V/I/F"}}, tmp_path
|
||||
)
|
||||
assert result.error_kind is ErrorKind.BINARY_MISSING
|
||||
|
||||
def test_fetch_delegates_and_passes_config(self, tmp_path, monkeypatch):
|
||||
import agent.secret_sources.onepassword as op
|
||||
|
||||
monkeypatch.setattr(op, "find_op", lambda *_a, **_kw: Path("/fake/op"))
|
||||
captured = {}
|
||||
|
||||
def _fake_fetch(**kwargs):
|
||||
captured.update(kwargs)
|
||||
return {"K": "v"}, ["warn"]
|
||||
|
||||
monkeypatch.setattr(op, "fetch_onepassword_secrets", _fake_fetch)
|
||||
result = op.OnePasswordSource().fetch(
|
||||
{"enabled": True, "env": {"K": "op://V/I/F"},
|
||||
"account": "team", "service_account_token_env": "MY_TOK"},
|
||||
tmp_path,
|
||||
)
|
||||
assert result.ok and result.secrets == {"K": "v"}
|
||||
assert captured["references"] == {"K": "op://V/I/F"}
|
||||
assert captured["account"] == "team"
|
||||
assert captured["token_env"] == "MY_TOK"
|
||||
|
||||
def test_invalid_refs_warned_not_fatal(self, tmp_path, monkeypatch):
|
||||
import agent.secret_sources.onepassword as op
|
||||
|
||||
monkeypatch.setattr(op, "find_op", lambda *_a, **_kw: Path("/fake/op"))
|
||||
monkeypatch.setattr(op, "fetch_onepassword_secrets",
|
||||
lambda **kw: ({"GOOD": "v"}, []))
|
||||
result = op.OnePasswordSource().fetch(
|
||||
{"enabled": True,
|
||||
"env": {"GOOD": "op://V/I/F", "BAD": "not-a-ref",
|
||||
"bad name": "op://V/I/F"}},
|
||||
tmp_path,
|
||||
)
|
||||
assert result.ok
|
||||
assert len(result.warnings) == 2
|
||||
|
||||
def test_mapped_op_beats_bulk_bitwarden_through_orchestrator(
|
||||
self, tmp_path, monkeypatch
|
||||
):
|
||||
"""The headline multi-source scenario: both vaults claim the same var."""
|
||||
import agent.secret_sources.bitwarden as bw
|
||||
import agent.secret_sources.onepassword as op
|
||||
|
||||
monkeypatch.setenv("BWS_ACCESS_TOKEN", "0.token")
|
||||
monkeypatch.setattr(bw, "find_bws", lambda **kw: Path("/fake/bws"))
|
||||
monkeypatch.setattr(
|
||||
bw, "fetch_bitwarden_secrets",
|
||||
lambda **kw: ({"SHARED_KEY": "from-bitwarden",
|
||||
"BW_ONLY": "bw-val"}, []),
|
||||
)
|
||||
monkeypatch.setattr(op, "find_op", lambda *_a, **_kw: Path("/fake/op"))
|
||||
monkeypatch.setattr(
|
||||
op, "fetch_onepassword_secrets",
|
||||
lambda **kw: ({"SHARED_KEY": "from-1password"}, []),
|
||||
)
|
||||
reg.register_source(bw.BitwardenSource())
|
||||
reg.register_source(op.OnePasswordSource())
|
||||
env = {"BWS_ACCESS_TOKEN": "0.token"}
|
||||
report = reg.apply_all(
|
||||
{
|
||||
# bitwarden listed FIRST — mapped 1Password must still win.
|
||||
"sources": ["bitwarden", "onepassword"],
|
||||
"bitwarden": {"enabled": True, "project_id": "proj"},
|
||||
"onepassword": {"enabled": True,
|
||||
"env": {"SHARED_KEY": "op://V/I/F"}},
|
||||
},
|
||||
tmp_path, environ=env,
|
||||
)
|
||||
assert env["SHARED_KEY"] == "from-1password"
|
||||
assert env["BW_ONLY"] == "bw-val"
|
||||
assert report.provenance["SHARED_KEY"].source == "onepassword"
|
||||
assert report.provenance["BW_ONLY"].source == "bitwarden"
|
||||
assert report.conflicts # the shadowed bitwarden claim is surfaced
|
||||
|
||||
|
||||
class TestOnePasswordConformance(SecretSourceConformance):
|
||||
@pytest.fixture
|
||||
def source(self, monkeypatch):
|
||||
import agent.secret_sources.onepassword as op
|
||||
|
||||
monkeypatch.setattr(op, "find_op", lambda *_a, **_kw: None)
|
||||
monkeypatch.delenv("OP_SERVICE_ACCOUNT_TOKEN", raising=False)
|
||||
return op.OnePasswordSource()
|
||||
|
|
|
|||
|
|
@ -185,10 +185,11 @@ def test_apply_external_secret_sources_dedupes_within_process(tmp_path, monkeypa
|
|||
|
||||
|
||||
def test_apply_external_secret_sources_records_onepassword_origin(tmp_path, monkeypatch):
|
||||
"""When ``apply_onepassword_secrets`` returns applied keys, they end up in
|
||||
"""When the 1Password source resolves refs, applied vars end up in
|
||||
``_SECRET_SOURCES`` labeled ``onepassword``."""
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False)
|
||||
(tmp_path / "config.yaml").write_text(
|
||||
"secrets:\n"
|
||||
" onepassword:\n"
|
||||
|
|
@ -198,16 +199,18 @@ def test_apply_external_secret_sources_records_onepassword_origin(tmp_path, monk
|
|||
encoding="utf-8",
|
||||
)
|
||||
|
||||
from agent.secret_sources.onepassword import FetchResult
|
||||
|
||||
def _fake_apply(**_kwargs):
|
||||
return FetchResult(
|
||||
secrets={"ANTHROPIC_API_KEY": "sk-ant-test"},
|
||||
applied=["ANTHROPIC_API_KEY"],
|
||||
)
|
||||
|
||||
import agent.secret_sources.onepassword as op_module
|
||||
monkeypatch.setattr(op_module, "apply_onepassword_secrets", _fake_apply)
|
||||
|
||||
monkeypatch.setattr(op_module, "find_op", lambda *_a, **_kw: Path("/fake/op"))
|
||||
monkeypatch.setattr(
|
||||
op_module,
|
||||
"fetch_onepassword_secrets",
|
||||
lambda **_kw: ({"ANTHROPIC_API_KEY": "sk-ant-test"}, []),
|
||||
)
|
||||
|
||||
from agent.secret_sources import registry as reg_module
|
||||
|
||||
reg_module._reset_registry_for_tests()
|
||||
|
||||
env_loader._apply_external_secret_sources(tmp_path)
|
||||
|
||||
|
|
@ -255,14 +258,17 @@ def test_apply_external_secret_sources_bad_ttl_does_not_crash(tmp_path, monkeypa
|
|||
|
||||
captured = {}
|
||||
|
||||
from agent.secret_sources.onepassword import FetchResult
|
||||
|
||||
def _fake_apply(**kwargs):
|
||||
def _fake_fetch(**kwargs):
|
||||
captured.update(kwargs)
|
||||
return FetchResult()
|
||||
return {}, []
|
||||
|
||||
import agent.secret_sources.onepassword as op_module
|
||||
monkeypatch.setattr(op_module, "apply_onepassword_secrets", _fake_apply)
|
||||
monkeypatch.setattr(op_module, "find_op", lambda *_a, **_kw: Path("/fake/op"))
|
||||
monkeypatch.setattr(op_module, "fetch_onepassword_secrets", _fake_fetch)
|
||||
|
||||
from agent.secret_sources import registry as reg_module
|
||||
|
||||
reg_module._reset_registry_for_tests()
|
||||
|
||||
env_loader._apply_external_secret_sources(tmp_path)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue