From 892bdf95a98dc8220f38ad2732cf40d7f04a4b40 Mon Sep 17 00:00:00 2001
From: Benoit Tigeot <benoit.tigeot@lifen.fr>
Date: Fri, 7 Nov 2025 11:29:20 +0100
Subject: [PATCH] fix: reject invalid 'm' suffix in byte size parsing

Rename parseQuantityOrInt64 to parseByteSizeOrInt64 and improve error
handling for invalid byte size quantities. Now properly rejects 'm'
(milli) suffix with a helpful message suggesting IEC values (Ki, Mi, Gi).

Also fixes misleading "too large" error for sub-byte quantities.

Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
---
 pkg/cli/environment.go      | 25 +++++++++++++++++--------
 pkg/cli/environment_test.go | 31 +++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go
index 606780c5f..2d30770a1 100644
--- a/pkg/cli/environment.go
+++ b/pkg/cli/environment.go
@@ -222,9 +222,9 @@ func envFloat32Or(name string, def float32) float32 {
 	return float32(ret)
 }
 
-// parseQuantityOrInt64 parses a string as either a Kubernetes Quantity or plain int64.
-// Returns the parsed value and an error if parsing fails.
-func parseQuantityOrInt64(s string) (int64, error) {
+// parseByteSizeOrInt64 parses a string as either a Kubernetes Quantity or plain int64,
+// specifically for byte sizes. Returns the parsed value in bytes
+func parseByteSizeOrInt64(s string) (int64, error) {
 	s = strings.TrimSpace(s)
 
 	// Try parsing as Kubernetes Quantity first
@@ -233,10 +233,19 @@ func parseQuantityOrInt64(s string) (int64, error) {
 			return v, nil
 		}
 		f := q.AsApproximateFloat64()
-		if f > 0 && f < float64(^uint64(0)>>1) {
-			return int64(f), nil
+		// Reject quantities that evaluate to less than 1 byte (e.g. "1m" -> 0.001)
+		// because file sizes must be whole bytes. Treat those as parsing errors.
+		if f < 1 {
+			// Provide a helpful message if the user tried to use "m" (milli) suffix
+			if strings.HasSuffix(strings.ToLower(s), "m") && !strings.HasSuffix(s, "M") {
+				return 0, fmt.Errorf("quantity %q uses 'm' (milli) suffix which represents 0.001; please use IEC values like Ki, Mi, Gi", s)
+			}
+			return 0, fmt.Errorf("quantity %q is too small (less than 1 byte)", s)
 		}
-		return 0, fmt.Errorf("quantity %q is too large to fit in int64", s)
+		if f >= float64(^uint64(0)>>1) {
+			return 0, fmt.Errorf("quantity %q is too large to fit in int64", s)
+		}
+		return int64(f), nil
 	}
 
 	// Fallback to plain int64
@@ -257,7 +266,7 @@ func envInt64OrQuantityBytes(name string, def int64) int64 {
 		return def
 	}
 
-	v, err := parseQuantityOrInt64(envVal)
+	v, err := parseByteSizeOrInt64(envVal)
 	if err != nil {
 		defQuantity := resource.NewQuantity(def, resource.BinarySI)
 		slog.Warn(err.Error() + fmt.Sprintf(": using default %s", defQuantity.String()))
@@ -278,7 +287,7 @@ func NewQuantityBytesValue(p *int64) *QuantityBytesValue {
 
 // Set parses the input string as either a Kubernetes Quantity or plain int64
 func (q *QuantityBytesValue) Set(s string) error {
-	v, err := parseQuantityOrInt64(s)
+	v, err := parseByteSizeOrInt64(s)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/cli/environment_test.go b/pkg/cli/environment_test.go
index 8413da83a..02604333a 100644
--- a/pkg/cli/environment_test.go
+++ b/pkg/cli/environment_test.go
@@ -325,6 +325,27 @@ func TestEnvInt64OrQuantityBytes(t *testing.T) {
 			def:      100,
 			expected: 100, // Returns default but prints error about uppercase requirement
 		},
+		{
+			name:     "suffix Mb rejected",
+			env:      envName,
+			val:      "1000Mb",
+			def:      100,
+			expected: 100, // Returns default but prints error about 'Mb' being invalid
+		},
+		{
+			name:     "suffix mo rejected",
+			env:      envName,
+			val:      "1000mo",
+			def:      100,
+			expected: 100, // Returns default but prints error about 'mo' being invalid
+		},
+		{
+			name:     "suffix m rejected (milli)",
+			env:      envName,
+			val:      "10m",
+			def:      100,
+			expected: 100, // Returns default but prints error about 'm' being invalid
+		},
 		{
 			name:     "whitespace trimmed",
 			env:      envName,
@@ -391,6 +412,16 @@ func TestQuantityBytesValue(t *testing.T) {
 			input:       "not-a-number",
 			expectError: true,
 		},
+		{
+			name:        "Mb suffix rejected",
+			input:       "1Mb",
+			expectError: true,
+		},
+		{
+			name:        "m suffix rejected",
+			input:       "1m",
+			expectError: true,
+		},
 	}
 
 	for _, tt := range tests {