fix(cli): validate CSV headers, encoding and numeric bounds

CHANGELOG.md

@@ -47,6 +47,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Changed
 
+- BREAKING: `--input-csv` now requires `Term` and `PaymentOption` columns in
+  addition to `Service`, `Region`, `ResourceType` and `Count`. `Term` must be
+  `1yr` or `3yr` and `PaymentOption` must be `all-upfront`, `partial-upfront`
+  or `no-upfront` on every row. CSVs with only the older four columns, or with
+  a blank or unknown value, now fail to load with an error naming the missing
+  column or the row.
 - Split the CLI out of the CUDly monorepo into its own module,
   `github.com/LeanerCloud/cloud-commitments-cli`. The shared client and
   provider code it depends on now lives in

cmd/cancellation_prepurchase_test.go

@@ -51,7 +51,7 @@ func TestFilterAndAdjustRecommendations_CancellationAndDeadlineAreTerminal(t *te
 func TestPrepareCSVPurchaseRun_CanceledContextStopsBeforeConfirmationAndPurchase(t *testing.T) {
 	dir := t.TempDir()
 	csvPath := filepath.Join(dir, "in.csv")
-	require.NoError(t, os.WriteFile(csvPath, []byte("Service,Region,ResourceType,Count,Engine\nrds,us-east-1,db.t3.small,2,mysql\n"), 0o600))
+	require.NoError(t, os.WriteFile(csvPath, []byte("Service,Region,ResourceType,Count,Engine,Term,PaymentOption\nrds,us-east-1,db.t3.small,2,mysql,1yr,no-upfront\n"), 0o600))
 	cfg := Config{CSVInput: csvPath, AuditLog: filepath.Join(dir, "audit.jsonl")}
 
 	for _, dry := range []bool{true, false} {

cmd/multi_service_csv.go

@@ -13,6 +13,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations"
@@ -48,9 +49,14 @@ func loadRecommendationsFromCSV(csvPath string) ([]common.Recommendation, error)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read CSV header: %w", err)
 	}
+	if err = validateCSVUTF8(header); err != nil {
+		return nil, fmt.Errorf("CSV header: %w", err)
+	}
 
-	// Build column index map
-	colIdx := buildColumnIndexMap(header)
+	colIdx, err := buildColumnIndexMap(header)
+	if err != nil {
+		return nil, err
+	}
 
 	// Parse all records
 	parsed, err := parseCSVRecords(reader, colIdx)
@@ -61,13 +67,60 @@ func loadRecommendationsFromCSV(csvPath string) ([]common.Recommendation, error)
 	return parsed, nil
 }
 
-// buildColumnIndexMap creates a map from column names to indices.
-func buildColumnIndexMap(header []string) map[string]int {
+// Minimal CSVs require these columns; service-specific columns stay optional.
+// Term and PaymentOption are required because the purchase path reads them per
+// row and must never guess a commitment's term or payment option.
+var requiredCSVColumns = []string{"Service", "Region", "ResourceType", "Count", "Term", "PaymentOption"}
+
+// Accepted Term values match fetchRecommendationsForRegion; accepted
+// PaymentOption values match validatePaymentAndTerm.
+var (
+	validCSVTerms          = []string{"1yr", "3yr"}
+	validCSVPaymentOptions = []string{"all-upfront", "partial-upfront", "no-upfront"}
+)
+
+// parseCSVEnum requires the cell to be exactly one of allowed.
+func parseCSVEnum(record []string, colIdx map[string]int, fieldName string, allowed []string) (string, error) {
+	col := colIdx[fieldName] + 1
+	value := getCSVField(record, colIdx, fieldName)
+	if strings.TrimSpace(value) == "" {
+		return "", fmt.Errorf("column %d %q: value is required (one of: %s)", col, fieldName, strings.Join(allowed, ", "))
+	}
+	for _, a := range allowed {
+		if value == a {
+			return value, nil
+		}
+	}
+	return "", fmt.Errorf("column %d %q: invalid value %q (must be one of: %s)", col, fieldName, value, strings.Join(allowed, ", "))
+}
+
+func validateCSVUTF8(fields []string) error {
+	for i, field := range fields {
+		if !utf8.ValidString(field) {
+			return fmt.Errorf("column %d: invalid UTF-8 encoding", i+1)
+		}
+	}
+	return nil
+}
+
+func buildColumnIndexMap(header []string) (map[string]int, error) {
+	if len(header) > 0 {
+		header[0] = strings.TrimPrefix(header[0], "\ufeff")
+	}
 	colIdx := make(map[string]int)
 	for i, col := range header {
 		colIdx[col] = i
 	}
-	return colIdx
+	var missing []string
+	for _, req := range requiredCSVColumns {
+		if _, ok := colIdx[req]; !ok {
+			missing = append(missing, req)
+		}
+	}
+	if len(missing) > 0 {
+		return nil, fmt.Errorf("CSV header missing required columns: %s", strings.Join(missing, ", "))
+	}
+	return colIdx, nil
 }
 
 // parseCSVRecords reads and parses all CSV records.
@@ -82,6 +135,10 @@ func parseCSVRecords(reader *csv.Reader, colIdx map[string]int) ([]common.Recomm
 		if err != nil {
 			return nil, fmt.Errorf("failed to read CSV record: %w", err)
 		}
+		if err = validateCSVUTF8(record); err != nil {
+			line, _ := reader.FieldPos(0)
+			return nil, fmt.Errorf("CSV line %d: %w", line, err)
+		}
 
 		// Skip the trailing TOTAL summary row that writeMultiServiceCSVReport
 		// emits (label in the Service column). Without this, feeding the tool
@@ -113,14 +170,25 @@ func parseCSVRecord(record []string, colIdx map[string]int) (common.Recommendati
 	rec.ResourceType = getCSVField(record, colIdx, "ResourceType")
 	rec.Account = getCSVField(record, colIdx, "Account")
 	rec.AccountName = getCSVField(record, colIdx, "AccountName")
-	rec.Term = getCSVField(record, colIdx, "Term")
-	rec.PaymentOption = getCSVField(record, colIdx, "PaymentOption")
 
-	if err := parseCSVCount(record, colIdx, &rec.Count); err != nil {
+	term, err := parseCSVEnum(record, colIdx, "Term", validCSVTerms)
+	if err != nil {
+		return rec, err
+	}
+	rec.Term = term
+	payment, err := parseCSVEnum(record, colIdx, "PaymentOption", validCSVPaymentOptions)
+	if err != nil {
+		return rec, err
+	}
+	rec.PaymentOption = payment
+
+	err = parseCSVCount(record, colIdx, &rec.Count)
+	if err != nil {
 		return rec, err
 	}
 
-	if err := parseCSVFloat(record, colIdx, "EstimatedSavings", &rec.EstimatedSavings); err != nil {
+	err = parseCSVFloat(record, colIdx, "EstimatedSavings", &rec.EstimatedSavings)
+	if err != nil {
 		return rec, err
 	}
 
@@ -143,9 +211,7 @@ func getCSVField(record []string, colIdx map[string]int, fieldName string) strin
 	return ""
 }
 
-// parseCSVCount parses the required Count field as a whole non-negative
-// integer. It drives purchase quantities, so a blank, missing, fractional or
-// otherwise malformed cell is an error rather than a truncated or zero value.
+// Savings Plans exports use Count=1, preserving positive-count round trips.
 func parseCSVCount(record []string, colIdx map[string]int, target *int) error {
 	const fieldName = "Count"
 	idx, ok := colIdx[fieldName]
@@ -158,17 +224,16 @@ func parseCSVCount(record []string, colIdx map[string]int, target *int) error {
 	}
 	n, err := strconv.Atoi(value)
 	if err != nil {
-		return fmt.Errorf("column %d %q: invalid integer %q: %w", idx+1, fieldName, value, err)
+		return fmt.Errorf("column %d %q: invalid integer: %w", idx+1, fieldName, err)
 	}
-	if n < 0 {
-		return fmt.Errorf("column %d %q: must not be negative, got %d", idx+1, fieldName, n)
+	if n < 1 {
+		return fmt.Errorf("column %d %q: must be at least 1, got %d", idx+1, fieldName, n)
 	}
 	*target = n
 	return nil
 }
 
-// parseCSVFloat parses a finite float field from a CSV record. A blank or
-// absent cell leaves target untouched (see requireRankingSignal).
+// Blank savings stay absent-as-zero for requireRankingSignal.
 func parseCSVFloat(record []string, colIdx map[string]int, fieldName string, target *float64) error {
 	value := strings.TrimSpace(getCSVField(record, colIdx, fieldName))
 	if value == "" {
@@ -177,11 +242,14 @@ func parseCSVFloat(record []string, colIdx map[string]int, fieldName string, tar
 	col := colIdx[fieldName] + 1
 	f, err := strconv.ParseFloat(value, 64)
 	if err != nil {
-		return fmt.Errorf("column %d %q: invalid number %q: %w", col, fieldName, value, err)
+		return fmt.Errorf("column %d %q: invalid number: %w", col, fieldName, err)
 	}
 	if math.IsNaN(f) || math.IsInf(f, 0) {
 		return fmt.Errorf("column %d %q: invalid number %q: must be finite", col, fieldName, value)
 	}
+	if f < 0 {
+		return fmt.Errorf("column %d %q: must not be negative, got %g", col, fieldName, f)
+	}
 	*target = f
 	return nil
 }

cmd/multi_service_csv_header_test.go

@@ -0,0 +1,151 @@
+package main
+
+import (
+	"encoding/binary"
+	"testing"
+	"unicode/utf16"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLoadRecommendationsFromCSV_HeaderValidation_1327(t *testing.T) {
+	tests := []struct {
+		name        string
+		header      string
+		row         string
+		errContains string
+	}{
+		{"missing Service", "Region,ResourceType,Count,Term,PaymentOption\n", "us-east-1,db.t3.micro,2,3yr,partial-upfront\n", "Service"},
+		{"missing Region", "Service,ResourceType,Count,Term,PaymentOption\n", "rds,db.t3.micro,2,3yr,partial-upfront\n", "Region"},
+		{"missing ResourceType", "Service,Region,Count,Term,PaymentOption\n", "rds,us-east-1,2,3yr,partial-upfront\n", "ResourceType"},
+		{"missing Count", "Service,Region,ResourceType,Term,PaymentOption\n", "rds,us-east-1,db.t3.micro,3yr,partial-upfront\n", "Count"},
+		{"missing Term", "Service,Region,ResourceType,Count,PaymentOption\n", "rds,us-east-1,db.t3.micro,2,partial-upfront\n", "Term"},
+		{"missing PaymentOption", "Service,Region,ResourceType,Count,Term\n", "rds,us-east-1,db.t3.micro,2,3yr\n", "PaymentOption"},
+		{"legacy 4-column header rejected", "Service,Region,ResourceType,Count\n", "rds,us-east-1,db.t3.micro,2\n", "Term, PaymentOption"},
+		{"multiple missing", "Service,Count\n", "rds,2\n", "Region, ResourceType, Term, PaymentOption"},
+		{
+			"legacy TEST-02 headers rejected",
+			"Service,Region,Instance Type,Instance Count\n",
+			"rds,us-east-1,db.t3.micro,2\n",
+			"ResourceType, Count, Term, PaymentOption",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := loadCSVContent(t, tt.header+tt.row)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "CSV header missing required columns")
+			assert.Contains(t, err.Error(), tt.errContains)
+		})
+	}
+}
+
+func TestLoadRecommendationsFromCSV_BOMPrefixed_1327(t *testing.T) {
+	recs, err := loadRecommendationsFromCSV(writeTestRecommendationsCSV(t,
+		"\ufeffService,Region,ResourceType,Count,Term,PaymentOption\nrds,us-east-1,db.t3.micro,2,3yr,partial-upfront\n"))
+	require.NoError(t, err)
+	require.Len(t, recs, 1)
+	assert.Equal(t, common.ServiceRDS, recs[0].Service)
+	assert.Equal(t, "us-east-1", recs[0].Region)
+	assert.Equal(t, 2, recs[0].Count)
+}
+
+func TestLoadRecommendationsFromCSV_Encoding_1327(t *testing.T) {
+	t.Run("UTF-16", func(t *testing.T) {
+		units := utf16.Encode([]rune("Service,Region,ResourceType,Count,Term,PaymentOption\nrds,us-east-1,db.t3.micro,2,3yr,partial-upfront\n"))
+		for _, tt := range []struct {
+			name  string
+			bom   []byte
+			order binary.ByteOrder
+		}{
+			{"little endian", []byte{0xff, 0xfe}, binary.LittleEndian},
+			{"big endian", []byte{0xfe, 0xff}, binary.BigEndian},
+		} {
+			t.Run(tt.name, func(t *testing.T) {
+				encoded := make([]byte, 2+2*len(units))
+				copy(encoded, tt.bom)
+				for i, unit := range units {
+					tt.order.PutUint16(encoded[2+2*i:], unit)
+				}
+				err := loadCSVContent(t, string(encoded))
+				require.Error(t, err)
+				assert.EqualError(t, err, "CSV header: column 1: invalid UTF-8 encoding")
+			})
+		}
+	})
+
+	t.Run("Latin-1 optional header", func(t *testing.T) {
+		err := loadCSVContent(t, "Service,Region,ResourceType,Count,Caf\xe9,Term,PaymentOption\n"+
+			"rds,us-east-1,db.t3.micro,2,prod,3yr,partial-upfront\n")
+		require.Error(t, err)
+		assert.EqualError(t, err, "CSV header: column 5: invalid UTF-8 encoding")
+	})
+
+	t.Run("Latin-1 AccountName", func(t *testing.T) {
+		err := loadCSVContent(t, "Service,Region,ResourceType,Count,AccountName,Term,PaymentOption\n"+
+			"rds,us-east-1,db.t3.micro,2,Caf\xe9,3yr,partial-upfront\n")
+		require.Error(t, err)
+		assert.EqualError(t, err, "CSV line 2: column 5: invalid UTF-8 encoding")
+	})
+
+	t.Run("invalid ignored field in TOTAL after multiline record", func(t *testing.T) {
+		err := loadCSVContent(t, "Service,Region,ResourceType,Count,Ignored,Term,PaymentOption\n"+
+			"rds,us-east-1,db.t3.micro,2,\"prod\naccount\",3yr,partial-upfront\n"+
+			"TOTAL,,,2,\xff,,\n")
+		require.Error(t, err)
+		assert.EqualError(t, err, "CSV line 4: column 5: invalid UTF-8 encoding")
+	})
+
+	t.Run("valid non-ASCII UTF-8", func(t *testing.T) {
+		recs, err := loadRecommendationsFromCSV(writeTestRecommendationsCSV(t,
+			"Service,Region,ResourceType,Count,AccountName,Term,PaymentOption\n"+
+				"rds,us-east-1,db.t3.micro,2,Café 東京,3yr,partial-upfront\n"))
+		require.NoError(t, err)
+		require.Len(t, recs, 1)
+		assert.Equal(t, "Café 東京", recs[0].AccountName)
+	})
+}
+
+// Lock in encoding/csv behaviors the purchase path relies on.
+func TestLoadRecommendationsFromCSV_EdgeCases_1327(t *testing.T) {
+	t.Run("CRLF line endings", func(t *testing.T) {
+		recs, err := loadRecommendationsFromCSV(writeTestRecommendationsCSV(t,
+			"Service,Region,ResourceType,Count,Term,PaymentOption\r\nrds,us-east-1,db.t3.micro,2,3yr,partial-upfront\r\n"))
+		require.NoError(t, err)
+		require.Len(t, recs, 1)
+		assert.Equal(t, 2, recs[0].Count)
+	})
+
+	t.Run("quoted commas and newlines inside fields", func(t *testing.T) {
+		recs, err := loadRecommendationsFromCSV(writeTestRecommendationsCSV(t,
+			"Service,Region,ResourceType,Count,AccountName,Term,PaymentOption\n"+
+				"rds,us-east-1,\"db.t3.micro, burstable\",2,\"prod\naccount\",3yr,partial-upfront\n"))
+		require.NoError(t, err)
+		require.Len(t, recs, 1)
+		assert.Equal(t, "db.t3.micro, burstable", recs[0].ResourceType)
+		assert.Equal(t, "prod\naccount", recs[0].AccountName)
+	})
+
+	t.Run("headers only loads zero recs", func(t *testing.T) {
+		recs, err := loadRecommendationsFromCSV(writeTestRecommendationsCSV(t,
+			"Service,Region,ResourceType,Count,Term,PaymentOption\n"))
+		require.NoError(t, err)
+		assert.Empty(t, recs)
+	})
+
+	t.Run("wrong field count on a data row", func(t *testing.T) {
+		err := loadCSVContent(t,
+			"Service,Region,ResourceType,Count,Term,PaymentOption\nrds,us-east-1,db.t3.micro\n")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "failed to read CSV record")
+	})
+
+	t.Run("TOTAL row still skipped with full header", func(t *testing.T) {
+		recs, err := loadRecommendationsFromCSV(writeTestRecommendationsCSV(t,
+			"Service,Region,ResourceType,Count,Term,PaymentOption\nrds,us-east-1,db.t3.micro,2,3yr,partial-upfront\nTOTAL,,,2,,\n"))
+		require.NoError(t, err)
+		require.Len(t, recs, 1)
+	})
+}