Skip to content

fix(simplex): scope multiplex secondary-profile config, not shared env - #100241

Closed
nftpoetrist wants to merge 1 commit into
NousResearch:mainfrom
nftpoetrist:fix/simplex-multiplex-profile-scope
Closed

fix(simplex): scope multiplex secondary-profile config, not shared env#100241
nftpoetrist wants to merge 1 commit into
NousResearch:mainfrom
nftpoetrist:fix/simplex-multiplex-profile-scope

Conversation

@nftpoetrist

Copy link
Copy Markdown
Contributor

What & why

The SimpleX adapter's __init__ (auto_accept, group allowlist), check_requirements, validate_config, is_connected, _env_enablement, and _standalone_send all read SIMPLEX_* settings via raw os.getenv unconditionally.

Under a multiplexed gateway, a secondary profile's adapter construction and config-load hooks run inside a profile-scoped context (_profile_runtime_scope) where os.environ still holds the default profile's YAML-to-env bridge output. A secondary profile with its own (different, or absent) SimpleX configuration silently inherits the default profile's:

  • daemon WebSocket URL (SIMPLEX_WS_URL) — cron/mid-turn delivery via _standalone_send connects to the wrong daemon
  • group allowlist (SIMPLEX_GROUP_ALLOWED) — a secondary profile's own, possibly tighter, allowlist gets silently widened to the default profile's (e.g. the default's * wildcard)
  • auto-accept setting (SIMPLEX_AUTO_ACCEPT)

check_requirements/_env_enablement also gate/seed platform enablement off the default profile's env during load_gateway_config(), which itself runs inside the secondary profile's scope.

Note: SIMPLEX_ALLOWED_USERS/SIMPLEX_ALLOW_ALL_USERS (the DM allowlist) are unaffected — those already route through the generic, already-scoped gateway/authz_mixin.py::_auth_env() mechanism via register()'s allowed_users_env/allow_all_env.

Fix

Mirrors the established Buzz adapter fix for the same bug class (#98738):

  • _profile_scoped() / _scoped_platform_setting() — a secondary profile's own PlatformConfig.extra becomes authoritative and env is not consulted; a missing key fails closed to the safe default instead of borrowing the default profile's value. Single-profile gateways and the default profile under multiplexing keep the legacy os.getenv precedence unchanged.
  • _profile_simplex_extra()check_requirements() has no PlatformConfig argument, so (like Buzz's _profile_buzz_extra()) it reads the profile's own config.yaml directly via the scoped home override when running inside a secondary profile's scope.
  • _env_enablement() returns None under a secondary profile's scope, so it does not fabricate a SimpleX platform for a profile that never configured one from the default profile's env.

Tests

Added TestMultiplexProfileScope to tests/gateway/test_simplex_plugin.py (7 tests), mirroring the existing Buzz adapter coverage for this exact scenario: secondary profile's extra wins over default's env, missing keys fail closed to safe defaults (not the default's wildcard/disabled values), the default profile stays unscoped and keeps env precedence, check_requirements consults the profile's own config.yaml, validate_config/is_connected respect scope, _env_enablement returns None when scoped, and _standalone_send connects to the scoped daemon URL rather than the default's.

  • tests/gateway/test_simplex_plugin.py — 23 passed (16 pre-existing + 7 new)
  • Mutation-verify: reverting the adapter fix makes 6 of the 7 new tests fail as expected (the 7th, the default-profile-unscoped case, was never buggy)
  • Adjacent suites unaffected: tests/hermes_cli/test_send_cmd.py, tests/gateway/test_channel_directory.py, tests/gateway/test_unauthorized_dm_behavior.py — 45 passed

Scope note / prior art check

Three open PRs currently touch plugins/platforms/simplex/adapter.py + tests/gateway/test_simplex_plugin.py — checked each diff directly and confirmed zero function-level overlap with this fix (textual proximity only, same files, different functions/concerns):

Merged PR #65629 (multiplex credential isolation cluster) does not touch this file — confirmed via gh pr view --json files.

The SimpleX adapter's __init__ (auto_accept, group allowlist),
check_requirements, validate_config, is_connected, _env_enablement, and
_standalone_send all read SIMPLEX_* settings via raw os.getenv unconditionally.
Under a multiplexed gateway, a secondary profile's adapter construction and
config-load hooks run inside a profile-scoped context where os.environ still
holds the DEFAULT profile's YAML-to-env bridge output — so a secondary
profile with its own (different or absent) SimpleX config silently inherits
the default profile's daemon URL, group allowlist, or auto-accept setting
instead of its own.

Mirrors the established Buzz adapter fix for the same bug class (NousResearch#98738):
add _profile_scoped()/_scoped_platform_setting() so a secondary profile's own
PlatformConfig.extra is authoritative and env is not consulted, add
_profile_simplex_extra() so check_requirements() (which has no PlatformConfig
argument) consults the profile's own config.yaml instead of the process env,
and make _env_enablement() return None under a secondary scope so it does not
fabricate a SimpleX platform from another profile's env.

Adds a TestMultiplexProfileScope test class mirroring the Buzz adapter's
existing coverage for this exact scenario.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/plugins Plugin system and bundled plugins area/config Config system, migrations, profiles area/profiles Multi-profile isolation, HERMES_HOME scoping P3 Low — cosmetic, nice to have sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Sep 1, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

This PR makes the SimpleX adapter multiplex-profile-aware, mirroring the Buzz fix (#98738): under a secondary-profile secret scope, os.environ holds the default profile's bridged YAML-to-env output, so all SIMPLEX_* reads are routed through _scoped_platform_setting (plugins/platforms/simplex/adapter.py:26), which consults PlatformConfig.extra instead. The fail-closed behavior is consistent across every site — auto-accept defaults to on, group allowlist to empty, _env_enablement to None, and check_requirements reads the profile's own config.yaml via _profile_simplex_extra (adapter.py:44). The docstrings correctly explain why env must never be consulted inside a scope.

Test coverage is excellent: the multiplex fixtures keep the suite hermetic, and the standalone-send test correctly manages the ContextVar token within the same asyncio task context (test_simplex_plugin.py:367).

Non-blocking: the scoped group-allowlist read assumes group_allowed is string-typed in extra — a YAML list value would reach _parse_comma_list and raise on .split(",") (adapter.py:103-104). No coercion/type guard is applied unlike the auto-accept path which str()-casts.

Verdict: LGTM

teknium1 added a commit that referenced this pull request Sep 2, 2026
…plexing

SimplexAdapter.__init__ (auto_accept, group_allowed), the registry gates
check_requirements/validate_config/is_connected, _env_enablement and
_standalone_send all read SIMPLEX_* via raw os.getenv. Under
gateway.multiplex_profiles those paths run inside a secondary profile's
scope where os.environ holds the DEFAULT profile's YAML-to-env bridge
output -- so a secondary profile that never configured SimpleX was
auto-enabled on the default's daemon URL and inherited its group
allowlist / auto-accept setting.

Route every read through the module-local `_get_scoped_secret` wrapper
(get_secret; UnscopedSecretError -> os.getenv for the default profile,
which constructs unscoped) -- the same helper the IRC/ntfy/Photon/
Mattermost siblings use. Unlike the extra-only `_scoped_platform_setting`
shape proposed in #100241, this honors BOTH the secondary profile's own
.env (the scope) and its config.yaml extra, and needs no config.yaml
re-read in check_requirements.

Rewrite of #100241.

Co-authored-by: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com>
teknium1 added a commit that referenced this pull request Sep 2, 2026
…plexing

SimplexAdapter.__init__ (auto_accept, group_allowed), the registry gates
check_requirements/validate_config/is_connected, _env_enablement and
_standalone_send all read SIMPLEX_* via raw os.getenv. Under
gateway.multiplex_profiles those paths run inside a secondary profile's
scope where os.environ holds the DEFAULT profile's YAML-to-env bridge
output -- so a secondary profile that never configured SimpleX was
auto-enabled on the default's daemon URL and inherited its group
allowlist / auto-accept setting.

Route every read through the module-local `_get_scoped_secret` wrapper
(get_secret; UnscopedSecretError -> os.getenv for the default profile,
which constructs unscoped) -- the same helper the IRC/ntfy/Photon/
Mattermost siblings use. Unlike the extra-only `_scoped_platform_setting`
shape proposed in #100241, this honors BOTH the secondary profile's own
.env (the scope) and its config.yaml extra, and needs no config.yaml
re-read in check_requirements.

Rewrite of #100241.

Co-authored-by: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com>
teknium1 added a commit that referenced this pull request Sep 2, 2026
…plexing

SimplexAdapter.__init__ (auto_accept, group_allowed), the registry gates
check_requirements/validate_config/is_connected, _env_enablement and
_standalone_send all read SIMPLEX_* via raw os.getenv. Under
gateway.multiplex_profiles those paths run inside a secondary profile's
scope where os.environ holds the DEFAULT profile's YAML-to-env bridge
output -- so a secondary profile that never configured SimpleX was
auto-enabled on the default's daemon URL and inherited its group
allowlist / auto-accept setting.

Route every read through the module-local `_get_scoped_secret` wrapper
(get_secret; UnscopedSecretError -> os.getenv for the default profile,
which constructs unscoped) -- the same helper the IRC/ntfy/Photon/
Mattermost siblings use. Unlike the extra-only `_scoped_platform_setting`
shape proposed in #100241, this honors BOTH the secondary profile's own
.env (the scope) and its config.yaml extra, and needs no config.yaml
re-read in check_requirements.

Rewrite of #100241.

Co-authored-by: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com>
teknium1 added a commit that referenced this pull request Sep 2, 2026
…plexing

SimplexAdapter.__init__ (auto_accept, group_allowed), the registry gates
check_requirements/validate_config/is_connected, _env_enablement and
_standalone_send all read SIMPLEX_* via raw os.getenv. Under
gateway.multiplex_profiles those paths run inside a secondary profile's
scope where os.environ holds the DEFAULT profile's YAML-to-env bridge
output -- so a secondary profile that never configured SimpleX was
auto-enabled on the default's daemon URL and inherited its group
allowlist / auto-accept setting.

Route every read through the module-local `_get_scoped_secret` wrapper
(get_secret; UnscopedSecretError -> os.getenv for the default profile,
which constructs unscoped) -- the same helper the IRC/ntfy/Photon/
Mattermost siblings use. Unlike the extra-only `_scoped_platform_setting`
shape proposed in #100241, this honors BOTH the secondary profile's own
.env (the scope) and its config.yaml extra, and needs no config.yaml
re-read in check_requirements.

Rewrite of #100241.

Co-authored-by: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com>
teknium1 added a commit that referenced this pull request Sep 2, 2026
…plexing

SimplexAdapter.__init__ (auto_accept, group_allowed), the registry gates
check_requirements/validate_config/is_connected, _env_enablement and
_standalone_send all read SIMPLEX_* via raw os.getenv. Under
gateway.multiplex_profiles those paths run inside a secondary profile's
scope where os.environ holds the DEFAULT profile's YAML-to-env bridge
output -- so a secondary profile that never configured SimpleX was
auto-enabled on the default's daemon URL and inherited its group
allowlist / auto-accept setting.

Route every read through the module-local `_get_scoped_secret` wrapper
(get_secret; UnscopedSecretError -> os.getenv for the default profile,
which constructs unscoped) -- the same helper the IRC/ntfy/Photon/
Mattermost siblings use. Unlike the extra-only `_scoped_platform_setting`
shape proposed in #100241, this honors BOTH the secondary profile's own
.env (the scope) and its config.yaml extra, and needs no config.yaml
re-read in check_requirements.

Rewrite of #100241.

Co-authored-by: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com>
teknium1 added a commit that referenced this pull request Sep 2, 2026
…plexing

SimplexAdapter.__init__ (auto_accept, group_allowed), the registry gates
check_requirements/validate_config/is_connected, _env_enablement and
_standalone_send all read SIMPLEX_* via raw os.getenv. Under
gateway.multiplex_profiles those paths run inside a secondary profile's
scope where os.environ holds the DEFAULT profile's YAML-to-env bridge
output -- so a secondary profile that never configured SimpleX was
auto-enabled on the default's daemon URL and inherited its group
allowlist / auto-accept setting.

Route every read through the module-local `_get_scoped_secret` wrapper
(get_secret; UnscopedSecretError -> os.getenv for the default profile,
which constructs unscoped) -- the same helper the IRC/ntfy/Photon/
Mattermost siblings use. Unlike the extra-only `_scoped_platform_setting`
shape proposed in #100241, this honors BOTH the secondary profile's own
.env (the scope) and its config.yaml extra, and needs no config.yaml
re-read in check_requirements.

Rewrite of #100241.

Co-authored-by: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com>
teknium1 added a commit that referenced this pull request Sep 2, 2026
…plexing

SimplexAdapter.__init__ (auto_accept, group_allowed), the registry gates
check_requirements/validate_config/is_connected, _env_enablement and
_standalone_send all read SIMPLEX_* via raw os.getenv. Under
gateway.multiplex_profiles those paths run inside a secondary profile's
scope where os.environ holds the DEFAULT profile's YAML-to-env bridge
output -- so a secondary profile that never configured SimpleX was
auto-enabled on the default's daemon URL and inherited its group
allowlist / auto-accept setting.

Route every read through the module-local `_get_scoped_secret` wrapper
(get_secret; UnscopedSecretError -> os.getenv for the default profile,
which constructs unscoped) -- the same helper the IRC/ntfy/Photon/
Mattermost siblings use. Unlike the extra-only `_scoped_platform_setting`
shape proposed in #100241, this honors BOTH the secondary profile's own
.env (the scope) and its config.yaml extra, and needs no config.yaml
re-read in check_requirements.

Rewrite of #100241.

Co-authored-by: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com>
@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thanks @nftpoetrist for this PR — Merged via #101252 (2e25b47) on current main.

This one was rewritten onto the shared _get_scoped_secret primitive so all the scoped-secret lookups go through a single path instead of per-site logic; your other five PRs in this series were cherry-picked as-is. You're credited via Co-authored-by on the merge and in the PR body of #101252.

Closing this PR as superseded by the merged work.

@teknium1 teknium1 closed this Sep 2, 2026
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…plexing

SimplexAdapter.__init__ (auto_accept, group_allowed), the registry gates
check_requirements/validate_config/is_connected, _env_enablement and
_standalone_send all read SIMPLEX_* via raw os.getenv. Under
gateway.multiplex_profiles those paths run inside a secondary profile's
scope where os.environ holds the DEFAULT profile's YAML-to-env bridge
output -- so a secondary profile that never configured SimpleX was
auto-enabled on the default's daemon URL and inherited its group
allowlist / auto-accept setting.

Route every read through the module-local `_get_scoped_secret` wrapper
(get_secret; UnscopedSecretError -> os.getenv for the default profile,
which constructs unscoped) -- the same helper the IRC/ntfy/Photon/
Mattermost siblings use. Unlike the extra-only `_scoped_platform_setting`
shape proposed in NousResearch#100241, this honors BOTH the secondary profile's own
.env (the scope) and its config.yaml extra, and needs no config.yaml
re-read in check_requirements.

Rewrite of NousResearch#100241.

Co-authored-by: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles area/profiles Multi-profile isolation, HERMES_HOME scoping comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants