Skip to content

fix(whatsapp): scrub operator environment from bridge subprocess - #38080

Open
coygeek wants to merge 1 commit into
NousResearch:mainfrom
coygeek:fix/whatsapp-bridge-subprocess-env-credential-exfil
Open

fix(whatsapp): scrub operator environment from bridge subprocess#38080
coygeek wants to merge 1 commit into
NousResearch:mainfrom
coygeek:fix/whatsapp-bridge-subprocess-env-credential-exfil

Conversation

@coygeek

@coygeek coygeek commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary

The plugin-based WhatsApp adapter now launches its long-lived Node bridge with the shared sanitized subprocess environment instead of a copy of the full gateway environment.

  • Filter Hermes-managed provider, gateway, platform, and remote-compute credentials through _sanitize_subprocess_env before bridge launch.
  • Explicitly restore only bridge-consumed values: WHATSAPP_REPLY_PREFIX, the force-prefixed WHATSAPP_ALLOWED_USERS intake allowlist, and the three profile-aware media cache directories.
  • Apply with_hermes_node_path() to the already-sanitized environment so current managed-Node discovery and PATH behavior remain intact.
  • Preserve the current plugin adapter architecture and Windows subprocess launch behavior.

Closes #38079

Review feedback addressed

  • Ported the fix from the deleted gateway/platforms/whatsapp.py path to plugins/platforms/whatsapp/adapter.py.
  • Ported the regression test to the current plugin-backed test setup.
  • Preserved managed Node PATH injection by passing the sanitized dictionary into with_hermes_node_path().
  • Preserved the bridge-side WHATSAPP_ALLOWED_USERS contract through the sanitizer's force-prefix mechanism.
  • Preserved profile-aware image, audio, and document cache paths introduced after the original branch was opened.

Tests and validation

  • scripts/run_tests.sh tests/gateway/test_whatsapp_connect.py tests/gateway/test_whatsapp_reply_prefix.py tests/gateway/test_whatsapp_stale_bridge.py — 49 passed, 0 failed.
  • scripts/run_tests.sh tests/tools/test_local_env_blocklist.py — 51 passed, 0 failed.
  • ruff check plugins/platforms/whatsapp/adapter.py tests/gateway/test_whatsapp_connect.py — passed.
  • git diff --check — passed.

The bridge regression verifies that provider, gateway, and remote-compute credentials are absent; the WhatsApp reply prefix, allowlist, profile-aware cache paths, and managed Node PATH are retained; and sanitization occurs before Node PATH augmentation.

Rebased onto 477c08b44766ace8b890faa72bf82ecbcf2b3ba8; current head is a28bcac074ad44e2b28363bf357e290a41627a48.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening platform/whatsapp WhatsApp Business adapter comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels Jun 3, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verified the subprocess environment scrubbing — the bridge process now receives only the explicitly whitelisted config values (WHATSAPP_REPLY_PREFIX, WHATSAPP_ALLOWED_USERS via force-prefix) plus system essentials, not the parent's full os.environ. The test confirms OPENAI_API_KEY, GATEWAY_ALLOWED_USERS, and MODAL_TOKEN_SECRET are all absent from the bridge env while PATH and WHATSAPP_REPLY_PREFIX survive. _sanitize_subprocess_env at tools/environments/local.py:205 handles the allowlist logic, and the force-prefix mechanism (_HERMES_PROVIDER_ENV_FORCE_PREFIX) correctly preserves the real WHATSAPP_ALLOWED_USERS key for the bridge.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the bridge environment boundary. The underlying issue remains present on current main: plugins/platforms/whatsapp/adapter.py:625-652 builds the Node bridge environment from with_hermes_node_path(), and hermes_constants.py:521-531 shows that helper copies os.environ when called without an argument.

Problems

  • The patch targets gateway/platforms/whatsapp.py, which no longer exists on current main. Commit 5600105478ffde29d7566b45421b100eaa29c4ef moved it to plugins/platforms/whatsapp/adapter.py.
  • The port must retain the managed-Node PATH setup currently supplied by with_hermes_node_path() at plugins/platforms/whatsapp/adapter.py:626; use it with the sanitized environment rather than dropping it.

Suggested changes

  • Port the sanitizer and regression test to the plugin adapter/current test setup.
  • Keep force-prefixed WHATSAPP_ALLOWED_USERS: the bridge consumes it at scripts/whatsapp-bridge/bridge.js:106, while the sanitizer blocklist contains it at tools/environments/local.py:222.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Launch the WhatsApp bridge with a sanitized subprocess environment so parent-process credentials and operator-only allowlists are not inherited by the Node bridge.

Keep the required WHATSAPP_REPLY_PREFIX bridge configuration by passing it as explicit subprocess-only extra environment, and cover the regression in the WhatsApp connect tests.
@coygeek
coygeek force-pushed the fix/whatsapp-bridge-subprocess-env-credential-exfil branch from 9a51d11 to a28bcac Compare July 21, 2026 03:49
@coygeek

coygeek commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the automated review in a28bcac074ad44e2b28363bf357e290a41627a48:

  • Rebased onto current main and ported the fix from the removed core adapter to plugins/platforms/whatsapp/adapter.py.
  • The bridge now receives _sanitize_subprocess_env(os.environ, bridge_extra_env) rather than the parent environment.
  • with_hermes_node_path() is applied to that sanitized dictionary, preserving managed Node PATH resolution without copying os.environ again.
  • The force-prefixed WHATSAPP_ALLOWED_USERS bridge contract is retained, along with WHATSAPP_REPLY_PREFIX and current profile-aware cache directories.
  • The current regression test asserts credentials are absent and required WhatsApp/runtime values survive, including sanitizer-before-Node-PATH ordering.

Validation: 49 focused WhatsApp tests and 51 sanitizer/blocklist tests passed; scoped Ruff and git diff --check passed.

@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins and removed comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 21, 2026
@andrexibiza

Copy link
Copy Markdown
Contributor

Bound to the child-process credential-inheritance class under #83565 (#83565) — same bug class, different surface. WhatsApp bridge subprocess env scrub; Wave E — sibling to #56245 on adapter.py; adjudicate the pair before merging. The EPIC carries the live class table, dedup adjudication, and the dependency-driven merge order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/whatsapp WhatsApp Business adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(whatsapp): scrub operator environment from bridge subprocess

5 participants