From 245ae865dc8a6bf107669efbb42cabc0b57de584 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 3 Sep 2026 10:12:21 +0200 Subject: [PATCH 01/19] feat(auth)!: establish secure remote transport boundaries Remote MCP access tokens and Sysdig API credentials cross different trust boundaries. Treating them as interchangeable exposed the configured upstream and token to client control. --- AGENTS.md | 14 +- README.md | 83 +++-- cmd/server/main.go | 42 ++- docs/TROUBLESHOOTING.md | 10 +- go.mod | 3 + go.sum | 6 + internal/config/config.go | 110 +++++- internal/config/config_test.go | 249 +++++++------- internal/infra/auth/token_verifier.go | 95 ++++++ internal/infra/auth/token_verifier_test.go | 188 ++++++++++ internal/infra/mcp/mcp_handler.go | 44 +-- internal/infra/mcp/mcp_handler_test.go | 321 +++++++++++------- internal/infra/mcp/remote_security.go | 133 ++++++++ internal/infra/sysdig/client.go | 59 +--- .../client_permissions_integration_test.go | 32 +- internal/infra/sysdig/client_test.go | 104 +----- package.nix | 2 +- 17 files changed, 994 insertions(+), 501 deletions(-) create mode 100644 internal/infra/auth/token_verifier.go create mode 100644 internal/infra/auth/token_verifier_test.go create mode 100644 internal/infra/mcp/remote_security.go diff --git a/AGENTS.md b/AGENTS.md index c6e0fad..4653cb6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,7 +11,7 @@ This document is a comprehensive guide for an AI agent tasked with developing an | Topic | Details | | --- | --- | | **Purpose** | Expose vetted Sysdig Monitor workflows to LLMs through MCP tools. | -| **Tech Stack** | Go 1.26+, `mcp-go`, Cobra CLI, Ginkgo/Gomega, `golangci-lint`, Nix. | +| **Tech Stack** | Go 1.27+, `mcp-go`, `go-oidc`, Cobra CLI, Ginkgo/Gomega, `golangci-lint`, Nix. | | **Entry Point** | `cmd/server/main.go` (Cobra CLI that wires config, Sysdig client, etc.). | | **Dev Shell** | `nix develop` provides a consistent development environment. | | **Key Commands** | `just fmt`, `just lint`, `just test`, `just check`, `just update`. | @@ -50,6 +50,7 @@ internal/ config/ - Environment variable loading and validation infra/ clock/ - System clock abstraction (for testing) + auth/ - Remote JWT access-token verification mcp/ - MCP server handler, transport setup, middleware tools/ - Individual MCP tool implementations sysdig/ - Sysdig API client (generated + extensions) @@ -68,19 +69,18 @@ package.nix - Defines how the package is going to be built with Nix 2. **Configuration (`internal/config/config.go`):** - Loads environment variables with `SYSDIG_MCP_*` prefix - - Validates required fields for stdio transport (API host and token mandatory) - - Supports remote transports where auth can come via HTTP headers + - Requires fixed Sysdig API credentials for every transport + - Validates the OAuth issuer, JWT/JWKS policy, resource audience, and exact browser origins for remote transports 3. **MCP Handler (`internal/infra/mcp/mcp_handler.go`):** - Wraps mcp-go server with permission filtering (`toolPermissionFiltering`, line 26-64) - Dynamically filters tools based on Sysdig API token permissions - - HTTP middleware extracts `Authorization` and `X-Sysdig-Host` headers for remote transports (line 108-138) + - Remote security validates OAuth access tokens and browser origins, then publishes RFC 9728 protected-resource metadata 4. **Sysdig Client (`internal/infra/sysdig/`):** - `client.gen.go`: Generated OpenAPI client (**DO NOT EDIT**, manually regenerated via oapi-codegen, not with `go generate`) - - `client.go`: Authentication strategies with fallback support - - Context-based auth: `WrapContextWithToken()` and `WrapContextWithHost()` for remote transports - - Fixed auth: `WithFixedHostAndToken()` for stdio mode and remote transports + - `client.go`: Fixed server-side authentication and version request editors + - `WithFixedHostAndToken()` is the only Sysdig authentication path; inbound MCP credentials must never be forwarded upstream - Custom extensions in `client_extension.go` and `client_*.go` files 5. **Tools (`internal/infra/mcp/tools/`):** diff --git a/README.md b/README.md index c090ca7..f7b93e4 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ Get up and running with the Sysdig MCP Server quickly using our pre-built Docker Substitute the following placeholders with your actual values: - ``: The hostname of your Sysdig instance (e.g., `https://us2.app.sysdig.com` or `https://eu1.app.sysdig.com`) - - ``: Your Sysdig API token (Secure or Monitor) + - ``: Your least-privilege Sysdig Monitor API token ## Available Tools @@ -141,14 +141,20 @@ The server dynamically filters the available tools based on the permissions asso > **Note:** When a time window is provided, the underlying PromQL is wrapped in the aggregation appropriate for each tool (`avg_over_time`, `max_over_time`, `min_over_time`, `increase`, etc.) and evaluated at `end`. See [`internal/infra/mcp/tools/README.md`](./internal/infra/mcp/tools/README.md) for the per-tool aggregation table. ## Requirements -- [Go](https://go.dev/doc/install) 1.26 or higher (if running without Docker). +- [Go](https://go.dev/doc/install) 1.27 or higher (if running without Docker). ## Configuration -The following environment variables are **required** for configuring the Sysdig SDK: +The following environment variables are **required for every transport**. They are deployment secrets and are never accepted from an MCP request: -- `SYSDIG_MCP_API_HOST`: The URL of your Sysdig instance (e.g., `https://us2.app.sysdig.com`). **Required when using `stdio` transport.** -- `SYSDIG_MCP_API_TOKEN`: Your Sysdig API token (Secure or Monitor). **Required only when using `stdio` transport.** +- `SYSDIG_MCP_API_HOST`: The absolute URL of your Sysdig instance (e.g., `https://us2.app.sysdig.com`). +- `SYSDIG_MCP_API_TOKEN`: A dedicated, least-privilege Sysdig Monitor API token used only for server-to-Sysdig requests. + +Remote transports (`streamable-http` and `sse`) are OAuth 2.1 protected resources and also require: + +- `SYSDIG_MCP_RESOURCE_URL`: The public MCP endpoint and expected JWT audience (for example, `https://mcp.example.com/sysdig-mcp-server`). +- `SYSDIG_MCP_AUTH_ISSUER`: The exact access-token issuer and authorization server URL. +- `SYSDIG_MCP_AUTH_JWKS_URL`: The issuer's JWKS endpoint used to verify JWT signatures. You can also set the following variables to override the default configuration: @@ -159,6 +165,11 @@ You can also set the following variables to override the default configuration: - `SYSDIG_MCP_LISTENING_PORT`: The port for the server when it is deployed using remote protocols (`streamable-http`, `sse`). Defaults to: `8080` - `SYSDIG_MCP_LISTENING_HOST`: The host for the server when it is deployed using remote protocols (`streamable-http`, `sse`). Defaults to all interfaces (`:port`). Set to `127.0.0.1` for local-only access. - `SYSDIG_MCP_STATELESS`: Enable stateless mode for `streamable-http` transport, where each request is self-contained with no session tracking (useful for AWS Bedrock AgentCore). Defaults to: `false`. +- `SYSDIG_MCP_AUTH_SCOPES`: Comma- or space-separated scopes required on remote MCP access tokens. Defaults to no required scopes. +- `SYSDIG_MCP_AUTH_SIGNING_ALGS`: Comma- or space-separated asymmetric JWT algorithms accepted from the issuer. Defaults to: `RS256`. Symmetric algorithms and `none` are rejected. +- `SYSDIG_MCP_ALLOWED_ORIGINS`: Comma- or space-separated browser origins allowed to call the remote transport, using exact `scheme://authority` values. Wildcards are rejected. If omitted, requests carrying an `Origin` header are denied; non-browser clients remain supported. + +All configured URLs must use HTTPS. Plain HTTP is accepted only for loopback development (`localhost` or a loopback IP). You can find your API token in the Sysdig UI under **Settings > Sysdig Secure API** (or **Sysdig Monitor API**). Make sure to copy the token as it will not be shown again. @@ -183,15 +194,24 @@ SYSDIG_MCP_LOGLEVEL=INFO ```bash # Required SYSDIG_MCP_TRANSPORT=streamable-http - -# Optional (Host and Token can be provided via HTTP headers) -# SYSDIG_MCP_API_HOST= -# SYSDIG_MCP_API_TOKEN=your-api-token-here +SYSDIG_MCP_API_HOST=https://us2.app.sysdig.com +SYSDIG_MCP_API_TOKEN=your-server-side-sysdig-token +SYSDIG_MCP_RESOURCE_URL=https://mcp.example.com/sysdig-mcp-server +SYSDIG_MCP_AUTH_ISSUER=https://identity.example.com +SYSDIG_MCP_AUTH_JWKS_URL=https://identity.example.com/.well-known/jwks.json + +# Optional remote policy +SYSDIG_MCP_AUTH_SCOPES=mcp:tools +SYSDIG_MCP_AUTH_SIGNING_ALGS=RS256 +SYSDIG_MCP_ALLOWED_ORIGINS=https://approved-client.example.com SYSDIG_MCP_LISTENING_PORT=8080 SYSDIG_MCP_LISTENING_HOST= SYSDIG_MCP_MOUNT_PATH=/sysdig-mcp-server ``` +> [!IMPORTANT] +> This is a breaking security boundary for remote deployments. MCP clients present an issuer-signed access token intended for `SYSDIG_MCP_RESOURCE_URL`; they never present a Sysdig API token. The server does not support `X-Sysdig-Host`, `X-Sysdig-Token`, or forwarding the request's `Authorization` value to Sysdig. + ### API Permissions To use the MCP server tools, your API token needs specific permissions on the Sysdig platform. We recommend creating a dedicated Service Account (SA) with a custom role containing only the required permissions. @@ -376,7 +396,7 @@ codex mcp add \ ### Kubernetes -Deploy the MCP server to a Kubernetes cluster as a remote service. MCP clients like Claude Desktop will connect to it via URL. +Deploy the MCP server to a Kubernetes cluster as an HTTPS remote service. MCP clients like Claude Desktop connect with an access token issued specifically for this MCP resource. **1. Create a Secret with your Sysdig credentials:** @@ -418,6 +438,16 @@ spec: env: - name: SYSDIG_MCP_TRANSPORT value: "streamable-http" + - name: SYSDIG_MCP_RESOURCE_URL + value: "https://mcp.example.com/sysdig-mcp-server" + - name: SYSDIG_MCP_AUTH_ISSUER + value: "https://identity.example.com" + - name: SYSDIG_MCP_AUTH_JWKS_URL + value: "https://identity.example.com/.well-known/jwks.json" + - name: SYSDIG_MCP_AUTH_SCOPES + value: "mcp:tools" + - name: SYSDIG_MCP_ALLOWED_ORIGINS + value: "https://approved-client.example.com" envFrom: - secretRef: name: mcp-server-secrets @@ -436,7 +466,7 @@ spec: targetPort: 8080 ``` -> **Note:** Expose the Service externally using a `NodePort`, `LoadBalancer`, or `Ingress` depending on your cluster setup. The examples in the [Client Configuration](#client-configuration) section assume the server is reachable at `http://:/sysdig-mcp-server`. +> **Note:** Terminate TLS at an Ingress or load balancer and set `SYSDIG_MCP_RESOURCE_URL` to the exact externally reachable endpoint. Keep the Kubernetes Service private; only the ingress should expose it. ## Local Development @@ -456,28 +486,34 @@ direnv allow ## Client Configuration -To use the MCP server with a client like Claude or Cursor, you need to provide the server's URL and authentication details. +To use the MCP server with a client like Claude or Cursor, provide the server URL and authentication details appropriate to the transport. ### Authentication -When using the `sse` or `streamable-http` transport, the server requires a Bearer token for authentication. The token is passed in the `X-Sysdig-Token` or default to `Authorization` header of the HTTP request (i.e `Bearer SYSDIG_MCP_API_TOKEN`). +With `stdio`, the locally launched process uses `SYSDIG_MCP_API_HOST` and `SYSDIG_MCP_API_TOKEN` directly. -Additionally, you can specify the Sysdig host by providing the `X-Sysdig-Host` header. +With `sse` or `streamable-http`, the server requires an issuer-signed JWT access token in the standard `Authorization` header. The token must: -> **Note:** When provided, the authentication headers (`Authorization`, `X-Sysdig-Token`) and host header (`X-Sysdig-Host`) take precedence over the configured environment variables. +- have an `iss` claim equal to `SYSDIG_MCP_AUTH_ISSUER`; +- include `SYSDIG_MCP_RESOURCE_URL` in `aud`; +- be unexpired and use an allowed asymmetric signing algorithm; +- contain every configured scope in `scope` or `scp`. + +The MCP access token and the server-side Sysdig token are different credentials for different audiences. The server validates the first and never forwards it; outbound Sysdig calls always use the second. Example headers: ``` -Authorization: Bearer -X-Sysdig-Host: +Authorization: Bearer ``` +The server publishes OAuth protected-resource metadata at `/.well-known/oauth-protected-resource[/]`. Missing or invalid credentials receive `401 Unauthorized` with a `WWW-Authenticate` challenge that points clients to this metadata. + ### URL -If you are running the server with the `sse` or `streamable-http` transport, the URL will be `http://:`, where `` is the value of `SYSDIG_MCP_MOUNT_PATH` (defaults to `/sysdig-mcp-server`). Do not include a trailing `/`. +If you are running the server with the `sse` or `streamable-http` transport, the production URL is `https://`, where `` is the value of `SYSDIG_MCP_MOUNT_PATH` (defaults to `/sysdig-mcp-server`). Do not include a trailing `/`. -For example, if you are running the server locally on port 8080 with the default mount path, the URL will be `http://localhost:8080/sysdig-mcp-server`. +For loopback development only, the default URL can be `http://localhost:8080/sysdig-mcp-server`. ### Claude Desktop App @@ -486,7 +522,7 @@ For the Claude Desktop app, configure the MCP server by editing the `claude_desk 1. Go to **Settings > Developer** in the Claude Desktop app. 2. Click on **Edit Config** to open the `claude_desktop_config.json` file. 3. Add the JSON configuration from the [Server Setup](#server-setup) section that matches your installation method (Go, Docker, or Binary). -4. Replace `` with your Sysdig host URL and `` with your Sysdig Secure or Monitor API token. +4. Replace `` with your Sysdig host URL and `` with your Sysdig Monitor API token. 5. Save the file and restart the Claude Desktop app. **Connecting to a Remote Server:** @@ -501,21 +537,20 @@ If the MCP server is deployed remotely (e.g., in a [Kubernetes cluster](#kuberne "args": [ "-y", "mcp-remote", - "http://:/sysdig-mcp-server", - "--allow-http" + "https://mcp.example.com/sysdig-mcp-server" ] } } } ``` -> **Note:** The `--allow-http` flag is required when connecting over plain HTTP. If your server is behind HTTPS (e.g., via an Ingress with TLS), you can omit it. No authentication headers or tokens are needed in the client configuration when the server has `SYSDIG_MCP_API_HOST` and `SYSDIG_MCP_API_TOKEN` set as environment variables. +> **Note:** Use `--allow-http` only for loopback development. In production, use HTTPS and authenticate through the authorization server advertised by the protected-resource metadata. Configuring the server-side Sysdig token does not authenticate MCP clients. ### MCP Inspector 1. Run the [MCP Inspector](https://modelcontextprotocol.io/docs/tools/inspector) locally. 2. Select the transport type and configure the connection to the Sysdig MCP server. -3. Pass the Authorization header if using `streamable-http` or the `SYSDIG_MCP_API_TOKEN` env var if using `stdio`. +3. For `streamable-http`, complete OAuth with the configured issuer or pass `Authorization: Bearer `. For `stdio`, configure `SYSDIG_MCP_API_TOKEN` in the launched process. ![mcp-inspector](./docs/assets/mcp-inspector.png) diff --git a/cmd/server/main.go b/cmd/server/main.go index b20f236..0cecc8c 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -9,9 +9,11 @@ import ( "os" "runtime/debug" "strings" + "time" "github.com/spf13/cobra" "github.com/sysdiglabs/sysdig-mcp-server/internal/config" + infraauth "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/auth" "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/clock" "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/mcp" "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/mcp/tools" @@ -85,10 +87,7 @@ func setupLogger(logLevel string) { func setupSysdigClient(cfg *config.Config) (sysdig.ExtendedClientWithResponsesInterface, error) { sysdigClientOptions := []sysdig.IntoClientOption{ sysdig.WithVersion(Version), - sysdig.WithFallbackAuthentication( - sysdig.WithHostAndTokenFromContext(), - sysdig.WithFixedHostAndToken(cfg.APIHost, cfg.APIToken), - ), + sysdig.WithFixedHostAndToken(cfg.APIHost, cfg.APIToken), } if cfg.SkipTLSVerification { @@ -109,6 +108,25 @@ func setupSysdigClient(cfg *config.Config) (sysdig.ExtendedClientWithResponsesIn return sysdigClient, nil } +func setupRemoteSecurity(cfg *config.Config) mcp.RemoteSecurity { + verifier := infraauth.NewJWTVerifier( + context.Background(), + cfg.AuthIssuer, + cfg.ResourceURL, + cfg.AuthJWKSURL, + cfg.AuthSigningAlgs, + cfg.AuthScopes, + ) + + return mcp.NewRemoteSecurity( + verifier, + cfg.ResourceURL, + cfg.AuthIssuer, + cfg.AuthScopes, + cfg.AllowedOrigins, + ) +} + func setupHandler(sysdigClient sysdig.ExtendedClientWithResponsesInterface) *mcp.Handler { systemClock := clock.NewSystemClock() handler := mcp.NewHandler(Version, sysdigClient) @@ -142,13 +160,25 @@ func startServer(cfg *config.Config, handler *mcp.Handler) error { case "streamable-http": addr := fmt.Sprintf("%s:%s", cfg.ListeningHost, cfg.ListeningPort) slog.Info("MCP Server listening", "addr", addr, "mountPath", cfg.MountPath, "stateless", cfg.Stateless) - if err := http.ListenAndServe(addr, handler.AsStreamableHTTP(cfg.MountPath, cfg.Stateless)); err != nil { + server := &http.Server{ + Addr: addr, + Handler: handler.AsStreamableHTTP(cfg.MountPath, cfg.Stateless, setupRemoteSecurity(cfg)), + ReadHeaderTimeout: 10 * time.Second, + IdleTimeout: 2 * time.Minute, + } + if err := server.ListenAndServe(); err != nil { return fmt.Errorf("error serving streamable http: %w", err) } case "sse": addr := fmt.Sprintf("%s:%s", cfg.ListeningHost, cfg.ListeningPort) slog.Info("MCP Server listening", "addr", addr, "mountPath", cfg.MountPath) - if err := http.ListenAndServe(addr, handler.AsSSE(cfg.MountPath)); err != nil { + server := &http.Server{ + Addr: addr, + Handler: handler.AsSSE(cfg.MountPath, setupRemoteSecurity(cfg)), + ReadHeaderTimeout: 10 * time.Second, + IdleTimeout: 2 * time.Minute, + } + if err := server.ListenAndServe(); err != nil { return fmt.Errorf("error serving sse: %w", err) } default: diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 482b8fa..6ca68bb 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -3,8 +3,14 @@ **Problem**: Tool not appearing in MCP client - **Solution**: Check API token permissions match tool's `WithRequiredPermissions()`. The token must have **all** permissions listed. -**Problem**: "unable to authenticate with any method" -- **Solution**: For `stdio`, verify `SYSDIG_MCP_API_HOST` and `SYSDIG_MCP_API_TOKEN` env vars are set correctly. For remote transports, check `Authorization: Bearer ` header format. +**Problem**: Server exits with a missing configuration error +- **Solution**: All transports require absolute `SYSDIG_MCP_API_HOST` and `SYSDIG_MCP_API_TOKEN` values. Remote transports also require `SYSDIG_MCP_RESOURCE_URL`, `SYSDIG_MCP_AUTH_ISSUER`, and `SYSDIG_MCP_AUTH_JWKS_URL`. + +**Problem**: Remote request returns `401 Unauthorized` +- **Solution**: Use an issuer-signed MCP access token, not the Sysdig API token. Verify its `iss`, `aud`, expiry, asymmetric signing algorithm, and required scopes. Follow the `resource_metadata` URL in the `WWW-Authenticate` response header to inspect the server's OAuth metadata. + +**Problem**: Browser request returns `403 Forbidden` +- **Solution**: Add the browser's exact origin to `SYSDIG_MCP_ALLOWED_ORIGINS`. Include only `scheme://authority`; wildcard origins and origins with paths are rejected. **Problem**: Connection failing with "certificate signed by unknown authority" - **Solution**: If using a self-signed certificate (e.g. on-prem), set `SYSDIG_MCP_API_SKIP_TLS_VERIFICATION=true`. diff --git a/go.mod b/go.mod index f583d0d..4fe827a 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,8 @@ module github.com/sysdiglabs/sysdig-mcp-server go 1.27 require ( + github.com/coreos/go-oidc/v3 v3.21.0 + github.com/go-jose/go-jose/v4 v4.1.4 github.com/mark3labs/mcp-go v1.1.1 github.com/oapi-codegen/runtime v1.7.0 github.com/onsi/ginkgo/v2 v2.33.0 @@ -28,6 +30,7 @@ require ( github.com/yosida95/uritemplate/v3 v3.0.2 // indirect golang.org/x/mod v0.41.0 // indirect golang.org/x/net v0.59.0 // indirect + golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.23.0 // indirect golang.org/x/sys v0.48.0 // indirect golang.org/x/text v0.42.0 // indirect diff --git a/go.sum b/go.sum index 41a90e4..22fad7e 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,8 @@ github.com/RaveNoX/go-jsoncommentstrip v1.0.0/go.mod h1:78ihd09MekBnJnxpICcwzCMz github.com/apapsch/go-jsonmerge/v2 v2.0.0 h1:axGnT1gRIfimI7gJifB699GoE/oq+F2MU7Dml6nw9rQ= github.com/apapsch/go-jsonmerge/v2 v2.0.0/go.mod h1:lvDnEdqiQrp0O42VQGgmlKpxL1AP2+08jFMw88y4klk= github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= +github.com/coreos/go-oidc/v3 v3.21.0 h1:wZo4Q9Pum8dYEj0eMUPrqR+kvuGkeUplbLpNCkBqoWM= +github.com/coreos/go-oidc/v3 v3.21.0/go.mod h1:DYCf24+ncYi+XkIH97GY1+dqoRlbaSI26KVTCI9SrY4= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -18,6 +20,8 @@ github.com/gkampitakis/go-diff v1.3.2 h1:Qyn0J9XJSDTgnsgHRdz9Zp24RaJeKMUHg2+PDZZ github.com/gkampitakis/go-diff v1.3.2/go.mod h1:LLgOrpqleQe26cte8s36HTWcTmMEur6OPYerdAAS9tk= github.com/gkampitakis/go-snaps v0.5.15 h1:amyJrvM1D33cPHwVrjo9jQxX8g/7E2wYdZ+01KS3zGE= github.com/gkampitakis/go-snaps v0.5.15/go.mod h1:HNpx/9GoKisdhw9AFOBT1N7DBs9DiHo/hGheFGBZ+mc= +github.com/go-jose/go-jose/v4 v4.1.4 h1:moDMcTHmvE6Groj34emNPLs/qtYXRVcd6S7NHbHz3kA= +github.com/go-jose/go-jose/v4 v4.1.4/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= github.com/go-logr/logr v1.4.4 h1:tG4xh9yMsRCAiodLVTxyrkzSZ9+o0L1Kg/+cPVcbP/8= github.com/go-logr/logr v1.4.4/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= @@ -93,6 +97,8 @@ golang.org/x/mod v0.41.0 h1:qJmnOUb4YB+FsEuM3HcWucdZASCPGhsX6uljO6pog0c= golang.org/x/mod v0.41.0/go.mod h1:Ek9pY8RKWXwsWvd3rQiHYtMqkjSUV+s1Rj7j4H5Ur6o= golang.org/x/net v0.59.0 h1:5zfYln+w5XCxwrnMMJPufRgNoXEaGxl0wo5GqPXyues= golang.org/x/net v0.59.0/go.mod h1:2DA/G1UfVbCpQPeWTmMPGY7Cs2PkBkwu743bVX5PIVg= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= diff --git a/internal/config/config.go b/internal/config/config.go index 9faed91..3f93dfa 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,7 +2,10 @@ package config import ( "fmt" + "net" + "net/url" "os" + "slices" "strconv" "strings" ) @@ -17,15 +20,62 @@ type Config struct { MountPath string LogLevel string Stateless bool + ResourceURL string + AuthIssuer string + AuthJWKSURL string + AuthScopes []string + AuthSigningAlgs []string + AllowedOrigins []string } func (c *Config) Validate() error { - if c.Transport == "stdio" && c.APIHost == "" { + if !slices.Contains([]string{"stdio", "streamable-http", "sse"}, c.Transport) { + return fmt.Errorf("unsupported SYSDIG_MCP_TRANSPORT %q", c.Transport) + } + if c.APIHost == "" { return fmt.Errorf("required configuration missing: SYSDIG_MCP_API_HOST") } - if c.Transport == "stdio" && c.APIToken == "" { + if c.APIToken == "" { return fmt.Errorf("required configuration missing: SYSDIG_MCP_API_TOKEN") } + if err := validateAbsoluteURL("SYSDIG_MCP_API_HOST", c.APIHost); err != nil { + return err + } + if c.Transport == "stdio" { + return nil + } + + if c.ResourceURL == "" { + return fmt.Errorf("required configuration missing: SYSDIG_MCP_RESOURCE_URL") + } + if c.AuthIssuer == "" { + return fmt.Errorf("required configuration missing: SYSDIG_MCP_AUTH_ISSUER") + } + if c.AuthJWKSURL == "" { + return fmt.Errorf("required configuration missing: SYSDIG_MCP_AUTH_JWKS_URL") + } + if err := validateAbsoluteURL("SYSDIG_MCP_RESOURCE_URL", c.ResourceURL); err != nil { + return err + } + if err := validateAbsoluteURL("SYSDIG_MCP_AUTH_ISSUER", c.AuthIssuer); err != nil { + return err + } + if err := validateAbsoluteURL("SYSDIG_MCP_AUTH_JWKS_URL", c.AuthJWKSURL); err != nil { + return err + } + for _, origin := range c.AllowedOrigins { + if err := validateOrigin(origin); err != nil { + return fmt.Errorf("invalid SYSDIG_MCP_ALLOWED_ORIGINS entry %q: %w", origin, err) + } + } + if len(c.AuthSigningAlgs) == 0 { + return fmt.Errorf("SYSDIG_MCP_AUTH_SIGNING_ALGS must contain at least one asymmetric signing algorithm") + } + for _, algorithm := range c.AuthSigningAlgs { + if !slices.Contains([]string{"RS256", "RS384", "RS512", "PS256", "PS384", "PS512", "ES256", "ES384", "ES512", "EdDSA"}, algorithm) { + return fmt.Errorf("unsupported asymmetric signing algorithm %q in SYSDIG_MCP_AUTH_SIGNING_ALGS", algorithm) + } + } return nil } @@ -40,6 +90,12 @@ func Load() (*Config, error) { MountPath: getEnv("SYSDIG_MCP_MOUNT_PATH", "/sysdig-mcp-server"), LogLevel: getEnv("SYSDIG_MCP_LOGLEVEL", "INFO"), Stateless: getEnv("SYSDIG_MCP_STATELESS", false), + ResourceURL: getEnv("SYSDIG_MCP_RESOURCE_URL", ""), + AuthIssuer: getEnv("SYSDIG_MCP_AUTH_ISSUER", ""), + AuthJWKSURL: getEnv("SYSDIG_MCP_AUTH_JWKS_URL", ""), + AuthScopes: getEnvList("SYSDIG_MCP_AUTH_SCOPES", nil), + AuthSigningAlgs: getEnvList("SYSDIG_MCP_AUTH_SIGNING_ALGS", []string{"RS256"}), + AllowedOrigins: getEnvList("SYSDIG_MCP_ALLOWED_ORIGINS", nil), } if err := cfg.Validate(); err != nil { @@ -49,6 +105,56 @@ func Load() (*Config, error) { return cfg, nil } +func getEnvList(key string, fallback []string) []string { + value, ok := os.LookupEnv(key) + if !ok { + return slices.Clone(fallback) + } + + return strings.FieldsFunc(value, func(r rune) bool { + return r == ',' || r == ' ' || r == '\t' || r == '\n' + }) +} + +func validateAbsoluteURL(name, rawURL string) error { + u, err := url.Parse(rawURL) + if err != nil || !u.IsAbs() || u.Host == "" { + return fmt.Errorf("%s must be an absolute URL", name) + } + if u.User != nil || u.Fragment != "" { + return fmt.Errorf("%s must not contain user information or a fragment", name) + } + if u.Scheme != "https" && !(u.Scheme == "http" && isLoopbackHostname(u.Hostname())) { + return fmt.Errorf("%s must use https (http is allowed only for loopback development)", name) + } + return nil +} + +func validateOrigin(origin string) error { + if origin == "*" { + return fmt.Errorf("wildcard origins are not allowed") + } + u, err := url.Parse(origin) + if err != nil || !u.IsAbs() || u.Host == "" { + return fmt.Errorf("origin must be an absolute URL") + } + if u.User != nil || u.Path != "" || u.RawQuery != "" || u.Fragment != "" { + return fmt.Errorf("origin must contain only scheme and authority") + } + if u.Scheme != "https" && !(u.Scheme == "http" && isLoopbackHostname(u.Hostname())) { + return fmt.Errorf("origin must use https (http is allowed only for loopback development)") + } + return nil +} + +func isLoopbackHostname(host string) bool { + if strings.EqualFold(host, "localhost") { + return true + } + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() +} + type envType interface { ~string | ~bool } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3800bbb..1199053 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -9,148 +9,157 @@ import ( "github.com/sysdiglabs/sysdig-mcp-server/internal/config" ) +func validConfig(transport string) *config.Config { + cfg := &config.Config{ + APIHost: "https://app.us4.sysdig.com", + APIToken: "sysdig-token", + Transport: transport, + } + if transport != "stdio" { + cfg.ResourceURL = "https://mcp.example.com/sysdig-mcp-server" + cfg.AuthIssuer = "https://identity.example.com" + cfg.AuthJWKSURL = "https://identity.example.com/.well-known/jwks.json" + cfg.AuthSigningAlgs = []string{"RS256"} + cfg.AllowedOrigins = []string{"https://client.example.com"} + } + return cfg +} + var _ = Describe("Config", func() { Describe("Validate", func() { - Context("with a valid config", func() { - It("should not return an error", func() { - cfg := &config.Config{ - APIHost: "host", - APIToken: "token", - } - Expect(cfg.Validate()).To(Succeed()) - }) + It("accepts stdio and secured remote configurations", func() { + Expect(validConfig("stdio").Validate()).To(Succeed()) + Expect(validConfig("streamable-http").Validate()).To(Succeed()) + Expect(validConfig("sse").Validate()).To(Succeed()) }) - Context("with a missing api host", func() { - It("should return an error if transport is stdio", func() { - cfg := &config.Config{ - Transport: "stdio", - APIToken: "token", - } - err := cfg.Validate() - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("SYSDIG_MCP_API_HOST")) - }) - - It("should not return an error if transport is not stdio", func() { - cfg := &config.Config{ - Transport: "sse", - APIToken: "token", - } - err := cfg.Validate() - Expect(err).ToNot(HaveOccurred()) - }) + DescribeTable("rejects missing credentials for every transport", + func(transport string) { + cfg := validConfig(transport) + cfg.APIHost = "" + Expect(cfg.Validate()).To(MatchError(ContainSubstring("SYSDIG_MCP_API_HOST"))) + + cfg = validConfig(transport) + cfg.APIToken = "" + Expect(cfg.Validate()).To(MatchError(ContainSubstring("SYSDIG_MCP_API_TOKEN"))) + }, + Entry("stdio", "stdio"), + Entry("streamable HTTP", "streamable-http"), + Entry("SSE", "sse"), + ) + + It("rejects unsupported transports", func() { + cfg := validConfig("websocket") + Expect(cfg.Validate()).To(MatchError(ContainSubstring("unsupported SYSDIG_MCP_TRANSPORT"))) }) - Context("with a missing api token", func() { - It("should not return an error if transport is not stdio", func() { - cfg := &config.Config{ - APIHost: "host", - } - err := cfg.Validate() - Expect(err).ToNot(HaveOccurred()) - }) - - It("should return an error if transport is stdio", func() { - cfg := &config.Config{ - Transport: "stdio", - APIHost: "host", - } - err := cfg.Validate() - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("SYSDIG_MCP_API_TOKEN")) - }) + DescribeTable("requires the remote OAuth configuration", + func(clear func(*config.Config), expected string) { + cfg := validConfig("streamable-http") + clear(cfg) + Expect(cfg.Validate()).To(MatchError(ContainSubstring(expected))) + }, + Entry("resource URL", func(cfg *config.Config) { cfg.ResourceURL = "" }, "SYSDIG_MCP_RESOURCE_URL"), + Entry("issuer", func(cfg *config.Config) { cfg.AuthIssuer = "" }, "SYSDIG_MCP_AUTH_ISSUER"), + Entry("JWKS URL", func(cfg *config.Config) { cfg.AuthJWKSURL = "" }, "SYSDIG_MCP_AUTH_JWKS_URL"), + ) + + DescribeTable("rejects unsafe URLs", + func(mutate func(*config.Config), expected string) { + cfg := validConfig("streamable-http") + mutate(cfg) + Expect(cfg.Validate()).To(MatchError(ContainSubstring(expected))) + }, + Entry("relative API host", func(cfg *config.Config) { cfg.APIHost = "app.example.com" }, "absolute URL"), + Entry("plaintext resource", func(cfg *config.Config) { cfg.ResourceURL = "http://mcp.example.com" }, "must use https"), + Entry("issuer with user info", func(cfg *config.Config) { cfg.AuthIssuer = "https://user@identity.example.com" }, "user information"), + Entry("fragmented JWKS URL", func(cfg *config.Config) { cfg.AuthJWKSURL += "#keys" }, "fragment"), + Entry("wildcard origin", func(cfg *config.Config) { cfg.AllowedOrigins = []string{"*"} }, "wildcard"), + Entry("origin path", func(cfg *config.Config) { cfg.AllowedOrigins = []string{"https://client.example.com/path"} }, "scheme and authority"), + Entry("empty signing algorithms", func(cfg *config.Config) { cfg.AuthSigningAlgs = nil }, "at least one"), + Entry("symmetric signing", func(cfg *config.Config) { cfg.AuthSigningAlgs = []string{"HS256"} }, "asymmetric signing algorithm"), + ) + + It("allows HTTP only for loopback development", func() { + cfg := validConfig("streamable-http") + cfg.APIHost = "http://127.0.0.1:9000" + cfg.ResourceURL = "http://localhost:8080/sysdig-mcp-server" + cfg.AuthIssuer = "http://[::1]:9001" + cfg.AuthJWKSURL = "http://localhost:9001/jwks" + cfg.AllowedOrigins = []string{"http://localhost:5173"} + Expect(cfg.Validate()).To(Succeed()) }) }) Describe("Load", func() { BeforeEach(func() { os.Clearenv() + _ = os.Setenv("SYSDIG_MCP_API_HOST", "https://app.us4.sysdig.com") + _ = os.Setenv("SYSDIG_MCP_API_TOKEN", "sysdig-token") }) - Context("with required env vars set for stdio", func() { - BeforeEach(func() { - _ = os.Setenv("SYSDIG_MCP_API_HOST", "host") - _ = os.Setenv("SYSDIG_MCP_API_TOKEN", "token") - }) - - It("should load default values", func() { - cfg, err := config.Load() - Expect(err).NotTo(HaveOccurred()) - Expect(cfg.Transport).To(Equal("stdio")) - Expect(cfg.ListeningHost).To(BeEmpty()) - Expect(cfg.ListeningPort).To(Equal("8080")) - Expect(cfg.MountPath).To(Equal("/sysdig-mcp-server")) - Expect(cfg.LogLevel).To(Equal("INFO")) - Expect(cfg.SkipTLSVerification).To(BeFalse()) - Expect(cfg.Stateless).To(BeFalse()) - }) + It("loads stdio defaults", func() { + cfg, err := config.Load() + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.Transport).To(Equal("stdio")) + Expect(cfg.ListeningHost).To(BeEmpty()) + Expect(cfg.ListeningPort).To(Equal("8080")) + Expect(cfg.MountPath).To(Equal("/sysdig-mcp-server")) + Expect(cfg.LogLevel).To(Equal("INFO")) + Expect(cfg.SkipTLSVerification).To(BeFalse()) + Expect(cfg.Stateless).To(BeFalse()) + Expect(cfg.AuthSigningAlgs).To(Equal([]string{"RS256"})) }) - Context("with required env vars set for http", func() { - BeforeEach(func() { - _ = os.Setenv("SYSDIG_MCP_API_HOST", "host") - _ = os.Setenv("SYSDIG_MCP_TRANSPORT", "streamable-http") - }) - - It("should load default values", func() { - cfg, err := config.Load() - Expect(err).NotTo(HaveOccurred()) - Expect(cfg.Transport).To(Equal("streamable-http")) - Expect(cfg.ListeningHost).To(BeEmpty()) - Expect(cfg.ListeningPort).To(Equal("8080")) - Expect(cfg.MountPath).To(Equal("/sysdig-mcp-server")) - Expect(cfg.LogLevel).To(Equal("INFO")) - }) + It("loads all remote security values", func() { + _ = os.Setenv("SYSDIG_MCP_API_SKIP_TLS_VERIFICATION", "true") + _ = os.Setenv("SYSDIG_MCP_TRANSPORT", "streamable-http") + _ = os.Setenv("SYSDIG_MCP_LISTENING_HOST", "0.0.0.0") + _ = os.Setenv("SYSDIG_MCP_LISTENING_PORT", "9090") + _ = os.Setenv("SYSDIG_MCP_MOUNT_PATH", "/custom") + _ = os.Setenv("SYSDIG_MCP_LOGLEVEL", "DEBUG") + _ = os.Setenv("SYSDIG_MCP_STATELESS", "true") + _ = os.Setenv("SYSDIG_MCP_RESOURCE_URL", "https://mcp.example.com/custom") + _ = os.Setenv("SYSDIG_MCP_AUTH_ISSUER", "https://identity.example.com") + _ = os.Setenv("SYSDIG_MCP_AUTH_JWKS_URL", "https://identity.example.com/jwks") + _ = os.Setenv("SYSDIG_MCP_AUTH_SCOPES", "mcp:tools, profile") + _ = os.Setenv("SYSDIG_MCP_AUTH_SIGNING_ALGS", "RS256 ES256") + _ = os.Setenv("SYSDIG_MCP_ALLOWED_ORIGINS", "https://one.example.com, https://two.example.com") + + cfg, err := config.Load() + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.SkipTLSVerification).To(BeTrue()) + Expect(cfg.Transport).To(Equal("streamable-http")) + Expect(cfg.ListeningHost).To(Equal("0.0.0.0")) + Expect(cfg.ListeningPort).To(Equal("9090")) + Expect(cfg.MountPath).To(Equal("/custom")) + Expect(cfg.LogLevel).To(Equal("DEBUG")) + Expect(cfg.Stateless).To(BeTrue()) + Expect(cfg.ResourceURL).To(Equal("https://mcp.example.com/custom")) + Expect(cfg.AuthIssuer).To(Equal("https://identity.example.com")) + Expect(cfg.AuthJWKSURL).To(Equal("https://identity.example.com/jwks")) + Expect(cfg.AuthScopes).To(Equal([]string{"mcp:tools", "profile"})) + Expect(cfg.AuthSigningAlgs).To(Equal([]string{"RS256", "ES256"})) + Expect(cfg.AllowedOrigins).To(Equal([]string{"https://one.example.com", "https://two.example.com"})) }) - Context("with all env vars set", func() { - BeforeEach(func() { - _ = os.Setenv("SYSDIG_MCP_API_HOST", "env-host") - _ = os.Setenv("SYSDIG_MCP_API_TOKEN", "env-token") - _ = os.Setenv("SYSDIG_MCP_API_SKIP_TLS_VERIFICATION", "true") - _ = os.Setenv("SYSDIG_MCP_TRANSPORT", "http") - _ = os.Setenv("SYSDIG_MCP_LISTENING_HOST", "0.0.0.0") - _ = os.Setenv("SYSDIG_MCP_LISTENING_PORT", "9090") - _ = os.Setenv("SYSDIG_MCP_MOUNT_PATH", "/custom") - _ = os.Setenv("SYSDIG_MCP_LOGLEVEL", "DEBUG") - _ = os.Setenv("SYSDIG_MCP_STATELESS", "true") - }) - - It("should load all values from the environment", func() { - cfg, err := config.Load() - Expect(err).NotTo(HaveOccurred()) - Expect(cfg.APIHost).To(Equal("env-host")) - Expect(cfg.APIToken).To(Equal("env-token")) - Expect(cfg.SkipTLSVerification).To(BeTrue()) - Expect(cfg.Transport).To(Equal("http")) - Expect(cfg.ListeningHost).To(Equal("0.0.0.0")) - Expect(cfg.ListeningPort).To(Equal("9090")) - Expect(cfg.MountPath).To(Equal("/custom")) - Expect(cfg.LogLevel).To(Equal("DEBUG")) - Expect(cfg.Stateless).To(BeTrue()) - }) + It("requires all remote settings", func() { + _ = os.Setenv("SYSDIG_MCP_TRANSPORT", "sse") + _, err := config.Load() + Expect(err).To(MatchError(ContainSubstring("SYSDIG_MCP_RESOURCE_URL"))) }) - Context("with invalid boolean env var", func() { - BeforeEach(func() { - _ = os.Setenv("SYSDIG_MCP_API_HOST", "host") - _ = os.Setenv("SYSDIG_MCP_API_TOKEN", "token") - _ = os.Setenv("SYSDIG_MCP_API_SKIP_TLS_VERIFICATION", "invalid-bool") - }) - - It("should fall back to default value", func() { - cfg, err := config.Load() - Expect(err).NotTo(HaveOccurred()) - Expect(cfg.SkipTLSVerification).To(BeFalse()) - }) + It("falls back for invalid booleans", func() { + _ = os.Setenv("SYSDIG_MCP_API_SKIP_TLS_VERIFICATION", "invalid-bool") + cfg, err := config.Load() + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.SkipTLSVerification).To(BeFalse()) }) - Context("without required env vars", func() { - It("should return an error", func() { - _, err := config.Load() - Expect(err).To(HaveOccurred()) - }) + It("fails without required values", func() { + os.Clearenv() + _, err := config.Load() + Expect(err).To(HaveOccurred()) }) }) }) diff --git a/internal/infra/auth/token_verifier.go b/internal/infra/auth/token_verifier.go new file mode 100644 index 0000000..ba7834a --- /dev/null +++ b/internal/infra/auth/token_verifier.go @@ -0,0 +1,95 @@ +package auth + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "slices" + "strings" + "time" + + "github.com/coreos/go-oidc/v3/oidc" +) + +const jwksRequestTimeout = 10 * time.Second + +// ErrInsufficientScope distinguishes an authenticated token from one that +// lacks the authorization required by this resource. +var ErrInsufficientScope = errors.New("access token has insufficient scope") + +// TokenVerifier validates an access token presented to the MCP server. +// Implementations must never forward the token to an upstream service. +type TokenVerifier interface { + Verify(context.Context, string) error +} + +// JWTVerifier validates signed JWT access tokens against a remote JWKS. +// Issuer, audience, expiry, and signing algorithm checks are delegated to the +// OIDC verifier. Optional scopes are checked after cryptographic validation. +type JWTVerifier struct { + verifier *oidc.IDTokenVerifier + requiredScopes []string +} + +func NewJWTVerifier( + ctx context.Context, + issuer string, + audience string, + jwksURL string, + signingAlgorithms []string, + requiredScopes []string, +) *JWTVerifier { + ctx = oidc.ClientContext(ctx, &http.Client{Timeout: jwksRequestTimeout}) + keySet := oidc.NewRemoteKeySet(ctx, jwksURL) + verifier := oidc.NewVerifier(issuer, keySet, &oidc.Config{ + ClientID: audience, + SupportedSigningAlgs: signingAlgorithms, + }) + + return &JWTVerifier{ + verifier: verifier, + requiredScopes: slices.Clone(requiredScopes), + } +} + +func (v *JWTVerifier) Verify(ctx context.Context, rawToken string) error { + token, err := v.verifier.Verify(ctx, rawToken) + if err != nil { + return fmt.Errorf("validating access token: %w", err) + } + + if len(v.requiredScopes) == 0 { + return nil + } + + var claims struct { + Scope string `json:"scope"` + SCP json.RawMessage `json:"scp"` + } + if err := token.Claims(&claims); err != nil { + return fmt.Errorf("decoding access token claims: %w", err) + } + + grantedScopes := strings.Fields(claims.Scope) + if len(claims.SCP) > 0 { + var scopeString string + if err := json.Unmarshal(claims.SCP, &scopeString); err == nil { + grantedScopes = append(grantedScopes, strings.Fields(scopeString)...) + } else { + var scopeList []string + if err := json.Unmarshal(claims.SCP, &scopeList); err != nil { + return fmt.Errorf("decoding scp claim: %w", err) + } + grantedScopes = append(grantedScopes, scopeList...) + } + } + for _, requiredScope := range v.requiredScopes { + if !slices.Contains(grantedScopes, requiredScope) { + return fmt.Errorf("%w: missing %q", ErrInsufficientScope, requiredScope) + } + } + + return nil +} diff --git a/internal/infra/auth/token_verifier_test.go b/internal/infra/auth/token_verifier_test.go new file mode 100644 index 0000000..f316c46 --- /dev/null +++ b/internal/infra/auth/token_verifier_test.go @@ -0,0 +1,188 @@ +package auth_test + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-jose/go-jose/v4" + "github.com/go-jose/go-jose/v4/jwt" + + infraauth "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/auth" +) + +const ( + testIssuer = "https://identity.example.com" + testAudience = "https://mcp.example.com/sysdig-mcp-server" +) + +type accessTokenClaims struct { + Scope string `json:"scope,omitempty"` + SCP any `json:"scp,omitempty"` +} + +func TestJWTVerifier(t *testing.T) { + t.Parallel() + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generating RSA key: %v", err) + } + + keyID := "test-key" + jwks := jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{ + Key: &privateKey.PublicKey, + KeyID: keyID, + Algorithm: string(jose.RS256), + Use: "sig", + }}} + jwksServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/jwks" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(jwks); err != nil { + t.Errorf("encoding JWKS: %v", err) + } + })) + defer jwksServer.Close() + + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: jose.RS256, Key: privateKey}, + (&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", keyID), + ) + if err != nil { + t.Fatalf("creating signer: %v", err) + } + + newToken := func(issuer string, audience jwt.Audience, expiry time.Time, claims accessTokenClaims) string { + t.Helper() + rawToken, err := jwt.Signed(signer). + Claims(jwt.Claims{ + Issuer: issuer, + Audience: audience, + Expiry: jwt.NewNumericDate(expiry), + }). + Claims(claims). + Serialize() + if err != nil { + t.Fatalf("signing token: %v", err) + } + return rawToken + } + + tests := []struct { + name string + issuer string + audience jwt.Audience + expiry time.Time + claims accessTokenClaims + signingAlgorithms []string + requiredScopes []string + wantError bool + wantScopeError bool + }{ + { + name: "valid scope claim", + issuer: testIssuer, + audience: jwt.Audience{testAudience}, + expiry: time.Now().Add(time.Hour), + claims: accessTokenClaims{Scope: "openid mcp:tools"}, + signingAlgorithms: []string{"RS256"}, + requiredScopes: []string{"mcp:tools"}, + }, + { + name: "valid scp claim", + issuer: testIssuer, + audience: jwt.Audience{"another-audience", testAudience}, + expiry: time.Now().Add(time.Hour), + claims: accessTokenClaims{SCP: []string{"mcp:tools", "profile"}}, + signingAlgorithms: []string{"RS256"}, + requiredScopes: []string{"mcp:tools", "profile"}, + }, + { + name: "valid space-delimited scp claim", + issuer: testIssuer, + audience: jwt.Audience{testAudience}, + expiry: time.Now().Add(time.Hour), + claims: accessTokenClaims{SCP: "mcp:tools profile"}, + signingAlgorithms: []string{"RS256"}, + requiredScopes: []string{"mcp:tools", "profile"}, + }, + { + name: "wrong issuer", + issuer: "https://attacker.example.com", + audience: jwt.Audience{testAudience}, + expiry: time.Now().Add(time.Hour), + signingAlgorithms: []string{"RS256"}, + wantError: true, + }, + { + name: "wrong audience", + issuer: testIssuer, + audience: jwt.Audience{"another-audience"}, + expiry: time.Now().Add(time.Hour), + signingAlgorithms: []string{"RS256"}, + wantError: true, + }, + { + name: "expired", + issuer: testIssuer, + audience: jwt.Audience{testAudience}, + expiry: time.Now().Add(-time.Hour), + signingAlgorithms: []string{"RS256"}, + wantError: true, + }, + { + name: "missing required scope", + issuer: testIssuer, + audience: jwt.Audience{testAudience}, + expiry: time.Now().Add(time.Hour), + claims: accessTokenClaims{Scope: "openid"}, + signingAlgorithms: []string{"RS256"}, + requiredScopes: []string{"mcp:tools"}, + wantError: true, + wantScopeError: true, + }, + { + name: "disallowed signing algorithm", + issuer: testIssuer, + audience: jwt.Audience{testAudience}, + expiry: time.Now().Add(time.Hour), + signingAlgorithms: []string{"ES256"}, + wantError: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + rawToken := newToken(test.issuer, test.audience, test.expiry, test.claims) + verifier := infraauth.NewJWTVerifier( + context.Background(), + testIssuer, + testAudience, + jwksServer.URL+"/jwks", + test.signingAlgorithms, + test.requiredScopes, + ) + + err := verifier.Verify(context.Background(), rawToken) + if test.wantError && err == nil { + t.Fatal("expected token verification to fail") + } + if test.wantScopeError && !errors.Is(err, infraauth.ErrInsufficientScope) { + t.Fatalf("expected insufficient scope error, got: %v", err) + } + if !test.wantError && err != nil { + t.Fatalf("expected token verification to succeed: %v", err) + } + }) + } +} diff --git a/internal/infra/mcp/mcp_handler.go b/internal/infra/mcp/mcp_handler.go index f9eea98..8e5cfe1 100644 --- a/internal/infra/mcp/mcp_handler.go +++ b/internal/infra/mcp/mcp_handler.go @@ -67,7 +67,7 @@ func NewHandler(version string, sysdigClient sysdig.ExtendedClientWithResponsesI s := server.NewMCPServer( "Sysdig MCP Server", version, - server.WithInstructions("Provides Sysdig Secure tools and resources."), + server.WithInstructions("Provides read-only Sysdig Monitor tools for infrastructure analysis."), server.WithToolCapabilities(true), server.WithToolFilter(toolPermissionFiltering(sysdigClient)), ) @@ -87,7 +87,7 @@ func (h *Handler) ServeStdio(ctx context.Context, stdin io.Reader, stdout io.Wri return server.NewStdioServer(h.server).Listen(ctx, stdin, stdout) } -func (h *Handler) AsStreamableHTTP(mountPath string, stateless bool) http.Handler { +func (h *Handler) AsStreamableHTTP(mountPath string, stateless bool, security RemoteSecurity) http.Handler { mux := http.NewServeMux() var opts []server.StreamableHTTPOption @@ -96,49 +96,19 @@ func (h *Handler) AsStreamableHTTP(mountPath string, stateless bool) http.Handle } httpServer := server.NewStreamableHTTPServer(h.server, opts...) - mux.Handle(mountPath, authMiddleware(httpServer)) + security.mountMetadata(mux) + mux.Handle(mountPath, security.protect(httpServer)) return mux } -func (h *Handler) AsSSE(mountPath string) http.Handler { +func (h *Handler) AsSSE(mountPath string, security RemoteSecurity) http.Handler { mux := http.NewServeMux() sseServer := server.NewSSEServer(h.server, server.WithStaticBasePath(mountPath)) - mux.Handle(mountPath, authMiddleware(sseServer)) + security.mountMetadata(mux) + mux.Handle(mountPath, security.protect(sseServer)) return mux } func (h *Handler) ServeInProcessClient() (*client.Client, error) { return client.NewInProcessClient(h.server) } - -func authMiddleware(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - slog.Debug("starting middleware", "headers", r.Header.Clone()) - ctx := r.Context() - - if host := r.Header.Get("X-Sysdig-Host"); host != "" { - slog.Debug("setting up host", "host", host) - ctx = sysdig.WrapContextWithHost(ctx, host) - } - - var token string - authHeader := r.Header.Get("Authorization") - if authHeader != "" { - parts := strings.Split(authHeader, " ") - if len(parts) == 2 && strings.ToLower(parts[0]) == "bearer" { - token = parts[1] - } - } - - if token == "" { - token = r.Header.Get("X-Sysdig-Token") - } - - if token != "" { - slog.Debug("setting up token", "token", token) - ctx = sysdig.WrapContextWithToken(ctx, token) - } - - next.ServeHTTP(w, r.WithContext(ctx)) - }) -} diff --git a/internal/infra/mcp/mcp_handler_test.go b/internal/infra/mcp/mcp_handler_test.go index c134924..0f90567 100644 --- a/internal/infra/mcp/mcp_handler_test.go +++ b/internal/infra/mcp/mcp_handler_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "log/slog" @@ -18,13 +19,19 @@ import ( . "github.com/onsi/gomega" "go.uber.org/mock/gomock" + infraauth "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/auth" localmcp "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/mcp" "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/mcp/tools" "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/sysdig" "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/sysdig/mocks" ) -// dummyTool implements the interface required by Handler.RegisterTools +const ( + validMCPToken = "mcp-access-token" + allowedOrigin = "https://client.example.com" + resourceURL = "https://mcp.example.com/sysdig-mcp-server" +) + type dummyTool struct { name string requiredPermissions []string @@ -32,20 +39,47 @@ type dummyTool struct { func (d *dummyTool) RegisterInServer(s *server.MCPServer) { tool := mcp.NewTool(d.name, mcp.WithDescription("dummy tool")) - // Initialize Meta to avoid nil pointer issues in strict checks if tool.Meta == nil { - tool.Meta = &mcp.Meta{ - AdditionalFields: make(map[string]any), - } + tool.Meta = &mcp.Meta{AdditionalFields: make(map[string]any)} } if len(d.requiredPermissions) > 0 { tools.WithRequiredPermissions(d.requiredPermissions...)(&tool) } - s.AddTool(tool, func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + s.AddTool(tool, func(context.Context, mcp.CallToolRequest) (*mcp.CallToolResult, error) { return mcp.NewToolResultText("success"), nil }) } +type fakeTokenVerifier struct { + validToken string + calls int +} + +func (v *fakeTokenVerifier) Verify(_ context.Context, rawToken string) error { + v.calls++ + if rawToken == "insufficient-scope" { + return infraauth.ErrInsufficientScope + } + if rawToken != v.validToken { + return errors.New("invalid access token") + } + return nil +} + +func remoteSecurity(verifier *fakeTokenVerifier) localmcp.RemoteSecurity { + return localmcp.NewRemoteSecurity( + verifier, + resourceURL, + "https://identity.example.com", + []string{"mcp:tools"}, + []string{allowedOrigin}, + ) +} + +func authorizationHeaders() http.Header { + return http.Header{"Authorization": []string{"Bearer " + validMCPToken}} +} + var _ = Describe("McpHandler", func() { var ( ctrl *gomock.Controller @@ -65,164 +99,194 @@ var _ = Describe("McpHandler", func() { }) Context("Permissions", func() { - It("should filter tools based on permissions", func() { - t1 := &dummyTool{name: "tool1", requiredPermissions: []string{"perm1"}} - t2 := &dummyTool{name: "tool2", requiredPermissions: []string{"perm2"}} - t3 := &dummyTool{name: "tool3" /* no permissions required */} - - handler.RegisterTools(t1, t2, t3) + It("filters tools based on permissions", func() { + handler.RegisterTools( + &dummyTool{name: "tool1", requiredPermissions: []string{"perm1"}}, + &dummyTool{name: "tool2", requiredPermissions: []string{"perm2"}}, + &dummyTool{name: "tool3"}, + ) mockClient.EXPECT().GetMyPermissionsWithResponse(gomock.Any()).Return(&sysdig.GetMyPermissionsResponse{ - HTTPResponse: &http.Response{StatusCode: 200}, - JSON200: &sysdig.UserPermissions{ - Permissions: []string{"perm1"}, - }, + HTTPResponse: &http.Response{StatusCode: http.StatusOK}, + JSON200: &sysdig.UserPermissions{Permissions: []string{"perm1"}}, }, nil) c := initializeInProcessClient(handler) - resp, err := c.ListTools(context.Background(), mcp.ListToolsRequest{}) Expect(err).NotTo(HaveOccurred()) var names []string - for _, t := range resp.Tools { - names = append(names, t.Name) + for _, tool := range resp.Tools { + names = append(names, tool.Name) } Expect(names).To(ConsistOf("tool1", "tool3")) }) - It("should handle permission errors gracefully", func() { - t1 := &dummyTool{name: "tool1", requiredPermissions: []string{"perm1"}} - handler.RegisterTools(t1) - - mockClient.EXPECT().GetMyPermissionsWithResponse(gomock.Any()).Return(nil, fmt.Errorf("error")) + It("handles permission errors without exposing tools", func() { + handler.RegisterTools(&dummyTool{name: "tool1", requiredPermissions: []string{"perm1"}}) + mockClient.EXPECT().GetMyPermissionsWithResponse(gomock.Any()).Return(nil, fmt.Errorf("permission lookup failed")) c := initializeInProcessClient(handler) - resp, err := c.ListTools(context.Background(), mcp.ListToolsRequest{}) Expect(err).NotTo(HaveOccurred()) Expect(resp.Tools).To(BeEmpty()) }) }) - Context("HTTP Handlers and Middleware", func() { - var testClient *HTTPTestClient + Context("Remote trust boundary", func() { + var ( + verifier *fakeTokenVerifier + testClient *HTTPTestClient + ) BeforeEach(func() { - // Default middleware setup for HTTP tests - h := handler.AsStreamableHTTP("/", false) - testClient = NewHTTPTestClient(h) + verifier = &fakeTokenVerifier{validToken: validMCPToken} + testClient = NewHTTPTestClient(handler.AsStreamableHTTP("/", false, remoteSecurity(verifier))) + }) + + It("serves an authenticated Streamable HTTP session", func(ctx SpecContext) { + handler.RegisterTools(&dummyTool{name: "tool1", requiredPermissions: []string{"perm1"}}) + mockClient.EXPECT().GetMyPermissionsWithResponse(gomock.Any()).Return(&sysdig.GetMyPermissionsResponse{ + HTTPResponse: &http.Response{StatusCode: http.StatusOK}, + JSON200: &sysdig.UserPermissions{Permissions: []string{"perm1"}}, + }, nil) + + testClient.Initialize(ctx, authorizationHeaders()) + resp := testClient.ListTools(ctx, authorizationHeaders()) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + Expect(verifier.calls).To(Equal(2)) + }, NodeTimeout(5*time.Second)) + + DescribeTable("rejects invalid authorization headers", + func(headers http.Header) { + resp := testClient.RPC(context.Background(), "tools/list", nil, headers) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized)) + Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring("resource_metadata=")) + }, + Entry("missing", http.Header{}), + Entry("wrong scheme", http.Header{"Authorization": []string{"Basic abc"}}), + Entry("empty bearer", http.Header{"Authorization": []string{"Bearer"}}), + Entry("duplicate", http.Header{"Authorization": []string{"Bearer one", "Bearer two"}}), + ) + + It("returns invalid_token when JWT verification fails", func() { + headers := http.Header{"Authorization": []string{"Bearer wrong-token"}} + resp := testClient.RPC(context.Background(), "tools/list", nil, headers) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized)) + Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring(`error="invalid_token"`)) }) - It("AsStreamableHTTP should serve correctly and middleware should extract headers", func(ctx SpecContext) { - expectedHost := "https://test.sysdig.com" - expectedToken := "my-token" + It("returns insufficient_scope with 403 for a valid but under-scoped token", func() { + headers := http.Header{"Authorization": []string{"Bearer insufficient-scope"}} + resp := testClient.RPC(context.Background(), "tools/list", nil, headers) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusForbidden)) + Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring(`error="insufficient_scope"`)) + Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring(`scope="mcp:tools"`)) + }) - t1 := &dummyTool{name: "tool1", requiredPermissions: []string{"perm1"}} - handler.RegisterTools(t1) + It("rejects non-allowlisted browser origins before token verification", func() { + headers := authorizationHeaders() + headers.Set("Origin", "https://evil.example.com") + resp := testClient.RPC(context.Background(), "tools/list", nil, headers) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusForbidden)) + Expect(verifier.calls).To(BeZero()) + }) - mockClient.EXPECT(). - GetMyPermissionsWithResponse(gomock.Any(), gomock.Any()). - DoAndReturn(func(c context.Context, reqEditors ...sysdig.RequestEditorFn) (*sysdig.GetMyPermissionsResponse, error) { - Expect(sysdig.GetHostFromContext(c)).To(Equal(expectedHost)) - Expect(sysdig.GetTokenFromContext(c)).To(Equal(expectedToken)) + It("returns CORS headers only for an exact allowlisted origin", func(ctx SpecContext) { + headers := authorizationHeaders() + headers.Set("Origin", allowedOrigin) + testClient.Initialize(ctx, headers) + resp := testClient.RPC(ctx, "ping", nil, headers) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + Expect(resp.Header.Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) + Expect(resp.Header.Get("Vary")).To(ContainSubstring("Origin")) + }, NodeTimeout(5*time.Second)) + + It("answers an allowlisted CORS preflight without a token", func() { + req := httptest.NewRequest(http.MethodOptions, "/", nil) + req.Header.Set("Origin", allowedOrigin) + req.Header.Set("Access-Control-Request-Method", http.MethodPost) + recorder := httptest.NewRecorder() + testClient.handler.ServeHTTP(recorder, req) + + Expect(recorder.Code).To(Equal(http.StatusNoContent)) + Expect(recorder.Header().Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) + Expect(recorder.Header().Get("Access-Control-Allow-Headers")).To(ContainSubstring("Authorization")) + Expect(verifier.calls).To(BeZero()) + }) - return &sysdig.GetMyPermissionsResponse{ - HTTPResponse: &http.Response{StatusCode: 200}, - JSON200: &sysdig.UserPermissions{ - Permissions: []string{"perm1"}, - }, - }, nil - }) + It("publishes OAuth protected-resource metadata without authentication", func() { + req := httptest.NewRequest(http.MethodGet, "/.well-known/oauth-protected-resource/sysdig-mcp-server", nil) + recorder := httptest.NewRecorder() + testClient.handler.ServeHTTP(recorder, req) - testClient.Initialize(ctx) + Expect(recorder.Code).To(Equal(http.StatusOK)) + Expect(recorder.Header().Get("Content-Type")).To(ContainSubstring("application/json")) + Expect(recorder.Body.String()).To(ContainSubstring(resourceURL)) + Expect(recorder.Body.String()).To(ContainSubstring("https://identity.example.com")) + Expect(verifier.calls).To(BeZero()) + }) - headers := map[string]string{ - "X-Sysdig-Host": expectedHost, - "Authorization": "Bearer " + expectedToken, - } - respList := testClient.ListTools(ctx, headers) - Expect(respList.StatusCode).To(Equal(http.StatusOK)) - }, NodeTimeout(time.Second*5)) + It("never forwards the MCP access token to Sysdig", func(ctx SpecContext) { + var upstreamAuthorization string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + upstreamAuthorization = r.Header.Get("Authorization") + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"permissions":[]}`)) + })) + defer upstream.Close() - It("should handle X-Sysdig-Token header in middleware", func(ctx SpecContext) { - expectedToken := "token-header" + sysdigClient, err := sysdig.NewSysdigClient(sysdig.WithFixedHostAndToken(upstream.URL, "server-side-sysdig-token")) + Expect(err).NotTo(HaveOccurred()) + isolatedHandler := localmcp.NewHandler("dev", sysdigClient) + isolatedHandler.RegisterTools(&dummyTool{name: "tool1"}) + client := NewHTTPTestClient(isolatedHandler.AsStreamableHTTP("/", false, remoteSecurity(verifier))) - handler.RegisterTools(&dummyTool{name: "tool1", requiredPermissions: []string{"perm1"}}) + client.Initialize(ctx, authorizationHeaders()) + resp := client.ListTools(ctx, authorizationHeaders()) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + Expect(upstreamAuthorization).To(Equal("Bearer server-side-sysdig-token")) + Expect(upstreamAuthorization).NotTo(ContainSubstring(validMCPToken)) + }, NodeTimeout(5*time.Second)) - mockClient.EXPECT(). - GetMyPermissionsWithResponse(gomock.Any(), gomock.Any()). - DoAndReturn(func(c context.Context, reqEditors ...sysdig.RequestEditorFn) (*sysdig.GetMyPermissionsResponse, error) { - Expect(sysdig.GetTokenFromContext(c)).To(Equal(expectedToken)) - - return &sysdig.GetMyPermissionsResponse{ - HTTPResponse: &http.Response{StatusCode: 200}, - JSON200: &sysdig.UserPermissions{Permissions: []string{"perm1"}}, - }, nil - }) - - testClient.Initialize(ctx) - - headers := map[string]string{"X-Sysdig-Token": expectedToken} - respList := testClient.ListTools(ctx, headers) - Expect(respList.StatusCode).To(Equal(http.StatusOK)) - }, NodeTimeout(time.Second*5)) - - It("should handle request with no auth headers", func(ctx SpecContext) { - mockClient.EXPECT(). - GetMyPermissionsWithResponse(gomock.Any(), gomock.Any()). - Return(nil, fmt.Errorf("no auth")) - - testClient.Initialize(ctx) - respList := testClient.ListTools(ctx, nil) - Expect(respList.StatusCode).To(Equal(http.StatusOK)) - }, NodeTimeout(time.Second*5)) - - It("AsSSE should return a handler", func() { - h := handler.AsSSE("/sse") - Expect(h).NotTo(BeNil()) + It("constructs a protected SSE handler", func() { + Expect(handler.AsSSE("/sse", remoteSecurity(verifier))).NotTo(BeNil()) }) - It("AsStreamableHTTP with stateless should serve tools/list without initialize", func(ctx SpecContext) { - h := handler.AsStreamableHTTP("/", true) - statelessClient := NewHTTPTestClient(h) - + It("serves stateless calls without initialization", func(ctx SpecContext) { + statelessClient := NewHTTPTestClient(handler.AsStreamableHTTP("/", true, remoteSecurity(verifier))) handler.RegisterTools(&dummyTool{name: "tool1"}) + mockClient.EXPECT().GetMyPermissionsWithResponse(gomock.Any()).Return(&sysdig.GetMyPermissionsResponse{ + HTTPResponse: &http.Response{StatusCode: http.StatusOK}, + JSON200: &sysdig.UserPermissions{Permissions: []string{}}, + }, nil) - mockClient.EXPECT(). - GetMyPermissionsWithResponse(gomock.Any(), gomock.Any()). - Return(&sysdig.GetMyPermissionsResponse{ - HTTPResponse: &http.Response{StatusCode: 200}, - JSON200: &sysdig.UserPermissions{Permissions: []string{}}, - }, nil) - - // Call tools/list directly without initialize — should work in stateless mode - resp := statelessClient.ListTools(ctx, nil) + resp := statelessClient.ListTools(ctx, authorizationHeaders()) + defer func() { _ = resp.Body.Close() }() Expect(resp.StatusCode).To(Equal(http.StatusOK)) Expect(resp.Header.Get("Mcp-Session-Id")).To(BeEmpty()) - }, NodeTimeout(time.Second*5)) + }, NodeTimeout(5*time.Second)) }) Context("Stdio", func() { - It("ServeStdio should return when context is cancelled", func(ctx SpecContext) { - c, cancel := context.WithCancel(ctx) + It("returns when the context is cancelled", func(ctx SpecContext) { + cancelled, cancel := context.WithCancel(ctx) cancel() + reader, writer := io.Pipe() + defer func() { _ = writer.Close() }() - r, w := io.Pipe() - defer func() { _ = w.Close() }() - - err := handler.ServeStdio(c, r, io.Discard) - // ServeStdio typically returns when the context is canceled or IO stream ends. - // We just want to ensure it doesn't block indefinitely and exits. - if err != nil { - Expect(err).To(HaveOccurred()) - } - }, NodeTimeout(time.Second*5)) + _ = handler.ServeStdio(cancelled, reader, io.Discard) + }, NodeTimeout(5*time.Second)) }) }) -// Helpers - func initializeInProcessClient(handler *localmcp.Handler) *client.Client { c, err := handler.ServeInProcessClient() Expect(err).NotTo(HaveOccurred()) @@ -237,13 +301,10 @@ type HTTPTestClient struct { } func NewHTTPTestClient(handler http.Handler) *HTTPTestClient { - return &HTTPTestClient{ - handler: handler, - } + return &HTTPTestClient{handler: handler} } -// RPC sends a JSON-RPC 2.0 request -func (c *HTTPTestClient) RPC(ctx context.Context, method string, params any, headers map[string]string) *http.Response { +func (c *HTTPTestClient) RPC(ctx context.Context, method string, params any, headers http.Header) *http.Response { payload := map[string]any{ "jsonrpc": "2.0", "id": 1, @@ -255,15 +316,16 @@ func (c *HTTPTestClient) RPC(ctx context.Context, method string, params any, hea body, err := json.Marshal(payload) Expect(err).NotTo(HaveOccurred()) - - req, err := http.NewRequestWithContext(ctx, "POST", "/", bytes.NewReader(body)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, "/", bytes.NewReader(body)) Expect(err).NotTo(HaveOccurred()) req.Header.Set("Content-Type", "application/json") if c.sessionID != "" { req.Header.Set("Mcp-Session-Id", c.sessionID) } - for k, v := range headers { - req.Header.Set(k, v) + for key, values := range headers { + for _, value := range values { + req.Header.Add(key, value) + } } recorder := httptest.NewRecorder() @@ -271,7 +333,7 @@ func (c *HTTPTestClient) RPC(ctx context.Context, method string, params any, hea return recorder.Result() } -func (c *HTTPTestClient) Initialize(ctx context.Context) { +func (c *HTTPTestClient) Initialize(ctx context.Context, headers http.Header) { params := mcp.InitializeParams{ ProtocolVersion: "2024-11-05", ClientInfo: mcp.Implementation{ @@ -281,13 +343,12 @@ func (c *HTTPTestClient) Initialize(ctx context.Context) { Capabilities: mcp.ClientCapabilities{}, } - resp := c.RPC(ctx, "initialize", params, nil) + resp := c.RPC(ctx, "initialize", params, headers) defer func() { _ = resp.Body.Close() }() - Expect(resp.StatusCode).To(Equal(http.StatusOK)) c.sessionID = resp.Header.Get("Mcp-Session-Id") } -func (c *HTTPTestClient) ListTools(ctx context.Context, headers map[string]string) *http.Response { +func (c *HTTPTestClient) ListTools(ctx context.Context, headers http.Header) *http.Response { return c.RPC(ctx, "tools/list", nil, headers) } diff --git a/internal/infra/mcp/remote_security.go b/internal/infra/mcp/remote_security.go new file mode 100644 index 0000000..3ab4ee1 --- /dev/null +++ b/internal/infra/mcp/remote_security.go @@ -0,0 +1,133 @@ +package mcp + +import ( + "errors" + "fmt" + "net/http" + "net/url" + "strings" + + "github.com/mark3labs/mcp-go/server" + infraauth "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/auth" +) + +type RemoteSecurity struct { + verifier infraauth.TokenVerifier + allowedOrigins map[string]struct{} + metadata server.ProtectedResourceMetadataConfig + metadataURL string +} + +func NewRemoteSecurity( + verifier infraauth.TokenVerifier, + resourceURL string, + authorizationServer string, + requiredScopes []string, + allowedOrigins []string, +) RemoteSecurity { + origins := make(map[string]struct{}, len(allowedOrigins)) + for _, origin := range allowedOrigins { + origins[origin] = struct{}{} + } + + metadata := server.ProtectedResourceMetadataConfig{ + Resource: resourceURL, + AuthorizationServers: []string{authorizationServer}, + ScopesSupported: requiredScopes, + BearerMethodsSupported: []string{"header"}, + ResourceName: "Sysdig MCP Server", + } + + return RemoteSecurity{ + verifier: verifier, + allowedOrigins: origins, + metadata: metadata, + metadataURL: protectedResourceMetadataURL(resourceURL), + } +} + +func (s RemoteSecurity) protect(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Vary", "Origin") + if origin := r.Header.Get("Origin"); origin != "" { + if _, allowed := s.allowedOrigins[origin]; !allowed { + http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) + return + } + w.Header().Set("Access-Control-Allow-Origin", origin) + w.Header().Set("Access-Control-Expose-Headers", "Mcp-Session-Id, WWW-Authenticate") + + if r.Method == http.MethodOptions { + w.Header().Set("Access-Control-Allow-Methods", "GET, POST, DELETE, OPTIONS") + w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type, Last-Event-ID, Mcp-Protocol-Version, Mcp-Session-Id") + w.WriteHeader(http.StatusNoContent) + return + } + } + + rawToken, ok := bearerToken(r.Header.Values("Authorization")) + if !ok { + s.writeUnauthorized(w, "") + return + } + + if err := s.verifier.Verify(r.Context(), rawToken); err != nil { + if errors.Is(err, infraauth.ErrInsufficientScope) { + s.writeInsufficientScope(w) + return + } + s.writeUnauthorized(w, "invalid_token") + return + } + + next.ServeHTTP(w, r) + }) +} + +func (s RemoteSecurity) writeInsufficientScope(w http.ResponseWriter) { + challenge := fmt.Sprintf( + `Bearer resource_metadata=%q, error="insufficient_scope", scope=%q`, + s.metadataURL, + strings.Join(s.metadata.ScopesSupported, " "), + ) + w.Header().Set("WWW-Authenticate", challenge) + http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) +} + +func (s RemoteSecurity) mountMetadata(mux *http.ServeMux) { + mux.Handle(server.ProtectedResourceMetadataPath(s.metadata.Resource), server.NewProtectedResourceMetadataHandler(s.metadata)) +} + +func (s RemoteSecurity) writeUnauthorized(w http.ResponseWriter, authError string) { + challenge := fmt.Sprintf(`Bearer resource_metadata=%q`, s.metadataURL) + if authError != "" { + challenge += fmt.Sprintf(`, error=%q`, authError) + } + w.Header().Set("WWW-Authenticate", challenge) + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) +} + +func bearerToken(values []string) (string, bool) { + if len(values) != 1 { + return "", false + } + + parts := strings.Fields(values[0]) + if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") || parts[1] == "" { + return "", false + } + + return parts[1], true +} + +func protectedResourceMetadataURL(resource string) string { + u, err := url.Parse(resource) + if err != nil { + return "" + } + u.Path = server.ProtectedResourceMetadataPath(resource) + u.RawPath = "" + u.RawQuery = "" + u.Fragment = "" + return u.String() +} diff --git a/internal/infra/sysdig/client.go b/internal/infra/sysdig/client.go index c7d802a..018dc5f 100644 --- a/internal/infra/sysdig/client.go +++ b/internal/infra/sysdig/client.go @@ -2,43 +2,18 @@ package sysdig import ( "context" - "errors" "fmt" "net/http" "net/url" ) -type contextKey string - -const ( - contextKeyToken contextKey = "sysdigApiToken" - contextKeyHost contextKey = "sysdigApiHost" -) - -func WrapContextWithToken(ctx context.Context, token string) context.Context { - return context.WithValue(ctx, contextKeyToken, token) -} - -func GetTokenFromContext(ctx context.Context) string { - return ctx.Value(contextKeyToken).(string) -} - -func WrapContextWithHost(ctx context.Context, host string) context.Context { - return context.WithValue(ctx, contextKeyHost, host) -} - -func GetHostFromContext(ctx context.Context) string { - return ctx.Value(contextKeyHost).(string) -} - func updateReqWithHostURL(req *http.Request, host string) error { u, err := url.Parse(host) if err != nil { - // If it's just a hostname without scheme, try prepending https:// - u, err = url.Parse("https://" + host) - if err != nil { - return err - } + return err + } + if !u.IsAbs() || u.Host == "" { + return fmt.Errorf("Sysdig API host must be an absolute URL") } req.URL.Scheme = u.Scheme req.URL.Host = u.Host @@ -55,32 +30,6 @@ func WithFixedHostAndToken(host, apiToken string) RequestEditorFn { } } -func WithHostAndTokenFromContext() RequestEditorFn { - return func(ctx context.Context, req *http.Request) error { - if host, ok := ctx.Value(contextKeyHost).(string); ok && host != "" { - if err := updateReqWithHostURL(req, host); err != nil { - return err - } - } - if token, ok := ctx.Value(contextKeyToken).(string); ok && token != "" { - req.Header.Set("Authorization", "Bearer "+token) - return nil - } - return errors.New("authorization token not present in context") - } -} - -func WithFallbackAuthentication(auths ...RequestEditorFn) RequestEditorFn { - return func(ctx context.Context, req *http.Request) error { - for _, auth := range auths { - if err := auth(ctx, req); err == nil { - return nil - } - } - return errors.New("unable to authenticate with any method") - } -} - func WithVersion(version string) RequestEditorFn { return func(ctx context.Context, req *http.Request) error { req.Header.Set("User-Agent", fmt.Sprintf("sysdig-mcp-server/%s", version)) diff --git a/internal/infra/sysdig/client_permissions_integration_test.go b/internal/infra/sysdig/client_permissions_integration_test.go index 704cd84..8837c2c 100644 --- a/internal/infra/sysdig/client_permissions_integration_test.go +++ b/internal/infra/sysdig/client_permissions_integration_test.go @@ -18,6 +18,9 @@ var _ = Describe("Sysdig Permissions Client", func() { BeforeEach(func() { sysdigURL = os.Getenv("SYSDIG_MCP_API_HOST") sysdigToken = os.Getenv("SYSDIG_MCP_API_TOKEN") + if sysdigURL == "" || sysdigToken == "" { + Skip("requires SYSDIG_MCP_API_HOST and SYSDIG_MCP_API_TOKEN") + } }) Context("when fetching user permissions", func() { @@ -33,33 +36,4 @@ var _ = Describe("Sysdig Permissions Client", func() { Expect(resp.JSON200.Permissions).ToNot(BeEmpty()) }) }) - - When("a token from context is used", func() { - It("loads the token from the context", func(ctx context.Context) { - var err error - client, err = sysdig.NewSysdigClient(sysdig.WithHostAndTokenFromContext()) - Expect(err).ToNot(HaveOccurred()) - - ctx = sysdig.WrapContextWithHost(ctx, sysdigURL) - ctx = sysdig.WrapContextWithToken(ctx, sysdigToken) - - resp, err := client.GetMyPermissionsWithResponse(ctx) - Expect(err).ToNot(HaveOccurred()) - Expect(resp.HTTPResponse.StatusCode).To(Equal(http.StatusOK)) - Expect(resp.JSON200).ToNot(BeNil()) - Expect(resp.JSON200.Permissions).ToNot(BeEmpty()) - }) - - When("the token is not in the context", func() { - It("fails to retrieve the permissions", func(ctx context.Context) { - var err error - client, err = sysdig.NewSysdigClient(sysdig.WithHostAndTokenFromContext()) - Expect(err).ToNot(HaveOccurred()) - - resp, err := client.GetMyPermissionsWithResponse(ctx) - Expect(err).To(MatchError("authorization token not present in context")) - Expect(resp).To(BeNil()) - }) - }) - }) }) diff --git a/internal/infra/sysdig/client_test.go b/internal/infra/sysdig/client_test.go index f40124e..a0930b1 100644 --- a/internal/infra/sysdig/client_test.go +++ b/internal/infra/sysdig/client_test.go @@ -67,30 +67,6 @@ var _ = Describe("Client TLS", func() { }) }) -var _ = Describe("Context helpers", func() { - It("roundtrips token through context", func() { - ctx := sysdig.WrapContextWithToken(context.Background(), "my-token") - Expect(sysdig.GetTokenFromContext(ctx)).To(Equal("my-token")) - }) - - It("roundtrips host through context", func() { - ctx := sysdig.WrapContextWithHost(context.Background(), "https://example.com") - Expect(sysdig.GetHostFromContext(ctx)).To(Equal("https://example.com")) - }) - - It("panics when token is missing from context", func() { - Expect(func() { - sysdig.GetTokenFromContext(context.Background()) - }).To(Panic()) - }) - - It("panics when host is missing from context", func() { - Expect(func() { - sysdig.GetHostFromContext(context.Background()) - }).To(Panic()) - }) -}) - var _ = Describe("Client authentication", func() { var ts *httptest.Server var lastHeaders http.Header @@ -105,74 +81,26 @@ var _ = Describe("Client authentication", func() { ts.Close() }) - Describe("WithHostAndTokenFromContext", func() { - It("authenticates using context values", func() { - client, err := sysdig.NewSysdigClient(sysdig.WithHostAndTokenFromContext()) - Expect(err).NotTo(HaveOccurred()) - - ctx := sysdig.WrapContextWithHost(context.Background(), ts.URL) - ctx = sysdig.WrapContextWithToken(ctx, "ctx-token") - - resp, err := client.GetMyPermissionsWithResponse(ctx) - Expect(err).NotTo(HaveOccurred()) - Expect(resp.HTTPResponse.StatusCode).To(Equal(http.StatusOK)) - Expect(lastHeaders.Get("Authorization")).To(Equal("Bearer ctx-token")) - }) - - It("fails when token is missing from context", func() { - client, err := sysdig.NewSysdigClient(sysdig.WithHostAndTokenFromContext()) - Expect(err).NotTo(HaveOccurred()) - - ctx := sysdig.WrapContextWithHost(context.Background(), ts.URL) + It("always sends the configured server-side token", func() { + client, err := sysdig.NewSysdigClient( + sysdig.WithFixedHostAndToken(ts.URL, "server-token"), + ) + Expect(err).NotTo(HaveOccurred()) - _, err = client.GetMyPermissionsWithResponse(ctx) - Expect(err).To(MatchError(ContainSubstring("authorization token not present"))) - }) + resp, err := client.GetMyPermissionsWithResponse(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.HTTPResponse.StatusCode).To(Equal(http.StatusOK)) + Expect(lastHeaders.Get("Authorization")).To(Equal("Bearer server-token")) }) - Describe("WithFallbackAuthentication", func() { - It("uses first auth when it succeeds", func() { - client, err := sysdig.NewSysdigClient( - sysdig.WithFallbackAuthentication( - sysdig.WithFixedHostAndToken(ts.URL, "primary-token"), - sysdig.WithFixedHostAndToken(ts.URL, "fallback-token"), - ), - ) - Expect(err).NotTo(HaveOccurred()) - - resp, err := client.GetMyPermissionsWithResponse(context.Background()) - Expect(err).NotTo(HaveOccurred()) - Expect(resp.HTTPResponse.StatusCode).To(Equal(http.StatusOK)) - Expect(lastHeaders.Get("Authorization")).To(Equal("Bearer primary-token")) - }) - - It("falls back to second auth when first fails", func() { - client, err := sysdig.NewSysdigClient( - sysdig.WithFallbackAuthentication( - sysdig.WithHostAndTokenFromContext(), - sysdig.WithFixedHostAndToken(ts.URL, "fallback-token"), - ), - ) - Expect(err).NotTo(HaveOccurred()) - - resp, err := client.GetMyPermissionsWithResponse(context.Background()) - Expect(err).NotTo(HaveOccurred()) - Expect(resp.HTTPResponse.StatusCode).To(Equal(http.StatusOK)) - Expect(lastHeaders.Get("Authorization")).To(Equal("Bearer fallback-token")) - }) - - It("fails when all auth methods fail", func() { - client, err := sysdig.NewSysdigClient( - sysdig.WithFallbackAuthentication( - sysdig.WithHostAndTokenFromContext(), - sysdig.WithHostAndTokenFromContext(), - ), - ) - Expect(err).NotTo(HaveOccurred()) + It("rejects a non-absolute configured host", func() { + client, err := sysdig.NewSysdigClient( + sysdig.WithFixedHostAndToken("app.example.com", "server-token"), + ) + Expect(err).NotTo(HaveOccurred()) - _, err = client.GetMyPermissionsWithResponse(context.Background()) - Expect(err).To(MatchError(ContainSubstring("unable to authenticate"))) - }) + _, err = client.GetMyPermissionsWithResponse(context.Background()) + Expect(err).To(MatchError(ContainSubstring("absolute URL"))) }) Describe("WithVersion", func() { diff --git a/package.nix b/package.nix index 0193e02..f7b2efa 100644 --- a/package.nix +++ b/package.nix @@ -4,7 +4,7 @@ buildGoLatestModule (finalAttrs: { version = "3.0.4"; src = ./.; # This hash is automatically re-calculated with `just rehash-package-nix`. This is automatically called as well by `just update`. - vendorHash = "sha256-Tg3OLihG0lJRb5u0/+EMy/0LhVEsTn6a60K6y1eMgo0="; + vendorHash = "sha256-XVCFDHEGY3du2YRJjbEgJwKoNiayjPTPsT9WBvcH2sk="; subPackages = [ "cmd/server" From f851af04b5a2198ffce8f8b122735cd349200856 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:06:56 +0200 Subject: [PATCH 02/19] fix(auth): address remote boundary review findings Fix SSE routing and CORS integration, tighten remote URL and scope validation, preserve Sysdig API path prefixes, add independent JWKS TLS policy, harden HTTP timeouts, and bump the breaking release to v4.0.0. --- cmd/server/main.go | 28 ++-- docs/TROUBLESHOOTING.md | 11 +- internal/config/config.go | 163 +++++++++++++++------ internal/config/config_test.go | 28 +++- internal/infra/auth/token_verifier.go | 32 +++- internal/infra/auth/token_verifier_test.go | 42 +++++- internal/infra/mcp/mcp_handler.go | 31 +++- internal/infra/mcp/mcp_handler_test.go | 74 +++++++++- internal/infra/mcp/remote_security.go | 71 ++++++--- internal/infra/sysdig/client.go | 11 ++ internal/infra/sysdig/client_test.go | 34 +++++ package.nix | 2 +- 12 files changed, 437 insertions(+), 90 deletions(-) diff --git a/cmd/server/main.go b/cmd/server/main.go index 0cecc8c..a6379ce 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -91,14 +91,7 @@ func setupSysdigClient(cfg *config.Config) (sysdig.ExtendedClientWithResponsesIn } if cfg.SkipTLSVerification { - transport := http.DefaultTransport.(*http.Transport).Clone() - if transport.TLSClientConfig == nil { - transport.TLSClientConfig = &tls.Config{} - } - transport.TLSClientConfig.InsecureSkipVerify = true - httpClient := &http.Client{Transport: transport} - - sysdigClientOptions = append(sysdigClientOptions, sysdig.WithHTTPClient(httpClient)) + sysdigClientOptions = append(sysdigClientOptions, sysdig.WithHTTPClient(insecureHTTPClient())) } sysdigClient, err := sysdig.NewSysdigClient(sysdigClientOptions...) @@ -109,13 +102,19 @@ func setupSysdigClient(cfg *config.Config) (sysdig.ExtendedClientWithResponsesIn } func setupRemoteSecurity(cfg *config.Config) mcp.RemoteSecurity { - verifier := infraauth.NewJWTVerifier( + var jwksHTTPClient *http.Client + if cfg.SkipJWKSTLSVerification { + jwksHTTPClient = insecureHTTPClient() + } + + verifier := infraauth.NewJWTVerifierWithHTTPClient( context.Background(), cfg.AuthIssuer, cfg.ResourceURL, cfg.AuthJWKSURL, cfg.AuthSigningAlgs, cfg.AuthScopes, + jwksHTTPClient, ) return mcp.NewRemoteSecurity( @@ -127,6 +126,15 @@ func setupRemoteSecurity(cfg *config.Config) mcp.RemoteSecurity { ) } +func insecureHTTPClient() *http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + if transport.TLSClientConfig == nil { + transport.TLSClientConfig = &tls.Config{} + } + transport.TLSClientConfig.InsecureSkipVerify = true + return &http.Client{Transport: transport} +} + func setupHandler(sysdigClient sysdig.ExtendedClientWithResponsesInterface) *mcp.Handler { systemClock := clock.NewSystemClock() handler := mcp.NewHandler(Version, sysdigClient) @@ -164,6 +172,7 @@ func startServer(cfg *config.Config, handler *mcp.Handler) error { Addr: addr, Handler: handler.AsStreamableHTTP(cfg.MountPath, cfg.Stateless, setupRemoteSecurity(cfg)), ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, IdleTimeout: 2 * time.Minute, } if err := server.ListenAndServe(); err != nil { @@ -176,6 +185,7 @@ func startServer(cfg *config.Config, handler *mcp.Handler) error { Addr: addr, Handler: handler.AsSSE(cfg.MountPath, setupRemoteSecurity(cfg)), ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, IdleTimeout: 2 * time.Minute, } if err := server.ListenAndServe(); err != nil { diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 6ca68bb..0af5cb1 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -4,16 +4,19 @@ - **Solution**: Check API token permissions match tool's `WithRequiredPermissions()`. The token must have **all** permissions listed. **Problem**: Server exits with a missing configuration error -- **Solution**: All transports require absolute `SYSDIG_MCP_API_HOST` and `SYSDIG_MCP_API_TOKEN` values. Remote transports also require `SYSDIG_MCP_RESOURCE_URL`, `SYSDIG_MCP_AUTH_ISSUER`, and `SYSDIG_MCP_AUTH_JWKS_URL`. +- **Solution**: All transports require absolute `SYSDIG_MCP_API_HOST` and `SYSDIG_MCP_API_TOKEN` values. Remote transports also require `SYSDIG_MCP_RESOURCE_URL`, `SYSDIG_MCP_AUTH_ISSUER`, and `SYSDIG_MCP_AUTH_JWKS_URL`. The resource URL path must exactly match `SYSDIG_MCP_MOUNT_PATH`. **Problem**: Remote request returns `401 Unauthorized` - **Solution**: Use an issuer-signed MCP access token, not the Sysdig API token. Verify its `iss`, `aud`, expiry, asymmetric signing algorithm, and required scopes. Follow the `resource_metadata` URL in the `WWW-Authenticate` response header to inspect the server's OAuth metadata. **Problem**: Browser request returns `403 Forbidden` -- **Solution**: Add the browser's exact origin to `SYSDIG_MCP_ALLOWED_ORIGINS`. Include only `scheme://authority`; wildcard origins and origins with paths are rejected. +- **Solution**: Add the browser's exact origin to `SYSDIG_MCP_ALLOWED_ORIGINS`. Include only `scheme://authority`; wildcard origins and origins with paths are rejected. Hostname matching is case-insensitive. -**Problem**: Connection failing with "certificate signed by unknown authority" -- **Solution**: If using a self-signed certificate (e.g. on-prem), set `SYSDIG_MCP_API_SKIP_TLS_VERIFICATION=true`. +**Problem**: Sysdig API connection fails with "certificate signed by unknown authority" +- **Solution**: If the Sysdig API uses a self-signed certificate (e.g. on-prem), set `SYSDIG_MCP_API_SKIP_TLS_VERIFICATION=true`. + +**Problem**: OAuth JWKS retrieval fails with "certificate signed by unknown authority" +- **Solution**: If the authorization server's JWKS endpoint uses a self-signed certificate, set `SYSDIG_MCP_AUTH_JWKS_SKIP_TLS_VERIFICATION=true`. This is intentionally separate from the Sysdig API TLS setting because the two endpoints are different trust domains. **Problem**: Tests failing with "command not found" - **Solution**: Enter Nix shell with `nix develop` or `direnv allow`. All dev tools are provided by the flake. diff --git a/internal/config/config.go b/internal/config/config.go index 3f93dfa..b05c6fe 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -5,27 +5,45 @@ import ( "net" "net/url" "os" + "path" "slices" "strconv" "strings" + "unicode" + + "github.com/coreos/go-oidc/v3/oidc" ) +var supportedAuthSigningAlgorithms = []string{ + oidc.RS256, + oidc.RS384, + oidc.RS512, + oidc.PS256, + oidc.PS384, + oidc.PS512, + oidc.ES256, + oidc.ES384, + oidc.ES512, + oidc.EdDSA, +} + type Config struct { - APIHost string - APIToken string - SkipTLSVerification bool - Transport string - ListeningHost string - ListeningPort string - MountPath string - LogLevel string - Stateless bool - ResourceURL string - AuthIssuer string - AuthJWKSURL string - AuthScopes []string - AuthSigningAlgs []string - AllowedOrigins []string + APIHost string + APIToken string + SkipTLSVerification bool + SkipJWKSTLSVerification bool + Transport string + ListeningHost string + ListeningPort string + MountPath string + LogLevel string + Stateless bool + ResourceURL string + AuthIssuer string + AuthJWKSURL string + AuthScopes []string + AuthSigningAlgs []string + AllowedOrigins []string } func (c *Config) Validate() error { @@ -38,13 +56,20 @@ func (c *Config) Validate() error { if c.APIToken == "" { return fmt.Errorf("required configuration missing: SYSDIG_MCP_API_TOKEN") } - if err := validateAbsoluteURL("SYSDIG_MCP_API_HOST", c.APIHost); err != nil { + apiHost, err := parseAbsoluteURL("SYSDIG_MCP_API_HOST", c.APIHost) + if err != nil { return err } + if apiHost.RawQuery != "" { + return fmt.Errorf("SYSDIG_MCP_API_HOST must not contain a query string") + } if c.Transport == "stdio" { return nil } + if err := validateMountPath(c.MountPath); err != nil { + return err + } if c.ResourceURL == "" { return fmt.Errorf("required configuration missing: SYSDIG_MCP_RESOURCE_URL") } @@ -54,13 +79,34 @@ func (c *Config) Validate() error { if c.AuthJWKSURL == "" { return fmt.Errorf("required configuration missing: SYSDIG_MCP_AUTH_JWKS_URL") } - if err := validateAbsoluteURL("SYSDIG_MCP_RESOURCE_URL", c.ResourceURL); err != nil { + + resourceURL, err := parseAbsoluteURL("SYSDIG_MCP_RESOURCE_URL", c.ResourceURL) + if err != nil { return err } - if err := validateAbsoluteURL("SYSDIG_MCP_AUTH_ISSUER", c.AuthIssuer); err != nil { + if resourceURL.RawQuery != "" { + return fmt.Errorf("SYSDIG_MCP_RESOURCE_URL must not contain a query string") + } + resourcePath := resourceURL.Path + if resourcePath == "" { + resourcePath = "/" + } + if resourcePath != c.MountPath { + return fmt.Errorf( + "SYSDIG_MCP_RESOURCE_URL path %q must match SYSDIG_MCP_MOUNT_PATH %q", + resourcePath, + c.MountPath, + ) + } + + authIssuer, err := parseAbsoluteURL("SYSDIG_MCP_AUTH_ISSUER", c.AuthIssuer) + if err != nil { return err } - if err := validateAbsoluteURL("SYSDIG_MCP_AUTH_JWKS_URL", c.AuthJWKSURL); err != nil { + if authIssuer.RawQuery != "" { + return fmt.Errorf("SYSDIG_MCP_AUTH_ISSUER must not contain a query string") + } + if _, err := parseAbsoluteURL("SYSDIG_MCP_AUTH_JWKS_URL", c.AuthJWKSURL); err != nil { return err } for _, origin := range c.AllowedOrigins { @@ -68,11 +114,16 @@ func (c *Config) Validate() error { return fmt.Errorf("invalid SYSDIG_MCP_ALLOWED_ORIGINS entry %q: %w", origin, err) } } + for _, scope := range c.AuthScopes { + if !validScopeToken(scope) { + return fmt.Errorf("invalid scope %q in SYSDIG_MCP_AUTH_SCOPES", scope) + } + } if len(c.AuthSigningAlgs) == 0 { return fmt.Errorf("SYSDIG_MCP_AUTH_SIGNING_ALGS must contain at least one asymmetric signing algorithm") } for _, algorithm := range c.AuthSigningAlgs { - if !slices.Contains([]string{"RS256", "RS384", "RS512", "PS256", "PS384", "PS512", "ES256", "ES384", "ES512", "EdDSA"}, algorithm) { + if !slices.Contains(supportedAuthSigningAlgorithms, algorithm) { return fmt.Errorf("unsupported asymmetric signing algorithm %q in SYSDIG_MCP_AUTH_SIGNING_ALGS", algorithm) } } @@ -81,21 +132,22 @@ func (c *Config) Validate() error { func Load() (*Config, error) { cfg := &Config{ - APIHost: getEnv("SYSDIG_MCP_API_HOST", ""), - APIToken: getEnv("SYSDIG_MCP_API_TOKEN", ""), - SkipTLSVerification: getEnv("SYSDIG_MCP_API_SKIP_TLS_VERIFICATION", false), - Transport: getEnv("SYSDIG_MCP_TRANSPORT", "stdio"), - ListeningHost: getEnv("SYSDIG_MCP_LISTENING_HOST", ""), - ListeningPort: getEnv("SYSDIG_MCP_LISTENING_PORT", "8080"), - MountPath: getEnv("SYSDIG_MCP_MOUNT_PATH", "/sysdig-mcp-server"), - LogLevel: getEnv("SYSDIG_MCP_LOGLEVEL", "INFO"), - Stateless: getEnv("SYSDIG_MCP_STATELESS", false), - ResourceURL: getEnv("SYSDIG_MCP_RESOURCE_URL", ""), - AuthIssuer: getEnv("SYSDIG_MCP_AUTH_ISSUER", ""), - AuthJWKSURL: getEnv("SYSDIG_MCP_AUTH_JWKS_URL", ""), - AuthScopes: getEnvList("SYSDIG_MCP_AUTH_SCOPES", nil), - AuthSigningAlgs: getEnvList("SYSDIG_MCP_AUTH_SIGNING_ALGS", []string{"RS256"}), - AllowedOrigins: getEnvList("SYSDIG_MCP_ALLOWED_ORIGINS", nil), + APIHost: getEnv("SYSDIG_MCP_API_HOST", ""), + APIToken: getEnv("SYSDIG_MCP_API_TOKEN", ""), + SkipTLSVerification: getEnv("SYSDIG_MCP_API_SKIP_TLS_VERIFICATION", false), + SkipJWKSTLSVerification: getEnv("SYSDIG_MCP_AUTH_JWKS_SKIP_TLS_VERIFICATION", false), + Transport: getEnv("SYSDIG_MCP_TRANSPORT", "stdio"), + ListeningHost: getEnv("SYSDIG_MCP_LISTENING_HOST", ""), + ListeningPort: getEnv("SYSDIG_MCP_LISTENING_PORT", "8080"), + MountPath: getEnv("SYSDIG_MCP_MOUNT_PATH", "/sysdig-mcp-server"), + LogLevel: getEnv("SYSDIG_MCP_LOGLEVEL", "INFO"), + Stateless: getEnv("SYSDIG_MCP_STATELESS", false), + ResourceURL: getEnv("SYSDIG_MCP_RESOURCE_URL", ""), + AuthIssuer: getEnv("SYSDIG_MCP_AUTH_ISSUER", ""), + AuthJWKSURL: getEnv("SYSDIG_MCP_AUTH_JWKS_URL", ""), + AuthScopes: getEnvList("SYSDIG_MCP_AUTH_SCOPES", nil), + AuthSigningAlgs: getEnvList("SYSDIG_MCP_AUTH_SIGNING_ALGS", []string{oidc.RS256}), + AllowedOrigins: getEnvList("SYSDIG_MCP_ALLOWED_ORIGINS", nil), } if err := cfg.Validate(); err != nil { @@ -112,20 +164,27 @@ func getEnvList(key string, fallback []string) []string { } return strings.FieldsFunc(value, func(r rune) bool { - return r == ',' || r == ' ' || r == '\t' || r == '\n' + return r == ',' || unicode.IsSpace(r) }) } -func validateAbsoluteURL(name, rawURL string) error { +func parseAbsoluteURL(name, rawURL string) (*url.URL, error) { u, err := url.Parse(rawURL) if err != nil || !u.IsAbs() || u.Host == "" { - return fmt.Errorf("%s must be an absolute URL", name) + return nil, fmt.Errorf("%s must be an absolute URL", name) } if u.User != nil || u.Fragment != "" { - return fmt.Errorf("%s must not contain user information or a fragment", name) + return nil, fmt.Errorf("%s must not contain user information or a fragment", name) } if u.Scheme != "https" && !(u.Scheme == "http" && isLoopbackHostname(u.Hostname())) { - return fmt.Errorf("%s must use https (http is allowed only for loopback development)", name) + return nil, fmt.Errorf("%s must use https (http is allowed only for loopback development)", name) + } + return u, nil +} + +func validateMountPath(mountPath string) error { + if mountPath == "" || !strings.HasPrefix(mountPath, "/") || path.Clean(mountPath) != mountPath { + return fmt.Errorf("SYSDIG_MCP_MOUNT_PATH must be an absolute, canonical URL path") } return nil } @@ -134,19 +193,29 @@ func validateOrigin(origin string) error { if origin == "*" { return fmt.Errorf("wildcard origins are not allowed") } - u, err := url.Parse(origin) - if err != nil || !u.IsAbs() || u.Host == "" { - return fmt.Errorf("origin must be an absolute URL") + u, err := parseAbsoluteURL("origin", origin) + if err != nil { + return err } - if u.User != nil || u.Path != "" || u.RawQuery != "" || u.Fragment != "" { + if u.Path != "" || u.RawQuery != "" { return fmt.Errorf("origin must contain only scheme and authority") } - if u.Scheme != "https" && !(u.Scheme == "http" && isLoopbackHostname(u.Hostname())) { - return fmt.Errorf("origin must use https (http is allowed only for loopback development)") - } return nil } +func validScopeToken(scope string) bool { + if scope == "" { + return false + } + for _, r := range scope { + // RFC 6749 scope-token = 1*( %x21 / %x23-5B / %x5D-7E ). + if r < 0x21 || r > 0x7e || r == '"' || r == '\\' { + return false + } + } + return true +} + func isLoopbackHostname(host string) bool { if strings.EqualFold(host, "localhost") { return true diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1199053..e8ab887 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -14,6 +14,7 @@ func validConfig(transport string) *config.Config { APIHost: "https://app.us4.sysdig.com", APIToken: "sysdig-token", Transport: transport, + MountPath: "/sysdig-mcp-server", } if transport != "stdio" { cfg.ResourceURL = "https://mcp.example.com/sysdig-mcp-server" @@ -64,22 +65,40 @@ var _ = Describe("Config", func() { Entry("JWKS URL", func(cfg *config.Config) { cfg.AuthJWKSURL = "" }, "SYSDIG_MCP_AUTH_JWKS_URL"), ) - DescribeTable("rejects unsafe URLs", + DescribeTable("rejects unsafe URLs and paths", func(mutate func(*config.Config), expected string) { cfg := validConfig("streamable-http") mutate(cfg) Expect(cfg.Validate()).To(MatchError(ContainSubstring(expected))) }, Entry("relative API host", func(cfg *config.Config) { cfg.APIHost = "app.example.com" }, "absolute URL"), - Entry("plaintext resource", func(cfg *config.Config) { cfg.ResourceURL = "http://mcp.example.com" }, "must use https"), + Entry("API host query", func(cfg *config.Config) { cfg.APIHost += "?tenant=one" }, "query string"), + Entry("plaintext resource", func(cfg *config.Config) { cfg.ResourceURL = "http://mcp.example.com/sysdig-mcp-server" }, "must use https"), + Entry("resource query", func(cfg *config.Config) { cfg.ResourceURL += "?tenant=one" }, "query string"), + Entry("resource path mismatch", func(cfg *config.Config) { cfg.ResourceURL = "https://mcp.example.com/other" }, "must match SYSDIG_MCP_MOUNT_PATH"), + Entry("non-canonical mount path", func(cfg *config.Config) { cfg.MountPath = "/sysdig-mcp-server/" }, "canonical URL path"), Entry("issuer with user info", func(cfg *config.Config) { cfg.AuthIssuer = "https://user@identity.example.com" }, "user information"), + Entry("issuer query", func(cfg *config.Config) { cfg.AuthIssuer += "?tenant=one" }, "query string"), Entry("fragmented JWKS URL", func(cfg *config.Config) { cfg.AuthJWKSURL += "#keys" }, "fragment"), Entry("wildcard origin", func(cfg *config.Config) { cfg.AllowedOrigins = []string{"*"} }, "wildcard"), Entry("origin path", func(cfg *config.Config) { cfg.AllowedOrigins = []string{"https://client.example.com/path"} }, "scheme and authority"), + Entry("invalid scope", func(cfg *config.Config) { cfg.AuthScopes = []string{"mcp:\"tools"} }, "invalid scope"), Entry("empty signing algorithms", func(cfg *config.Config) { cfg.AuthSigningAlgs = nil }, "at least one"), Entry("symmetric signing", func(cfg *config.Config) { cfg.AuthSigningAlgs = []string{"HS256"} }, "asymmetric signing algorithm"), ) + It("allows an API path prefix when it is otherwise safe", func() { + cfg := validConfig("streamable-http") + cfg.APIHost = "https://gateway.example.com/sysdig-proxy" + Expect(cfg.Validate()).To(Succeed()) + }) + + It("allows a JWKS URL with a query component", func() { + cfg := validConfig("streamable-http") + cfg.AuthJWKSURL = "https://identity.example.com/jwks?tenant=one" + Expect(cfg.Validate()).To(Succeed()) + }) + It("allows HTTP only for loopback development", func() { cfg := validConfig("streamable-http") cfg.APIHost = "http://127.0.0.1:9000" @@ -107,12 +126,14 @@ var _ = Describe("Config", func() { Expect(cfg.MountPath).To(Equal("/sysdig-mcp-server")) Expect(cfg.LogLevel).To(Equal("INFO")) Expect(cfg.SkipTLSVerification).To(BeFalse()) + Expect(cfg.SkipJWKSTLSVerification).To(BeFalse()) Expect(cfg.Stateless).To(BeFalse()) Expect(cfg.AuthSigningAlgs).To(Equal([]string{"RS256"})) }) It("loads all remote security values", func() { _ = os.Setenv("SYSDIG_MCP_API_SKIP_TLS_VERIFICATION", "true") + _ = os.Setenv("SYSDIG_MCP_AUTH_JWKS_SKIP_TLS_VERIFICATION", "true") _ = os.Setenv("SYSDIG_MCP_TRANSPORT", "streamable-http") _ = os.Setenv("SYSDIG_MCP_LISTENING_HOST", "0.0.0.0") _ = os.Setenv("SYSDIG_MCP_LISTENING_PORT", "9090") @@ -122,13 +143,14 @@ var _ = Describe("Config", func() { _ = os.Setenv("SYSDIG_MCP_RESOURCE_URL", "https://mcp.example.com/custom") _ = os.Setenv("SYSDIG_MCP_AUTH_ISSUER", "https://identity.example.com") _ = os.Setenv("SYSDIG_MCP_AUTH_JWKS_URL", "https://identity.example.com/jwks") - _ = os.Setenv("SYSDIG_MCP_AUTH_SCOPES", "mcp:tools, profile") + _ = os.Setenv("SYSDIG_MCP_AUTH_SCOPES", "mcp:tools,\r\n profile") _ = os.Setenv("SYSDIG_MCP_AUTH_SIGNING_ALGS", "RS256 ES256") _ = os.Setenv("SYSDIG_MCP_ALLOWED_ORIGINS", "https://one.example.com, https://two.example.com") cfg, err := config.Load() Expect(err).NotTo(HaveOccurred()) Expect(cfg.SkipTLSVerification).To(BeTrue()) + Expect(cfg.SkipJWKSTLSVerification).To(BeTrue()) Expect(cfg.Transport).To(Equal("streamable-http")) Expect(cfg.ListeningHost).To(Equal("0.0.0.0")) Expect(cfg.ListeningPort).To(Equal("9090")) diff --git a/internal/infra/auth/token_verifier.go b/internal/infra/auth/token_verifier.go index ba7834a..c5a1280 100644 --- a/internal/infra/auth/token_verifier.go +++ b/internal/infra/auth/token_verifier.go @@ -41,7 +41,37 @@ func NewJWTVerifier( signingAlgorithms []string, requiredScopes []string, ) *JWTVerifier { - ctx = oidc.ClientContext(ctx, &http.Client{Timeout: jwksRequestTimeout}) + return NewJWTVerifierWithHTTPClient( + ctx, + issuer, + audience, + jwksURL, + signingAlgorithms, + requiredScopes, + nil, + ) +} + +func NewJWTVerifierWithHTTPClient( + ctx context.Context, + issuer string, + audience string, + jwksURL string, + signingAlgorithms []string, + requiredScopes []string, + httpClient *http.Client, +) *JWTVerifier { + if httpClient == nil { + httpClient = &http.Client{} + } else { + clone := *httpClient + httpClient = &clone + } + if httpClient.Timeout == 0 { + httpClient.Timeout = jwksRequestTimeout + } + + ctx = oidc.ClientContext(ctx, httpClient) keySet := oidc.NewRemoteKeySet(ctx, jwksURL) verifier := oidc.NewVerifier(issuer, keySet, &oidc.Config{ ClientID: audience, diff --git a/internal/infra/auth/token_verifier_test.go b/internal/infra/auth/token_verifier_test.go index f316c46..342eb72 100644 --- a/internal/infra/auth/token_verifier_test.go +++ b/internal/infra/auth/token_verifier_test.go @@ -42,7 +42,7 @@ func TestJWTVerifier(t *testing.T) { Algorithm: string(jose.RS256), Use: "sig", }}} - jwksServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + jwksHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/jwks" { http.NotFound(w, r) return @@ -51,7 +51,8 @@ func TestJWTVerifier(t *testing.T) { if err := json.NewEncoder(w).Encode(jwks); err != nil { t.Errorf("encoding JWKS: %v", err) } - })) + }) + jwksServer := httptest.NewServer(jwksHandler) defer jwksServer.Close() signer, err := jose.NewSigner( @@ -185,4 +186,41 @@ func TestJWTVerifier(t *testing.T) { } }) } + + t.Run("uses a supplied HTTP client for JWKS TLS policy", func(t *testing.T) { + tlsServer := httptest.NewTLSServer(jwksHandler) + defer tlsServer.Close() + + rawToken := newToken( + testIssuer, + jwt.Audience{testAudience}, + time.Now().Add(time.Hour), + accessTokenClaims{}, + ) + + defaultVerifier := infraauth.NewJWTVerifier( + context.Background(), + testIssuer, + testAudience, + tlsServer.URL+"/jwks", + []string{"RS256"}, + nil, + ) + if err := defaultVerifier.Verify(context.Background(), rawToken); err == nil { + t.Fatal("expected the default JWKS client to reject the self-signed certificate") + } + + customVerifier := infraauth.NewJWTVerifierWithHTTPClient( + context.Background(), + testIssuer, + testAudience, + tlsServer.URL+"/jwks", + []string{"RS256"}, + nil, + tlsServer.Client(), + ) + if err := customVerifier.Verify(context.Background(), rawToken); err != nil { + t.Fatalf("expected custom JWKS HTTP client to be used: %v", err) + } + }) } diff --git a/internal/infra/mcp/mcp_handler.go b/internal/infra/mcp/mcp_handler.go index 8e5cfe1..e1d1ec5 100644 --- a/internal/infra/mcp/mcp_handler.go +++ b/internal/infra/mcp/mcp_handler.go @@ -15,6 +15,8 @@ import ( "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/sysdig" ) +const corsMaxAgeSeconds = 600 + type Handler struct { server *server.MCPServer } @@ -90,7 +92,9 @@ func (h *Handler) ServeStdio(ctx context.Context, stdin io.Reader, stdout io.Wri func (h *Handler) AsStreamableHTTP(mountPath string, stateless bool, security RemoteSecurity) http.Handler { mux := http.NewServeMux() - var opts []server.StreamableHTTPOption + opts := []server.StreamableHTTPOption{ + server.WithStreamableHTTPCORS(remoteCORSOptions(security)...), + } if stateless { opts = append(opts, server.WithStateLess(true)) } @@ -103,12 +107,33 @@ func (h *Handler) AsStreamableHTTP(mountPath string, stateless bool, security Re func (h *Handler) AsSSE(mountPath string, security RemoteSecurity) http.Handler { mux := http.NewServeMux() - sseServer := server.NewSSEServer(h.server, server.WithStaticBasePath(mountPath)) + sseServer := server.NewSSEServer( + h.server, + server.WithStaticBasePath(mountPath), + server.WithSSECORS(remoteCORSOptions(security)...), + ) security.mountMetadata(mux) - mux.Handle(mountPath, security.protect(sseServer)) + mux.Handle(sseServer.CompleteSsePath(), security.protect(sseServer.SSEHandler())) + mux.Handle(sseServer.CompleteMessagePath(), security.protect(sseServer.MessageHandler())) return mux } +func remoteCORSOptions(security RemoteSecurity) []server.CORSOption { + return []server.CORSOption{ + server.WithCORSAllowedOrigins(security.corsOrigins()...), + server.WithCORSAllowedMethods(http.MethodGet, http.MethodPost, http.MethodDelete, http.MethodOptions), + server.WithCORSAllowedHeaders( + "Authorization", + "Content-Type", + "Last-Event-ID", + server.HeaderKeyProtocolVersion, + server.HeaderKeySessionID, + ), + server.WithCORSExposedHeaders(server.HeaderKeySessionID, "WWW-Authenticate"), + server.WithCORSMaxAge(corsMaxAgeSeconds), + } +} + func (h *Handler) ServeInProcessClient() (*client.Client, error) { return client.NewInProcessClient(h.server) } diff --git a/internal/infra/mcp/mcp_handler_test.go b/internal/infra/mcp/mcp_handler_test.go index 0f90567..cdb356b 100644 --- a/internal/infra/mcp/mcp_handler_test.go +++ b/internal/infra/mcp/mcp_handler_test.go @@ -10,6 +10,7 @@ import ( "log/slog" "net/http" "net/http/httptest" + "strings" "time" "github.com/mark3labs/mcp-go/client" @@ -197,6 +198,16 @@ var _ = Describe("McpHandler", func() { Expect(verifier.calls).To(BeZero()) }) + It("rejects duplicate Origin headers", func() { + headers := authorizationHeaders() + headers.Add("Origin", allowedOrigin) + headers.Add("Origin", allowedOrigin) + resp := testClient.RPC(context.Background(), "tools/list", nil, headers) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusBadRequest)) + Expect(verifier.calls).To(BeZero()) + }) + It("returns CORS headers only for an exact allowlisted origin", func(ctx SpecContext) { headers := authorizationHeaders() headers.Set("Origin", allowedOrigin) @@ -208,6 +219,25 @@ var _ = Describe("McpHandler", func() { Expect(resp.Header.Get("Vary")).To(ContainSubstring("Origin")) }, NodeTimeout(5*time.Second)) + It("normalizes origin host case before exact comparison", func() { + mixedCaseSecurity := localmcp.NewRemoteSecurity( + verifier, + resourceURL, + "https://identity.example.com", + []string{"mcp:tools"}, + []string{"https://Client.Example.com"}, + ) + client := NewHTTPTestClient(handler.AsStreamableHTTP("/", false, mixedCaseSecurity)) + req := httptest.NewRequest(http.MethodOptions, "/", nil) + req.Header.Set("Origin", "https://client.example.com") + req.Header.Set("Access-Control-Request-Method", http.MethodPost) + recorder := httptest.NewRecorder() + client.handler.ServeHTTP(recorder, req) + + Expect(recorder.Code).To(Equal(http.StatusNoContent)) + Expect(recorder.Header().Get("Access-Control-Allow-Origin")).To(Equal("https://client.example.com")) + }) + It("answers an allowlisted CORS preflight without a token", func() { req := httptest.NewRequest(http.MethodOptions, "/", nil) req.Header.Set("Origin", allowedOrigin) @@ -218,6 +248,17 @@ var _ = Describe("McpHandler", func() { Expect(recorder.Code).To(Equal(http.StatusNoContent)) Expect(recorder.Header().Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) Expect(recorder.Header().Get("Access-Control-Allow-Headers")).To(ContainSubstring("Authorization")) + Expect(recorder.Header().Get("Access-Control-Max-Age")).To(Equal("600")) + Expect(verifier.calls).To(BeZero()) + }) + + It("does not treat a plain OPTIONS request as a CORS preflight", func() { + req := httptest.NewRequest(http.MethodOptions, "/", nil) + req.Header.Set("Origin", allowedOrigin) + recorder := httptest.NewRecorder() + testClient.handler.ServeHTTP(recorder, req) + + Expect(recorder.Code).To(Equal(http.StatusUnauthorized)) Expect(verifier.calls).To(BeZero()) }) @@ -256,8 +297,37 @@ var _ = Describe("McpHandler", func() { Expect(upstreamAuthorization).NotTo(ContainSubstring(validMCPToken)) }, NodeTimeout(5*time.Second)) - It("constructs a protected SSE handler", func() { - Expect(handler.AsSSE("/sse", remoteSecurity(verifier))).NotTo(BeNil()) + It("routes the protected SSE message endpoint and preserves exact-origin CORS", func() { + sseHandler := handler.AsSSE("/sysdig-mcp-server", remoteSecurity(verifier)) + req := httptest.NewRequest( + http.MethodPost, + "/sysdig-mcp-server/message?sessionId=missing", + strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"ping"}`), + ) + req.Header = authorizationHeaders() + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Origin", allowedOrigin) + recorder := httptest.NewRecorder() + sseHandler.ServeHTTP(recorder, req) + + Expect(recorder.Code).NotTo(Equal(http.StatusNotFound)) + Expect(recorder.Header().Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) + Expect(recorder.Header().Get("Access-Control-Allow-Origin")).NotTo(Equal("*")) + Expect(verifier.calls).To(Equal(1)) + }) + + It("serves SSE preflight on the actual SSE endpoint", func() { + sseHandler := handler.AsSSE("/sysdig-mcp-server", remoteSecurity(verifier)) + req := httptest.NewRequest(http.MethodOptions, "/sysdig-mcp-server/sse", nil) + req.Header.Set("Origin", allowedOrigin) + req.Header.Set("Access-Control-Request-Method", http.MethodGet) + recorder := httptest.NewRecorder() + sseHandler.ServeHTTP(recorder, req) + + Expect(recorder.Code).To(Equal(http.StatusNoContent)) + Expect(recorder.Header().Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) + Expect(recorder.Header().Get("Access-Control-Max-Age")).To(Equal("600")) + Expect(verifier.calls).To(BeZero()) }) It("serves stateless calls without initialization", func(ctx SpecContext) { diff --git a/internal/infra/mcp/remote_security.go b/internal/infra/mcp/remote_security.go index 3ab4ee1..7e9b1f7 100644 --- a/internal/infra/mcp/remote_security.go +++ b/internal/infra/mcp/remote_security.go @@ -12,10 +12,11 @@ import ( ) type RemoteSecurity struct { - verifier infraauth.TokenVerifier - allowedOrigins map[string]struct{} - metadata server.ProtectedResourceMetadataConfig - metadataURL string + verifier infraauth.TokenVerifier + allowedOrigins map[string]struct{} + corsAllowedOrigins []string + metadata server.ProtectedResourceMetadataConfig + metadataURL string } func NewRemoteSecurity( @@ -26,8 +27,14 @@ func NewRemoteSecurity( allowedOrigins []string, ) RemoteSecurity { origins := make(map[string]struct{}, len(allowedOrigins)) + corsOrigins := make([]string, 0, len(allowedOrigins)) for _, origin := range allowedOrigins { - origins[origin] = struct{}{} + normalized := normalizeOrigin(origin) + if _, exists := origins[normalized]; exists { + continue + } + origins[normalized] = struct{}{} + corsOrigins = append(corsOrigins, normalized) } metadata := server.ProtectedResourceMetadataConfig{ @@ -39,28 +46,28 @@ func NewRemoteSecurity( } return RemoteSecurity{ - verifier: verifier, - allowedOrigins: origins, - metadata: metadata, - metadataURL: protectedResourceMetadataURL(resourceURL), + verifier: verifier, + allowedOrigins: origins, + corsAllowedOrigins: corsOrigins, + metadata: metadata, + metadataURL: protectedResourceMetadataURL(resourceURL), } } func (s RemoteSecurity) protect(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Add("Vary", "Origin") - if origin := r.Header.Get("Origin"); origin != "" { + origin, hasOrigin, ok := requestOrigin(r.Header.Values("Origin")) + if !ok { + http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) + return + } + if hasOrigin { if _, allowed := s.allowedOrigins[origin]; !allowed { http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) return } - w.Header().Set("Access-Control-Allow-Origin", origin) - w.Header().Set("Access-Control-Expose-Headers", "Mcp-Session-Id, WWW-Authenticate") - - if r.Method == http.MethodOptions { - w.Header().Set("Access-Control-Allow-Methods", "GET, POST, DELETE, OPTIONS") - w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type, Last-Event-ID, Mcp-Protocol-Version, Mcp-Session-Id") - w.WriteHeader(http.StatusNoContent) + if isCORSPreflight(r) { + next.ServeHTTP(w, r) return } } @@ -84,6 +91,10 @@ func (s RemoteSecurity) protect(next http.Handler) http.Handler { }) } +func (s RemoteSecurity) corsOrigins() []string { + return append([]string(nil), s.corsAllowedOrigins...) +} + func (s RemoteSecurity) writeInsufficientScope(w http.ResponseWriter) { challenge := fmt.Sprintf( `Bearer resource_metadata=%q, error="insufficient_scope", scope=%q`, @@ -107,6 +118,30 @@ func (s RemoteSecurity) writeUnauthorized(w http.ResponseWriter, authError strin http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) } +func requestOrigin(values []string) (origin string, present bool, ok bool) { + if len(values) == 0 { + return "", false, true + } + if len(values) != 1 || strings.TrimSpace(values[0]) == "" { + return "", true, false + } + return normalizeOrigin(values[0]), true, true +} + +func normalizeOrigin(origin string) string { + u, err := url.Parse(origin) + if err != nil || !u.IsAbs() || u.Host == "" { + return origin + } + u.Scheme = strings.ToLower(u.Scheme) + u.Host = strings.ToLower(u.Host) + return u.String() +} + +func isCORSPreflight(r *http.Request) bool { + return r.Method == http.MethodOptions && r.Header.Get("Access-Control-Request-Method") != "" +} + func bearerToken(values []string) (string, bool) { if len(values) != 1 { return "", false diff --git a/internal/infra/sysdig/client.go b/internal/infra/sysdig/client.go index 018dc5f..602392f 100644 --- a/internal/infra/sysdig/client.go +++ b/internal/infra/sysdig/client.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/url" + "strings" ) func updateReqWithHostURL(req *http.Request, host string) error { @@ -15,8 +16,18 @@ func updateReqWithHostURL(req *http.Request, host string) error { if !u.IsAbs() || u.Host == "" { return fmt.Errorf("Sysdig API host must be an absolute URL") } + if u.User != nil || u.RawQuery != "" || u.Fragment != "" { + return fmt.Errorf("Sysdig API host must not contain user information, a query string, or a fragment") + } + req.URL.Scheme = u.Scheme req.URL.Host = u.Host + + basePath := strings.TrimSuffix(u.Path, "/") + if basePath != "" { + req.URL.Path = basePath + "/" + strings.TrimPrefix(req.URL.Path, "/") + req.URL.RawPath = "" + } return nil } diff --git a/internal/infra/sysdig/client_test.go b/internal/infra/sysdig/client_test.go index a0930b1..7e0e7d3 100644 --- a/internal/infra/sysdig/client_test.go +++ b/internal/infra/sysdig/client_test.go @@ -93,6 +93,30 @@ var _ = Describe("Client authentication", func() { Expect(lastHeaders.Get("Authorization")).To(Equal("Bearer server-token")) }) + It("preserves a configured reverse-proxy path prefix", func() { + var requestedPath string + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestedPath = r.URL.Path + if requestedPath != "/sysdig-proxy/api/users/me/permissions" { + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"permissions":[]}`)) + })) + defer proxy.Close() + + client, err := sysdig.NewSysdigClient( + sysdig.WithFixedHostAndToken(proxy.URL+"/sysdig-proxy", "server-token"), + ) + Expect(err).NotTo(HaveOccurred()) + + resp, err := client.GetMyPermissionsWithResponse(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.HTTPResponse.StatusCode).To(Equal(http.StatusOK)) + Expect(requestedPath).To(Equal("/sysdig-proxy/api/users/me/permissions")) + }) + It("rejects a non-absolute configured host", func() { client, err := sysdig.NewSysdigClient( sysdig.WithFixedHostAndToken("app.example.com", "server-token"), @@ -103,6 +127,16 @@ var _ = Describe("Client authentication", func() { Expect(err).To(MatchError(ContainSubstring("absolute URL"))) }) + It("rejects configured host query strings", func() { + client, err := sysdig.NewSysdigClient( + sysdig.WithFixedHostAndToken(ts.URL+"?tenant=one", "server-token"), + ) + Expect(err).NotTo(HaveOccurred()) + + _, err = client.GetMyPermissionsWithResponse(context.Background()) + Expect(err).To(MatchError(ContainSubstring("query string"))) + }) + Describe("WithVersion", func() { It("sends User-Agent header with version", func() { client, err := sysdig.NewSysdigClient( diff --git a/package.nix b/package.nix index f7b2efa..72fdc27 100644 --- a/package.nix +++ b/package.nix @@ -1,7 +1,7 @@ { buildGoLatestModule, versionCheckHook }: buildGoLatestModule (finalAttrs: { pname = "sysdig-mcp-server"; - version = "3.0.4"; + version = "4.0.0"; src = ./.; # This hash is automatically re-calculated with `just rehash-package-nix`. This is automatically called as well by `just update`. vendorHash = "sha256-XVCFDHEGY3du2YRJjbEgJwKoNiayjPTPsT9WBvcH2sk="; From f164a5b4c80ad86ebf2db1c6058bed1a431482b2 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:27:00 +0200 Subject: [PATCH 03/19] fix(auth): bind sessions and bound JWKS refreshes --- internal/config/config.go | 33 +++- internal/config/config_test.go | 13 ++ internal/infra/auth/keyset.go | 214 +++++++++++++++++++++ internal/infra/auth/keyset_test.go | 119 ++++++++++++ internal/infra/auth/token_verifier.go | 101 +++++++--- internal/infra/auth/token_verifier_test.go | 30 ++- internal/infra/mcp/mcp_handler.go | 23 ++- internal/infra/mcp/mcp_handler_test.go | 48 ++++- internal/infra/mcp/remote_security.go | 48 ++++- internal/infra/mcp/session_security.go | 90 +++++++++ 10 files changed, 662 insertions(+), 57 deletions(-) create mode 100644 internal/infra/auth/keyset.go create mode 100644 internal/infra/auth/keyset_test.go create mode 100644 internal/infra/mcp/session_security.go diff --git a/internal/config/config.go b/internal/config/config.go index b05c6fe..ab777b0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -66,6 +66,9 @@ func (c *Config) Validate() error { if c.Transport == "stdio" { return nil } + if err := validateSecureURL("SYSDIG_MCP_API_HOST", apiHost); err != nil { + return err + } if err := validateMountPath(c.MountPath); err != nil { return err @@ -80,7 +83,7 @@ func (c *Config) Validate() error { return fmt.Errorf("required configuration missing: SYSDIG_MCP_AUTH_JWKS_URL") } - resourceURL, err := parseAbsoluteURL("SYSDIG_MCP_RESOURCE_URL", c.ResourceURL) + resourceURL, err := parseSecureAbsoluteURL("SYSDIG_MCP_RESOURCE_URL", c.ResourceURL) if err != nil { return err } @@ -99,14 +102,14 @@ func (c *Config) Validate() error { ) } - authIssuer, err := parseAbsoluteURL("SYSDIG_MCP_AUTH_ISSUER", c.AuthIssuer) + authIssuer, err := parseSecureAbsoluteURL("SYSDIG_MCP_AUTH_ISSUER", c.AuthIssuer) if err != nil { return err } if authIssuer.RawQuery != "" { return fmt.Errorf("SYSDIG_MCP_AUTH_ISSUER must not contain a query string") } - if _, err := parseAbsoluteURL("SYSDIG_MCP_AUTH_JWKS_URL", c.AuthJWKSURL); err != nil { + if _, err := parseSecureAbsoluteURL("SYSDIG_MCP_AUTH_JWKS_URL", c.AuthJWKSURL); err != nil { return err } for _, origin := range c.AllowedOrigins { @@ -176,12 +179,30 @@ func parseAbsoluteURL(name, rawURL string) (*url.URL, error) { if u.User != nil || u.Fragment != "" { return nil, fmt.Errorf("%s must not contain user information or a fragment", name) } - if u.Scheme != "https" && !(u.Scheme == "http" && isLoopbackHostname(u.Hostname())) { - return nil, fmt.Errorf("%s must use https (http is allowed only for loopback development)", name) + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("%s must use http or https", name) + } + return u, nil +} + +func parseSecureAbsoluteURL(name, rawURL string) (*url.URL, error) { + u, err := parseAbsoluteURL(name, rawURL) + if err != nil { + return nil, err + } + if err := validateSecureURL(name, u); err != nil { + return nil, err } return u, nil } +func validateSecureURL(name string, u *url.URL) error { + if u.Scheme != "https" && !(u.Scheme == "http" && isLoopbackHostname(u.Hostname())) { + return fmt.Errorf("%s must use https (http is allowed only for loopback development)", name) + } + return nil +} + func validateMountPath(mountPath string) error { if mountPath == "" || !strings.HasPrefix(mountPath, "/") || path.Clean(mountPath) != mountPath { return fmt.Errorf("SYSDIG_MCP_MOUNT_PATH must be an absolute, canonical URL path") @@ -193,7 +214,7 @@ func validateOrigin(origin string) error { if origin == "*" { return fmt.Errorf("wildcard origins are not allowed") } - u, err := parseAbsoluteURL("origin", origin) + u, err := parseSecureAbsoluteURL("origin", origin) if err != nil { return err } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e8ab887..51474e7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -87,6 +87,19 @@ var _ = Describe("Config", func() { Entry("symmetric signing", func(cfg *config.Config) { cfg.AuthSigningAlgs = []string{"HS256"} }, "asymmetric signing algorithm"), ) + + It("preserves plaintext non-loopback API hosts for stdio", func() { + cfg := validConfig("stdio") + cfg.APIHost = "http://10.0.0.5" + Expect(cfg.Validate()).To(Succeed()) + }) + + It("requires HTTPS for the same API host on remote transports", func() { + cfg := validConfig("streamable-http") + cfg.APIHost = "http://10.0.0.5" + Expect(cfg.Validate()).To(MatchError(ContainSubstring("must use https"))) + }) + It("allows an API path prefix when it is otherwise safe", func() { cfg := validConfig("streamable-http") cfg.APIHost = "https://gateway.example.com/sysdig-proxy" diff --git a/internal/infra/auth/keyset.go b/internal/infra/auth/keyset.go new file mode 100644 index 0000000..cfa1ec4 --- /dev/null +++ b/internal/infra/auth/keyset.go @@ -0,0 +1,214 @@ +package auth + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "sync" + "time" + + jose "github.com/go-jose/go-jose/v4" +) + +const ( + defaultJWKSMinRefreshInterval = 30 * time.Second + defaultJWKSMaxAge = 5 * time.Minute + maxJWKSResponseBytes = 1 << 20 +) + +// boundedRemoteKeySet keeps JWKS refreshes bounded in both directions: +// unknown kids cannot force an HTTP request per token, while cached keys have +// a hard maximum age so removed keys do not remain trusted indefinitely. +type boundedRemoteKeySet struct { + jwksURL string + client *http.Client + algorithms []jose.SignatureAlgorithm + allowedAlgs map[string]struct{} + minRefresh time.Duration + maxAge time.Duration + refreshMu sync.Mutex + mu sync.RWMutex + keys []jose.JSONWebKey + fetchedAt time.Time + lastAttempt time.Time + lastFetchErr error +} + +func newBoundedRemoteKeySet( + jwksURL string, + client *http.Client, + signingAlgorithms []string, + minRefresh time.Duration, + maxAge time.Duration, +) *boundedRemoteKeySet { + algs := make([]jose.SignatureAlgorithm, 0, len(signingAlgorithms)) + allowed := make(map[string]struct{}, len(signingAlgorithms)) + for _, algorithm := range signingAlgorithms { + algs = append(algs, jose.SignatureAlgorithm(algorithm)) + allowed[algorithm] = struct{}{} + } + return &boundedRemoteKeySet{ + jwksURL: jwksURL, + client: client, + algorithms: algs, + allowedAlgs: allowed, + minRefresh: minRefresh, + maxAge: maxAge, + } +} + +func (r *boundedRemoteKeySet) VerifySignature(ctx context.Context, rawJWT string) ([]byte, error) { + jws, err := jose.ParseSigned(rawJWT, r.algorithms) + if err != nil { + return nil, fmt.Errorf("parsing jwt: %w", err) + } + + keys, fetchedAt, _, _ := r.snapshot() + if len(keys) == 0 || r.maxAge <= 0 || time.Since(fetchedAt) >= r.maxAge { + keys, err = r.refresh(ctx, true) + if err != nil { + return nil, fmt.Errorf("refreshing JWKS: %w", err) + } + } + + if payload, ok := verifyWithKeys(jws, keys); ok { + return payload, nil + } + + // A new kid may indicate a normal key rotation. Refresh at most once per + // minimum interval so arbitrary kids cannot amplify traffic to the IdP. + keys, err = r.refresh(ctx, false) + if err != nil { + return nil, fmt.Errorf("refreshing JWKS: %w", err) + } + if payload, ok := verifyWithKeys(jws, keys); ok { + return payload, nil + } + return nil, errors.New("failed to verify token signature") +} + +func verifyWithKeys(jws *jose.JSONWebSignature, keys []jose.JSONWebKey) ([]byte, bool) { + keyID := "" + if len(jws.Signatures) > 0 { + keyID = jws.Signatures[0].Header.KeyID + } + for _, key := range keys { + if keyID != "" && key.KeyID != keyID { + continue + } + if payload, err := jws.Verify(&key); err == nil { + return payload, true + } + } + return nil, false +} + +func (r *boundedRemoteKeySet) snapshot() ([]jose.JSONWebKey, time.Time, time.Time, error) { + r.mu.RLock() + defer r.mu.RUnlock() + return append([]jose.JSONWebKey(nil), r.keys...), r.fetchedAt, r.lastAttempt, r.lastFetchErr +} + +func (r *boundedRemoteKeySet) refresh(ctx context.Context, requireFresh bool) ([]jose.JSONWebKey, error) { + r.refreshMu.Lock() + defer r.refreshMu.Unlock() + + keys, fetchedAt, lastAttempt, lastErr := r.snapshot() + now := time.Now() + fresh := len(keys) > 0 && r.maxAge > 0 && now.Sub(fetchedAt) < r.maxAge + if requireFresh && fresh { + return keys, nil + } + + if !lastAttempt.IsZero() && now.Sub(lastAttempt) < r.minRefresh { + if requireFresh && !fresh { + if lastErr != nil { + return keys, lastErr + } + return keys, errors.New("JWKS refresh is temporarily throttled") + } + return keys, nil + } + + r.mu.Lock() + r.lastAttempt = now + r.mu.Unlock() + + newKeys, err := r.fetchKeys(ctx) + + r.mu.Lock() + defer r.mu.Unlock() + if err != nil { + r.lastFetchErr = err + return append([]jose.JSONWebKey(nil), r.keys...), err + } + + r.keys = newKeys + r.fetchedAt = now + r.lastFetchErr = nil + return append([]jose.JSONWebKey(nil), r.keys...), nil +} + +func (r *boundedRemoteKeySet) fetchKeys(ctx context.Context) ([]jose.JSONWebKey, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, r.jwksURL, nil) + if err != nil { + return nil, fmt.Errorf("creating JWKS request: %w", err) + } + req.Header.Set("Cache-Control", "no-cache") + + resp, err := r.client.Do(req) + if err != nil { + return nil, fmt.Errorf("fetching JWKS: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxJWKSResponseBytes+1)) + if err != nil { + return nil, fmt.Errorf("reading JWKS response: %w", err) + } + if len(body) > maxJWKSResponseBytes { + return nil, fmt.Errorf("JWKS response exceeds %d bytes", maxJWKSResponseBytes) + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("JWKS endpoint returned %s", resp.Status) + } + + var raw struct { + Keys []json.RawMessage `json:"keys"` + } + if err := json.Unmarshal(body, &raw); err != nil { + return nil, fmt.Errorf("decoding JWKS: %w", err) + } + + keys := make([]jose.JSONWebKey, 0, len(raw.Keys)) + for _, rawKey := range raw.Keys { + var metadata struct { + Algorithm string `json:"alg"` + Use string `json:"use"` + } + if err := json.Unmarshal(rawKey, &metadata); err != nil { + return nil, fmt.Errorf("decoding JWK metadata: %w", err) + } + if metadata.Use != "" && metadata.Use != "sig" { + continue + } + if metadata.Algorithm != "" { + if _, ok := r.allowedAlgs[metadata.Algorithm]; !ok { + continue + } + } + + var key jose.JSONWebKey + if err := json.Unmarshal(rawKey, &key); err != nil { + if errors.Is(err, jose.ErrUnsupportedKeyType) { + continue + } + return nil, fmt.Errorf("decoding JWK: %w", err) + } + keys = append(keys, key) + } + return keys, nil +} diff --git a/internal/infra/auth/keyset_test.go b/internal/infra/auth/keyset_test.go new file mode 100644 index 0000000..dca8078 --- /dev/null +++ b/internal/infra/auth/keyset_test.go @@ -0,0 +1,119 @@ +package auth + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "sync/atomic" + "testing" + "time" + + jose "github.com/go-jose/go-jose/v4" +) + +func TestBoundedRemoteKeySetThrottlesUnknownKids(t *testing.T) { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + var requests atomic.Int32 + jwks := jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{ + Key: &privateKey.PublicKey, KeyID: "known", Algorithm: string(jose.RS256), Use: "sig", + }}} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + _ = json.NewEncoder(w).Encode(jwks) + })) + defer server.Close() + + sign := func(kid string) string { + t.Helper() + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: jose.RS256, Key: privateKey}, + (&jose.SignerOptions{}).WithHeader("kid", kid), + ) + if err != nil { + t.Fatal(err) + } + raw, err := signer.Sign([]byte(`{"sub":"test"}`)) + if err != nil { + t.Fatal(err) + } + serialized, err := raw.CompactSerialize() + if err != nil { + t.Fatal(err) + } + return serialized + } + + keySet := newBoundedRemoteKeySet(server.URL, server.Client(), []string{"RS256"}, time.Hour, time.Hour) + if _, err := keySet.VerifySignature(context.Background(), sign("known")); err != nil { + t.Fatalf("initial verification failed: %v", err) + } + for i := 0; i < 5; i++ { + if _, err := keySet.VerifySignature(context.Background(), sign("unknown")); err == nil { + t.Fatal("expected unknown kid to fail") + } + } + if got := requests.Load(); got != 1 { + t.Fatalf("unknown kids triggered %d JWKS fetches, want 1", got) + } +} + +func TestBoundedRemoteKeySetExpiresRemovedKeys(t *testing.T) { + key1, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + key2, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + + var mu sync.RWMutex + current := jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{ + Key: &key1.PublicKey, KeyID: "key-1", Algorithm: string(jose.RS256), Use: "sig", + }}} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + mu.RLock() + defer mu.RUnlock() + _ = json.NewEncoder(w).Encode(current) + })) + defer server.Close() + + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: jose.RS256, Key: key1}, + (&jose.SignerOptions{}).WithHeader("kid", "key-1"), + ) + if err != nil { + t.Fatal(err) + } + signed, err := signer.Sign([]byte(`{"sub":"test"}`)) + if err != nil { + t.Fatal(err) + } + raw, err := signed.CompactSerialize() + if err != nil { + t.Fatal(err) + } + + keySet := newBoundedRemoteKeySet(server.URL, server.Client(), []string{"RS256"}, 0, time.Nanosecond) + if _, err := keySet.VerifySignature(context.Background(), raw); err != nil { + t.Fatalf("initial verification failed: %v", err) + } + + mu.Lock() + current = jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{ + Key: &key2.PublicKey, KeyID: "key-2", Algorithm: string(jose.RS256), Use: "sig", + }}} + mu.Unlock() + time.Sleep(time.Millisecond) + + if _, err := keySet.VerifySignature(context.Background(), raw); err == nil { + t.Fatal("expected token signed by removed key to fail after cache expiry") + } +} diff --git a/internal/infra/auth/token_verifier.go b/internal/infra/auth/token_verifier.go index c5a1280..24569ad 100644 --- a/internal/infra/auth/token_verifier.go +++ b/internal/infra/auth/token_verifier.go @@ -2,6 +2,8 @@ package auth import ( "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -19,16 +21,31 @@ const jwksRequestTimeout = 10 * time.Second // lacks the authorization required by this resource. var ErrInsufficientScope = errors.New("access token has insufficient scope") -// TokenVerifier validates an access token presented to the MCP server. -// Implementations must never forward the token to an upstream service. +// Principal is the authenticated identity used to bind stateful MCP sessions. +// Subject is normally the JWT sub claim. Subject-less access tokens fall back +// to a digest of the verified token, which is safe but intentionally makes the +// session valid only for that token's lifetime. +type Principal struct { + Issuer string + Subject string +} + +func (p Principal) valid() bool { + return p.Issuer != "" && p.Subject != "" +} + +// TokenVerifier validates an access token presented to the MCP server and +// returns the identity that owns any stateful transport session created by the +// request. Implementations must never forward the token to an upstream service. type TokenVerifier interface { - Verify(context.Context, string) error + Verify(context.Context, string) (Principal, error) } -// JWTVerifier validates signed JWT access tokens against a remote JWKS. +// JWTVerifier validates signed JWT access tokens against a bounded remote JWKS. // Issuer, audience, expiry, and signing algorithm checks are delegated to the // OIDC verifier. Optional scopes are checked after cryptographic validation. type JWTVerifier struct { + issuer string verifier *oidc.IDTokenVerifier requiredScopes []string } @@ -53,7 +70,7 @@ func NewJWTVerifier( } func NewJWTVerifierWithHTTPClient( - ctx context.Context, + _ context.Context, issuer string, audience string, jwksURL string, @@ -71,55 +88,79 @@ func NewJWTVerifierWithHTTPClient( httpClient.Timeout = jwksRequestTimeout } - ctx = oidc.ClientContext(ctx, httpClient) - keySet := oidc.NewRemoteKeySet(ctx, jwksURL) + keySet := newBoundedRemoteKeySet( + jwksURL, + httpClient, + signingAlgorithms, + defaultJWKSMinRefreshInterval, + defaultJWKSMaxAge, + ) verifier := oidc.NewVerifier(issuer, keySet, &oidc.Config{ ClientID: audience, SupportedSigningAlgs: signingAlgorithms, }) return &JWTVerifier{ + issuer: issuer, verifier: verifier, requiredScopes: slices.Clone(requiredScopes), } } -func (v *JWTVerifier) Verify(ctx context.Context, rawToken string) error { +func (v *JWTVerifier) Verify(ctx context.Context, rawToken string) (Principal, error) { token, err := v.verifier.Verify(ctx, rawToken) if err != nil { - return fmt.Errorf("validating access token: %w", err) - } - - if len(v.requiredScopes) == 0 { - return nil + return Principal{}, fmt.Errorf("validating access token: %w", err) } var claims struct { - Scope string `json:"scope"` - SCP json.RawMessage `json:"scp"` + Subject string `json:"sub"` + Scope json.RawMessage `json:"scope"` + SCP json.RawMessage `json:"scp"` } if err := token.Claims(&claims); err != nil { - return fmt.Errorf("decoding access token claims: %w", err) + return Principal{}, fmt.Errorf("decoding access token claims: %w", err) } - grantedScopes := strings.Fields(claims.Scope) - if len(claims.SCP) > 0 { - var scopeString string - if err := json.Unmarshal(claims.SCP, &scopeString); err == nil { - grantedScopes = append(grantedScopes, strings.Fields(scopeString)...) - } else { - var scopeList []string - if err := json.Unmarshal(claims.SCP, &scopeList); err != nil { - return fmt.Errorf("decoding scp claim: %w", err) - } - grantedScopes = append(grantedScopes, scopeList...) - } + subject := claims.Subject + if subject == "" { + sum := sha256.Sum256([]byte(rawToken)) + subject = "token-sha256:" + hex.EncodeToString(sum[:]) + } + principal := Principal{Issuer: v.issuer, Subject: subject} + + grantedScopes, err := parseScopeClaim(claims.Scope) + if err != nil { + return Principal{}, fmt.Errorf("decoding scope claim: %w", err) + } + scpScopes, err := parseScopeClaim(claims.SCP) + if err != nil { + return Principal{}, fmt.Errorf("decoding scp claim: %w", err) } + grantedScopes = append(grantedScopes, scpScopes...) + for _, requiredScope := range v.requiredScopes { if !slices.Contains(grantedScopes, requiredScope) { - return fmt.Errorf("%w: missing %q", ErrInsufficientScope, requiredScope) + return Principal{}, fmt.Errorf("%w: missing %q", ErrInsufficientScope, requiredScope) } } - return nil + return principal, nil +} + +func parseScopeClaim(raw json.RawMessage) ([]string, error) { + if len(raw) == 0 || string(raw) == "null" { + return nil, nil + } + + var scopeString string + if err := json.Unmarshal(raw, &scopeString); err == nil { + return strings.Fields(scopeString), nil + } + + var scopeList []string + if err := json.Unmarshal(raw, &scopeList); err != nil { + return nil, err + } + return scopeList, nil } diff --git a/internal/infra/auth/token_verifier_test.go b/internal/infra/auth/token_verifier_test.go index 342eb72..4f3466a 100644 --- a/internal/infra/auth/token_verifier_test.go +++ b/internal/infra/auth/token_verifier_test.go @@ -20,11 +20,12 @@ import ( const ( testIssuer = "https://identity.example.com" testAudience = "https://mcp.example.com/sysdig-mcp-server" + testSubject = "test-user" ) type accessTokenClaims struct { - Scope string `json:"scope,omitempty"` - SCP any `json:"scp,omitempty"` + Scope any `json:"scope,omitempty"` + SCP any `json:"scp,omitempty"` } func TestJWTVerifier(t *testing.T) { @@ -68,6 +69,7 @@ func TestJWTVerifier(t *testing.T) { rawToken, err := jwt.Signed(signer). Claims(jwt.Claims{ Issuer: issuer, + Subject: testSubject, Audience: audience, Expiry: jwt.NewNumericDate(expiry), }). @@ -99,6 +101,15 @@ func TestJWTVerifier(t *testing.T) { signingAlgorithms: []string{"RS256"}, requiredScopes: []string{"mcp:tools"}, }, + { + name: "valid array scope claim", + issuer: testIssuer, + audience: jwt.Audience{testAudience}, + expiry: time.Now().Add(time.Hour), + claims: accessTokenClaims{Scope: []string{"openid", "mcp:tools"}}, + signingAlgorithms: []string{"RS256"}, + requiredScopes: []string{"mcp:tools"}, + }, { name: "valid scp claim", issuer: testIssuer, @@ -174,15 +185,20 @@ func TestJWTVerifier(t *testing.T) { test.requiredScopes, ) - err := verifier.Verify(context.Background(), rawToken) + principal, err := verifier.Verify(context.Background(), rawToken) if test.wantError && err == nil { t.Fatal("expected token verification to fail") } if test.wantScopeError && !errors.Is(err, infraauth.ErrInsufficientScope) { t.Fatalf("expected insufficient scope error, got: %v", err) } - if !test.wantError && err != nil { - t.Fatalf("expected token verification to succeed: %v", err) + if !test.wantError { + if err != nil { + t.Fatalf("expected token verification to succeed: %v", err) + } + if principal.Issuer != testIssuer || principal.Subject != testSubject { + t.Fatalf("unexpected principal: %#v", principal) + } } }) } @@ -206,7 +222,7 @@ func TestJWTVerifier(t *testing.T) { []string{"RS256"}, nil, ) - if err := defaultVerifier.Verify(context.Background(), rawToken); err == nil { + if _, err := defaultVerifier.Verify(context.Background(), rawToken); err == nil { t.Fatal("expected the default JWKS client to reject the self-signed certificate") } @@ -219,7 +235,7 @@ func TestJWTVerifier(t *testing.T) { nil, tlsServer.Client(), ) - if err := customVerifier.Verify(context.Background(), rawToken); err != nil { + if _, err := customVerifier.Verify(context.Background(), rawToken); err != nil { t.Fatalf("expected custom JWKS HTTP client to be used: %v", err) } }) diff --git a/internal/infra/mcp/mcp_handler.go b/internal/infra/mcp/mcp_handler.go index e1d1ec5..4ae9140 100644 --- a/internal/infra/mcp/mcp_handler.go +++ b/internal/infra/mcp/mcp_handler.go @@ -2,6 +2,7 @@ package mcp import ( "context" + "fmt" "io" "log/slog" "net/http" @@ -94,8 +95,13 @@ func (h *Handler) AsStreamableHTTP(mountPath string, stateless bool, security Re opts := []server.StreamableHTTPOption{ server.WithStreamableHTTPCORS(remoteCORSOptions(security)...), + // Legacy protocol sessions are bound to the identity established by + // RemoteSecurity. Modern 2026-07-28 requests are sessionless and do not + // use this manager. + server.WithSessionIdManagerResolver(principalSessionIDManagerResolver{}), } if stateless { + // Keep the explicit stateless mode fully sessionless. opts = append(opts, server.WithStateLess(true)) } @@ -111,6 +117,13 @@ func (h *Handler) AsSSE(mountPath string, security RemoteSecurity) http.Handler h.server, server.WithStaticBasePath(mountPath), server.WithSSECORS(remoteCORSOptions(security)...), + server.WithSessionIDGenerator(func(ctx context.Context, _ *http.Request) (string, error) { + principal, ok := principalFromContext(ctx) + if !ok { + return "", fmt.Errorf("authenticated principal is unavailable") + } + return generatePrincipalSessionID(principal) + }), ) security.mountMetadata(mux) mux.Handle(sseServer.CompleteSsePath(), security.protect(sseServer.SSEHandler())) @@ -125,11 +138,13 @@ func remoteCORSOptions(security RemoteSecurity) []server.CORSOption { server.WithCORSAllowedHeaders( "Authorization", "Content-Type", - "Last-Event-ID", - server.HeaderKeyProtocolVersion, - server.HeaderKeySessionID, + mcp.HeaderLastEventID, + mcp.HeaderProtocolVersion, + mcp.HeaderSessionID, + mcp.HeaderMethod, + mcp.HeaderName, ), - server.WithCORSExposedHeaders(server.HeaderKeySessionID, "WWW-Authenticate"), + server.WithCORSExposedHeaders(mcp.HeaderSessionID, "WWW-Authenticate"), server.WithCORSMaxAge(corsMaxAgeSeconds), } } diff --git a/internal/infra/mcp/mcp_handler_test.go b/internal/infra/mcp/mcp_handler_test.go index cdb356b..31d3a71 100644 --- a/internal/infra/mcp/mcp_handler_test.go +++ b/internal/infra/mcp/mcp_handler_test.go @@ -29,6 +29,7 @@ import ( const ( validMCPToken = "mcp-access-token" + otherMCPToken = "other-mcp-access-token" allowedOrigin = "https://client.example.com" resourceURL = "https://mcp.example.com/sysdig-mcp-server" ) @@ -56,15 +57,19 @@ type fakeTokenVerifier struct { calls int } -func (v *fakeTokenVerifier) Verify(_ context.Context, rawToken string) error { +func (v *fakeTokenVerifier) Verify(_ context.Context, rawToken string) (infraauth.Principal, error) { v.calls++ if rawToken == "insufficient-scope" { - return infraauth.ErrInsufficientScope + return infraauth.Principal{}, infraauth.ErrInsufficientScope } - if rawToken != v.validToken { - return errors.New("invalid access token") + switch rawToken { + case v.validToken: + return infraauth.Principal{Issuer: "https://identity.example.com", Subject: "alice"}, nil + case otherMCPToken: + return infraauth.Principal{Issuer: "https://identity.example.com", Subject: "bob"}, nil + default: + return infraauth.Principal{}, errors.New("invalid access token") } - return nil } func remoteSecurity(verifier *fakeTokenVerifier) localmcp.RemoteSecurity { @@ -78,7 +83,11 @@ func remoteSecurity(verifier *fakeTokenVerifier) localmcp.RemoteSecurity { } func authorizationHeaders() http.Header { - return http.Header{"Authorization": []string{"Bearer " + validMCPToken}} + return authorizationHeadersFor(validMCPToken) +} + +func authorizationHeadersFor(token string) http.Header { + return http.Header{"Authorization": []string{"Bearer " + token}} } var _ = Describe("McpHandler", func() { @@ -159,6 +168,15 @@ var _ = Describe("McpHandler", func() { Expect(verifier.calls).To(Equal(2)) }, NodeTimeout(5*time.Second)) + It("binds a stateful session to the authenticated principal", func(ctx SpecContext) { + testClient.Initialize(ctx, authorizationHeaders()) + Expect(testClient.sessionID).NotTo(BeEmpty()) + + resp := testClient.RPC(ctx, "ping", nil, authorizationHeadersFor(otherMCPToken)) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusNotFound)) + }, NodeTimeout(5*time.Second)) + DescribeTable("rejects invalid authorization headers", func(headers http.Header) { resp := testClient.RPC(context.Background(), "tools/list", nil, headers) @@ -180,6 +198,17 @@ var _ = Describe("McpHandler", func() { Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring(`error="invalid_token"`)) }) + It("exposes OAuth challenges on allowlisted browser auth failures", func() { + headers := http.Header{"Authorization": []string{"Bearer wrong-token"}} + headers.Set("Origin", allowedOrigin) + resp := testClient.RPC(context.Background(), "tools/list", nil, headers) + defer func() { _ = resp.Body.Close() }() + Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized)) + Expect(resp.Header.Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) + Expect(resp.Header.Get("Access-Control-Expose-Headers")).To(ContainSubstring("WWW-Authenticate")) + Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring(`error="invalid_token"`)) + }) + It("returns insufficient_scope with 403 for a valid but under-scoped token", func() { headers := http.Header{"Authorization": []string{"Bearer insufficient-scope"}} resp := testClient.RPC(context.Background(), "tools/list", nil, headers) @@ -231,6 +260,7 @@ var _ = Describe("McpHandler", func() { req := httptest.NewRequest(http.MethodOptions, "/", nil) req.Header.Set("Origin", "https://client.example.com") req.Header.Set("Access-Control-Request-Method", http.MethodPost) + req.Header.Set("Access-Control-Request-Headers", mcp.HeaderMethod+", "+mcp.HeaderName) recorder := httptest.NewRecorder() client.handler.ServeHTTP(recorder, req) @@ -248,6 +278,8 @@ var _ = Describe("McpHandler", func() { Expect(recorder.Code).To(Equal(http.StatusNoContent)) Expect(recorder.Header().Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) Expect(recorder.Header().Get("Access-Control-Allow-Headers")).To(ContainSubstring("Authorization")) + Expect(recorder.Header().Get("Access-Control-Allow-Headers")).To(ContainSubstring(mcp.HeaderMethod)) + Expect(recorder.Header().Get("Access-Control-Allow-Headers")).To(ContainSubstring(mcp.HeaderName)) Expect(recorder.Header().Get("Access-Control-Max-Age")).To(Equal("600")) Expect(verifier.calls).To(BeZero()) }) @@ -297,7 +329,7 @@ var _ = Describe("McpHandler", func() { Expect(upstreamAuthorization).NotTo(ContainSubstring(validMCPToken)) }, NodeTimeout(5*time.Second)) - It("routes the protected SSE message endpoint and preserves exact-origin CORS", func() { + It("rejects an unbound SSE session ID while preserving exact-origin CORS", func() { sseHandler := handler.AsSSE("/sysdig-mcp-server", remoteSecurity(verifier)) req := httptest.NewRequest( http.MethodPost, @@ -310,7 +342,7 @@ var _ = Describe("McpHandler", func() { recorder := httptest.NewRecorder() sseHandler.ServeHTTP(recorder, req) - Expect(recorder.Code).NotTo(Equal(http.StatusNotFound)) + Expect(recorder.Code).To(Equal(http.StatusNotFound)) Expect(recorder.Header().Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) Expect(recorder.Header().Get("Access-Control-Allow-Origin")).NotTo(Equal("*")) Expect(verifier.calls).To(Equal(1)) diff --git a/internal/infra/mcp/remote_security.go b/internal/infra/mcp/remote_security.go index 7e9b1f7..39cfb4f 100644 --- a/internal/infra/mcp/remote_security.go +++ b/internal/infra/mcp/remote_security.go @@ -1,6 +1,7 @@ package mcp import ( + "context" "errors" "fmt" "net/http" @@ -11,6 +12,8 @@ import ( infraauth "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/auth" ) +type remotePrincipalContextKey struct{} + type RemoteSecurity struct { verifier infraauth.TokenVerifier allowedOrigins map[string]struct{} @@ -70,6 +73,10 @@ func (s RemoteSecurity) protect(next http.Handler) http.Handler { next.ServeHTTP(w, r) return } + // Authentication happens outside mcp-go's CORS middleware. Apply the + // simple-response headers here as well so browser clients can read + // WWW-Authenticate on 401/403 responses. + s.applySimpleCORS(w, origin) } rawToken, ok := bearerToken(r.Header.Values("Authorization")) @@ -78,7 +85,8 @@ func (s RemoteSecurity) protect(next http.Handler) http.Handler { return } - if err := s.verifier.Verify(r.Context(), rawToken); err != nil { + principal, err := s.verifier.Verify(r.Context(), rawToken) + if err != nil { if errors.Is(err, infraauth.ErrInsufficientScope) { s.writeInsufficientScope(w) return @@ -87,10 +95,46 @@ func (s RemoteSecurity) protect(next http.Handler) http.Handler { return } - next.ServeHTTP(w, r) + // Legacy SSE carries the session ID in the message URL. Validate its + // owner before mcp-go looks up the session so a leaked URL cannot be + // replayed by another authenticated principal. + if sessionID := r.URL.Query().Get("sessionId"); sessionID != "" { + if err := validatePrincipalSessionID(sessionID, principal); err != nil { + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) + return + } + } + + ctx := context.WithValue(r.Context(), remotePrincipalContextKey{}, principal) + next.ServeHTTP(w, r.WithContext(ctx)) }) } +func principalFromContext(ctx context.Context) (infraauth.Principal, bool) { + principal, ok := ctx.Value(remotePrincipalContextKey{}).(infraauth.Principal) + return principal, ok && principal.Issuer != "" && principal.Subject != "" +} + +func (s RemoteSecurity) applySimpleCORS(w http.ResponseWriter, origin string) { + w.Header().Set("Access-Control-Allow-Origin", origin) + appendVary(w.Header(), "Origin") + w.Header().Set( + "Access-Control-Expose-Headers", + strings.Join([]string{server.HeaderKeySessionID, "WWW-Authenticate"}, ", "), + ) +} + +func appendVary(header http.Header, value string) { + for _, existing := range header.Values("Vary") { + for _, item := range strings.Split(existing, ",") { + if strings.EqualFold(strings.TrimSpace(item), value) { + return + } + } + } + header.Add("Vary", value) +} + func (s RemoteSecurity) corsOrigins() []string { return append([]string(nil), s.corsAllowedOrigins...) } diff --git a/internal/infra/mcp/session_security.go b/internal/infra/mcp/session_security.go new file mode 100644 index 0000000..d3b0cd4 --- /dev/null +++ b/internal/infra/mcp/session_security.go @@ -0,0 +1,90 @@ +package mcp + +import ( + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "encoding/base64" + "errors" + "fmt" + "net/http" + "strings" + + "github.com/mark3labs/mcp-go/server" + infraauth "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/auth" +) + +const principalSessionPrefix = "mcp-session-" + +type principalSessionIDManager struct { + principal infraauth.Principal +} + +type principalSessionIDManagerResolver struct{} + +func (principalSessionIDManagerResolver) ResolveSessionIdManager(r *http.Request) server.SessionIdManager { + if r == nil { + return &principalSessionIDManager{} + } + principal, _ := principalFromContext(r.Context()) + return &principalSessionIDManager{principal: principal} +} + +func (m *principalSessionIDManager) Generate() string { + sessionID, err := generatePrincipalSessionID(m.principal) + if err != nil { + // SessionIdManager.Generate cannot return an error. A CSPRNG failure is + // not recoverable without weakening the session boundary, so fail closed. + panic(fmt.Sprintf("generating MCP session ID: %v", err)) + } + return sessionID +} + +func (m *principalSessionIDManager) Validate(sessionID string) (bool, error) { + return false, validatePrincipalSessionID(sessionID, m.principal) +} + +func (m *principalSessionIDManager) Terminate(sessionID string) (bool, error) { + return false, validatePrincipalSessionID(sessionID, m.principal) +} + +func generatePrincipalSessionID(principal infraauth.Principal) (string, error) { + if principal.Issuer == "" || principal.Subject == "" { + return "", errors.New("authenticated principal is unavailable") + } + random := make([]byte, 18) + if _, err := rand.Read(random); err != nil { + return "", err + } + nonce := base64.RawURLEncoding.EncodeToString(random) + return principalSessionPrefix + nonce + "." + principalFingerprint(principal), nil +} + +func validatePrincipalSessionID(sessionID string, principal infraauth.Principal) error { + if principal.Issuer == "" || principal.Subject == "" { + return errors.New("authenticated principal is unavailable") + } + if !strings.HasPrefix(sessionID, principalSessionPrefix) { + return errors.New("invalid session ID") + } + encoded, fingerprint, ok := strings.Cut(strings.TrimPrefix(sessionID, principalSessionPrefix), ".") + if !ok || encoded == "" || fingerprint == "" || strings.Contains(fingerprint, ".") { + return errors.New("invalid session ID") + } + nonce, err := base64.RawURLEncoding.DecodeString(encoded) + if err != nil || len(nonce) != 18 { + return errors.New("invalid session ID") + } + + expected := principalFingerprint(principal) + if len(fingerprint) != len(expected) || + subtle.ConstantTimeCompare([]byte(fingerprint), []byte(expected)) != 1 { + return errors.New("session ID belongs to a different principal") + } + return nil +} + +func principalFingerprint(principal infraauth.Principal) string { + sum := sha256.Sum256([]byte(principal.Issuer + "\x00" + principal.Subject)) + return base64.RawURLEncoding.EncodeToString(sum[:16]) +} From 2b9377253db48c8c96bf3f9bfe166166717787ed Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:28:53 +0200 Subject: [PATCH 04/19] chore(auth): remove unused principal helper --- internal/infra/auth/token_verifier.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/infra/auth/token_verifier.go b/internal/infra/auth/token_verifier.go index 24569ad..95b8e7a 100644 --- a/internal/infra/auth/token_verifier.go +++ b/internal/infra/auth/token_verifier.go @@ -30,10 +30,6 @@ type Principal struct { Subject string } -func (p Principal) valid() bool { - return p.Issuer != "" && p.Subject != "" -} - // TokenVerifier validates an access token presented to the MCP server and // returns the identity that owns any stateful transport session created by the // request. Implementations must never forward the token to an upstream service. From cc9fa3847b326fea5bfb1110a11d36ffb6e791ee Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:31:58 +0200 Subject: [PATCH 05/19] Harden JWT identity and JWKS refresh handling --- internal/infra/auth/token_verifier.go | 199 +++++++++++++++++++++----- 1 file changed, 162 insertions(+), 37 deletions(-) diff --git a/internal/infra/auth/token_verifier.go b/internal/infra/auth/token_verifier.go index 95b8e7a..d24dc12 100644 --- a/internal/infra/auth/token_verifier.go +++ b/internal/infra/auth/token_verifier.go @@ -2,46 +2,47 @@ package auth import ( "context" - "crypto/sha256" - "encoding/hex" "encoding/json" "errors" "fmt" + "io" "net/http" "slices" "strings" + "sync" "time" + jose "github.com/go-jose/go-jose/v4" "github.com/coreos/go-oidc/v3/oidc" ) -const jwksRequestTimeout = 10 * time.Second +const ( + jwksRequestTimeout = 10 * time.Second + jwksMinRefreshInterval = 30 * time.Second + jwksMaxKeyAge = 15 * time.Minute + maxJWKSResponseBodySize = 1 << 20 +) // ErrInsufficientScope distinguishes an authenticated token from one that // lacks the authorization required by this resource. var ErrInsufficientScope = errors.New("access token has insufficient scope") -// Principal is the authenticated identity used to bind stateful MCP sessions. -// Subject is normally the JWT sub claim. Subject-less access tokens fall back -// to a digest of the verified token, which is safe but intentionally makes the -// session valid only for that token's lifetime. +// Principal is the verified identity that owns an MCP request/session. type Principal struct { Issuer string Subject string } -// TokenVerifier validates an access token presented to the MCP server and -// returns the identity that owns any stateful transport session created by the -// request. Implementations must never forward the token to an upstream service. +// TokenVerifier validates an access token presented to the MCP server. +// Implementations must never forward the token to an upstream service. type TokenVerifier interface { Verify(context.Context, string) (Principal, error) } -// JWTVerifier validates signed JWT access tokens against a bounded remote JWKS. +// JWTVerifier validates signed JWT access tokens against a remote JWKS. // Issuer, audience, expiry, and signing algorithm checks are delegated to the // OIDC verifier. Optional scopes are checked after cryptographic validation. type JWTVerifier struct { - issuer string verifier *oidc.IDTokenVerifier requiredScopes []string } @@ -66,7 +67,7 @@ func NewJWTVerifier( } func NewJWTVerifierWithHTTPClient( - _ context.Context, + ctx context.Context, issuer string, audience string, jwksURL string, @@ -84,12 +85,12 @@ func NewJWTVerifierWithHTTPClient( httpClient.Timeout = jwksRequestTimeout } - keySet := newBoundedRemoteKeySet( - jwksURL, + keySet := newRefreshingRemoteKeySet( httpClient, + jwksURL, signingAlgorithms, - defaultJWKSMinRefreshInterval, - defaultJWKSMaxAge, + jwksMinRefreshInterval, + jwksMaxKeyAge, ) verifier := oidc.NewVerifier(issuer, keySet, &oidc.Config{ ClientID: audience, @@ -97,7 +98,6 @@ func NewJWTVerifierWithHTTPClient( }) return &JWTVerifier{ - issuer: issuer, verifier: verifier, requiredScopes: slices.Clone(requiredScopes), } @@ -108,30 +108,25 @@ func (v *JWTVerifier) Verify(ctx context.Context, rawToken string) (Principal, e if err != nil { return Principal{}, fmt.Errorf("validating access token: %w", err) } + if token.Subject == "" { + return Principal{}, errors.New("validating access token: missing sub claim") + } var claims struct { - Subject string `json:"sub"` - Scope json.RawMessage `json:"scope"` - SCP json.RawMessage `json:"scp"` + Scope json.RawMessage `json:"scope"` + SCP json.RawMessage `json:"scp"` } if err := token.Claims(&claims); err != nil { return Principal{}, fmt.Errorf("decoding access token claims: %w", err) } - subject := claims.Subject - if subject == "" { - sum := sha256.Sum256([]byte(rawToken)) - subject = "token-sha256:" + hex.EncodeToString(sum[:]) - } - principal := Principal{Issuer: v.issuer, Subject: subject} - - grantedScopes, err := parseScopeClaim(claims.Scope) + grantedScopes, err := parseScopeClaim("scope", claims.Scope) if err != nil { - return Principal{}, fmt.Errorf("decoding scope claim: %w", err) + return Principal{}, err } - scpScopes, err := parseScopeClaim(claims.SCP) + scpScopes, err := parseScopeClaim("scp", claims.SCP) if err != nil { - return Principal{}, fmt.Errorf("decoding scp claim: %w", err) + return Principal{}, err } grantedScopes = append(grantedScopes, scpScopes...) @@ -141,10 +136,10 @@ func (v *JWTVerifier) Verify(ctx context.Context, rawToken string) (Principal, e } } - return principal, nil + return Principal{Issuer: token.Issuer, Subject: token.Subject}, nil } -func parseScopeClaim(raw json.RawMessage) ([]string, error) { +func parseScopeClaim(name string, raw json.RawMessage) ([]string, error) { if len(raw) == 0 || string(raw) == "null" { return nil, nil } @@ -155,8 +150,138 @@ func parseScopeClaim(raw json.RawMessage) ([]string, error) { } var scopeList []string - if err := json.Unmarshal(raw, &scopeList); err != nil { - return nil, err + if err := json.Unmarshal(raw, &scopeList); err == nil { + return scopeList, nil + } + + return nil, fmt.Errorf("decoding %s claim: expected string or string array", name) +} + +// refreshingRemoteKeySet bounds both sides of JWKS caching: unknown key IDs can +// trigger at most one refresh per minimum interval, while cached keys are never +// trusted beyond maxKeyAge without a successful refresh. +type refreshingRemoteKeySet struct { + client *http.Client + jwksURL string + signingAlgorithms []jose.SignatureAlgorithm + minRefreshInterval time.Duration + maxKeyAge time.Duration + + mu sync.Mutex + keys []jose.JSONWebKey + fetchedAt time.Time + lastRefreshAttempt time.Time +} + +func newRefreshingRemoteKeySet( + client *http.Client, + jwksURL string, + signingAlgorithms []string, + minRefreshInterval time.Duration, + maxKeyAge time.Duration, +) *refreshingRemoteKeySet { + algs := make([]jose.SignatureAlgorithm, 0, len(signingAlgorithms)) + for _, algorithm := range signingAlgorithms { + algs = append(algs, jose.SignatureAlgorithm(algorithm)) + } + return &refreshingRemoteKeySet{ + client: client, + jwksURL: jwksURL, + signingAlgorithms: algs, + minRefreshInterval: minRefreshInterval, + maxKeyAge: maxKeyAge, + } +} + +func (k *refreshingRemoteKeySet) VerifySignature(ctx context.Context, rawToken string) ([]byte, error) { + jws, err := jose.ParseSigned(rawToken, k.signingAlgorithms) + if err != nil { + return nil, fmt.Errorf("parsing jwt: %w", err) + } + if len(jws.Signatures) != 1 { + return nil, errors.New("jwt must contain exactly one signature") + } + keyID := jws.Signatures[0].Header.KeyID + + k.mu.Lock() + defer k.mu.Unlock() + + now := time.Now() + if len(k.keys) == 0 || k.fetchedAt.IsZero() || now.Sub(k.fetchedAt) >= k.maxKeyAge { + if err := k.refreshLocked(ctx, now); err != nil { + return nil, err + } + } + + if payload, ok := verifyWithJWKS(jws, keyID, k.keys); ok { + return payload, nil + } + + // A miss can indicate normal key rotation, but refreshing on every random + // kid lets unauthenticated traffic exhaust the IdP. Rate-limit miss-driven + // refreshes while maxKeyAge still guarantees eventual key eviction. + if now.Sub(k.lastRefreshAttempt) >= k.minRefreshInterval { + if err := k.refreshLocked(ctx, now); err != nil { + return nil, err + } + if payload, ok := verifyWithJWKS(jws, keyID, k.keys); ok { + return payload, nil + } + } + + return nil, errors.New("failed to verify access token signature") +} + +func (k *refreshingRemoteKeySet) refreshLocked(ctx context.Context, now time.Time) error { + k.lastRefreshAttempt = now + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, k.jwksURL, nil) + if err != nil { + return fmt.Errorf("creating JWKS request: %w", err) + } + req.Header.Set("Accept", "application/json") + req.Header.Set("Cache-Control", "no-cache") + + resp, err := k.client.Do(req) + if err != nil { + return fmt.Errorf("fetching JWKS: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxJWKSResponseBodySize+1)) + if err != nil { + return fmt.Errorf("reading JWKS response: %w", err) + } + if len(body) > maxJWKSResponseBodySize { + return fmt.Errorf("JWKS response exceeds %d bytes", maxJWKSResponseBodySize) + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("fetching JWKS: unexpected HTTP status %s", resp.Status) + } + + var keySet jose.JSONWebKeySet + if err := json.Unmarshal(body, &keySet); err != nil { + return fmt.Errorf("decoding JWKS: %w", err) + } + if len(keySet.Keys) == 0 { + return errors.New("decoding JWKS: key set is empty") + } + + k.keys = slices.Clone(keySet.Keys) + k.fetchedAt = now + return nil +} + +func verifyWithJWKS(jws *jose.JSONWebSignature, keyID string, keys []jose.JSONWebKey) ([]byte, bool) { + for i := range keys { + key := &keys[i] + if keyID != "" && key.KeyID != keyID { + continue + } + payload, err := jws.Verify(key) + if err == nil { + return payload, true + } } - return scopeList, nil + return nil, false } From d7fc33c864557d3a8f530b48ef1be18b633629dd Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:32:35 +0200 Subject: [PATCH 06/19] Bind remote MCP sessions to verified principals --- internal/infra/mcp/remote_security.go | 170 ++++++++++++++++++++------ 1 file changed, 134 insertions(+), 36 deletions(-) diff --git a/internal/infra/mcp/remote_security.go b/internal/infra/mcp/remote_security.go index 39cfb4f..623692f 100644 --- a/internal/infra/mcp/remote_security.go +++ b/internal/infra/mcp/remote_security.go @@ -2,6 +2,9 @@ package mcp import ( "context" + "crypto/rand" + "crypto/sha256" + "encoding/hex" "errors" "fmt" "net/http" @@ -12,7 +15,13 @@ import ( infraauth "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/auth" ) -type remotePrincipalContextKey struct{} +const ( + sessionIDPrefix = "mcp-session-" + principalFingerprintSize = 16 + sessionNonceSize = 16 +) + +type principalContextKey struct{} type RemoteSecurity struct { verifier infraauth.TokenVerifier @@ -73,10 +82,7 @@ func (s RemoteSecurity) protect(next http.Handler) http.Handler { next.ServeHTTP(w, r) return } - // Authentication happens outside mcp-go's CORS middleware. Apply the - // simple-response headers here as well so browser clients can read - // WWW-Authenticate on 401/403 responses. - s.applySimpleCORS(w, origin) + applyAuthCORSHeaders(w, origin) } rawToken, ok := bearerToken(r.Header.Values("Authorization")) @@ -95,48 +101,30 @@ func (s RemoteSecurity) protect(next http.Handler) http.Handler { return } - // Legacy SSE carries the session ID in the message URL. Validate its - // owner before mcp-go looks up the session so a leaked URL cannot be - // replayed by another authenticated principal. - if sessionID := r.URL.Query().Get("sessionId"); sessionID != "" { - if err := validatePrincipalSessionID(sessionID, principal); err != nil { - http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) - return - } + if err := validateRequestSessionOwner(r, principal); err != nil { + http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) + return } - ctx := context.WithValue(r.Context(), remotePrincipalContextKey{}, principal) + ctx := context.WithValue(r.Context(), principalContextKey{}, principal) next.ServeHTTP(w, r.WithContext(ctx)) }) } -func principalFromContext(ctx context.Context) (infraauth.Principal, bool) { - principal, ok := ctx.Value(remotePrincipalContextKey{}).(infraauth.Principal) - return principal, ok && principal.Issuer != "" && principal.Subject != "" +func (s RemoteSecurity) corsOrigins() []string { + return append([]string(nil), s.corsAllowedOrigins...) } -func (s RemoteSecurity) applySimpleCORS(w http.ResponseWriter, origin string) { - w.Header().Set("Access-Control-Allow-Origin", origin) - appendVary(w.Header(), "Origin") - w.Header().Set( - "Access-Control-Expose-Headers", - strings.Join([]string{server.HeaderKeySessionID, "WWW-Authenticate"}, ", "), - ) +func (s RemoteSecurity) sessionIDManagerResolver() server.SessionIdManagerResolver { + return principalSessionResolver{} } -func appendVary(header http.Header, value string) { - for _, existing := range header.Values("Vary") { - for _, item := range strings.Split(existing, ",") { - if strings.EqualFold(strings.TrimSpace(item), value) { - return - } - } +func (s RemoteSecurity) newSessionID(ctx context.Context) (string, error) { + principal, ok := principalFromContext(ctx) + if !ok { + return "", errors.New("verified principal missing from request context") } - header.Add("Vary", value) -} - -func (s RemoteSecurity) corsOrigins() []string { - return append([]string(nil), s.corsAllowedOrigins...) + return generatePrincipalSessionID(principal) } func (s RemoteSecurity) writeInsufficientScope(w http.ResponseWriter) { @@ -162,6 +150,12 @@ func (s RemoteSecurity) writeUnauthorized(w http.ResponseWriter, authError strin http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) } +func applyAuthCORSHeaders(w http.ResponseWriter, origin string) { + w.Header().Set("Access-Control-Allow-Origin", origin) + w.Header().Add("Vary", "Origin") + w.Header().Set("Access-Control-Expose-Headers", server.HeaderKeySessionID+", WWW-Authenticate") +} + func requestOrigin(values []string) (origin string, present bool, ok bool) { if len(values) == 0 { return "", false, true @@ -210,3 +204,107 @@ func protectedResourceMetadataURL(resource string) string { u.Fragment = "" return u.String() } + +func principalFromContext(ctx context.Context) (infraauth.Principal, bool) { + principal, ok := ctx.Value(principalContextKey{}).(infraauth.Principal) + return principal, ok && principal.Issuer != "" && principal.Subject != "" +} + +func principalFingerprint(principal infraauth.Principal) string { + sum := sha256.Sum256([]byte(principal.Issuer + "\x00" + principal.Subject)) + return hex.EncodeToString(sum[:principalFingerprintSize]) +} + +func generatePrincipalSessionID(principal infraauth.Principal) (string, error) { + nonce := make([]byte, sessionNonceSize) + if _, err := rand.Read(nonce); err != nil { + return "", fmt.Errorf("generating MCP session ID: %w", err) + } + return sessionIDPrefix + principalFingerprint(principal) + "-" + hex.EncodeToString(nonce), nil +} + +func sessionPrincipalFingerprint(sessionID string) (string, bool) { + if !strings.HasPrefix(sessionID, sessionIDPrefix) { + return "", false + } + rest := strings.TrimPrefix(sessionID, sessionIDPrefix) + parts := strings.SplitN(rest, "-", 2) + if len(parts) != 2 || + len(parts[0]) != principalFingerprintSize*2 || + len(parts[1]) != sessionNonceSize*2 { + return "", false + } + if _, err := hex.DecodeString(parts[0]); err != nil { + return "", false + } + if _, err := hex.DecodeString(parts[1]); err != nil { + return "", false + } + return parts[0], true +} + +func validateSessionOwner(sessionID string, principal infraauth.Principal) error { + fingerprint, ok := sessionPrincipalFingerprint(sessionID) + if !ok { + return errors.New("invalid MCP session ID") + } + if fingerprint != principalFingerprint(principal) { + return errors.New("MCP session belongs to a different principal") + } + return nil +} + +func validateRequestSessionOwner(r *http.Request, principal infraauth.Principal) error { + if sessionID := r.Header.Get(server.HeaderKeySessionID); sessionID != "" { + if err := validateSessionOwner(sessionID, principal); err != nil { + return err + } + } + if sessionID := r.URL.Query().Get("sessionId"); sessionID != "" { + if err := validateSessionOwner(sessionID, principal); err != nil { + return err + } + } + return nil +} + +type principalSessionResolver struct{} + +func (principalSessionResolver) ResolveSessionIdManager(r *http.Request) server.SessionIdManager { + if r == nil { + return principalSessionManager{} + } + principal, _ := principalFromContext(r.Context()) + return principalSessionManager{fingerprint: principalFingerprint(principal)} +} + +type principalSessionManager struct { + fingerprint string +} + +func (m principalSessionManager) Generate() string { + if m.fingerprint == "" { + return "" + } + nonce := make([]byte, sessionNonceSize) + if _, err := rand.Read(nonce); err != nil { + return "" + } + return sessionIDPrefix + m.fingerprint + "-" + hex.EncodeToString(nonce) +} + +func (m principalSessionManager) Validate(sessionID string) (bool, error) { + fingerprint, ok := sessionPrincipalFingerprint(sessionID) + if !ok { + return false, errors.New("invalid MCP session ID") + } + if m.fingerprint != "" && fingerprint != m.fingerprint { + return false, errors.New("MCP session belongs to a different principal") + } + return false, nil +} + +func (m principalSessionManager) Terminate(sessionID string) (bool, error) { + _, err := m.Validate(sessionID) + return false, err +} From 38bd55b8643e64c561f17e36bf73cd8b9a18c50b Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:32:43 +0200 Subject: [PATCH 07/19] Apply principal-bound sessions across remote transports From 076cd2ae496d23ed8810a36db0c1c28313debc18 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:32:59 +0200 Subject: [PATCH 08/19] Preserve stdio HTTP compatibility while tightening remote URLs --- internal/config/config.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/config/config.go b/internal/config/config.go index ab777b0..808c636 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -66,6 +66,9 @@ func (c *Config) Validate() error { if c.Transport == "stdio" { return nil } + if err := requireSecureURL("SYSDIG_MCP_API_HOST", apiHost); err != nil { + return err + } if err := validateSecureURL("SYSDIG_MCP_API_HOST", apiHost); err != nil { return err } From 9043c0098ff9062d99ec9ff179372d2b99843602 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:33:13 +0200 Subject: [PATCH 09/19] Cover principal extraction and array scope claims --- internal/infra/auth/token_verifier_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/infra/auth/token_verifier_test.go b/internal/infra/auth/token_verifier_test.go index 4f3466a..acd5b9b 100644 --- a/internal/infra/auth/token_verifier_test.go +++ b/internal/infra/auth/token_verifier_test.go @@ -110,6 +110,15 @@ func TestJWTVerifier(t *testing.T) { signingAlgorithms: []string{"RS256"}, requiredScopes: []string{"mcp:tools"}, }, + { + name: "valid scope array claim", + issuer: testIssuer, + audience: jwt.Audience{testAudience}, + expiry: time.Now().Add(time.Hour), + claims: accessTokenClaims{Scope: []string{"mcp:tools", "profile"}}, + signingAlgorithms: []string{"RS256"}, + requiredScopes: []string{"mcp:tools", "profile"}, + }, { name: "valid scp claim", issuer: testIssuer, From 7cf95323445aa4e69fc8bb5e9af91246251955f0 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:33:35 +0200 Subject: [PATCH 10/19] Test session ownership and OAuth CORS behavior --- internal/infra/mcp/mcp_handler_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/internal/infra/mcp/mcp_handler_test.go b/internal/infra/mcp/mcp_handler_test.go index 31d3a71..ded5ebc 100644 --- a/internal/infra/mcp/mcp_handler_test.go +++ b/internal/infra/mcp/mcp_handler_test.go @@ -167,6 +167,15 @@ var _ = Describe("McpHandler", func() { Expect(resp.StatusCode).To(Equal(http.StatusOK)) Expect(verifier.calls).To(Equal(2)) }, NodeTimeout(5*time.Second)) + It("rejects reuse of a session by a different valid principal", func(ctx SpecContext) { + testClient.Initialize(ctx, authorizationHeaders()) + + resp := testClient.ListTools(ctx, tokenHeaders(otherMCPToken)) + defer func() { _ = resp.Body.Close() }() + + Expect(resp.StatusCode).To(Equal(http.StatusForbidden)) + }, NodeTimeout(5*time.Second)) + It("binds a stateful session to the authenticated principal", func(ctx SpecContext) { testClient.Initialize(ctx, authorizationHeaders()) @@ -197,6 +206,18 @@ var _ = Describe("McpHandler", func() { Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized)) Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring(`error="invalid_token"`)) }) + It("keeps OAuth discovery headers visible on browser auth failures", func() { + headers := tokenHeaders("wrong-token") + headers.Set("Origin", allowedOrigin) + resp := testClient.RPC(context.Background(), "tools/list", nil, headers) + defer func() { _ = resp.Body.Close() }() + + Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized)) + Expect(resp.Header.Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) + Expect(resp.Header.Get("Access-Control-Expose-Headers")).To(ContainSubstring("WWW-Authenticate")) + Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring(`error="invalid_token"`)) + }) + It("exposes OAuth challenges on allowlisted browser auth failures", func() { headers := http.Header{"Authorization": []string{"Bearer wrong-token"}} From ea1bcc9b40ba0deed73c4fffe9ba767416a37090 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:33:43 +0200 Subject: [PATCH 11/19] Cover stdio on-prem HTTP compatibility --- internal/config/config_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 51474e7..d23391c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -72,6 +72,7 @@ var _ = Describe("Config", func() { Expect(cfg.Validate()).To(MatchError(ContainSubstring(expected))) }, Entry("relative API host", func(cfg *config.Config) { cfg.APIHost = "app.example.com" }, "absolute URL"), + Entry("plaintext remote API host", func(cfg *config.Config) { cfg.APIHost = "http://10.0.0.5" }, "must use https"), Entry("API host query", func(cfg *config.Config) { cfg.APIHost += "?tenant=one" }, "query string"), Entry("plaintext resource", func(cfg *config.Config) { cfg.ResourceURL = "http://mcp.example.com/sysdig-mcp-server" }, "must use https"), Entry("resource query", func(cfg *config.Config) { cfg.ResourceURL += "?tenant=one" }, "query string"), @@ -112,6 +113,13 @@ var _ = Describe("Config", func() { Expect(cfg.Validate()).To(Succeed()) }) + It("preserves stdio access to on-prem HTTP API endpoints", func() { + cfg := validConfig("stdio") + cfg.APIHost = "http://10.0.0.5" + Expect(cfg.Validate()).To(Succeed()) + }) + + It("allows HTTP only for loopback development", func() { cfg := validConfig("streamable-http") cfg.APIHost = "http://127.0.0.1:9000" From 5b6f7b8e9fc8cab64ab971307414811dd0f97005 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:38:14 +0200 Subject: [PATCH 12/19] Avoid parsing unused scope claims --- internal/infra/auth/token_verifier.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/infra/auth/token_verifier.go b/internal/infra/auth/token_verifier.go index d24dc12..4a361a9 100644 --- a/internal/infra/auth/token_verifier.go +++ b/internal/infra/auth/token_verifier.go @@ -112,6 +112,11 @@ func (v *JWTVerifier) Verify(ctx context.Context, rawToken string) (Principal, e return Principal{}, errors.New("validating access token: missing sub claim") } + principal := Principal{Issuer: token.Issuer, Subject: token.Subject} + if len(v.requiredScopes) == 0 { + return principal, nil + } + var claims struct { Scope json.RawMessage `json:"scope"` SCP json.RawMessage `json:"scp"` @@ -136,7 +141,7 @@ func (v *JWTVerifier) Verify(ctx context.Context, rawToken string) (Principal, e } } - return Principal{Issuer: token.Issuer, Subject: token.Subject}, nil + return principal, nil } func parseScopeClaim(name string, raw json.RawMessage) ([]string, error) { From 67896aebeba1fab631e591017870979e546bb7e4 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:38:24 +0200 Subject: [PATCH 13/19] Fail closed when session principal context is missing --- internal/infra/mcp/remote_security.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/infra/mcp/remote_security.go b/internal/infra/mcp/remote_security.go index 623692f..cc4ad3b 100644 --- a/internal/infra/mcp/remote_security.go +++ b/internal/infra/mcp/remote_security.go @@ -274,7 +274,10 @@ func (principalSessionResolver) ResolveSessionIdManager(r *http.Request) server. if r == nil { return principalSessionManager{} } - principal, _ := principalFromContext(r.Context()) + principal, ok := principalFromContext(r.Context()) + if !ok { + return principalSessionManager{} + } return principalSessionManager{fingerprint: principalFingerprint(principal)} } From c390528c2f718ee773b9604351754bd957ae11e2 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:40:44 +0200 Subject: [PATCH 14/19] fix(auth): converge remote session and JWKS hardening --- internal/config/config.go | 3 - internal/config/config_test.go | 7 -- internal/infra/auth/token_verifier.go | 156 ++----------------------- internal/infra/mcp/mcp_handler_test.go | 23 ---- internal/infra/mcp/remote_security.go | 133 ++++----------------- 5 files changed, 30 insertions(+), 292 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 808c636..ab777b0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -66,9 +66,6 @@ func (c *Config) Validate() error { if c.Transport == "stdio" { return nil } - if err := requireSecureURL("SYSDIG_MCP_API_HOST", apiHost); err != nil { - return err - } if err := validateSecureURL("SYSDIG_MCP_API_HOST", apiHost); err != nil { return err } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d23391c..c4a9e08 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -113,13 +113,6 @@ var _ = Describe("Config", func() { Expect(cfg.Validate()).To(Succeed()) }) - It("preserves stdio access to on-prem HTTP API endpoints", func() { - cfg := validConfig("stdio") - cfg.APIHost = "http://10.0.0.5" - Expect(cfg.Validate()).To(Succeed()) - }) - - It("allows HTTP only for loopback development", func() { cfg := validConfig("streamable-http") cfg.APIHost = "http://127.0.0.1:9000" diff --git a/internal/infra/auth/token_verifier.go b/internal/infra/auth/token_verifier.go index 4a361a9..59fd614 100644 --- a/internal/infra/auth/token_verifier.go +++ b/internal/infra/auth/token_verifier.go @@ -5,23 +5,15 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" "slices" "strings" - "sync" "time" - jose "github.com/go-jose/go-jose/v4" "github.com/coreos/go-oidc/v3/oidc" ) -const ( - jwksRequestTimeout = 10 * time.Second - jwksMinRefreshInterval = 30 * time.Second - jwksMaxKeyAge = 15 * time.Minute - maxJWKSResponseBodySize = 1 << 20 -) +const jwksRequestTimeout = 10 * time.Second // ErrInsufficientScope distinguishes an authenticated token from one that // lacks the authorization required by this resource. @@ -33,13 +25,14 @@ type Principal struct { Subject string } -// TokenVerifier validates an access token presented to the MCP server. -// Implementations must never forward the token to an upstream service. +// TokenVerifier validates an access token presented to the MCP server and +// returns the identity that owns any stateful MCP session created by the +// request. Implementations must never forward the token upstream. type TokenVerifier interface { Verify(context.Context, string) (Principal, error) } -// JWTVerifier validates signed JWT access tokens against a remote JWKS. +// JWTVerifier validates signed JWT access tokens against a bounded remote JWKS. // Issuer, audience, expiry, and signing algorithm checks are delegated to the // OIDC verifier. Optional scopes are checked after cryptographic validation. type JWTVerifier struct { @@ -67,7 +60,7 @@ func NewJWTVerifier( } func NewJWTVerifierWithHTTPClient( - ctx context.Context, + _ context.Context, issuer string, audience string, jwksURL string, @@ -85,12 +78,12 @@ func NewJWTVerifierWithHTTPClient( httpClient.Timeout = jwksRequestTimeout } - keySet := newRefreshingRemoteKeySet( - httpClient, + keySet := newBoundedRemoteKeySet( jwksURL, + httpClient, signingAlgorithms, - jwksMinRefreshInterval, - jwksMaxKeyAge, + defaultJWKSMinRefreshInterval, + defaultJWKSMaxAge, ) verifier := oidc.NewVerifier(issuer, keySet, &oidc.Config{ ClientID: audience, @@ -161,132 +154,3 @@ func parseScopeClaim(name string, raw json.RawMessage) ([]string, error) { return nil, fmt.Errorf("decoding %s claim: expected string or string array", name) } - -// refreshingRemoteKeySet bounds both sides of JWKS caching: unknown key IDs can -// trigger at most one refresh per minimum interval, while cached keys are never -// trusted beyond maxKeyAge without a successful refresh. -type refreshingRemoteKeySet struct { - client *http.Client - jwksURL string - signingAlgorithms []jose.SignatureAlgorithm - minRefreshInterval time.Duration - maxKeyAge time.Duration - - mu sync.Mutex - keys []jose.JSONWebKey - fetchedAt time.Time - lastRefreshAttempt time.Time -} - -func newRefreshingRemoteKeySet( - client *http.Client, - jwksURL string, - signingAlgorithms []string, - minRefreshInterval time.Duration, - maxKeyAge time.Duration, -) *refreshingRemoteKeySet { - algs := make([]jose.SignatureAlgorithm, 0, len(signingAlgorithms)) - for _, algorithm := range signingAlgorithms { - algs = append(algs, jose.SignatureAlgorithm(algorithm)) - } - return &refreshingRemoteKeySet{ - client: client, - jwksURL: jwksURL, - signingAlgorithms: algs, - minRefreshInterval: minRefreshInterval, - maxKeyAge: maxKeyAge, - } -} - -func (k *refreshingRemoteKeySet) VerifySignature(ctx context.Context, rawToken string) ([]byte, error) { - jws, err := jose.ParseSigned(rawToken, k.signingAlgorithms) - if err != nil { - return nil, fmt.Errorf("parsing jwt: %w", err) - } - if len(jws.Signatures) != 1 { - return nil, errors.New("jwt must contain exactly one signature") - } - keyID := jws.Signatures[0].Header.KeyID - - k.mu.Lock() - defer k.mu.Unlock() - - now := time.Now() - if len(k.keys) == 0 || k.fetchedAt.IsZero() || now.Sub(k.fetchedAt) >= k.maxKeyAge { - if err := k.refreshLocked(ctx, now); err != nil { - return nil, err - } - } - - if payload, ok := verifyWithJWKS(jws, keyID, k.keys); ok { - return payload, nil - } - - // A miss can indicate normal key rotation, but refreshing on every random - // kid lets unauthenticated traffic exhaust the IdP. Rate-limit miss-driven - // refreshes while maxKeyAge still guarantees eventual key eviction. - if now.Sub(k.lastRefreshAttempt) >= k.minRefreshInterval { - if err := k.refreshLocked(ctx, now); err != nil { - return nil, err - } - if payload, ok := verifyWithJWKS(jws, keyID, k.keys); ok { - return payload, nil - } - } - - return nil, errors.New("failed to verify access token signature") -} - -func (k *refreshingRemoteKeySet) refreshLocked(ctx context.Context, now time.Time) error { - k.lastRefreshAttempt = now - - req, err := http.NewRequestWithContext(ctx, http.MethodGet, k.jwksURL, nil) - if err != nil { - return fmt.Errorf("creating JWKS request: %w", err) - } - req.Header.Set("Accept", "application/json") - req.Header.Set("Cache-Control", "no-cache") - - resp, err := k.client.Do(req) - if err != nil { - return fmt.Errorf("fetching JWKS: %w", err) - } - defer resp.Body.Close() - - body, err := io.ReadAll(io.LimitReader(resp.Body, maxJWKSResponseBodySize+1)) - if err != nil { - return fmt.Errorf("reading JWKS response: %w", err) - } - if len(body) > maxJWKSResponseBodySize { - return fmt.Errorf("JWKS response exceeds %d bytes", maxJWKSResponseBodySize) - } - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("fetching JWKS: unexpected HTTP status %s", resp.Status) - } - - var keySet jose.JSONWebKeySet - if err := json.Unmarshal(body, &keySet); err != nil { - return fmt.Errorf("decoding JWKS: %w", err) - } - if len(keySet.Keys) == 0 { - return errors.New("decoding JWKS: key set is empty") - } - - k.keys = slices.Clone(keySet.Keys) - k.fetchedAt = now - return nil -} - -func verifyWithJWKS(jws *jose.JSONWebSignature, keyID string, keys []jose.JSONWebKey) ([]byte, bool) { - for i := range keys { - key := &keys[i] - if keyID != "" && key.KeyID != keyID { - continue - } - payload, err := jws.Verify(key) - if err == nil { - return payload, true - } - } - return nil, false -} diff --git a/internal/infra/mcp/mcp_handler_test.go b/internal/infra/mcp/mcp_handler_test.go index ded5ebc..e178418 100644 --- a/internal/infra/mcp/mcp_handler_test.go +++ b/internal/infra/mcp/mcp_handler_test.go @@ -167,16 +167,6 @@ var _ = Describe("McpHandler", func() { Expect(resp.StatusCode).To(Equal(http.StatusOK)) Expect(verifier.calls).To(Equal(2)) }, NodeTimeout(5*time.Second)) - It("rejects reuse of a session by a different valid principal", func(ctx SpecContext) { - testClient.Initialize(ctx, authorizationHeaders()) - - resp := testClient.ListTools(ctx, tokenHeaders(otherMCPToken)) - defer func() { _ = resp.Body.Close() }() - - Expect(resp.StatusCode).To(Equal(http.StatusForbidden)) - }, NodeTimeout(5*time.Second)) - - It("binds a stateful session to the authenticated principal", func(ctx SpecContext) { testClient.Initialize(ctx, authorizationHeaders()) Expect(testClient.sessionID).NotTo(BeEmpty()) @@ -206,19 +196,6 @@ var _ = Describe("McpHandler", func() { Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized)) Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring(`error="invalid_token"`)) }) - It("keeps OAuth discovery headers visible on browser auth failures", func() { - headers := tokenHeaders("wrong-token") - headers.Set("Origin", allowedOrigin) - resp := testClient.RPC(context.Background(), "tools/list", nil, headers) - defer func() { _ = resp.Body.Close() }() - - Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized)) - Expect(resp.Header.Get("Access-Control-Allow-Origin")).To(Equal(allowedOrigin)) - Expect(resp.Header.Get("Access-Control-Expose-Headers")).To(ContainSubstring("WWW-Authenticate")) - Expect(resp.Header.Get("WWW-Authenticate")).To(ContainSubstring(`error="invalid_token"`)) - }) - - It("exposes OAuth challenges on allowlisted browser auth failures", func() { headers := http.Header{"Authorization": []string{"Bearer wrong-token"}} headers.Set("Origin", allowedOrigin) diff --git a/internal/infra/mcp/remote_security.go b/internal/infra/mcp/remote_security.go index cc4ad3b..4032939 100644 --- a/internal/infra/mcp/remote_security.go +++ b/internal/infra/mcp/remote_security.go @@ -2,9 +2,6 @@ package mcp import ( "context" - "crypto/rand" - "crypto/sha256" - "encoding/hex" "errors" "fmt" "net/http" @@ -15,12 +12,6 @@ import ( infraauth "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/auth" ) -const ( - sessionIDPrefix = "mcp-session-" - principalFingerprintSize = 16 - sessionNonceSize = 16 -) - type principalContextKey struct{} type RemoteSecurity struct { @@ -82,6 +73,9 @@ func (s RemoteSecurity) protect(next http.Handler) http.Handler { next.ServeHTTP(w, r) return } + // Authentication runs outside mcp-go's CORS middleware. Add the + // simple-response headers here too so a browser can read the OAuth + // challenge from a 401/403 response. applyAuthCORSHeaders(w, origin) } @@ -102,7 +96,9 @@ func (s RemoteSecurity) protect(next http.Handler) http.Handler { } if err := validateRequestSessionOwner(r, principal); err != nil { - http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) + // mcp-go uses 404 for an unknown/invalid session. Preserve that + // behavior instead of exposing whether a leaked session exists. + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) return } @@ -115,18 +111,6 @@ func (s RemoteSecurity) corsOrigins() []string { return append([]string(nil), s.corsAllowedOrigins...) } -func (s RemoteSecurity) sessionIDManagerResolver() server.SessionIdManagerResolver { - return principalSessionResolver{} -} - -func (s RemoteSecurity) newSessionID(ctx context.Context) (string, error) { - principal, ok := principalFromContext(ctx) - if !ok { - return "", errors.New("verified principal missing from request context") - } - return generatePrincipalSessionID(principal) -} - func (s RemoteSecurity) writeInsufficientScope(w http.ResponseWriter) { challenge := fmt.Sprintf( `Bearer resource_metadata=%q, error="insufficient_scope", scope=%q`, @@ -152,10 +136,21 @@ func (s RemoteSecurity) writeUnauthorized(w http.ResponseWriter, authError strin func applyAuthCORSHeaders(w http.ResponseWriter, origin string) { w.Header().Set("Access-Control-Allow-Origin", origin) - w.Header().Add("Vary", "Origin") + appendVary(w.Header(), "Origin") w.Header().Set("Access-Control-Expose-Headers", server.HeaderKeySessionID+", WWW-Authenticate") } +func appendVary(header http.Header, value string) { + for _, existing := range header.Values("Vary") { + for _, item := range strings.Split(existing, ",") { + if strings.EqualFold(strings.TrimSpace(item), value) { + return + } + } + } + header.Add("Vary", value) +} + func requestOrigin(values []string) (origin string, present bool, ok bool) { if len(values) == 0 { return "", false, true @@ -210,104 +205,16 @@ func principalFromContext(ctx context.Context) (infraauth.Principal, bool) { return principal, ok && principal.Issuer != "" && principal.Subject != "" } -func principalFingerprint(principal infraauth.Principal) string { - sum := sha256.Sum256([]byte(principal.Issuer + "\x00" + principal.Subject)) - return hex.EncodeToString(sum[:principalFingerprintSize]) -} - -func generatePrincipalSessionID(principal infraauth.Principal) (string, error) { - nonce := make([]byte, sessionNonceSize) - if _, err := rand.Read(nonce); err != nil { - return "", fmt.Errorf("generating MCP session ID: %w", err) - } - return sessionIDPrefix + principalFingerprint(principal) + "-" + hex.EncodeToString(nonce), nil -} - -func sessionPrincipalFingerprint(sessionID string) (string, bool) { - if !strings.HasPrefix(sessionID, sessionIDPrefix) { - return "", false - } - rest := strings.TrimPrefix(sessionID, sessionIDPrefix) - parts := strings.SplitN(rest, "-", 2) - if len(parts) != 2 || - len(parts[0]) != principalFingerprintSize*2 || - len(parts[1]) != sessionNonceSize*2 { - return "", false - } - if _, err := hex.DecodeString(parts[0]); err != nil { - return "", false - } - if _, err := hex.DecodeString(parts[1]); err != nil { - return "", false - } - return parts[0], true -} - -func validateSessionOwner(sessionID string, principal infraauth.Principal) error { - fingerprint, ok := sessionPrincipalFingerprint(sessionID) - if !ok { - return errors.New("invalid MCP session ID") - } - if fingerprint != principalFingerprint(principal) { - return errors.New("MCP session belongs to a different principal") - } - return nil -} - func validateRequestSessionOwner(r *http.Request, principal infraauth.Principal) error { if sessionID := r.Header.Get(server.HeaderKeySessionID); sessionID != "" { - if err := validateSessionOwner(sessionID, principal); err != nil { + if err := validatePrincipalSessionID(sessionID, principal); err != nil { return err } } if sessionID := r.URL.Query().Get("sessionId"); sessionID != "" { - if err := validateSessionOwner(sessionID, principal); err != nil { + if err := validatePrincipalSessionID(sessionID, principal); err != nil { return err } } return nil } - -type principalSessionResolver struct{} - -func (principalSessionResolver) ResolveSessionIdManager(r *http.Request) server.SessionIdManager { - if r == nil { - return principalSessionManager{} - } - principal, ok := principalFromContext(r.Context()) - if !ok { - return principalSessionManager{} - } - return principalSessionManager{fingerprint: principalFingerprint(principal)} -} - -type principalSessionManager struct { - fingerprint string -} - -func (m principalSessionManager) Generate() string { - if m.fingerprint == "" { - return "" - } - nonce := make([]byte, sessionNonceSize) - if _, err := rand.Read(nonce); err != nil { - return "" - } - return sessionIDPrefix + m.fingerprint + "-" + hex.EncodeToString(nonce) -} - -func (m principalSessionManager) Validate(sessionID string) (bool, error) { - fingerprint, ok := sessionPrincipalFingerprint(sessionID) - if !ok { - return false, errors.New("invalid MCP session ID") - } - if m.fingerprint != "" && fingerprint != m.fingerprint { - return false, errors.New("MCP session belongs to a different principal") - } - return false, nil -} - -func (m principalSessionManager) Terminate(sessionID string) (bool, error) { - _, err := m.Validate(sessionID) - return false, err -} From 66085f32516f0f506c5069259b08494a61900175 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:46:31 +0200 Subject: [PATCH 15/19] Respect sessionless Streamable HTTP modes --- internal/infra/mcp/remote_security.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/internal/infra/mcp/remote_security.go b/internal/infra/mcp/remote_security.go index 4032939..9f4cfe7 100644 --- a/internal/infra/mcp/remote_security.go +++ b/internal/infra/mcp/remote_security.go @@ -8,6 +8,7 @@ import ( "net/url" "strings" + mcpprotocol "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" infraauth "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/auth" ) @@ -57,7 +58,7 @@ func NewRemoteSecurity( } } -func (s RemoteSecurity) protect(next http.Handler) http.Handler { +func (s RemoteSecurity) protect(next http.Handler, bindSessions bool) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { origin, hasOrigin, ok := requestOrigin(r.Header.Values("Origin")) if !ok { @@ -95,11 +96,13 @@ func (s RemoteSecurity) protect(next http.Handler) http.Handler { return } - if err := validateRequestSessionOwner(r, principal); err != nil { - // mcp-go uses 404 for an unknown/invalid session. Preserve that - // behavior instead of exposing whether a leaked session exists. - http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) - return + if bindSessions { + if err := validateRequestSessionOwner(r, principal); err != nil { + // mcp-go uses 404 for an unknown/invalid session. Preserve that + // behavior instead of exposing whether a leaked session exists. + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) + return + } } ctx := context.WithValue(r.Context(), principalContextKey{}, principal) @@ -206,9 +209,14 @@ func principalFromContext(ctx context.Context) (infraauth.Principal, bool) { } func validateRequestSessionOwner(r *http.Request, principal infraauth.Principal) error { - if sessionID := r.Header.Get(server.HeaderKeySessionID); sessionID != "" { - if err := validatePrincipalSessionID(sessionID, principal); err != nil { - return err + // Protocol 2026-07-28 removed protocol-level sessions. Match mcp-go and + // ignore a stale Mcp-Session-Id rather than applying legacy ownership + // semantics to a modern request. + if r.Header.Get(mcpprotocol.HeaderProtocolVersion) != mcpprotocol.ProtocolVersion20260728 { + if sessionID := r.Header.Get(mcpprotocol.HeaderSessionID); sessionID != "" { + if err := validatePrincipalSessionID(sessionID, principal); err != nil { + return err + } } } if sessionID := r.URL.Query().Get("sessionId"); sessionID != "" { From d89d1f5985e493c993326f7f2637612cb6d92c18 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:46:41 +0200 Subject: [PATCH 16/19] Bind sessions only for stateful remote transports --- internal/infra/mcp/mcp_handler.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/infra/mcp/mcp_handler.go b/internal/infra/mcp/mcp_handler.go index 4ae9140..e116667 100644 --- a/internal/infra/mcp/mcp_handler.go +++ b/internal/infra/mcp/mcp_handler.go @@ -107,7 +107,7 @@ func (h *Handler) AsStreamableHTTP(mountPath string, stateless bool, security Re httpServer := server.NewStreamableHTTPServer(h.server, opts...) security.mountMetadata(mux) - mux.Handle(mountPath, security.protect(httpServer)) + mux.Handle(mountPath, security.protect(httpServer, !stateless)) return mux } @@ -126,8 +126,8 @@ func (h *Handler) AsSSE(mountPath string, security RemoteSecurity) http.Handler }), ) security.mountMetadata(mux) - mux.Handle(sseServer.CompleteSsePath(), security.protect(sseServer.SSEHandler())) - mux.Handle(sseServer.CompleteMessagePath(), security.protect(sseServer.MessageHandler())) + mux.Handle(sseServer.CompleteSsePath(), security.protect(sseServer.SSEHandler(), true)) + mux.Handle(sseServer.CompleteMessagePath(), security.protect(sseServer.MessageHandler(), true)) return mux } From 135af14ffabaecd1c758d63c7b1a3738aae0c367 Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:47:14 +0200 Subject: [PATCH 17/19] Cover sessionless Streamable HTTP behavior --- internal/infra/mcp/mcp_handler_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/internal/infra/mcp/mcp_handler_test.go b/internal/infra/mcp/mcp_handler_test.go index e178418..eb65cbe 100644 --- a/internal/infra/mcp/mcp_handler_test.go +++ b/internal/infra/mcp/mcp_handler_test.go @@ -176,6 +176,19 @@ var _ = Describe("McpHandler", func() { Expect(resp.StatusCode).To(Equal(http.StatusNotFound)) }, NodeTimeout(5*time.Second)) + It("does not apply legacy session ownership to the 2026-07-28 protocol", func(ctx SpecContext) { + modernClient := NewHTTPTestClient(handler.AsStreamableHTTP("/", false, remoteSecurity(verifier))) + headers := authorizationHeaders() + headers.Set(mcp.HeaderProtocolVersion, mcp.ProtocolVersion20260728) + headers.Set(mcp.HeaderMethod, string(mcp.MethodPing)) + headers.Set(mcp.HeaderSessionID, "stale-session-id") + + resp := modernClient.RPC(ctx, "ping", nil, headers) + defer func() { _ = resp.Body.Close() }() + + Expect(resp.StatusCode).NotTo(Equal(http.StatusNotFound)) + }, NodeTimeout(5*time.Second)) + DescribeTable("rejects invalid authorization headers", func(headers http.Header) { resp := testClient.RPC(context.Background(), "tools/list", nil, headers) @@ -373,6 +386,17 @@ var _ = Describe("McpHandler", func() { Expect(resp.StatusCode).To(Equal(http.StatusOK)) Expect(resp.Header.Get("Mcp-Session-Id")).To(BeEmpty()) }, NodeTimeout(5*time.Second)) + + It("ignores legacy session IDs in explicit stateless mode", func(ctx SpecContext) { + statelessClient := NewHTTPTestClient(handler.AsStreamableHTTP("/", true, remoteSecurity(verifier))) + headers := authorizationHeaders() + headers.Set(mcp.HeaderSessionID, "not-a-valid-session") + + resp := statelessClient.RPC(ctx, "ping", nil, headers) + defer func() { _ = resp.Body.Close() }() + + Expect(resp.StatusCode).NotTo(Equal(http.StatusNotFound)) + }, NodeTimeout(5*time.Second)) }) Context("Stdio", func() { From 39548e57f734c51b23054a2cfc34f7cd0df9db3e Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:50:59 +0200 Subject: [PATCH 18/19] Allow server lifecycle to parse bound session IDs --- internal/infra/mcp/session_security.go | 34 +++++++++++++++++--------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/internal/infra/mcp/session_security.go b/internal/infra/mcp/session_security.go index d3b0cd4..5c25940 100644 --- a/internal/infra/mcp/session_security.go +++ b/internal/infra/mcp/session_security.go @@ -41,11 +41,15 @@ func (m *principalSessionIDManager) Generate() string { } func (m *principalSessionIDManager) Validate(sessionID string) (bool, error) { + if m.principal.Issuer == "" || m.principal.Subject == "" { + _, err := parsePrincipalSessionID(sessionID) + return false, err + } return false, validatePrincipalSessionID(sessionID, m.principal) } func (m *principalSessionIDManager) Terminate(sessionID string) (bool, error) { - return false, validatePrincipalSessionID(sessionID, m.principal) + return m.Validate(sessionID) } func generatePrincipalSessionID(principal infraauth.Principal) (string, error) { @@ -64,16 +68,9 @@ func validatePrincipalSessionID(sessionID string, principal infraauth.Principal) if principal.Issuer == "" || principal.Subject == "" { return errors.New("authenticated principal is unavailable") } - if !strings.HasPrefix(sessionID, principalSessionPrefix) { - return errors.New("invalid session ID") - } - encoded, fingerprint, ok := strings.Cut(strings.TrimPrefix(sessionID, principalSessionPrefix), ".") - if !ok || encoded == "" || fingerprint == "" || strings.Contains(fingerprint, ".") { - return errors.New("invalid session ID") - } - nonce, err := base64.RawURLEncoding.DecodeString(encoded) - if err != nil || len(nonce) != 18 { - return errors.New("invalid session ID") + fingerprint, err := parsePrincipalSessionID(sessionID) + if err != nil { + return err } expected := principalFingerprint(principal) @@ -84,6 +81,21 @@ func validatePrincipalSessionID(sessionID string, principal infraauth.Principal) return nil } +func parsePrincipalSessionID(sessionID string) (string, error) { + if !strings.HasPrefix(sessionID, principalSessionPrefix) { + return "", errors.New("invalid session ID") + } + encoded, fingerprint, ok := strings.Cut(strings.TrimPrefix(sessionID, principalSessionPrefix), ".") + if !ok || encoded == "" || fingerprint == "" || strings.Contains(fingerprint, ".") { + return "", errors.New("invalid session ID") + } + nonce, err := base64.RawURLEncoding.DecodeString(encoded) + if err != nil || len(nonce) != 18 { + return "", errors.New("invalid session ID") + } + return fingerprint, nil +} + func principalFingerprint(principal infraauth.Principal) string { sum := sha256.Sum256([]byte(principal.Issuer + "\x00" + principal.Subject)) return base64.RawURLEncoding.EncodeToString(sum[:16]) From 15c041a27c24b68f54069427dd7e5b254a0bd08a Mon Sep 17 00:00:00 2001 From: Riccardo Menegazzo <52837869+riccardomenegazzo@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:51:11 +0200 Subject: [PATCH 19/19] Assert sessionless requests complete successfully --- internal/infra/mcp/mcp_handler_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/infra/mcp/mcp_handler_test.go b/internal/infra/mcp/mcp_handler_test.go index eb65cbe..370f126 100644 --- a/internal/infra/mcp/mcp_handler_test.go +++ b/internal/infra/mcp/mcp_handler_test.go @@ -186,7 +186,7 @@ var _ = Describe("McpHandler", func() { resp := modernClient.RPC(ctx, "ping", nil, headers) defer func() { _ = resp.Body.Close() }() - Expect(resp.StatusCode).NotTo(Equal(http.StatusNotFound)) + Expect(resp.StatusCode).To(Equal(http.StatusOK)) }, NodeTimeout(5*time.Second)) DescribeTable("rejects invalid authorization headers", @@ -395,7 +395,7 @@ var _ = Describe("McpHandler", func() { resp := statelessClient.RPC(ctx, "ping", nil, headers) defer func() { _ = resp.Body.Close() }() - Expect(resp.StatusCode).NotTo(Equal(http.StatusNotFound)) + Expect(resp.StatusCode).To(Equal(http.StatusOK)) }, NodeTimeout(5*time.Second)) })