fix(discord): read gate config from per-adapter extra, not process-global env (#72348)

[JonthanaHanh]

Summary

Fixes #72348. Under gateway.multiplex_profiles: true, multiple Discord adapters share a single process-global os.environ. The allow/deny gates (allowed_channels, ignored_channels, allowed_users, allowed_roles) were read via os.getenv(), causing first-writer-wins: the first profile to initialize pins the gate values for all other profiles.

Changes

• plugins/platforms/discord/adapter.py:
  • Seed gate values into seeded_extra in _apply_yaml_config() so they flow through PlatformConfig.extra (per-adapter isolated).
  • Add _get_allowed_channels(), _get_ignored_channels(), _get_allowed_users(), _get_allowed_roles() helper methods that read from self.config.extra first, falling back to os.getenv for backward compatibility.
  • Update critical call sites in _handle_message() and connect() to use the new helpers.

This matches the existing pattern used by require_mention and free_response_channels which already read from self.config.extra.

Testing

• Syntax: python3 -m py_compile plugins/platforms/discord/adapter.py -- OK
• Lint: ruff check plugins/platforms/discord/adapter.py -- All checks passed
• Existing tests: 7 passed (test_discord_adapter.py)

[jackjin1997]

The issue premise is valid, but this head does not yet fix the multiplex reproduction and introduces a startup exception.

1. seeded_extra is referenced in the allowed_users_cfg block before it is assigned (seeded_extra = {} appears later, immediately before backfill_cfg). Any YAML allow_from that enters this branch raises UnboundLocalError during config application.
2. More fundamentally, every new seed remains inside the old first-writer guard. For example, seeded_extra["allowed_channels"] only runs under if ac is not None and not os.getenv("DISCORD_ALLOWED_CHANNELS"). Profile A writes the env; profile B then skips the whole block, receives no per-adapter extra, and _get_allowed_channels() falls back to profile A’s global env. The exact two-profile/order-dependent failure therefore remains. Normalize and seed the current profile unconditionally; guard only the legacy os.environ write.
3. Only connect() and _handle_message() were migrated. At head 09c2300f1e7f, direct global reads remain at adapter lines 1974, 4185, 4329, 4412, and 4429 for channel/slash/component/fail-closed paths. A normal-message test alone would leave native commands/components enforcing another profile’s policy.
4. The authorization bypasses called out on [Bug]: Discord adapter allow/deny gates are process-global, breaking per-profile isolation under multiplex_profiles #72348 are untouched: DISCORD_ALLOW_ALL_USERS / GATEWAY_ALLOW_ALL_USERS remain global at 4254–4256, 4331–4333, and 7813–7815; _resolve_allowed_usernames() still unconditionally rewrites os.environ["DISCORD_ALLOWED_USERS"] at 4960. allowed_roles is read by the new helper but is not seeded from profile config in this diff.
5. Env-only per-profile values are not captured. Falling back to os.getenv after connect() cannot recover profile B’s authoritative .env, because _profile_runtime_scope deliberately does not mutate process env. Snapshot the scoped value while the adapter is connected, then keep it adapter-local.

There are no new regressions in this PR, so the reported “7 existing tests” cannot exercise any of these paths. The minimum failing-first test is two adapters with distinct gates, initialized A→B and B→A, asserting normal messages + slash/component authorization remain isolated. Add a negative case where A enables allow-all and B does not, plus username resolution after connect() proving no global mutation. Cover both YAML and profile-scoped .env inputs.

This needs a policy object/single adapter-local accessor used by all consumers, not four partial helpers whose fallback is the shared state that caused the bug.

Signed: GPT-5.6-sol-xhigh in Codex

[egilewski]

suggesting changes

The patch introduces per-adapter gate readers, and those readers correctly prefer PlatformConfig.extra when it is present, but the producer side does not satisfy that contract and several authorization sinks still bypass it. A focused two-profile probe reproduced current main's process-global first-writer behavior. On the PR head, an allow_from configuration raises UnboundLocalError before the seed dictionary is initialized; the loader catches that exception and discards the hook result. Independently, channel seed values are only returned while the corresponding global environment variable is empty, so only the first profile receives an extra value. Finally, channel-based user authorization and Discord slash authorization still read process-global channel gates directly. These defects preserve cross-profile authorization coupling and can apply one profile's trusted channel boundary to another adapter, so the change is not mergeable as a security fix.

• [P1] Build every profile's gate seed independently of the environment bridge (plugins/platforms/discord/adapter.py:9482)
  At line 9482, seeded_extra is assigned before its initialization at line 9498. With a normal allow_from value and an initially empty DISCORD_ALLOWED_USERS, _apply_yaml_config raises UnboundLocalError. gateway/config.py catches hook exceptions and continues, so the profile silently receives no returned gate extras. Even after moving the initialization, the new allowed_users, ignored_channels, and allowed_channels assignments remain inside not os.getenv(...) guards. In a shared multiplex process, the first hook invocation populates the global variable, and later profiles therefore skip their own seed assignment. The focused head probe observed the first allow_from call raise, and the channel-only probe returned profile A's seed followed by None for profile B. Consequently adapters fall back to whichever process-global value another profile established, preserving the cross-profile access-control defect this PR claims to fix.
  Remediation: Initialize seeded_extra before any use. For each configured gate, always normalize and place the profile's value in seeded_extra; guard only the legacy os.environ assignment so environment precedence remains intact. Add a regression test that invokes the hook for two profiles with distinct allow_from, allowed_channels, and ignored_channels values and asserts both returned extras remain distinct despite the shared environment.

• [P1] Route all channel authorization surfaces through the per-adapter gate (plugins/platforms/discord/adapter.py:4185)
  The new _get_allowed_channels/_get_ignored_channels methods are used only by _handle_message. Security-relevant sibling paths still consult shared process state: _discord_channel_ids_allowed reads DISCORD_ALLOWED_CHANNELS at line 4185 and is the channel-only grant used by _is_allowed_user; _evaluate_slash_authorization reads DISCORD_ALLOWED_CHANNELS and DISCORD_IGNORED_CHANNELS directly at lines 4412 and 4429. A focused probe gave two fake adapters different extra.allowed_channels values while the global value named profile A's channel; _discord_channel_ids_allowed returned true for profile A's channel and false for profile B's channel for both adapters. Thus a channel-only deployment can grant or deny regular-message and slash-command authorization according to another profile's configuration even if producer seeding is repaired.
  Remediation: Replace the remaining authorization-time environment reads with _get_allowed_channels() and _get_ignored_channels(), including _discord_channel_ids_allowed and _evaluate_slash_authorization. Audit the other direct gate reads, and add two-adapter tests covering channel-only regular-message authorization plus positive and negative slash-command channel cases.

Security evidence:

• trust boundary: Untrusted Discord users, messages, interactions, and recovered events cross into an adapter that can dispatch prompts and privileged slash actions to Hermes. Operator-controlled per-profile YAML and PlatformConfig.extra define which Discord users, roles, and channels may cross that boundary. Under multiplex_profiles, adapters share os.environ but must not share authorization policy.
• source/sink/invariant: Sources are discord.allow_from, discord.allowed_channels, discord.ignored_channels, nested platform extras, and legacy DISCORD_* variables. _apply_yaml_config is the validator/normalizer and must return a complete per-profile seed; the helpers parse strings into normalized sets. Sinks are _handle_message, _is_allowed_user via _discord_channel_ids_allowed, and _evaluate_slash_authorization before dispatch. The claimed invariant is that each sink uses its own adapter's extra first and falls back to environment only when that key is absent.
• current-main reproduction: Using the bound current-main object cb06017, an AST-extracted _apply_yaml_config was called sequentially with profiles 111/chan-a/ignore-a and 222/chan-b/ignore-b in one process. Both calls returned None and the environment remained 111/chan-a/ignore-a, reproducing first-writer-wins. Current main's adapter blob is unchanged from the PR merge base, so this is the relevant baseline.
• PR-head or patch-replay validation: git merge-tree using base b9ba7c7, bound main, and head 09c2300 produced a coherent merge with no conflict, and git diff --check was clean. On the exact head source, the first profile with allow_from raised UnboundLocalError for seeded_extra; a second call returned only its channel extras. In a channel-only run, profile A returned distinct extras and profile B returned None because the global guard was already populated.
• positive/negative cases: Positive helper tests showed explicitly supplied extras isolate adapters despite conflicting environment values: profile A parsed its own channel, ignore, user, and role sets, and profile B parsed its distinct values. The negative empty-string case correctly produced an empty ignored-channel set without falling back. Negative producer tests showed allow_from crashes and second-profile channel seeds disappear. A residual negative test showed both adapters authorize the global first-channel and reject profile B's own second-channel in _discord_channel_ids_allowed.
• residual bypass search: A repository search for direct DISCORD_ALLOWED_USERS, DISCORD_ALLOWED_ROLES, DISCORD_ALLOWED_CHANNELS, and DISCORD_IGNORED_CHANNELS reads found authorization bypasses outside the new helpers. The material sinks are _discord_channel_ids_allowed at 4185 and slash authorization at 4412/4429. _warn_if_fail_closed_default and missed-message backfill also retain global channel reads; they should be audited for per-adapter correctness, though the two published findings cover the demonstrated authorization impact.
• reviewer validation: The review inspected the full one-file diff, config-hook merge contract in gateway/config.py, message/user/slash authorization call paths, and all direct gate-variable references. Focused AST probes executed exact current-main and head function bodies without modifying Git state. The repository pytest suite could not be run because the available Python reports No module named pytest; no network or external review service was used because the work order forbids network tools.

Uncertainty: A live Discord multiplex end-to-end run was not possible without service credentials and network access.; The full test suite and focused existing tests were not runnable because pytest is absent from the available Python environment.; The runtime order in which profiles populate global variables may differ by deployment; the order changes which profile's policy leaks but does not remove the demonstrated coupling.

Signed: GPT-5.6-sol-xhigh in Codex

[teknium1]

Thanks for pursuing a real multiplex authorization-isolation defect. Current main still bridges Discord gates through first-writer process-global environment variables (plugins/platforms/discord/adapter.py:9678-9712) and still consumes them on message, channel-only, and slash paths (plugins/platforms/discord/adapter.py:4418, 4645, 4662, 7454).

Problems

• In PR head 09c2300f, seeded_extra is written before its existing initialization (plugins/platforms/discord/adapter.py:9482 versus 9498), so configured allow_from can raise during config application.
• The new seeds remain inside the existing not os.getenv(...) guards. Profile B therefore still receives no adapter-local gate after profile A populated the shared variable; the diff also has no allowed_roles seed.
• The change leaves direct global authorization reads outside connect() and _handle_message(), including channel-only and slash checks (plugins/platforms/discord/adapter.py:4418, 4645, 4662) and allow-all checks (4487-4489, 8013-8015).

Suggested changes

• Build one complete adapter-local policy independently for each profile, then use it at every authorization sink. Keep any environment bridge strictly as legacy fallback.
• Add order-independent two-profile regression tests for YAML and scoped .env inputs, normal messages, slash authorization, and a negative cross-profile allow-all case.

Automated hermes-sweeper review.

[teknium1]

seeded_extra is initialized later in this function, so this write raises UnboundLocalError when allow_from is configured. Initialize and populate the per-profile seed before this block; keep only the legacy os.environ write behind the existing environment-precedence guard.

[teknium1]

The per-adapter gate direction you took here landed via PR #75970 (#75970), credited in the commit. Your PR pointed the right way — per-adapter reads instead of process env — but had a few gaps flagged in review (seeded_extra before init, first-writer seed guards, uncovered slash/allow-all sites), so #75970 implements the full class: all 11 Discord gate vars, slash/component/backfill paths, and the Telegram mirror, with 21 isolation tests. Fixes #72348. Thanks for breaking the trail.