Skip to content

fix(slack): scope Socket Mode app token per profile - #59869

Closed
kohoj wants to merge 1 commit into
NousResearch:mainfrom
kohoj:codex/fix-slack-app-token-secret-scope-59739
Closed

fix(slack): scope Socket Mode app token per profile#59869
kohoj wants to merge 1 commit into
NousResearch:mainfrom
kohoj:codex/fix-slack-app-token-secret-scope-59739

Conversation

@kohoj

@kohoj kohoj commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #59739.

In multiplex gateway mode, each profile gets its own secret scope, but the Slack adapter still read SLACK_APP_TOKEN from process os.environ when opening Socket Mode. That lets secondary profiles authenticate their websocket with the default profile's app-level token while their bot token is correctly profile-scoped, so the adapter logs as connected but Slack delivers the secondary app's events to no gateway connection.

Root Cause

SlackAdapter.connect() used the scoped PlatformConfig.token for the bot token but bypassed agent.secret_scope for the app token:

app_token = os.getenv("SLACK_APP_TOKEN")

In a multiplexer, process env can hold another profile's secrets by design, and _profile_runtime_scope() installs the active profile's .env into get_secret() instead.

Fix

Resolve the Socket Mode app token through get_secret("SLACK_APP_TOKEN"). This preserves legacy single-profile behavior because get_secret() falls back to os.environ when multiplexing is inactive, while making the active profile scope authoritative when multiplexing is on.

Tests

  • scripts/run_tests.sh tests/gateway/test_slack.py -q
  • $HOME/.hermes/hermes-agent/venv/bin/python -m ruff check plugins/platforms/slack/adapter.py tests/gateway/test_slack.py

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins platform/slack Slack app adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 6, 2026
@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-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #65629 — your commit was cherry-picked onto current main with your authorship preserved in git log (rebase merge). Your scope-authoritative get_secret("SLACK_APP_TOKEN") semantics were the right call (the competing or os.getenv variant reintroduced the cross-profile leak); we added a narrow UnscopedSecretError → env fallback on top so the default-profile startup loop and background reconnect rebuild (which call connect() unscoped under multiplex) don't fail-loop, plus a regression test for that path. Closes the #59739 report. Thanks!

@teknium1 teknium1 closed this Jul 16, 2026
@teknium1 teknium1 added the area/profiles Multi-profile isolation, HERMES_HOME scoping label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/slack Slack app adapter sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiplex: Slack adapter reads SLACK_APP_TOKEN from process env — all profiles connect Socket Mode to the default profile's Slack app

3 participants