diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 4aa6e9d..de247f7 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -41,24 +41,33 @@ type Server struct { defaultTenant string } -// New creates a new MCP server. +// New creates a new MCP server. defaultTenant is the fallback tenant applied +// to header-less MCP requests; an empty string falls back to +// storage.DefaultTenantID. Required at construction time so production startup +// cannot accidentally drop cfg.DefaultTenant — a missing argument is a compile +// error rather than a silent regression. func New( + defaultTenant string, repo *storage.Repository, metrics *telemetry.Metrics, svcGraph *graph.Graph, vectorIdx *vectordb.Index, ) *Server { + if defaultTenant == "" { + defaultTenant = storage.DefaultTenantID + } return &Server{ repo: repo, metrics: metrics, svcGraph: svcGraph, vectorIdx: vectorIdx, - defaultTenant: storage.DefaultTenantID, + defaultTenant: defaultTenant, } } -// SetDefaultTenant overrides the fallback tenant used when an MCP request -// carries no X-Tenant-ID header. Call from startup wiring with cfg.DefaultTenant. +// SetDefaultTenant overrides the fallback tenant at runtime. Empty strings are +// ignored so callers can pass through optional config without clobbering the +// constructor-provided value. func (s *Server) SetDefaultTenant(t string) { if t != "" { s.defaultTenant = t diff --git a/internal/mcp/server_ran22_test.go b/internal/mcp/server_ran22_test.go new file mode 100644 index 0000000..37f6de6 --- /dev/null +++ b/internal/mcp/server_ran22_test.go @@ -0,0 +1,138 @@ +package mcp + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/RandomCodeSpace/otelcontext/internal/storage" + "github.com/RandomCodeSpace/otelcontext/internal/vectordb" +) + +// TestNew_DefaultTenant_FromConstructor is the RAN-22 regression bar at the +// type level: the configured default tenant is a required leading parameter +// to New, so production startup wiring (main.go) cannot drop it without a +// compile error. Empty input falls back to storage.DefaultTenantID; a +// non-empty value is preserved verbatim. +func TestNew_DefaultTenant_FromConstructor(t *testing.T) { + t.Run("empty falls back to storage.DefaultTenantID", func(t *testing.T) { + srv := New("", nil, nil, nil, vectordb.New(1)) + if srv.defaultTenant != storage.DefaultTenantID { + t.Fatalf(`New("") defaultTenant = %q, want %q`, srv.defaultTenant, storage.DefaultTenantID) + } + }) + t.Run("non-empty value is preserved", func(t *testing.T) { + srv := New("acme", nil, nil, nil, vectordb.New(1)) + if srv.defaultTenant != "acme" { + t.Fatalf(`New("acme") defaultTenant = %q, want "acme"`, srv.defaultTenant) + } + }) + t.Run("SetDefaultTenant runtime override still works", func(t *testing.T) { + srv := New("acme", nil, nil, nil, vectordb.New(1)) + srv.SetDefaultTenant("globex") + if srv.defaultTenant != "globex" { + t.Fatalf(`SetDefaultTenant("globex") defaultTenant = %q, want "globex"`, srv.defaultTenant) + } + // Empty argument is a no-op so optional config doesn't clobber. + srv.SetDefaultTenant("") + if srv.defaultTenant != "globex" { + t.Fatalf(`SetDefaultTenant("") clobbered field to %q`, srv.defaultTenant) + } + }) +} + +// TestNew_DefaultTenant_FlowsThroughHTTPTransport proves that the constructor- +// supplied tenant is the actual fallback used by the JSON-RPC HTTP handler +// when no X-Tenant-ID header is present, and that an explicit header still +// wins over the default. This locks in the end-to-end behavior the RAN-22 +// fix delivers: a deployment with DEFAULT_TENANT=acme returns acme-scoped +// data from header-less MCP tool calls. +func TestNew_DefaultTenant_FlowsThroughHTTPTransport(t *testing.T) { + idx := vectordb.New(100) + idx.Add(1, "acme", "checkout", "ERROR", "payment gateway timeout acme-marker-xyz") + idx.Add(2, "globex", "auth", "ERROR", "payment gateway 500 globex-marker-qqq") + idx.Add(3, "default", "svc", "ERROR", "payment gateway refused default-marker-aaa") + + body := mustMarshalJSONRPC(t, "find_similar_logs", map[string]any{ + "query": "payment gateway", + "limit": float64(50), + }) + + srv := New("acme", nil, nil, nil, idx) + + // Header-less tools/call must scope to the constructor-provided default. + resp1 := callNoHeader(t, srv, body) + mustContain(t, resp1, "acme-marker-xyz") + mustNotContain(t, resp1, "globex-marker-qqq", "default-marker-aaa") + + // Explicit X-Tenant-ID header beats the configured default — precedence + // invariant is preserved. + resp2 := callWithHeader(t, srv, body, "globex") + mustContain(t, resp2, "globex-marker-qqq") + mustNotContain(t, resp2, "acme-marker-xyz", "default-marker-aaa") + + // SetDefaultTenant runtime override flows to the same transport path so + // future runtime-config-reload paths behave correctly. + srv.SetDefaultTenant("globex") + resp3 := callNoHeader(t, srv, body) + mustContain(t, resp3, "globex-marker-qqq") + mustNotContain(t, resp3, "acme-marker-xyz", "default-marker-aaa") +} + +func mustMarshalJSONRPC(t *testing.T, tool string, args map[string]any) []byte { + t.Helper() + b, err := json.Marshal(map[string]any{ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": map[string]any{"name": tool, "arguments": args}, + }) + if err != nil { + t.Fatalf("marshal: %v", err) + } + return b +} + +func callNoHeader(t *testing.T, srv *Server, body []byte) string { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/mcp", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + srv.Handler().ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("HTTP %d: %s", rr.Code, rr.Body.String()) + } + return rr.Body.String() +} + +func callWithHeader(t *testing.T, srv *Server, body []byte, tenant string) string { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/mcp", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Tenant-ID", tenant) + rr := httptest.NewRecorder() + srv.Handler().ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("HTTP %d: %s", rr.Code, rr.Body.String()) + } + return rr.Body.String() +} + +func mustContain(t *testing.T, body, want string) { + t.Helper() + if !strings.Contains(body, want) { + t.Fatalf("response missing expected marker %q:\n%s", want, body) + } +} + +func mustNotContain(t *testing.T, body string, forbidden ...string) { + t.Helper() + for _, f := range forbidden { + if strings.Contains(body, f) { + t.Fatalf("response leaked forbidden marker %q:\n%s", f, body) + } + } +} diff --git a/internal/mcp/tenant_isolation_test.go b/internal/mcp/tenant_isolation_test.go index 653ab90..bb4d5cc 100644 --- a/internal/mcp/tenant_isolation_test.go +++ b/internal/mcp/tenant_isolation_test.go @@ -94,7 +94,7 @@ func setupTenantIsolationServer(t *testing.T) (*httptest.Server, *graphrag.Graph bgCtx, cancel := context.WithCancel(context.Background()) go g.Start(bgCtx) - srv := New(repo, nil, nil, vIdx) + srv := New("", repo, nil, nil, vIdx) srv.SetGraphRAG(g) httpSrv := httptest.NewServer(srv.Handler()) diff --git a/main.go b/main.go index 83f96cb..bafd4dc 100644 --- a/main.go +++ b/main.go @@ -377,9 +377,9 @@ func main() { apiServer.SetVectorIndex(vectorIdx) // 6b. Initialize MCP Server (HTTP Streamable, JSON-RPC 2.0 + SSE) - mcpServer := mcp.New(repo, metrics, svcGraph, vectorIdx) + mcpServer := mcp.New(cfg.DefaultTenant, repo, metrics, svcGraph, vectorIdx) mcpServer.SetGraphRAG(graphRAG) - slog.Info("🤖 MCP server initialized", "path", cfg.MCPPath, "enabled", cfg.MCPEnabled) + slog.Info("🤖 MCP server initialized", "path", cfg.MCPPath, "enabled", cfg.MCPEnabled, "default_tenant", cfg.DefaultTenant) // 7. Initialize OTLP Ingestion (gRPC) traceServer := ingest.NewTraceServer(repo, metrics, cfg)