refactor(agent/creds): compact secret_scope and command_token_source

secret_scope: _environ_or helper replaces four os.environ.get/default blocks;
_is_global_env uses tuple startswith; prose essays reduced to the invariants
(resolution order, fail-closed rule, cron overlay rationale, BOM handling).
command_token_source: rationale prose compacted; expiry arithmetic flattened.
This commit is contained in:
Teknium
2026-09-02 10:07:22 -07:00
parent 9989dd4f48
commit afcad8335a
2 changed files with 94 additions and 194 deletions

View File

@@ -1,12 +1,9 @@
"""Mint a provider API key by running a command (``key_cmd``).
Static API keys are the exception at enterprise gateways: SSO/OIDC brokers,
cloud IAM, and internal auth proxies all issue SHORT-LIVED bearers instead.
A key copied into ``.env`` (``key_env``) is stale within the hour, so every
request after that 401s and the user has to restart the session.
``key_cmd`` names a command that PRINTS a token, so the credential is derived
rather than stored::
Enterprise gateways (SSO/OIDC brokers, cloud IAM, auth proxies) issue
SHORT-LIVED bearers; a key copied into ``.env`` goes stale within the hour.
``key_cmd`` names a command that PRINTS a token (the ``apiKeyHelper`` /
``gcloud auth print-access-token`` / ``databricks auth token`` idiom)::
providers:
my-gateway:
@@ -14,22 +11,14 @@ rather than stored::
api_mode: chat_completions
key_cmd: my-auth-cli print-token --profile prod
This is the established pattern for agent tooling — Claude Code's
``apiKeyHelper``, the ``gcloud auth print-access-token`` / ``aws ecr
get-login-password`` idiom, and vendor helpers such as ``databricks auth
token`` all expose exactly this contract. Hermes already accepts a callable
API key on both wire clients (the Entra ID / Azure identity path) and invokes
it per request, so nothing downstream changes: the token is simply always
fresh. It is cached until shortly before expiry, so the command runs about
once per token lifetime rather than once per request.
Both wire clients already accept a callable API key and invoke it per request,
so the token is simply always fresh; it is cached until shortly before expiry.
Output contract: print ONLY the token on stdout, either bare or as JSON with
an ``access_token`` field (``expires_in`` is honoured when present) — the
shape OAuth 2.0 token endpoints and the helpers above already emit.
Output contract: ONLY the token on stdout, bare or as JSON with an
``access_token`` field (``expires_in`` honoured when present).
Precedence: an explicit ``--api-key`` still wins (the one-off recovery escape
hatch); otherwise ``key_cmd`` is preferred over a static ``api_key`` /
``key_env`` on the same entry.
Precedence: an explicit ``--api-key`` still wins (one-off recovery escape
hatch); otherwise ``key_cmd`` beats a static ``api_key`` / ``key_env``.
"""
from __future__ import annotations
@@ -43,19 +32,15 @@ from typing import Callable, Optional
logger = logging.getLogger(__name__)
# Treat a cached token as spent slightly before its stated expiry, so a request
# can't be signed with a token that dies in flight. 60s matches the leeway used
# by comparable OAuth token caches.
# Treat a token as spent slightly before expiry so a request can't be signed
# with one that dies in flight (60s = usual OAuth cache leeway).
_TOKEN_REFRESH_LEEWAY_SECONDS = 60.0
# A token helper reads a local credential cache and should answer in
# milliseconds; anything approaching this budget is hung, not slow.
# Helpers answer from a local cache in milliseconds; this long means hung.
_MINT_TIMEOUT_SECONDS = 15
# When a helper advertises NO expiry, the token cannot be cached for the life
# of the process: nothing in the request path re-mints on 401 (the SDK retries
# 429/5xx only), so an expired no-TTL token would 401 every request until
# restart. Re-mint on a bounded window instead — the helper answers from a
# local credential cache in milliseconds, so a periodic re-run is cheap, and a
# helper that wants a longer cache can simply advertise its real expiry.
# No advertised expiry: nothing in the request path re-mints on 401 (the SDK
# retries 429/5xx only), so a process-lifetime cache would 401 forever once the
# token died. Re-mint on a bounded window instead; helpers wanting a longer
# cache advertise their real expiry.
_NO_TTL_REFRESH_SECONDS = 900.0
@@ -84,12 +69,8 @@ def _mint(command: str, label: str) -> tuple[str, Optional[float]]:
) from exc
if completed.returncode != 0:
# NEVER include stdout/stderr: a partially-successful auth helper can
# print a token or refresh secret there. The command STRING is also
# withheld — a key_cmd can legitimately embed a secret
# (`print-token --client-secret=…`), so echoing it back would leak the
# very credential this module exists to protect. Name the provider so
# the user knows which config entry to run by hand.
# NEVER include stdout/stderr (may hold a token) or the command string
# (may embed `--client-secret=…`); name the provider instead.
raise CommandTokenError(
f"key_cmd for provider {label!r} exited {completed.returncode}. "
f"Run that provider's key_cmd manually to see why "
@@ -101,8 +82,6 @@ def _mint(command: str, label: str) -> tuple[str, Optional[float]]:
raise CommandTokenError(f"key_cmd for provider {label!r} produced no output")
# JSON payload — the shape `databricks auth token --output json` prints.
# Token extraction mirrors databricks/ucode's get_databricks_token:
# json.loads(result.stdout or "{}").get("access_token", "")
if stdout.lstrip().startswith("{"):
try:
payload = json.loads(stdout)
@@ -118,12 +97,9 @@ def _mint(command: str, label: str) -> tuple[str, Optional[float]]:
ttl = payload.get("expires_in")
if isinstance(ttl, (int, float)) and ttl > 0:
return token, float(ttl)
# A relative lifetime is the OAuth 2.0 field, but CLI token helpers
# commonly print an absolute ISO 8601 deadline instead. Treating
# that as "no TTL advertised" caches the token for the life of the
# process, so every request 401s once the deadline passes.
# Imported lazily: hermes_cli.auth imports from agent.* at module
# level, so a top-level import here would risk a cycle.
# CLI helpers often print an absolute ISO 8601 deadline instead of
# OAuth's relative lifetime; honour it or the token 401s once past.
# Lazy import: hermes_cli.auth imports agent.* at module level.
from hermes_cli.auth import _parse_iso_timestamp
for field in ("expiry", "expiresOn"):
@@ -134,12 +110,9 @@ def _mint(command: str, label: str) -> tuple[str, Optional[float]]:
return token, remaining
return token, None
# Bare token. The contract every comparable helper documents is "stdout
# carries the token and nothing else" — extra output would be consumed as
# part of the credential. Strip surrounding whitespace and take the rest
# verbatim; do NOT silently keep one line of several, which converts a
# misconfigured helper (banner, warning, two tokens) into a corrupt-key 401
# that is far harder to diagnose than an explicit refusal.
# Bare token: stdout carries the token and nothing else. Do NOT keep one
# line of several — that turns a misconfigured helper (banner, warning) into
# a corrupt-key 401 far harder to diagnose than an explicit refusal.
token = stdout.strip()
if "\n" in token:
raise CommandTokenError(
@@ -165,12 +138,8 @@ class CommandTokenSource:
return self._token
token, ttl = _mint(self._command, self._label)
self._token = token
self._expires_at = (
time.monotonic() + max(ttl - _TOKEN_REFRESH_LEEWAY_SECONDS, 5.0)
if ttl
# No advertised TTL: bounded cache (see _NO_TTL_REFRESH_SECONDS)
# — there is no 401-driven re-mint hook to fall back on.
else time.monotonic() + _NO_TTL_REFRESH_SECONDS
self._expires_at = time.monotonic() + (
max(ttl - _TOKEN_REFRESH_LEEWAY_SECONDS, 5.0) if ttl else _NO_TTL_REFRESH_SECONDS
)
logger.debug(
"Minted key_cmd token for provider %s (ttl=%s)",

View File

@@ -1,24 +1,22 @@
"""Profile-scoped credential resolution for multi-profile gateway multiplexing.
The multiplexing gateway serves many profiles from one process. Each profile
has its own ``.env`` with its own provider keys and platform tokens, so we
**cannot** union them into the process-global ``os.environ`` (that would leak
profile A's keys to profile B's turns, and to every subprocess spawned with
``env=dict(os.environ)``).
The multiplexing gateway serves many profiles from one process. Each profile
has its own ``.env`` with its own keys, so they **cannot** be unioned into the
process-global ``os.environ`` (profile A's keys would leak into profile B's
turns and into every subprocess spawned with ``env=dict(os.environ)``).
This module provides a fail-closed, context-local secret scope:
This module is a fail-closed, context-local secret scope:
- ``set_secret_scope(mapping)`` installs the active profile's secrets for the
current task (a contextvar, so it propagates into the agent's worker thread
via ``copy_context()`` exactly like the HERMES_HOME override).
- ``get_secret(name)`` reads from that scope. When multiplexing is **active**
and no scope is set, it RAISES rather than silently falling back to
``os.environ`` — an un-migrated or newly-added call site fails loud at that
exact line instead of leaking another profile's value. When multiplexing is
**off** (the default), it transparently reads ``os.environ`` so the
single-profile gateway and every non-gateway caller behave exactly as before.
- ``get_secret(name)`` reads from that scope. When multiplexing is **active**
and no scope is set it RAISES rather than falling back to ``os.environ`` —
an un-migrated call site fails loud at that line instead of leaking another
profile's value. When multiplexing is **off** (default) it reads
``os.environ`` so every non-multiplex caller behaves exactly as before.
Design rationale lives in ``docs/design/multiplexing-gateway.md`` (Workstream A).
Design rationale: ``docs/design/multiplexing-gateway.md`` (Workstream A).
"""
from __future__ import annotations
@@ -29,72 +27,52 @@ from pathlib import Path
from typing import Dict, Mapping, Optional
# ── multiplex-active flag ────────────────────────────────────────────────
# Process-global: set once at gateway startup when gateway.multiplex_profiles
# is true. Governs whether get_secret() fails closed on an unscoped read.
# A plain module global (not a contextvar): it describes the deployment mode,
# not a per-task value.
# Process-global (describes the deployment mode, not a per-task value): set once
# at gateway startup when gateway.multiplex_profiles is true.
_MULTIPLEX_ACTIVE: bool = False
def set_multiplex_active(active: bool) -> None:
"""Mark whether the process is running as a profile multiplexer.
Called once at gateway startup. When True, ``get_secret`` fails closed on
an unscoped read instead of falling back to ``os.environ``.
"""
"""Mark whether the process is a profile multiplexer (get_secret fails closed)."""
global _MULTIPLEX_ACTIVE
_MULTIPLEX_ACTIVE = bool(active)
def is_multiplex_active() -> bool:
"""Return whether the process is running as a profile multiplexer."""
return _MULTIPLEX_ACTIVE
# ── the secret scope contextvar ──────────────────────────────────────────
_SECRET_SCOPE: ContextVar[Optional[Mapping[str, str]]] = ContextVar(
"_SECRET_SCOPE", default=None
)
class UnscopedSecretError(RuntimeError):
"""Raised when a secret is read in multiplex mode with no scope installed.
"""A secret was read in multiplex mode with no scope installed.
This is the fail-closed signal: it means a credential read reached
``get_secret`` without a profile scope active, which in a multiplexer would
otherwise leak whichever profile's value happened to be in ``os.environ``.
The fix is to wrap the call path in ``set_secret_scope(...)`` (the per-turn
/ per-adapter profile scope), not to widen the allowlist.
/ per-adapter profile scope), not to widen the global allowlist.
"""
def set_secret_scope(secrets: Optional[Mapping[str, str]]) -> Token:
"""Install the active profile's secret mapping for the current context.
Returns a token for ``reset_secret_scope``. Pass ``None`` to clear.
"""
"""Install the active profile's secret mapping; ``None`` clears. Returns a reset token."""
return _SECRET_SCOPE.set(secrets)
def reset_secret_scope(token: Token) -> None:
"""Restore the previous secret scope."""
_SECRET_SCOPE.reset(token)
def current_secret_scope() -> Optional[Mapping[str, str]]:
"""Return the active secret mapping, or None when no scope is installed."""
"""The active secret mapping, or None when no scope is installed."""
return _SECRET_SCOPE.get()
# ── genuinely-global env vars (NOT per-profile secrets) ──────────────────
# These are process/deployment-level settings, not profile credentials. They
# legitimately live in os.environ and must keep reading from it even in
# multiplex mode — routing them through the fail-closed path would wrongly
# crash. Anything matching is read from os.environ regardless of scope.
#
# Membership test is by exact name OR prefix (see _is_global_env). Keep this
# list tight: when in doubt a value is a profile secret, not a global.
# Genuinely-global env vars: process/deployment settings, NOT profile secrets.
# They keep reading os.environ even in multiplex mode (routing them through the
# fail-closed path would wrongly crash). Keep this tight — when in doubt a
# value is a profile secret. Membership is exact name OR prefix.
_GLOBAL_ENV_EXACT = frozenset({
# Hermes runtime / deployment
"HERMES_HOME", "HERMES_PROFILE", "HERMES_GATEWAY_LOCK_DIR",
@@ -106,26 +84,16 @@ _GLOBAL_ENV_EXACT = frozenset({
"VIRTUAL_ENV", "PYTHONPATH", "SSL_CERT_FILE",
# Kanban paths (per-board, not per-profile-secret)
"HERMES_KANBAN_DB", "HERMES_KANBAN_WORKSPACES_ROOT", "HERMES_KANBAN_BOARD",
# API-server LISTENER settings — deployment config (Docker compose
# ``environment:`` block, systemd ``Environment=``), not profile secrets.
# The scoped runner reload (#64674) must keep seeing them or container
# deployments silently lose the api_server platform (#69379). NOTE:
# API_SERVER_KEY is deliberately NOT here — it IS a credential and stays
# profile-scoped.
# API-server LISTENER settings — deployment config (compose/systemd env),
# which the scoped runner reload must keep seeing or containers silently
# lose the api_server platform. API_SERVER_KEY is a credential: NOT here.
"API_SERVER_ENABLED", "API_SERVER_HOST", "API_SERVER_PORT",
"API_SERVER_CORS_ORIGINS",
# Relay-connector ROUTING stamps — deployment config injected into the
# container/process env by managed deploys (the same shape as the
# API_SERVER listener settings above). The scoped runner reload and the
# relay-exclusive sweep in gateway/config.py must keep seeing them, and
# every reader (gateway.config, gateway.relay.relay_url()/registration/
# self-provision) must resolve the SAME value — a scope-dependent split
# leaves the adapter registered but the platform absent from config (or
# vice versa). Mirrors the non-secret/secret line drawn by the terminal
# env blocklist (tools/environments/local.py): routing hints are global;
# GATEWAY_RELAY_SECRET / GATEWAY_RELAY_ID / GATEWAY_RELAY_DELIVERY_KEY
# and the IDP_* credentials are auth material and deliberately NOT here —
# they stay profile-scoped with the fail-closed multiplex guard.
# Relay-connector ROUTING stamps injected by managed deploys. Every reader
# (gateway.config, relay_url()/registration/self-provision) must resolve
# the SAME value or the adapter registers while the platform is absent
# from config. GATEWAY_RELAY_SECRET/_ID/_DELIVERY_KEY and IDP_* are auth
# material and deliberately stay profile-scoped.
"GATEWAY_RELAY_URL", "GATEWAY_RELAY_ENDPOINT",
"GATEWAY_RELAY_ALLOW_DIRECT_PLATFORMS",
"GATEWAY_RELAY_PLATFORMS", "GATEWAY_RELAY_BOT_IDS",
@@ -140,38 +108,31 @@ _GLOBAL_ENV_PREFIXES = (
def _is_global_env(name: str) -> bool:
"""Return True for genuinely process-global (non-profile-secret) env vars."""
if name in _GLOBAL_ENV_EXACT:
return True
return any(name.startswith(p) for p in _GLOBAL_ENV_PREFIXES)
"""True for genuinely process-global (non-profile-secret) env vars."""
return name in _GLOBAL_ENV_EXACT or name.startswith(_GLOBAL_ENV_PREFIXES)
def _environ_or(name: str, default: Optional[str]) -> Optional[str]:
val = os.environ.get(name)
return val if val is not None else default
def get_secret(name: str, default: Optional[str] = None) -> Optional[str]:
"""Resolve a credential by env-var name, honoring the active profile scope.
Resolution order:
1. Genuinely-global vars (``_is_global_env``) always read ``os.environ`` —
they are deployment settings, not profile secrets.
2. When a secret scope is installed (multiplexed turn), read from it. Under
multiplexing the scope is authoritative — an absent key returns
``default`` and we do NOT fall through to ``os.environ``, because in a
multiplexer ``os.environ`` may hold another profile's value. When
multiplexing is OFF, a scope miss falls through to ``os.environ``:
single-profile deployments legitimately provide credentials via the
process environment (systemd ``Environment=``, secret-manager wrappers
like ``pass-cli run`` / ``op run``, plain shell exports) rather than
``<home>/.env``, and the scope — installed unconditionally around e.g.
every cron job — must stay a ``.env`` overlay, not a blindfold.
3. No scope installed:
- multiplex INACTIVE (default deployment): read ``os.environ`` —
identical to the legacy ``os.getenv`` behavior every caller had before.
- multiplex ACTIVE: FAIL CLOSED. Raise ``UnscopedSecretError`` so the
missing scope is caught loudly instead of leaking a cross-profile value.
1. Global vars (``_is_global_env``) always read ``os.environ``.
2. Scope installed: read from it. Under multiplexing the scope is
authoritative (a miss returns ``default``, never ``os.environ``, which
may hold another profile's value). With multiplexing OFF a miss falls
through to ``os.environ``: single-profile deployments legitimately
inject credentials via the process env (systemd, ``op run``, shell
exports) and the scope — installed around e.g. every cron job — must
stay a ``.env`` overlay, not a blindfold (otherwise cron 401s).
3. No scope: multiplex INACTIVE reads ``os.environ`` (legacy behavior);
ACTIVE raises ``UnscopedSecretError`` (fail closed).
"""
if _is_global_env(name):
val = os.environ.get(name)
return val if val is not None else default
return _environ_or(name, default)
scope = _SECRET_SCOPE.get()
if scope is not None:
@@ -180,14 +141,7 @@ def get_secret(name: str, default: Optional[str] = None) -> Optional[str]:
return val
if _MULTIPLEX_ACTIVE:
return default
# Multiplex off: the scope is an overlay over the process environment,
# not an isolation boundary — there is no other profile to leak from.
# Without this fallthrough, credentials injected only into the process
# environment vanish inside any set_secret_scope(...) block (the cron
# scheduler installs one around every job), so cron jobs send a
# placeholder API key and 401 while interactive turns keep working.
val = os.environ.get(name)
return val if val is not None else default
return _environ_or(name, default)
if _MULTIPLEX_ACTIVE:
raise UnscopedSecretError(
@@ -199,25 +153,18 @@ def get_secret(name: str, default: Optional[str] = None) -> Optional[str]:
f"(Workstream A)."
)
val = os.environ.get(name)
return val if val is not None else default
return _environ_or(name, default)
def _strip_inline_comment(value: str) -> str:
"""Strip a dotenv-style inline comment from a raw ``.env`` value.
Mirrors python-dotenv (1.2.2) semantics, verified empirically:
- Quoted values: scan for the matching close quote
(backslash-escape-aware for double quotes, since ``save_env_value``
writes ``\\"``/``\\\\`` escapes). Everything through the close quote is
kept; a trailing ``# ...`` remainder after it is discarded, so
``KEY="has # inside" # trailing`` yields ``has # inside``. Non-comment
trailing junk leaves the value untouched (lenient, unlike dotenv's
hard parse error).
- Unquoted values: truncate only at a ``#`` PRECEDED BY WHITESPACE, so
``KEY=foo#bar`` keeps ``foo#bar`` while ``KEY=value # comment`` keeps
``value``. A value that *starts* with ``#`` (``KEY=#leading``) is kept.
Mirrors python-dotenv semantics: for quoted values scan to the matching
close quote (backslash-escape-aware for double quotes, since
``save_env_value`` writes ``\\"``/``\\\\``) and drop a trailing ``# ...``;
other trailing junk (or an unterminated quote) leaves the value untouched.
Unquoted values truncate only at a ``#`` PRECEDED BY WHITESPACE, so
``foo#bar`` and a leading ``#`` survive while ``value # comment`` → ``value``.
"""
value = value.strip()
if not value:
@@ -241,19 +188,13 @@ def _strip_inline_comment(value: str) -> str:
def load_env_file(env_path: Path) -> Dict[str, str]:
"""Parse a ``.env`` file into a plain dict WITHOUT touching ``os.environ``.
"""Parse a ``.env`` file into a dict WITHOUT touching ``os.environ``.
Used to load a profile's secrets into an isolated mapping for
``set_secret_scope``. Parses the small KEY=VALUE subset Hermes writes
itself (``export`` prefix, ``#`` comments — full-line and
dotenv-compatible inline, matching quotes with the
writer's ``\\"``/``\\\\`` escapes reversed — the same semantics as
``hermes_cli.config._parse_env_value``) but never mutates the process
environment — that isolation is the whole point.
Encoding is ``utf-8-sig`` so a leading UTF-8 BOM (Windows Notepad /
PowerShell ``Set-Content -Encoding UTF8``) does not prefix the first
key as ``\\ufeffNAME`` and make ``get_secret('NAME')`` miss under scope.
Handles the subset Hermes writes (``export`` prefix, full-line and inline
``#`` comments, quotes with the writer's escapes reversed via the canonical
``_parse_env_value`` — stripping only outer quotes would corrupt credentials
containing ``"`` or ``\\``). ``utf-8-sig`` so a Windows BOM doesn't prefix
the first key as ``\\ufeffNAME``.
"""
secrets: Dict[str, str] = {}
try:
@@ -261,12 +202,6 @@ def load_env_file(env_path: Path) -> Dict[str, str]:
except (FileNotFoundError, OSError, UnicodeDecodeError):
return secrets
# Parse values with the canonical Hermes parser: save_env_value
# escapes " and \ inside double quotes, and every other reader
# (load_env, python-dotenv) reverses those escapes. Stripping only
# the outer quotes here would corrupt credentials containing "
# or \ — they work interactively but fail in scoped (cron /
# multiplex) resolution.
from hermes_cli.config import _parse_env_value
for raw in text.splitlines():
@@ -287,12 +222,9 @@ def load_env_file(env_path: Path) -> Dict[str, str]:
def build_profile_secret_scope(hermes_home: Path) -> Dict[str, str]:
"""Build a profile's secret mapping from its ``<home>/.env``.
Returns a fresh dict (safe to install via ``set_secret_scope``). Genuinely
global vars are intentionally NOT copied in — ``get_secret`` reads those
from ``os.environ`` directly, so the scope holds only profile secrets.
"""
"""Build a profile's secret mapping from ``<home>/.env`` plus its external
secret sources. Global vars are NOT copied in — ``get_secret`` reads those
from ``os.environ`` — so the scope holds only profile secrets."""
home = Path(hermes_home)
secrets = load_env_file(home / ".env")
@@ -303,8 +235,7 @@ def build_profile_secret_scope(hermes_home: Path) -> Dict[str, str]:
external_secrets = {}
for key, value in external_secrets.items():
if _is_global_env(key):
continue
secrets[key] = value
if not _is_global_env(key):
secrets[key] = value
return secrets