MCP: Cap /api/v1/mcp body size and shorten session idle timeout #5024

Signed-off-by: Michael Mayer <michael@photoprism.app>

internal/api/mcp.go

@@ -1,19 +1,32 @@
 package api
 
 import (
+	"errors"
+	"io"
 	"log/slog"
 	"net/http"
 	"os"
+	"sync/atomic"
 	"time"
 
 	"github.com/gin-gonic/gin"
 	sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp"
 
 	"github.com/photoprism/photoprism/internal/auth/acl"
+	"github.com/photoprism/photoprism/internal/event"
 	"github.com/photoprism/photoprism/internal/mcp"
 	"github.com/photoprism/photoprism/internal/photoprism/get"
+	"github.com/photoprism/photoprism/pkg/i18n"
+	"github.com/photoprism/photoprism/pkg/log/status"
 )
 
+// McpSessionTimeout configures the idle lifetime of MCP Streamable HTTP
+// sessions. It is shorter than a typical IDE editing window so that sessions
+// abandoned without a DELETE tear-down do not accumulate SDK-side bookkeeping
+// for long periods; active clients renew the idle timer on every request, so
+// interactive use is unaffected.
+var McpSessionTimeout = 5 * time.Minute
+
 // ServeMCP registers the Model Context Protocol (MCP) Streamable HTTP
 // endpoint at /api/v1/mcp.
 //
@@ -22,8 +35,8 @@ import (
 //	@Tags		MCP
 //	@Accept		json
 //	@Produce	json
-//	@Success	200				{string}	string	"JSON-RPC 2.0 response"
-//	@Failure	401,403,404,429	{object}	i18n.Response
+//	@Success	200					{string}	string	"JSON-RPC 2.0 response"
+//	@Failure	401,403,404,413,429	{object}	i18n.Response
 //	@Router		/api/v1/mcp [post]
 func ServeMCP(router *gin.RouterGroup) {
 	if router == nil {
@@ -56,22 +69,26 @@ func ServeMCP(router *gin.RouterGroup) {
 
 	// Streamable HTTP handler. Warn-level logging keeps the default log
 	// quiet under normal operation while still surfacing SDK warnings.
-	// The 30-minute session timeout matches typical IDE idle windows.
+	// McpSessionTimeout bounds how long idle sessions linger; active
+	// clients renew the timer on every request, so interactive IDE use
+	// is unaffected while abandoned sessions free up promptly.
 	handler := sdkmcp.NewStreamableHTTPHandler(
 		func(r *http.Request) *sdkmcp.Server { return mcpServer },
 		&sdkmcp.StreamableHTTPOptions{
-			SessionTimeout: 30 * time.Minute,
+			SessionTimeout: McpSessionTimeout,
 			Logger:         slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})),
 		},
 	)
 
-	// mcpHandler authenticates each request and delegates to the SDK
-	// handler. In public mode Session() returns the default public session
-	// (treated as admin by the ACL), so the read-only tools registered
-	// today are reachable anonymously — intentional to support demo
-	// deployments such as demo.photoprism.app. Any future tool that
-	// touches per-user state, the database, or mutates anything MUST NOT
-	// be registered on this shared server without an additional per-tool
+	// mcpHandler authenticates each request, caps the JSON-RPC payload
+	// size before delegating to the SDK handler, and translates an
+	// oversized-body condition into a standard 413 response. In public
+	// mode Session() returns the default public session (treated as
+	// admin by the ACL), so the read-only tools registered today are
+	// reachable anonymously — intentional to support demo deployments
+	// such as demo.photoprism.app. Any future tool that touches
+	// per-user state, the database, or mutates anything MUST NOT be
+	// registered on this shared server without an additional per-tool
 	// gate; see internal/mcp/README.md for the recommended patterns.
 	mcpHandler := func(c *gin.Context) {
 		s := Auth(c, acl.ResourceMCP, acl.ActionView)
@@ -82,7 +99,35 @@ func ServeMCP(router *gin.RouterGroup) {
 			return
 		}
 
-		handler.ServeHTTP(c.Writer, c.Request)
+		// Reject oversized requests up front when Content-Length is
+		// known; chunked or unknown-length bodies fall through to
+		// MaxBytesReader below.
+		if c.Request.ContentLength > MaxMCPRequestBytes {
+			event.AuditWarn([]string{ClientIP(c), "session %s", "mcp", "request body too large", status.Failed}, s.RefID)
+			AbortRequestTooLarge(c, i18n.ErrBadRequest)
+			return
+		}
+
+		// Cap the request body size before the upstream SDK reads it
+		// via io.ReadAll. LimitRequestBodyBytes swaps c.Request.Body
+		// for an http.MaxBytesReader, matching the shared pattern used
+		// by every other JSON API handler; mcpLimitReader then tracks
+		// whether the cap was exceeded so the response-writer wrapper
+		// can rewrite the SDK's 400 "failed to read body" into the
+		// standard 413 we use for oversized JSON bodies elsewhere.
+		LimitRequestBodyBytes(c, MaxMCPRequestBytes)
+
+		tripped := &atomic.Bool{}
+		c.Request.Body = &mcpLimitReader{ReadCloser: c.Request.Body, tripped: tripped}
+		writer := &mcpLimitWriter{ResponseWriter: c.Writer, tripped: tripped}
+
+		handler.ServeHTTP(writer, c.Request)
+
+		// Audit the rewritten 413 so the audit log reflects the final
+		// status the client received, not the SDK's internal 400.
+		if writer.suppress.Load() {
+			event.AuditWarn([]string{ClientIP(c), "session %s", "mcp", "request body too large", status.Failed}, s.RefID)
+		}
 	}
 
 	// Streamable HTTP uses POST for requests, GET for the event stream,
@@ -92,3 +137,60 @@ func ServeMCP(router *gin.RouterGroup) {
 	router.GET("/mcp", mcpHandler)
 	router.DELETE("/mcp", mcpHandler)
 }
+
+// mcpLimitReader wraps an http.MaxBytesReader-bounded body and records
+// whether the read failed with *http.MaxBytesError so the paired writer
+// can translate the SDK's 400 response into the standard 413.
+type mcpLimitReader struct {
+	io.ReadCloser
+	tripped *atomic.Bool
+}
+
+// Read delegates to the wrapped body and flips the shared flag when the
+// MaxBytesReader cap is exceeded.
+func (r *mcpLimitReader) Read(p []byte) (int, error) {
+	n, err := r.ReadCloser.Read(p)
+
+	if err != nil {
+		var maxBytesErr *http.MaxBytesError
+		if errors.As(err, &maxBytesErr) {
+			r.tripped.Store(true)
+		}
+	}
+
+	return n, err
+}
+
+// mcpLimitWriter wraps the outgoing response so the SDK's 400 "failed to
+// read body" is rewritten to 413 when the body cap was exceeded. Body
+// writes that arrive after the rewrite are suppressed so the SDK's
+// internal error phrasing does not replace PhotoPrism's standard 413
+// payload.
+type mcpLimitWriter struct {
+	gin.ResponseWriter
+	tripped  *atomic.Bool
+	suppress atomic.Bool
+}
+
+// WriteHeader rewrites 400 to 413 when the request body exceeded the cap
+// and marks the response body as suppressed so subsequent Write calls
+// cannot leak the SDK's 400 payload.
+func (w *mcpLimitWriter) WriteHeader(statusCode int) {
+	if statusCode == http.StatusBadRequest && w.tripped.Load() {
+		statusCode = http.StatusRequestEntityTooLarge
+		w.suppress.Store(true)
+	}
+
+	w.ResponseWriter.WriteHeader(statusCode)
+}
+
+// Write forwards payload bytes unless the response is a rewritten 413,
+// in which case the SDK's body is silently dropped so the final response
+// stays consistent with AbortRequestTooLarge.
+func (w *mcpLimitWriter) Write(b []byte) (int, error) {
+	if w.suppress.Load() {
+		return len(b), nil
+	}
+
+	return w.ResponseWriter.Write(b)
+}

internal/api/mcp_test.go

@@ -1,11 +1,14 @@
 package api
 
 import (
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"sync/atomic"
 	"testing"
 
+	"github.com/gin-gonic/gin"
 	"github.com/stretchr/testify/assert"
 
 	"github.com/photoprism/photoprism/internal/config"
@@ -117,6 +120,57 @@ func TestServeMCP(t *testing.T) {
 		r := mcpPost(app, `{"jsonrpc":"2.0","id":1,"method":"initialize"}`, authToken, "")
 		assert.Equal(t, http.StatusForbidden, r.Code)
 	})
+	t.Run("RequestTooLargeContentLength", func(t *testing.T) {
+		// The handler must reject oversized POST bodies with a 413 before
+		// the MCP SDK reads them into memory. This branch covers the
+		// early-reject path that fires when the client sends a
+		// Content-Length header larger than MaxMCPRequestBytes.
+		app, router, _ := NewApiTest()
+		prepareMCPTest(t)
+		ServeMCP(router)
+
+		authToken := AuthenticateAdmin(app, router)
+
+		req, _ := http.NewRequest(http.MethodPost, "/api/v1/mcp", strings.NewReader(""))
+		header.SetAuthorization(req, authToken)
+		req.Header.Set(header.ContentType, header.ContentTypeJson)
+		req.Header.Set(header.Accept, header.ContentTypeJson+", "+header.ContentTypeEventStream)
+		req.ContentLength = MaxMCPRequestBytes + 1
+
+		w := httptest.NewRecorder()
+		app.ServeHTTP(w, req)
+
+		assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code)
+	})
+	t.Run("RequestTooLargeStreamed", func(t *testing.T) {
+		// MaxBytesReader plus the response-writer wrapper must convert the
+		// SDK's 400 "failed to read body" into the standard 413 when the
+		// client streams a body larger than the cap without a
+		// Content-Length header. This path is what protects deployments
+		// that sit behind proxies using chunked transfer encoding.
+		app, router, _ := NewApiTest()
+		prepareMCPTest(t)
+		ServeMCP(router)
+
+		authToken := AuthenticateAdmin(app, router)
+
+		oversizedBody := strings.Repeat("a", int(MaxMCPRequestBytes)+1024)
+		req, _ := http.NewRequest(http.MethodPost, "/api/v1/mcp", strings.NewReader(oversizedBody))
+		header.SetAuthorization(req, authToken)
+		req.Header.Set(header.ContentType, header.ContentTypeJson)
+		req.Header.Set(header.Accept, header.ContentTypeJson+", "+header.ContentTypeEventStream)
+		// Force chunked transfer encoding semantics by clearing the
+		// Content-Length signal, matching the behavior of a streaming
+		// upstream proxy or client that sends Transfer-Encoding: chunked.
+		req.ContentLength = -1
+
+		w := httptest.NewRecorder()
+		app.ServeHTTP(w, req)
+
+		assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code)
+		// The rewritten response must not leak the SDK's 400 phrasing.
+		assert.NotContains(t, w.Body.String(), "failed to read body")
+	})
 	t.Run("InitializeAndCallTools", func(t *testing.T) {
 		app, router, _ := NewApiTest()
 		prepareMCPTest(t)
@@ -148,3 +202,72 @@ func TestServeMCP(t *testing.T) {
 		assert.Contains(t, w4.Body.String(), "matches")
 	})
 }
+
+// TestMcpLimitReader exercises the body wrapper that sets the shared
+// "tripped" flag when http.MaxBytesReader returns its sentinel error.
+func TestMcpLimitReader(t *testing.T) {
+	t.Run("FlagsOnMaxBytesError", func(t *testing.T) {
+		rec := httptest.NewRecorder()
+		body := io.NopCloser(strings.NewReader("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"))
+		tripped := &atomic.Bool{}
+		r := &mcpLimitReader{
+			ReadCloser: http.MaxBytesReader(rec, body, 4),
+			tripped:    tripped,
+		}
+		buf := make([]byte, 16)
+		n, err := r.Read(buf)
+		assert.Less(t, n, 16)
+		assert.Error(t, err)
+		assert.True(t, tripped.Load())
+	})
+	t.Run("NoFlagOnNormalRead", func(t *testing.T) {
+		rec := httptest.NewRecorder()
+		body := io.NopCloser(strings.NewReader("hello"))
+		tripped := &atomic.Bool{}
+		r := &mcpLimitReader{
+			ReadCloser: http.MaxBytesReader(rec, body, 1024),
+			tripped:    tripped,
+		}
+		buf := make([]byte, 64)
+		n, _ := r.Read(buf)
+		assert.Equal(t, 5, n)
+		assert.False(t, tripped.Load())
+	})
+}
+
+// TestMcpLimitWriter exercises the response-writer wrapper that translates
+// the SDK's 400 into a 413 once the paired reader has tripped.
+func TestMcpLimitWriter(t *testing.T) {
+	t.Run("RewritesBadRequestWhenTripped", func(t *testing.T) {
+		rec := httptest.NewRecorder()
+		c, _ := gin.CreateTestContext(rec)
+		tripped := &atomic.Bool{}
+		tripped.Store(true)
+		w := &mcpLimitWriter{ResponseWriter: c.Writer, tripped: tripped}
+		w.WriteHeader(http.StatusBadRequest)
+		_, _ = w.Write([]byte("failed to read body\n"))
+		assert.Equal(t, http.StatusRequestEntityTooLarge, c.Writer.Status())
+		assert.True(t, w.suppress.Load())
+		// The SDK-provided body must not reach the client on the 413
+		// rewrite path; only the status code is preserved.
+		assert.NotContains(t, rec.Body.String(), "failed to read body")
+	})
+	t.Run("PassesThroughWhenNotTripped", func(t *testing.T) {
+		c, _ := gin.CreateTestContext(httptest.NewRecorder())
+		tripped := &atomic.Bool{}
+		w := &mcpLimitWriter{ResponseWriter: c.Writer, tripped: tripped}
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte("ok"))
+		assert.Equal(t, http.StatusOK, c.Writer.Status())
+		assert.False(t, w.suppress.Load())
+	})
+	t.Run("LeavesNonBadRequestAlone", func(t *testing.T) {
+		c, _ := gin.CreateTestContext(httptest.NewRecorder())
+		tripped := &atomic.Bool{}
+		tripped.Store(true)
+		w := &mcpLimitWriter{ResponseWriter: c.Writer, tripped: tripped}
+		w.WriteHeader(http.StatusInternalServerError)
+		assert.Equal(t, http.StatusInternalServerError, c.Writer.Status())
+		assert.False(t, w.suppress.Load())
+	})
+}

internal/api/request_limits.go

@@ -35,6 +35,10 @@ const (
 	MaxMultipartOverheadBytes int64 = 1024 * 1024
 	// MaxAvatarUploadBytes bounds avatar uploads including multipart overhead.
 	MaxAvatarUploadBytes int64 = 20000000 + MaxMultipartOverheadBytes
+	// MaxMCPRequestBytes bounds Model Context Protocol (MCP) JSON-RPC payloads.
+	// The upstream SDK reads the full POST body into memory via io.ReadAll, so
+	// this cap must be enforced at the handler boundary before dispatch.
+	MaxMCPRequestBytes int64 = MaxMutationRequestBytes
 )
 
 // LimitRequestBodyBytes caps the readable request body size for the current handler.

internal/api/swagger.json

@@ -8694,6 +8694,12 @@
                             "$ref": "#/definitions/i18n.Response"
                         }
                     },
+                    "413": {
+                        "description": "Request Entity Too Large",
+                        "schema": {
+                            "$ref": "#/definitions/i18n.Response"
+                        }
+                    },
                     "429": {
                         "description": "Too Many Requests",
                         "schema": {

internal/mcp/README.md

@@ -1,6 +1,6 @@
 ## PhotoPrism MCP Server
 
-**Last Updated:** April 18, 2026
+**Last Updated:** April 20, 2026
 
 > See `specs/platform/mcp.md` for the canonical specification, including the rationale for the user-access policy and the role/grant matrix per edition.
 
@@ -10,6 +10,8 @@
   - CLI: `photoprism mcp serve` (stdio, no auth; development and testing)
   - HTTP: `POST/GET/DELETE /api/v1/mcp` (Streamable HTTP, authenticated). Can be disabled via `--disable-mcp` / `PHOTOPRISM_DISABLE_MCP` / `DisableMCP` so the route responds with the standard 404 when an operator does not want the endpoint exposed. The flag is also surfaced to the frontend through `ClientConfig.Disable.MCP` (`disable.mcp`), letting the UI hide MCP-related controls while the endpoint is off.
 - **Authorization:** HTTP endpoint enforces the `ResourceMCP` ACL (admin plus the API client roles in every edition, manager in Pro/Portal); anonymous access is permitted in public mode for the currently registered read-only tools.
+- **Request Body Cap:** HTTP POST bodies are bounded at `MaxMCPRequestBytes` (currently `MaxMutationRequestBytes`, 256 KiB). Oversized requests receive the standard `413 Request Entity Too Large` response before the upstream SDK reads the body. Early rejection via `Content-Length` protects against large known-size payloads; `http.MaxBytesReader` plus a response-writer wrapper handle chunked bodies. The wrapper translates the SDK's internal `400 "failed to read body"` into a consistent `413` and suppresses the SDK's error phrasing so it does not leak to clients.
+- **Session Timeout:** Streamable HTTP sessions idle out after `McpSessionTimeout` (5 minutes by default). Active clients renew the idle timer on every JSON-RPC request, so interactive IDE use is unaffected; sessions abandoned without the `DELETE` tear-down free up promptly instead of lingering.
 - Read-only resources:
   - `photoprism://config-options`
   - `photoprism://search-filters`
@@ -191,6 +193,8 @@ The MCP handler does not install a custom rate limiter — there is no per-endpo
 
 A per-endpoint limiter (via `limiter.Auth` / `limiter.Login` / `limiter.AbortJSON`) is only worth adding when MCP grows write-capable tools or endpoints that warrant stricter throttling than the generic IP limiter — for example, anything that mutates state or that triggers expensive backend work.
 
+CE and Plus deployments that expose the endpoint to untrusted networks should enforce per-IP request limits at the reverse proxy. The application-level `McpSessionTimeout` (5 minutes) and `MaxMCPRequestBytes` (256 KiB) bound the per-session memory footprint and per-request allocation, but they do not throttle request frequency; a proxy rule (for example nginx `limit_req_zone` or Traefik `rateLimit` middleware) is the recommended companion control in builds without a generic HTTP limiter.
+
 ### Scope Plumbing
 
 `mcp` is the canonical scope token for `ResourceMCP`. The relevant pieces: