From d91c8bbf0deadf3944f3be22683be660887cd98e Mon Sep 17 00:00:00 2001 From: Omid Astaraki Date: Mon, 16 Mar 2026 18:50:10 +0000 Subject: [PATCH 1/3] fix(mcp): replace SeerrAPIKeyMiddleware with MCPAuthMiddleware X-Api-Key on the MCP endpoint is for client authentication (checked against --auth-token), not for forwarding the Seerr API key. The Seerr API key is always read from the application config (seerr.api_key). Remove SeerrAPIKeyMiddleware, which incorrectly extracted X-Api-Key from incoming requests and injected it as the Seerr API key. Replace the bearer-only auth middleware with MCPAuthMiddleware, which accepts the configured auth token via Authorization: Bearer, X-Api-Key header, or ?api_key= query parameter (when --allow-api-key-query-param is set). Clients configured with no auth token no longer receive spurious 401s. --- cmd/mcp/flags.go | 2 +- cmd/mcp/serve.go | 55 ++++-------- tests/mcp_api_key_middleware_test.go | 125 +++++++++++++++------------ tests/mcp_serve_test.go | 3 +- 4 files changed, 91 insertions(+), 94 deletions(-) diff --git a/cmd/mcp/flags.go b/cmd/mcp/flags.go index ffe0ecd..7b7eaf9 100644 --- a/cmd/mcp/flags.go +++ b/cmd/mcp/flags.go @@ -73,7 +73,7 @@ var ServeFlags = []FlagDef{ ViperKey: "mcp.allow_api_key_query_param", Default: "false", IsBool: true, - Usage: "Accept the Seerr API key via the api_key query parameter in addition to the X-Api-Key header (HTTP transport only)", + Usage: "Accept the MCP auth token via the api_key query parameter in addition to headers (HTTP transport only)", }, { Name: "log-file", diff --git a/cmd/mcp/serve.go b/cmd/mcp/serve.go index 685f08a..aaf9451 100644 --- a/cmd/mcp/serve.go +++ b/cmd/mcp/serve.go @@ -159,11 +159,9 @@ func runServe(_ *cobra.Command, args []string) error { httpHandler := server.NewStreamableHTTPServer(s) handler := http.Handler(httpHandler) - // Per-request Seerr API key injection (header or optional query param). - handler = SeerrAPIKeyMiddleware(allowAPIKeyQueryParam, handler) handler = httpLoggingMiddleware(handler) if authToken != "" { - handler = bearerAuthMiddleware(authToken, handler) + handler = MCPAuthMiddleware(authToken, allowAPIKeyQueryParam, handler) } // The health endpoint must be reachable without auth, so register it in // a top-level mux that sits above the bearer-auth middleware. @@ -243,50 +241,31 @@ func corsMiddleware(next http.Handler) http.Handler { }) } -func bearerAuthMiddleware(token string, next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - authHeader := r.Header.Get("Authorization") - const prefix = "Bearer " - if !strings.HasPrefix(authHeader, prefix) { - http.Error(w, "Unauthorized", http.StatusUnauthorized) - return - } - provided := strings.TrimPrefix(authHeader, prefix) - if subtle.ConstantTimeCompare([]byte(provided), []byte(token)) != 1 { - http.Error(w, "Unauthorized", http.StatusUnauthorized) - return - } - next.ServeHTTP(w, r) - }) -} - -// SeerrAPIKeyMiddleware extracts the Seerr API key from the incoming request -// and injects it into the request context for use by MCP tool handlers. +// MCPAuthMiddleware authenticates incoming MCP HTTP requests against token. // -// The key is read from the X-Api-Key request header first. When -// allowQueryParam is true the middleware also accepts the key via the -// api_key query parameter; the header takes precedence when both are present. +// Accepted credential locations (in precedence order): +// 1. Authorization: Bearer +// 2. X-Api-Key: +// 3. ?api_key= query parameter — only when allowQueryParam is true. // -// If neither location provides a key the middleware responds with 401. -func SeerrAPIKeyMiddleware(allowQueryParam bool, next http.Handler) http.Handler { +// Requests that do not supply a matching credential are rejected with 401. +// The Seerr API key used for outbound Seerr calls is always read from the +// application config (seerr.api_key) and is never sourced from this middleware. +func MCPAuthMiddleware(token string, allowQueryParam bool, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var apiKey string - if v := r.Header.Get("X-Api-Key"); v != "" { - apiKey = v + var provided string + if v := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer "); v != r.Header.Get("Authorization") { + provided = v + } else if v := r.Header.Get("X-Api-Key"); v != "" { + provided = v } else if allowQueryParam { - if v := r.URL.Query().Get("api_key"); v != "" { - apiKey = v - } + provided = r.URL.Query().Get("api_key") } - if apiKey != "" { - ctx := context.WithValue(r.Context(), apiKeyCtxKey, apiKey) - r = r.Clone(ctx) - } else { + if subtle.ConstantTimeCompare([]byte(provided), []byte(token)) != 1 { http.Error(w, "Unauthorized", http.StatusUnauthorized) return } - next.ServeHTTP(w, r) }) } diff --git a/tests/mcp_api_key_middleware_test.go b/tests/mcp_api_key_middleware_test.go index d5bf279..8124fb9 100644 --- a/tests/mcp_api_key_middleware_test.go +++ b/tests/mcp_api_key_middleware_test.go @@ -10,100 +10,119 @@ import ( "github.com/stretchr/testify/assert" ) -// captureContextKey is a helper that records the API key injected into the -// request context by the middleware. -func captureContextKey(t *testing.T) (handler http.Handler, captured *string) { - t.Helper() - s := "" - h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - s, _ = r.Context().Value(cmdmcp.APIKeyContextKey).(string) +const testAuthToken = "super-secret" + +// newAuthTestHandler returns an inner handler that records whether it was called. +func newAuthTestHandler(called *bool) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *called = true w.WriteHeader(http.StatusOK) }) - return h, &s } -func TestSeerrAPIKeyMiddleware_headerOnly(t *testing.T) { - inner, captured := captureContextKey(t) - handler := cmdmcp.SeerrAPIKeyMiddleware(false, inner) +func TestMCPAuthMiddleware_bearerToken(t *testing.T) { + var called bool + handler := cmdmcp.MCPAuthMiddleware(testAuthToken, false, newAuthTestHandler(&called)) - req := httptest.NewRequest(http.MethodGet, "/mcp", nil) - req.Header.Set("X-Api-Key", "header-key") + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + req.Header.Set("Authorization", "Bearer "+testAuthToken) rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) assert.Equal(t, http.StatusOK, rec.Code) - assert.Equal(t, "header-key", *captured) + assert.True(t, called) } -func TestSeerrAPIKeyMiddleware_queryParamOnly(t *testing.T) { - inner, captured := captureContextKey(t) - handler := cmdmcp.SeerrAPIKeyMiddleware(true, inner) +func TestMCPAuthMiddleware_xApiKeyHeader(t *testing.T) { + var called bool + handler := cmdmcp.MCPAuthMiddleware(testAuthToken, false, newAuthTestHandler(&called)) - req := httptest.NewRequest(http.MethodGet, "/mcp?api_key=qparam-key", nil) + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + req.Header.Set("X-Api-Key", testAuthToken) rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) assert.Equal(t, http.StatusOK, rec.Code) - assert.Equal(t, "qparam-key", *captured) + assert.True(t, called) } -func TestSeerrAPIKeyMiddleware_headerPrecedenceOverQueryParam(t *testing.T) { - inner, captured := captureContextKey(t) - handler := cmdmcp.SeerrAPIKeyMiddleware(true, inner) +func TestMCPAuthMiddleware_queryParam(t *testing.T) { + var called bool + handler := cmdmcp.MCPAuthMiddleware(testAuthToken, true, newAuthTestHandler(&called)) - req := httptest.NewRequest(http.MethodGet, "/mcp?api_key=qparam-key", nil) - req.Header.Set("X-Api-Key", "header-key") + req := httptest.NewRequest(http.MethodPost, "/mcp?api_key="+testAuthToken, nil) rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) assert.Equal(t, http.StatusOK, rec.Code) - assert.Equal(t, "header-key", *captured, "header must take precedence over query param") + assert.True(t, called) } -func TestSeerrAPIKeyMiddleware_queryParamDisabled_ignoresQueryParam(t *testing.T) { - var innerCalled bool - inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - innerCalled = true - w.WriteHeader(http.StatusOK) - }) - // allowQueryParam=false; query param present but should not satisfy auth. - handler := cmdmcp.SeerrAPIKeyMiddleware(false, inner) +func TestMCPAuthMiddleware_queryParamDisabled_returns401(t *testing.T) { + var called bool + handler := cmdmcp.MCPAuthMiddleware(testAuthToken, false, newAuthTestHandler(&called)) - req := httptest.NewRequest(http.MethodGet, "/mcp?api_key=qparam-key", nil) + req := httptest.NewRequest(http.MethodPost, "/mcp?api_key="+testAuthToken, nil) rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) - // No header, query param disabled — must return 401. assert.Equal(t, http.StatusUnauthorized, rec.Code) - assert.False(t, innerCalled) + assert.False(t, called) } -func TestSeerrAPIKeyMiddleware_neitherPresent_returns401(t *testing.T) { - var innerCalled bool - inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - innerCalled = true - w.WriteHeader(http.StatusOK) - }) - handler := cmdmcp.SeerrAPIKeyMiddleware(true, inner) +func TestMCPAuthMiddleware_wrongToken_returns401(t *testing.T) { + var called bool + handler := cmdmcp.MCPAuthMiddleware(testAuthToken, true, newAuthTestHandler(&called)) - req := httptest.NewRequest(http.MethodGet, "/mcp", nil) + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + req.Header.Set("X-Api-Key", "wrong-token") rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) assert.Equal(t, http.StatusUnauthorized, rec.Code) - assert.False(t, innerCalled) + assert.False(t, called) } -func TestSeerrAPIKeyMiddleware_queryParam_sensitiveValueNotLogged(t *testing.T) { - // This test ensures SafeLogQuery redacts the api_key value from query strings. +func TestMCPAuthMiddleware_noCredentials_returns401(t *testing.T) { + var called bool + handler := cmdmcp.MCPAuthMiddleware(testAuthToken, true, newAuthTestHandler(&called)) + + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.False(t, called) +} + +// TestMCPAuthMiddleware_noAuthToken verifies that when no --auth-token is +// configured the middleware is not applied and requests pass through without +// any credential check. This is the no-auth / --no-auth scenario. +func TestMCPAuthMiddleware_noAuthToken_requestPassesThrough(t *testing.T) { + // When authToken is empty the serve command does not wrap the handler with + // MCPAuthMiddleware at all. The Seerr API key is always read from the app + // config (seerr.api_key) and never sourced from the incoming request. + var called bool + inner := newAuthTestHandler(&called) + + // Simulate the no-auth path: handler is NOT wrapped with MCPAuthMiddleware. + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + rec := httptest.NewRecorder() + inner.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.True(t, called) +} + +func TestMCPAuthMiddlewareIsTheOnlyPerRequestAuthMechanism(t *testing.T) { + // Compile-time check that MCPAuthMiddleware exists and has the expected signature. + _ = cmdmcp.MCPAuthMiddleware +} + +func TestMCPLogQueryRedaction(t *testing.T) { + // Ensure SafeLogQuery redacts the api_key value from query strings. redacted := cmdmcp.SafeLogQuery("api_key=secret123&page=1") assert.NotContains(t, redacted, "secret123") assert.Contains(t, redacted, "api_key={redacted}") assert.Contains(t, redacted, "page=1") } - -func TestSeerrAPIKeyMiddlewareIsTheOnlyPerRequestKeyMechanism(t *testing.T) { - // SeerrAPIKeyMiddleware must compile and be the sole per-request API key - // injection mechanism — path-based routing has been removed. - _ = cmdmcp.SeerrAPIKeyMiddleware -} diff --git a/tests/mcp_serve_test.go b/tests/mcp_serve_test.go index 6f1d23a..317cd76 100644 --- a/tests/mcp_serve_test.go +++ b/tests/mcp_serve_test.go @@ -416,8 +416,7 @@ func TestMCPBlocklistListHandler(t *testing.T) { func TestAPIKeyContextPropagation(t *testing.T) { // Verify that an API key injected into the context is forwarded to the - // Seerr API as the X-Api-Key header, matching how SeerrAPIKeyMiddleware - // injects keys for downstream tool handlers. + // Seerr API as the X-Api-Key header by tool handlers. var receivedAPIKey string ts, cleanup := newMCPTestServer(func(w http.ResponseWriter, r *http.Request) { From 9216a02677ebbcde43ee2afa3f236b89b092a903 Mon Sep 17 00:00:00 2001 From: Omid Astaraki Date: Sun, 29 Mar 2026 12:24:39 +0100 Subject: [PATCH 2/3] fix(mcp): add seasons parameter to request_create tool Overseerr requires a seasons field for tv requests. The MCP tool was missing this parameter, causing a server-side crash when the API tried to filter an undefined seasons array. --- cmd/mcp/tools_request.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/cmd/mcp/tools_request.go b/cmd/mcp/tools_request.go index 4754a49..b7b07ee 100644 --- a/cmd/mcp/tools_request.go +++ b/cmd/mcp/tools_request.go @@ -3,6 +3,8 @@ package mcp import ( "context" "encoding/json" + "strconv" + "strings" api "seerr-cli/pkg/api" @@ -44,6 +46,7 @@ func registerRequestTools(s *server.MCPServer) { mcp.WithIdempotentHintAnnotation(false), mcp.WithString("mediaType", mcp.Required(), mcp.Description("Media type: movie or tv")), mcp.WithNumber("mediaId", mcp.Required(), mcp.Description("TMDB media ID")), + mcp.WithString("seasons", mcp.Description(`Seasons to request (required for tv): "all" or comma-separated numbers e.g. "1,2,3"`)), mcp.WithBoolean("is4k", mcp.Description("Request 4K version")), ), RequestCreateHandler(), @@ -159,6 +162,23 @@ func RequestCreateHandler() server.ToolHandlerFunc { return nil, err } body := api.NewRequestPostRequest(mediaType, float32(mediaId)) + if seasons := req.GetString("seasons", ""); seasons != "" { + if seasons == "all" { + s := "all" + body.SetSeasons(api.StringAsRequestPostRequestSeasons(&s)) + } else { + parts := strings.Split(seasons, ",") + nums := make([]float32, 0, len(parts)) + for _, p := range parts { + n, err := strconv.ParseInt(strings.TrimSpace(p), 10, 64) + if err != nil { + return mcp.NewToolResultError("invalid season number: " + p), nil + } + nums = append(nums, float32(n)) + } + body.SetSeasons(api.ArrayOfFloat32AsRequestPostRequestSeasons(&nums)) + } + } if is4k := req.GetBool("is4k", false); is4k { body.Is4k = &is4k } From 09980c7a6907e1b667ed265e7deeee350642a446 Mon Sep 17 00:00:00 2001 From: Omid Astaraki Date: Sun, 29 Mar 2026 12:26:49 +0100 Subject: [PATCH 3/3] test(mcp): add request_create handler tests for seasons parameter Cover movie requests (no seasons), tv with "all", tv with specific season numbers, and invalid season input producing a tool error. --- tests/mcp_serve_test.go | 113 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/tests/mcp_serve_test.go b/tests/mcp_serve_test.go index 317cd76..1b3ec8e 100644 --- a/tests/mcp_serve_test.go +++ b/tests/mcp_serve_test.go @@ -3,6 +3,7 @@ package tests import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" "strings" @@ -412,6 +413,118 @@ func TestMCPBlocklistListHandler(t *testing.T) { assert.Contains(t, text, `"results"`) } +func TestMCPRequestCreateMovieHandler(t *testing.T) { + var capturedBody []byte + ts, cleanup := newMCPTestServer(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/api/v1/request" { + capturedBody, _ = io.ReadAll(r.Body) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"id":1,"status":1,"media":{"mediaType":"movie"}}`)) + return + } + w.WriteHeader(http.StatusNotFound) + }) + defer cleanup() + _ = ts + + handler := cmdmcp.RequestCreateHandler() + result := callTool(t, handler, map[string]any{ + "mediaType": "movie", + "mediaId": float64(550), + }) + text := resultText(t, result) + + assert.Contains(t, text, `"id"`) + + var body map[string]interface{} + require.NoError(t, json.Unmarshal(capturedBody, &body)) + assert.Equal(t, "movie", body["mediaType"]) + assert.NotContains(t, body, "seasons", "movie requests must not send a seasons field") +} + +func TestMCPRequestCreateTVSeasonsAll(t *testing.T) { + var capturedBody []byte + ts, cleanup := newMCPTestServer(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/api/v1/request" { + capturedBody, _ = io.ReadAll(r.Body) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"id":2,"status":1,"media":{"mediaType":"tv"}}`)) + return + } + w.WriteHeader(http.StatusNotFound) + }) + defer cleanup() + _ = ts + + handler := cmdmcp.RequestCreateHandler() + result := callTool(t, handler, map[string]any{ + "mediaType": "tv", + "mediaId": float64(1399), + "seasons": "all", + }) + text := resultText(t, result) + + assert.Contains(t, text, `"id"`) + + var body map[string]interface{} + require.NoError(t, json.Unmarshal(capturedBody, &body)) + assert.Equal(t, "all", body["seasons"], "seasons=all must be forwarded as the string \"all\"") +} + +func TestMCPRequestCreateTVSpecificSeasons(t *testing.T) { + var capturedBody []byte + ts, cleanup := newMCPTestServer(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/api/v1/request" { + capturedBody, _ = io.ReadAll(r.Body) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"id":3,"status":1,"media":{"mediaType":"tv"}}`)) + return + } + w.WriteHeader(http.StatusNotFound) + }) + defer cleanup() + _ = ts + + handler := cmdmcp.RequestCreateHandler() + result := callTool(t, handler, map[string]any{ + "mediaType": "tv", + "mediaId": float64(1399), + "seasons": "1,2", + }) + text := resultText(t, result) + + assert.Contains(t, text, `"id"`) + + var body map[string]interface{} + require.NoError(t, json.Unmarshal(capturedBody, &body)) + seasons, ok := body["seasons"].([]interface{}) + require.True(t, ok, "seasons must be a JSON array") + assert.Equal(t, []interface{}{float64(1), float64(2)}, seasons) +} + +func TestMCPRequestCreateInvalidSeason(t *testing.T) { + ts, cleanup := newMCPTestServer(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + defer cleanup() + _ = ts + + handler := cmdmcp.RequestCreateHandler() + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]any{ + "mediaType": "tv", + "mediaId": float64(1399), + "seasons": "1,abc", + } + result, err := handler(context.Background(), req) + require.NoError(t, err) + require.NotNil(t, result) + assert.True(t, result.IsError, "invalid season number must produce a tool error") +} + // --- API key context propagation test --- func TestAPIKeyContextPropagation(t *testing.T) {