fix(security): prevent multiplex dotenv credential leakage

[lesterlxt]

What does this PR do?

Prevents turn-scoped load_hermes_dotenv() calls from copying a routed profile's credentials into process-global os.environ while a multiplex profile scope is active.

The root cause was that load_hermes_dotenv() always reached _load_dotenv_with_fallback(..., override=True), even when the active Hermes home came from a routed multiplex profile. The gateway helper guarded one reload call site, but lazy imports, cron, and other callers could invoke the loader directly and bypass that guard.

The guard now lives at the shared loader boundary and requires both:

• multiplex mode is active; and
• a routed profile-home override is installed.

This keeps unscoped gateway startup loading unchanged. Inside a routed profile scope, the loader refreshes external secret providers through hydrate_profile_secret_sources(), which writes to the existing profile-private snapshot, and returns without mutating the shared process environment.

Related Issue

Fixes #77562

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• hermes_cli/env_loader.py: skip process-global dotenv mutation only during an active routed multiplex profile scope, while retaining profile-private external-secret hydration.
• tests/gateway/test_multiplex_credential_isolation.py: prove two routed profiles retain distinct credentials and channel allowlists while the process-global allowlist remains unchanged.
• tests/test_env_loader_secret_sources.py: prove unscoped multiplex startup still loads .env, and routed profile loading still hydrates Bitwarden-backed credentials without exporting bootstrap/provider secrets globally.

Review Follow-up

This revision addresses the startup-order concern raised by @DonShelly and @egilewski:

• the early return is now restricted by get_hermes_home_override() is not None;
• multiplex startup without a routed profile scope still loads DISCORD_ALLOWED_CHANNELS;
• the profile-private secret-source hydration path remains covered;
• the field scenario from fix(gateway): profile-scoped .env load clobbers shared env under multiplex #77970 is covered by distinct profile A/B DISCORD_ALLOWED_CHANNELS values plus an unchanged process-global value.

The routed-scope discriminator follows the boundary identified by @DonShelly in #77970, while this PR keeps its existing external secret-provider hydration behavior and coverage.

How to Test

1. RED proof before narrowing the production guard:
   scripts/run_tests.sh tests/test_env_loader_secret_sources.py -k multiplex_without_profile_scope_still_loads -q
   failed because DISCORD_ALLOWED_CHANNELS remained unset instead of loading 123,456.
2. Targeted regression files:
   scripts/run_tests.sh tests/gateway/test_multiplex_credential_isolation.py tests/test_env_loader_secret_sources.py -q
   → 25 passed.
3. Broader affected-surface suite across env loader, secret scope, gateway, cron, and runtime profile isolation:
   → 91 passed across 9 files.
4. Ruff, git diff --check, and scripts/check-windows-footguns.py on the three changed files:
   → all passed.
5. Repository-standard full suite:
   scripts/run_tests.sh -j 16
   → 30,678 passed, 22 failed, 264 skipped across 2,816 files in 1,313.8s.
6. Re-ran all 16 files implicated by the full run with identical -j 4 parameters on this branch and a clean upstream/main worktree at 762610538:
   → both produced 698 passed, 21 failed, 6 skipped, with the same failing tests and the same separate import-error file. The extra FIFO timing failure seen only in the 16-worker full run passed in both four-worker comparisons.

GitHub main advanced after that full comparison. The final commit was rebased onto fe5e7799f; none of the 20 intervening commits touched this PR's three files, and the 91-test affected-surface suite passed again after the rebase. The full-suite checkbox remains unchecked because the repository-wide suite has existing macOS/environment failures.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 26.5.1, Python 3.11.15. Linux and Windows were not tested directly.

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A (function behavior documented in its docstring)
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — pure Python control flow; Windows footgun scan passed
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

No screenshot is applicable because this is a process-level credential-isolation fix with no UI change.

Targeted regression files: 25 passed, 0 failed
Broader affected-surface suite: 91 passed, 0 failed
Ruff: All checks passed
Windows footgun scan: No issues found
git diff --check: passed

Full repository suite (-j 16):
  2,816 files
  30,678 passed
  22 failed
  264 skipped
  1,313.8 seconds

Failed-file comparison (-j 4, same 16 files):
  fix branch:    698 passed, 21 failed, 6 skipped
  upstream/main: 698 passed, 21 failed, 6 skipped

[Ruanjq98]

Code Review PR #77592: fix(security): prevent multiplex dotenv credential leakage. LGTM. Clean security fix at the load_hermes_dotenv shared boundary. Covers all call sites. Tests prove Profile A and B keep distinct credentials. Approve.

[GottZ]

This was generated by AI during triage.

Summary

One open PR addresses Issue #77562. Its diff moves the multiplex isolation guard to the shared load_hermes_dotenv() boundary, preventing routed profile .env values from entering process-global os.environ while retaining profile-private external secret hydration, with regression coverage for cross-profile isolation and provider refresh.

Related pull requests

• fix(security): prevent multiplex dotenv credential leakage #77592 fixes — (+97/-3) — n/a: Adds an is_multiplex_active() guard at the shared dotenv-loader boundary, calls hydrate_profile_secret_sources() against the profile-local scope, and returns before global dotenv mutation; tests cover distinct routed credentials and external-source hydration without os.environ leakage.

Suggested consolidation

keep open with a salvage path: retain #77592's shared-loader guard, profile-private hydration, and both regression tests as the concrete implementation path for #77562; no duplicate PRs are present.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I77562(["issue #77562 (open)"])
    P77592["PR #77592 (open)"]
    P77592 -->|fixes| I77562
    class I77562 open
    class P77592 open
    class P77592 target
    click I77562 "https://github.com/NousResearch/hermes-agent/issues/77562"
    click P77592 "https://github.com/NousResearch/hermes-agent/pull/77592"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 6 kB of PR diffs, 15 kB of issue/PR text, <1 kB of discussion (1 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[DonShelly]

Ran into this same clobber in production and filed #77969 / #77970 before triage pointed me here — that issue is now closed as a duplicate of #77562, and this PR's guard is the broader fix, so I'd rather help this one land than compete with it.

One observation from the field, plus tests you're welcome to take.

Field data. Five Discord bots in one multiplexed gateway. DISCORD_ALLOWED_CHANNELS held the full 17-channel list at startup and collapsed to one profile's 5 a few minutes after every restart, once the first long turn in that profile's channel pulled in trajectory_compressor. The other bots then rejected slash commands in their own channels and silently dropped plain messages — the allowlist gate returns before any inbound logging, so there was nothing in the logs to chase. Three separate fixes looked verified and then "regressed", because each verification happened inside the window before the next turn re-clobbered it. Your test docstring names the cause exactly: the lazy-import and cron call sites bypass _reload_runtime_env_preserving_config_authority's guard.

The bit I'd pin down. This PR returns early on is_multiplex_active() alone, with no check that a profile scope is actually installed. That's strictly safer than my narrower version, and it does not break startup today — but only because the module-level loads at hermes_cli/main.py:691 and gateway/run.py:1670 execute at import time, before GatewayRunner.__init__ flips the flag at gateway/run.py:5525. Nothing currently pins that ordering. If a future entry point constructs the runner before those module-level loads, os.environ stays empty in multiplex mode and every os.getenv-based adapter setting silently becomes unset — DISCORD_ALLOWED_CHANNELS unset means "no allowlist", i.e. the gate opens rather than closes. Same silent-failure class this PR is killing, one layer up.

Cheap insurance is a test asserting the startup path still populates the process env, so the ordering invariant fails loudly in CI instead of in someone's guild.

Tests on offer. #77970 carries three, written against this code path and passing on my install:

• test_profile_scoped_load_does_not_clobber_shared_env — the bug; fails on stock with assert 'profile-a-only' == 'all-channels'.
• test_multiplex_without_profile_scope_still_loads — the ordering guard above. This one fails against this PR as written, since you return [] whenever multiplex is active. If the unconditional skip is deliberate, the useful version asserts the equivalent at the entry-point level instead (env populated after import, before runner construction).
• test_single_profile_gateway_is_unaffected — non-multiplex behaviour unchanged.

Happy to open them as a PR against this branch, or just paste them here — whichever suits. I'll close #77970 once this lands.

[egilewski]

suggesting changes

The shared loader guard currently fires whenever the process-wide multiplex flag is true, even when no routed profile scope is installed. That makes startup/shared environment loading depend on the current module-import order: with multiplexing active but no scope, current main loads an env-only DISCORD_ALLOWED_CHANNELS, while this current-main replay returns [] and leaves the setting absent. For env-only Discord configurations, an absent allowed-channel set means no channel restriction. Please restrict the early return to an active routed profile/home scope, or otherwise preserve and regression-test the unscoped startup path while keeping scoped profile loads out of os.environ.

Security evidence:

• trust boundary: deployment/startup .env values cross into process-global security-gate configuration before per-profile routing begins.
• source/sink/invariant: routed profile loads must not mutate shared os.environ, while an unscoped startup load must still populate shared non-secret gate settings.
• current-main reproduction: with multiplexing active and no profile scope, load_hermes_dotenv() loaded DISCORD_ALLOWED_CHANNELS from the selected home into os.environ.
• PR-head or patch-replay validation: the conflict-free current-main replay returned no loaded path and left the same setting absent; a routed scoped load was also blocked as intended.
• positive/negative cases: the replay preserved ordinary single-profile loading, but it did not distinguish routed multiplex loads from unscoped multiplex startup/shared loads.
• residual bypass search: loader call sites, multiplex activation order, profile-secret scope, and Discord's allowed-channel fallback were checked; the missing discriminator is whether a routed scope is actually installed.
• reviewer validation: invocation-level probes imported both decisive modules from the bound review tree, and the merge replay plus diff checks completed cleanly.

Not checked:

• Pytest validation
• Full test suite
• CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex

[lesterlxt]

Addressed the startup-scope review concern in 1dfd324e9.

• load_hermes_dotenv() now skips process-global mutation only when multiplex mode is active and a routed profile-home override is installed.
• Unscoped multiplex startup still loads DISCORD_ALLOWED_CHANNELS (covered by a new RED/GREEN regression test).
• Routed profile A/B allowlists remain in their private scopes while the process-global DISCORD_ALLOWED_CHANNELS stays unchanged.
• External secret-source hydration remains profile-private.

Validation after the final rebase onto fe5e7799f: 25 targeted tests and 91 affected-surface tests passed; Ruff, the Windows footgun scan, and git diff --check passed. For the full-suite failures, the same 16 files matched clean upstream/main exactly: 698 passed, 21 failed, 6 skipped.

[egilewski]

looks mergeable

Reviewed the multiplex dotenv isolation change. The scoped loader branch keeps routed profile credentials out of process-global os.environ while preserving unscoped startup behavior. Current-main reproduction showed the pre-fix leak; focused PR-head isolation and secret-source tests passed. No residual source-backed bypass was found.

Security evidence:

• trust boundary: The multiplex gateway routes turns through a context-local profile home and secret scope. Profile .env and external-source values must not cross the process-global os.environ boundary or enter unrelated child processes; unscoped startup remains the legacy process configuration path.
• source/sink/invariant: When multiplexing is active with a profile-home override, load_hermes_dotenv resolves external sources through a private per-home mapping and returns before dotenv parsing, so profile credentials are available to the active secret scope but are not written to os.environ.
• current-main reproduction: On current main, enabling multiplex mode and a profile-home override then loading that profile's dotenv placed its PROFILE_SCOPED_API_KEY in the shared process environment. The same setup on the PR head returned an empty loaded list and left the shared variable unset.
• PR-head or patch-replay validation: The selected checkout is the PR head. Its new isolation regression test covers two routed profiles and its external-source tests cover cold profile hydration, .op.env bootstrap precedence, and global-environment non-mutation.
• positive/negative cases: Positive cases resolved distinct profile A/B credentials and platform settings while preserving the inherited process value; external-source hydration populated only the profile snapshot. The negative case verified that multiplex startup without a profile override still loads dotenv into process configuration.
• residual bypass search: Reviewed the changed loader, its profile-runtime-scope caller, the per-fetch source environment, credential-read wrappers, and subprocess environment factories. No remaining path in the changed flow writes a routed profile's dotenv credentials to the shared environment.
• reviewer validation: Static source and diff review was paired with focused tests under the bound interpreter: the multiplex credential-isolation file passed 5 tests and the external-secret-source file passed 20 tests.

Not checked:

• Ruff validation
• CodeRabbit review

Signed: GPT-5.6-luna-max in Codex

[teknium1]

Thanks for this PR. Merged via #101244 (0fd9218) on current main — routed multiplex profiles stop leaking .env into os.environ or borrowing default creds.

This PR was one of the vehicles for that merge: your commits were cherry-picked into #101244 with your git authorship preserved. Closing this one since the same change is now on main.

If anything from your original change is still missing on main >= 0fd9218, please open a fresh PR/issue against main and tag it. Thanks again.