From 2ae3627c7a3006822efa7dd9748eb3cb37e0b1bd Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:13:54 -0400 Subject: [PATCH 1/3] feat: ServeMCP, a solution exposes its tools as the signed-in person MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A solution exposes its experience to an agent client the way it exposes it to a browser: as the viewer, through the host's gateway, with the same authority. The solution owns its tool surface; the runtime owns serving it, exactly as it owns serving HTTP. ServeMCP mounts the official SDK's stateless Streamable HTTP handler at /mcp — /solutions//mcp through the gateway, a route it already fronts — plus the RFC 9728 Protected Resource Metadata document a client discovers how to authenticate from. Sessions are stateless because the gateway is a reverse proxy with no sticky routing: a session held in one replica's memory is unreachable from the next request. No token verification lives here. The gateway runs ext_authz on the bearer and stamps the viewer's identity, so verifying it again would be a second, divergent implementation of the host's admission rules. What is enforced is that the gateway did it: a request with no bearer, or with a bearer but no stamped identity, is answered 401 with the challenge that names the metadata document, and a credential that authenticates without a session is refused 403 by its stamped kind — an organization api_key cannot mint a Work Context at all, so it is refused at the boundary rather than once per tool call, where it would read as a broken tool. Each tool call runs as the viewer: a receiving middleware binds the stamped identity to the request's context, and ViewerFromContext hands a tool handler the same caller-bound Gateway a Handler is given, so ForModule mints the viewer's own Work Context. The identity never comes from the tool's arguments, which are written by a model. The resource identifier a client binds its token to is the public MCP URL, which no resolver here holds: the origin is the host's and the path is the host's route layout, which this runtime deliberately does not encode. It is MCP_PUBLIC_URL when set — paired with the metadata document's own URL by the same suffix swap the registration token URLs use, and refused at boot when it cannot be paired — and otherwise reconstructed from what the proxy forwarded, with a boot log line saying so. The issuer is resolved by role like every other host endpoint and is never request-derived, so a crafted Host header cannot point a client at another authorization server. Co-Authored-By: Claude Opus 5 (1M context) --- go.mod | 8 + go.sum | 20 ++ mcp.go | 474 +++++++++++++++++++++++++++++++++++++++++++++++ mcp_test.go | 521 ++++++++++++++++++++++++++++++++++++++++++++++++++++ solution.go | 36 +++- 5 files changed, 1058 insertions(+), 1 deletion(-) create mode 100644 mcp.go create mode 100644 mcp_test.go diff --git a/go.mod b/go.mod index 59793e7..ecec0ba 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/codefly-dev/core v0.3.41 github.com/codefly-dev/sdk-go v0.1.65 github.com/google/uuid v1.6.0 + github.com/modelcontextprotocol/go-sdk v1.8.0 google.golang.org/genproto/googleapis/api v0.0.0-20260819154853-08b0e4226688 google.golang.org/protobuf v1.36.12 gopkg.in/yaml.v3 v3.0.1 @@ -36,19 +37,26 @@ require ( github.com/go-openapi/swag/typeutils v0.29.1 // indirect github.com/go-openapi/swag/yamlutils v0.29.1 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect + github.com/google/jsonschema-go v0.4.3 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/kr/text v0.2.0 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/oklog/ulid/v2 v2.1.2 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/segmentio/asm v1.1.3 // indirect + github.com/segmentio/encoding v0.5.4 // indirect github.com/yoheimuta/go-protoparser/v4 v4.14.2 // indirect + github.com/yosida95/uritemplate/v3 v3.0.2 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect golang.org/x/exp v0.0.0-20260820142414-ca536658362e // 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/term v0.46.0 // indirect golang.org/x/text v0.42.0 // indirect + golang.org/x/time v0.15.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260825221802-da73d73af1c5 // indirect google.golang.org/grpc v1.83.2 // indirect ) diff --git a/go.sum b/go.sum index 0caccac..f00909f 100644 --- a/go.sum +++ b/go.sum @@ -113,6 +113,8 @@ github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPE github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/gofrs/flock v0.13.1 h1:jjREztyBeSKBZYAC+mgc1laB+xsgy4kYMf3FbKF2UBo= github.com/gofrs/flock v0.13.1/go.mod h1:sf4BFiHwnvgxa25DlQoDqXQnwRMEOwqxRq37P6MzzmE= +github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= +github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/gnostic-models v0.6.9 h1:MU/8wDLif2qCXZmzncUQ/BOfxWfthHi63KqpoNbWqVw= @@ -123,6 +125,8 @@ github.com/google/go-github/v89 v89.0.0 h1:35bEK5XoEcF3PZrlVbl9XN63f5BcJRA/UGkxe github.com/google/go-github/v89 v89.0.0/go.mod h1:QLcbU0ipeAqQuR5KSg8c2lql4Qk1EwJ2dWz/0rP4Nho= github.com/google/go-querystring v1.2.0 h1:yhqkPbu2/OH+V9BfpCVPZkNmUXhb2gBxJArfhIxNtP0= github.com/google/go-querystring v1.2.0/go.mod h1:8IFJqpSRITyJ8QhQ13bmbeMBDfmeEJZD5A0egEOmkqU= +github.com/google/jsonschema-go v0.4.3 h1:/DBOLZTfDow7pe2GmaJNhltueGTtDKICi8V8p+DQPd0= +github.com/google/jsonschema-go v0.4.3/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= @@ -148,6 +152,8 @@ github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3N github.com/moby/docker-image-spec v1.3.1/go.mod h1:eKmb5VW8vQEh/BAr2yvVNvuiJuY6UIocYsFu/DxxRpo= github.com/moby/patternmatcher v0.6.1 h1:qlhtafmr6kgMIJjKJMDmMWq7WLkKIo23hsrpR3x084U= github.com/moby/patternmatcher v0.6.1/go.mod h1:hDPoyOpDY7OrrMDLaYoY3hf52gNCR/YOUYxkhApJIxc= +github.com/modelcontextprotocol/go-sdk v1.8.0 h1:KIvahhYqwtbeniWVPs3TcXEA7b8jEtwfBpOTAI+Urx4= +github.com/modelcontextprotocol/go-sdk v1.8.0/go.mod h1:dL7u98E/zjJTGzEq+j30jQ8K2k1mb6LeAH4inEcSGts= github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/2gBQ3RWajuToeY6ZtZTIKv2v7ThUy5KKusIT0yc0= github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4= github.com/oklog/ulid/v2 v2.1.2 h1:IEclFb9JNvzYA6MW2SCxbLzcHTVsfqm3PrqGQJH5zec= @@ -167,6 +173,10 @@ github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0t github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/santhosh-tekuri/jsonschema/v6 v6.0.3 h1:1EYB5IzjZawrrnELUi78f9fPu57HuXjmddZPjrls/28= github.com/santhosh-tekuri/jsonschema/v6 v6.0.3/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= +github.com/segmentio/asm v1.1.3 h1:WM03sfUOENvvKexOLp+pCqgb/WDjsi7EK8gIsICtzhc= +github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg= +github.com/segmentio/encoding v0.5.4 h1:OW1VRern8Nw6ITAtwSZ7Idrl3MXCFwXHPgqESYfvNt0= +github.com/segmentio/encoding v0.5.4/go.mod h1:HS1ZKa3kSN32ZHVZ7ZLPLXWvOVIiZtyJnO1gPH1sKt0= github.com/shirou/gopsutil/v3 v3.24.5 h1:i0t8kL+kQTvpAYToeuiVk3TgDeKOFioZO3Ztz/iZ9pI= github.com/shirou/gopsutil/v3 v3.24.5/go.mod h1:bsoOS1aStSs9ErQ1WWfxllSeS1K5D+U30r2NfcubMVk= github.com/shoenig/go-m1cpu v0.2.1 h1:yqRB4fvOge2+FyRXFkXqsyMoqPazv14Yyy+iyccT2E4= @@ -181,6 +191,8 @@ github.com/xlab/treeprint v1.2.0 h1:HzHnuAF1plUN2zGlAFHbSQP2qJ0ZAD3XF5XD7OesXRQ= github.com/xlab/treeprint v1.2.0/go.mod h1:gj5Gd3gPdKtR1ikdDK6fnFLdmIS0X30kTTuNd/WEJu0= github.com/yoheimuta/go-protoparser/v4 v4.14.2 h1:/P/LlX1CF9NaTWEltGcIZVvNlPbhABuAnBtAWpb3+74= github.com/yoheimuta/go-protoparser/v4 v4.14.2/go.mod h1:AHNNnSWnb0UoL4QgHPiOAg2BniQceFscPI5X/BZNHl8= +github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= +github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0= github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= @@ -207,12 +219,20 @@ golang.org/x/exp v0.0.0-20260820142414-ca536658362e h1:01Ju2A/fZKkci4zqx0eZxw//D golang.org/x/exp v0.0.0-20260820142414-ca536658362e/go.mod h1:zeBbvyFKDaLwa7CH/zI8KXt7gTl14SF7sO08Pl5jBCM= 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= golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= golang.org/x/term v0.46.0 h1:3+OXuTbaKDgwk8jTi3aSLHRlmWqHEUDUtxnbFigO4YE= golang.org/x/term v0.46.0/go.mod h1:+K02xbkittuwc0Am4abfA3Fc+XRGXkvBXNO88NCXPoc= golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI= golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E= +golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= +golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= +golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI= +golang.org/x/tools v0.49.0/go.mod h1:SJNXV9DBKT0UbdttsQjbfJlAE/q+y36++zo3uL3N0Oo= gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= google.golang.org/genproto/googleapis/api v0.0.0-20260819154853-08b0e4226688 h1:ax2KzoSRIZU/M0cIxri3pKxy99vniH1PVxWC6si/eZI= diff --git a/mcp.go b/mcp.go new file mode 100644 index 0000000..c499eef --- /dev/null +++ b/mcp.go @@ -0,0 +1,474 @@ +package solution + +import ( + "context" + "fmt" + "log" + "net/http" + "net/url" + "strings" + + "github.com/modelcontextprotocol/go-sdk/auth" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/modelcontextprotocol/go-sdk/oauthex" +) + +// --- MCP --- +// +// A solution exposes its experience to an agent client the way it exposes it to +// a browser: as the signed-in person, through the host's gateway, with the same +// authority. The solution owns its tool surface; this runtime owns serving it. +// +// Nothing MCP-specific lives in the host's gateway, and no token verification +// lives here: the gateway strips caller identity headers, authenticates the +// bearer and stamps the viewer's identity, exactly as it does for every other +// request this runtime serves. The MCP surface reads that stamped identity and +// refuses a request that arrives without it — it never inspects the bearer. + +const ( + // MCPPath is where the MCP endpoint serves, relative to the solution's own + // backend. Through the gateway that is /solutions//mcp, the route the + // gateway already fronts. + MCPPath = "/mcp" + + // ProtectedResourceMetadataPath is where the OAuth 2.0 Protected Resource + // Metadata document (RFC 9728) serves, at the well-known location a client + // discovers it at. It is the document the 401 challenge points to, so a + // client that cannot reach it cannot learn which authorization server to + // use: it is served without authentication, and the gateway must admit it + // unauthenticated for discovery to work at all. + ProtectedResourceMetadataPath = "/.well-known/oauth-protected-resource" +) + +// userHeader carries the viewer's user id, and credentialKindHeader the kind of +// credential the gateway authenticated. They are the other half of the identity +// the gateway stamps from verified claims (see orgHeader and sessionHeader, +// which the Work Context mint is built on) and replace anything the caller sent. +// +// Only the MCP surface reads them, for one refusal each: nothing stamped at all +// means the request did not come through the gateway, and a credential kind that +// authenticates without a session cannot act for a viewer at all. +const ( + userHeader = "x-user-id" + credentialKindHeader = "x-credential-kind" +) + +// mcpSurface is the MCP server a solution declared with ServeMCP. +type mcpSurface struct { + name, version string + register func(*mcp.Server) +} + +// validate rejects a declaration that cannot serve anything. Both mistakes are +// the author's and are the same in every environment, so they are refused +// before the environment is consulted, as a mis-declared surface is. +func (m *mcpSurface) validate() error { + if strings.TrimSpace(m.name) == "" { + return fmt.Errorf("ServeMCP was given no server name: it is what an MCP client displays for this server") + } + if strings.TrimSpace(m.version) == "" { + return fmt.Errorf("ServeMCP server %q was given no version: a client negotiates and caches against it", m.name) + } + if m.register == nil { + return fmt.Errorf("ServeMCP server %q was given no register function: the solution owns its tool surface, and a server with no tools answers tools/list with nothing", m.name) + } + return nil +} + +// ServeMCP exposes the solution as an MCP server over stateless Streamable HTTP +// at MCPPath, with the RFC 9728 Protected Resource Metadata document that lets a +// client discover how to authenticate. Chainable, like Handle. +// +// name and version identify the server to a client. register adds the tools, +// prompts and resources the solution offers, on the official SDK's own +// *mcp.Server — the solution's tool surface is its own, and this runtime adds +// nothing to it. +// +// Every tool call runs as the viewer who made the request: ViewerFromContext +// returns the same caller-bound Gateway a Handler is given, so a tool reads a +// composed module through ForModule exactly as the page does, under the viewer's +// own Work Context and no more authority than it asks for. +// +// Sessions are stateless because the gateway is a reverse proxy with no sticky +// routing: a session held in one replica's memory is unreachable from the next +// request, so the MCP session would break under any rollout or scale-out. Each +// request carries its own identity, which is all a tool call needs. +func (s *Server) ServeMCP(name, version string, register func(*mcp.Server)) *Server { + s.mcp = &mcpSurface{name: name, version: version, register: register} + return s +} + +// mountMCP serves the MCP endpoint and its metadata document on mux, or does +// nothing when the solution declared no MCP surface. +func (s *Server) mountMCP(mux *http.ServeMux) error { + if s.mcp == nil { + return nil + } + if err := s.mcp.validate(); err != nil { + return fmt.Errorf("solution %q: %w", s.manifest.ID, err) + } + srv := mcp.NewServer(&mcp.Implementation{Name: s.mcp.name, Version: s.mcp.version}, nil) + s.mcp.register(srv) + // Receiving middleware rather than the HTTP context, because this is the + // seam the SDK documents for carrying per-request state into a handler: it + // sees the request's own headers (RequestExtra.Header) and the context it + // passes on is the one the tool handler runs under. + srv.AddReceivingMiddleware(s.mcpViewer) + handler := mcp.NewStreamableHTTPHandler( + func(*http.Request) *mcp.Server { return srv }, + &mcp.StreamableHTTPOptions{Stateless: true}, + ) + // No CORS on the MCP endpoint: an MCP client is not a browser page, and the + // runtime's permissive policy (any origin, with the authorization header) + // would be handing a page the one route that acts with the viewer's full + // authority. The metadata document does carry CORS — discovery is public + // (RFC 9728 §3.1) — and the SDK's own handler sets it. + mux.Handle(MCPPath, s.requireStampedViewer(handler)) + mux.Handle(ProtectedResourceMetadataPath, http.HandlerFunc(s.handleProtectedResourceMetadata)) + if s.cfg.mcpPublicURL == "" { + // The one signal that this is happening. The resource identifier a + // client binds its token to must be the URL the client dialled, and + // without MCP_PUBLIC_URL the runtime can only reconstruct it from what + // the proxy forwards: a gateway that forwards no prefix yields an + // identifier missing the path it stripped, which a conforming client + // rejects and a tolerant one silently binds to the wrong resource. + log.Printf("solution %q: MCP resource identifier derived per request from forwarded headers (%s unset): set it to the URL clients dial (%q) if this host's proxy does not forward %s", + s.manifest.ID, mcpPublicURLEnvironmentVariable, "https:///solutions/"+s.manifest.ID+MCPPath, forwardedPrefixHeader) + } + return nil +} + +// MCPEnvironment is what Serve resolves from the composition for the MCP +// surface, supplied instead by a caller of MCPHandler. +type MCPEnvironment struct { + // GatewayURL is the host gateway a tool call's Work Context is minted + // through and its composed-module reads go to. + GatewayURL string + // IssuerURL is the host's OAuth issuer, published as the authorization + // server in the metadata document. + IssuerURL string + // PublicURL is the canonical public MCP URL, as Serve reads it from + // MCP_PUBLIC_URL. Empty derives it from each request's forwarded headers. + PublicURL string +} + +// MCPHandler is the MCP surface exactly as Serve serves it — the same endpoint, +// the same metadata document, the same refusals, mounted by the same code — +// against an environment the caller supplies instead of the one Serve resolves. +// Nothing else of the solution is served, and nothing registers anywhere. +// +// It exists for tests: a solution drives its own tools under the runtime's +// identity handling, with a fake host standing in for the gateway. A solution +// serves with Serve, which overwrites the environment set here when it resolves +// its own. +func (s *Server) MCPHandler(env MCPEnvironment) (http.Handler, error) { + if s.mcp == nil { + return nil, fmt.Errorf("solution %q declares no MCP server (ServeMCP)", s.manifest.ID) + } + s.cfg.gatewayURL, s.cfg.mcpIssuerURL, s.cfg.mcpPublicURL = env.GatewayURL, env.IssuerURL, env.PublicURL + s.cfg.mcp = true + if err := s.cfg.validateMCP(); err != nil { + return nil, fmt.Errorf("solution %q: %w", s.manifest.ID, err) + } + mux := http.NewServeMux() + if err := s.mountMCP(mux); err != nil { + return nil, err + } + return mux, nil +} + +// --- the viewer a tool call runs as --- + +type mcpViewerKey struct{} + +// ViewerFromContext returns the Gateway bound to the viewer whose MCP request +// this tool call belongs to: the same caller-bound Gateway a Handler is given, +// carrying the viewer's bearer and the organization and session the gateway +// stamped, so ForModule mints the viewer's own Work Context. +// +// It is the one line a tool handler writes: +// +// gw, err := solution.ViewerFromContext(ctx) +// if err != nil { +// return nil, out, err +// } +// docs, err := gw.ForModule(ctx, "documents", solution.Scope{...}) +// +// The error is for a handler invoked outside a request this runtime served — +// a tool called from a test harness of its own, or an *mcp.Server mounted +// without ServeMCP. It never happens on a served request: the endpoint refuses +// one without a stamped identity before any tool runs. +func ViewerFromContext(ctx context.Context) (*Gateway, error) { + gw, _ := ctx.Value(mcpViewerKey{}).(*Gateway) + if gw == nil { + return nil, fmt.Errorf("no viewer in this context: a tool handler receives one only on a request served through ServeMCP, which reads the identity the host gateway stamped") + } + return gw, nil +} + +// mcpViewer binds each incoming MCP request to the viewer it arrived as, so a +// tool handler reads composed modules on their behalf and not as the solution. +// +// The identity comes from the headers the gateway stamped from verified claims, +// never from the MCP payload: a tool's arguments are written by the model, and a +// session id taken from them would let a client ask for another viewer's +// authority. A request that reached here has already been refused if it carried +// no stamped identity (see requireStampedViewer). +func (s *Server) mcpViewer(next mcp.MethodHandler) mcp.MethodHandler { + return func(ctx context.Context, method string, req mcp.Request) (mcp.Result, error) { + header := http.Header{} + if extra := req.GetExtra(); extra != nil && extra.Header != nil { + header = extra.Header + } + id := stampedIdentity(header) + gw := newGateway(s.cfg.gatewayURL, id.bearer, id.orgID, id.sessionID) + return next(context.WithValue(ctx, mcpViewerKey{}, gw), method, req) + } +} + +// --- the identity the gateway stamps --- + +// mcpIdentity is one MCP request's caller, as the gateway presented it. +type mcpIdentity struct { + bearer, userID, orgID, sessionID, credentialKind string +} + +func stampedIdentity(h http.Header) mcpIdentity { + return mcpIdentity{ + bearer: h.Get("authorization"), + userID: h.Get(userHeader), + orgID: h.Get(orgHeader), + sessionID: h.Get(sessionHeader), + credentialKind: h.Get(credentialKindHeader), + } +} + +// stamped reports whether the gateway named this caller at all. Any one of the +// three is enough: the question this answers is whether the request came through +// the gateway, and which of them a given credential gets stamped with is the +// gateway's business — an org-less API key is stamped a principal and no org. +func (i mcpIdentity) stamped() bool { + return i.userID != "" || i.orgID != "" || i.sessionID != "" +} + +// apiKeyCredentialKind is the credential kind the gateway stamps for a caller +// that authenticated with an organization API key: a principal and no session. +const apiKeyCredentialKind = "api_key" + +// requireStampedViewer refuses an MCP request the gateway did not authenticate, +// and one whose credential cannot act for a viewer. +// +// This is the whole of the runtime's authentication for MCP: the gateway runs +// ext_authz on the bearer and stamps the result, so verifying the token again +// here would be a second, divergent implementation of the host's admission +// rules — with its own JWKS fetch, its own audience logic, and its own bugs. +// What is enforced here is that the gateway did it. +func (s *Server) requireStampedViewer(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + id := stampedIdentity(r.Header) + switch { + case id.bearer == "": + // The ordinary first contact of an MCP client, and the reason the + // challenge exists: it names the metadata document the client reads + // to find the authorization server to get a token from. + s.challenge(w, r, "no bearer token: this MCP server is a protected resource") + return + case !id.stamped(): + // A bearer arrived but no identity did, so either this request did + // not come through the host gateway — nothing else may reach this + // endpoint, and no header a caller sends is trusted here — or the + // gateway did not authenticate it. Answering the challenge rather + // than a bare 401 lets a client that went to the wrong address + // still discover the right authorization server. + s.challenge(w, r, fmt.Sprintf("no stamped identity: the host gateway authenticates the bearer and stamps %s, %s and %s, and none of them arrived", + userHeader, orgHeader, sessionHeader)) + return + case id.sessionID == "": + // Not fixable by getting another token, so not a challenge: this + // credential authenticates without a session, and every tool call + // that acts for the viewer mints a Work Context rooted in one + // (accounts requires a Task to name a session). Named by kind, + // because the kind is what the caller has to change. + kind := id.credentialKind + if kind == "" { + kind = "(unstamped)" + } + http.Error(w, fmt.Sprintf("MCP requires a user session: the gateway stamped %s=%q and %s empty, and a credential that authenticates without a session cannot act for a viewer — an organization %s cannot read composed modules. Connect as a signed-in person.", + credentialKindHeader, kind, sessionHeader, apiKeyCredentialKind), http.StatusForbidden) + return + } + next.ServeHTTP(w, r) + }) +} + +// challenge answers 401 with the RFC 9728 pointer to this resource's metadata +// document, which is how an MCP client learns which authorization server to +// authenticate against (RFC 9728 §5.1). The URL is absolute and names the +// resource as the client reached it, so a client following it arrives back here +// rather than at an address only the cluster can dial. +func (s *Server) challenge(w http.ResponseWriter, r *http.Request, message string) { + metadata := siblingURL(s.mcpResource(r), MCPPath, ProtectedResourceMetadataPath) + if metadata != "" { + w.Header().Add("WWW-Authenticate", fmt.Sprintf("Bearer resource_metadata=%q", metadata)) + } + http.Error(w, message, http.StatusUnauthorized) +} + +// handleProtectedResourceMetadata serves the RFC 9728 document for this MCP +// resource: the identifier a client binds its token to, and the authorization +// server it gets that token from. +// +// The document is built per request because the resource identifier can only be +// derived from the request when no canonical URL is configured. The +// authorization server never is: it is the host issuer resolved at boot, so a +// crafted Host header cannot point a client at an issuer of someone's choosing. +func (s *Server) handleProtectedResourceMetadata(w http.ResponseWriter, r *http.Request) { + auth.ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{ + Resource: s.mcpResource(r), + AuthorizationServers: []string{s.cfg.mcpIssuerURL}, + BearerMethodsSupported: []string{"header"}, + ResourceName: s.manifest.Title, + }).ServeHTTP(w, r) +} + +// mcpResource is this endpoint's resource identifier: the absolute URL an MCP +// client dialled to get here. +// +// It is what the client asks the authorization server for a token for (RFC 8707) +// and what the metadata document must name, so it has to be the public URL — not +// this process's listen address and not the in-cluster address the gateway dials. +// The runtime cannot resolve it: the origin is the host's and the path prefix is +// the host's route layout, which this runtime deliberately does not encode (see +// frontendManifestURL). So it is the configured MCP_PUBLIC_URL, and otherwise +// reconstructed from what the proxy in front of this solution forwarded. +func (s *Server) mcpResource(r *http.Request) string { + if s.cfg.mcpPublicURL != "" { + return s.cfg.mcpPublicURL + } + return forwardedOrigin(r) + forwardedPrefix(r) + MCPPath +} + +const ( + forwardedProtoHeader = "x-forwarded-proto" + forwardedHostHeader = "x-forwarded-host" + forwardedPrefixHeader = "x-forwarded-prefix" +) + +// forwardedOrigin is the scheme and host the client used, as the proxy in front +// of this solution reported them, falling back to the request's own. These +// headers are only worth reading because the one route to this endpoint is the +// host gateway; what a caller could do by crafting them is misname the resource +// identifier in its own 401 and its own metadata document, never redirect +// anyone to an authorization server of its choosing. +func forwardedOrigin(r *http.Request) string { + scheme := firstForwarded(r, forwardedProtoHeader) + if scheme == "" { + scheme = "http" + if r.TLS != nil { + scheme = "https" + } + } + host := firstForwarded(r, forwardedHostHeader) + if host == "" { + host = r.Host + } + return scheme + "://" + host +} + +// forwardedPrefix is the path the proxy stripped before this solution saw the +// request — "/solutions/" at the host gateway. Empty when nothing forwarded +// one, which is the case MCP_PUBLIC_URL exists for: the alternative is to +// reconstruct the host's route layout here, and this runtime does not know it. +func forwardedPrefix(r *http.Request) string { + prefix := strings.TrimRight(firstForwarded(r, forwardedPrefixHeader), "/") + // A prefix that is not a plain absolute path is not a prefix: "//evil.test" + // is a protocol-relative URL, and anything with a query, a fragment or a + // space does not belong in a URL path. None of them can be repaired, so the + // identifier is built without the prefix and is refused by the client for + // not matching what it dialled — loud, where a sanitized guess would be a + // resource identifier that looks right and names something else. + if !pathPrefix(prefix) { + return "" + } + return prefix +} + +// pathPrefix reports whether raw is a plain absolute URL path: one that can be +// concatenated in front of MCPPath and still address this resource. +func pathPrefix(raw string) bool { + if !strings.HasPrefix(raw, "/") || strings.HasPrefix(raw, "//") { + return false + } + for i := 0; i < len(raw); i++ { + switch c := raw[i]; { + case c <= ' ', c >= 0x7f, c == '?', c == '#', c == '\\': + return false + } + } + return true +} + +// firstForwarded is the first value of a forwarded header, which a chain of +// proxies appends to: the client-most hop, comma-separated or repeated. +func firstForwarded(r *http.Request, name string) string { + raw := r.Header.Get(name) + if comma := strings.IndexByte(raw, ','); comma >= 0 { + raw = raw[:comma] + } + return strings.TrimSpace(raw) +} + +// --- boot configuration --- + +const ( + // hostIssuerURLEnvironmentVariable overrides the host's OAuth issuer, which + // is otherwise the resolved host frontend origin. A deployment sets it + // because the resolved address is the one this composition dials the host + // at — in-cluster, reachable by no MCP client. + hostIssuerURLEnvironmentVariable = "HOST_ISSUER_URL" + + // mcpPublicURLEnvironmentVariable sets the canonical public MCP URL: the + // URL clients dial, which is the resource identifier their tokens are bound + // to. It must end in MCPPath, because the metadata document's own URL is + // derived from it by swapping that suffix — the same pairing the + // registration token URLs use, and refused at boot for the same reason when + // it cannot be made. + mcpPublicURLEnvironmentVariable = "MCP_PUBLIC_URL" +) + +// validateMCP refuses a configuration whose MCP surface would serve a metadata +// document no client can act on. It runs only when the solution declared one: +// a solution that serves no MCP has never needed the host's issuer, and must +// not start failing to boot because this runtime now resolves one. +// +// Both refusals are silent damage otherwise. An empty issuer publishes +// `authorization_servers: [""]`, which a client reads as "there is nowhere to +// authenticate" — on a document that is otherwise well-formed, served with a +// 200, by a solution that looks healthy. An unpairable public URL leaves the +// 401 challenge with no metadata URL at all, so a client that could have +// discovered the authorization server simply does not. +func (c config) validateMCP() error { + if !c.mcp { + return nil + } + if u, err := url.Parse(c.mcpIssuerURL); err != nil || !u.IsAbs() || u.Host == "" { + if c.environmentLoadErr != nil { + return fmt.Errorf("unresolved host issuer %q, and loading Codefly's injected environment failed first: %w — nothing the SDK resolves can be trusted to be absent until that is fixed", + c.mcpIssuerURL, c.environmentLoadErr) + } + return fmt.Errorf("unresolved host issuer %q: ServeMCP publishes it as the authorization server of this MCP resource, so a client has nowhere to authenticate without it. Set %s to the origin MCP clients authenticate against, or ensure the SDK resolves the host frontend's http endpoint", + c.mcpIssuerURL, hostIssuerURLEnvironmentVariable) + } + if c.mcpPublicURL == "" { + return nil + } + if u, err := url.Parse(c.mcpPublicURL); err != nil || !u.IsAbs() || u.Host == "" { + return fmt.Errorf("unusable %s %q: it is the resource identifier an MCP client binds its token to, so it must be the absolute URL clients dial", + mcpPublicURLEnvironmentVariable, c.mcpPublicURL) + } + if siblingURL(c.mcpPublicURL, MCPPath, ProtectedResourceMetadataPath) == "" { + return fmt.Errorf("unpairable %s %q: it must end in %s, because the metadata document a 401 points a client to is derived from it by swapping that suffix", + mcpPublicURLEnvironmentVariable, c.mcpPublicURL, MCPPath) + } + return nil +} diff --git a/mcp_test.go b/mcp_test.go new file mode 100644 index 0000000..1aeaa05 --- /dev/null +++ b/mcp_test.go @@ -0,0 +1,521 @@ +package solution + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// --- the host gateway, as an MCP request arrives from it --- + +// stampedTransport is the host gateway's half of an MCP request: it authenticates +// the client's bearer and replaces the identity headers with what it resolved. +// Everything the runtime's MCP surface knows about the caller comes from here. +type stampedTransport struct { + bearer, userID, orgID, sessionID, credentialKind string + // prefix is the path the gateway strips before the solution sees the + // request, forwarded as x-forwarded-prefix. + prefix string +} + +func (s stampedTransport) RoundTrip(r *http.Request) (*http.Response, error) { + r.Header.Del(userHeader) + r.Header.Del(orgHeader) + r.Header.Del(sessionHeader) + r.Header.Del(credentialKindHeader) + if s.bearer != "" { + r.Header.Set("authorization", s.bearer) + } + for name, value := range map[string]string{ + userHeader: s.userID, + orgHeader: s.orgID, + sessionHeader: s.sessionID, + credentialKindHeader: s.credentialKind, + forwardedPrefixHeader: s.prefix, + } { + if value != "" { + r.Header.Set(name, value) + } + } + return http.DefaultTransport.RoundTrip(r) +} + +// viewerStamp is the identity the gateway stamps for a signed-in person: the +// same org and session every other test's viewer acts under. +func viewerStamp() stampedTransport { + return stampedTransport{ + bearer: "Bearer viewer-token", + userID: "viewer-principal", + orgID: viewerOrg, + sessionID: viewerSession, + credentialKind: "user_session", + } +} + +const ( + testIssuer = "https://host.test" + mcpToolName = "read_collection" + mcpServerName = "wiki" +) + +type askInput struct { + Question string `json:"question"` +} + +type askOutput struct { + Status int `json:"status"` +} + +// serveMCP mounts a solution's MCP surface the way Serve does, against a fake +// host. register adds the tools the test drives. +func serveMCP(t *testing.T, env MCPEnvironment, register func(*mcp.Server)) *httptest.Server { + t.Helper() + s := New(Manifest{ID: mcpServerName, Title: "Wiki"}).ServeMCP(mcpServerName, "v1.2.3", register) + handler, err := s.MCPHandler(env) + if err != nil { + t.Fatalf("mount MCP: %v", err) + } + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + return server +} + +// readCollectionTool is the tool a solution writes: it asks the runtime who the +// viewer is and reads a composed module on their behalf. +func readCollectionTool(srv *mcp.Server) { + mcp.AddTool(srv, &mcp.Tool{Name: mcpToolName, Description: "read the collection"}, + func(ctx context.Context, _ *mcp.CallToolRequest, _ askInput) (*mcp.CallToolResult, askOutput, error) { + gw, err := ViewerFromContext(ctx) + if err != nil { + return nil, askOutput{}, err + } + status, err := readModule(ctx, gw, "documents") + return nil, askOutput{Status: status}, err + }) +} + +// connectMCP drives the endpoint with a real MCP client through the gateway +// double, as Claude Code would. +func connectMCP(t *testing.T, server *httptest.Server, gateway stampedTransport) *mcp.ClientSession { + t.Helper() + client := mcp.NewClient(&mcp.Implementation{Name: "agent", Version: "v0"}, nil) + session, err := client.Connect(context.Background(), &mcp.StreamableClientTransport{ + Endpoint: server.URL + MCPPath, + HTTPClient: &http.Client{Transport: gateway}, + }, nil) + if err != nil { + t.Fatalf("connect to %s: %v", server.URL+MCPPath, err) + } + t.Cleanup(func() { _ = session.Close() }) + return session +} + +// TestMCPToolCallRunsAsTheViewer is the whole point of the surface: a tool call +// arriving from an agent client mints the Work Context of the person who signed +// in, so the module read it makes is the one the browser would have made. The +// module refuses a bearer alone, so a 200 is only possible because the viewer's +// own capability was minted and presented. +func TestMCPToolCallRunsAsTheViewer(t *testing.T) { + host := newWorkContextGateway(t, &workContextGateway{}) + server := serveMCP(t, MCPEnvironment{GatewayURL: host.URL, IssuerURL: testIssuer}, readCollectionTool) + session := connectMCP(t, server, viewerStamp()) + + tools, err := session.ListTools(context.Background(), nil) + if err != nil { + t.Fatalf("tools/list: %v", err) + } + if len(tools.Tools) != 1 || tools.Tools[0].Name != mcpToolName { + t.Fatalf("tools/list = %+v, want the solution's own tool %q", tools.Tools, mcpToolName) + } + + result, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: mcpToolName, + Arguments: map[string]any{"question": "what is the handbook"}, + }) + if err != nil { + t.Fatalf("tools/call: %v", err) + } + if result.IsError { + t.Fatalf("tools/call failed: %v", toolErrorText(result)) + } + var out askOutput + if err := remarshal(result.StructuredContent, &out); err != nil { + t.Fatalf("decode tool output %v: %v", result.StructuredContent, err) + } + if out.Status != http.StatusOK { + t.Fatalf("module answered %d, want 200 — the read was not authenticated as the viewer", out.Status) + } + + mint := <-host.mints + if mint.Bearer != "Bearer viewer-token" { + t.Errorf("mint presented bearer %q, want the viewer's — accounts must resolve the viewer as owner", mint.Bearer) + } + if mint.OrgID != viewerOrg { + t.Errorf("mint org = %q, want the org the gateway stamped", mint.OrgID) + } + if mint.SessionID != viewerSession { + t.Errorf("mint session = %q, want the viewer's %q: a tool call is rooted in the session the gateway stamped, never one the runtime invents", mint.SessionID, viewerSession) + } + if call := <-host.calls; call.WorkContext == "" { + t.Errorf("module read carried no work context: the tool acted as the solution, not as the viewer") + } +} + +// remarshal reads a tool's structured output into the type the tool returned. +func remarshal(from any, into any) error { + raw, err := json.Marshal(from) + if err != nil { + return err + } + return json.Unmarshal(raw, into) +} + +// toolErrorText is whatever a failed tool call said, for a test failure message. +func toolErrorText(result *mcp.CallToolResult) string { + var parts []string + for _, content := range result.Content { + if text, ok := content.(*mcp.TextContent); ok { + parts = append(parts, text.Text) + } + } + return strings.Join(parts, " ") +} + +// post issues one raw MCP POST, which is how the refusals are observed: the +// client library cannot complete a handshake the endpoint refuses. +func post(t *testing.T, server *httptest.Server, header http.Header) *http.Response { + t.Helper() + body := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"agent","version":"v0"}}}` + req, err := http.NewRequest(http.MethodPost, server.URL+MCPPath, strings.NewReader(body)) + if err != nil { + t.Fatalf("new request: %v", err) + } + req.Header.Set("content-type", "application/json") + req.Header.Set("accept", "application/json, text/event-stream") + for name, values := range header { + req.Header[name] = values + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("POST %s: %v", MCPPath, err) + } + t.Cleanup(func() { drainAndClose(resp) }) + return resp +} + +// TestMCPChallengesARequestWithNoBearer pins the discovery handshake: an MCP +// client's first contact is unauthenticated, and the 401 is what tells it where +// to find the authorization server. Without the challenge the client has nothing +// to go on but the status code. +func TestMCPChallengesARequestWithNoBearer(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) + + resp := post(t, server, http.Header{"x-forwarded-proto": {"https"}, "x-forwarded-host": {"host.test"}, "x-forwarded-prefix": {"/solutions/wiki"}}) + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("answered %d, want 401", resp.StatusCode) + } + challenge := resp.Header.Get("WWW-Authenticate") + want := `Bearer resource_metadata="https://host.test/solutions/wiki` + ProtectedResourceMetadataPath + `"` + if challenge != want { + t.Errorf("challenge = %q, want %q", challenge, want) + } +} + +// TestMCPChallengesARequestTheGatewayDidNotStamp covers the request that carries +// a bearer the runtime never looks at. Nothing but the gateway may reach this +// endpoint, so a bearer with no stamped identity is not a caller this runtime can +// serve — and it is answered with the challenge rather than a bare 401, because a +// client that dialled the wrong address can still discover the right issuer from +// it. +func TestMCPChallengesARequestTheGatewayDidNotStamp(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer, PublicURL: "https://host.test/solutions/wiki" + MCPPath}, readCollectionTool) + + resp := post(t, server, http.Header{"authorization": {"Bearer whatever"}}) + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("answered %d, want 401", resp.StatusCode) + } + if got := resp.Header.Get("WWW-Authenticate"); !strings.Contains(got, ProtectedResourceMetadataPath) { + t.Errorf("challenge = %q, want the metadata document", got) + } + body := readBody(t, resp) + for _, header := range []string{userHeader, orgHeader, sessionHeader} { + if !strings.Contains(body, header) { + t.Errorf("refusal %q does not name %s: a refusal must name what is missing", body, header) + } + } +} + +// TestMCPRefusesACredentialWithNoSession pins the one credential kind MCP cannot +// serve, named by its kind. An organization API key authenticates a principal and +// no session, and every tool call that acts for a viewer mints a Work Context +// rooted in a session — so this is refused at the boundary rather than once per +// tool call, where the same refusal would read as the tool being broken. +func TestMCPRefusesACredentialWithNoSession(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) + + resp := post(t, server, http.Header{ + "authorization": {"Bearer org-api-key"}, + userHeader: {"service-principal"}, + credentialKindHeader: {apiKeyCredentialKind}, + }) + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("answered %d, want 403: another token cannot fix the kind of credential this is", resp.StatusCode) + } + if resp.Header.Get("WWW-Authenticate") != "" { + t.Errorf("challenge = %q, want none: re-authenticating with the same kind of credential would be refused again", resp.Header.Get("WWW-Authenticate")) + } + body := readBody(t, resp) + if !strings.Contains(body, apiKeyCredentialKind) || !strings.Contains(body, credentialKindHeader) { + t.Errorf("refusal %q names neither %s nor %q", body, credentialKindHeader, apiKeyCredentialKind) + } +} + +func readBody(t *testing.T, resp *http.Response) string { + t.Helper() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + return string(body) +} + +// --- RFC 9728 Protected Resource Metadata --- + +func metadata(t *testing.T, server *httptest.Server, header http.Header) map[string]any { + t.Helper() + req, err := http.NewRequest(http.MethodGet, server.URL+ProtectedResourceMetadataPath, nil) + if err != nil { + t.Fatalf("new request: %v", err) + } + for name, values := range header { + req.Header[name] = values + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("GET %s: %v", ProtectedResourceMetadataPath, err) + } + defer drainAndClose(resp) + if resp.StatusCode != http.StatusOK { + t.Fatalf("metadata answered %d, want 200: a client that cannot read it cannot authenticate", resp.StatusCode) + } + if origin := resp.Header.Get("Access-Control-Allow-Origin"); origin != "*" { + t.Errorf("metadata CORS origin = %q, want *: discovery is public (RFC 9728 §3.1)", origin) + } + var document map[string]any + if err := json.NewDecoder(resp.Body).Decode(&document); err != nil { + t.Fatalf("decode metadata: %v", err) + } + return document +} + +// TestMCPMetadataNamesTheResolvedIssuer pins both halves of the document a client +// acts on: the resource it binds its token to, and the authorization server it +// gets that token from. The resource is reconstructed from what the proxy +// forwarded — the URL the client actually dialled — while the issuer is the one +// resolved at boot and never anything the request carried. +func TestMCPMetadataNamesTheResolvedIssuer(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) + + document := metadata(t, server, http.Header{ + "x-forwarded-proto": {"https"}, + "x-forwarded-host": {"host.test"}, + "x-forwarded-prefix": {"/solutions/wiki"}, + }) + if got, want := document["resource"], "https://host.test/solutions/wiki"+MCPPath; got != want { + t.Errorf("resource = %v, want %q — the identifier a client binds its token to must be the URL it dialled", got, want) + } + if got, want := document["authorization_servers"], []any{testIssuer}; len(document["authorization_servers"].([]any)) != 1 || got.([]any)[0] != want[0] { + t.Errorf("authorization_servers = %v, want %v", got, want) + } + if got := document["bearer_methods_supported"]; len(got.([]any)) != 1 || got.([]any)[0] != "header" { + t.Errorf("bearer_methods_supported = %v, want [header]", got) + } + if got := document["resource_name"]; got != "Wiki" { + t.Errorf("resource_name = %v, want the solution's title", got) + } +} + +// TestMCPMetadataIssuerIsNotRequestDerived is the security property of the +// document: `resource` is reconstructed from headers a caller could craft, and +// `authorization_servers` — the field that decides where a client sends its +// credentials — is not derivable from a request at all. +func TestMCPMetadataIssuerIsNotRequestDerived(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) + + document := metadata(t, server, http.Header{ + "x-forwarded-proto": {"https"}, + "x-forwarded-host": {"evil.test"}, + }) + if got := document["authorization_servers"].([]any)[0]; got != testIssuer { + t.Errorf("authorization_servers = %v, want the resolved issuer %q: a crafted host must not point a client at another issuer", got, testIssuer) + } +} + +// TestMCPMetadataPrefersTheConfiguredPublicURL: a deployment whose proxy forwards +// no prefix has no way for the runtime to reconstruct the path it stripped, so +// the configured URL is what the document and the challenge name — whatever the +// request carried. +func TestMCPMetadataPrefersTheConfiguredPublicURL(t *testing.T) { + const public = "https://app.example.com/solutions/wiki" + MCPPath + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer, PublicURL: public}, readCollectionTool) + + document := metadata(t, server, http.Header{"x-forwarded-host": {"evil.test"}, "x-forwarded-prefix": {"/elsewhere"}}) + if got := document["resource"]; got != public { + t.Errorf("resource = %v, want the configured %q", got, public) + } +} + +// TestMCPMetadataDropsAnUnusablePrefix: a forwarded prefix that is not a plain +// absolute path cannot be concatenated in front of the MCP path. The identifier +// is built without it — which a client rejects for not matching what it +// dialled — rather than sanitized into one that looks right and names something +// else. +func TestMCPMetadataDropsAnUnusablePrefix(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) + + for _, prefix := range []string{"//evil.test", "/solutions/wiki?x=1", "/solutions/wiki#f", "solutions/wiki"} { + document := metadata(t, server, http.Header{ + "x-forwarded-proto": {"https"}, + "x-forwarded-host": {"host.test"}, + "x-forwarded-prefix": {prefix}, + }) + if got, want := document["resource"], "https://host.test"+MCPPath; got != want { + t.Errorf("with forwarded prefix %q, resource = %v, want %q", prefix, got, want) + } + } +} + +// --- transport shape --- + +// TestMCPIsStateless pins the transport the gateway can actually proxy: a +// session held in one replica's memory is unreachable from the next request +// through a reverse proxy with no sticky routing, so no session is issued and +// the methods that only exist to serve one are not offered. +func TestMCPIsStateless(t *testing.T) { + host := newWorkContextGateway(t, &workContextGateway{}) + server := serveMCP(t, MCPEnvironment{GatewayURL: host.URL, IssuerURL: testIssuer}, readCollectionTool) + + stamp := viewerStamp() + resp := post(t, server, http.Header{ + "authorization": {stamp.bearer}, + userHeader: {stamp.userID}, + orgHeader: {stamp.orgID}, + sessionHeader: {stamp.sessionID}, + credentialKindHeader: {stamp.credentialKind}, + }) + if resp.StatusCode != http.StatusOK { + t.Fatalf("initialize answered %d (%s), want 200", resp.StatusCode, readBody(t, resp)) + } + if id := resp.Header.Get("Mcp-Session-Id"); id != "" { + t.Errorf("initialize issued session %q: a stateless server must not, or the next request is routed to a replica that never saw it", id) + } + + req, err := http.NewRequest(http.MethodGet, server.URL+MCPPath, nil) + if err != nil { + t.Fatalf("new request: %v", err) + } + req.Header.Set("authorization", stamp.bearer) + req.Header.Set(sessionHeader, stamp.sessionID) + stream, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("GET %s: %v", MCPPath, err) + } + defer drainAndClose(stream) + if stream.StatusCode != http.StatusMethodNotAllowed { + t.Errorf("GET answered %d, want 405: there is no server-initiated stream without a session", stream.StatusCode) + } +} + +// TestMCPEndpointServesNoCORS: the one route that acts with the viewer's full +// authority is not offered to a browser page. The runtime's CORS policy admits +// any origin with an authorization header, which is the right answer for the +// solution's own API and the wrong one here; an MCP client is not a page. +func TestMCPEndpointServesNoCORS(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) + + resp := post(t, server, http.Header{"origin": {"https://evil.test"}}) + if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("MCP endpoint answered CORS origin %q, want none", got) + } +} + +// --- declaration and configuration refusals --- + +// TestServeMCPRefusesAMisDeclaredSurface: each of these is the author's mistake +// and is the same in every environment, so it is refused before any address is +// resolved — as a mis-declared surface is. +func TestServeMCPRefusesAMisDeclaredSurface(t *testing.T) { + for name, surface := range map[string]*mcpSurface{ + "no name": {version: "v1", register: readCollectionTool}, + "no version": {name: "wiki", register: readCollectionTool}, + "no register": {name: "wiki", version: "v1"}, + } { + if err := surface.validate(); err == nil { + t.Errorf("%s: declaration accepted, want a refusal", name) + } + } +} + +// TestMCPHandlerRefusesWithoutAServeMCPDeclaration keeps the test seam honest: +// it serves the surface Serve serves, and there is none to serve here. +func TestMCPHandlerRefusesWithoutAServeMCPDeclaration(t *testing.T) { + if _, err := New(Manifest{ID: "wiki"}).MCPHandler(MCPEnvironment{IssuerURL: testIssuer}); err == nil { + t.Error("MCPHandler accepted a solution that declares no MCP server") + } +} + +// TestValidateRefusesAnUnresolvedIssuerOnlyWhenMCPIsServed: an empty issuer +// publishes `authorization_servers: [""]` on a 200, from a solution that looks +// healthy — so it is refused at boot. A solution serving no MCP has never needed +// the host's issuer and must not start failing to boot because this runtime now +// resolves one. +func TestValidateRefusesAnUnresolvedIssuerOnlyWhenMCPIsServed(t *testing.T) { + cfg := config{mcp: true} + err := cfg.validateMCP() + if err == nil { + t.Fatal("an unresolved issuer was accepted while serving MCP") + } + if !strings.Contains(err.Error(), hostIssuerURLEnvironmentVariable) { + t.Errorf("refusal %q does not name %s, the variable that fixes it", err, hostIssuerURLEnvironmentVariable) + } + if err := (config{}).validateMCP(); err != nil { + t.Errorf("a solution serving no MCP was refused: %v", err) + } +} + +// TestValidateRefusesAnUnpairablePublicURL: the metadata document's URL is +// derived from the public MCP URL by swapping the path suffix, exactly as the +// registration token URLs are derived from their register URLs. An override that +// drops the suffix cannot be paired, so it is refused at boot naming the suffix, +// rather than leaving every 401 challenge without the one URL a client needs. +func TestValidateRefusesAnUnpairablePublicURL(t *testing.T) { + for _, public := range []string{"https://host.test/solutions/wiki", "https://host.test/mcp/", "/solutions/wiki/mcp"} { + cfg := config{mcp: true, mcpIssuerURL: testIssuer, mcpPublicURL: public} + if err := cfg.validateMCP(); err == nil { + t.Errorf("%s %q was accepted, want a refusal", mcpPublicURLEnvironmentVariable, public) + } + } + cfg := config{mcp: true, mcpIssuerURL: testIssuer, mcpPublicURL: "https://host.test/solutions/wiki" + MCPPath} + if err := cfg.validateMCP(); err != nil { + t.Errorf("a pairable public URL was refused: %v", err) + } +} + +// TestViewerFromContextOutsideAServedRequest: a tool handler invoked by a +// harness of its own gets a refusal that says where the viewer comes from, +// rather than a nil gateway that fails later inside a module read. +func TestViewerFromContextOutsideAServedRequest(t *testing.T) { + gw, err := ViewerFromContext(context.Background()) + if gw != nil || err == nil { + t.Fatalf("ViewerFromContext = (%v, %v), want a refusal", gw, err) + } + if !strings.Contains(err.Error(), "ServeMCP") { + t.Errorf("refusal %q does not name ServeMCP", err) + } +} diff --git a/solution.go b/solution.go index 5e87da1..584489d 100644 --- a/solution.go +++ b/solution.go @@ -222,7 +222,10 @@ type Server struct { // resolved routes, checked by Serve before it listens. consumed []ConsumedModule passthrough map[string]passthroughRoute - cfg config + // mcp is the MCP server the solution declared with ServeMCP, nil when it + // declared none. + mcp *mcpSurface + cfg config // registrationInterval is how long a registration heartbeat waits between // beats. Zero means defaultRegistrationInterval. Per-server rather than a // package value so a test can drive several beats without every other @@ -266,6 +269,17 @@ type config struct { // apiConsumes is the api.consumes projection Codefly injected, which the // passthrough declaration is checked against before the boot listens. apiConsumes string + // mcp says the solution declared an MCP surface (ServeMCP), which is what + // makes the two values below load-bearing: validate() checks them only + // then, so a solution that serves no MCP is unaffected by a host whose + // issuer does not resolve. + mcp bool + // mcpIssuerURL is the host's OAuth issuer, published as the authorization + // server of the MCP resource (RFC 9728). + mcpIssuerURL string + // mcpPublicURL is the canonical public MCP URL an operator set, empty when + // none was (see mcpResource). + mcpPublicURL string } func env(key, fallback string) string { @@ -473,6 +487,14 @@ func loadConfig(ctx context.Context, id string) config { moduleSecrets: parseModuleRegistrationSecrets(env(ModuleRegistrationSecretsEnvironmentVariable, "")), runtimeContext: strings.TrimSpace(env(resources.RuntimeContextPrefix, "")), apiConsumes: env(manifest.APIConsumesEnvironmentVariable, ""), + // The host is the authorization server an MCP client authenticates + // against, so its issuer is the host's own origin — resolved by role + // like every other host endpoint, never typed. The resolved address is + // the one this composition reaches the host at, which is right for a + // local run and is an in-cluster address in a deployment, where no + // public client could reach it: that is what the override is for. + mcpIssuerURL: strings.TrimRight(env(hostIssuerURLEnvironmentVariable, frontendURL), "/"), + mcpPublicURL: strings.TrimRight(env(mcpPublicURLEnvironmentVariable, ""), "/"), } cfg.registrationInterval, cfg.registrationIntervalErr = registrationIntervalFromEnv() return cfg @@ -695,6 +717,9 @@ func (c config) validate() error { c.publicURL, c.runtimeContext) } } + if err := c.validateMCP(); err != nil { + return err + } if c.registrationIntervalErr != nil { return c.registrationIntervalErr } @@ -751,6 +776,11 @@ func (s *Server) Serve() error { if err := s.manifest.validateSurfaces(); err != nil { return fmt.Errorf("solution %q: %w", s.manifest.ID, err) } + if s.mcp != nil { + if err := s.mcp.validate(); err != nil { + return fmt.Errorf("solution %q: %w", s.manifest.ID, err) + } + } // The SDK owns environment resolution: load Codefly's injected carriers so // endpoint and workspace-secret lookups resolve from them (falling back to // the local native workspace map when not running under the runtime). @@ -763,6 +793,7 @@ func (s *Server) Serve() error { } s.cfg = loadConfig(ctx, s.manifest.ID) s.cfg.environmentLoadErr = environmentLoadErr + s.cfg.mcp = s.mcp != nil if err := s.cfg.validate(); err != nil { return fmt.Errorf("solution %q: %w", s.manifest.ID, err) } @@ -796,6 +827,9 @@ func (s *Server) serve(ctx context.Context, ln net.Listener) error { if err := s.mountPassthrough(mux); err != nil { return err } + if err := s.mountMCP(mux); err != nil { + return err + } manifestBody, _ := json.Marshal(s.manifestMap()) upstreamBody, _ := json.Marshal(map[string]string{"id": s.manifest.ID, "upstream": s.cfg.selfUpstream}) From d68bc8b0fac890473157b5a181230d080b8aa3cf Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:24:25 -0400 Subject: [PATCH 2/3] docs: the MCP surface in the README, and the trust boundary in AGENTS.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three lines a solution writes, the one line a tool handler writes to get the viewer, every refusal with the reason it is that status, and the two knobs an operator has — plus what the runtime cannot answer and the host must: admitting the unauthenticated GET discovery needs, and the RFC 9728 location derived from the resource path, which sits at the host's root and not on the solution's. AGENTS.md gains the boundary rule, because it is the one an agent would otherwise undo by being helpful: the MCP surface verifies no token and must not start, the no-session refusal belongs at the boundary and not inside a tool, and no identity ever comes from a tool's arguments, which a model writes. Also guards a forwarded host that is not a host, which would otherwise be spliced into the middle of the resource identifier. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 22 ++++++-- README.md | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++- mcp.go | 21 ++++++- mcp_test.go | 16 ++++++ solution.go | 13 +++-- 5 files changed, 214 insertions(+), 14 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f16b86c..7c7a8cc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,8 +5,9 @@ deployed independently of the host they extend, with no build-time coupling. It owns what every solution needs identically: configuration resolution through the codefly SDK, self-registration with the host frontend and the gateway (the credential exchanges and their heartbeats), CORS, Module Federation asset -serving, the capability handshake, the manifest, and the gateway client a -handler uses to read composed modules on the viewer's behalf. +serving, the capability handshake, the manifest, serving the solution's MCP +server to agent clients, and the gateway client a handler uses to read composed +modules on the viewer's behalf. It owns none of the counterparts it talks to. The registration surfaces and their admission rules belong to the host (`codefly-dev/module-saas-starter`) and @@ -48,6 +49,13 @@ own: it builds on `Server.PassthroughHandler`, the handler `Serve` mounts. - A registration that cannot attest which publisher it speaks for is not a registration. There is no fallback to the shared cluster-internal token and no downgrade path when an exchange answers `404`. +- The MCP surface verifies no token, and must not start: the gateway runs + ext_authz and stamps the identity, so a check here would be a second, + divergent copy of the host's admission rules. What `requireStampedViewer` + enforces is that the gateway did it — and the refusal for a credential with no + session is at that boundary, not inside a tool, because there it reads as a + broken tool. A tool's arguments are written by a model: no identity ever comes + from them. - Tests live beside the code in the same package. Every behaviour change brings one, and the counterpart it exercises is an `httptest` server, never a real host. @@ -146,10 +154,12 @@ Keep this file short; add depth to the file that owns the subject. - [README.md](README.md) — the consumer-facing contract: handler errors, the full configuration and env-override table, self-registration and its - provisioning, consumed-module federation, and reading a - Work-Context-authenticated module. -- The package doc comment and the comments in [`solution.go`](solution.go) — - why a refusal, a backoff cap or a cache lifetime is what it is. Several + provisioning, consumed-module federation, reading a Work-Context-authenticated + module, and the MCP surface (what is served, every refusal, and why the + resource identifier cannot be resolved here). +- The package doc comment and the comments in [`solution.go`](solution.go) and + [`mcp.go`](mcp.go) — why a refusal, a backoff cap or a cache lifetime is what + it is. Several record a failure mode that is not obvious from the code; read the one next to what you are changing before you change it. - `.claude/skills/` — procedures an agent repeats, loaded only when relevant. diff --git a/README.md b/README.md index a87e5d6..cb33f58 100644 --- a/README.md +++ b/README.md @@ -3,8 +3,9 @@ Generic Go runtime for **codefly solutions** — independently deployed modules that plug into a host at runtime with no build-time coupling. Owns registration (host + gateway, with heartbeat), CORS, Module Federation asset serving, the -capability handshake, the manifest, and the gateway client each handler uses -to read composed modules on the viewer's behalf. +capability handshake, the manifest, serving the solution's MCP server to agent +clients, and the gateway client each handler uses to read composed modules on +the viewer's behalf. A solution author writes a manifest and one handler: @@ -62,6 +63,8 @@ the SDK-resolved value is the default. | Self upstream | the reachable self endpoint Codefly injects as `CODEFLY__SELF_ENDPOINT______HTTP__HTTP` (core ≥ v0.5.6); without it, the listen address `http://localhost:` | `SELF_UPSTREAM` | | Deployed or local | `CODEFLY__RUNTIME_CONTEXT`, injected by Codefly: `native`/`nix`/`container`/`free` (or unset) is a local run, anything else (a GitOps render's `kubernetes`, core ≥ v0.5.6) a deployment | — | | MF assets | `Manifest.Assets` when set (see below), else the `../fe-remote/dist` directory | `ASSETS_DIR` (directory only) | +| Host issuer (MCP) | the resolved host frontend origin — the host is the authorization server an MCP client authenticates against (see [Exposing an MCP server](#exposing-an-mcp-server)); checked at boot only when `ServeMCP` is declared | `HOST_ISSUER_URL` | +| Public MCP URL | none — the resource identifier is then reconstructed per request from `x-forwarded-proto` / `x-forwarded-host` / `x-forwarded-prefix` | `MCP_PUBLIC_URL` (must end in `/mcp`) | The host it plugs into is named by Codefly-convention **service roles**, not by its workspace module name: the runtime discovers the single module that owns each @@ -546,6 +549,155 @@ cancellation. It never proves that a real host admits the solution, that accounts grants the scopes, or that the real module answers its binding the way the test's stand-in does. +### Exposing an MCP server + +A solution exposes its experience to an agent client — Claude Code, Claude +Desktop, any MCP client — the way it exposes it to a browser: as the signed-in +person, through the host's gateway, with the same authority. The solution owns +its tool surface; this runtime owns serving it. These are the three lines: + +```go +solution.New(solution.Manifest{ID: "wiki", Title: "Wiki"}). + ServeMCP("wiki", "v1.0.0", func(srv *mcp.Server) { + mcp.AddTool(srv, &mcp.Tool{Name: "ask_wiki", Description: "ask the wiki a question"}, askWiki) + }). + Serve() +``` + +`register` receives the official SDK's own `*mcp.Server` +(`github.com/modelcontextprotocol/go-sdk`, pinned in this module's `go.mod`), so +tools, prompts and resources are declared exactly as that SDK documents them and +this runtime adds nothing to them. It is called once, when the endpoint is +mounted. + +A tool call runs as the viewer who made the request. `ViewerFromContext` returns +the same caller-bound `Gateway` a `Handler` is given, so a tool reads a composed +module through `ForModule` under the viewer's own Work Context — the identical +call the page makes, with the identical typed refusals: + +```go +func askWiki(ctx context.Context, _ *mcp.CallToolRequest, in askIn) (*mcp.CallToolResult, askOut, error) { + gw, err := solution.ViewerFromContext(ctx) + if err != nil { + return nil, askOut{}, err + } + robin, err := gw.ForModule(ctx, "robin", solution.Scope{ResourceKind: "conversations", Actions: []string{"create"}}) + if err != nil { + return nil, askOut{}, err + } + ... +} +``` + +The viewer comes from the identity headers the gateway stamped, never from the +tool's arguments: those are written by a model, and a session id taken from them +would be a client asking for another viewer's authority. `ViewerFromContext` +errors only for a handler invoked outside a request this runtime served — a +harness of its own, or an `*mcp.Server` mounted without `ServeMCP`. + +#### What is served, and what is refused + +| Path | What | +| --- | --- | +| `/mcp` (`solution.MCPPath`) | Stateless Streamable HTTP. `/solutions//mcp` through the gateway, a route it already fronts. `POST` only: `GET` and `DELETE` are `405` and no `Mcp-Session-Id` is ever issued, because the gateway is a reverse proxy with no sticky routing — a session held in one replica's memory is unreachable from the next request. The SDK's DNS-rebinding protection is left on, so a request reaching a loopback listener with a non-loopback `Host` header is refused `403`: on a developer's machine that protection is the one that still applies, since a rebound page is same-origin and can forge identity headers that a cross-origin page cannot. | +| `/.well-known/oauth-protected-resource` (`solution.ProtectedResourceMetadataPath`) | The OAuth 2.0 Protected Resource Metadata document (RFC 9728): the `resource` a client binds its token to, the host issuer in `authorization_servers`, and `bearer_methods_supported: ["header"]`. Served unauthenticated, with `Access-Control-Allow-Origin: *`, because discovery is public (RFC 9728 §3.1). | + +The MCP endpoint itself carries **no** CORS. The runtime's policy for a +solution's own API admits any origin with an authorization header, which is the +right answer there and the wrong one for the single route that acts with the +viewer's full authority; an MCP client is not a browser page. + +The runtime verifies no token: the gateway strips caller identity headers, runs +ext_authz on the bearer and stamps what it resolved, so verifying it again here +would be a second, divergent implementation of the host's admission rules, with +its own JWKS fetch, its own audience logic and its own bugs. What is enforced is +that the gateway did it: + +| The request | Answer | +| --- | --- | +| No `authorization` | `401` with `WWW-Authenticate: Bearer resource_metadata="…"`. An MCP client's first contact is unauthenticated, and this challenge is what tells it where to find the authorization server. | +| A bearer, but none of `x-user-id`, `x-org-id`, `x-session-id` stamped | `401` with the same challenge: nothing but the gateway may reach this endpoint, so either the request did not come through it or it did not authenticate the bearer — and a client that dialled the wrong address can still discover the right issuer from the challenge. | +| `x-session-id` stamped empty | `403`, naming the `x-credential-kind` the gateway stamped. An organization API key authenticates a principal and no session, and every tool call that acts for the viewer mints a Work Context rooted in one, so this is refused at the boundary rather than once per tool call — where the same refusal would read as the tool being broken. Another token cannot fix it, so it carries no challenge. | + +#### The resource identifier and the issuer + +The `resource` in the metadata document is what a client asks the authorization +server for a token for (RFC 8707) and what its token is audience-bound to, so it +must be the **public** MCP URL — not this process's listen address, not the +in-cluster address the gateway dials. The runtime cannot resolve it: the origin +is the host's, and the path is the host's route layout, which this runtime +deliberately does not encode (see +[Where the host reaches the solution](#where-the-host-reaches-the-solution)). + +So `MCP_PUBLIC_URL` is it when set, and must end in `/mcp` — the metadata +document's own URL is derived from it by swapping that suffix, the same pairing +the registration token URLs use, refused at boot for the same reason when it +cannot be made. Unset, the identifier is reconstructed per request from +`x-forwarded-proto`, `x-forwarded-host` and `x-forwarded-prefix`, and `ServeMCP` +logs at boot that it is doing so: a proxy that forwards no prefix yields an +identifier missing the path it stripped, which a conforming client rejects and a +tolerant one binds to the wrong resource. Set it in any deployment. + +`authorization_servers` is the host issuer resolved at boot — by role, like every +other host endpoint — and is never derived from a request, so a crafted `Host` +header cannot point a client at an authorization server of someone's choosing. +The resolved value is the address this composition reaches the host at: right for +a local run, and in a deployment an in-cluster address no public client could +reach, which is what `HOST_ISSUER_URL` is for. A solution that declares +`ServeMCP` and resolves no issuer is refused at boot; one that declares no MCP +server is unaffected. + +Two things about discovery belong to the host, not to this runtime. It must admit +an unauthenticated `GET` on +`/solutions//.well-known/oauth-protected-resource`, or no client can read the +document the challenge points at. And that is where the document is — on the +solution's own path — whereas RFC 9728 §3.1 also defines a location derived from +the resource's path, `/.well-known/oauth-protected-resource/solutions//mcp`, +at the **host's** root. A client that follows the `resource_metadata` of the 401, +as an MCP client does, reaches the document either way; one that only guesses the +derived location needs the host to serve it there. + +#### Connecting + +Until the host is an MCP-conformant OAuth 2.1 authorization server, a client +presents a token it already has, which the gateway already accepts: + +```sh +claude mcp add --transport http wiki https:///solutions//mcp \ + --header "Authorization: Bearer " +``` + +Once the host supports it, the 401 challenge and the metadata document above are +the whole of what a client needs to authenticate on its own. + +#### Testing the tools: `MCPHandler` + +`Serve` resolves the gateway and the issuer from the composition and registers +with the host, so a solution cannot drive its own tools in a test through it. +`Server.MCPHandler` serves the MCP surface exactly as `Serve` serves it — the +same endpoint, the same metadata document, the same refusals, mounted by the same +code — against an environment the test supplies. Nothing else of the solution is +served and nothing registers anywhere: + +```go +handler, err := s.MCPHandler(solution.MCPEnvironment{ + GatewayURL: fakeHost.URL, // answers the accounts StartTask mint and /v1//* + IssuerURL: "https://host.test", +}) +server := httptest.NewServer(handler) +session, err := mcp.NewClient(&mcp.Implementation{Name: "agent", Version: "v0"}, nil). + Connect(ctx, &mcp.StreamableClientTransport{ + Endpoint: server.URL + solution.MCPPath, + HTTPClient: &http.Client{Transport: stampsTheViewersIdentity{}}, + }, nil) +``` + +What a green test there proves is the solution's side: the tools it offers, the +authority each one mints, and the refusals its callers get. It never proves that +a real gateway stamps what the test's `RoundTripper` stamps, that accounts mints +a Work Context, or that a real MCP client's OAuth flow completes against the +host. + ### Generated messages in a response A handler's value is encoded with `encoding/json`, which reads a generated diff --git a/mcp.go b/mcp.go index c499eef..fd52b30 100644 --- a/mcp.go +++ b/mcp.go @@ -369,12 +369,31 @@ func forwardedOrigin(r *http.Request) string { } } host := firstForwarded(r, forwardedHostHeader) - if host == "" { + // A forwarded host that is not a host is not usable: it would be spliced + // into the middle of the identifier, where a space or a slash makes the + // rest of the URL mean something else. The request's own host is what the + // identifier is built from then. + if !validHost(host) { host = r.Host } return scheme + "://" + host } +// validHost reports whether raw is a plain host[:port] — enough to be +// concatenated into an origin without changing where the URL points. +func validHost(raw string) bool { + if raw == "" || len(raw) > 253+6 { + return false + } + for i := 0; i < len(raw); i++ { + switch c := raw[i]; { + case c <= ' ', c >= 0x7f, c == '/', c == '?', c == '#', c == '@', c == '\\': + return false + } + } + return true +} + // forwardedPrefix is the path the proxy stripped before this solution saw the // request — "/solutions/" at the host gateway. Empty when nothing forwarded // one, which is the case MCP_PUBLIC_URL exists for: the alternative is to diff --git a/mcp_test.go b/mcp_test.go index 1aeaa05..b9c8a82 100644 --- a/mcp_test.go +++ b/mcp_test.go @@ -391,6 +391,22 @@ func TestMCPMetadataDropsAnUnusablePrefix(t *testing.T) { } } +// TestMCPMetadataDropsAnUnusableForwardedHost: a forwarded host that is not a +// host would be spliced into the middle of the identifier, where a space or a +// slash makes the rest of the URL mean something else. The request's own host is +// what the identifier is built from then. +func TestMCPMetadataDropsAnUnusableForwardedHost(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) + own := strings.TrimPrefix(server.URL, "http://") + + for _, host := range []string{"host.test/../elsewhere", "host.test other.test", "user@host.test", "host.test?x=1"} { + document := metadata(t, server, http.Header{"x-forwarded-host": {host}}) + if got, want := document["resource"], "http://"+own+MCPPath; got != want { + t.Errorf("with forwarded host %q, resource = %v, want the request's own host %q", host, got, want) + } + } +} + // --- transport shape --- // TestMCPIsStateless pins the transport the gateway can actually proxy: a diff --git a/solution.go b/solution.go index 584489d..b439fbb 100644 --- a/solution.go +++ b/solution.go @@ -2,12 +2,15 @@ // deployed extensions that plug into a host at runtime with no build-time // coupling. It owns everything every solution needs identically: env/config, // self-registration with the host and the gateway (with a heartbeat), CORS, -// static Module Federation asset serving, the capability handshake, and the -// solution manifest. A solution author supplies a manifest and one or more -// handlers; each handler receives a Gateway that forwards the caller's bearer. +// static Module Federation asset serving, the capability handshake, the +// solution manifest, and serving the solution's MCP server to agent clients. A +// solution author supplies a manifest and one or more handlers; each handler +// receives a Gateway that forwards the caller's bearer, and ServeMCP exposes +// the same experience to an MCP client as the signed-in person. // -// This package depends on nothing but the standard library and knows nothing -// about any specific host or solution. +// This package knows nothing about any specific host or solution: what it +// depends on beyond the standard library is the codefly SDK it resolves +// configuration through, Connect for the wire, and the official MCP SDK. package solution import ( From a1b1249c7f3bdf6b2ccbca9e41bdd3af9358ab59 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:49:04 -0400 Subject: [PATCH 3/3] fix: refuse a deployed MCP surface nobody addressed, and say who sees the 401 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #47 found both halves of the same mistake: a document served with a 200 that names something no client can reach, and a README that told the reader the runtime's own 401 is what starts a client's OAuth flow. The refusal (F2). In a deployed runtime context — the same signal that already refuses a loopback SELF_UPSTREAM or PUBLIC_URL — an unset MCP_PUBLIC_URL leaves the resource identifier reconstructed from the forwarded headers, missing the route prefix the gateway strips and carrying the hop's scheme rather than the client's, and an unset HOST_ISSUER_URL publishes the in-cluster address this composition dials the host at as the authorization server. Both were served from a solution that registered and looked healthy, with one boot log line as the only signal. Now the boot is refused, naming the variable that fixes each; a local run resolves a host it can actually reach and is untouched. Pinned by a test, confirmed by deleting the refusal and watching it go red. Who sees the challenge (F1). The gateway strips identity headers and runs ext_authz before proxying, so an unauthenticated MCP request is denied there and never reaches the solution: the runtime's 401 is the direct and local caller's answer, and the shape the gateway's own challenge has to match. Emitting it there is codefly-dev/module-saas-starter#1003. The README said otherwise in the one place a reader would check, which is worse than silence — it sends whoever is debugging discovery to the wrong repository. The metadata document is still served here: it is what that challenge has to name. Four smaller ones from the same review. A forwarded proto is accepted only as http or https, so "javascript" cannot be spliced in front of "://" in a URL handed to a client (F3). The no-session 403 names the kind the gateway actually stamped instead of asserting api_key, which sent a caller of another kind looking for an API key it does not have (F4). A tool call logs its name — the one thing the accounts mint audit cannot reconstruct — and neither its arguments nor any credential (F6). And both collisions are refused at boot by name rather than panicking inside ServeMux after the listener is open, or silently keeping the last of two ServeMCP calls and serving none of the first's tools (F7). Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 41 ++++++---- mcp.go | 119 ++++++++++++++++++++++++---- mcp_test.go | 219 ++++++++++++++++++++++++++++++++++++++++++++++++++-- solution.go | 24 +++--- 4 files changed, 356 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index cb33f58..798f271 100644 --- a/README.md +++ b/README.md @@ -615,9 +615,20 @@ that the gateway did it: | The request | Answer | | --- | --- | -| No `authorization` | `401` with `WWW-Authenticate: Bearer resource_metadata="…"`. An MCP client's first contact is unauthenticated, and this challenge is what tells it where to find the authorization server. | -| A bearer, but none of `x-user-id`, `x-org-id`, `x-session-id` stamped | `401` with the same challenge: nothing but the gateway may reach this endpoint, so either the request did not come through it or it did not authenticate the bearer — and a client that dialled the wrong address can still discover the right issuer from the challenge. | -| `x-session-id` stamped empty | `403`, naming the `x-credential-kind` the gateway stamped. An organization API key authenticates a principal and no session, and every tool call that acts for the viewer mints a Work Context rooted in one, so this is refused at the boundary rather than once per tool call — where the same refusal would read as the tool being broken. Another token cannot fix it, so it carries no challenge. | +| No `authorization` | `401` with `WWW-Authenticate: Bearer resource_metadata="…"`. | +| A bearer, but none of `x-user-id`, `x-org-id`, `x-session-id` stamped | `401` with the same challenge: nothing but the gateway may reach this endpoint, so either the request did not come through it or it did not authenticate the bearer. | +| `x-session-id` stamped empty | `403`, naming the `x-credential-kind` the gateway stamped. A credential that authenticates without a session cannot act for a viewer, and every tool call that does mints a Work Context rooted in one — so this is refused at the boundary rather than once per tool call, where the same refusal would read as the tool being broken. Another token cannot fix it, so it carries no challenge. | + +**Which caller sees those two 401s.** Not an MCP client coming through the +gateway: the gateway strips the caller's identity headers and runs ext_authz on +the bearer *before* proxying, so an unauthenticated request is denied there with +its own `401 authentication required` and never reaches the solution. The +runtime's 401 is what a caller reaching this solution directly sees — a local +run, a port-forward, a composition dialling the backend — and it is the shape +the gateway's own challenge has to match for discovery to work at all. Emitting +it at the gateway is the host's half +([codefly-dev/module-saas-starter#1003](https://github.com/codefly-dev/module-saas-starter/issues/1003)); +the document that challenge must point at is served here either way. #### The resource identifier and the issuer @@ -647,15 +658,17 @@ reach, which is what `HOST_ISSUER_URL` is for. A solution that declares `ServeMCP` and resolves no issuer is refused at boot; one that declares no MCP server is unaffected. -Two things about discovery belong to the host, not to this runtime. It must admit -an unauthenticated `GET` on -`/solutions//.well-known/oauth-protected-resource`, or no client can read the -document the challenge points at. And that is where the document is — on the -solution's own path — whereas RFC 9728 §3.1 also defines a location derived from -the resource's path, `/.well-known/oauth-protected-resource/solutions//mcp`, -at the **host's** root. A client that follows the `resource_metadata` of the 401, -as an MCP client does, reaches the document either way; one that only guesses the -derived location needs the host to serve it there. +Three things about discovery belong to the host, not to this runtime, all of them +tracked by +[module-saas-starter#1003](https://github.com/codefly-dev/module-saas-starter/issues/1003). +The gateway emits the 401 challenge, as above. It must admit an unauthenticated +`GET` on `/solutions//.well-known/oauth-protected-resource`, or no client can +read the document that challenge points at. And that is where the document is — +on the solution's own path — whereas RFC 9728 §3.1 also defines a location +derived from the resource's path, +`/.well-known/oauth-protected-resource/solutions//mcp`, at the **host's** +root. A client that follows `resource_metadata` reaches the document either way; +one that only guesses the derived location needs the host to serve it there. #### Connecting @@ -667,8 +680,8 @@ claude mcp add --transport http wiki https:///solutions//mcp \ --header "Authorization: Bearer " ``` -Once the host supports it, the 401 challenge and the metadata document above are -the whole of what a client needs to authenticate on its own. +Once the host supports it, the gateway's 401 challenge and the metadata document +above are the whole of what a client needs to authenticate on its own. #### Testing the tools: `MCPHandler` diff --git a/mcp.go b/mcp.go index fd52b30..c253dd5 100644 --- a/mcp.go +++ b/mcp.go @@ -33,10 +33,12 @@ const ( // ProtectedResourceMetadataPath is where the OAuth 2.0 Protected Resource // Metadata document (RFC 9728) serves, at the well-known location a client - // discovers it at. It is the document the 401 challenge points to, so a + // discovers it at. It is the document a 401 challenge points to, so a // client that cannot reach it cannot learn which authorization server to // use: it is served without authentication, and the gateway must admit it - // unauthenticated for discovery to work at all. + // unauthenticated for discovery to work at all. Through the gateway that + // challenge is the gateway's own — see requireStampedViewer — and this is + // the document it has to name. ProtectedResourceMetadataPath = "/.well-known/oauth-protected-resource" ) @@ -94,17 +96,49 @@ func (m *mcpSurface) validate() error { // request, so the MCP session would break under any rollout or scale-out. Each // request carries its own identity, which is all a tool call needs. func (s *Server) ServeMCP(name, version string, register func(*mcp.Server)) *Server { + // A second call is not a second server — there is one MCP endpoint — so it + // replaces the first. Recorded rather than applied silently, and refused at + // boot by validateMCPDeclaration: a solution assembled from two places, + // each declaring its own tools, would otherwise serve only the tools of + // whichever ran last, with tools/list answering as if that were all there + // is. ServeMCP cannot return the error itself, being chainable. + if s.mcp != nil { + s.mcpRedeclared = true + } s.mcp = &mcpSurface{name: name, version: version, register: register} return s } +// validateMCPDeclaration refuses what the author declared, before any address +// is resolved and before the listener opens: these are the same in every +// environment. It is also what keeps a route collision from reaching net/http, +// which panics on a duplicate pattern — inside serve(), with http's own message +// and after the listener is open. +func (s *Server) validateMCPDeclaration() error { + if s.mcp == nil { + return nil + } + if s.mcpRedeclared { + return fmt.Errorf("ServeMCP was called more than once: there is one MCP endpoint, so the last call would be the only surface served and the tools registered by the others would simply not exist. Register every tool in one ServeMCP") + } + if err := s.mcp.validate(); err != nil { + return err + } + for _, path := range []string{MCPPath, ProtectedResourceMetadataPath} { + if _, taken := s.handlers[path]; taken { + return fmt.Errorf("a handler is registered at %q, which ServeMCP serves: one of the two would have to win, and the solution would serve either its own handler with no MCP endpoint or an MCP endpoint with the handler unreachable. Move the handler to another path", path) + } + } + return nil +} + // mountMCP serves the MCP endpoint and its metadata document on mux, or does // nothing when the solution declared no MCP surface. func (s *Server) mountMCP(mux *http.ServeMux) error { if s.mcp == nil { return nil } - if err := s.mcp.validate(); err != nil { + if err := s.validateMCPDeclaration(); err != nil { return fmt.Errorf("solution %q: %w", s.manifest.ID, err) } srv := mcp.NewServer(&mcp.Implementation{Name: s.mcp.name, Version: s.mcp.version}, nil) @@ -166,6 +200,9 @@ func (s *Server) MCPHandler(env MCPEnvironment) (http.Handler, error) { return nil, fmt.Errorf("solution %q declares no MCP server (ServeMCP)", s.manifest.ID) } s.cfg.gatewayURL, s.cfg.mcpIssuerURL, s.cfg.mcpPublicURL = env.GatewayURL, env.IssuerURL, env.PublicURL + // The caller named the issuer, so it is as explicit as the operator's + // override: what validate() refuses is an issuer nobody chose. + s.cfg.mcpIssuerExplicit = env.IssuerURL != "" s.cfg.mcp = true if err := s.cfg.validateMCP(); err != nil { return nil, fmt.Errorf("solution %q: %w", s.manifest.ID, err) @@ -220,6 +257,14 @@ func (s *Server) mcpViewer(next mcp.MethodHandler) mcp.MethodHandler { if extra := req.GetExtra(); extra != nil && extra.Header != nil { header = extra.Header } + // The one thing a tool call leaves nowhere else. Minting is audited on + // accounts and names the viewer and the module, but not which tool + // asked — so an operator reading that audit cannot tell an "ask the + // wiki" from a bulk export. The name only: arguments are the viewer's + // content and the headers carry their credentials. + if call, ok := req.(*mcp.CallToolRequest); ok && call.Params != nil { + log.Printf("solution %q: mcp tool %q", s.manifest.ID, call.Params.Name) + } id := stampedIdentity(header) gw := newGateway(s.cfg.gatewayURL, id.bearer, id.orgID, id.sessionID) return next(context.WithValue(ctx, mcpViewerKey{}, gw), method, req) @@ -268,9 +313,13 @@ func (s *Server) requireStampedViewer(next http.Handler) http.Handler { id := stampedIdentity(r.Header) switch { case id.bearer == "": - // The ordinary first contact of an MCP client, and the reason the - // challenge exists: it names the metadata document the client reads - // to find the authorization server to get a token from. + // Not the request an MCP client makes through the gateway: the + // gateway runs ext_authz before it proxies, so an unauthenticated + // request is denied there and never arrives. What reaches here + // without a bearer is a caller dialling this solution directly — a + // local run, a port-forward — and the challenge is both the only + // answer that tells such a caller where to authenticate and the + // shape the gateway's own has to match. s.challenge(w, r, "no bearer token: this MCP server is a protected resource") return case !id.stamped(): @@ -278,8 +327,8 @@ func (s *Server) requireStampedViewer(next http.Handler) http.Handler { // not come through the host gateway — nothing else may reach this // endpoint, and no header a caller sends is trusted here — or the // gateway did not authenticate it. Answering the challenge rather - // than a bare 401 lets a client that went to the wrong address - // still discover the right authorization server. + // than a bare 401 leaves a caller that reached the wrong address + // something to act on. s.challenge(w, r, fmt.Sprintf("no stamped identity: the host gateway authenticates the bearer and stamps %s, %s and %s, and none of them arrived", userHeader, orgHeader, sessionHeader)) return @@ -289,12 +338,22 @@ func (s *Server) requireStampedViewer(next http.Handler) http.Handler { // that acts for the viewer mints a Work Context rooted in one // (accounts requires a Task to name a session). Named by kind, // because the kind is what the caller has to change. - kind := id.credentialKind - if kind == "" { - kind = "(unstamped)" + // The kind is named as the gateway stamped it, and nothing is + // asserted about which kind it is: told about an API key, a caller + // whose credential is of some other kind goes looking for an API + // key it does not have, and the one fact that would have helped — + // what the gateway actually stamped — is the one the message + // replaced with a guess. + stamped := fmt.Sprintf("%s=%q", credentialKindHeader, id.credentialKind) + if id.credentialKind == "" { + stamped = credentialKindHeader + " empty" + } + detail := "" + if id.credentialKind == apiKeyCredentialKind { + detail = " — an organization API key names a principal and no session" } - http.Error(w, fmt.Sprintf("MCP requires a user session: the gateway stamped %s=%q and %s empty, and a credential that authenticates without a session cannot act for a viewer — an organization %s cannot read composed modules. Connect as a signed-in person.", - credentialKindHeader, kind, sessionHeader, apiKeyCredentialKind), http.StatusForbidden) + http.Error(w, fmt.Sprintf("MCP requires a user session: the gateway stamped %s and %s empty%s. A credential that authenticates without a session cannot act for a viewer: every tool call that does mints a Work Context rooted in one. Connect as a signed-in person.", + stamped, sessionHeader, detail), http.StatusForbidden) return } next.ServeHTTP(w, r) @@ -302,10 +361,16 @@ func (s *Server) requireStampedViewer(next http.Handler) http.Handler { } // challenge answers 401 with the RFC 9728 pointer to this resource's metadata -// document, which is how an MCP client learns which authorization server to +// document, the way a protected resource says which authorization server to // authenticate against (RFC 9728 §5.1). The URL is absolute and names the -// resource as the client reached it, so a client following it arrives back here +// resource as the caller reached it, so one following it arrives back here // rather than at an address only the cluster can dial. +// +// Through the gateway nothing unauthenticated gets this far, so this is the +// direct and local caller's answer, and the shape the gateway's own challenge +// has to match (codefly-dev/module-saas-starter#1003). It is kept here rather +// than dropped because a solution reached directly would otherwise answer a +// bare 401 that names nothing. func (s *Server) challenge(w http.ResponseWriter, r *http.Request, message string) { metadata := siblingURL(s.mcpResource(r), MCPPath, ProtectedResourceMetadataPath) if metadata != "" { @@ -361,8 +426,12 @@ const ( // identifier in its own 401 and its own metadata document, never redirect // anyone to an authorization server of its choosing. func forwardedOrigin(r *http.Request) string { - scheme := firstForwarded(r, forwardedProtoHeader) - if scheme == "" { + // Only the two schemes an HTTP resource can be dialled over. A forwarded + // proto is a hop's claim about the client's connection, and anything else + // would be spliced in front of "://" — "javascript" included, which is a + // URL a client might hand to something that executes it. + scheme := strings.ToLower(firstForwarded(r, forwardedProtoHeader)) + if scheme != "http" && scheme != "https" { scheme = "http" if r.TLS != nil { scheme = "https" @@ -478,6 +547,22 @@ func (c config) validateMCP() error { return fmt.Errorf("unresolved host issuer %q: ServeMCP publishes it as the authorization server of this MCP resource, so a client has nowhere to authenticate without it. Set %s to the origin MCP clients authenticate against, or ensure the SDK resolves the host frontend's http endpoint", c.mcpIssuerURL, hostIssuerURLEnvironmentVariable) } + // In a deployment both values below are the operator's to set, for the + // reason the loopback refusals above exist: the runtime cannot tell that + // what it resolved is unreachable, and serves a well-formed document naming + // it with a 200, from a solution that registered and looks healthy. A boot + // log line was the only signal, and a log line nobody reads is how a + // solution is absent while every gate is green. + if deployedRuntimeContext(c.runtimeContext) { + if c.mcpPublicURL == "" { + return fmt.Errorf("no %s set in the deployed runtime context %q: the gateway strips the route it proxies this solution under, so the identifier reconstructed from the forwarded headers is missing that prefix (and names the scheme of the hop, not the client's) — an MCP client binds its token to the identifier it dialled and rejects any other. Set it to the URL clients dial, ending in %s", + mcpPublicURLEnvironmentVariable, c.runtimeContext, MCPPath) + } + if !c.mcpIssuerExplicit { + return fmt.Errorf("no %s set in the deployed runtime context %q: the issuer resolved from the SDK (%q) is the address this composition dials the host at, which no MCP client can reach, and it would be published as this resource's authorization server. Set it to the origin clients authenticate against", + hostIssuerURLEnvironmentVariable, c.runtimeContext, c.mcpIssuerURL) + } + } if c.mcpPublicURL == "" { return nil } diff --git a/mcp_test.go b/mcp_test.go index b9c8a82..96875f1 100644 --- a/mcp_test.go +++ b/mcp_test.go @@ -1,12 +1,16 @@ package solution import ( + "bytes" "context" "encoding/json" "io" + "log" "net/http" "net/http/httptest" + "os" "strings" + "sync" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -209,10 +213,12 @@ func post(t *testing.T, server *httptest.Server, header http.Header) *http.Respo return resp } -// TestMCPChallengesARequestWithNoBearer pins the discovery handshake: an MCP -// client's first contact is unauthenticated, and the 401 is what tells it where -// to find the authorization server. Without the challenge the client has nothing -// to go on but the status code. +// TestMCPChallengesARequestWithNoBearer pins the direct caller's answer and the +// shape the gateway's own challenge has to match. Through the gateway an +// unauthenticated request is denied by ext_authz before it is proxied, so what +// arrives here with no bearer reached this solution directly — a local run, a +// port-forward — and without the challenge it would have nothing to go on but a +// status code. func TestMCPChallengesARequestWithNoBearer(t *testing.T) { server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) @@ -230,9 +236,8 @@ func TestMCPChallengesARequestWithNoBearer(t *testing.T) { // TestMCPChallengesARequestTheGatewayDidNotStamp covers the request that carries // a bearer the runtime never looks at. Nothing but the gateway may reach this // endpoint, so a bearer with no stamped identity is not a caller this runtime can -// serve — and it is answered with the challenge rather than a bare 401, because a -// client that dialled the wrong address can still discover the right issuer from -// it. +// serve — and it is answered with the challenge rather than a bare 401 so that a +// caller which reached the wrong address has something to act on. func TestMCPChallengesARequestTheGatewayDidNotStamp(t *testing.T) { server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer, PublicURL: "https://host.test/solutions/wiki" + MCPPath}, readCollectionTool) @@ -276,6 +281,31 @@ func TestMCPRefusesACredentialWithNoSession(t *testing.T) { } } +// TestMCPNoSessionRefusalNamesTheStampedKind: told about an API key, a caller +// whose credential is of another kind goes looking for an API key it does not +// have — while the one fact that would have helped, what the gateway stamped, is +// what the message replaced with a guess. +func TestMCPNoSessionRefusalNamesTheStampedKind(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) + + resp := post(t, server, http.Header{ + "authorization": {"Bearer token"}, + userHeader: {"viewer-principal"}, + orgHeader: {viewerOrg}, + credentialKindHeader: {"service_account"}, + }) + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("answered %d, want 403", resp.StatusCode) + } + body := readBody(t, resp) + if !strings.Contains(body, "service_account") { + t.Errorf("refusal %q does not name the stamped kind", body) + } + if strings.Contains(body, apiKeyCredentialKind) { + t.Errorf("refusal %q names %q, which is not what the gateway stamped", body, apiKeyCredentialKind) + } +} + func readBody(t *testing.T, resp *http.Response) string { t.Helper() body, err := io.ReadAll(resp.Body) @@ -407,6 +437,88 @@ func TestMCPMetadataDropsAnUnusableForwardedHost(t *testing.T) { } } +// TestMCPMetadataDropsAnUnusableForwardedScheme: a forwarded proto is a hop's +// claim about the client's connection, and only the two schemes an HTTP resource +// can be dialled over are usable. "javascript" would otherwise be spliced in +// front of "://" and handed to a client as the URL of this resource. +func TestMCPMetadataDropsAnUnusableForwardedScheme(t *testing.T) { + server := serveMCP(t, MCPEnvironment{IssuerURL: testIssuer}, readCollectionTool) + + for _, scheme := range []string{"javascript", "file", "ws", "httpss", "data"} { + document := metadata(t, server, http.Header{ + "x-forwarded-proto": {scheme}, + "x-forwarded-host": {"host.test"}, + }) + if got, want := document["resource"], "http://host.test"+MCPPath; got != want { + t.Errorf("with forwarded proto %q, resource = %v, want the request's own scheme in %q", scheme, got, want) + } + } + // The one a proxy actually forwards is used. + document := metadata(t, server, http.Header{"x-forwarded-proto": {"HTTPS"}, "x-forwarded-host": {"host.test"}}) + if got, want := document["resource"], "https://host.test"+MCPPath; got != want { + t.Errorf("resource = %v, want %q: https is the scheme a proxy in front of TLS forwards", got, want) + } +} + +// TestMCPToolCallIsRecordedByName is the only trace a tool call leaves here: +// minting is audited on accounts and names the viewer and the module, but not +// which tool asked, so an operator reading that audit cannot tell one tool from +// another. The name only — arguments are the viewer's content, and the headers +// carry their credentials. +func TestMCPToolCallIsRecordedByName(t *testing.T) { + host := newWorkContextGateway(t, &workContextGateway{}) + server := serveMCP(t, MCPEnvironment{GatewayURL: host.URL, IssuerURL: testIssuer}, readCollectionTool) + session := connectMCP(t, server, viewerStamp()) + + recorded := captureLog(t) + if _, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: mcpToolName, + Arguments: map[string]any{"question": "a secret question"}, + }); err != nil { + t.Fatalf("tools/call: %v", err) + } + logged := recorded.String() + if !strings.Contains(logged, mcpToolName) { + t.Errorf("the log %q does not name the tool that ran", logged) + } + if strings.Contains(logged, "a secret question") || strings.Contains(logged, "viewer-token") { + t.Errorf("the log %q carries the call's arguments or the viewer's credential", logged) + } +} + +// captureLog collects what the runtime logs for the rest of the test. It is +// locked because the heartbeat goroutines of tests that have finished are still +// writing to the same logger. +func captureLog(t *testing.T) *lockedBuffer { + t.Helper() + buffer := &lockedBuffer{} + flags := log.Flags() + log.SetOutput(buffer) + log.SetFlags(0) + t.Cleanup(func() { + log.SetOutput(os.Stderr) + log.SetFlags(flags) + }) + return buffer +} + +type lockedBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *lockedBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *lockedBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + // --- transport shape --- // TestMCPIsStateless pins the transport the gateway can actually proxy: a @@ -523,6 +635,99 @@ func TestValidateRefusesAnUnpairablePublicURL(t *testing.T) { } } +// TestServeMCPRefusesARedeclaredSurface: there is one MCP endpoint, so a second +// ServeMCP would be the only one served and the tools registered by the first +// would simply not exist — with tools/list answering as if that were all there +// is. Refused at boot rather than resolved by call order. +func TestServeMCPRefusesARedeclaredSurface(t *testing.T) { + s := New(Manifest{ID: "wiki"}). + ServeMCP("wiki", "v1", readCollectionTool). + ServeMCP("wiki", "v1", readCollectionTool) + err := s.validateMCPDeclaration() + if err == nil { + t.Fatal("two ServeMCP calls were accepted") + } + if !strings.Contains(err.Error(), "ServeMCP") { + t.Errorf("refusal %q does not name ServeMCP", err) + } + if _, err := s.MCPHandler(MCPEnvironment{IssuerURL: testIssuer}); err == nil { + t.Error("MCPHandler served a redeclared surface") + } +} + +// TestServeMCPRefusesAHandlerOnItsOwnPath: net/http panics on a duplicate +// pattern, which in serve() happens with http's own message and after the +// listener is open. One of the two would have to win, so the boot names them +// both instead. +func TestServeMCPRefusesAHandlerOnItsOwnPath(t *testing.T) { + for _, path := range []string{MCPPath, ProtectedResourceMetadataPath} { + s := New(Manifest{ID: "wiki"}).ServeMCP("wiki", "v1", readCollectionTool) + s.Handle(path, func(context.Context, *Gateway) (any, error) { return nil, nil }) + err := s.validateMCPDeclaration() + if err == nil { + t.Errorf("a handler at %q was accepted alongside ServeMCP", path) + continue + } + if !strings.Contains(err.Error(), path) { + t.Errorf("refusal %q does not name %q", err, path) + } + } + // The same solution without the collision is served. + if err := New(Manifest{ID: "wiki"}).ServeMCP("wiki", "v1", readCollectionTool). + Handle("/search", func(context.Context, *Gateway) (any, error) { return nil, nil }). + validateMCPDeclaration(); err != nil { + t.Errorf("a handler on its own path was refused: %v", err) + } +} + +// TestValidateRefusesADeployedMCPThatNobodyAddressed is the F2 refusal: in a +// deployment the runtime cannot tell that what it resolved is unreachable. The +// identifier reconstructed from forwarded headers is missing the route prefix +// the gateway stripped, and the resolved issuer is the address this composition +// dials the host at — both served with a 200, from a solution that registered +// and looks healthy, with a boot log line as the only signal. It is the same +// silent damage the loopback refusals above already refuse, so it is refused the +// same way. +func TestValidateRefusesADeployedMCPThatNobodyAddressed(t *testing.T) { + const inCluster = "http://frontend.saas.svc.cluster.local:3000" + const public = "https://app.example.com/solutions/wiki" + MCPPath + + deployed := config{mcp: true, runtimeContext: "kubernetes", mcpIssuerURL: inCluster} + err := deployed.validateMCP() + if err == nil { + t.Fatal("a deployed MCP surface with no public URL was accepted") + } + if !strings.Contains(err.Error(), mcpPublicURLEnvironmentVariable) { + t.Errorf("refusal %q does not name %s", err, mcpPublicURLEnvironmentVariable) + } + + addressed := deployed + addressed.mcpPublicURL = public + err = addressed.validateMCP() + if err == nil { + t.Fatal("a deployed MCP surface with an issuer nobody chose was accepted") + } + if !strings.Contains(err.Error(), hostIssuerURLEnvironmentVariable) { + t.Errorf("refusal %q does not name %s", err, hostIssuerURLEnvironmentVariable) + } + + told := addressed + told.mcpIssuerExplicit = true + told.mcpIssuerURL = "https://app.example.com" + if err := told.validateMCP(); err != nil { + t.Errorf("a deployed MCP surface an operator addressed was refused: %v", err) + } + + // A local run resolves a host it can actually reach and is unaffected: the + // refusal is about a deployment, never about the environment's name. + for _, local := range []string{"", "native", "nix", "container", "free"} { + cfg := config{mcp: true, runtimeContext: local, mcpIssuerURL: "http://localhost:3000"} + if err := cfg.validateMCP(); err != nil { + t.Errorf("runtime context %q was refused: %v", local, err) + } + } +} + // TestViewerFromContextOutsideAServedRequest: a tool handler invoked by a // harness of its own gets a refusal that says where the viewer comes from, // rather than a nil gateway that fails later inside a module read. diff --git a/solution.go b/solution.go index b439fbb..4c3e243 100644 --- a/solution.go +++ b/solution.go @@ -226,9 +226,11 @@ type Server struct { consumed []ConsumedModule passthrough map[string]passthroughRoute // mcp is the MCP server the solution declared with ServeMCP, nil when it - // declared none. - mcp *mcpSurface - cfg config + // declared none; mcpRedeclared records a second ServeMCP, which the boot + // refuses rather than letting the last call quietly be the only surface. + mcp *mcpSurface + mcpRedeclared bool + cfg config // registrationInterval is how long a registration heartbeat waits between // beats. Zero means defaultRegistrationInterval. Per-server rather than a // package value so a test can drive several beats without every other @@ -283,6 +285,11 @@ type config struct { // mcpPublicURL is the canonical public MCP URL an operator set, empty when // none was (see mcpResource). mcpPublicURL string + // mcpIssuerExplicit says the issuer came from the operator rather than from + // the SDK. It is what tells a deployment that resolved the host's + // in-cluster address — which validate() refuses — from one that was told + // the origin clients authenticate against. + mcpIssuerExplicit bool } func env(key, fallback string) string { @@ -496,8 +503,9 @@ func loadConfig(ctx context.Context, id string) config { // the one this composition reaches the host at, which is right for a // local run and is an in-cluster address in a deployment, where no // public client could reach it: that is what the override is for. - mcpIssuerURL: strings.TrimRight(env(hostIssuerURLEnvironmentVariable, frontendURL), "/"), - mcpPublicURL: strings.TrimRight(env(mcpPublicURLEnvironmentVariable, ""), "/"), + mcpIssuerURL: strings.TrimRight(env(hostIssuerURLEnvironmentVariable, frontendURL), "/"), + mcpIssuerExplicit: env(hostIssuerURLEnvironmentVariable, "") != "", + mcpPublicURL: strings.TrimRight(env(mcpPublicURLEnvironmentVariable, ""), "/"), } cfg.registrationInterval, cfg.registrationIntervalErr = registrationIntervalFromEnv() return cfg @@ -779,10 +787,8 @@ func (s *Server) Serve() error { if err := s.manifest.validateSurfaces(); err != nil { return fmt.Errorf("solution %q: %w", s.manifest.ID, err) } - if s.mcp != nil { - if err := s.mcp.validate(); err != nil { - return fmt.Errorf("solution %q: %w", s.manifest.ID, err) - } + if err := s.validateMCPDeclaration(); err != nil { + return fmt.Errorf("solution %q: %w", s.manifest.ID, err) } // The SDK owns environment resolution: load Codefly's injected carriers so // endpoint and workspace-secret lookups resolve from them (falling back to