diff --git a/internal/api/mcp.go b/internal/api/mcp.go index b7ad78eef..adf753c15 100644 --- a/internal/api/mcp.go +++ b/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) +} diff --git a/internal/api/mcp_test.go b/internal/api/mcp_test.go index 03cad0fc7..4ccc56f1c 100644 --- a/internal/api/mcp_test.go +++ 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()) + }) +} diff --git a/internal/api/request_limits.go b/internal/api/request_limits.go index 905947224..1f7ef38ac 100644 --- a/internal/api/request_limits.go +++ b/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. diff --git a/internal/api/swagger.json b/internal/api/swagger.json index 8ae4b8744..d69bbb511 100644 --- a/internal/api/swagger.json +++ b/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": { diff --git a/internal/mcp/README.md b/internal/mcp/README.md index fdd520555..fa94864a8 100644 --- a/internal/mcp/README.md +++ b/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: