fix(mcp): refuse multi-operation documents and resolve fragments in checks

internal/mcp/tools/graphql.go

@@ -200,7 +200,7 @@ func registerGraphQLTools(s *server.MCPServer, ctx *toolContext) {
 
 IMPORTANT: Before using this tool, use the schema exploration tools (schema_list_queries, schema_get_type, schema_get_field) to understand the available types and fields.
 
-This tool only supports queries (read operations). Mutations are not allowed.
+This tool only supports a single query (read operation) per request. Mutations, subscriptions and documents with multiple operations are not allowed.
 
 `+naisAPIGuidance),
 		mcp.WithInputSchema[ExecuteGraphQLInput](),
@@ -417,8 +417,12 @@ var forbiddenTypes = map[string]bool{
 }
 
 // checkForSecrets recursively checks if a selection set accesses any forbidden secret-related types.
-// It validates against the GraphQL schema to ensure queries don't access Secret or SecretValue types.
+// Named fragment spreads are resolved and traversed, so fragments cannot be used to bypass the check.
 func checkForSecrets(selectionSet ast.SelectionSet, schema *ast.Schema) (bool, string) {
+	return checkSelectionsForSecrets(selectionSet, map[string]bool{})
+}
+
+func checkSelectionsForSecrets(selectionSet ast.SelectionSet, visited map[string]bool) (bool, string) {
 	for _, selection := range selectionSet {
 		switch sel := selection.(type) {
 		case *ast.Field:
@@ -433,7 +437,7 @@ func checkForSecrets(selectionSet ast.SelectionSet, schema *ast.Schema) (bool, s
 
 			// Recursively check nested selections
 			if len(sel.SelectionSet) > 0 {
-				if found, reason := checkForSecrets(sel.SelectionSet, schema); found {
+				if found, reason := checkSelectionsForSecrets(sel.SelectionSet, visited); found {
 					return true, reason
 				}
 			}
@@ -443,14 +447,23 @@ func checkForSecrets(selectionSet ast.SelectionSet, schema *ast.Schema) (bool, s
 				return true, fmt.Sprintf("MCP security policy: inline fragment on type '%s' which contains sensitive data that cannot be accessed via this interface", sel.TypeCondition)
 			}
 			// Check inline fragments recursively
-			if found, reason := checkForSecrets(sel.SelectionSet, schema); found {
+			if found, reason := checkSelectionsForSecrets(sel.SelectionSet, visited); found {
 				return true, reason
 			}
 		case *ast.FragmentSpread:
-			// Fragment spreads would need fragment definitions to be fully validated
-			// For now, we flag any fragment that has "secret" in its name as a heuristic
-			if strings.Contains(strings.ToLower(sel.Name), "secret") {
-				return true, fmt.Sprintf("MCP security policy: fragment '%s' may access sensitive data that cannot be accessed via this interface", sel.Name)
+			// Unresolved spreads are rejected by schema validation; refuse them here as well to fail closed.
+			if sel.Definition == nil {
+				return true, fmt.Sprintf("MCP security policy: fragment '%s' could not be resolved", sel.Name)
+			}
+			if visited[sel.Name] {
+				continue
+			}
+			visited[sel.Name] = true
+			if forbiddenTypes[sel.Definition.TypeCondition] {
+				return true, fmt.Sprintf("MCP security policy: fragment '%s' on type '%s' which contains sensitive data that cannot be accessed via this interface", sel.Name, sel.Definition.TypeCondition)
+			}
+			if found, reason := checkSelectionsForSecrets(sel.Definition.SelectionSet, visited); found {
+				return true, reason
 			}
 		}
 	}
@@ -479,75 +492,90 @@ func (t *toolContext) validateGraphQLQuery(reqCtx context.Context, query string)
 		return nil, fmt.Errorf("failed to parse schema: %v", gqlErr)
 	}
 
-	// Parse the query
+	return validateQueryDocument(schema, query), nil
+}
+
+// validateQueryDocument validates a query string against a parsed schema and the MCP security policy:
+// exactly one operation, which must be a query, within the depth limit and without access to secrets.
+// Named fragments are resolved for both the depth and the secrets checks.
+func validateQueryDocument(schema *ast.Schema, query string) *queryValidationResult {
 	doc, errList := gqlparser.LoadQueryWithRules(schema, query, nil)
 	if len(errList) > 0 {
 		return &queryValidationResult{
 			valid: false,
 			error: errList.Error(),
-		}, nil
+		}
 	}
 
-	// Check that we have at least one operation
 	if len(doc.Operations) == 0 {
 		return &queryValidationResult{
 			valid: false,
 			error: "no operations found in query",
-		}, nil
+		}
+	}
+
+	// Refuse documents with multiple operations, so the operation checked is always the one executed.
+	if len(doc.Operations) > 1 {
+		return &queryValidationResult{
+			valid: false,
+			error: fmt.Sprintf("only a single operation is allowed per request, got %d", len(doc.Operations)),
+		}
 	}
 
-	// Check operation type - only allow queries
-	// We only validate the first operation
 	op := doc.Operations[0]
 	if op.Operation != ast.Query {
 		return &queryValidationResult{
 			valid: false,
 			error: fmt.Sprintf("only query operations are allowed, got: %s", op.Operation),
-		}, nil
+		}
 	}
 
-	// Check query depth
 	depth := calculateQueryDepth(op.SelectionSet, 0)
 	if depth > maxQueryDepth {
 		return &queryValidationResult{
 			valid: false,
 			error: fmt.Sprintf("query depth %d exceeds maximum allowed depth of %d", depth, maxQueryDepth),
-		}, nil
+		}
 	}
 
-	// Check for forbidden secret-related types and fields
 	if found, reason := checkForSecrets(op.SelectionSet, schema); found {
 		return &queryValidationResult{
 			valid: false,
 			error: reason,
-		}, nil
+		}
 	}
 
 	return &queryValidationResult{
 		valid:         true,
 		operationType: string(op.Operation),
 		operationName: op.Name,
 		depth:         depth,
-	}, nil
+	}
 }
 
+// calculateQueryDepth returns the maximum field nesting depth of a selection set.
+// Named fragment spreads are resolved and counted as if their selections were inlined.
 func calculateQueryDepth(selectionSet ast.SelectionSet, currentDepth int) int {
-	if len(selectionSet) == 0 {
-		return currentDepth
-	}
+	return selectionDepth(selectionSet, currentDepth, map[string]bool{})
+}
 
+func selectionDepth(selectionSet ast.SelectionSet, currentDepth int, inProgress map[string]bool) int {
 	maxDepth := currentDepth
 	for _, selection := range selectionSet {
 		var childDepth int
 		switch sel := selection.(type) {
 		case *ast.Field:
-			childDepth = calculateQueryDepth(sel.SelectionSet, currentDepth+1)
+			childDepth = selectionDepth(sel.SelectionSet, currentDepth+1, inProgress)
 		case *ast.InlineFragment:
-			childDepth = calculateQueryDepth(sel.SelectionSet, currentDepth)
+			childDepth = selectionDepth(sel.SelectionSet, currentDepth, inProgress)
 		case *ast.FragmentSpread:
-			// Fragment spreads would need to be resolved against fragment definitions
-			// For simplicity, we count them as +1 depth
-			childDepth = currentDepth + 1
+			// Cycles are rejected by schema validation; guard anyway to avoid infinite recursion.
+			if sel.Definition == nil || inProgress[sel.Name] {
+				continue
+			}
+			inProgress[sel.Name] = true
+			childDepth = selectionDepth(sel.Definition.SelectionSet, currentDepth, inProgress)
+			delete(inProgress, sel.Name)
 		}
 		if childDepth > maxDepth {
 			maxDepth = childDepth

internal/mcp/tools/graphql_validation_test.go

@@ -0,0 +1,100 @@
+package tools
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+)
+
+func TestValidateQueryDocument(t *testing.T) {
+	schema := loadRealSchema(t)
+
+	tests := []struct {
+		name      string
+		query     string
+		wantValid bool
+		wantErr   string
+	}{
+		{
+			name:      "single query is allowed",
+			query:     `query { team(slug: "a") { slug } }`,
+			wantValid: true,
+		},
+		{
+			name:      "named fragment without secrets is allowed",
+			query:     `query { team(slug: "a") { ...TeamFields } } fragment TeamFields on Team { slug purpose }`,
+			wantValid: true,
+		},
+		{
+			name:    "mutation is refused",
+			query:   `mutation { restartApplication(input: {name: "a", teamSlug: "b", environmentName: "c"}) { __typename } }`,
+			wantErr: "only query operations are allowed",
+		},
+		{
+			name: "mutation after query in same document is refused",
+			query: `
+				query Harmless { team(slug: "a") { slug } }
+				mutation Evil { restartApplication(input: {name: "a", teamSlug: "b", environmentName: "c"}) { __typename } }
+			`,
+			wantErr: "only a single operation is allowed",
+		},
+		{
+			name: "multiple queries are refused",
+			query: `
+				query A { team(slug: "a") { slug } }
+				query B { team(slug: "b") { slug } }
+			`,
+			wantErr: "only a single operation is allowed",
+		},
+		{
+			name:    "secrets accessed through named fragment are refused",
+			query:   `query { team(slug: "a") { ...Innocent } } fragment Innocent on Team { secrets(first: 1) { nodes { name } } }`,
+			wantErr: "field 'secrets' returns type 'SecretConnection'",
+		},
+		{
+			name: "secrets accessed through nested named fragments are refused",
+			query: `
+				query { team(slug: "a") { ...Outer } }
+				fragment Outer on Team { slug ...Inner }
+				fragment Inner on Team { secrets(first: 1) { nodes { name } } }
+			`,
+			wantErr: "field 'secrets' returns type 'SecretConnection'",
+		},
+		{
+			name:    "fragment on forbidden type is refused",
+			query:   `query { team(slug: "a") { secrets(first: 1) { nodes { ...S } } } } fragment S on Secret { name }`,
+			wantErr: "MCP security policy",
+		},
+		{
+			name:    "depth hidden in named fragments is counted",
+			query:   `query { team(slug: "a") { ...F1 } }` + deepFragments(20),
+			wantErr: "exceeds maximum allowed depth",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			res := validateQueryDocument(schema, tt.query)
+			if res.valid != tt.wantValid {
+				t.Fatalf("valid = %v, want %v (error: %q)", res.valid, tt.wantValid, res.error)
+			}
+			if tt.wantErr != "" && !strings.Contains(res.error, tt.wantErr) {
+				t.Errorf("error = %q, want it to contain %q", res.error, tt.wantErr)
+			}
+		})
+	}
+}
+
+// deepFragments builds a chain of n fragments on Team, each adding nesting via
+// team.environments.team, so the total depth is only visible when fragments are resolved.
+func deepFragments(n int) string {
+	var b strings.Builder
+	for i := 1; i <= n; i++ {
+		if i < n {
+			fmt.Fprintf(&b, "\nfragment F%d on Team { slug environments { team { ...F%d } } }", i, i+1)
+		} else {
+			fmt.Fprintf(&b, "\nfragment F%d on Team { slug }", i)
+		}
+	}
+	return b.String()
+}