fix(cli): bound --min-savings-pct to 0-100 (#2231)

--min-savings-pct was only checked for being finite. A negative value
behaved as no filter and a value above 100 dropped every recommendation,
since savings percentages are measured against on-demand and cannot
exceed 100. Reject both at validation, document the range, and rename the
flag-parsing validation test to cover out-of-range as well as non-finite
values.

Closes #2161

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: cristim

CHANGELOG.md

@@ -13,6 +13,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- `--min-savings-pct` now rejects values below 0 or above 100 at validation.
+  Previously a negative value was accepted as no filter and a value above 100
+  silently dropped every recommendation (#2161).
 - `configure-gcp`: skipping (or giving an unknown answer at) the "Create Service
   Account" step no longer continues with the default
   `cudly-service-account@<project>` email that was never created. The wizard now

cmd/validators.go

@@ -145,8 +145,8 @@ func requireFinite(name string, v float64) error {
 	return nil
 }
 
-// validateCoverageAndSavingsPct checks --coverage (finite, 0-100) and that
-// --min-savings-pct is finite. target-coverage and min-pool-size call
+// validateCoverageAndSavingsPct checks that --coverage and --min-savings-pct
+// are finite and within 0-100. target-coverage and min-pool-size call
 // requireFinite in their own validators.
 func validateCoverageAndSavingsPct() error {
 	if err := requireFinite("coverage", toolCfg.Coverage); err != nil {
@@ -155,7 +155,13 @@ func validateCoverageAndSavingsPct() error {
 	if toolCfg.Coverage < 0 || toolCfg.Coverage > 100 {
 		return fmt.Errorf("coverage percentage must be between 0 and 100, got: %.2f", toolCfg.Coverage)
 	}
-	return requireFinite("min-savings-pct", toolCfg.MinSavingsPct)
+	if err := requireFinite("min-savings-pct", toolCfg.MinSavingsPct); err != nil {
+		return err
+	}
+	if toolCfg.MinSavingsPct < 0 || toolCfg.MinSavingsPct > 100 {
+		return fmt.Errorf("min-savings-pct must be between 0 and 100, got: %.2f", toolCfg.MinSavingsPct)
+	}
+	return nil
 }
 
 // validateMinPoolSize checks that --min-pool-size is a non-negative whole number.

cmd/validators_test.go

@@ -815,10 +815,11 @@ func TestCheckDuplicates_HonorsIdempotencyWindow(t *testing.T) {
 	}
 }
 
-// TestValidateNumericFlagsRejectNonFinite drives the real flag set through
+// TestValidateNumericFlagsRejectInvalid drives the real flag set through
 // rootCmd.ParseFlags (cobra/pflag parse "NaN" and "Inf" as floats) and then
-// the validators. Nothing here calls runTool, so no AWS client is built.
-func TestValidateNumericFlagsRejectNonFinite(t *testing.T) {
+// the validators, covering non-finite and out-of-range values. Nothing here
+// calls runTool, so no AWS client is built.
+func TestValidateNumericFlagsRejectInvalid(t *testing.T) {
 	saved := toolCfg
 	reset := func() {
 		toolCfg = Config{}
@@ -848,6 +849,9 @@ func TestValidateNumericFlagsRejectNonFinite(t *testing.T) {
 		{name: "min-pool-size NaN", args: []string{"--min-pool-size", "NaN"}, wantErr: "min-pool-size must be a finite number, got: NaN"},
 		{name: "min-savings-pct NaN", args: []string{"--min-savings-pct", "NaN"}, wantErr: "min-savings-pct must be a finite number, got: NaN"},
 		{name: "min-savings-pct Inf", args: []string{"--min-savings-pct", "Inf"}, wantErr: "min-savings-pct must be a finite number, got: +Inf"},
+		{name: "min-savings-pct negative", args: []string{"--min-savings-pct", "-1"}, wantErr: "min-savings-pct must be between 0 and 100, got: -1.00"},
+		{name: "min-savings-pct above 100", args: []string{"--min-savings-pct", "100.5"}, wantErr: "min-savings-pct must be between 0 and 100, got: 100.50"},
+		{name: "min-savings-pct bounds stay valid", args: []string{"--min-savings-pct", "100"}},
 		{name: "overflow rejected by the flag parser", args: []string{"--target-coverage", "1e999"}, parseErr: true},
 		{name: "explicit target 0 stays valid", args: []string{"--target-coverage", "0"}},
 		{name: "valid values pass", args: []string{"--target-coverage", "95", "--coverage", "50", "--min-pool-size", "2", "--min-savings-pct", "5"}},

docs/cli/filtering.md

@@ -149,7 +149,7 @@ cudly --services rds --min-count 3
 --min-savings-pct <float>   Minimum savings percentage (0 = no filter)
 ```
 
-Drop recommendations whose estimated savings percentage falls below this threshold.
+Drop recommendations whose estimated savings percentage falls below this threshold. The value must be between 0 and 100.
 
 **Important naming distinction:** `--min-savings-pct` is a **percentage** value (e.g. `10` means "at least 10% savings"). This is different from the GUI and API `min_savings` parameter, which filters by **dollar amount**. A value of `30` in `--min-savings-pct` means "30% savings", but `min_savings=30` in the API means "$30 in savings". Mixing up the two by copying a CLI value into the GUI filter (or vice versa) produces silent, incorrect filtering.