fix(cli): fmt.Sscanf CSV parsing truncates 3.7 to 3 and accepts trailing garbage

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.

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 (tree 950f3adc6f0b1fb866d30129b623e1782fae3e6b). Fresh Go 1.26.6 tests on the actual merge commit passed with GOWORK=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.7 with 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

Navigation Menu

Repository navigation

fix(cli): fmt.Sscanf CSV parsing truncates 3.7 to 3 and accepts trailing garbage #1944

Summary

Location

Failure scenario

Evidence

Suggested fix

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

fix(cli): fmt.Sscanf CSV parsing truncates 3.7 to 3 and accepts trailing garbage #1944

Description

Summary

Location

Failure scenario

Evidence

Suggested fix

Activity

cristim commented on Sep 29, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions