[Server] Answer 503 when the session owner cannot be read

A failed read no longer sends the owner to re-initialize, and a store outage no longer escapes from a DELETE.

Author: chr-hertel

CHANGELOG.md

@@ -30,7 +30,7 @@ All notable changes to `mcp/sdk` will be documented in this file.
 * [BC Break] Answer `server/discover` with `DiscoverHandler`, which a handler added through `Builder::addRequestHandler()` can replace, and dispatch `RequestEvent`, `ResponseEvent` and `ErrorEvent` for it. A `StatelessProtocol` built without the `Builder` needs `DiscoverHandler` and a `MessageFactory` with `DiscoverRequest` to answer it.
 * Add a `binder` argument to `Builder::setRequestState()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; a token without a subject fails closed, and a `subject` resolver identifies users by another claim. `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes.
 * Rotate the `requestState` signing key without failing the calls in flight: `Builder::setRequestState()` and `RequestStateCodec` take a list of keys, the first signing and every key verifying. The wire format is unchanged, and a single key works as before.
-* [BC Break] Bind handshake-era sessions to the principal that opened them, as the security best practices recommend. A request of another principal with the session's id is answered with `404`, like an unknown session, and its `DELETE` does not end the session. Requests to sessions opened before the upgrade are answered with `404` too, so clients open new ones. The principal is `AccessToken::getPrincipal()`: the token's issuer, client and subject. An application that identifies users another way sets the `StreamableHttpTransport::PRINCIPAL_ATTRIBUTE` request attribute to a non-empty string. `Protocol::processInput()`, `BaseTransport::handleMessage()`, `BaseTransport::handleSessionEnd()`, `StreamableHttpTransport::handlePostRequest()` and `StreamableHttpTransport::handleDeleteRequest()` take a `?string $principal` parameter, as do the `TransportInterface::onMessage()` and `onSessionEnd()` listeners as last argument.
+* [BC Break] Bind handshake-era sessions to the principal that opened them: `AccessToken::getPrincipal()`, or the `StreamableHttpTransport::PRINCIPAL_ATTRIBUTE` request attribute when set. Requests of another principal, and to sessions opened before the upgrade, are answered with `404`, and to a session the store cannot read with `503`. `Protocol::processInput()`, the transports' `handleMessage()`, `handleSessionEnd()`, `handlePostRequest()` and `handleDeleteRequest()`, and the `onMessage()` and `onSessionEnd()` listeners take a `?string $principal` as last argument.
 
 ### Client
 

src/Server/Protocol.php

@@ -70,6 +70,8 @@ class Protocol
 
     public const SESSION_LOGGING_LEVEL = '_mcp.logging_level';
 
+    private const SESSION_NOT_FOUND_MESSAGE = 'Session not found or has expired.';
+
     /** Session key for the fingerprint of the principal that opened the session, null when it was opened without one */
     private const SESSION_PRINCIPAL = '_mcp.principal';
 
@@ -261,7 +263,15 @@ private function doProcessInput(TransportInterface $transport, string $input, ?U
         if ($takesLock) {
             // Who opened the session is checked before waiting for the lock too, so that another
             // principal can neither tell the session is busy nor keep it busy.
-            if (!$this->servesPrincipal($this->sessionManager->createWithId($sessionId), $principal)) {
+            try {
+                $serves = $this->servesPrincipal($this->sessionManager->createWithId($sessionId), $principal);
+            } catch (SessionStoreException $e) {
+                $this->refuseUnavailableStore($transport, $sessionId, self::findResponseId($input), $e);
+
+                return;
+            }
+
+            if (!$serves) {
                 $this->refuseOtherPrincipal($transport, $sessionId);
 
                 return;
@@ -1168,27 +1178,29 @@ private function resolveSession(TransportInterface $transport, ?Uuid $sessionId,
         try {
             $exists = $this->sessionManager->exists($sessionId);
         } catch (SessionStoreException $e) {
-            // Not a 404: the session may well exist, so the client should retry instead of re-initializing.
-            $this->logger->error('Session store is unavailable.', [
-                'session_id' => $sessionId->toRfc4122(),
-                'exception' => $e,
-            ]);
-            $error = Error::forInternalError('Session store is unavailable.', self::findRequestId($messages));
-            $this->sendResponse($transport, $error, null, ['status_code' => 503]);
+            $this->refuseUnavailableStore($transport, $sessionId, self::findRequestId($messages), $e);
 
             return null;
         }
 
         if (!$exists) {
-            $error = Error::forInvalidRequest('Session not found or has expired.');
+            $error = Error::forInvalidRequest(self::SESSION_NOT_FOUND_MESSAGE);
             $this->sendResponse($transport, $error, null, ['status_code' => 404]);
 
             return null;
         }
 
         $session = $this->sessionManager->createWithId($sessionId);
 
-        if (!$this->servesPrincipal($session, $principal)) {
+        try {
+            $serves = $this->servesPrincipal($session, $principal);
+        } catch (SessionStoreException $e) {
+            $this->refuseUnavailableStore($transport, $sessionId, self::findRequestId($messages), $e);
+
+            return null;
+        }
+
+        if (!$serves) {
             $this->refuseOtherPrincipal($transport, $sessionId);
 
             return null;
@@ -1200,11 +1212,35 @@ private function resolveSession(TransportInterface $transport, ?Uuid $sessionId,
     /**
      * Whether the session serves the principal: the one that opened it, or no principal for a session opened without one.
      *
-     * A session that does not record who opened it, for example because the store failed to read it, serves no one.
+     * A session that does not record who opened it serves no one.
+     *
+     * @throws SessionStoreException when the session exists but its data cannot be read
      */
     private function servesPrincipal(SessionInterface $session, ?string $principal): bool
     {
-        return $session->get(self::SESSION_PRINCIPAL, false) === self::fingerprint($principal);
+        if ($session->get(self::SESSION_PRINCIPAL, false) === self::fingerprint($principal)) {
+            return true;
+        }
+
+        if ([] === $session->all()) {
+            throw new SessionStoreException(\sprintf('Session "%s" could not be read.', $session->getId()->toRfc4122()));
+        }
+
+        return false;
+    }
+
+    /**
+     * Not a 404: the session may well exist, so the client should retry instead of re-initializing.
+     *
+     * @param TransportInterface<mixed> $transport
+     */
+    private function refuseUnavailableStore(TransportInterface $transport, Uuid $sessionId, string|int|null $requestId, SessionStoreException $exception): void
+    {
+        $this->logger->error('Session store is unavailable.', [
+            'session_id' => $sessionId->toRfc4122(),
+            'exception' => $exception,
+        ]);
+        $this->sendResponse($transport, Error::forInternalError('Session store is unavailable.', $requestId), null, ['status_code' => 503]);
     }
 
     /**
@@ -1217,7 +1253,7 @@ private function refuseOtherPrincipal(TransportInterface $transport, Uuid $sessi
         ]);
 
         // Answered like an unknown session, so that another principal cannot tell the session exists.
-        $error = Error::forInvalidRequest('Session not found or has expired.');
+        $error = Error::forInvalidRequest(self::SESSION_NOT_FOUND_MESSAGE);
         $this->sendResponse($transport, $error, null, ['status_code' => 404]);
     }
 
@@ -1245,6 +1281,8 @@ private static function findRequestId(array $messages): string|int|null
      * Ends a session at the request of a client, which must be the principal that opened it.
      *
      * @return bool false when the session does not exist, or is another principal's
+     *
+     * @throws SessionStoreException when who opened the session cannot be established; the session is not ended
      */
     private function endSession(Uuid $sessionId, ?string $principal = null): bool
     {

src/Server/Transport/BaseTransport.php

@@ -11,6 +11,7 @@
 
 namespace Mcp\Server\Transport;
 
+use Mcp\Exception\SessionStoreException;
 use Mcp\Schema\JsonRpc\Error;
 use Mcp\Schema\JsonRpc\Response;
 use Mcp\Server\Authorization\AccessToken;
@@ -154,6 +155,8 @@ protected function handleMessage(string $payload, ?Uuid $sessionId, ?AccessToken
 
     /**
      * @return bool false when the listener reported that the session does not exist
+     *
+     * @throws SessionStoreException when the listener cannot tell who opened the session
      */
     protected function handleSessionEnd(?Uuid $sessionId, ?string $principal = null): bool
     {

src/Server/Transport/StreamableHttpTransport.php

@@ -13,6 +13,7 @@
 
 use Http\Discovery\Psr17FactoryDiscovery;
 use Mcp\Exception\InvalidArgumentException;
+use Mcp\Exception\SessionStoreException;
 use Mcp\Schema\Enum\ProtocolVersion;
 use Mcp\Schema\JsonRpc\Error;
 use Mcp\Server\Authorization\AccessToken;
@@ -254,7 +255,15 @@ protected function handleDeleteRequest(?string $principal = null): ResponseInter
             return $this->createErrorResponse(Error::forInvalidRequest(self::SESSION_HEADER.' header is required.'), 400);
         }
 
-        if (!$this->handleSessionEnd($this->sessionId, $principal)) {
+        try {
+            $ended = $this->handleSessionEnd($this->sessionId, $principal);
+        } catch (SessionStoreException $e) {
+            $this->logger->error('Session store is unavailable.', ['session_id' => $this->sessionId->toRfc4122(), 'exception' => $e]);
+
+            return $this->createErrorResponse(Error::forInternalError('Session store is unavailable.'), 503);
+        }
+
+        if (!$ended) {
             return $this->createErrorResponse(Error::forInvalidRequest('Session not found or has expired.'), 404);
         }
 

src/Server/Transport/TransportInterface.php

@@ -84,6 +84,7 @@ public function onMessage(callable $listener): void;
      *
      * The transport calls this when a client disconnects or explicitly ends their session.
      * The principal identifies who ended it, if the transport authenticates requests.
+     * The listener throws a {@see \Mcp\Exception\SessionStoreException} when it cannot tell who opened the session.
      *
      * @param callable(Uuid $sessionId, ?string $principal): bool $listener The callback function to execute when destroying a session, returning false when the session does not exist
      */

tests/Unit/Server/Transport/SessionBindingTest.php

@@ -11,6 +11,7 @@
 
 namespace Mcp\Tests\Unit\Server\Transport;
 
+use Mcp\Exception\SessionStoreException;
 use Mcp\Schema\Enum\ProtocolVersion;
 use Mcp\Server;
 use Mcp\Server\Authorization\AccessToken;
@@ -22,6 +23,7 @@
 use Mcp\Server\Transport\Http\OAuth\AuthorizationTokenValidatorInterface;
 use Mcp\Server\Transport\Http\OAuth\ProtectedResourceMetadata;
 use Mcp\Server\Transport\StreamableHttpTransport;
+use Mcp\Tests\Unit\Server\Session\Fixture\RecordingSessionLock;
 use Nyholm\Psr7\Factory\Psr17Factory;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
@@ -106,19 +108,56 @@ public function testDeleteOfAnotherPrincipalKeepsTheSession(): void
         $this->assertSame(404, $this->call($session, 'alice.ide.1')->getStatusCode());
     }
 
-    public function testSessionThatCannotBeReadServesNoRequestWithoutPrincipal(): void
+    public function testSessionThatCannotBeReadIsRetriedByEveryone(): void
     {
         $session = $this->initialize('alice.ide.1');
         $passthrough = [new PassthroughMiddleware()];
 
         $this->store->unreadable = true;
-        $this->assertSame(404, $this->call($session, null, $passthrough)->getStatusCode());
-        $this->serve($this->request('DELETE', '', null, $session), $passthrough);
+        $this->assertSame(503, $this->call($session, 'alice.ide.1')->getStatusCode());
+        $this->assertSame(503, $this->call($session, null, $passthrough)->getStatusCode());
+        $this->assertSame(503, $this->serve($this->request('DELETE', '', 'alice.ide.1', $session))->getStatusCode());
+        $this->assertSame(503, $this->serve($this->request('DELETE', '', null, $session), $passthrough)->getStatusCode());
         $this->store->unreadable = false;
 
         $this->assertSame('alice', $this->text($this->call($session, 'alice.ide.1')));
     }
 
+    public function testSessionThatCannotBeReadIsRetriedWithASessionLock(): void
+    {
+        $log = [];
+        $server = Server::builder()
+            ->setServerInfo('test', '1.0.0')
+            ->setSession($this->store)
+            ->setSessionLock(new RecordingSessionLock($log))
+            ->build();
+        $session = $this->server($server, $this->request('POST', $this->initializeRequest(), 'alice.ide.1'))->getHeaderLine('Mcp-Session-Id');
+        $log = [];
+
+        $this->store->unreadable = true;
+        $this->assertSame(503, $this->server($server, $this->request('POST', $this->toolCall(), 'alice.ide.1', $session))->getStatusCode());
+        $this->assertSame([], $log);
+    }
+
+    public function testDeleteDuringAStoreOutageKeepsTheSession(): void
+    {
+        $session = $this->initialize('alice.ide.1');
+
+        $this->store->unavailable = true;
+        $this->assertSame(503, $this->serve($this->request('DELETE', '', 'alice.ide.1', $session))->getStatusCode());
+        $this->store->unavailable = false;
+
+        $this->assertSame('alice', $this->text($this->call($session, 'alice.ide.1')));
+    }
+
+    public function testSessionFromBeforeTheUpgradeIsNotFound(): void
+    {
+        $session = Uuid::v4();
+        $this->store->write($session, json_encode(['initialized' => true], \JSON_THROW_ON_ERROR));
+
+        $this->assertSame(404, $this->call($session->toRfc4122(), 'alice.ide.1')->getStatusCode());
+    }
+
     public function testPrincipalSetByTheApplicationBindsTheSession(): void
     {
         $middleware = new class implements MiddlewareInterface {
@@ -240,6 +279,11 @@ private function serveWith(ServerRequestInterface $request, array $attributes):
         return $this->serve($request, [new PassthroughMiddleware()]);
     }
 
+    private function server(Server $server, ServerRequestInterface $request): ResponseInterface
+    {
+        return $server->run(new StreamableHttpTransport($request, $this->factory, $this->factory, middleware: [$this->authorization()]));
+    }
+
     private function initialize(string $token): string
     {
         $session = $this->serve($this->request('POST', $this->initializeRequest(), $token))->getHeaderLine('Mcp-Session-Id');
@@ -341,10 +385,21 @@ private function text(ResponseInterface $response): string
 
 /**
  * Finds sessions but fails to read them while $unreadable is set, like a cache that is reachable but fails on get().
+ * Fails to look them up while $unavailable is set, like a cache that is down.
  */
 final class UnreadableSessionStore extends InMemorySessionStore
 {
     public bool $unreadable = false;
+    public bool $unavailable = false;
+
+    public function exists(Uuid $id): bool
+    {
+        if ($this->unavailable) {
+            throw new SessionStoreException('Session store is unavailable.');
+        }
+
+        return parent::exists($id);
+    }
 
     public function read(Uuid $id): string|false
     {