Skip to content

security(gateway): re-resolve hooks directory per call to fix profile isolation - #56508

Open
srojk34 wants to merge 1 commit into
NousResearch:mainfrom
srojk34:fix/gateway-hooks-dir-profile-leak
Open

security(gateway): re-resolve hooks directory per call to fix profile isolation#56508
srojk34 wants to merge 1 commit into
NousResearch:mainfrom
srojk34:fix/gateway-hooks-dir-profile-leak

Conversation

@srojk34

@srojk34 srojk34 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

gateway/hooks.py::HOOKS_DIR is resolved once at import time via get_hermes_home(), which reads a context-local ContextVar (_HERMES_HOME_OVERRIDE) set per-request under the multiplexed gateway (multiple profiles served from one process, e.g. the desktop tui_gateway). Freezing the resolved path at import time pins every later HookRegistry.discover_and_load() call to whichever profile's HERMES_HOME happened to be active the first time this module was imported in the process.

This is more serious than the typical instance of this bug class (already fixed for cache dirs, skills_hub.py, rich_sent_store.py, and — from this same audit — the Anthropic OAuth file, Nous auth.json, sessions.json index, checkpoint store, and sticker cache): the hooks system loads and executes arbitrary Python (handler.py, dynamically imported via importlib.util) in response to live agent events. Under the multiplexed gateway, each profile owns its own Gateway/HookRegistry instance, but they all read the same frozen module constant — so a later-starting profile's HookRegistry discovers and executes the FIRST profile's hook handlers against its own live event context (session_id, chat_id, message, response text truncated to 500 chars, per the module's own docstring), not the profile's own hooks. This is both a cross-profile data leak and an isolation-boundary violation (profile B's process runs profile A's hook code).

Changes

  • gateway/hooks.py: added _resolve_hooks_dir(), called from discover_and_load() instead of reading HOOKS_DIR directly. Keeps the module constant for backward compat and to preserve the existing test seam (tests/gateway/test_hooks.py has many call sites that patch("gateway.hooks.HOOKS_DIR", tmp_path)) — the resolver honors a monkeypatched value away from its import-time default, otherwise re-resolves fresh, mirroring gateway/platforms/base.py::_resolve_cache_dir's established pattern.
  • tests/test_profile_isolation_runtime.py (the existing profile-isolation regression suite): added TestGatewayHooksDirResolution with three tests — the resolved path actually differs between two distinct profile overrides, an end-to-end test proving a hook that only exists under profile B's hooks dir is NOT discovered when profile A's override is active (and vice versa), and a "monkeypatched constant still wins" regression test for the test-seam-preserving resolver.

Test plan

  • pytest tests/gateway/test_hooks.py tests/gateway/test_background_command.py -q — 42 passed, 1 pre-existing unrelated failure (confirmed via git stash: a macOS /private/var vs /var symlink path-comparison quirk in test_media_files_routed_by_type, present identically before this change)
  • pytest tests/test_profile_isolation_runtime.py -q — 13 passed (10 pre-existing + 3 new)
  • ruff check on all changed files — clean

… isolation

gateway/hooks.py::HOOKS_DIR is resolved once at import time via
get_hermes_home(), which is a context-local ContextVar under the
multiplexed gateway (multiple profiles sharing one process, each owning
its own Gateway/HookRegistry instance). Freezing the path at import time
pins every later HookRegistry.discover_and_load() call to whichever
profile's HERMES_HOME was active when this module was first imported --
so a later-starting profile silently discovers and executes the FIRST
profile's hook handlers (arbitrary Python code, not just data) against
its own live event context, including session_id/message/response text.
Same bug class already fixed for cache dirs, skills_hub, rich_sent_store,
and the OAuth/auth.json/sessions.json/checkpoint/sticker-cache paths.

Add a per-call resolver, following the established "respect an existing
test monkeypatch of the constant, otherwise re-resolve through
get_hermes_home()" pattern so the existing test seam in
tests/gateway/test_hooks.py keeps working unmodified.
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools 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 P2 Medium — degraded but workaround exists labels Jul 1, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the import-time path pattern and preserving the existing HOOKS_DIR monkeypatch seam. The resolver itself matches the established cache-directory pattern, but it is not wired into the actual multiplex hook lifecycle.

Problems

  • Gateway.start() calls self.hooks.discover_and_load() only once at gateway/run.py:7079, before secondary adapters start at gateway/run.py:7271. Therefore the new resolver at gateway/hooks.py:123 only discovers the startup profile's hooks.
  • Secondary adapters route into shared _handle_message (gateway/run.py:8588-8592), and agent events emit through shared self.hooks (gateway/run.py:11752). The startup profile's handlers would still receive secondary-profile contexts.
  • The new tests construct HookRegistry instances directly under overrides, so they do not cover this startup-and-dispatch path.

Suggested changes

  • Make hook registries profile-aware in the real multiplex lifecycle: load them under each profile scope and route emits by source.profile.
  • Add a gateway-level multiplex regression proving a secondary profile invokes only its own hook.

Automated hermes-sweeper review.

Comment thread gateway/hooks.py
self._register_builtin_hooks()

if not HOOKS_DIR.exists():
hooks_dir = _resolve_hooks_dir()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This resolver is only effective if discovery occurs under each profile scope. On current main, Gateway.start() calls self.hooks.discover_and_load() once at gateway/run.py:7079, before _start_secondary_profile_adapters() at line 7271, so this still resolves only the startup profile's directory in the real multiplex path.

@teknium1 teknium1 added sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/profiles Multi-profile isolation, HERMES_HOME scoping labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/profiles Multi-profile isolation, HERMES_HOME scoping comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants