diff --git a/README.md b/README.md index 61cc666..321510e 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,8 @@ allows for clients older than `2025-06-18`. * `forward-operator-identity` - Copy the `X-Operator-Identity` header from inbound MCP requests onto outbound gRPC calls, so the backend can attribute agent calls to the human operator. The header is read per request, so it identifies the caller of that tool call. It must be minted by a trusted proxy in front of this server; grpcmcp does not verify it. This needs `hostport`, because stdio carries no HTTP headers. +* `forward-header` string - Copy a named header from inbound MCP requests onto outbound gRPC calls, if present. Repeatable (e.g. `--forward-header=X-Forwarded-User --forward-header=X-Forwarded-Access-Token`). Same trust model as `forward-operator-identity` -- the header must be minted by a trusted proxy in front of this server, and grpcmcp does not verify it -- generalized to whatever header name that proxy actually uses instead of `X-Operator-Identity` specifically. This needs `hostport`, for the same reason as above. + * `string64` - If set, expose 64-bit protobuf integer fields (`int64`, `uint64`, `sint64`, `fixed64`, `sfixed64`) as strings only in MCP JSON schemas. This avoids precision ambiguity for JavaScript-based clients and agents. By default, schemas continue to allow either JSON numbers or strings for compatibility. * `refresh-interval` duration - How often to re-run reflection so methods added to the backend appear without a restart. Defaults to `5m`. This applies when `reflect` is set and `descriptors` is not: a descriptor file wins over reflection for the initial load, so a refresh from reflection would replace the set the operator asked for. diff --git a/example/echo.go b/example/echo.go index eb3d5db..56d1ee4 100644 --- a/example/echo.go +++ b/example/echo.go @@ -110,7 +110,14 @@ func echoHandler(method protoreflect.MethodDescriptor) http.HandlerFunc { } resp := dynamicpb.NewMessage(outputDesc) - resp.Set(outMessageField, req.Get(messageField)) + message := req.Get(messageField).String() + // Surface a forwarded header in the response, if present, so a test + // can confirm grpcmcp actually forwarded it -- not just that grpcmcp's + // own code constructed the header, but that it arrived at the backend. + if forwardedUser := r.Header.Get("X-Forwarded-User"); forwardedUser != "" { + message += "|" + forwardedUser + } + resp.Set(outMessageField, protoreflect.ValueOfString(message)) respBytes, err := proto.Marshal(resp) if err != nil { diff --git a/forward_header_flag_validation_test.go b/forward_header_flag_validation_test.go new file mode 100644 index 0000000..1eaf71f --- /dev/null +++ b/forward_header_flag_validation_test.go @@ -0,0 +1,139 @@ +package main + +import ( + "bytes" + "os/exec" + "testing" + "time" +) + +// waitForExit runs cmd and waits up to timeout for it to exit on its own, +// rather than blocking forever if it doesn't -- which would hang the test +// suite instead of failing it when a regression makes grpcmcp start serving +// where it used to reject the flags and exit. +func waitForExit(t *testing.T, cmd *exec.Cmd, timeout time.Duration) (exited bool, err error) { + t.Helper() + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + defer cmd.Process.Kill() + + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + + select { + case err := <-done: + return true, err + case <-time.After(timeout): + return false, nil + } +} + +// TestForwardHeaderRequiresHostport checks that -forward-header (and +// -forward-operator-identity) are rejected without -hostport, rather than +// silently doing nothing: stdio has no inbound HTTP headers to forward, so +// an operator relying on either flag for identity attribution would +// otherwise get no error and no forwarded header. +func TestForwardHeaderRequiresHostport(t *testing.T) { + bin := buildGrpcmcp(t) + descFile := emptyDescriptorFile(t) + + for _, flagName := range []string{"--forward-header=X-Forwarded-User", "--forward-operator-identity"} { + t.Run(flagName, func(t *testing.T) { + cmd := exec.Command(bin, flagName, "--descriptors="+descFile) + var stderr bytes.Buffer + cmd.Stderr = &stderr + + exited, err := waitForExit(t, cmd, 2*time.Second) + if !exited { + t.Fatalf("expected the process to exit without -hostport; it is still running instead of rejecting the flag") + } + if err == nil { + t.Fatalf("expected a non-zero exit without -hostport, got success") + } + if !bytes.Contains(stderr.Bytes(), []byte("need -hostport")) { + t.Errorf("stderr = %q, want a message naming the -hostport requirement", stderr.String()) + } + }) + } +} + +// TestForwardHeaderCollisionRejected checks that -forward-header naming a +// header already set via -header (most dangerously Authorization) is +// rejected at startup, rather than letting an inbound MCP client silently +// override grpcmcp's own trusted backend credential. +func TestForwardHeaderCollisionRejected(t *testing.T) { + bin := buildGrpcmcp(t) + descFile := emptyDescriptorFile(t) + + cmd := exec.Command(bin, + "--hostport=localhost:0", + "--header=Authorization: Bearer backend-secret", + "--forward-header=Authorization", + "--descriptors="+descFile, + ) + var stderr bytes.Buffer + cmd.Stderr = &stderr + + exited, err := waitForExit(t, cmd, 2*time.Second) + if !exited { + t.Fatalf("expected the process to reject a colliding -forward-header; it is still running instead") + } + if err == nil { + t.Fatalf("expected a non-zero exit for a colliding -forward-header, got success") + } + if !bytes.Contains(stderr.Bytes(), []byte("collides")) { + t.Errorf("stderr = %q, want a message naming the collision", stderr.String()) + } +} + +// TestForwardOperatorIdentityCollisionRejected mirrors +// TestForwardHeaderCollisionRejected for -forward-operator-identity, whose +// forwarded header name (X-Operator-Identity) is fixed rather than +// configurable. +func TestForwardOperatorIdentityCollisionRejected(t *testing.T) { + bin := buildGrpcmcp(t) + descFile := emptyDescriptorFile(t) + + cmd := exec.Command(bin, + "--hostport=localhost:0", + "--header=X-Operator-Identity: someone-else", + "--forward-operator-identity", + "--descriptors="+descFile, + ) + var stderr bytes.Buffer + cmd.Stderr = &stderr + + exited, err := waitForExit(t, cmd, 2*time.Second) + if !exited { + t.Fatalf("expected the process to reject a colliding -forward-operator-identity; it is still running instead") + } + if err == nil { + t.Fatalf("expected a non-zero exit for a colliding -forward-operator-identity, got success") + } + if !bytes.Contains(stderr.Bytes(), []byte("collides")) { + t.Errorf("stderr = %q, want a message naming the collision", stderr.String()) + } +} + +// TestForwardHeaderNoCollisionStarts checks that the collision guard does not +// reject a -forward-header configuration with no actual collision. +func TestForwardHeaderNoCollisionStarts(t *testing.T) { + bin := buildGrpcmcp(t) + descFile := emptyDescriptorFile(t) + + cmd := exec.Command(bin, + "--hostport=localhost:0", + "--header=Authorization: Bearer backend-secret", + "--forward-header=X-Forwarded-User", + "--descriptors="+descFile, + ) + var stderr bytes.Buffer + cmd.Stderr = &stderr + + exited, err := waitForExit(t, cmd, 1*time.Second) + if exited { + t.Fatalf("process exited, expected it to keep serving: %v, stderr: %s", err, stderr.String()) + } + // Still running after the timeout, as expected: no collision, nothing to reject. +} diff --git a/forward_header_test.go b/forward_header_test.go new file mode 100644 index 0000000..c293b00 --- /dev/null +++ b/forward_header_test.go @@ -0,0 +1,48 @@ +package main + +import ( + "context" + "net/http" + "testing" + + "github.com/Basic-Capital/grpcmcp/grpcmcp" + "github.com/mark3labs/mcp-go/mcp" +) + +func TestForwardHeadersCopiesNamedHeaders(t *testing.T) { + static := make(http.Header) + static.Set("Authorization", "Bearer token") + provider := forwardHeaders(grpcmcp.StaticHeaders(static), []string{"X-Forwarded-User", "X-Forwarded-Access-Token"}) + + request := mcp.CallToolRequest{} + request.Header = http.Header{} + request.Header.Set("X-Forwarded-User", "alice@basiccapital.com") + request.Header.Set("X-Forwarded-Access-Token", "opaque-token") + request.Header.Set("X-Forwarded-Email", "alice@basiccapital.com") // not in the forward list + + h, err := provider(context.Background(), request) + if err != nil { + t.Fatal(err) + } + if got := h.Get("X-Forwarded-User"); got != "alice@basiccapital.com" { + t.Errorf("X-Forwarded-User = %q, want alice@basiccapital.com", got) + } + if got := h.Get("X-Forwarded-Access-Token"); got != "opaque-token" { + t.Errorf("X-Forwarded-Access-Token = %q, want opaque-token", got) + } + if got := h.Get("X-Forwarded-Email"); got != "" { + t.Errorf("X-Forwarded-Email = %q, want empty: not in the configured forward list", got) + } + if got := h.Get("Authorization"); got != "Bearer token" { + t.Errorf("Authorization = %q, want static headers preserved", got) + } + + // Without any inbound headers, nothing is added. + h, err = provider(context.Background(), mcp.CallToolRequest{}) + if err != nil { + t.Fatal(err) + } + if got := h.Get("X-Forwarded-User"); got != "" { + t.Errorf("X-Forwarded-User = %q, want empty", got) + } +} diff --git a/main.go b/main.go index 6434ded..6bbc6cc 100644 --- a/main.go +++ b/main.go @@ -44,6 +44,29 @@ func operatorIdentityHeaders(base grpcmcp.ToolHeaderProvider) grpcmcp.ToolHeader } } +// forwardHeaders wraps base to copy each named header from the inbound MCP +// request onto the outbound gRPC call, if present. Like operatorIdentityHeaders, +// this trusts whatever minted the header -- grpcmcp does not verify it -- so +// it is only as safe as the proxy in front of this server. Unlike +// operatorIdentityHeaders, the header name is not fixed: this is the general +// mechanism for a reverse proxy (e.g. an OAuth-terminating proxy) that +// authenticates the caller and asserts identity via its own header names +// (e.g. X-Forwarded-User) rather than X-Operator-Identity specifically. +func forwardHeaders(base grpcmcp.ToolHeaderProvider, names []string) grpcmcp.ToolHeaderProvider { + return func(ctx context.Context, req mcp.CallToolRequest) (http.Header, error) { + h, err := base(ctx, req) + if err != nil { + return nil, err + } + for _, name := range names { + if v := req.Header.Get(name); v != "" { + h.Set(name, v) + } + } + return h, nil + } +} + type headerFlags http.Header func (s *headerFlags) String() string { @@ -192,6 +215,11 @@ func main() { useConnect := flag.Bool("connect", false, "Use connect protocol (instead of gRPC)") requireMethodOption := flag.String("require-method-option", "", "Only expose methods with this option (fieldNumber:value or fieldNumber:value1,value2, e.g. 50003:1 or 50003:1,2)") forwardOperatorIdentity := flag.Bool("forward-operator-identity", false, "Copy the X-Operator-Identity header from inbound MCP requests onto outbound gRPC calls. The header must be minted by a trusted proxy in front of this server; grpcmcp does not verify it.") + var forwardHeaderNames []string + flag.Func("forward-header", "Copy a named header from inbound MCP requests onto outbound gRPC calls, if present. Repeatable. The header must be minted by a trusted proxy in front of this server; grpcmcp does not verify it.", func(v string) error { + forwardHeaderNames = append(forwardHeaderNames, v) + return nil + }) string64 := flag.Bool("string64", false, "Expose 64-bit protobuf integer fields as strings only in JSON schemas") refreshInterval := flag.Duration("refresh-interval", 5*time.Minute, "How often to re-run reflection so new backend methods appear. Applies when reflect is set without descriptors.") refreshTimeout := flag.Duration("refresh-timeout", time.Minute, "Time limit for one reflection refresh attempt") @@ -243,6 +271,14 @@ func main() { fmt.Fprint(os.Stderr, "-tls-crt, -tls-key, and -ca-file need -hostport. Without it the server uses stdio, which has no TLS.\n") os.Exit(-1) } + // Same reasoning: stdio has no inbound HTTP headers to forward, so these + // flags would silently do nothing rather than forward anything, which is + // worse than an error for something an operator is relying on for identity + // attribution. + if (*forwardOperatorIdentity || len(forwardHeaderNames) > 0) && !serveHTTP { + fmt.Fprint(os.Stderr, "-forward-operator-identity and -forward-header need -hostport. Without it the server uses stdio, which has no HTTP headers to forward.\n") + os.Exit(-1) + } tlsBackendClient, err := backendTLSClient(*clientCAFile, *clientTLSCrt, *clientTLSKey) if err != nil { @@ -289,6 +325,23 @@ func main() { } } + // forwardHeaders and operatorIdentityHeaders overwrite whatever base already + // set for that header name (see forwardHeaders' doc comment). If a forwarded + // name collides with one set here from -header or -bearer-env -- most + // dangerously Authorization -- an inbound MCP client would silently replace + // grpcmcp's own trusted backend credential on every call. Reject that + // configuration outright rather than let it happen quietly. + for _, name := range forwardHeaderNames { + if http.Header(headers).Get(name) != "" { + fmt.Fprintf(os.Stderr, "-forward-header=%q collides with a header already set via -header or -bearer-env; refusing to let an inbound client override it.\n", name) + os.Exit(-1) + } + } + if *forwardOperatorIdentity && http.Header(headers).Get(operatorIdentityHeader) != "" { + fmt.Fprintf(os.Stderr, "-forward-operator-identity collides with a -header value already set for %s; refusing to let an inbound client override it.\n", operatorIdentityHeader) + os.Exit(-1) + } + ctx := context.Background() if *descriptors == "" && !*reflect { @@ -329,6 +382,9 @@ func main() { if *forwardOperatorIdentity { headersProvider = operatorIdentityHeaders(headersProvider) } + if len(forwardHeaderNames) > 0 { + headersProvider = forwardHeaders(headersProvider, forwardHeaderNames) + } // listChanged promises the client a notification when the tool set changes. // Only stdio can keep that promise. The HTTP server is stateless and serves diff --git a/scripts/e2e_test.sh b/scripts/e2e_test.sh index 9ec26af..1daf68c 100755 --- a/scripts/e2e_test.sh +++ b/scripts/e2e_test.sh @@ -27,7 +27,8 @@ WORKDIR=$(mktemp -d) BACKEND_PID= HTTP_PID= SSE_PID= -trap 'kill $BACKEND_PID $HTTP_PID $SSE_PID 2>/dev/null; wait 2>/dev/null; rm -rf "$WORKDIR"' EXIT +FWD_PID= +trap 'kill $BACKEND_PID $HTTP_PID $SSE_PID $FWD_PID 2>/dev/null; wait 2>/dev/null; rm -rf "$WORKDIR"' EXIT echo "== building ==" go build -o "$WORKDIR/grpcmcp" . || exit 1 @@ -48,9 +49,15 @@ echo "== starting grpcmcp --transport=sse (:8092) ==" SSE_PID=$! sleep 0.5 +echo "== starting grpcmcp --forward-header=X-Forwarded-User (:8093) ==" +"$WORKDIR/grpcmcp" --hostport=localhost:8093 --reflect --transport=http --forward-header=X-Forwarded-User >"$WORKDIR/fwd.log" 2>&1 & +FWD_PID=$! +sleep 0.5 + if ! kill -0 "$BACKEND_PID" 2>/dev/null; then echo "backend failed to start"; cat "$WORKDIR"/*.log; exit 1; fi if ! kill -0 "$HTTP_PID" 2>/dev/null; then echo "http server failed to start"; cat "$WORKDIR/http.log"; exit 1; fi if ! kill -0 "$SSE_PID" 2>/dev/null; then echo "sse server failed to start"; cat "$WORKDIR/sse.log"; exit 1; fi +if ! kill -0 "$FWD_PID" 2>/dev/null; then echo "forward-header server failed to start"; cat "$WORKDIR/fwd.log"; exit 1; fi INIT_BODY='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"e2e","version":"1"}}}' LIST_BODY='{"jsonrpc":"2.0","id":2,"method":"tools/list"}' @@ -89,6 +96,25 @@ status=$(curl -s -o /dev/null -w '%{http_code}' -X POST http://localhost:8091/mc check "POST /mcp with bogus Mcp-Protocol-Version -> 400" "$status" "400" echo +echo +echo "== --forward-header=X-Forwarded-User (:8093) ==" + +curl -s -X POST http://localhost:8093/mcp \ + -H 'Content-Type: application/json' -H 'Accept: application/json, text/event-stream' \ + -d "$INIT_BODY" > /dev/null + +fwd_result=$(curl -s -X POST http://localhost:8093/mcp \ + -H 'Content-Type: application/json' -H 'Accept: application/json, text/event-stream' \ + -H 'X-Forwarded-User: alice@basiccapital.com' \ + -d "$CALL_BODY") +if echo "$fwd_result" | grep -q 'hello-e2e|alice@basiccapital.com'; then + echo " ok - X-Forwarded-User reached the backend over a real gRPC call" + PASS=$((PASS+1)) +else + echo " FAIL - X-Forwarded-User did not reach the backend: $fwd_result" + FAIL=$((FAIL+1)) +fi + echo "== SSE (:8092, deprecated) ==" if grep -q '\[deprecated\].*transport=sse' "$WORKDIR/sse.log"; then