Skip to content

security: clamp int → int32 casts (CodeQL go/incorrect-integer-conversion)#9

Merged
hacka0wi merged 2 commits into
mainfrom
security/codeql-fixes
May 6, 2026
Merged

security: clamp int → int32 casts (CodeQL go/incorrect-integer-conversion)#9
hacka0wi merged 2 commits into
mainfrom
security/codeql-fixes

Conversation

@hacka0wi

@hacka0wi hacka0wi commented May 6, 2026

Copy link
Copy Markdown
Member

Summary

CodeQL flagged three places where an int (env-var-derived or parsed
from a Kafka header) was cast to int32 without bounds-checking. In
practice the values are tiny — connection pool sizes ≤ 200, attempt
counts ≤ 30 — but a malicious or misconfigured input could wrap into a
negative number, corrupting the pool config or silently bypassing the
max-attempts guard.

This patch:

  • Adds clampInt32(int) int32 in internal/db/postgres.go (≤ 0 → 0,
    ≥ MaxInt32 → MaxInt32) and routes the two pool-size assignments
    through it.
  • Bounds-checks the Kafka x-attempts header before the int32 cast in
    internal/queue/backend_kafka.go.
  • Adds a unit test for clampInt32 covering MinInt32, 0, normal range,
    MaxInt32, and overflow.

Closes alerts

  • go/incorrect-integer-conversion at postgres.go:29
  • go/incorrect-integer-conversion at postgres.go:30
  • go/incorrect-integer-conversion at backend_kafka.go:312

What this does NOT close

The two remaining open alerts are js/clear-text-logging in the seed
scripts (prisma/seed.ts:91, prisma/seed-demo.ts:456) — both of
those console.log calls intentionally print the auto-generated admin
password so the operator can capture it from the seed output. That's
a documented operator-tool pattern, not a runtime bug. I'll dismiss
those alerts separately with reason "won't fix — intentional".

Test plan

  • go test ./internal/db/... -run TestClampInt32 passes
  • go vet ./... clean
  • go build ./... clean
  • CodeQL re-runs and the three Go alerts close automatically

🤖 Generated with Claude Code

…rect-integer-conversion)

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.
Comment thread apps/runloop-engine/internal/queue/backend_kafka.go Fixed
…per bound

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.
@hacka0wi hacka0wi merged commit bdc862a into main May 6, 2026
8 checks passed
@hacka0wi hacka0wi deleted the security/codeql-fixes branch May 6, 2026 06:28
hacka0wi added a commit that referenced this pull request May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants