Skip to content

fix: reject overflow when scanning SQL enum numbers - #327

Open
jihtsan wants to merge 1 commit into
abice:masterfrom
jihtsan:codex/252-sql-integer-overflow
Open

jihtsan wants to merge 1 commit into
abice:masterfrom
jihtsan:codex/252-sql-integer-overflow

Conversation

@jihtsan

@jihtsan jihtsan commented Oct 18, 2026 •

Copy link
Copy Markdown

Fixes #252.

SQL Scan currently casts values directly to the enum's integer type, so out-of-range values can silently become different named values (for example, int64(256) becomes the zero value of an int8 enum). Parse integers with the destination width and signedness before assigning; use the same check for pointers and the sqlnullint numeric-string fallback. Float inputs retain truncation of representable fractional values but reject overflow and non-finite values. Failed numeric conversions preserve the receiver.

Keep valid unnamed values, nil handling, string enum parsing and existing Scan entry types supported. Named integer underlying types keep their actual width/signedness. Regenerate the affected examples and snapshots; Valuer behavior and the broader SQL redesign in #239 are outside this change.

Validation:

  • Generated-code regression exercises 13 built-in integer/alias types plus two named integer types under --sql, --sqlnullstr and --sqlnullint. The original implementation fails overflow cases; the correction passes.
  • Go 1.25.0 and 1.26.0: make cover passes, including race/shuffle and example-tag tests. Go 1.26.0 go vet ./... passes.
  • An independent Go 1.25.5 runtime harness passes 4,055 assertions across 15 integer types and SQL/nullable/marshal/noparse combinations, including float boundaries, unsigned negative zero, typed nil and Null.Valid/Set. Linux/386 test binaries cross-compile for all three combinations (not executed).
  • make generate followed by make assert-no-changes passes after committing generated files.
  • gosec v2.29.0, G115 only, with generated files included: the two-type issue reproduction reports 10 findings before and 0 after. GOTOOLCHAIN=go1.25.0 is required for this check; the analyzer cannot read Go 1.27 export data.

Validation ran on macOS/arm64. Native 32-bit, Linux and Windows execution were not run locally.

Summary by CodeRabbit

  • Bug Fixes
    • Numeric values scanned into SQL-backed enums are now checked against the enum’s supported range. Out-of-range values return errors instead of being silently converted.
    • Fractional floating-point values are truncated toward zero before conversion; values that exceed the supported range return errors.
    • Numeric strings are checked for overflow when scanning into integer-backed enums.

@jihtsan
jihtsan requested a review from abice as a code owner October 18, 2026 10:20
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 77f67ac7-5edd-4102-b4a1-2566341fbe86
📥 Commits

Reviewing files that changed from the base of the PR and between ad6b63d and b13fa5a.

📒 Files selected for processing: 17
  • example/noparse_sql_enum.go
  • example/sql_enum.go
  • example/sql_int_enum.go
  • example/sql_str_enum.go
  • generator/.snapshots/Test118CustomPrefixExampleFile-1.18
  • generator/.snapshots/Test118CustomPrefixExampleFile-og
  • generator/.snapshots/Test118ExampleFile-1.18
  • generator/.snapshots/Test118ExampleFile-og
  • generator/.snapshots/Test118ExampleFileMoreOptions-1.18
  • generator/.snapshots/Test118ExampleFileMoreOptions-og
  • generator/.snapshots/TestCustomPrefixExampleFile
  • generator/.snapshots/TestExampleFile
  • generator/.snapshots/TestExampleFileEmptyHeaders
  • generator/.snapshots/TestExampleFileMoreOptions
  • generator/.snapshots/TestExampleFileMoreOptionsWithForceUpper
  • generator/enum.tmpl
  • generator/sql_integer_test.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

SQL enum scanners now validate numeric conversions against the enum’s integer width and signedness. Float inputs are truncated before parsing. The generator template, example scanners, generated snapshots, and generated tests reflect these changes.

Changes: SQL enum numeric scanning

Layer / File(s) Summary
Generator parsing logic and validation
generator/enum.tmpl, generator/sql_integer_test.go
The template parses numeric inputs with width and signedness checks, truncates floats, and supports numeric-string fallback for SQL-null-integer enums. Generated tests cover integer boundaries, invalid inputs, and receiver state.
Hand-written scanner conversions
example/noparse_sql_enum.go, example/sql_enum.go, example/sql_int_enum.go, example/sql_str_enum.go
Example scanners use checked numeric parsing instead of direct casts. Float inputs are truncated before conversion, and existing nil-pointer errors remain.
Generated scanner snapshots
generator/.snapshots/*
Snapshots update generated scanner code across enum widths and generator options to show checked numeric conversions and corresponding string handling.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: High

Merge Risk: ⚪ Minimal · up to b13fa

Generated SQL enum scanners now reject numeric values that do not fit the enum's integer type, instead of silently wrapping them. Existing valid inputs and string parsing keep working. No outstanding defects were found, so the change looks ready to merge.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: rejecting integer overflow during SQL enum scanning.
Linked Issues check Passed Issue #252 requires the generated --sql Scan path to avoid unsafe integer overflow conversions. generator/enum.tmpl now parses numeric inputs with strconv.ParseInt or strconv.ParseUint at the en…
Out of Scope Changes check Passed The changed template, generated examples, snapshots, and regression test support the overflow-safe SQL Scan behavior for issue #252. Pointer checks, float checks, named-type handling, and sqlnullint…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 5 files. (12 skipped: 1…
Full details: Linked Issues check

Explanation

Issue #252 requires the generated --sql Scan path to avoid unsafe integer overflow conversions. generator/enum.tmpl now parses numeric inputs with strconv.ParseInt or strconv.ParseUint at the enum width before assignment. The change covers built-in and named integer types, pointers, and the sqlnullint numeric-string fallback. generator/sql_integer_test.go adds overflow and receiver-preservation tests across 15 integer types and three SQL modes. The PR reports zero G115 findings after the change.

Full details: Out of Scope Changes check

Explanation

The changed template, generated examples, snapshots, and regression test support the overflow-safe SQL Scan behavior for issue #252. Pointer checks, float checks, named-type handling, and sqlnullint parsing apply the same numeric safety requirement to supported Scan entry types. The PR states that Valuer behavior and the broader SQL redesign in #239 remain unchanged.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 5 files. (12 skipped: 12 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

--sql and gosec = G115 (CWE-190): integer overflow conversion uint -> int (Confidence: MEDIUM, Severity: HIGH)

1 participant