Skip to content

Commit 57dd394

Browse files
committed
fix: address PR review concerns for tenant session isolation
- Split tenant mismatch log: WARNING with session ID only, DEBUG for tenant values to avoid leaking sensitive data at default log levels - _set_tenant/_reset_tenant helpers always set/reset the contextvar unconditionally, avoiding subtle bugs when tenant is None - Wrap all blocking-session tests in try/finally to ensure stop.set() runs even on assertion failures, preventing test hangs
1 parent d048944 commit 57dd394

2 files changed

Lines changed: 42 additions & 36 deletions

File tree

src/mcp/server/streamable_http_manager.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,11 @@ async def _handle_stateful_request(self, scope: Scope, receive: Receive, send: S
201201
session_tenant = self._session_tenants.get(request_mcp_session_id)
202202
request_tenant = tenant_id_var.get()
203203
if session_tenant is not None and request_tenant != session_tenant:
204-
logger.warning(
205-
f"Tenant mismatch for session {request_mcp_session_id[:64]}: "
206-
f"session bound to '{session_tenant}', request from '{request_tenant}'"
204+
logger.warning("Tenant mismatch for session %s", request_mcp_session_id[:64])
205+
logger.debug(
206+
"Tenant mismatch detail: session bound to '%s', request from '%s'",
207+
session_tenant,
208+
request_tenant,
207209
)
208210
error_response = JSONRPCError(
209211
jsonrpc="2.0",

tests/server/test_streamable_http_manager.py

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -478,18 +478,17 @@ async def _mock_receive() -> dict[str, Any]: # pragma: no cover
478478

479479

480480
def _set_tenant(tenant: str | None) -> Any:
481-
"""Set tenant_id_var if tenant is not None; return the token (or None)."""
481+
"""Set tenant_id_var and return the reset token."""
482482
from mcp.shared._context import tenant_id_var
483483

484-
return tenant_id_var.set(tenant) if tenant is not None else None
484+
return tenant_id_var.set(tenant)
485485

486486

487487
def _reset_tenant(token: Any) -> None:
488-
"""Reset tenant_id_var if a token was set."""
488+
"""Reset tenant_id_var to its previous value."""
489489
from mcp.shared._context import tenant_id_var
490490

491-
if token is not None:
492-
tenant_id_var.reset(token)
491+
tenant_id_var.reset(token)
493492

494493

495494
async def _create_session_blocking(
@@ -540,10 +539,11 @@ async def test_tenant_mismatch_returns_404(running_manager: tuple[StreamableHTTP
540539
"""A request from tenant-b cannot access a session created by tenant-a."""
541540
manager, app = running_manager
542541
stop = anyio.Event()
543-
session_id = await _create_session_blocking(manager, app, stop, tenant="tenant-a")
544-
545-
assert await _access_session(manager, session_id, tenant="tenant-b") == 404
546-
stop.set()
542+
try:
543+
session_id = await _create_session_blocking(manager, app, stop, tenant="tenant-a")
544+
assert await _access_session(manager, session_id, tenant="tenant-b") == 404
545+
finally:
546+
stop.set()
547547

548548

549549
@pytest.mark.anyio
@@ -553,40 +553,43 @@ async def test_two_tenants_cannot_access_each_others_sessions(
553553
"""Two tenants each create a session; neither can access the other's."""
554554
manager, app = running_manager
555555
stop = anyio.Event()
556-
557-
session_a = await _create_session_blocking(manager, app, stop, tenant="tenant-a")
558-
session_b = await _create_session_blocking(manager, app, stop, tenant="tenant-b")
559-
assert session_a != session_b
560-
561-
# Tenant-a tries to access tenant-b's session → 404
562-
assert await _access_session(manager, session_b, tenant="tenant-a") == 404
563-
# Tenant-b tries to access tenant-a's session → 404
564-
assert await _access_session(manager, session_a, tenant="tenant-b") == 404
565-
stop.set()
556+
try:
557+
session_a = await _create_session_blocking(manager, app, stop, tenant="tenant-a")
558+
session_b = await _create_session_blocking(manager, app, stop, tenant="tenant-b")
559+
assert session_a != session_b
560+
561+
# Tenant-a tries to access tenant-b's session → 404
562+
assert await _access_session(manager, session_b, tenant="tenant-a") == 404
563+
# Tenant-b tries to access tenant-a's session → 404
564+
assert await _access_session(manager, session_a, tenant="tenant-b") == 404
565+
finally:
566+
stop.set()
566567

567568

568569
@pytest.mark.anyio
569570
async def test_same_tenant_can_reuse_session(running_manager: tuple[StreamableHTTPSessionManager, Server]):
570571
"""A request from the same tenant can access its own session."""
571572
manager, app = running_manager
572573
stop = anyio.Event()
573-
session_id = await _create_session_blocking(manager, app, stop, tenant="tenant-a")
574-
575-
status = await _access_session(manager, session_id, tenant="tenant-a")
576-
assert status != 404, "Same tenant should be able to reuse its own session"
577-
stop.set()
574+
try:
575+
session_id = await _create_session_blocking(manager, app, stop, tenant="tenant-a")
576+
status = await _access_session(manager, session_id, tenant="tenant-a")
577+
assert status != 404, "Same tenant should be able to reuse its own session"
578+
finally:
579+
stop.set()
578580

579581

580582
@pytest.mark.anyio
581583
async def test_no_tenant_session_allows_any_access(running_manager: tuple[StreamableHTTPSessionManager, Server]):
582584
"""Sessions created without a tenant (no auth) allow access from any request."""
583585
manager, app = running_manager
584586
stop = anyio.Event()
585-
session_id = await _create_session_blocking(manager, app, stop, tenant=None)
586-
587-
status = await _access_session(manager, session_id, tenant="tenant-a")
588-
assert status != 404, "Session without tenant binding should allow access from any tenant"
589-
stop.set()
587+
try:
588+
session_id = await _create_session_blocking(manager, app, stop, tenant=None)
589+
status = await _access_session(manager, session_id, tenant="tenant-a")
590+
assert status != 404, "Session without tenant binding should allow access from any tenant"
591+
finally:
592+
stop.set()
590593

591594

592595
@pytest.mark.anyio
@@ -596,10 +599,11 @@ async def test_unauthenticated_request_cannot_access_tenant_session(
596599
"""A request with no tenant cannot access a session bound to a tenant."""
597600
manager, app = running_manager
598601
stop = anyio.Event()
599-
session_id = await _create_session_blocking(manager, app, stop, tenant="tenant-a")
600-
601-
assert await _access_session(manager, session_id, tenant=None) == 404
602-
stop.set()
602+
try:
603+
session_id = await _create_session_blocking(manager, app, stop, tenant="tenant-a")
604+
assert await _access_session(manager, session_id, tenant=None) == 404
605+
finally:
606+
stop.set()
603607

604608

605609
@pytest.mark.anyio

0 commit comments

Comments
 (0)