fix(whatsapp): route WHATSAPP_* env reads through secret scope for multiplex profiles - #75382
fix(whatsapp): route WHATSAPP_* env reads through secret scope for multiplex profiles#75382x7peeps wants to merge 3 commits into
Conversation
…ltiplex profiles Fix NousResearch#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.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the multiplex secret-scope boundary; the core diagnosis is correct. Current main reads WHATSAPP_MODE via os.getenv before spawning the bridge (plugins/platforms/whatsapp/adapter.py:634), while multiplex scopes intentionally keep profile .env values out of os.environ (agent/secret_scope.py:123-177).
Problems
- The new child-environment overlay is incomplete.
scripts/whatsapp-bridge/bridge.js:56-124also readsWHATSAPP_DEBUG,WHATSAPP_FORWARD_OWNER_MESSAGES,WHATSAPP_REPLY_PREFIX,WHATSAPP_MAX_MESSAGE_LENGTH,WHATSAPP_CHUNK_DELAY_MS, andWHATSAPP_SEND_TIMEOUT_MS; these remain process-global for secondary profiles. Please scope and forward the bridge's completeWHATSAPP_*input set. - The secondary startup guard is still unscoped:
_start_one_profile_adapters()leaves_profile_runtime_scopebefore_own_policy_open_startup_violation()(gateway/run.py:12520-12522), whose policy reads useos.getenv(gateway/run.py:2269-2284). This can validate different policy values than the adapter uses after this change.
Suggested changes
- Add a mocked
connect()regression test assertingPopen(..., env=...)receives scoped mode, allowlist, and an additional bridge-only setting. - Evaluate the startup access-policy guard through the profile secret scope.
Automated hermes-sweeper review.
| # own configuration instead of falling back to self-chat defaults. | ||
| _profile_wa_mode = _wenv("WHATSAPP_MODE", "self-chat") | ||
| if _profile_wa_mode != "self-chat" or _profile_wa_mode: | ||
| bridge_env["WHATSAPP_MODE"] = _profile_wa_mode |
There was a problem hiding this comment.
Please forward the complete scoped environment contract consumed by scripts/whatsapp-bridge/bridge.js, not only this subset. The bridge also reads WHATSAPP_REPLY_PREFIX, WHATSAPP_FORWARD_OWNER_MESSAGES, WHATSAPP_DEBUG, WHATSAPP_MAX_MESSAGE_LENGTH, WHATSAPP_CHUNK_DELAY_MS, and WHATSAPP_SEND_TIMEOUT_MS; otherwise secondary profiles still silently fall back to process-global/default behavior for those settings.
|
CI check — verifying branch health. |
|
/ci-checks |
|
@teknium1 Could you please approve the CI workflow runs for this PR? The fork workflow needs maintainer approval to trigger. All fixes have been pushed and are ready for review. |
…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.
…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.
…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.
|
Salvaged and merged in #76573 — your substantive commit is on main as 4f4ea9a with your authorship, closing #75349 (the CI-retrigger and stray-changelog commits were dropped in salvage). We added follow-ups (5438e9c) for the review findings: an UnscopedSecretError→os.getenv fallback in _wenv/get_wsecret so the DEFAULT profile's adapter (which runs unscoped under multiplexing) doesn't crash — the Slack #59739 pattern; the bridge_env overlay extended to the full WHATSAPP* set bridge.js consumes; and the always-true WHATSAPP_MODE conditional removed. During rebase we also routed main's new allowlist-precedence reads through your scoped helpers, so that feature is multiplex-safe too. Thanks @x7peeps! |
✅ Resolved on mainPR has been closed. The review feedback was addressed on upstream main via commit No further action needed on this PR. |
…idge env set Follow-ups on the NousResearch#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 (NousResearch#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.
…idge env set Follow-ups on the NousResearch#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 (NousResearch#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.
Fixes #75349