fix(secrets): secret-scope migration wave — /compress, Matrix, model tools, cold hydration, WhatsApp (cluster salvage) - #76573
Open
teknium1 wants to merge 9 commits into
Open
fix(secrets): secret-scope migration wave — /compress, Matrix, model tools, cold hydration, WhatsApp (cluster salvage)#76573teknium1 wants to merge 9 commits into
teknium1 wants to merge 9 commits into
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on fe869f5 ℹ️ InfoDesktop E2E visual evidence · View test artifacts · View job3 visual diffs. inline evidence upload failed. Failed to upload diff-1508682a2ae8-boot-ready-diff.png with gh image (exit code 1): Error uploading /home/runner/work/_temp/e2e-evidence/diff-1508682a2ae8-boot-ready-diff.png: step 0 (get upload token): uploadToken not found on repo page — do you have write access to NousResearch/hermes-agent? (or, if NousResearch enforces SAML SSO, authorize at https://github.com/orgs/NousResearch/sso) |
Multiplexed gateways resolve credentials through the fail-closed
per-profile secret scope (agent/secret_scope.py, Workstream A): any
get_secret() read outside a set_secret_scope(...) block raises
UnscopedSecretError. The agent turn installs the scope via _run_agent's
profile-scoping wrapper, but slash-command dispatch does not — so manual
/compress reached provider resolution unscoped and every invocation on a
gateway.multiplex_profiles: true deployment failed with:
Manual compress failed: get_secret('OPENROUTER_BASE_URL') called with
no profile secret scope active while multiplexing is on.
Same bug class as the cron scheduler (#57692) and the /v1/runs agent
path — an un-migrated call site the fail-closed design is meant to catch.
Two changes, both required:
- _handle_compress_command becomes a profile-scoping wrapper around the
existing handler (renamed _handle_compress_command_inner), mirroring
_run_agent: gated on multiplex_profiles, resolves the source profile's
home and runs the whole handler inside _profile_runtime_scope. Covers
the coroutine-side read (_resolve_session_agent_runtime).
- The compressor call switches from a bare loop.run_in_executor(None, …)
to the existing _run_in_executor_with_context helper, so the scope
contextvar survives the thread hop into _compress_context, where the
aux-client provider resolution reads credentials.
Single-profile gateways take the pass-through branch — zero behavior
change (pinned by test).
Tests: 2 added (scoped read inside the executor under fail-closed
multiplexing reproduces the field failure pre-fix; single-profile
pass-through). 162 gateway compress/multiplex-scope tests green.
The Matrix adapter read MATRIX_RECOVERY_KEY via os.getenv, so under gateway.multiplex_profiles every profile resolved the default profile's key. That produced "recovery key verification failed: Key MAC does not match" and broke E2EE for secondary profiles (#69090). Route the read through agent.secret_scope.get_secret, which honors the active profile's scope, with an os.getenv fallback for an unscoped read under multiplex (default-profile startup loop) — mirroring the Slack app-token pattern (#59739). Applied to both the startup verification site and the status diagnostic. Fixes #69090
…ltiplex profiles Fix #75349 Root cause: Under multiplex_profiles, secondary profiles run inside _profile_runtime_scope which installs a per-profile secret scope via set_secret_scope. The WhatsApp adapter (and the shared WhatsAppBehaviorMixin + Cloud API adapter) read WHATSAPP_MODE, WHATSAPP_DM_POLICY, etc. via raw os.getenv(), bypassing the secret scope. Since os.environ doesn't contain secondary profile .env values, the bridge silently falls back to 'self-chat' and rejects all inbound messages with self_chat_mode_rejects_non_self. Fix: - Add _wenv() helper in adapter.py that reads WHATSAPP_* vars through get_secret() (agent.secret_scope), which honors the active scope. - Replace all os.getenv('WHATSAPP_*') calls in adapter.py, whatsapp_common.py, and whatsapp_cloud.py with get_secret()-based equivalents. - Inject resolved WHATSAPP_* values into the bridge subprocess environment so the Node.js bridge (which reads process.env) sees the profile's own configuration. Changes: - plugins/platforms/whatsapp/adapter.py: 37 lines (+ helper, bridge_env injection, 2 os.getenv→_wenv) - gateway/platforms/whatsapp_common.py: 13 lines (6 os.getenv→_get_wsecret) - gateway/platforms/whatsapp_cloud.py: 21 lines (9 os.getenv→_get_wsecret) - New regression test: 6 test cases covering scope isolation, fallback, and cross-profile non-leakage.
…idge env set Follow-ups on the #75382 salvage (review findings): - _wenv/_get_wsecret now catch UnscopedSecretError and fall back to os.getenv for the DEFAULT profile's adapter, which constructs and sends outside any _profile_runtime_scope under multiplexing — a bare get_secret would crash its WhatsApp path (fixing one profile by breaking another). Same pattern as Slack SLACK_APP_TOKEN (#59739) and the Matrix recovery key. Scoped misses still return the default — no cross-profile borrow. - bridge_env overlay extended to the full WHATSAPP_* set bridge.js consumes (DEBUG, FORWARD_OWNER_MESSAGES, REPLY_PREFIX, MAX_MESSAGE_LENGTH, CHUNK_DELAY_MS, SEND_TIMEOUT_MS). - Removed the always-true conditional on WHATSAPP_MODE injection.
The salvaged hydrate_profile_secret_sources (#74549) seeded its profile-local env from <home>/.env only, but the documented 1Password bootstrap flow puts OP_SERVICE_ACCOUNT_TOKEN in the gitignored <home>/.op.env (mirrored from load_hermes_dotenv). A cold profile using that flow still failed 1Password hydration — the one unaddressed item from the sweeper review on #74549. Seed .op.env via setdefault so .env values win; never touches os.environ. Two regression tests.
teknium1
force-pushed
the
fix/secret-scope-wave
branch
from
August 2, 2026 04:41
366e1ae to
fe869f5
Compare
alt-glitch
added
type/bug
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
7 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.
Summary
Consolidated salvage of the secret-scope migration wave: five contributor fixes land the remaining per-subsystem scope migrations, all verified against the profile-boundary rule (isolation between profiles; a profile's own children — subagents, cron, review forks, bridge subprocesses — inherit its secrets).
Every premise was re-verified on current main by parallel review before cherry-picking; contributor authorship preserved per-commit.
Changes (in composition order)
_profile_runtime_scopeand itsrun_in_executor(None, …)hop dropped the ContextVar scope →UnscopedSecretErrorunder multiplexing. Now wraps the handler in the profile scope and uses_run_in_executor_with_context— scopes the spawn, no fallback, no swallowing.MATRIX_RECOVERY_KEYread viaget_secret; scoped miss returns empty (no cross-profile borrow); default-profile startup uses the established Slack-patternUnscopedSecretError→os.getenvfallback. 6 tests.get_secret;check_fn/tool-defs caches gain a(fn, profile_home)key (unresolved scope ⇒ cache bypass, fail-closed); fixes the default profile's own unscoped ingress via_primary_message_handlerscope wrap. Child-inheritance verified: cache scope rides the same ContextVarcopy_contextpropagates._profile_runtime_scopebefore scope build — profile-local env, never mutatesos.environ. Our follow-up seeds the documented.op.envbootstrap (OP_SERVICE_ACCOUNT_TOKEN) the original missed (sweeper-flagged); 2 regression tests.WHATSAPP_*reads via scope-aware helpers; bridge subprocess overlay injects the routed profile's values (child inherits — correct direction). Our follow-ups:UnscopedSecretErrorfallback so the default profile's adapter doesn't crash (the PR's one real defect), full bridge.js env set (DEBUG/FORWARD/PREFIX/lengths/delays), dead-conditional removal. Junk commits (CI-retrigger, stray changelog) dropped from the original.Validation
_wenv_scoped_recovery_key.op.envtoken seeded into profile-local env;os.environuntouchedCredits: @CocaKova (#74964), @sergioperezcheco (#69110), @tachyon-r (#74023), @kingrubic (#74549), @x7peeps (#75382).
Closes #69090, #75349, #74317.
Cluster context: #74722 and #76199 close separately as superseded (#74340 / #76213); #70616 closes as wrong-premise (roots-into-every-profile inheritance violates the isolation contract).
Infographic