Summary

loadRecommendationsFromCSV() in cmd/multi_service.go:166-230 reads Term and PaymentOption directly from the CSV without any allowlist validation. validateFlags() checks these fields for CLI flags but is not called for CSV-supplied values.

Location

cmd/multi_service.go:218-225 (CSV field parsing for Term and PaymentOption)
cmd/multi_service.go:119-241 (validateFlags — validates CLI flags only, not CSV content)

Impact

A user who edits the output CSV and enters a typo (e.g., 3-year instead of 3yr, or partial_upfront instead of partial-upfront) will receive a confusing deep AWS API error like invalid payment option from findOfferingID/convertPaymentOption rather than a clear validation message at load time. The Count field uses fmt.Sscanf without range checks, so a value like 99999 would pass unnoticed and trigger purchases far beyond --max-instances intent (since filterAndAdjustRecommendations only applies MaxInstances as an instance-count cap, not per-recommendation count cap).

This is not a remote code execution risk (the tool runs locally with the user's own credentials), but it violates the principle that --input-csv is a trusted-but-validated input path, and can lead to unintended purchases.

Reproduction

1. Run ri-helper --services rds to produce an output CSV.
2. Edit the CSV: change PaymentOption from no-upfront to no_upfront and Term from 1yr to 1-year.
3. Run ri-helper --input-csv <file> --purchase.
4. Observe: validateFlags passes (no CLI flags set), but the purchase attempt fails with a cryptic error from deep inside convertPaymentOption.

Suggested Fix

Add a validateCSVRecord helper called from loadRecommendationsFromCSV (or from filterAndAdjustRecommendations) that checks each row's Term and PaymentOption against the same allowlists used in validateFlags, and that validates Count is > 0 and <= MaxReasonableInstances. Log and skip invalid rows with a clear message.

func validateCSVRecord(rec common.Recommendation) error {
    validPayment := map[string]bool{"all-upfront": true, "partial-upfront": true, "no-upfront": true}
    if !validPayment[rec.PaymentOption] {
        return fmt.Errorf("invalid PaymentOption %q", rec.PaymentOption)
    }
    if rec.Term != "1yr" && rec.Term != "3yr" {
        return fmt.Errorf("invalid Term %q (expected 1yr or 3yr)", rec.Term)
    }
    if rec.Count <= 0 || rec.Count > MaxReasonableInstances {
        return fmt.Errorf("Count %d out of range [1, %d]", rec.Count, MaxReasonableInstances)
    }
    return nil
}

Severity Rationale

severity/low — Affects only users who hand-edit the CSV input, causes confusing errors rather than incorrect purchases (the API rejects bad offering parameters).

Findings from the 2026-09-02 codebase audit

Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.

A10-007 (medium)

The same boundary gap applies to a third column this issue does not name. parseCSVRecord (cmd/multi_service_csv.go:106, the current home of the code) casts an arbitrary Service cell straight to common.ServiceType, bypassing the serviceMap at cmd/main.go:190. Even a valid flag alias such as elasticsearch, or rds with a trailing space, produces a ServiceType that createServiceClient's switch cannot match (cmd/main.go:244-268), so the row is dropped at cmd/multi_service.go:538-541 with 'Service client not yet implemented' rather than an error naming the bad cell. The operator sees a short run and no failure. Worth covering by the same validateCSVRecord helper. One narrowing on the PaymentOption half: every AWS provider converts and errors on an unknown value before any purchase, so the bad value produces a deep error rather than a wrong purchase. Audit finding A10-007.