mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(secrets): make 1Password bootstrap token reliable outside systemd
The 1Password secret source resolves op:// references using OP_SERVICE_ACCOUNT_TOKEN read from os.environ. Under systemd the gateway gets that token via EnvironmentFile, but cron jobs, subprocesses, CLI runs, macOS launchd, and Docker containers spawn fresh interpreters with no inherited shell state — so they silently failed to resolve any reference and fell back to empty strings. Two patches close the gap, matching Bitwarden's reliability guarantees: 1. env_loader: auto-load ~/.hermes/.op.env after .env so the gitignored bootstrap token is available everywhere. override=False plus an explicit guard ensure it never clobbers a token already in env (e.g. from a systemd EnvironmentFile, which keeps precedence). 2. credential_pool: _get_env_prefer_dotenv() now prefers the resolved value in os.environ when .env still holds a raw op:// reference, instead of handing a URL to provider auth. Non-op:// values keep the existing .env-takes-precedence behaviour. Also gitignore .op.env, document the three bootstrap-token options, and add tests covering auto-load, no-override, and the resolved-vs-raw precedence (plus regression guards). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
2dc4286e00
commit
8a76de962f
5 changed files with 220 additions and 2 deletions
1
.gitignore
vendored
1
.gitignore
vendored
|
|
@ -8,6 +8,7 @@ __pycache__/
|
|||
.venv
|
||||
.vscode/
|
||||
.env
|
||||
.op.env
|
||||
.env.local
|
||||
.env.development.local
|
||||
.env.test.local
|
||||
|
|
|
|||
|
|
@ -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 <provider> <N>` for an
|
||||
# env-seeded credential marks the env:<VAR> source as suppressed so it
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
165
tests/test_env_loader_op_bootstrap.py
Normal file
165
tests/test_env_loader_op_bootstrap.py
Normal file
|
|
@ -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"
|
||||
|
|
@ -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 <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`
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue