diff --git a/AGENT.md b/AGENT.md index 12f63f7..cf7b5db 100644 --- a/AGENT.md +++ b/AGENT.md @@ -33,30 +33,24 @@ env: - name: SEERR_MCP_AUTH_TOKEN description: >- Bearer token for MCP HTTP transport clients. When using HTTP transport, - at least one of SEERR_MCP_AUTH_TOKEN, SEERR_MCP_ROUTE_TOKEN, or - SEERR_MCP_NO_AUTH=true must be set. Not used for stdio transport. + at least one of SEERR_MCP_AUTH_TOKEN, SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM, + or SEERR_MCP_NO_AUTH=true must be set. Not used for stdio transport. required: false - - name: SEERR_MCP_ROUTE_TOKEN + - name: SEERR_MCP_NO_AUTH description: >- - Secret path prefix that replaces Bearer auth for clients that cannot send - custom headers (e.g. claude.ai remote MCP). Endpoint becomes - http:////mcp. + Set to "true" to disable all MCP HTTP authentication. Only use in trusted + environments where the endpoint is access-controlled by other means. required: false - - name: SEERR_MCP_NO_AUTH + - name: SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM description: >- - Set to "true" to disable all MCP HTTP authentication. Requires - SEERR_MCP_ROUTE_TOKEN to be set or explicit acknowledgement that the - endpoint is access-controlled by other means. + Set to "true" to accept the Seerr API key via the api_key query parameter + in addition to the X-Api-Key header. Useful for clients that cannot send + custom headers (e.g. claude.ai remote MCP). The MCP endpoint is always + /mcp; append ?api_key= to authenticate. HTTP transport only. required: false - name: SEERR_MCP_CORS description: Set to "true" to enable CORS headers for browser-based MCP clients (e.g. claude.ai) required: false - - name: SEERR_MCP_MULTI_TENANT - description: >- - Set to "true" to enable multi-tenant mode (HTTP transport only). The - endpoint becomes /{seerr-api-token}/mcp and the path segment is used as - the per-user Seerr API key instead of SEERR_API_KEY. - required: false - name: SEERR_MCP_TLS_CERT description: Path to a TLS certificate file for HTTPS on the MCP HTTP server required: false @@ -109,22 +103,21 @@ docker run --rm \ MCP endpoint: `http://localhost:8811/mcp` — set `Authorization: Bearer your-secret-token` in your MCP client. -For clients that cannot send custom headers (e.g. claude.ai remote MCP), use a secret path prefix: +For clients that cannot send custom headers (e.g. claude.ai remote MCP), use query parameter transport: ```bash docker run --rm \ -e SEERR_SERVER=http://your-seerr-instance:5055 \ -e SEERR_API_KEY=your-api-key \ - -e SEERR_MCP_ROUTE_TOKEN=your-secret-path \ - -e SEERR_MCP_NO_AUTH=true \ + -e SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM=true \ -e SEERR_MCP_CORS=true \ -p 8811:8811 \ ghcr.io/electather/seerr-cli:latest ``` -MCP endpoint: `http://localhost:8811/your-secret-path/mcp` — no auth header required. +MCP endpoint: `http://localhost:8811/mcp?api_key=your-api-key` — no auth header required. -At least one of `SEERR_MCP_AUTH_TOKEN`, `SEERR_MCP_ROUTE_TOKEN`, or `SEERR_MCP_NO_AUTH=true` must be set for HTTP transport. +At least one of `SEERR_MCP_AUTH_TOKEN`, `SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM`, or `SEERR_MCP_NO_AUTH=true` must be set for HTTP transport. ### docker-compose deployment @@ -402,32 +395,26 @@ seerr-cli mcp serve --transport http --addr :8811 --auth-token mysecrettoken Endpoint: `http://localhost:8811/mcp` — set `Authorization: Bearer mysecrettoken` in your client. -For clients that cannot send custom headers (e.g. claude.ai remote MCP), use a secret path prefix via `--route-token` (or `SEERR_MCP_ROUTE_TOKEN`): +For clients that cannot send custom headers (e.g. claude.ai remote MCP), use `--allow-api-key-query-param` (or `SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM`): ```bash # Add --cors if connecting from a browser-based client (e.g. claude.ai) -seerr-cli mcp serve --transport http --addr :8811 --route-token abc123 --no-auth --cors -# Endpoint becomes: http://localhost:8811/abc123/mcp -``` - -Both methods can be combined for defense in depth: - -```bash -seerr-cli mcp serve --transport http --route-token abc123 --auth-token mysecrettoken +seerr-cli mcp serve --transport http --addr :8811 --allow-api-key-query-param --cors +# Endpoint: http://localhost:8811/mcp?api_key=YOUR_SEERR_API_KEY ``` All flags are configurable via environment variables: -| Flag | Environment variable | Default | -| --------------- | ----------------------- | ------- | -| `--transport` | `SEERR_MCP_TRANSPORT` | `stdio` | -| `--addr` | `SEERR_MCP_ADDR` | `:8811` | -| `--auth-token` | `SEERR_MCP_AUTH_TOKEN` | — | -| `--no-auth` | `SEERR_MCP_NO_AUTH` | `false` | -| `--route-token` | `SEERR_MCP_ROUTE_TOKEN` | — | -| `--cors` | `SEERR_MCP_CORS` | `false` | -| `--tls-cert` | `SEERR_MCP_TLS_CERT` | — | -| `--tls-key` | `SEERR_MCP_TLS_KEY` | — | +| Flag | Environment variable | Default | +| ----------------------------- | ------------------------------------- | ------- | +| `--transport` | `SEERR_MCP_TRANSPORT` | `stdio` | +| `--addr` | `SEERR_MCP_ADDR` | `:8811` | +| `--auth-token` | `SEERR_MCP_AUTH_TOKEN` | — | +| `--no-auth` | `SEERR_MCP_NO_AUTH` | `false` | +| `--allow-api-key-query-param` | `SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM` | `false` | +| `--cors` | `SEERR_MCP_CORS` | `false` | +| `--tls-cert` | `SEERR_MCP_TLS_CERT` | — | +| `--tls-key` | `SEERR_MCP_TLS_KEY` | — | > Pass `--cors` (or `SEERR_MCP_CORS=true`) to enable CORS headers for browser-based clients (e.g. claude.ai). Disabled by default. diff --git a/README.md b/README.md index f148dad..37810ec 100644 --- a/README.md +++ b/README.md @@ -372,41 +372,48 @@ seerr-cli mcp serve --transport http --addr :8811 \ seerr-cli mcp serve --transport http --addr :8811 --no-auth ``` -The MCP endpoint will be `http://localhost:8811/mcp`. Configure your client with `Authorization: Bearer mysecrettoken`. +The MCP endpoint is always `http://localhost:8811/mcp`. Configure your client with `Authorization: Bearer mysecrettoken`. -#### Secret path prefix (for clients that cannot send custom headers) +> **Note:** The HTTP transport does not implement OAuth 2.0 and is not compatible with clients that require OAuth. Use stdio for Claude Desktop. + +#### API key via query parameter (opt-in) -Some MCP clients (e.g. claude.ai remote MCP integration) do not support custom `Authorization` headers. Use `--route-token` to embed a secret in the URL path instead: +For clients that cannot set custom headers, the Seerr API key can be passed via the `api_key` query parameter when `--allow-api-key-query-param` is enabled. The `X-Api-Key` header takes precedence when both are present; requests with neither are rejected. ```sh -# Endpoint becomes http://localhost:8811/abc123/mcp — no auth header needed -seerr-cli mcp serve --transport http --addr :8811 --route-token abc123 --no-auth +# Enable query parameter API key transport +seerr-cli mcp serve --transport http --allow-api-key-query-param # Add --cors for browser-based clients (e.g. claude.ai) -seerr-cli mcp serve --transport http --addr :8811 --route-token abc123 --no-auth --cors - -# Combine with Bearer auth for defense in depth -seerr-cli mcp serve --transport http --addr :8811 --route-token abc123 --auth-token mysecrettoken +seerr-cli mcp serve --transport http --allow-api-key-query-param --cors ``` -> **Note:** A secret path is weaker than a proper Bearer token since it may appear in proxy logs. For production use, combine it with TLS. +MCP endpoint: `http://localhost:8811/mcp?api_key=YOUR_SEERR_API_KEY` -> **Note:** The HTTP transport does not implement OAuth 2.0 and is not compatible with clients that require OAuth. Use stdio for Claude Desktop. +> **Security note:** Query parameters may appear in proxy logs and browser history. Always serve over HTTPS when using query parameter transport. + +#### Migration from `--route-token` or `--multi-tenant` + +Both flags and their path-based routing have been removed. The MCP endpoint is now always `/mcp`. Clients that previously relied on these mechanisms should migrate to: + +- **Header transport** — send the Seerr API key as `X-Api-Key: ` on each request. +- **Query parameter transport** — enable `--allow-api-key-query-param` and append `?api_key=` to the `/mcp` URL. +- **Bearer token** — use `--auth-token` for MCP server access control (separate from the Seerr API key). #### Environment variables All `mcp serve` flags can be set via environment variables, which is especially useful for Docker deployments: -| Flag | Environment variable | Default | -| --------------- | ----------------------- | ------- | -| `--transport` | `SEERR_MCP_TRANSPORT` | `stdio` | -| `--addr` | `SEERR_MCP_ADDR` | `:8811` | -| `--auth-token` | `SEERR_MCP_AUTH_TOKEN` | — | -| `--no-auth` | `SEERR_MCP_NO_AUTH` | `false` | -| `--route-token` | `SEERR_MCP_ROUTE_TOKEN` | — | -| `--cors` | `SEERR_MCP_CORS` | `false` | -| `--tls-cert` | `SEERR_MCP_TLS_CERT` | — | -| `--tls-key` | `SEERR_MCP_TLS_KEY` | — | +| Flag | Environment variable | Default | +| ----------------------------- | ------------------------------------- | ------- | +| `--transport` | `SEERR_MCP_TRANSPORT` | `stdio` | +| `--addr` | `SEERR_MCP_ADDR` | `:8811` | +| `--auth-token` | `SEERR_MCP_AUTH_TOKEN` | — | +| `--no-auth` | `SEERR_MCP_NO_AUTH` | `false` | +| `--allow-api-key-query-param` | `SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM` | `false` | +| `--cors` | `SEERR_MCP_CORS` | `false` | +| `--tls-cert` | `SEERR_MCP_TLS_CERT` | — | +| `--tls-key` | `SEERR_MCP_TLS_KEY` | — | ### Docker (HTTP transport) @@ -428,7 +435,7 @@ Configure your MCP client with: - **URL:** `http://localhost:8811/mcp` - **Authorization:** `Bearer mysecrettoken` -For clients that cannot send custom headers (e.g. claude.ai remote MCP), use a secret path prefix instead: +For clients that cannot send custom headers (e.g. claude.ai remote MCP), use query parameter transport instead: ```sh docker run -d \ @@ -436,15 +443,14 @@ docker run -d \ -p 8811:8811 \ -e SEERR_SERVER=https://your-seerr-instance.com \ -e SEERR_API_KEY=your-api-key \ - -e SEERR_MCP_ROUTE_TOKEN=abc123 \ - -e SEERR_MCP_NO_AUTH=true \ + -e SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM=true \ -e SEERR_MCP_CORS=true \ ghcr.io/electather/seerr-cli:latest ``` Configure your MCP client with: -- **URL:** `http://localhost:8811/abc123/mcp` +- **URL:** `http://localhost:8811/mcp?api_key=your-api-key` To bind to a different port or address, pass `--addr` explicitly: @@ -460,7 +466,7 @@ docker run -d \ ### Claude web (claude.ai) -Claude.ai connects to remote MCP servers over HTTPS. Since the browser cannot send custom `Authorization` headers to external MCP endpoints, the recommended approach is to embed a secret in the URL path using `--route-token` and expose the server via an HTTPS reverse proxy. +Claude.ai connects to remote MCP servers over HTTPS. Since the browser cannot send custom headers, use `--allow-api-key-query-param` and expose the server via an HTTPS reverse proxy. #### 1. Start the MCP server @@ -468,12 +474,11 @@ Claude.ai connects to remote MCP servers over HTTPS. Since the browser cannot se seerr-cli mcp serve \ --transport http \ --addr :8811 \ - --route-token YOUR_SECRET_TOKEN \ - --no-auth \ + --allow-api-key-query-param \ --cors ``` -The MCP endpoint will be `http://localhost:8811/YOUR_SECRET_TOKEN/mcp`. +The MCP endpoint will be `http://localhost:8811/mcp`. #### 2. Expose via HTTPS with a reverse proxy @@ -509,10 +514,10 @@ server { 1. Go to **claude.ai → Settings → Integrations**. 2. Click **Add integration**. -3. Enter the MCP URL: `https://mcp.example.com/YOUR_SECRET_TOKEN/mcp` +3. Enter the MCP URL: `https://mcp.example.com/mcp?api_key=YOUR_SEERR_API_KEY` 4. Save. The Seerr tools will appear in new conversations. -> **Security note:** The route token is the only secret protecting this endpoint. Use a long random value (e.g. `openssl rand -hex 32`) and always serve over HTTPS. +> **Security note:** The Seerr API key in the query string is the credential protecting this endpoint. Always serve over HTTPS and use a key with appropriate permissions. #### Health check diff --git a/cmd/mcp/flags.go b/cmd/mcp/flags.go index 19ed943..ffe0ecd 100644 --- a/cmd/mcp/flags.go +++ b/cmd/mcp/flags.go @@ -49,12 +49,6 @@ var ServeFlags = []FlagDef{ IsBool: true, Usage: "Disable authentication (insecure — must be explicit) (env: SEERR_MCP_NO_AUTH)", }, - { - Name: "route-token", - ViperKey: "mcp.route_token", - Default: "", - Usage: "Secret path prefix for the MCP endpoint (e.g. 'abc123' → /abc123/mcp) (env: SEERR_MCP_ROUTE_TOKEN)", - }, { Name: "tls-cert", ViperKey: "mcp.tls_cert", @@ -75,11 +69,11 @@ var ServeFlags = []FlagDef{ Usage: "Enable CORS headers (required for browser-based clients such as claude.ai) (env: SEERR_MCP_CORS)", }, { - Name: "multi-tenant", - ViperKey: "mcp.multi_tenant", + Name: "allow-api-key-query-param", + ViperKey: "mcp.allow_api_key_query_param", Default: "false", IsBool: true, - Usage: "Route /{seerr-api-token}/mcp for per-user API keys (HTTP transport only)", + Usage: "Accept the Seerr API key via the api_key query parameter in addition to the X-Api-Key header (HTTP transport only)", }, { Name: "log-file", diff --git a/cmd/mcp/logger.go b/cmd/mcp/logger.go index bbd8a2f..21efbe1 100644 --- a/cmd/mcp/logger.go +++ b/cmd/mcp/logger.go @@ -87,35 +87,25 @@ func (r *statusRecorder) WriteHeader(code int) { r.ResponseWriter.WriteHeader(code) } -// SafeLogPath returns a redacted version of path that omits sensitive tokens. -// -// In route-token mode the raw token in the URL prefix is replaced with -// {redacted}. In multi-tenant mode the per-user API key that occupies the -// first path segment is replaced with {tenant}. Plain /mcp paths are returned -// unchanged. -func SafeLogPath(path, routeToken string, multiTenant bool) string { - if routeToken != "" { - prefix := "/" + routeToken - if strings.HasPrefix(path, prefix+"/") { - return "/{redacted}" + path[len(prefix):] - } - if path == prefix { - return "/{redacted}" - } +// SafeLogQuery returns a redacted version of a raw query string, replacing the +// value of the api_key parameter with {redacted} to prevent credential leakage +// in logs. +func SafeLogQuery(rawQuery string) string { + if rawQuery == "" { + return "" } - if multiTenant { - trimmed := strings.TrimPrefix(path, "/") - idx := strings.Index(trimmed, "/") - if idx > 0 { - return "/{tenant}" + trimmed[idx:] + // Replace api_key= with api_key={redacted}. + parts := strings.Split(rawQuery, "&") + for i, part := range parts { + if strings.HasPrefix(part, "api_key=") { + parts[i] = "api_key={redacted}" } } - return path + return strings.Join(parts, "&") } // httpLoggingMiddleware logs every HTTP request at Info level (Warn for 4xx/5xx). -// routeToken and multiTenant are used to redact sensitive tokens from the logged path. -func httpLoggingMiddleware(next http.Handler, routeToken string, multiTenant bool) http.Handler { +func httpLoggingMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { start := time.Now() rec := &statusRecorder{ResponseWriter: w, status: http.StatusOK} @@ -124,11 +114,14 @@ func httpLoggingMiddleware(next http.Handler, routeToken string, multiTenant boo args := []any{ "method", r.Method, - "path", SafeLogPath(r.URL.Path, routeToken, multiTenant), + "path", r.URL.Path, "remote_addr", r.RemoteAddr, "status", rec.status, "duration_ms", duration.Milliseconds(), } + if r.URL.RawQuery != "" { + args = append(args, "query", SafeLogQuery(r.URL.RawQuery)) + } if rec.status >= 400 { mcpLog.Warn("http request", args...) } else { diff --git a/cmd/mcp/serve.go b/cmd/mcp/serve.go index 3b2f170..c4c7653 100644 --- a/cmd/mcp/serve.go +++ b/cmd/mcp/serve.go @@ -61,21 +61,14 @@ var serveCmd = &cobra.Command{ # Start over HTTPS with TLS seerr-cli mcp serve --transport http --auth-token mysecret --tls-cert /path/to/cert.pem --tls-key /path/to/key.pem - # Start over HTTP with a secret path prefix (for clients that cannot send custom headers) - seerr-cli mcp serve --transport http --route-token abc123 --no-auth - # MCP endpoint becomes: http://localhost:8811/abc123/mcp + # Accept Seerr API key via X-Api-Key header or ?api_key= query parameter + seerr-cli mcp serve --transport http --allow-api-key-query-param # Enable CORS for browser-based clients (e.g. claude.ai) - seerr-cli mcp serve --transport http --route-token abc123 --no-auth --cors - - # Combine both auth methods for defense in depth - seerr-cli mcp serve --transport http --route-token abc123 --auth-token mysecret + seerr-cli mcp serve --transport http --allow-api-key-query-param --cors # Start over HTTP without auth (insecure, not recommended) - seerr-cli mcp serve --transport http --no-auth - - # Start in multi-tenant mode (per-user API keys in URL path) - seerr-cli mcp serve --transport http --no-auth --multi-tenant`, + seerr-cli mcp serve --transport http --no-auth`, RunE: runServe, } @@ -89,12 +82,11 @@ func runServe(_ *cobra.Command, args []string) error { transport := viper.GetString("mcp.transport") addr := viper.GetString("mcp.addr") authToken := viper.GetString("mcp.auth_token") - routeToken := viper.GetString("mcp.route_token") noAuth := viper.GetBool("mcp.no_auth") tlsCert := viper.GetString("mcp.tls_cert") tlsKey := viper.GetString("mcp.tls_key") cors := viper.GetBool("mcp.cors") - multiTenant := viper.GetBool("mcp.multi_tenant") + allowAPIKeyQueryParam := viper.GetBool("mcp.allow_api_key_query_param") logFile := viper.GetString("mcp.log_file") logLevel := viper.GetString("mcp.log_level") logFormat := viper.GetString("mcp.log_format") @@ -107,12 +99,8 @@ func runServe(_ *cobra.Command, args []string) error { return err } - if transport == "http" && authToken == "" && routeToken == "" && !noAuth { - return fmt.Errorf("HTTP transport requires --auth-token, --route-token, or --no-auth (insecure) to be set explicitly") - } - - if multiTenant && transport != "http" { - return fmt.Errorf("--multi-tenant requires --transport http") + if transport == "http" && authToken == "" && !allowAPIKeyQueryParam && !noAuth { + return fmt.Errorf("HTTP transport requires --auth-token, --allow-api-key-query-param, or --no-auth (insecure) to be set explicitly") } s := server.NewMCPServer("electather/seerr-cli", buildVersion) @@ -156,13 +144,7 @@ func runServe(_ *cobra.Command, args []string) error { if strings.HasPrefix(host, ":") { host = "localhost" + host } - mcpPath := "/mcp" - if multiTenant { - mcpPath = "/{seerr-api-token}/mcp" - } else if routeToken != "" { - mcpPath = "/" + routeToken + "/mcp" - } - endpoint := fmt.Sprintf("%s://%s%s", scheme, host, mcpPath) + endpoint := fmt.Sprintf("%s://%s/mcp", scheme, host) mcpLog.Info("starting MCP server", "transport", "http", @@ -171,25 +153,15 @@ func runServe(_ *cobra.Command, args []string) error { "tools", 46, "resources", 9, "prompts", 6, "tls", tlsCert != "", "auth_token", authToken != "", - "route_token", routeToken != "", "cors", cors, - "multi_tenant", multiTenant, + "allow_api_key_query_param", allowAPIKeyQueryParam, ) httpHandler := server.NewStreamableHTTPServer(s) - var handler http.Handler - if multiTenant { - handler = tenantRoutingHandler(httpHandler) - } else if routeToken != "" { - // Strip the route-token prefix so mcp-go still sees /mcp, /mcp/sse, etc. - prefix := "/" + routeToken - mux := http.NewServeMux() - mux.Handle(prefix+"/", http.StripPrefix(prefix, httpHandler)) - handler = mux - } else { - handler = httpHandler - } - handler = httpLoggingMiddleware(handler, routeToken, multiTenant) + 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) } @@ -288,30 +260,33 @@ func bearerAuthMiddleware(token string, next http.Handler) http.Handler { }) } -// TenantRoutingHandler extracts the Seerr API token from /{token}/mcp paths and -// injects it into the request context before forwarding to the MCP handler. -// Exported for testing. -func TenantRoutingHandler(mcpHandler http.Handler) http.Handler { - return tenantRoutingHandler(mcpHandler) -} - -func tenantRoutingHandler(mcpHandler http.Handler) http.Handler { +// SeerrAPIKeyMiddleware extracts the Seerr API key from the incoming request +// and injects it into the request context for use by MCP tool handlers. +// +// 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. +// +// If neither location provides a key the middleware responds with 401. +func SeerrAPIKeyMiddleware(allowQueryParam bool, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Expect /{token}/mcp or /{token}/mcp/... - path := strings.TrimPrefix(r.URL.Path, "/") - slash := strings.Index(path, "/") - if slash < 0 { - http.NotFound(w, r) - return + var apiKey string + if v := r.Header.Get("X-Api-Key"); v != "" { + apiKey = v + } else if allowQueryParam { + if v := r.URL.Query().Get("api_key"); v != "" { + apiKey = v + } } - token, rest := path[:slash], path[slash:] // rest = "/mcp" or "/mcp/..." - if token == "" || !strings.HasPrefix(rest, "/mcp") { - http.NotFound(w, r) + + if apiKey != "" { + ctx := context.WithValue(r.Context(), apiKeyCtxKey, apiKey) + r = r.Clone(ctx) + } else { + http.Error(w, "Unauthorized", http.StatusUnauthorized) return } - ctx := context.WithValue(r.Context(), apiKeyCtxKey, token) - r2 := r.Clone(ctx) - r2.URL.Path = rest - mcpHandler.ServeHTTP(w, r2) + + next.ServeHTTP(w, r) }) } diff --git a/docker-compose.yml b/docker-compose.yml index f3809df..5968528 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -19,20 +19,14 @@ services: # --- MCP authentication (HTTP transport) --- # Bearer token that clients must supply in the Authorization header. - # Required unless SEERR_MCP_NO_AUTH or SEERR_MCP_ROUTE_TOKEN is set. + # Required unless SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM or SEERR_MCP_NO_AUTH is set. SEERR_MCP_AUTH_TOKEN: ${SEERR_MCP_AUTH_TOKEN} # Set to "true" to disable all authentication (insecure — not recommended). # SEERR_MCP_NO_AUTH: "false" - # Secret path prefix added to the MCP endpoint URL - # (e.g. "abc123" → /abc123/mcp). Useful for clients that cannot send - # custom headers. Can be combined with SEERR_MCP_AUTH_TOKEN for defense in - # depth. - # SEERR_MCP_ROUTE_TOKEN: "" - - # --- MCP multi-tenant mode (HTTP transport) --- - # When "true", the endpoint becomes /{seerr-api-token}/mcp and the path - # segment is used as the per-user Seerr API key. Requires HTTP transport. - # SEERR_MCP_MULTI_TENANT: "false" + # Accept the Seerr API key via the api_key query parameter in addition to + # the X-Api-Key header. Useful for clients that cannot send custom headers. + # Each request must supply the key; requests without a key are rejected. + # SEERR_MCP_ALLOW_API_KEY_QUERY_PARAM: "false" # --- MCP TLS (HTTP transport) --- # Paths to a TLS certificate and private key. When both are set the server diff --git a/seerr-cli.schema.json b/seerr-cli.schema.json index 98c4a36..04abd4e 100644 --- a/seerr-cli.schema.json +++ b/seerr-cli.schema.json @@ -56,10 +56,6 @@ "type": "boolean", "default": false }, - "route_token": { - "description": "Secret path prefix for the MCP endpoint (e.g. 'abc123' → /abc123/mcp). Useful for clients that cannot send custom headers.", - "type": "string" - }, "tls_cert": { "description": "Path to TLS certificate file (PEM). Required together with tls_key to enable HTTPS.", "type": "string" @@ -73,8 +69,8 @@ "type": "boolean", "default": false }, - "multi_tenant": { - "description": "Route /{seerr-api-token}/mcp so each user supplies their own API key in the URL.", + "allow_api_key_query_param": { + "description": "Accept the Seerr API key via the api_key query parameter in addition to the X-Api-Key header. Opt-in; disabled by default. HTTP transport only.", "type": "boolean", "default": false }, diff --git a/tests/mcp_api_key_middleware_test.go b/tests/mcp_api_key_middleware_test.go new file mode 100644 index 0000000..d5bf279 --- /dev/null +++ b/tests/mcp_api_key_middleware_test.go @@ -0,0 +1,109 @@ +package tests + +import ( + "net/http" + "net/http/httptest" + "testing" + + cmdmcp "seerr-cli/cmd/mcp" + + "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) + w.WriteHeader(http.StatusOK) + }) + return h, &s +} + +func TestSeerrAPIKeyMiddleware_headerOnly(t *testing.T) { + inner, captured := captureContextKey(t) + handler := cmdmcp.SeerrAPIKeyMiddleware(false, inner) + + req := httptest.NewRequest(http.MethodGet, "/mcp", nil) + req.Header.Set("X-Api-Key", "header-key") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "header-key", *captured) +} + +func TestSeerrAPIKeyMiddleware_queryParamOnly(t *testing.T) { + inner, captured := captureContextKey(t) + handler := cmdmcp.SeerrAPIKeyMiddleware(true, inner) + + req := httptest.NewRequest(http.MethodGet, "/mcp?api_key=qparam-key", nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "qparam-key", *captured) +} + +func TestSeerrAPIKeyMiddleware_headerPrecedenceOverQueryParam(t *testing.T) { + inner, captured := captureContextKey(t) + handler := cmdmcp.SeerrAPIKeyMiddleware(true, inner) + + req := httptest.NewRequest(http.MethodGet, "/mcp?api_key=qparam-key", nil) + req.Header.Set("X-Api-Key", "header-key") + 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") +} + +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) + + req := httptest.NewRequest(http.MethodGet, "/mcp?api_key=qparam-key", 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) +} + +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) + + req := httptest.NewRequest(http.MethodGet, "/mcp", nil) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.False(t, innerCalled) +} + +func TestSeerrAPIKeyMiddleware_queryParam_sensitiveValueNotLogged(t *testing.T) { + // This test ensures 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_flags_test.go b/tests/mcp_flags_test.go new file mode 100644 index 0000000..048304a --- /dev/null +++ b/tests/mcp_flags_test.go @@ -0,0 +1,35 @@ +package tests + +import ( + "testing" + + cmdmcp "seerr-cli/cmd/mcp" + + "github.com/stretchr/testify/assert" +) + +func TestRouteTokenFlagNotRegistered(t *testing.T) { + // --route-token has been removed; ensure it is not present in ServeFlags. + for _, f := range cmdmcp.ServeFlags { + assert.NotEqual(t, "route-token", f.Name, "--route-token must not appear in ServeFlags") + } +} + +func TestMultiTenantFlagNotRegistered(t *testing.T) { + // --multi-tenant has been removed; ensure it is not present in ServeFlags. + for _, f := range cmdmcp.ServeFlags { + assert.NotEqual(t, "multi-tenant", f.Name, "--multi-tenant must not appear in ServeFlags") + } +} + +func TestAllowAPIKeyQueryParamFlagRegistered(t *testing.T) { + // --allow-api-key-query-param must be present in ServeFlags. + var found bool + for _, f := range cmdmcp.ServeFlags { + if f.Name == "allow-api-key-query-param" { + found = true + break + } + } + assert.True(t, found, "--allow-api-key-query-param must appear in ServeFlags") +} diff --git a/tests/mcp_logger_test.go b/tests/mcp_logger_test.go index e2ee3f5..f5eca31 100644 --- a/tests/mcp_logger_test.go +++ b/tests/mcp_logger_test.go @@ -8,88 +8,42 @@ import ( "github.com/stretchr/testify/assert" ) -func TestSafeLogPath(t *testing.T) { +func TestSafeLogQuery(t *testing.T) { tests := []struct { - name string - path string - routeToken string - multiTenant bool - want string + name string + query string + want string }{ { - name: "plain mcp path unchanged", - path: "/mcp", - want: "/mcp", + name: "empty query unchanged", + query: "", + want: "", }, { - name: "plain mcp sse path unchanged", - path: "/mcp/sse", - want: "/mcp/sse", + name: "api_key value is redacted", + query: "api_key=secret123", + want: "api_key={redacted}", }, { - name: "route token in prefix is redacted", - path: "/abc123/mcp", - routeToken: "abc123", - want: "/{redacted}/mcp", + name: "api_key redacted while other params preserved", + query: "api_key=secret&page=1", + want: "api_key={redacted}&page=1", }, { - name: "route token sse path is redacted", - path: "/abc123/mcp/sse", - routeToken: "abc123", - want: "/{redacted}/mcp/sse", + name: "query without api_key is unchanged", + query: "page=1&limit=10", + want: "page=1&limit=10", }, { - name: "route token exact match is redacted", - path: "/abc123", - routeToken: "abc123", - want: "/{redacted}", - }, - { - name: "unrelated path is unchanged in route token mode", - path: "/health", - routeToken: "abc123", - want: "/health", - }, - { - name: "multi-tenant api key in path is redacted", - path: "/user-api-key/mcp", - multiTenant: true, - want: "/{tenant}/mcp", - }, - { - name: "multi-tenant api key with sse suffix is redacted", - path: "/user-api-key/mcp/sse", - multiTenant: true, - want: "/{tenant}/mcp/sse", - }, - { - name: "root path unchanged in multi-tenant mode", - path: "/", - multiTenant: true, - want: "/", - }, - { - name: "single segment path unchanged in multi-tenant mode", - path: "/health", - multiTenant: true, - want: "/health", - }, - { - name: "no route token and no multi-tenant returns path unchanged", - path: "/unexpected/path", - routeToken: "", - multiTenant: false, - want: "/unexpected/path", + name: "api_key in middle of query is redacted", + query: "page=1&api_key=mysecret&limit=10", + want: "page=1&api_key={redacted}&limit=10", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := cmdmcp.SafeLogPath(tc.path, tc.routeToken, tc.multiTenant) + got := cmdmcp.SafeLogQuery(tc.query) assert.Equal(t, tc.want, got) - // Verify the raw token never appears in the output. - if tc.routeToken != "" { - assert.NotContains(t, got, tc.routeToken) - } }) } } diff --git a/tests/mcp_serve_test.go b/tests/mcp_serve_test.go index 0f63d1f..6f1d23a 100644 --- a/tests/mcp_serve_test.go +++ b/tests/mcp_serve_test.go @@ -412,44 +412,12 @@ func TestMCPBlocklistListHandler(t *testing.T) { assert.Contains(t, text, `"results"`) } -// --- Multi-tenancy tests --- +// --- API key context propagation test --- -func TestTenantRoutingExtractsToken(t *testing.T) { - var capturedPath string - var capturedKey string - - inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - capturedPath = r.URL.Path - capturedKey, _ = r.Context().Value(cmdmcp.APIKeyContextKey).(string) - w.WriteHeader(http.StatusOK) - }) - - handler := cmdmcp.TenantRoutingHandler(inner) - - req := httptest.NewRequest(http.MethodGet, "/mytoken/mcp", nil) - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - assert.Equal(t, http.StatusOK, rec.Code) - assert.Equal(t, "/mcp", capturedPath) - assert.Equal(t, "mytoken", capturedKey) -} - -func TestTenantRoutingRejects404(t *testing.T) { - inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - }) - handler := cmdmcp.TenantRoutingHandler(inner) - - for _, path := range []string{"/mcp", "/", "/nopath"} { - req := httptest.NewRequest(http.MethodGet, path, nil) - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - assert.Equal(t, http.StatusNotFound, rec.Code, "path %q should return 404", path) - } -} - -func TestMultiTenantAPIKeyPropagation(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. var receivedAPIKey string ts, cleanup := newMCPTestServer(func(w http.ResponseWriter, r *http.Request) { @@ -467,12 +435,12 @@ func TestMultiTenantAPIKeyPropagation(t *testing.T) { handler := cmdmcp.StatusSystemHandler() - ctx := context.WithValue(context.Background(), cmdmcp.APIKeyContextKey, "tenant-api-key") + ctx := context.WithValue(context.Background(), cmdmcp.APIKeyContextKey, "injected-api-key") req := mcp.CallToolRequest{} req.Params.Arguments = nil result, err := handler(ctx, req) require.NoError(t, err) require.NotNil(t, result) - assert.Equal(t, "tenant-api-key", receivedAPIKey) + assert.Equal(t, "injected-api-key", receivedAPIKey) }