Repository navigation
fix(cli): fmt.Sscanf CSV parsing truncates 3.7 to 3 and accepts trailing garbage #1944
Description
Summary
parseCSVInt and its float sibling parse operator-supplied CSV cells with fmt.Sscanf, which stops at the first character it cannot consume and still reports success. Money quantities read from an operator-editable file therefore pass validation with wrong values, contrary to the project's strict-integer-parsing rule. The parsed counts flow into savingsPerInstance, ApplyInstanceLimit and the purchase loop.
Location
cmd/multi_service_csv.go:167-190 at 3c0f8ac
Failure scenario
A Count cell reading 3.7 parses as 3 with err == nil. 12 units parses as 12. -5 parses as -5 and reaches the purchase loop as a negative count. An EstimatedSavings cell of 1000 USD becomes 1000. All four were run against the pinned toolchain and behave exactly this way. No boundary rejection exists for any of them.
Evidence
func parseCSVInt(record []string, colIdx map[string]int, fieldName string, target *int) error {
value := getCSVField(record, colIdx, fieldName)
if value == "" { return nil }
if _, err := fmt.Sscanf(value, "%d", target); err != nil {
return fmt.Errorf("invalid %s value '%s': %w", fieldName, value, err)
}
return nil
}Suggested fix
Parse the whole trimmed cell with strconv.Atoi and strconv.ParseFloat, which reject trailing characters, and reject a negative Count explicitly.
Found by the 2026-09-02 codebase audit, finding A10-006, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.
Activity
cristim commentedon Sep 29, 2026
Verified after merge: the production parser fix landed in #2114, and #2118 added orchestration regression coverage. Follow-up merge commit:
6c44a7852a04d63c197db58cc43ef9c3c4ea12b2.The merged tree exactly matches independently reviewed head
3fb73210b2618f4df334e6ed0c96677563e04b51(tree950f3adc6f0b1fb866d30129b623e1782fae3e6b). Fresh Go 1.26.6 tests on the actual merge commit passed withGOWORK=off go test -race -short ./cmd -run 'TestRunToolFromCSV_RejectsMalformedNumbers|TestLoadRecommendationsFromCSV|TestCSVCap' -count=1.Disk CSV tests cover fractional, suffixed, negative, and overflowing counts, non-finite or suffixed savings, valid values, and binding-cap behavior. Orchestration tests confirm rejection before a purchase report is written. Restoring the original parser caused twelve acceptance assertions to fail, including four orchestration cases. Overflow is preservation coverage. A built-CLI dry-run check rejected Count
3.7with line/column context and no report. No real purchases were made.Recommendation: keep #1944 closed. PR checks passed before merge; main-branch CI is monitored separately. Independent review used GPT-6 Astra under the session's authorization, not Opus.
fix(cli): parse CSV Count and EstimatedSavings strictly