From f9e3f485535916703602e031123faca0e36be486 Mon Sep 17 00:00:00 2001 From: hacka0wi <124970567+hacka0wi@users.noreply.github.com> Date: Wed, 6 May 2026 13:18:54 +0700 Subject: [PATCH 1/2] fix(security): clamp env-var ints before int32 casts (CodeQL go/incorrect-integer-conversion) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged three call sites where an `int` value (env-var-derived or parsed from an attacker-controllable Kafka header) was cast to `int32` without bounds-checking. In normal operation the values are tiny (connection pool sizes ≤ a few hundred, retry attempts ≤ a few dozen), so the cast doesn't actually overflow — but a malicious or misconfigured input could wrap into a negative number, corrupting the pool config or silently bypassing the max-attempts guard in the Kafka requeue path. - `internal/db/postgres.go` — new `clampInt32(int) int32` helper. ≤ 0 becomes 0 (lets pgxpool fall back to its own defaults), ≥ MaxInt32 pins to MaxInt32. Used for `MaxConns` / `MinConns`. Test in `postgres_test.go` covers boundary values. - `internal/queue/backend_kafka.go` — clamp the parsed `x-attempts` header before the `int32` cast and the `+1` increment. Closes the three open `go/incorrect-integer-conversion` alerts at postgres.go:29-30 and backend_kafka.go:312. --- apps/runloop-engine/internal/db/postgres.go | 26 ++++++++++++++--- .../internal/db/postgres_test.go | 29 +++++++++++++++++++ .../internal/queue/backend_kafka.go | 17 ++++++++--- 3 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 apps/runloop-engine/internal/db/postgres_test.go diff --git a/apps/runloop-engine/internal/db/postgres.go b/apps/runloop-engine/internal/db/postgres.go index 44da94c..7890bce 100644 --- a/apps/runloop-engine/internal/db/postgres.go +++ b/apps/runloop-engine/internal/db/postgres.go @@ -3,11 +3,12 @@ package db import ( "context" "fmt" + "math" "time" "github.com/jackc/pgx/v5/pgxpool" - "github.com/runloop/runloop-engine/internal/config" "github.com/rs/zerolog/log" + "github.com/runloop/runloop-engine/internal/config" ) // Postgres represents a PostgreSQL database connection @@ -15,6 +16,20 @@ type Postgres struct { Pool *pgxpool.Pool } +// clampInt32 narrows an int from config (env-var-derived) to the int32 range +// pgxpool requires. Values ≤ 0 produce 0 so callers can fall back to library +// defaults; large values are pinned to math.MaxInt32 instead of silently +// wrapping into a negative number. +func clampInt32(v int) int32 { + if v <= 0 { + return 0 + } + if v >= math.MaxInt32 { + return math.MaxInt32 + } + return int32(v) +} + // NewPostgres creates a new PostgreSQL connection pool func NewPostgres(cfg *config.Config) (*Postgres, error) { ctx := context.Background() @@ -25,9 +40,12 @@ func NewPostgres(cfg *config.Config) (*Postgres, error) { return nil, fmt.Errorf("unable to parse database URL: %w", err) } - // Set pool configuration - poolConfig.MaxConns = int32(cfg.DatabaseMaxConns) - poolConfig.MinConns = int32(cfg.DatabaseMaxIdle) + // Set pool configuration. pgxpool stores pool sizes as int32, so clamp + // the int values from config to the int32 range. In practice these come + // from env vars and are small (≤ a few hundred); the clamp is a safety + // belt that also satisfies CodeQL's incorrect-integer-conversion rule. + poolConfig.MaxConns = clampInt32(cfg.DatabaseMaxConns) + poolConfig.MinConns = clampInt32(cfg.DatabaseMaxIdle) // Create connection pool pool, err := pgxpool.NewWithConfig(ctx, poolConfig) diff --git a/apps/runloop-engine/internal/db/postgres_test.go b/apps/runloop-engine/internal/db/postgres_test.go new file mode 100644 index 0000000..11335fe --- /dev/null +++ b/apps/runloop-engine/internal/db/postgres_test.go @@ -0,0 +1,29 @@ +package db + +import ( + "math" + "testing" +) + +// clampInt32 keeps pgxpool's int32 fields safe from out-of-range int values +// coming out of the env-var config. + +func TestClampInt32(t *testing.T) { + cases := []struct { + in int + want int32 + }{ + {0, 0}, + {-1, 0}, + {math.MinInt32, 0}, + {1, 1}, + {200, 200}, + {math.MaxInt32, math.MaxInt32}, + {math.MaxInt32 + 1, math.MaxInt32}, + } + for _, c := range cases { + if got := clampInt32(c.in); got != c.want { + t.Errorf("clampInt32(%d) = %d, want %d", c.in, got, c.want) + } + } +} diff --git a/apps/runloop-engine/internal/queue/backend_kafka.go b/apps/runloop-engine/internal/queue/backend_kafka.go index 42caa9e..6c0621d 100644 --- a/apps/runloop-engine/internal/queue/backend_kafka.go +++ b/apps/runloop-engine/internal/queue/backend_kafka.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "math" "strconv" "sync" "time" @@ -43,8 +44,8 @@ type KafkaBackend struct { workerID string mu sync.Mutex - readers map[string]*kafka.Reader // key = topic+groupId — consumer - writers map[string]*kafka.Writer // key = brokers-joined + readers map[string]*kafka.Reader // key = topic+groupId — consumer + writers map[string]*kafka.Writer // key = brokers-joined ackState map[string]*kafka.Message // handle -> message (for commit lookup) } @@ -303,13 +304,21 @@ func (k *KafkaBackend) Nack(ctx context.Context, q *QueueDef, handle string, req } } - // Increment attempts header and re-publish. + // Increment attempts header and re-publish. The header's previous count + // is read from an attacker-controllable bytestream, so clamp before the + // int32 cast — a maliciously huge value shouldn't wrap into a negative + // attempt count and bypass the max-attempts check below. attempts := int32(0) out := make([]kafka.Header, 0, len(msg.Headers)) for _, h := range msg.Headers { if h.Key == "x-attempts" { n, _ := strconv.Atoi(string(h.Value)) - attempts = int32(n) + 1 + if n < 0 { + n = 0 + } else if n >= math.MaxInt32 { + n = math.MaxInt32 - 1 + } + attempts = int32(n) + 1 //nolint:gosec // bounded above continue } if h.Key == "x-last-error" { From a7ede45b0aaeb152d19613c5ce18031ea8ac4ae7 Mon Sep 17 00:00:00 2001 From: hacka0wi <124970567+hacka0wi@users.noreply.github.com> Date: Wed, 6 May 2026 13:23:26 +0700 Subject: [PATCH 2/2] fix(security): rewrite kafka attempts increment so CodeQL sees the upper bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL's data-flow analysis didn't track the int → int32 narrowing across the if-else clamp from the previous attempt — even though `n` was bounded, the cast `int32(n) + 1` still got flagged as `go/incorrect-integer-conversion`. Restructure so the cast is preceded by an explicit upper-bound check on the exact same variable: do the `+1` in int64 (where overflow is impossible for any int input), then clamp into [1, MaxInt32], then cast. Same effective behavior, but the bound is now visible to CodeQL right at the cast site. --- .../internal/queue/backend_kafka.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/apps/runloop-engine/internal/queue/backend_kafka.go b/apps/runloop-engine/internal/queue/backend_kafka.go index 6c0621d..fbb2683 100644 --- a/apps/runloop-engine/internal/queue/backend_kafka.go +++ b/apps/runloop-engine/internal/queue/backend_kafka.go @@ -305,20 +305,23 @@ func (k *KafkaBackend) Nack(ctx context.Context, q *QueueDef, handle string, req } // Increment attempts header and re-publish. The header's previous count - // is read from an attacker-controllable bytestream, so clamp before the - // int32 cast — a maliciously huge value shouldn't wrap into a negative - // attempt count and bypass the max-attempts check below. + // is read from an attacker-controllable bytestream, so do the +1 in int64 + // (no overflow) and clamp into int32 right at the cast — a maliciously + // huge value shouldn't wrap into a negative attempt count and bypass the + // max-attempts check below. attempts := int32(0) out := make([]kafka.Header, 0, len(msg.Headers)) for _, h := range msg.Headers { if h.Key == "x-attempts" { n, _ := strconv.Atoi(string(h.Value)) - if n < 0 { - n = 0 - } else if n >= math.MaxInt32 { - n = math.MaxInt32 - 1 + v := int64(n) + 1 + if v < 1 { + v = 1 } - attempts = int32(n) + 1 //nolint:gosec // bounded above + if v > math.MaxInt32 { + v = math.MaxInt32 + } + attempts = int32(v) continue } if h.Key == "x-last-error" {