fix(caddy): parse thread and request counts as int (#2722)

`num_threads`, `max_threads`, `max_requests`, the worker `num` and
`max_threads` options, and `php-server --worker file,N` are parsed with
`strconv.ParseUint(s, 10, 32)` and then converted to `int`. On 32-bit
platforms (we ship `linux/386`, `linux/arm/v6` and `linux/arm/v7`
images), values between 2^31 and 2^32-1 wrap to negative numbers:
`max_threads 4294967295` becomes `-1`, which means `auto`.

These values are now parsed with `strconv.Atoi` through a
`parseNonNegativeInt` helper. Nothing gets converted, so nothing can
wrap, and out-of-range input returns a range error on every platform.
Negative values are still rejected explicitly, because the runtime
treats them as sentinels (auto thread count, default worker count).
64-bit builds accept the same values as before.

Reported by CodeQL (`go/incorrect-integer-conversion`) while setting it
up in #2720.

Author: dunglas

caddy/app.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"log/slog"
 	"path/filepath"
-	"strconv"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -308,12 +307,12 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 					return d.ArgErr()
 				}
 
-				v, err := strconv.ParseUint(d.Val(), 10, 32)
+				v, err := parseNonNegativeInt(d)
 				if err != nil {
 					return err
 				}
 
-				f.NumThreads = int(v)
+				f.NumThreads = v
 			case "max_threads":
 				if !d.NextArg() {
 					return d.ArgErr()
@@ -324,12 +323,12 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 					continue
 				}
 
-				v, err := strconv.ParseUint(d.Val(), 10, 32)
+				v, err := parseNonNegativeInt(d)
 				if err != nil {
 					return err
 				}
 
-				f.MaxThreads = int(v)
+				f.MaxThreads = v
 			case "max_wait_time":
 				if !d.NextArg() {
 					return d.ArgErr()
@@ -357,12 +356,12 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 					return d.ArgErr()
 				}
 
-				v, err := strconv.ParseUint(d.Val(), 10, 32)
+				v, err := parseNonNegativeInt(d)
 				if err != nil {
-					return d.WrapErr(err)
+					return err
 				}
 
-				f.MaxRequests = int(v)
+				f.MaxRequests = v
 			case "php_ini":
 				parseIniLine := func(d *caddyfile.Dispenser) error {
 					key := d.Val()

caddy/caddy.go

@@ -6,9 +6,11 @@ package caddy
 import (
 	"context"
 	"fmt"
+	"strconv"
 	"time"
 
 	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 	"github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile"
 	"github.com/dunglas/frankenphp"
 )
@@ -51,3 +53,16 @@ func init() {
 func wrongSubDirectiveError(module string, allowedDirectives string, wrongValue string) error {
 	return fmt.Errorf("unknown %q subdirective: %s (allowed directives are: %s)", module, wrongValue, allowedDirectives)
 }
+
+// parseNonNegativeInt rejects negative values, which the runtime treats as sentinels (auto, default).
+func parseNonNegativeInt(d *caddyfile.Dispenser) (int, error) {
+	v, err := strconv.Atoi(d.Val())
+	if err != nil {
+		return 0, d.WrapErr(err)
+	}
+	if v < 0 {
+		return 0, d.Errf("%q must not be negative", d.Val())
+	}
+
+	return v, nil
+}

caddy/config_test.go

@@ -25,6 +25,17 @@ func TestWorkerOptionsKeepProvisionedOptions(t *testing.T) {
 	require.Len(t, opts, base+1, "worker options set during provisioning must be forwarded")
 }
 
+// negative counts are runtime sentinels (auto, default) and must not be reachable from the Caddyfile
+func TestAppRejectsNegativeCounts(t *testing.T) {
+	for _, directive := range []string{"num_threads -1", "max_threads -1", "max_requests -1", "worker index.php -1", "worker {\n num -1\n }", "worker {\n max_threads -1\n }"} {
+		t.Run(directive, func(t *testing.T) {
+			d := caddyfile.NewTestDispenser("frankenphp {\n" + directive + "\n}")
+
+			require.ErrorContains(t, (&FrankenPHPApp{}).UnmarshalCaddyfile(d), "must not be negative")
+		})
+	}
+}
+
 func TestModuleRequestBodyTimeout(t *testing.T) {
 	d := caddyfile.NewTestDispenser(`
 	{

caddy/php-server.go

@@ -91,12 +91,12 @@ func cmdPHPServer(fs caddycmd.Flags) (int, error) {
 		for _, worker := range workers {
 			parts := strings.SplitN(worker, ",", 2)
 
-			var num uint64
+			var num int
 			if len(parts) > 1 {
-				num, _ = strconv.ParseUint(parts[1], 10, 32)
+				num, _ = strconv.Atoi(parts[1])
 			}
 
-			workersOption = append(workersOption, workerConfig{FileName: parts[0], Num: int(num)})
+			workersOption = append(workersOption, workerConfig{FileName: parts[0], Num: num})
 		}
 		workersOption[0].Watch = watch
 	}

caddy/workerconfig.go

@@ -52,12 +52,12 @@ func unmarshalWorker(d *caddyfile.Dispenser) (workerConfig, error) {
 		if d.Val() == "watch" {
 			wc.Watch = append(wc.Watch, defaultWatchPattern)
 		} else {
-			v, err := strconv.ParseUint(d.Val(), 10, 32)
+			v, err := parseNonNegativeInt(d)
 			if err != nil {
 				return wc, err
 			}
 
-			wc.Num = int(v)
+			wc.Num = v
 		}
 	}
 
@@ -82,23 +82,23 @@ func unmarshalWorker(d *caddyfile.Dispenser) (workerConfig, error) {
 				return wc, d.ArgErr()
 			}
 
-			v, err := strconv.ParseUint(d.Val(), 10, 32)
+			v, err := parseNonNegativeInt(d)
 			if err != nil {
-				return wc, d.WrapErr(err)
+				return wc, err
 			}
 
-			wc.Num = int(v)
+			wc.Num = v
 		case "max_threads":
 			if !d.NextArg() {
 				return wc, d.ArgErr()
 			}
 
-			v, err := strconv.ParseUint(d.Val(), 10, 32)
+			v, err := parseNonNegativeInt(d)
 			if err != nil {
-				return wc, d.WrapErr(err)
+				return wc, err
 			}
 
-			wc.MaxThreads = int(v)
+			wc.MaxThreads = v
 		case "env":
 			args := d.RemainingArgs()
 			if len(args) != 2 {