fix(normalizer): validate numeric float64 inputs in typed read normalizer (#3762)

pkg/github/typed_read_normalizers.go

@@ -1,15 +1,45 @@
 package github
 
 import (
+	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math"
+	"math/big"
 	"strconv"
 	"strings"
 
 	"github.com/github/github-mcp-server/v2/pkg/inventory"
 )
 
+// maxNumericTokenLen and maxNumericExponent bound decimal or exponent number
+// tokens before they are checked as exact rationals.
+const (
+	maxNumericTokenLen = 64
+	maxNumericExponent = 40
+)
+
+// exactIntegerToken reports the int64 value of a JSON number token written in
+// decimal or exponent form, but only when the token is exactly an integer that
+// fits in int64.
+func exactIntegerToken(token string) (int64, bool) {
+	if len(token) > maxNumericTokenLen {
+		return 0, false
+	}
+	if i := strings.IndexAny(token, "eE"); i >= 0 {
+		exp, err := strconv.Atoi(token[i+1:])
+		if err != nil || exp > maxNumericExponent || exp < -maxNumericExponent {
+			return 0, false
+		}
+	}
+	exact, ok := new(big.Rat).SetString(token)
+	if !ok || !exact.IsInt() || !exact.Num().IsInt64() {
+		return 0, false
+	}
+	return exact.Num().Int64(), true
+}
+
 func normalizeTypedReadArguments(uppercaseFields []string, preserveZeroPage bool) inventory.InputNormalizer {
 	return func(raw json.RawMessage) (json.RawMessage, error) {
 		var args map[string]json.RawMessage
@@ -37,37 +67,56 @@ func normalizeTypedReadArguments(uppercaseFields []string, preserveZeroPage bool
 				continue
 			}
 			var number any
-			if err := json.Unmarshal(value, &number); err != nil {
+			decoder := json.NewDecoder(bytes.NewReader(value))
+			decoder.UseNumber()
+			if err := decoder.Decode(&number); err != nil {
 				return nil, err
 			}
-			if text, ok := number.(string); ok {
-				parsed, err := strconv.ParseFloat(text, 64)
-				if err != nil || math.IsNaN(parsed) || math.IsInf(parsed, 0) || parsed != math.Trunc(parsed) || !inSignedIntRange(parsed, strconv.IntSize) {
+			var intVal int
+			switch v := number.(type) {
+			case string:
+				parsed, err := strconv.ParseFloat(v, 64)
+				if err != nil {
 					return nil, fmt.Errorf("parameter %s is not a valid number", field)
 				}
-				number = int(parsed)
-			}
-			if field == "page" && !preserveZeroPage {
-				if parsed, ok := number.(float64); ok && parsed == 0 {
-					delete(args, field)
-					continue
-				}
-				if parsed, ok := number.(int); ok && parsed == 0 {
-					delete(args, field)
-					continue
+				if math.IsNaN(parsed) || math.IsInf(parsed, 0) || parsed != math.Trunc(parsed) || !inSignedIntRange(parsed, strconv.IntSize) {
+					return nil, fmt.Errorf("parameter %s is not a valid number", field)
 				}
-			}
-			if field == "perPage" {
-				if parsed, ok := number.(float64); ok && parsed == 0 {
-					delete(args, field)
-					continue
+				intVal = int(parsed)
+			case json.Number:
+				// Validate the original token before any lossy float64 conversion.
+				// Plain integer tokens are parsed exactly. Decimal or exponent
+				// forms are checked as exact rationals; the token length and
+				// exponent are bounded first so a hostile exponent cannot make
+				// the exact check expensive.
+				integer, err := strconv.ParseInt(v.String(), 10, strconv.IntSize)
+				if err != nil {
+					var numErr *strconv.NumError
+					if errors.As(err, &numErr) && errors.Is(numErr.Err, strconv.ErrRange) {
+						return nil, fmt.Errorf("parameter %s is not a valid number", field)
+					}
+					exact, ok := exactIntegerToken(v.String())
+					if !ok {
+						return nil, fmt.Errorf("parameter %s is not a valid number", field)
+					}
+					integer = exact
 				}
-				if parsed, ok := number.(int); ok && parsed == 0 {
-					delete(args, field)
-					continue
+				if int64(int(integer)) != integer {
+					return nil, fmt.Errorf("parameter %s is not a valid number", field)
 				}
+				intVal = int(integer)
+			default:
+				continue
+			}
+			if field == "page" && !preserveZeroPage && intVal == 0 {
+				delete(args, field)
+				continue
+			}
+			if field == "perPage" && intVal == 0 {
+				delete(args, field)
+				continue
 			}
-			normalized, err := json.Marshal(number)
+			normalized, err := json.Marshal(intVal)
 			if err != nil {
 				return nil, err
 			}

pkg/github/typed_search_outputs_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"net/http"
+	"strconv"
 	"testing"
 
 	"github.com/github/github-mcp-server/v2/internal/toolsnaps"
@@ -383,6 +384,15 @@ func TestNormalizeTypedReadArgumentsRejectsOutOfRangeIntegers(t *testing.T) {
 		`{"page":"9223372036854775808"}`,
 		`{"perPage":"9223372036854775807"}`,
 		`{"page":"-9223372036854777856"}`,
+		`{"page": 9223372036854775808}`,
+		`{"page": -9223372036854777856}`,
+		`{"page": -9223372036854775809}`,
+		`{"perPage": -9223372036854775809.0}`,
+		`{"page": -9.223372036854775809e18}`,
+		`{"page": 1.00000000000000001}`,
+		`{"page": 1e999999999}`,
+		`{"page": 1e-999999999}`,
+		`{"page": 1.5}`,
 	} {
 		t.Run(input, func(t *testing.T) {
 			_, err := normalize(json.RawMessage(input))
@@ -391,6 +401,26 @@ func TestNormalizeTypedReadArgumentsRejectsOutOfRangeIntegers(t *testing.T) {
 	}
 }
 
+func TestNormalizeTypedReadArgumentsPreservesExactNumbers(t *testing.T) {
+	normalize := normalizeTypedReadArguments(nil, false)
+	for _, value := range []string{"25.0", "2.5e1", "250e-1"} {
+		t.Run(value, func(t *testing.T) {
+			normalized, err := normalize(json.RawMessage(`{"page":` + value + `}`))
+			require.NoError(t, err)
+			assert.JSONEq(t, `{"page":25}`, string(normalized))
+		})
+	}
+	if strconv.IntSize == 64 {
+		for _, value := range []string{"9223372036854775807", "-9223372036854775808", "9007199254740993"} {
+			t.Run(value, func(t *testing.T) {
+				normalized, err := normalize(json.RawMessage(`{"perPage":` + value + `}`))
+				require.NoError(t, err)
+				assert.Equal(t, `{"perPage":`+value+`}`, string(normalized))
+			})
+		}
+	}
+}
+
 func mustMarshalJSON(t *testing.T, value any) string {
 	t.Helper()
 	encoded, err := json.Marshal(value)