diff --git a/.gitignore b/.gitignore index c820e0a5510..e4240ea36e7 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ __pycache__/ .venv .vscode/ .env +.op.env .env.local .env.development.local .env.test.local diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 2c7a4825e8d..9d5d81b2386 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -2105,8 +2105,20 @@ def _seed_from_env(provider: str, entries: List[PooledCredential]) -> Tuple[bool # changes to the .env file. def _get_env_prefer_dotenv(key: str) -> str: env_file = load_env() - val = env_file.get(key) or _get_secret(key, "") or "" - return val.strip() + raw = env_file.get(key, "").strip() + env_val = os.environ.get(key, "").strip() + # If .env contains an unresolved op:// reference, prefer the + # already-resolved value from os.environ (set by + # load_hermes_dotenv() -> apply_onepassword_secrets()). The raw + # "op://Vault/Item/field" string would otherwise win and every + # provider auth attempt would receive a URL instead of a key. This + # happens during a partial migration, or when the user wrote op:// + # references straight into .env rather than the secrets.onepassword + # config block. For every non-op:// value the original + # .env-takes-precedence behaviour is preserved unchanged. + if raw.startswith("op://") and env_val: + return env_val + return raw or _get_secret(key, "") or env_val # Honour user suppression — `hermes auth remove ` for an # env-seeded credential marks the env: source as suppressed so it diff --git a/hermes_cli/env_loader.py b/hermes_cli/env_loader.py index 4352e4bdf9e..3ea53f7aaa7 100644 --- a/hermes_cli/env_loader.py +++ b/hermes_cli/env_loader.py @@ -246,6 +246,20 @@ def load_hermes_dotenv( _load_dotenv_with_fallback(user_env, override=True) loaded.append(user_env) + # Load .op.env AFTER .env so that .env values win, but the bootstrap + # token (OP_SERVICE_ACCOUNT_TOKEN) becomes available for + # apply_onepassword_secrets() even in cron / subprocess environments + # that inherit no shell state (no systemd EnvironmentFile, no op run). + # .op.env is gitignored — the service-account token never enters the + # committed .env file. + # Users on systemd can alternatively use: + # EnvironmentFile=-/path/to/.hermes/.op.env + # in their gateway unit, which takes precedence (override=False below + # ensures .op.env never clobbers a token already in the environment). + op_env = home_path / ".op.env" + if op_env.exists() and not os.environ.get("OP_SERVICE_ACCOUNT_TOKEN"): + _load_dotenv_with_fallback(op_env, override=False) + if project_env_path and project_env_path.exists(): _load_dotenv_with_fallback(project_env_path, override=not loaded) loaded.append(project_env_path) diff --git a/tests/test_env_loader_op_bootstrap.py b/tests/test_env_loader_op_bootstrap.py new file mode 100644 index 00000000000..feca229337e --- /dev/null +++ b/tests/test_env_loader_op_bootstrap.py @@ -0,0 +1,165 @@ +"""Tests for the 1Password bootstrap-token reliability patches. + +Two behaviours are covered: + +1. ``load_hermes_dotenv()`` auto-loads ``~/.hermes/.op.env`` so the + ``OP_SERVICE_ACCOUNT_TOKEN`` bootstrap token is available to + ``apply_onepassword_secrets()`` in cron / subprocess / macOS / Docker + contexts that inherit no shell state (no systemd EnvironmentFile, no + ``op run``). ``.op.env`` must never override a token already present + in the environment (e.g. injected by a systemd ``EnvironmentFile``). + +2. ``credential_pool._seed_from_env`` (via the inner + ``_get_env_prefer_dotenv``) must prefer an already-resolved value from + ``os.environ`` over a raw ``op://`` reference still sitting in ``.env``, + while leaving the normal ``.env``-takes-precedence behaviour untouched + for every non-``op://`` value. + +These stay fully hermetic — the real ``op`` binary is never invoked and no +1Password integration is enabled. +""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path +from unittest import mock + +import pytest + +# Make the worktree importable without depending on the installed wheel. +ROOT = Path(__file__).resolve().parents[1] +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + +from hermes_cli import env_loader # noqa: E402 +import agent.credential_pool as credential_pool # noqa: E402 + + +@pytest.fixture(autouse=True) +def _isolate_op_token(monkeypatch): + """Each test starts with OP_SERVICE_ACCOUNT_TOKEN unset and a clean cache.""" + monkeypatch.delenv("OP_SERVICE_ACCOUNT_TOKEN", raising=False) + env_loader.reset_secret_source_cache() + yield + env_loader.reset_secret_source_cache() + + +# --------------------------------------------------------------------------- +# Patch 1 — .op.env bootstrap-token auto-load +# --------------------------------------------------------------------------- + + +def test_op_env_autoloads_bootstrap_token_in_cron_context(tmp_path, monkeypatch): + """A fresh interpreter (no inherited shell state) picks up the token.""" + home = tmp_path / ".hermes" + home.mkdir() + # .env carries user secrets / op:// references but NOT the bootstrap token. + (home / ".env").write_text("FOO=bar\n", encoding="utf-8") + # The gitignored .op.env holds only the service-account token. + (home / ".op.env").write_text( + "OP_SERVICE_ACCOUNT_TOKEN=test-token\n", encoding="utf-8" + ) + + assert os.environ.get("OP_SERVICE_ACCOUNT_TOKEN") is None + + env_loader.load_hermes_dotenv(hermes_home=home) + + assert os.environ["OP_SERVICE_ACCOUNT_TOKEN"] == "test-token" + + +def test_op_env_does_not_override_existing_token(tmp_path, monkeypatch): + """A token already in the environment (e.g. systemd EnvironmentFile) wins.""" + home = tmp_path / ".hermes" + home.mkdir() + (home / ".env").write_text("FOO=bar\n", encoding="utf-8") + (home / ".op.env").write_text( + "OP_SERVICE_ACCOUNT_TOKEN=test-token\n", encoding="utf-8" + ) + + monkeypatch.setenv("OP_SERVICE_ACCOUNT_TOKEN", "live-token") + + env_loader.load_hermes_dotenv(hermes_home=home) + + # override=False AND the explicit guard both protect the live token. + assert os.environ["OP_SERVICE_ACCOUNT_TOKEN"] == "live-token" + + +def test_missing_op_env_is_a_noop(tmp_path): + """No .op.env present must not raise and must not invent a token.""" + home = tmp_path / ".hermes" + home.mkdir() + (home / ".env").write_text("FOO=bar\n", encoding="utf-8") + + env_loader.load_hermes_dotenv(hermes_home=home) + + assert os.environ.get("OP_SERVICE_ACCOUNT_TOKEN") is None + + +# --------------------------------------------------------------------------- +# Patch 2 — credential_pool prefers resolved value over raw op:// ref +# --------------------------------------------------------------------------- + + +def _seed_openrouter_token(monkeypatch, dotenv_value, environ_value): + """Drive _seed_from_env('openrouter') and return the seeded access_token. + + _get_env_prefer_dotenv is a closure inside _seed_from_env, so we exercise + it through the openrouter seeding path, which calls + _get_env_prefer_dotenv('OPENROUTER_API_KEY') and stores the result as the + pooled credential's access_token. + """ + monkeypatch.setattr( + credential_pool, + "load_env", + lambda: {"OPENROUTER_API_KEY": dotenv_value}, + ) + if environ_value is None: + monkeypatch.delenv("OPENROUTER_API_KEY", raising=False) + else: + monkeypatch.setenv("OPENROUTER_API_KEY", environ_value) + # Never treat the synthetic source as suppressed. + monkeypatch.setattr( + "hermes_cli.auth.is_source_suppressed", lambda _p, _s: False + ) + + entries: list = [] + changed, sources = credential_pool._seed_from_env("openrouter", entries) + assert changed and entries, "expected a seeded openrouter credential" + return entries[0].access_token + + +def test_credential_pool_prefers_resolved_env_over_raw_op_ref(monkeypatch): + """A raw op:// reference in .env must lose to the resolved os.environ value.""" + token = _seed_openrouter_token( + monkeypatch, + dotenv_value="op://Vault/Item/field", + environ_value="resolved-value", + ) + assert token == "resolved-value" + + +def test_credential_pool_still_prefers_dotenv_for_non_op_values(monkeypatch): + """Regression guard: .env still beats os.environ for ordinary values.""" + token = _seed_openrouter_token( + monkeypatch, + dotenv_value="dotenv-value", + environ_value="shell-value", + ) + assert token == "dotenv-value" + + +def test_credential_pool_falls_back_to_env_when_dotenv_is_only_op_ref(monkeypatch): + """An unresolved op:// in .env with no resolved env value yields the raw ref. + + This is the pre-resolution / misconfigured edge: there is nothing better + to return, so behaviour is unchanged (the raw reference is surfaced rather + than silently dropping the credential). + """ + token = _seed_openrouter_token( + monkeypatch, + dotenv_value="op://Vault/Item/field", + environ_value=None, + ) + assert token == "op://Vault/Item/field" diff --git a/website/docs/user-guide/secrets/onepassword.md b/website/docs/user-guide/secrets/onepassword.md index 1f20668883a..203ca3ec1c1 100644 --- a/website/docs/user-guide/secrets/onepassword.md +++ b/website/docs/user-guide/secrets/onepassword.md @@ -18,6 +18,32 @@ Hermes never authenticates on your behalf and never downloads `op`: it shells ou - **Service accounts** (recommended for servers/CI): create a service account in 1Password, grant it read access to the relevant vault, and export its token as `OP_SERVICE_ACCOUNT_TOKEN` in `~/.hermes/.env`. The token is the credential — treat it like any other bearer token. - **Desktop / interactive sessions** (laptops): run `op signin` (or enable CLI integration in the 1Password app). Hermes passes your `OP_SESSION_*` variables through to the `op` child process. The 1Password cache key includes those session variables, so signing into a different account never serves a value cached under the previous identity. +## Bootstrap token + +When you authenticate with a **service-account token**, that token is itself the bootstrap credential Hermes needs *before* it can resolve any `op://` reference. It must be present in `os.environ` of every process that resolves secrets — including cron jobs (`kanban.dispatch_in_gateway: false`), subprocess invocations, CLI runs, macOS launchd agents, and Docker containers — not just the interactive gateway. There are three ways to make it available, in order of precedence: + +1. **In `~/.hermes/.env` (recommended).** `hermes secrets onepassword setup --token ` writes the token to `~/.hermes/.env`, exactly like Bitwarden's `BWS_ACCESS_TOKEN`. Because `load_hermes_dotenv()` always loads `.env`, the token is available everywhere with zero extra setup. This is the simplest reliable option. + +2. **In `~/.hermes/.op.env` (gitignored).** If you'd rather keep the service-account token out of `.env` — for example so `.env` can be checked into a private dotfiles repo while the token stays out of version control — place it in `~/.hermes/.op.env`: + + ```bash + echo 'OP_SERVICE_ACCOUNT_TOKEN=ops_...' > ~/.hermes/.op.env + chmod 600 ~/.hermes/.op.env + ``` + + Hermes auto-loads `.op.env` at startup, **after** `.env`, and **never** overrides a token already present in the environment. `.op.env` is gitignored so the token never enters a committed file. + +3. **Via systemd `EnvironmentFile` (Linux gateway).** If you run the gateway under systemd, you can inject the token directly into the service environment: + + ```ini + [Service] + EnvironmentFile=-/home/youruser/.hermes/.op.env + ``` + + A token injected this way takes precedence — Hermes detects that `OP_SERVICE_ACCOUNT_TOKEN` is already set and skips loading `.op.env` entirely. + +If the token is reachable only through an interactive shell (`op signin`, `OP_SESSION_*` exports in `.bashrc`, etc.), it will **not** be inherited by cron jobs or freshly spawned subprocesses, and those contexts will log a warning and fall back to whatever credentials `.env` already held. Use one of the three options above for any non-interactive workload. + ## Setup ### 1. Install and sign in to `op`