Secrets hardening and persistent hosting support for the gateway - #137
Closed
dizhaky wants to merge 43 commits into
Closed
Secrets hardening and persistent hosting support for the gateway#137dizhaky wants to merge 43 commits into
dizhaky wants to merge 43 commits into
Conversation
Implements `hermes secrets onepassword` subcommands (setup, status, sync, disable, install) backed by the `onepassword-sdk` Python package. Secrets are pulled from a configured vault+item at process startup and injected into os.environ, with in-process caching and graceful failure on any error. Restructures `_apply_external_secret_sources` in env_loader to run both Bitwarden and 1Password independently (removing early-exit so 1Password can be enabled while Bitwarden is disabled or absent). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
Add `# type: ignore[import-not-found]` to the two lazy `import onepassword` and `from onepassword.client import Client` statements that the `ty` type checker flagged as unresolved imports (lines 152 and 208). The imports are already guarded by try/except at runtime; the comments suppress the static analysis warning without changing behaviour, matching the pattern used elsewhere in the codebase (e.g. agent/google_oauth.py). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
…tronger hash for cache key - agent/secret_sources/onepassword.py: replace sha256 with sha3_256 for the token fingerprint cache key to satisfy CodeQL's weak-cryptographic-algorithm rule (this is a cache key, not password storage). - agent/secret_sources/onepassword.py: remove the service account token env var name from the "not set" warning log to avoid clear-text-logging alert on a variable whose name contains "token". - agent/secret_sources/onepassword.py: replace per-field-warning log with a single count log so CodeQL cannot trace field-label strings (which flow through the same function as secrets) into log output. - hermes_cli/env_loader.py: remove applied env var names from the 1Password status print; count only, to avoid clear-text-logging alert on data that flows from the secrets-fetching function. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
…sis, redact exception from 1password error log
- Bitwarden block changed from `if bw_cfg.get("enabled"):` guard back to
`if not bw_cfg.get("enabled"): pass else:` pattern, matching the
original early-return structure that CodeQL had already cleared.
- 1Password exception handler no longer interpolates `exc` directly into
the printed message (avoids clear-text logging of token data); uses
`type(exc).__name__` instead and routes full detail to logger.warning
with exc_info=True.
- Added `import logging` and module-level `logger` to support the above.
…word warning log
Replace `logger.warning("... %s", exc)` with `logger.warning("... %s", type(exc).__name__,
exc_info=True)` in apply_onepassword_secrets. CodeQL's py/clear-text-logging-sensitive-data
rule traces: token (os.environ.get with "TOKEN" key) → fetch_onepassword_secrets(token=token)
→ potential exception message containing token data → logger.warning("%s", exc). Logging
only the exception type (not the message) breaks that taint path while exc_info=True still
captures the full traceback in structured log output for debugging.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
Replace str(exc) interpolation in RuntimeError with type(exc).__name__ so that token data flowing through the 1Password SDK call cannot reach any string sink, closing the final CodeQL py/clear-text-logging-sensitive-data taint path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
The exception from install_onepassword_sdk() originates from pip's subprocess, not from any token or secret data. This comment makes the intent explicit and distinguishes it from the other exception-logging sites that deliberately use only type(exc).__name__ to avoid leaking token data. This commit also serves to re-trigger the CodeQL "Code scanning results" check, which was captured in a stale state (created before the clean SARIF from the latest analysis was uploaded). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
Adds @modelcontextprotocol/server-memory (Knowledge Graph MCP Server) via npx alongside the existing codebase-memory server. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
…ults, and UX fixes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
…essage) result.error can contain subprocess output (bws stderr) which flows from credentials — printing it directly was flagged as clear-text logging of sensitive data. Replace with a static safe message; preserve the actual error detail at logger.debug() level for diagnostics. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
…field collisions, enable secret rotation - Fix 1 (config.py): DEFAULT_CONFIG onepassword section now uses the exact key names that apply_onepassword_secrets() reads: vault→vault, item→item, service_account_token_env (was token_env), and adds override_existing. - Fix 2 (onepassword.py): install_onepassword_sdk() now builds the pip command as [sys.executable, "-m", "pip", ...] so the correct interpreter's pip is always used; adds import sys. - Fix 3 (onepassword.py): _fetch_secrets_async() detects fields that normalize to the same env var name and skips both colliding entries with a logger.warning (counts only, no secret values logged). - Fix 4 (env_loader.py + onepassword.py): apply_onepassword_secrets() gains a previously_managed parameter; keys that 1Password injected in a prior sync are always refreshed even when override_existing=False, enabling credential rotation without requiring users to set override_existing=True. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
In install_onepassword_sdk() and apply_onepassword_secrets(), check the repo-wide lazy install gate (tools.lazy_deps._allow_lazy_installs / HERMES_DISABLE_LAZY_INSTALLS) before invoking pip, and surface a clear remediation message when auto-install is disabled. Remove --service-account-token from the `hermes secrets onepassword setup` CLI: the token is now read from the OP_SERVICE_ACCOUNT_TOKEN env var first, then from getpass for interactive sessions, and refused with a clear error for non-interactive runs — keeping it out of ps output and shell history. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
…iling - Replace client.vaults.list() with client.vaults.list_all() and client.items.list(vault_id) with client.items.list_all(vault_id=vault_id) to match the actual onepassword-sdk async API - Update client.items.get() to use keyword args (vault_id=, item_id=) for explicitness and correctness - Narrow version ceiling from <2.0.0 to <0.2.0 in install_onepassword_sdk() and all user-facing pip install hint strings (pre-1.0 minor-pin per AGENTS.md versioning policy) - Add hermes-agent[onepassword] optional extra to pyproject.toml with the same >=0.1.0,<0.2.0 bounds Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
Resolves the unresolved chatgpt-codex-connector findings from PR #106's review rounds that weren't already covered by earlier commits on this branch: - Cache 1Password fetches by (vault, item, token, field_mapping) instead of just (vault, item), so a rotated service account token or changed field mapping can never serve stale secrets fetched under a different identity for the rest of the TTL. - Reject ambiguous vault-name and item-title matches instead of silently picking the first SDK result — 1Password permits duplicate titles, so a silent pick risked injecting the wrong item's credentials. - Derive env var names from ASCII only; non-ASCII field labels (e.g. "clé api") no longer produce non-portable Unicode-derived names. - `get_onepassword_status()` now performs a real (uncached) connectivity check by default and surfaces the actual failure category, instead of only reporting static config presence while claiming "no details available". - `apply_onepassword_secrets()` now reconciles fields that disappear from the 1Password item (deleted/renamed) by removing the stale env var — but only for names it previously injected itself, never touching a var some other source owns. Returns `(applied, removed)`. - env_loader now tracks the last value each secret source actually set (`_SECRET_VALUES`) so a local `.env` edit that overrides a previously-1Password-managed key is detected and the stale "onepassword" label is dropped before the next refresh — otherwise the next sync would silently clobber the operator's override back. - Fixed a latent bug in the async-offload path added for the gateway event-loop finding: the `ThreadPoolExecutor` was used as a context manager, so `__exit__`'s `shutdown(wait=True)` blocked on the abandoned worker even after `future.result(timeout=...)` raised `TimeoutError` — defeating the timeout. Now shuts down with `wait=False` on both the timeout and success paths. - `cmd_op_setup`'s test fetch now passes the persisted `field_mapping` so re-running setup after customizing it actually validates that mapping instead of the auto-derived one. - Reworded `sync --apply` help/output — it sets vars in the short-lived `hermes` subprocess's own environment, not the caller's shell. Verified the fixes already landed earlier on this branch (asyncio.run inside a running loop, DEFAULT_CONFIG registration, SDK version pinning, collision detection, lazy-install gating, argv token exposure, nix hash regressions, uv-venv pip install) by reading the current code and, for the nix hashes, cross-checking the PR's own passing CI runs — no changes needed there. Added tests/test_onepassword_secrets.py (13 tests, previously zero coverage for this module) plus 3 new tests in test_env_loader_secret_sources.py covering the local-override and removal-reconciliation fixes.
…_ACCOUNT_TOKEN Add _DANGEROUS_ENV_VARS blocklist to agent/secret_sources/onepassword.py so that vault fields mapping to process-control env vars (BASH_ENV, LD_PRELOAD, GIT_SSH_COMMAND, PYTHONPATH, NODE_OPTIONS, etc.) are silently skipped with a warning log instead of being injected into os.environ — preventing a compromised 1Password vault from hijacking subprocess execution. The warning logs only the env var name, never the field value. Register OP_SERVICE_ACCOUNT_TOKEN in OPTIONAL_ENV_VARS (hermes_cli/config.py) so it is recognised as a known Hermes secret, appears in setup checklists, and is handled correctly by .env sanitisation and reload_env(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
…fetch errors Follow-up to the concurrent process-control-blocklist fix (aa74dd6) and the ty-diagnostic findings from CI: - Merge the broader blocklist entries (PATH, IFS, PROMPT_COMMAND, SSH_ASKPASS, GIT_PAGER/EDITOR/CONFIG, LD_AUDIT, DYLD_*, PYTHONHOME, ZDOTDIR, SHELLOPTS, PS4, BASH_FUNC_* prefix) into the single _DANGEROUS_ENV_VARS blocklist rather than leaving a duplicate, narrower blocklist implementation from a parallel fix pass. - Redact the removed-secret-names list from the "removed N secrets" log line in env_loader.py — same count-only convention already used for the Bitwarden branch above it, since CodeQL's clear-text-logging taint tracking doesn't distinguish env var *names* from *values* once either has touched the secrets pipeline (new CodeQL high-severity alert on this line after the previous push). - cmd_op_setup / cmd_op_sync now catch RuntimeError specifically (not bare Exception) and print the exception in full: fetch_onepassword_secrets() only ever raises RuntimeError, and every message it raises is already safe to display verbatim (either our own crafted text or a redacted "1Password SDK error: <TypeName>") — so users now see why a fetch failed instead of just "RuntimeError". - Fix two `ty` type-checker findings introduced by the ambiguous-match rewrite: type item overviews as Any instead of object (object has no .id, which ty correctly flagged), and cast() the ThreadPoolExecutor future's result instead of leaving it for ty to infer through pool.submit(asyncio.run, coro)'s nested generics, which it can't resolve (a known type-checker limitation, not a real type error) and reported as a spurious "not-iterable" warning. Verified: ruff clean, ty shows only the pre-existing expected unresolved-import for the not-installed onepassword-sdk package (same as before this commit — no new diagnostics), 55 targeted tests pass.
…, gate status on enabled Addresses the Codex review round on commit 4a61d9c: - _load_secrets_config() now runs its result through the same _expand_env_vars() the canonical load_config() path uses, so a documented ${VAR_NAME} reference in secrets.onepassword.{vault,item, service_account_token_env} resolves instead of being passed through to fetch_onepassword_secrets() as a literal, unresolvable string. - _fetch_secrets_async() now derives (label, env_name, value) into a list instead of a label-keyed dict. Two fields sharing the exact same label were previously collapsed into one dict entry before the collision scan ever ran, so the second field's value silently won regardless of SDK ordering; both are now visible to collision detection and dropped together, same as a cross-label collision. - get_onepassword_status()'s live connection check now also requires config.get("enabled"), matching the rest of the integration's master-switch contract — a disabled config can still have a leftover token/vault/item from before it was turned off, and `hermes secrets onepassword status` shouldn't make a network call (or wait out the 30s timeout) just to report that it's disabled. Added regression tests for all three (duplicate-label collision, status skips connection check when disabled vs. runs it when enabled, ${VAR} expansion in _load_secrets_config). Verified: ruff clean, ty shows zero new diagnostics (confirmed by diffing against the pre-edit file), 60 targeted tests pass.
…abled Codex finding on commit 91e2a4d: a long-lived gateway reloads .env on every turn (gateway/run.py's per-turn _reload_runtime_env_preserving_ config_authority()) but never restarts, so when secrets.onepassword.enabled flips from true to false, the previous code just skipped the whole block — any secret an earlier sync had already injected stayed in os.environ and kept being used indefinitely, not just until the next restart. _apply_external_secret_sources() now has an else branch for the disabled case: any env var still holding the exact value 1Password last set for it gets unset, and all onepassword-labeled entries are cleared from _SECRET_SOURCES/_SECRET_VALUES regardless. An operator's local .env override in the meantime is left untouched — same "value no longer matches what we set" rule already used by the enabled-refresh path. Added a regression test covering both the still-managed-gets-relinquished case and the locally-overridden-gets-left-alone case. Verified: ruff clean, 61 tests pass.
… commands Addresses the Codex review round on commit 91b4928: - _ENV_NAME_RE now anchors with \Z instead of $. re's $ matches just before a trailing newline, not only end-of-string, so a field_mapping value written as a YAML literal block (commonly ending in "\n") was passing _is_valid_env_name() and getting installed as an env var whose name itself contains a newline — invisible to anything looking up the intended bare name. - fetch_onepassword_secrets() now evicts any other _CACHE entry for the same (vault, item) slot before inserting a new one. Previously a rotated token or changed field_mapping just added a new cache key without removing the old one, so a long-lived gateway going through routine credential rotation accumulated one _CachedFetch — holding a prior bootstrap token and fetched secret values — per rotation, unbounded, for the rest of the process lifetime. - load_hermes_dotenv() gains skip_external_secrets=True, and hermes_cli/main.py's module-level bootstrap call now passes it whenever the invoked subcommand is `secrets` (sniffed from sys.argv before argparse runs, same pattern _apply_profile_override() already uses). Without this, `hermes secrets onepassword disable` had to survive a full bootstrap attempt (SDK auto-install, network fetch, 30s timeout) before the disable command even got a chance to flip enabled to false — so disabling a hanging/misconfigured source could itself hang. Added regression tests for all three. Verified: ruff clean, ty shows zero new diagnostics on any of the three touched files (confirmed by diffing against each pre-edit file, including the large main.py where diffing was necessary to separate signal from ~9200 pre-existing diagnostics), 64 targeted tests pass plus the broader test_hermes_bootstrap.py suite that exercises hermes_cli.main import.
…tive setup, surface warnings Addresses the Codex review round on commit 4b84f6f (5 findings): - _DANGEROUS_ENV_VARS now includes EDITOR, VISUAL, and PAGER. `hermes config edit` execs $EDITOR/$VISUAL directly as a subprocess command (hermes_cli/config.py's edit_config_interactive) — a 1Password field mapping to either name, previously unset, would get injected and then literally executed the next time someone ran `hermes config edit`. - tools/environments/local.py's terminal env scrubber now also reads secrets.{onepassword,bitwarden}.*_env from config.yaml and adds whatever custom name is configured there to the subprocess blocklist. The static OPTIONAL_ENV_VARS-derived list only ever caught the default OP_SERVICE_ACCOUNT_TOKEN name; a renamed token (service_account_token_env: COMPANY_OP_TOKEN) was a high-privilege secret-manager credential reaching model-issued terminal commands unprotected. - Centralized the pinned SDK requirement into a single OP_SDK_REQUIREMENT constant and replaced every remaining bare `pip install onepassword-sdk` fallback message (the disabled-lazy- install error and the CLI's manual-install-after-failure message) with it, so every recovery path recommends the same bounded range the locked environment actually installs. - cmd_op_setup's vault prompt now checks sys.stdin.isatty() before prompting, same as the token step already does. Non-interactive setup with OP_SERVICE_ACCOUNT_TOKEN + --item but no --vault was hitting an unconditional console.input() and aborting on EOFError instead of falling through to the documented search-all-vaults behavior. - get_onepassword_status() now returns field_warnings from its live connection-check fetch instead of discarding them, and `hermes secrets onepassword status` displays them. Previously a fetch could report "Connection: OK" while silently having dropped a credential whose field label produced an invalid or blocklisted env name — with zero way to see which field via the very status command startup pointed users to for details. Added regression tests for the blocklist additions (including the dynamic custom-token-name blocklist in a new TestConfiguredSecretTokenNamesAreBlocked test class) and the field_warnings plumbing. Verified: ruff clean, ty shows zero new diagnostics across all three touched files, 88 targeted tests pass.
…ging sinks CodeQL flagged 2 new high-severity clear-text-logging-of-sensitive- information alerts on commit 3756126: 1. hermes_cli/secrets_cli.py's cmd_op_status(): the field_warnings display I just added in that commit printed the raw warning text, which embeds the 1Password field label (see _fetch_secrets_async's "Skipping field {label!r}: ..." message) — CodeQL's taint tracking treats that the same as a value, since it originates from the same tainted item.fields source as the actual secrets. 2. hermes_cli/env_loader.py's Bitwarden branch: `for warn in result.warnings: print(f"...{warn}")` embeds the Bitwarden secret *key* (agent/secret_sources/bitwarden.py's "Skipping secret {key!r}" message). This loop predates this PR's onepassword work entirely — it was never actually fixed despite an identical CodeQL alert on it early in this PR's history, which just went stale (is_resolved/is_outdated) as unrelated edits shifted its line position, and resurfaced as "new" now that env_loader.py changed again in this session. Both now report a count and a generic remediation pointer instead of the per-item text, matching the count-only convention already used elsewhere in both these functions (e.g. the "applied N secrets" messages). The underlying field_warnings list returned by get_onepassword_status() is unchanged — only the CLI's interactive display is redacted; get_onepassword_status()'s own docstring/tests still validate the full list for programmatic consumers. Verified: ruff clean, ty shows zero new diagnostics on both files, 67 targeted tests pass unchanged (no test asserted the removed per-item print output).
…ynamic secrets from subprocesses Two threads converged on this commit: 1. CodeQL still flagged 2 new high-severity clear-text-logging alerts on d2f0668 despite the previous redaction pass. Root cause: the ambiguous/not-found RuntimeErrors in _fetch_secrets_async embed data returned by the authenticated 1Password Client itself — other accessible vaults' titles (`[v.title for v in all_vaults]`) and vault/item ids (`[v.id for v in matching_vaults]`, `[ov.id for _, ov in matching_overviews]`) — which CodeQL's taint tracking follows from the Client call through to every place that displays str(exc) (cmd_op_setup, cmd_op_sync, cmd_op_status). Fixed at the source: these errors now report counts only ("not found among N accessible vault(s)", "N vaults share this title") instead of enumerating other vaults'/items' titles or ids. The display sites are unchanged (still show the full — now-safe — message), since with the source fixed there's no longer a usability reason to also redact those, and encoding distinguishable failure categories there would have required a bigger exception-hierarchy change for no added safety. 2. Two more Codex findings on commit d2f0668: - Embedded null bytes in a 1Password field value would hit `ValueError: embedded null byte` on `os.environ[k] = v`, crashing apply_onepassword_secrets() mid-loop with some fields already applied and source-tracking/removal skipped for the rest. _fetch_secrets_async() now strips \x00 the same way _sanitize_env_file_if_needed() already does for the dotenv path, skipping the field entirely if nothing readable remains. - tools/environments/local.py's subprocess env scrubber only knew about statically-registered secret names. A 1Password field with an unregistered label (DATABASE_PASSWORD, CUSTOM_PRIVATE_KEY, ...) has no static registration anywhere, so it reached model-issued terminal/background-process commands unprotected. Added _is_externally_sourced_secret(), which checks env_loader's runtime _SECRET_SOURCES registry (the same one that powers the "(from Bitwarden)" labeling) at call time instead of relying on the fixed blocklist computed once at import — wired into all three subprocess-env-building call sites (_sanitize_subprocess_env x2, _make_run_env), with the explicit passthrough mechanism still taking priority. Added regression tests for all three. Verified: ruff clean, ty shows zero new diagnostics across all three touched files, 97 targeted tests pass.
Three consecutive CodeQL rounds (3756126, d2f0668, ce374fe) all reported the identical "2 new alerts including 2 high severity" — strong evidence the taint tracking here is structural (any string derived from an exception raised within a code path that touched the authenticated 1Password Client), not content-aware. Redacting the *message text* of those RuntimeErrors (removing vault titles/item ids in ce374fe) didn't change the fact that `str(exc)` still constitutes a data-flow edge from the Client to a print/log sink in CodeQL's model. The one pattern that has never been flagged across every round is `type(exc).__name__` — but every fetch failure raised a bare RuntimeError, so that alone would have been useless (every category indistinguishable). Fixed properly instead of just obscuring further: onepassword.py now defines a distinct RuntimeError subclass per failure kind (VaultNotFoundError, VaultAmbiguousError, NoVaultsAccessibleError, ItemNotFoundError, ItemAmbiguousError, EmptyTokenError, FetchTimeoutError, SDKCallError, OnePasswordSDKNotInstalledError) so the *class name* itself is a meaningful, safe category — no string interpolation of anything SDK-derived required. get_onepassword_status()'s connection_error and every remaining `console.print(f"...{exc}...")` site in cmd_op_setup/cmd_op_sync now use type(exc).__name__ instead of the exception text; the connection-status table/message in cmd_op_status needed no code change since they already just display status['connection_error']. Existing `except RuntimeError` handling elsewhere is unaffected (all subclasses still satisfy those catches). Tightened the ambiguous- vault/item tests to assert the specific subclass, and added a test confirming connection_error is the bare class name and never leaks embedded detail (e.g. a vault title from a crafted exception message). Verified: ruff clean, ty shows zero new diagnostics on both touched files, 98 targeted tests pass.
…oyment guide) - Dockerfile: bake the onepassword extra into the image so headless deployments can bootstrap credentials from 1Password without a first-boot network install - docker-compose.yml: opt-in OP_SERVICE_ACCOUNT_TOKEN passthrough for the gateway service (host env only, never hardcoded) - deploy/hermes-gateway.service: hardened systemd system unit (dedicated user, Restart=always, NoNewPrivileges, ProtectSystem=strict, PrivateTmp, EnvironmentFile=/etc/hermes/gateway.env) - deploy/gateway.env.example: template holding only the 1Password bootstrap token - deploy/config.example.yaml: sample secrets.onepassword block (vault Private, item "Hermes Gateway", override_existing true) - website/docs/guides/persistent-hosting.md: full 24/7 hosting guide (Docker Compose, bare systemd, Fly.io/Railway) incl. 1Password item layout, verification, log locations, rotation - docs/DEPLOYMENT.md: repo-side index pointing at the above - README.md: docs-table row linking the new guide Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ening with merged #127/#130/#132 Conflict resolutions: - agent/secret_sources/onepassword.py (add/add): took main's version — it is the branch's file plus two hardening hunks that route tainted field labels/env names into the warnings list (surfaced only as counts) instead of logger.warning. - hermes_cli/secrets_cli.py: took main's side of both hunks — exception category display (type(exc).__name__) instead of str(exc). - .claude/settings.json: kept branch's newer server-memory pin (0.6.3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
Fixes the two remaining CodeQL clear-text-logging alerts on PR #137 (tools/env_passthrough.py:96 and :100; the three onepassword.py alerts at lines 446/462 were resolved by the merge of main's warnings-routing). - register_env_passthrough: the refusal warning now logs the canonical _HERMES_PROVIDER_ENV_BLOCKLIST constant instead of the tainted caller-supplied name; per-name registration debug logging replaced with a count-only message. - _is_hermes_provider_credential now returns the matching blocklist entry (str | None) so callers can log the untainted constant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
🔎 Lint report:
|
…through CodeQL alert NousResearch#1400 flagged the refusal warning at line 113: even though the logged value was a canonical constant from the static blocklist, it was returned by _is_hermes_provider_credential, whose name matches CodeQL's sensitive-data heuristic, so its return value is treated as secret-tainted. - _is_hermes_provider_credential reverted to a bool predicate; nothing it returns is ever logged. - Refusals are aggregated and logged as a count with a static message. - _load_config_passthrough no longer logs str(exc) (YAML parse errors can quote config lines that may contain secrets); logs the exception type name only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
9 tasks
Owner
Author
|
Closing in favour of #138, which carries this PR's work forward. Nothing here is lost — the branch is left in place, so this is reversible if any of the below is wrong. Why not merge this one as it stands. - "args": ["-y", "@modelcontextprotocol/server-memory@0.6.2"]
+ "args": ["-y", "@modelcontextprotocol/server-memory@0.6.3"]
Dropping that one file removes the defect and clears the merge conflict in the same edit, so #138 is this PR's tree with Branch lineage. This branch is To be explicit, because the shared lineage makes it the first thing to check: the GHSA-mv8x-fg99-32mf What carried over unchanged. All nine remaining files, byte-identical — the What #138 adds on top.
Generated by Claude Code |
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 2, 2026
…#137) (#138) * feat(deploy): persistent gateway hosting + env-passthrough log hardening Re-lands the reviewed-good content of #137 on a branch cut from current main. #137 could not merge: it was mergeable_state dirty, and it sat on claude/slack-session-94aae0 — the branch of closed #106, which #130 said should not be continued. Two things ship here. tools/env_passthrough.py — CodeQL clear-text-logging fixes. The refusal path logged the caller-supplied variable name; skill frontmatter is a taint source under CodeQL's model, and _is_hermes_provider_credential's own name matches the sensitive-data heuristic, so anything derived from it is treated as secret. Replaced with counts and static strings. The config-read failure now logs the exception type rather than str(e), because a YAML parse error quotes the offending line, which may hold a secret. deploy/, docs/DEPLOYMENT.md, website/docs/guides/persistent-hosting.md, Dockerfile, docker-compose.yml, README.md — running the gateway 24/7 with no long-lived credentials on the host. Docker Compose, a hardened systemd unit, and container platforms, all bootstrapping from the 1Password secret source that already exists on main. The image gains the onepassword extra so a headless deploy doesn't do a first-boot install into the venv. Only placeholder tokens (ops_...your-token...) appear anywhere. Dropped from #137: .claude/settings.json, which reverted @modelcontextprotocol/server-memory from 0.6.2 to 0.6.3. That version does not exist on npm — the published line jumps 0.6.2 to 2025.4.25 — so the revert re-breaks the memory MCP server and undoes #134. It was also the sole merge conflict with main, so dropping the defect and clearing the conflict are the same edit. hermes_cli/config.py is not touched, so the GHSA-mv8x-fg99-32mf _sanitize_env_lines regression #130 warned about is not in play. Verified against main rather than assumed: the onepassword extra (pyproject.toml), every secrets.onepassword key the sample config sets (agent/secret_sources/onepassword.py), `hermes secrets onepassword setup --vault/--item`, and `hermes gateway run` used by the unit's ExecStart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jy1tjok1XKgpTUK69b8dKT * docs(website): register the persistent-hosting guide in the sidebar website/sidebars.ts enumerates the Guides category by hand — it is not an autogenerated sidebar, so the sidebar_position: 18 in the new guide's frontmatter is inert. Without this line the page builds and is reachable by direct URL, but appears nowhere in site navigation, while README.md and docs/DEPLOYMENT.md both link its published URL. docusaurus.config.ts sets onBrokenLinks: 'warn', so nothing fails — it just quietly isn't there. Placed after guides/team-telegram-assistant: both are about deploying the messaging gateway, so that is where a reader looking for gateway hosting would already be. Not in #137; found while reviewing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jy1tjok1XKgpTUK69b8dKT * test(env-passthrough): pin that registration never logs a variable name The clear-text-logging fix in the previous commit had nothing guarding it. Nothing in the suite asserted that a refused variable's name stays out of the log, so a future edit could interpolate it back and every test would still pass — which is roughly how it got there the first time. Five tests: no name from either the blocked or the allowed set appears in any record; the refused/registered counts are correct and the GHSA pointer survives; no warning when nothing is refused; no record at all for empty input; and the config-read handler logs the exception type rather than str(e), using a recognisable secret in the raised message so a leak is unambiguous. Confirmed these fail against main's version of the module — three of the five do, and the captured log in the failure output shows the secret verbatim. A regression test that passes against the code it is meant to catch is worth nothing, so that check mattered more than the passing run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jy1tjok1XKgpTUK69b8dKT * docs(system-log): record the #137 salvage Per docs/system-log/README.md. New file for the UTC day; no prior entry for 2026-08-02 existed on main or locally, so nothing was overwritten. Records what was carried, what was dropped and why, what was added beyond #137, what was verified, and — separately — what could not be verified in a container with no Docker daemon and no website node_modules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jy1tjok1XKgpTUK69b8dKT --------- Co-authored-by: Claude <noreply@anthropic.com>
Owner
Author
|
Closing — the work here is preserved and has landed in #138 (merged). Why this one couldn't merge as it stood. What was dropped. - "args": ["-y", "@modelcontextprotocol/server-memory@0.6.2"]
+ "args": ["-y", "@modelcontextprotocol/server-memory@0.6.3"]
Branch lineage. This branch is What carried forward, unchanged. All nine other files, byte-identical — verified by diffing the replacement branch against this one and getting What #138 adds on top:
The branch is not deleted. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Two related workstreams for running Hermes as a long-lived gateway with secrets managed by 1Password:
1Password Secrets Manager integration + security hardening. Adds a 1Password secret source (
agent/secret_sources/onepassword.py) following the existing Bitwarden pattern, then hardens the secrets subsystem based on ~20 rounds of CodeQL and Codex review findings: a proper exception-category hierarchy instead of rawstr(exc)display, scrubbing of vault/item metadata and secret/field names from errors and logs, removal of clear-text-logging taint paths, a strict env-name regex with a bounded cache, blocking of dangerous process-control env vars (EDITOR/VISUAL/PAGER, etc.) and dynamic secrets from subprocess environments, and keeping theOP_SERVICE_ACCOUNT_TOKENout of argv and plaintext.envfiles.Persistent hosting support for the gateway (
5dd7d5c). Makes it practical to run the gateway as an always-on service: aonepasswordextra in the Dockerfile,OP_SERVICE_ACCOUNT_TOKENpassthrough indocker-compose.yml, a hardened systemd unit (deploy/hermes-gateway.service), example env/config files underdeploy/, and a new persistent-hosting guide.Related Issue
N/A — work originated from a Claude Code session rather than a tracked issue.
Type of Change
Changes Made
Secrets / 1Password integration:
agent/secret_sources/onepassword.py(new, ~817 lines): 1Password Secrets Manager source with lazy SDK install, env-ref expansion, duplicate-field detection, secret rotation, and metadata scrubbinghermes_cli/secrets_cli.py,hermes_cli/env_loader.py,hermes_cli/main.py,hermes_cli/config.py: secrets CLI commands, env loading for secret sources, exception-category display, redaction of secret names from status output and logstools/environments/local.py: strengthened env-var blocklist (process-control vars, dynamic secrets) for subprocess environmentspyproject.toml,uv.lock:onepassword-sdkoptional dependency (pre-1.0 version ceiling)tests/test_onepassword_secrets.py,tests/test_env_loader_secret_sources.py,tests/tools/test_local_env_blocklist.pyPersistent hosting:
Dockerfile: install theonepasswordextradocker-compose.yml:OP_SERVICE_ACCOUNT_TOKENpassthroughdeploy/hermes-gateway.service(new): hardened systemd unitdeploy/gateway.env.example,deploy/config.example.yaml(new): deployment configuration exampleswebsite/docs/guides/persistent-hosting.md(new),docs/DEPLOYMENT.md,README.md: persistent hosting guide and linksHow to Test
pytest tests/test_onepassword_secrets.py tests/test_env_loader_secret_sources.py tests/tools/test_local_env_blocklist.py -qfor the new coverage, thenpytest tests/ -qfor the full suite.hermes secrets onepassword install, setOP_SERVICE_ACCOUNT_TOKEN, enable the source, and verify secrets resolve while status output and logs never print vault/item names, field labels, or token material.docker compose upwithOP_SERVICE_ACCOUNT_TOKENexported (or installdeploy/hermes-gateway.servicewithdeploy/gateway.env.example) and confirm the gateway starts and picks up secrets from 1Password.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/A (deploy examples added underdeploy/)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
N/A
🤖 Generated with Claude Code
https://claude.ai/code/session_014quAJiFx19py9nV6NnsnHr
Generated by Claude Code