Skip to content

fix(gateway): run /insights, /debug and /goal draft inside the routed profile - #78440

Closed
Drexuxux wants to merge 1 commit into
NousResearch:mainfrom
Drexuxux:fix/slash-command-profile-scope
Closed

Drexuxux wants to merge 1 commit into
NousResearch:mainfrom
Drexuxux:fix/slash-command-profile-scope

Conversation

@Drexuxux

@Drexuxux Drexuxux commented Aug 4, 2026 •

Copy link
Copy Markdown

What does this PR do?

The multiplexed inbound handler wraps every message in _profile_runtime_scope,
which installs the routed profile's HERMES_HOME override and its secret scope
as contextvars. A bare loop.run_in_executor(None, fn) starts the worker
with an empty context, so neither reaches the blocking work.

GatewaySlashCommandsMixin already knows this — /compress goes through
_run_in_executor_with_context, and the call site spells out why:

_run_in_executor_with_context (not a bare run_in_executor): the profile
secret scope installed by the wrapper is a contextvar, and the
default-executor hop would drop it …

Three siblings in the same file still used the bare hop:

Command What the empty context caused
/insights SessionDB() with no explicit path resolves get_hermes_home() at call time (_default_db_path), so the worker opened the default profile's state.db. The command reported another profile's conversations, session counts and sources to this profile's user.
/debug Collects that home's logs/config and uploads them to a public paste — it published the default profile's diagnostics from another profile's chat.
/goal draft Calls the auxiliary LLM, whose provider/credential resolution reads the profile secret scope. Unscoped it falls back to process-global os.environ, which under multiplexing may hold a different profile's keys.

/insights is the sharpest of the three: it is a read that renders another
profile's conversation history into this profile's chat
.

How it was found

Continued the canonical-helper-bypass sweep: grep the repo for its own declared
invariants, then look for call sites that violate them. This file documents
the rule at one call site
and breaks it at three others, so the remaining bare
hops were enumerated and filtered down to the ones that provably touch
get_hermes_home() or the secret scope.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/slash_commands.py — /insights, /debug and /goal draft now
    dispatch their blocking work through self._run_in_executor_with_context(...),
    matching the /compress sibling. Each site carries a short comment naming the
    contextvar it depends on. Removed the two loop = asyncio.get_running_loop()
    locals the change orphaned.
  • tests/gateway/test_slash_command_profile_scope.py — 3 tests.

Deliberately left alone

/reload-skills also uses a bare hop, but tools.skills_tool binds
SKILLS_DIR at import time, so it does not follow the contextvar with or
without context propagation. Fixing it needs the module-global retarget that
web_server._profile_scope performs under a lock — a different change from
context propagation, and out of scope here.

Single-profile gateways never enter _profile_runtime_scope, so their behaviour
is unchanged.

How to Test

  1. Run a multiplexed gateway with two profiles that each have their own session
    history.
  2. From the non-default profile's chat, send /insights.
  3. Before: the report is built from the default profile's state.db — the
    other profile's conversations.
    After: it reads <root>/profiles/<name>/state.db.

Test Results

New file tests/gateway/test_slash_command_profile_scope.py — 3 tests. They
drive the real mixin handler and the real _profile_runtime_scope; the
contextvar loss is a property of the hop, so mocking the hop away would test
nothing:

Test Asserts
test_session_db_opens_under_the_profile_home /insights end-to-end: the SessionDB the worker constructs resolves <root>/profiles/coder
test_without_a_scope_it_still_uses_the_launch_home no scope installed → launch home, i.e. single-profile behaviour unchanged
test_helper_preserves_the_override_a_bare_hop_drops the shared mechanism: _run_in_executor_with_context sees the override, a bare run_in_executor(None, …) does not

/debug and /goal draft take the identical one-line substitution but sit
behind adapter and goal-manager scaffolding; rather than build a fake deep
enough to stop testing the real thing, their shared guarantee is pinned by the
third test.

Red-without-fix, with only gateway/slash_commands.py reverted:

1 failed, 2 passed

With the fix:

3 passed in 3.37s

Regression sweep

slash / insights / debug / goal suites + new file:   70 passed
ruff check gateway/slash_commands.py:                All checks passed

Whole tests/gateway/ directory, both sides on the same main, comparing the
failure sets rather than just counts:

baseline (change stashed):  71 failures
with this fix:              71 failures
new failures introduced:    none

The two sets are byte-identical — the pre-existing failures live in unrelated
files (feishu, runtime_footer, update, discord, systemd) that this change does
not touch.

… profile

The multiplexed inbound handler wraps every message in _profile_runtime_scope,
which installs the routed profile's HERMES_HOME override and its secret scope
as contextvars. A bare loop.run_in_executor(None, fn) starts the worker with an
EMPTY context, so neither reaches the blocking work.

GatewaySlashCommandsMixin already knows this -- /compress goes through
_run_in_executor_with_context and the call site says why. Three siblings in the
same file still used the bare hop:

  /insights   SessionDB() with no explicit path resolves get_hermes_home() at
              call time (_default_db_path), so the worker opened the DEFAULT
              profile's state.db. Under multiplexing the command reported
              another profile's conversations, session counts and sources to
              this profile's user.

  /debug      collects that home's logs/config and uploads them to a public
              paste, so it published the default profile's diagnostics from
              another profile's chat.

  /goal draft calls the auxiliary LLM, whose provider/credential resolution
              reads the profile secret scope -- unscoped it falls back to
              process-global os.environ, which under multiplexing may hold a
              different profile's keys.

Route all three through _run_in_executor_with_context.

/reload-skills is deliberately left alone: tools.skills_tool binds SKILLS_DIR
at import time, so it does not follow the contextvar either way. Fixing that
needs the module-global retarget web_server._profile_scope performs under a
lock, which is a different change from context propagation.

Single-profile gateways never enter the scope, so their behaviour is unchanged.
@Drexuxux Drexuxux changed the title fix(gateway): carry the profile scope into pre-turn hygiene compression fix(gateway): run /insights, /debug and /goal draft inside the routed profile Aug 4, 2026
@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/gateway Gateway runner, session dispatch, delivery area/profiles Multi-profile isolation, HERMES_HOME scoping P2 Medium — degraded but workaround exists 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 labels Aug 4, 2026
@egilewski

Copy link
Copy Markdown

suggesting changes

The context-preserving executor changes correctly route goal drafting, insights, and debug to the selected profile, but the debug flow still loads the selected profile credentials into the process-global environment during dump collection. In a multiplexed gateway, unrelated readers or child processes can then observe keys belonging to another profile. Please fix this before merge by making dump credential inspection use the active profile secret scope or a private mapping without mutating shared environment, and add coverage for concurrent profile and child-process isolation.

Security evidence:

  • trust boundary: Multiplexed requests bind a profile home and secret mapping in context-local state; debug performs blocking collection and uploads the resulting report.
  • source/sink/invariant: run_dump calls load_hermes_dotenv for the routed home, and that loader writes credentials to shared process state. The invariant is that diagnostics use only the routed profile and leave shared credentials unchanged.
  • current-main reproduction: Before the change, these handlers used context-dropping executor hops and could read the default profile. The change preserves profile context but leaves the dump loader mutation in shared environment.
  • PR-head or patch-replay validation: The three targeted handlers use context-preserving execution, while the debug call chain still reaches the global dotenv loader.
  • positive/negative cases: Positive: context propagation selects the routed home for profile-scoped state. Negative: dump credential loading leaves the routed keys in shared environment after the worker returns.
  • residual bypass search: No other bare executor hop reaches the three reviewed sinks; the dump loader remains a separate shared-environment bypass.
  • reviewer validation: Source tracing and focused probes confirm the profile routing fix and the remaining shared-environment credential mutation.

Not checked:

  • gateway profile-scope tests
  • debug runtime execution
  • concurrent child-environment isolation

Signed: GPT-5.6-luna-max in Codex

@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this PR. Merged via #101246 (527da60) on current main — slash config writes, executor hops, personality and status follow the routed profile.

This PR was one of the vehicles for that merge: your commits were cherry-picked into #101246 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 >= 527da60, please open a fresh PR/issue against main and tag it. Thanks again.

@teknium1 teknium1 closed this Sep 2, 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/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 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.

4 participants