fix(mcp): isolate OAuth credentials by authenticated requester

cli-config.yaml.example

@@ -1297,6 +1297,15 @@ platform_toolsets:
 # Each server's tools are automatically discovered and registered.
 # See website/docs/user-guide/features/mcp.md for full documentation.
 #
+# OAuth identity isolation (shared gateway):
+#   mcp:
+#     oauth:
+#       identity_mode: shared    # default. One token per profile+server.
+#       # identity_mode: per_user  # scope OAuth to the authenticated requester
+#                                  # (Slack/Discord/Telegram/…). Typos are
+#                                  # rejected; they never fall back to shared.
+#                                  # Direct CLI cannot pick a user's token.
+#
 # Stdio servers (spawn a subprocess):
 #   command: the executable to run
 #   args: command-line arguments

cli.py

@@ -14166,13 +14166,15 @@ def _confirm_and_reload_mcp(self, cmd_original: str = "") -> None:
             self._reload_mcp()
 
     def _reload_mcp(self):
-        """Reload MCP servers: disconnect all, re-read config.yaml, reconnect.
+        """Reload MCP servers: recycle connections, re-read config.yaml, reconnect.
 
-        After reconnecting, refreshes the agent's tool list so the model
-        sees the updated tools on the next turn.
+        Unbound CLI/TUI (and shared mode) disconnect every live server.
+        A bound ``per_user`` gateway request leaves other principals'
+        OAuth sessions up. After reconnecting, refreshes the agent's tool
+        list so the model sees the updated tools on the next turn.
         """
         try:
-            from tools.mcp_tool import shutdown_mcp_servers, discover_mcp_tools, _servers, _lock
+            from tools.mcp_tool import reload_mcp_connections, discover_mcp_tools, _servers, _lock
 
             # Capture old server names
             with _lock:
@@ -14181,8 +14183,7 @@ def _reload_mcp(self):
             if not self._command_running:
                 print("🔄 Reloading MCP servers...")
 
-            # Shutdown existing connections
-            shutdown_mcp_servers()
+            reload_mcp_connections()
 
             # Reconnect (reads config.yaml fresh)
             new_tools = discover_mcp_tools()

docs/rfc/requester-scoped-mcp-oauth.md

@@ -0,0 +1,95 @@
+# RFC (Revised): Requester-Scoped MCP OAuth Isolation
+
+- **Status:** Revised after adversarial code review
+- **Date:** 2026-08-26
+- **Upstream issue:** [NousResearch/hermes-agent#78174](https://github.com/NousResearch/hermes-agent/issues/78174)
+- **Related (out of scope):** [#78169](https://github.com/NousResearch/hermes-agent/issues/78169) headless consent UX
+- **Do not rebase:** [#79449](https://github.com/NousResearch/hermes-agent/pull/79449)
+
+This document **supersedes** the draft RFC where they disagree. The draft's
+threat model, invariants I1–I18, and "fresh native scope" decision stand.
+The sections below are the review findings and the locked implementation
+decisions.
+
+## Adversarial findings (must change)
+
+1. **`scope_id` is empty on most adapters.** Telegram, WhatsApp, Feishu,
+   Mattermost, SMS, IRC, and Discord DMs do not populate tenant scope.
+   Requiring a non-empty `scope_id` would fail-closed almost every
+   non-Slack/Guild path. Empty bound `scope_id` is canonicalized to `"~"`.
+   `_UNSET` (not bound) is still a hard miss.
+
+2. **`get_session_env` falls back to `os.environ` when `_UNSET`.** A
+   per-user principal MUST use a dedicated bound-only getter. Env vars
+   are not a credential selector.
+
+3. **Gateway/CLI/cron call `discover_mcp_tools()` at process start** with
+   no human principal, and will connect OAuth servers from
+   `mcp-tokens/<server>.json` under `suppress_interactive_oauth`. In
+   `per_user`, OAuth-protected servers MUST NOT pick a human credential at
+   startup. They are implicitly lazy until a request with a bound
+   principal arrives. Tool *names* may still be registered from the
+   unscoped schema cache so the model can see them.
+
+4. **`handle_401` / unpinned `HermesTokenStorage` re-resolve ambient
+   `get_hermes_home()`.** Scope and `hermes_home` MUST be captured at
+   provider/connection construction and passed explicitly into refresh,
+   401, reconnect, and disk-watch.
+
+5. **`_lazy_server_configs` is popped on first connect.** In `per_user`,
+   Alice's first use must not delete the lazy config Bob still needs.
+
+6. **In-memory Hermes tool registry is process-global.** Closing the
+   confused-deputy hole does not require per-session tool schemas (that
+   would also fight prompt caching if done mid-conversation). Live calls
+   use the requester's connection; a tool Bob does not actually have fails
+   at the MCP server. Disk `cacheScope=private` entries are still scoped.
+
+7. **No metrics/telemetry in this change** (project policy: no outbound
+   telemetry without a user-facing opt-in). Structured log fields only,
+   with opaque principal keys, never tokens.
+
+8. **Do not implement #78169** (consent URL delivery, paste-back, gateway
+   message injection). Do not add `hermes mcp login --user`.
+
+9. **OAuth isolation applies to `auth: oauth` servers.** Stdio and static
+   header servers keep process-level connections. Stdio env credentials
+   remain shared; that is an explicit non-goal.
+
+10. **Subagents inherit the parent's bound principal** via ContextVar copy.
+    Cron blanks identity and therefore fail-closes in `per_user`.
+
+11. **`_run_on_mcp_loop` copies the MCP loop thread's ContextVars, not the
+    agent's.** `run_coroutine_threadsafe` creates the task inside the loop
+    thread. Without an explicit wrap, `_capture_oauth_identity` (and any
+    `get_bound_session_principal()` inside connect) would fail closed on a
+    live gateway request, or worse inherit a stale loop-thread principal.
+    The scheduling thread's bound principal MUST be re-applied inside the
+    scheduled task (same hop as `HERMES_HOME` override). Identity is also
+    pinned on `MCPServerTask` in `start()` before `ensure_future(run())`
+    so reconnects never re-resolve ambient identity.
+
+## Locked decisions
+
+| Topic | Decision |
+|---|---|
+| Default | `mcp.oauth.identity_mode: shared` (absent key = shared) |
+| Invalid mode | Reject (`per-user`, typos). Never downgrade to shared |
+| Principal | `(v1, platform, scope_id, user_id)` from bound ContextVars only |
+| Empty `scope_id` | Canonical `"~"` when the field is bound-or-absent-as-empty |
+| Persistence key | `u-v1-` + SHA-256 of canonical JSON array; never raw IDs in paths |
+| Shared layout | Unchanged: `$HERMES_HOME/mcp-tokens/<server>.*` |
+| Per-user layout | `$HERMES_HOME/mcp-tokens/by-user/<persistence_key>/<server>.*` |
+| Migration | Never assign a legacy shared token to a requester |
+| Registry key | `server_name` in shared; `server_name + \\x1f + persistence_key` in per_user |
+| Lookup | Exact key only. No "any connection named github" fallback |
+| CLI / TUI / desktop / cron in `per_user` | Fail closed with an actionable error when no bound principal |
+| MCP loop hop | `_run_on_mcp_loop` re-binds the caller's principal; `start()` pins it on the connection |
+| `hermes mcp remove` | Admin: may delete that server's artifacts across `by-user/*` |
+| Idle eviction / per-server override / HMAC path keys | Deferred |
+
+## Core invariant
+
+A request authenticated as principal A MUST NOT read, refresh, select,
+reuse, reconnect, disconnect, or otherwise affect any credential-bearing
+object belonging to principal B.

gateway/run.py

@@ -23904,23 +23904,27 @@ async def _restore_telegram_topic_session(self, event: MessageEvent, raw_session
 
 
     async def _execute_mcp_reload(self, event: MessageEvent) -> str:
-        """Actually disconnect, reconnect, and notify MCP tool changes.
+        """Recycle MCP connections, reconnect, and notify tool changes.
 
         Split out from ``_handle_reload_mcp_command`` so the confirmation
         wrapper can invoke the same path whether the user confirmed via
         button, text reply, or has the confirm gate disabled.
+
+        Uses ``reload_mcp_connections`` so a ``per_user`` requester cannot
+        tear down another principal's live OAuth session.
         """
         loop = asyncio.get_running_loop()
         try:
-            from tools.mcp_tool import shutdown_mcp_servers, discover_mcp_tools, _servers, _lock
+            from tools.mcp_tool import reload_mcp_connections, discover_mcp_tools, _servers, _lock
 
             # Capture old server names before shutdown
             with _lock:
                 old_servers = set(_servers.keys())
 
-            # Read new config before shutting down, so we know what will be added/removed
-            # Shutdown existing connections
-            await loop.run_in_executor(None, shutdown_mcp_servers)
+            # Recycle connections under the /reload-mcp policy: full wipe in
+            # shared/unbound mode; in per_user, other principals' OAuth
+            # sessions stay up (Alice must not disconnect Bob).
+            await loop.run_in_executor(None, reload_mcp_connections)
 
             # Reconnect by discovering tools (reads config.yaml fresh)
             new_tools = await loop.run_in_executor(None, discover_mcp_tools)

gateway/session_context.py

@@ -38,7 +38,8 @@
 
 from contextlib import contextmanager
 from contextvars import ContextVar
-from typing import Any, Iterator
+from dataclasses import dataclass
+from typing import Any, Iterator, Optional
 
 # Sentinel to distinguish "never set in this context" from "explicitly set to empty".
 # When a contextvar holds _UNSET, we fall back to os.environ (CLI/cron compat).
@@ -390,6 +391,85 @@ def reset_session_vars() -> None:
         pass
 
 
+@dataclass(frozen=True, slots=True)
+class BoundSessionPrincipal:
+    """Trusted requester identity bound on the current task.
+
+    Returned only by :func:`get_bound_session_principal`. Values come from
+    ContextVars set by :func:`set_session_vars`; they never fall back to
+    ``os.environ``. ``scope_id`` may be empty on platforms that have no
+    tenant namespace (Telegram, Discord DMs, …).
+    """
+
+    platform: str
+    scope_id: str
+    user_id: str
+
+
+def _bound_session_str(var: ContextVar) -> Optional[str]:
+    """Return a ContextVar string only when it was explicitly bound.
+
+    ``None`` means the var is ``_UNSET`` (not bound in this task — env
+    fallback must not be consulted). An empty string means it was bound
+    empty via :func:`set_session_vars` / :func:`clear_session_vars`.
+    """
+    value = var.get()
+    if value is _UNSET:
+        return None
+    if value is None:
+        return ""
+    return str(value)
+
+
+def get_bound_session_principal() -> Optional[BoundSessionPrincipal]:
+    """Return the authenticated requester, or None if identity is not bound.
+
+    Unlike :func:`get_session_env`, this never reads ``os.environ``. A
+    missing platform or user_id (unset *or* bound-empty) yields ``None`` so
+    MCP OAuth ``per_user`` mode can fail closed instead of impersonating a
+    process-global leftover. Empty ``scope_id`` is allowed: many adapters
+    do not have a tenant namespace.
+    """
+    platform = _bound_session_str(_SESSION_PLATFORM)
+    user_id = _bound_session_str(_SESSION_USER_ID)
+    if platform is None or user_id is None:
+        return None
+    platform = platform.strip()
+    user_id = user_id.strip()
+    if not platform or not user_id:
+        return None
+    scope_raw = _bound_session_str(_SESSION_SCOPE_ID)
+    scope_id = "" if scope_raw is None else scope_raw.strip()
+    return BoundSessionPrincipal(
+        platform=platform, scope_id=scope_id, user_id=user_id
+    )
+
+
+@contextmanager
+def apply_bound_session_principal(
+    principal: BoundSessionPrincipal,
+) -> Iterator[None]:
+    """Re-bind a previously captured principal in this task, then restore.
+
+    Used to carry gateway identity onto the dedicated MCP event-loop thread.
+    ``run_coroutine_threadsafe`` copies that thread's context, not the
+    scheduling thread's, so OAuth ``per_user`` capture would otherwise see
+    no requester (or a stale one). Tokens are reset, not cleared to ``""``,
+    so the loop thread does not retain the principal after the call.
+    """
+    tokens = (
+        _SESSION_PLATFORM.set(principal.platform),
+        _SESSION_SCOPE_ID.set(principal.scope_id),
+        _SESSION_USER_ID.set(principal.user_id),
+    )
+    try:
+        yield
+    finally:
+        _SESSION_USER_ID.reset(tokens[2])
+        _SESSION_SCOPE_ID.reset(tokens[1])
+        _SESSION_PLATFORM.reset(tokens[0])
+
+
 def get_session_env(name: str, default: str = "") -> str:
     """Read a session context variable by its legacy ``HERMES_SESSION_*`` name.
 

hermes_cli/config_defaults.py

@@ -708,6 +708,15 @@
         # When disabled, the watcher still detects the change and prints
         # guidance to apply it deliberately via /reload-mcp.
         "auto_reload_on_config_change": True,
+        # MCP OAuth identity isolation. ``shared`` (default) keeps the
+        # historical one-token-per-profile layout. ``per_user`` scopes
+        # OAuth credentials, providers, and live connections to the
+        # authenticated gateway requester — required for a shared Slack /
+        # Discord / Telegram gateway. Typos are rejected; they must never
+        # silently fall back to shared. See docs/rfc/requester-scoped-mcp-oauth.md.
+        "oauth": {
+            "identity_mode": "shared",
+        },
     },
 
     # Tool-output truncation thresholds. When terminal output or a

hermes_cli/mcp_config.py

@@ -56,6 +56,29 @@ def _error(text: str):
     print(color(f"  ✗ {text}", Colors.RED))
 
 
+def _per_user_oauth_cli_block() -> Optional[str]:
+    """Return an error message when per_user mode has no bound requester.
+
+    Direct CLI / TUI / desktop / cron cannot pick a human OAuth token, and
+    there is no ``--user`` selector. Gateway sessions with a bound principal
+    are the intended ``per_user`` path.
+    """
+    from tools.mcp_oauth_identity import (
+        IDENTITY_MODE_PER_USER,
+        MissingRequesterIdentity,
+        configured_identity_mode,
+        resolve_mcp_oauth_scope,
+    )
+
+    if configured_identity_mode() != IDENTITY_MODE_PER_USER:
+        return None
+    try:
+        resolve_mcp_oauth_scope(uses_oauth=True)
+    except MissingRequesterIdentity as exc:
+        return str(exc)
+    return None
+
+
 def _confirm(question: str, default: bool = True) -> bool:
     default_str = "Y/n" if default else "y/N"
     try:
@@ -410,7 +433,11 @@ def _oauth_tokens_present(name: str) -> bool:
     """
     try:
         from tools.mcp_oauth import HermesTokenStorage
+        from tools.mcp_oauth_identity import MissingRequesterIdentity
+
         return HermesTokenStorage(name).has_cached_tokens()
+    except MissingRequesterIdentity:
+        return False
     except Exception as exc:  # pragma: no cover — defensive
         logger.debug("Could not check OAuth tokens for '%s': %s", name, exc)
         # Be permissive on unexpected errors: don't block a real success.
@@ -508,6 +535,16 @@ def cmd_mcp_add(args):
     # ── Authentication ────────────────────────────────────────────────
 
     if url and auth_type == "oauth":
+        blocked = _per_user_oauth_cli_block()
+        if blocked:
+            _error(blocked)
+            _info(
+                "mcp.oauth.identity_mode is per_user. Direct CLI cannot "
+                "complete OAuth for a gateway requester. There is no "
+                "--user selector."
+            )
+            return
+
         print()
         _info(f"Starting OAuth flow for '{name}'...")
         oauth_ok = False
@@ -664,9 +701,11 @@ def cmd_mcp_remove(args):
     # Clean up OAuth tokens if they exist — route through MCPOAuthManager so
     # any provider instance cached in the current process (e.g. from an
     # earlier `hermes mcp test` in the same session) is evicted too.
+    # ``all_identities=True`` is the admin path: removing the server from
+    # config deletes shared artifacts and every by-user namespace for it.
     try:
         from tools.mcp_oauth_manager import get_manager
-        get_manager().remove(name)
+        get_manager().remove(name, all_identities=True)
         _success("Cleaned up OAuth tokens")
     except Exception:
         pass
@@ -824,6 +863,16 @@ def _reauth_oauth_server(name: str, server_config: dict) -> bool:
         _info("Use `hermes mcp remove` + `hermes mcp add` to reconfigure auth.")
         return False
 
+    blocked = _per_user_oauth_cli_block()
+    if blocked:
+        _error(blocked)
+        _info(
+            "mcp.oauth.identity_mode is per_user. Direct CLI cannot "
+            "complete OAuth for a gateway requester. There is no "
+            "--user selector."
+        )
+        return False
+
     # Wipe both disk and in-memory cache so the next probe forces a fresh
     # OAuth flow.
     try:

tests/gateway/test_mcp_reload_refreshes_cached_agents.py

@@ -106,6 +106,7 @@ async def test_reload_mcp_refreshes_cached_agent_tools():
     ]
 
     with (
+        patch("tools.mcp_tool.reload_mcp_connections"),
         patch("tools.mcp_tool.shutdown_mcp_servers"),
         patch("tools.mcp_tool.discover_mcp_tools", return_value=["HassTurnOn", "HassTurnOff"]),
         patch.dict("tools.mcp_tool._servers", {"homeassistant": object()}, clear=True),
@@ -136,6 +137,7 @@ async def test_reload_mcp_handles_empty_agent_cache():
     assert len(runner._agent_cache) == 0
 
     with (
+        patch("tools.mcp_tool.reload_mcp_connections"),
         patch("tools.mcp_tool.shutdown_mcp_servers"),
         patch("tools.mcp_tool.discover_mcp_tools", return_value=[]),
         patch.dict("tools.mcp_tool._servers", {}, clear=True),
@@ -164,6 +166,7 @@ def _capture_get_tool_definitions(**kwargs):
         return [{"type": "function", "function": {"name": "refreshed"}}]
 
     with (
+        patch("tools.mcp_tool.reload_mcp_connections"),
         patch("tools.mcp_tool.shutdown_mcp_servers"),
         patch("tools.mcp_tool.discover_mcp_tools", return_value=["refreshed"]),
         patch.dict("tools.mcp_tool._servers", {"homeassistant": object()}, clear=True),