feat(egress): CA rotation — follow-up to #35149 - #5
Conversation
Add `hermes egress rotate-ca` plus a `ca-rotation` doctor check, closing PR NousResearch#30179's scope cut: "The CA is a 10-year self-signed cert. Rotation is manual for now." - rotate_ca(): archive old ca.crt -> ca-archive/ (keep 5), mint fresh CA via ensure_ca_cert(force=True), restart daemon if running, append a structured entry to rotation-history.jsonl. - CLI flags: --dry-run, --reason, --no-restart, --force. - New additive doctor check 'ca-rotation' (annual-hygiene warn tier); does not mutate NousResearch#35149's 'ca' expiry check. - Fingerprints redacted first-8/last-8 in error paths (_redact_fingerprint). - 10 new hermetic tests; full iron-proxy suite stays green (155 -> 165).
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Daemon restart after rotation loses bitwarden secrets
- CA rotation now carries Bitwarden refresh options through CLI validation and daemon restart so restarted proxies fetch Bitwarden-backed secrets.
- ✅ Fixed: Duplicated date-parsing logic in
_ca_not_beforeand_ca_not_after- The duplicated openssl certificate date parsing was consolidated into a shared helper used by both notBefore and notAfter readers.
Or push these changes by commenting:
@cursor push db1a23a751
Preview (db1a23a751)
diff --git a/agent/proxy_sources/iron_proxy.py b/agent/proxy_sources/iron_proxy.py
--- a/agent/proxy_sources/iron_proxy.py
+++ b/agent/proxy_sources/iron_proxy.py
@@ -981,6 +981,8 @@
reason: Optional[str] = None,
restart: bool = True,
operator: Optional[str] = None,
+ refresh_secrets_from_bitwarden: bool = False,
+ bitwarden_config: Optional[Dict] = None,
) -> RotationPlan:
"""Generate a fresh CA, archive the old one, and (optionally) restart.
@@ -1040,7 +1042,10 @@
# 3. Restart the daemon if it was running and the caller wants it.
if restart and plan.daemon_running:
stop_proxy()
- start_proxy()
+ start_proxy(
+ refresh_secrets_from_bitwarden=refresh_secrets_from_bitwarden,
+ bitwarden_config=bitwarden_config,
+ )
# 4. Append the audit record.
record = {
@@ -2503,24 +2508,20 @@
return "****" + value[-4:]
-def _ca_not_after(ca_crt: Path) -> Optional[datetime]:
- """Return the CA cert's notAfter as an aware UTC datetime, or None.
-
- Uses ``openssl x509 -enddate`` (already a hard dep for CA generation)
- so we don't pull in ``cryptography`` just to read one field.
- """
+def _ca_x509_date(ca_crt: Path, flag: str) -> Optional[datetime]:
+ """Return an openssl x509 date field as an aware UTC datetime, or None."""
if shutil.which("openssl") is None:
return None
try:
res = subprocess.run( # noqa: S603 -- openssl trusted PATH lookup
- ["openssl", "x509", "-enddate", "-noout", "-in", str(ca_crt)],
+ ["openssl", "x509", flag, "-noout", "-in", str(ca_crt)],
capture_output=True, text=True, timeout=10,
)
except (OSError, subprocess.TimeoutExpired):
return None
if res.returncode != 0:
return None
- # Output: ``notAfter=Jun 1 12:00:00 2035 GMT``
+ # Output: ``notAfter=...`` or ``notBefore=...``.
line = res.stdout.strip()
if "=" not in line:
return None
@@ -2534,6 +2535,15 @@
return None
+def _ca_not_after(ca_crt: Path) -> Optional[datetime]:
+ """Return the CA cert's notAfter as an aware UTC datetime, or None.
+
+ Uses ``openssl x509 -enddate`` (already a hard dep for CA generation)
+ so we don't pull in ``cryptography`` just to read one field.
+ """
+ return _ca_x509_date(ca_crt, "-enddate")
+
+
def _ca_not_before(ca_crt: Path) -> Optional[datetime]:
"""Return the CA cert's notBefore as an aware UTC datetime, or None.
@@ -2541,29 +2551,7 @@
``rotation-history.jsonl`` is absent (a CA generated by ``setup`` but
never explicitly rotated).
"""
- if shutil.which("openssl") is None:
- return None
- try:
- res = subprocess.run( # noqa: S603 -- openssl trusted PATH lookup
- ["openssl", "x509", "-startdate", "-noout", "-in", str(ca_crt)],
- capture_output=True, text=True, timeout=10,
- )
- except (OSError, subprocess.TimeoutExpired):
- return None
- if res.returncode != 0:
- return None
- # Output: ``notBefore=Jun 1 12:00:00 2025 GMT``
- line = res.stdout.strip()
- if "=" not in line:
- return None
- raw = line.split("=", 1)[1].strip()
- for fmt in ("%b %d %H:%M:%S %Y %Z", "%b %d %H:%M:%S %Y"):
- try:
- dt = datetime.strptime(raw, fmt)
- return dt.replace(tzinfo=timezone.utc)
- except ValueError:
- continue
- return None
+ return _ca_x509_date(ca_crt, "-startdate")
def _check_ca_rotation(ca_crt: Path) -> DoctorCheck:
diff --git a/hermes_cli/proxy_cli.py b/hermes_cli/proxy_cli.py
--- a/hermes_cli/proxy_cli.py
+++ b/hermes_cli/proxy_cli.py
@@ -19,7 +19,7 @@
import argparse
import os
from pathlib import Path
-from typing import List
+from typing import Dict, List, Optional, Tuple
from rich.console import Console
from rich.panel import Panel
@@ -203,6 +203,64 @@
# ---------------------------------------------------------------------------
+def _bitwarden_start_options(cfg: Dict) -> Tuple[bool, Optional[Dict]]:
+ proxy_cfg = cfg.get("proxy") or {}
+ credential_source = proxy_cfg.get("credential_source", "env")
+ bw_cfg = (cfg.get("secrets") or {}).get("bitwarden")
+ refresh_bw = (
+ credential_source == "bitwarden"
+ and bw_cfg is not None
+ and bool(bw_cfg.get("enabled"))
+ )
+ # Pass the proxy-side allow_env_fallback opt-in through to
+ # start_proxy. This is a deliberate, documented escape hatch: when
+ # set, the daemon silently falls back to host env if BWS is
+ # unreachable, instead of raising. Default is strict (raise).
+ if refresh_bw and bw_cfg is not None:
+ bw_cfg = dict(bw_cfg)
+ bw_cfg["allow_env_fallback"] = bool(
+ proxy_cfg.get("allow_env_fallback", False)
+ )
+ return refresh_bw, bw_cfg
+
+
+def _bitwarden_start_ready(
+ *, refresh_bw: bool, bw_cfg: Optional[Dict], console: Console,
+) -> bool:
+ # stephenschoettler #1: when `credential_source: bitwarden`, the
+ # operator picked BWS specifically to get the rotation guarantee —
+ # silently falling back to parent-env at start_proxy time reintroduces
+ # exactly the bug class the BW mode is supposed to defeat (host env
+ # is stale / mismatched). Pre-check at the wizard layer so we fail
+ # loud with actionable error messages BEFORE start_proxy degrades.
+ if not refresh_bw:
+ return True
+ bw_access_env = (bw_cfg or {}).get("access_token_env", "BWS_ACCESS_TOKEN")
+ if not os.environ.get(bw_access_env, "").strip():
+ console.print(
+ f"[red]✗ Refusing to start: credential_source=bitwarden but "
+ f"{bw_access_env} is not set in the environment.[/red]"
+ )
+ console.print(
+ " Either export the access token, or run "
+ "`hermes egress setup --no-bitwarden` to switch back to "
+ "env-based credentials."
+ )
+ return False
+ if not (bw_cfg or {}).get("project_id"):
+ console.print(
+ "[red]✗ Refusing to start: credential_source=bitwarden but "
+ "secrets.bitwarden.project_id is empty.[/red]"
+ )
+ console.print(
+ " Run `hermes secrets bitwarden setup` to configure the "
+ "project, or switch back via `hermes egress setup "
+ "--no-bitwarden`."
+ )
+ return False
+ return True
+
+
def cmd_install(args: argparse.Namespace) -> int:
console = Console()
try:
@@ -567,22 +625,7 @@
# rotation guarantee that distinguishes ``credential_source:
# bitwarden`` from ``credential_source: env``. Without it, rotating
# a key in the Bitwarden web app doesn't reach the proxy.
- credential_source = proxy_cfg.get("credential_source", "env")
- bw_cfg = (cfg.get("secrets") or {}).get("bitwarden")
- refresh_bw = (
- credential_source == "bitwarden"
- and bw_cfg is not None
- and bool(bw_cfg.get("enabled"))
- )
- # Pass the proxy-side allow_env_fallback opt-in through to
- # start_proxy. This is a deliberate, documented escape hatch: when
- # set, the daemon silently falls back to host env if BWS is
- # unreachable, instead of raising. Default is strict (raise).
- if refresh_bw and bw_cfg is not None:
- bw_cfg = dict(bw_cfg)
- bw_cfg["allow_env_fallback"] = bool(
- proxy_cfg.get("allow_env_fallback", False)
- )
+ refresh_bw, bw_cfg = _bitwarden_start_options(cfg)
# fail_on_uncovered_providers: when true, refuse to start if any
# LLM-specific non-bearer providers (Anthropic native, Azure OpenAI,
@@ -610,36 +653,10 @@
)
return 1
- # stephenschoettler #1: when `credential_source: bitwarden`, the
- # operator picked BWS specifically to get the rotation guarantee —
- # silently falling back to parent-env at start_proxy time reintroduces
- # exactly the bug class the BW mode is supposed to defeat (host env
- # is stale / mismatched). Pre-check at the wizard layer so we fail
- # loud with actionable error messages BEFORE start_proxy degrades.
- if refresh_bw:
- bw_access_env = (bw_cfg or {}).get("access_token_env", "BWS_ACCESS_TOKEN")
- if not os.environ.get(bw_access_env, "").strip():
- console.print(
- f"[red]✗ Refusing to start: credential_source=bitwarden but "
- f"{bw_access_env} is not set in the environment.[/red]"
- )
- console.print(
- " Either export the access token, or run "
- "`hermes egress setup --no-bitwarden` to switch back to "
- "env-based credentials."
- )
- return 1
- if not (bw_cfg or {}).get("project_id"):
- console.print(
- "[red]✗ Refusing to start: credential_source=bitwarden but "
- "secrets.bitwarden.project_id is empty.[/red]"
- )
- console.print(
- " Run `hermes secrets bitwarden setup` to configure the "
- "project, or switch back via `hermes egress setup "
- "--no-bitwarden`."
- )
- return 1
+ if not _bitwarden_start_ready(
+ refresh_bw=refresh_bw, bw_cfg=bw_cfg, console=console,
+ ):
+ return 1
try:
status = ip.start_proxy(
@@ -847,9 +864,19 @@
# ------------------------------------------------------------- do it
restart = not getattr(args, "no_restart", False)
+ cfg = load_config()
+ refresh_bw, bw_cfg = _bitwarden_start_options(cfg)
+ if restart and plan.daemon_running and not _bitwarden_start_ready(
+ refresh_bw=refresh_bw, bw_cfg=bw_cfg, console=console,
+ ):
+ return 1
try:
- result = ip.rotate_ca(reason=getattr(args, "reason", None),
- restart=restart)
+ result = ip.rotate_ca(
+ reason=getattr(args, "reason", None),
+ restart=restart,
+ refresh_secrets_from_bitwarden=refresh_bw,
+ bitwarden_config=bw_cfg,
+ )
except Exception as exc: # noqa: BLE001 -- user-facing error funnel
console.print(f"[red]✗ CA rotation failed:[/red] {exc}")
return 1
diff --git a/tests/test_iron_proxy_cli.py b/tests/test_iron_proxy_cli.py
--- a/tests/test_iron_proxy_cli.py
+++ b/tests/test_iron_proxy_cli.py
@@ -304,6 +304,77 @@
assert captured.get("refresh_secrets_from_bitwarden") is False
+def test_cmd_rotate_ca_passes_bitwarden_refresh_when_restarting(
+ hermes_home, monkeypatch,
+):
+ from hermes_cli.config import load_config, save_config
+ cfg = load_config()
+ cfg.setdefault("proxy", {})["credential_source"] = "bitwarden"
+ cfg.setdefault("secrets", {})["bitwarden"] = {
+ "enabled": True,
+ "project_id": "test-proj-id",
+ "access_token_env": "BWS_ACCESS_TOKEN",
+ }
+ save_config(cfg)
+ monkeypatch.setenv("BWS_ACCESS_TOKEN", "bwsk-test-access-token")
+
+ plan = ip.RotationPlan(
+ ca_crt=hermes_home / "proxy" / "ca.crt",
+ ca_key=hermes_home / "proxy" / "ca.key",
+ archive_path=hermes_home / "proxy" / "ca-archive" / "ca-test.crt",
+ old_fingerprint="deadbeef" * 8,
+ daemon_running=True,
+ sandboxes=[],
+ )
+ captured: dict = {}
+
+ def fake_rotate_ca(**kw):
+ captured.update(kw)
+ plan.new_fingerprint = "feedface" * 8
+ plan.new_valid_until = "2035-06-01T12:00:00+00:00"
+ return plan
+
+ monkeypatch.setattr(ip, "plan_ca_rotation", lambda: plan)
+ monkeypatch.setattr(ip, "rotate_ca", fake_rotate_ca)
+
+ rc = proxy_cli.cmd_rotate_ca(_args())
+ assert rc == 0
+ assert captured.get("refresh_secrets_from_bitwarden") is True
+ assert captured.get("bitwarden_config") is not None
+
+
+def test_cmd_rotate_ca_refuses_when_bitwarden_token_missing(
+ hermes_home, monkeypatch,
+):
+ from hermes_cli.config import load_config, save_config
+ cfg = load_config()
+ cfg.setdefault("proxy", {})["credential_source"] = "bitwarden"
+ cfg.setdefault("secrets", {})["bitwarden"] = {
+ "enabled": True,
+ "project_id": "test-proj-id",
+ "access_token_env": "BWS_ACCESS_TOKEN",
+ }
+ save_config(cfg)
+ monkeypatch.delenv("BWS_ACCESS_TOKEN", raising=False)
+
+ plan = ip.RotationPlan(
+ ca_crt=hermes_home / "proxy" / "ca.crt",
+ ca_key=hermes_home / "proxy" / "ca.key",
+ archive_path=hermes_home / "proxy" / "ca-archive" / "ca-test.crt",
+ daemon_running=True,
+ sandboxes=[],
+ )
+ monkeypatch.setattr(ip, "plan_ca_rotation", lambda: plan)
+
+ def must_not_rotate(**kw):
+ pytest.fail("rotate_ca should not be invoked when BWS token missing")
+
+ monkeypatch.setattr(ip, "rotate_ca", must_not_rotate)
+
+ rc = proxy_cli.cmd_rotate_ca(_args())
+ assert rc == 1
+
+
# ---------------------------------------------------------------------------
# cmd_stop, cmd_status, cmd_disable, cmd_config
# ---------------------------------------------------------------------------
diff --git a/tests/test_iron_proxy_rotate_ca.py b/tests/test_iron_proxy_rotate_ca.py
--- a/tests/test_iron_proxy_rotate_ca.py
+++ b/tests/test_iron_proxy_rotate_ca.py
@@ -105,6 +105,38 @@
assert calls == ["stop", "start"]
+def test_rotate_running_daemon_preserves_bitwarden_start_options(
+ hermes_home, monkeypatch,
+):
+ """rotate-ca restarts the daemon with the same Bitwarden refresh options."""
+ _write_ca(hermes_home)
+ monkeypatch.setattr(ip, "_proxy_is_running", lambda: True)
+ monkeypatch.setattr(ip, "list_hermes_sandboxes", lambda: [])
+ calls = []
+ captured = {}
+ monkeypatch.setattr(ip, "stop_proxy", lambda: calls.append("stop"))
+
+ def fake_start_proxy(**kw):
+ calls.append("start")
+ captured.update(kw)
+
+ monkeypatch.setattr(ip, "start_proxy", fake_start_proxy)
+ bw_cfg = {"enabled": True, "project_id": "test-proj-id"}
+
+ ip.rotate_ca(
+ reason="test",
+ restart=True,
+ refresh_secrets_from_bitwarden=True,
+ bitwarden_config=bw_cfg,
+ )
+
+ assert calls == ["stop", "start"]
+ assert captured == {
+ "refresh_secrets_from_bitwarden": True,
+ "bitwarden_config": bw_cfg,
+ }
+
+
def test_dry_run_plan_does_not_touch_filesystem(hermes_home, monkeypatch):
"""plan_ca_rotation (the dry-run engine) must not change any mtimes."""
crt = _write_ca(hermes_home)You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 3650b23. Configure here.
| # 3. Restart the daemon if it was running and the caller wants it. | ||
| if restart and plan.daemon_running: | ||
| stop_proxy() | ||
| start_proxy() |
There was a problem hiding this comment.
Daemon restart after rotation loses bitwarden secrets
Medium Severity
rotate_ca calls start_proxy() with no arguments when restarting the daemon. Since start_proxy defaults refresh_secrets_from_bitwarden=False and bitwarden_config=None, operators using credential_source: bitwarden will get a proxy that starts without fetching upstream API keys from bitwarden. The proxy will be running but unable to perform credential injection, silently degrading functionality until the operator manually restarts with the proper bitwarden flags.
Reviewed by Cursor Bugbot for commit 3650b23. Configure here.
| return dt.replace(tzinfo=timezone.utc) | ||
| except ValueError: | ||
| continue | ||
| return None |
There was a problem hiding this comment.
Duplicated date-parsing logic in _ca_not_before and _ca_not_after
Low Severity
_ca_not_before is a near-exact copy of the existing _ca_not_after function, differing only in the openssl flag (-startdate vs -enddate) and one comment line. Both share identical openssl-availability checks, subprocess invocation patterns, output parsing, and date-format iteration. A single parameterized helper accepting the flag/field name would eliminate ~25 lines of duplicated logic.
Reviewed by Cursor Bugbot for commit 3650b23. Configure here.
|
Superseded by upstream PR NousResearch#35188 — retargeted to NousResearch/hermes-agent feat/iron-proxy branch with Bugbot fixes baked in. Companion ca-rotation doctor check deferred to follow-up since DoctorCheck lives on the doctor+audit stacked branch. |
…review #5) The fence-cancel poll loops (sync host wait in conversation_compression, async hygiene wait in gateway/run) spun at 1kHz while the worker held the fence through its lock-setup window — which rides SessionDB write patience and can last seconds. 25ms keeps sub-tick cancel latency without the spin.



Why this complements NousResearch#30179 and NousResearch#35149
PR NousResearch#30179 shipped the iron-proxy credential-injection firewall and explicitly cut one corner in its Scope cuts (v1) section:
PR NousResearch#35149 then layered a
doctor+audit+ Anthropic-native pass on top, including acadoctor check that fails on expiry. This PR closes the remaining gap: a first-class, audited rotation workflow so operators never have to hand-runopenssl genrsa(and so an annual-hygiene reminder exists). It is additive — it does not touch the behaviour of either prior PR.Stack: NousResearch#30179 → NousResearch#35149 → this PR (base:
feat/iron-proxy-doctor-audit-anthropic).What lands
hermes egress rotate-ca— archive the live CA, mint a fresh one (via the same atomicensure_ca_cert(force=True)path), restart the daemon if running, and record a structured audit entry.ca-rotationdoctor check — additive entry inDOCTOR_CHECK_NAMES; an annual-hygienewarntier distinct from feat(egress): doctor + audit + Anthropic native — follow-up to #30179 NousResearch/hermes-agent#35149'scaexpiry check. Readsrotation-history.jsonl, falls back to the cert'snotBeforewhen history is absent.rotate-cacommand in the CLI reference.tests/test_iron_proxy_doctor.py.Rotation steps, in order (so a crash never leaves a missing live CA):
ca.crt→~/.hermes/proxy/ca-archive/ca-<YYYYMMDD-HHMMSS>.crt, then prune to the 5 most recent.0o600key write,os.replace).--no-restart:stop_proxy()→start_proxy().rotation-history.jsonl:{ts, old_fingerprint_sha256, new_fingerprint_sha256, reason, operator, subject, valid_until}.New surfaces
hermes egress rotate-ca--dry-run,--reason TEXT,--no-restart,--forceca-rotationpass<365d,warn365d–~9y; expiry staysca's jobrotate_ca()/plan_ca_rotation()__all__RotationPlanlast_rotation_entry()/list_hermes_sandboxes()~/.hermes/proxy/rotation-history.jsonl0o644)~/.hermes/proxy/ca-archive/0o700, last-5 retentionFailure modes considered
list_hermes_sandboxes()returns[], never raises_redact_fingerprint; rotation still completes (recordsnullfingerprint)os.replace)rotate-calists them and prompts by default (bypass with--force); docs + console output tell the operator to restart them--dry-runplan_ca_rotation()is read-only; verified by stat-before/after in tests--no-restartrotation-history.jsonllinelast_rotation_entry()scans from the end, skips unparseable linesValidation
tests/test_iron_proxy_rotate_ca.py)New tests: writes/archives with no daemon; restart order (
stop→start); dry-run no-mtime-change; reason recorded with valid 64-hex fingerprints (old≠new); archive pruning (7→5);--no-restartleaves daemon alone; doctor pass/warn/notBefore-fallback; fingerprint redaction.Manual end-to-end smoke (isolated
HERMES_HOME):--dry-runchanged nothing; realrotate-ca --reason "annual rotation"archived the old cert, minted a 10-year CA, wrote a parseable history line, and theca-rotationdoctor check returnedpass.Coverage gaps
verify-trustsubcommand (verifying live sandboxes still trust the new CA after restart). Deliberately deferred — documented as a known follow-up in both the PR and the user guide.rotate_caengine andlist_hermes_sandboxesdiscovery are).Ambiguity flags
cavsca-rotationoverlap on hard expiry. Chose option (b) from the brief — a separate additive check — and explicitly madeca-rotationnot duplicate the expiry failure (that stays thecacheck's job;ca-rotationonly adds an age/hygienewarn). This keeps feat(egress): doctor + audit + Anthropic native — follow-up to #30179 NousResearch/hermes-agent#35149's testedcabehaviour untouched.rotation-history.jsonlis missing. Fell back to the cert'snotBefore(when was this CA minted?) rather than treating "no history" as a warn — a freshly-setupCA shouldn't nag on day one.subjectfield shape.openssl x509 -subjectemits e.g.subject=CN=hermes iron-proxy CA; recorded verbatim rather than parsing out a bare CN, to stay faithful to the source and avoid a brittle parser.This PR was opened by Bartok at Daniel's request, in response to Teknium's invitation to Catalin for a security review of NousResearch#30179.
Note
Medium Risk
Rotation replaces the TLS-intercept CA and signing key; mis-timed restarts or unrestarted sandboxes can break trust until containers are recreated, though ordering (archive before mint) limits leaving the host without a live CA.
Overview
Adds a first-class iron-proxy CA rotation workflow on top of existing
ensure_ca_cert(force=True), replacing the prior manual-only rotation story.hermes egress rotate-caarchives the liveca.crtunder~/.hermes/proxy/ca-archive/(keeps five), mints a new CA/key atomically, optionally restarts the managed daemon (stop_proxy→start_proxy), and appends audit lines torotation-history.jsonl.plan_ca_rotation/RotationPlanshare logic with--dry-run; the CLI lists labeledhermes.sandbox=truecontainers, prompts before rotate unless--force, and supports--reason,--no-restart.Doctor gains
ca-rotation: pass if last rotation (or certnotBeforewhen no history) is under 365 days, warn otherwise—separate from the existingcaexpiry check. New helpers cover fingerprints (with redaction), sandbox discovery, and history parsing;__all__exports the public API.Docs describe the command and flags; 10 hermetic tests cover archive/restart ordering, dry-run immutability, history, pruning, and doctor tiers.
Reviewed by Cursor Bugbot for commit 3650b23. Bugbot is set up for automated code reviews on this repo. Configure here.