You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2026/05/01 00:58:59 UTC

(superset) branch auth-issue created (now 737b99b5a98)

This is an automated email from the ASF dual-hosted git repository.

eschutho pushed a change to branch auth-issue
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 737b99b5a98 fix(mcp): clear stale thread-local DB session in sync tool wrapper

This branch includes the following new commits:

     new 737b99b5a98 fix(mcp): clear stale thread-local DB session in sync tool wrapper

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 01/01: fix(mcp): clear stale thread-local DB session in sync tool wrapper

Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

eschutho pushed a commit to branch auth-issue
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 737b99b5a985dccefdac44566d0b52556484f4b1
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Fri May 1 00:58:40 2026 +0000

    fix(mcp): clear stale thread-local DB session in sync tool wrapper
    
    Thread pool workers reuse threads across requests. SQLAlchemy's
    ScopedSession is scoped by thread (not ContextVar), so a prior request's
    session may still be bound to a different tenant's DB engine when a new
    MCP tool call lands on the same worker thread.
    
    The workspace context (g.__CURRENT_WORKSPACE_NAME__) is correctly
    propagated from the HTTP request via ContextVar, but the thread-local
    session binding is stale — causing MultiTenantException when
    load_user_with_relationships queries the DB.
    
    Fix: call db.session.remove() at the top of sync_wrapper before user
    setup, so the next DB access creates a fresh session bound to the correct
    workspace engine. async_wrapper is unaffected (runs in the event loop
    thread, no thread-local stale state).
    
    Co-Authored-By: Claude Sonnet 4.6 <no...@anthropic.com>
---
 superset/mcp_service/auth.py                       | 12 +++++++++
 .../mcp_service/test_auth_user_resolution.py       | 31 ++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/superset/mcp_service/auth.py b/superset/mcp_service/auth.py
index ef7c202dae6..7ce0ef3f618 100644
--- a/superset/mcp_service/auth.py
+++ b/superset/mcp_service/auth.py
@@ -632,6 +632,18 @@ def mcp_auth_hook(tool_func: F) -> F:  # noqa: C901
         @functools.wraps(tool_func)
         def sync_wrapper(*args: Any, **kwargs: Any) -> Any:
             with _get_app_context_manager():
+                # Clear any stale thread-local SQLAlchemy session before user lookup.
+                # Thread pool workers reuse threads across requests; db.session is
+                # scoped by thread (not ContextVar), so a prior request's session may
+                # still be bound to a different tenant's DB engine. Removing it here
+                # ensures the next DB access creates a fresh session bound to the
+                # correct engine for the current request's workspace context.
+                # The workspace context itself (g.__CURRENT_WORKSPACE_NAME__) is
+                # correctly propagated from the request via ContextVar — only the
+                # thread-local session binding is stale.
+                from superset.extensions import db
+
+                db.session.remove()
                 user = _setup_user_context()
 
                 # No Flask context - this is a FastMCP internal operation
diff --git a/tests/unit_tests/mcp_service/test_auth_user_resolution.py b/tests/unit_tests/mcp_service/test_auth_user_resolution.py
index 658e131f18b..7cc737666ac 100644
--- a/tests/unit_tests/mcp_service/test_auth_user_resolution.py
+++ b/tests/unit_tests/mcp_service/test_auth_user_resolution.py
@@ -372,6 +372,37 @@ def test_mcp_auth_hook_preserves_g_user_in_request_context(app) -> None:
     assert result == "middleware_user"
 
 
+def test_mcp_auth_hook_removes_stale_db_session_in_sync_wrapper(app) -> None:
+    """sync_wrapper calls db.session.remove() before user setup.
+
+    Thread pool workers reuse threads across requests; db.session is
+    thread-local and may be bound to a different tenant's DB engine from a
+    prior request. Removing it before user lookup ensures a fresh session is
+    created for the current request's workspace context.
+    """
+    fresh_user = _make_mock_user("fresh")
+
+    def dummy_tool():
+        """Dummy tool."""
+        return g.user.username
+
+    wrapped = mcp_auth_hook(dummy_tool)
+
+    with app.test_request_context():
+        g.user = fresh_user
+        with (
+            patch(
+                "superset.mcp_service.auth.get_user_from_request",
+                return_value=fresh_user,
+            ),
+            patch("superset.extensions.db") as mock_db,
+        ):
+            result = wrapped()
+            mock_db.session.remove.assert_called_once()
+
+    assert result == "fresh"
+
+
 # -- default_user_resolver --