fix(mcp): Add nil checks to prevent panic in cloud cost transformation (#3511)

Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>

pkg/mcp/server.go

@@ -868,6 +868,13 @@ func transformCloudCostSetRange(ccsr *opencost.CloudCostSetRange) *CloudCostResp
 	// Process each cloud cost set in the range
 	for i, ccSet := range ccsr.CloudCostSets {
 		if ccSet == nil {
+			log.Warnf("transformCloudCostSetRange: skipping nil CloudCostSet at index %d", i)
+			continue
+		}
+
+		// Check for nil Window or nil Start/End pointers before dereferencing
+		if ccSet.Window.Start() == nil || ccSet.Window.End() == nil {
+			log.Warnf("transformCloudCostSetRange: skipping CloudCostSet at index %d with invalid window (start=%v, end=%v)", i, ccSet.Window.Start(), ccSet.Window.End())
 			continue
 		}
 
@@ -885,6 +892,13 @@ func transformCloudCostSetRange(ccsr *opencost.CloudCostSetRange) *CloudCostResp
 		// Convert each cloud cost item
 		for _, item := range ccSet.CloudCosts {
 			if item == nil {
+				log.Warnf("transformCloudCostSetRange: skipping nil CloudCost item in set %s", setName)
+				continue
+			}
+
+			// Check for nil Window or nil Start/End pointers on the item
+			if item.Window.Start() == nil || item.Window.End() == nil {
+				log.Warnf("transformCloudCostSetRange: skipping CloudCost item with invalid window (start=%v, end=%v) in set %s", item.Window.Start(), item.Window.End(), setName)
 				continue
 			}
 
@@ -947,8 +961,23 @@ func transformCloudCostSetRange(ccsr *opencost.CloudCostSetRange) *CloudCostResp
 	var avgKubernetesPercent float64
 	var numerator, denominator float64
 	for _, ccSet := range ccsr.CloudCostSets {
+		if ccSet == nil {
+			log.Warnf("transformCloudCostSetRange: skipping nil CloudCostSet in Kubernetes percent calculation")
+			continue
+		}
+		// Skip sets with invalid windows (consistent with first loop)
+		if ccSet.Window.Start() == nil || ccSet.Window.End() == nil {
+			log.Warnf("transformCloudCostSetRange: skipping CloudCostSet with invalid window (start=%v, end=%v) in Kubernetes percent calculation", ccSet.Window.Start(), ccSet.Window.End())
+			continue
+		}
 		for _, item := range ccSet.CloudCosts {
 			if item == nil {
+				log.Warnf("transformCloudCostSetRange: skipping nil CloudCost item in Kubernetes percent calculation")
+				continue
+			}
+			// Skip items with invalid windows (consistent with first loop)
+			if item.Window.Start() == nil || item.Window.End() == nil {
+				log.Warnf("transformCloudCostSetRange: skipping CloudCost item with invalid window (start=%v, end=%v) in Kubernetes percent calculation", item.Window.Start(), item.Window.End())
 				continue
 			}
 			cost := item.NetCost.Cost

pkg/mcp/server_test.go

@@ -1359,3 +1359,106 @@ func TestEfficiencyConstants(t *testing.T) {
 func TestEfficiencyQueryType(t *testing.T) {
 	assert.Equal(t, QueryType("efficiency"), EfficiencyQueryType)
 }
+
+// TestTransformCloudCostSetRange_NilPointerHandling verifies that nil pointer dereferences
+// are prevented in transformCloudCostSetRange for issue #3502
+func TestTransformCloudCostSetRange_NilPointerHandling(t *testing.T) {
+	now := time.Now().UTC()
+	start := now.Add(-24 * time.Hour)
+	end := now
+
+	tests := []struct {
+		name              string
+		ccsr              *opencost.CloudCostSetRange
+		expectedCostCount int
+		expectEmpty       bool
+	}{
+		{
+			name:        "nil CloudCostSetRange",
+			ccsr:        nil,
+			expectEmpty: true,
+		},
+		{
+			name:        "nil CloudCostSet in slice",
+			ccsr:        &opencost.CloudCostSetRange{CloudCostSets: []*opencost.CloudCostSet{nil}},
+			expectEmpty: true,
+		},
+		{
+			name: "CloudCostSet with nil Window.Start",
+			ccsr: &opencost.CloudCostSetRange{
+				CloudCostSets: []*opencost.CloudCostSet{
+					{CloudCosts: map[string]*opencost.CloudCost{}, Window: opencost.NewWindow(nil, &end)},
+				},
+			},
+			expectEmpty: true,
+		},
+		{
+			name: "CloudCostSet with nil Window.End",
+			ccsr: &opencost.CloudCostSetRange{
+				CloudCostSets: []*opencost.CloudCostSet{
+					{CloudCosts: map[string]*opencost.CloudCost{}, Window: opencost.NewWindow(&start, nil)},
+				},
+			},
+			expectEmpty: true,
+		},
+		{
+			name: "CloudCost item with nil Window.Start",
+			ccsr: &opencost.CloudCostSetRange{
+				CloudCostSets: []*opencost.CloudCostSet{
+					{
+						CloudCosts: map[string]*opencost.CloudCost{
+							"cost1": {
+								Properties: &opencost.CloudCostProperties{Provider: "aws"},
+								Window:     opencost.NewWindow(nil, &end),
+								NetCost:    opencost.CostMetric{Cost: 100.0},
+							},
+						},
+						Window: opencost.NewClosedWindow(start, end),
+					},
+				},
+			},
+			expectedCostCount: 0,
+		},
+		{
+			name: "Mixed valid and invalid items",
+			ccsr: &opencost.CloudCostSetRange{
+				CloudCostSets: []*opencost.CloudCostSet{
+					{
+						CloudCosts: map[string]*opencost.CloudCost{
+							"invalid": nil,
+							"valid": {
+								Properties: &opencost.CloudCostProperties{Provider: "gcp"},
+								Window:     opencost.NewClosedWindow(start, end),
+								NetCost:    opencost.CostMetric{Cost: 200.0, KubernetesPercent: 0.5},
+							},
+						},
+						Window: opencost.NewClosedWindow(start, end),
+					},
+				},
+			},
+			expectedCostCount: 1,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Should not panic
+			result := transformCloudCostSetRange(tt.ccsr)
+
+			require.NotNil(t, result)
+			require.NotNil(t, result.CloudCosts)
+			require.NotNil(t, result.Summary)
+
+			if tt.expectEmpty {
+				assert.Empty(t, result.CloudCosts)
+				assert.Equal(t, 0.0, result.Summary.TotalNetCost)
+			} else {
+				totalCosts := 0
+				for _, set := range result.CloudCosts {
+					totalCosts += len(set.CloudCosts)
+				}
+				assert.Equal(t, tt.expectedCostCount, totalCosts)
+			}
+		})
+	}
+}