Skip to content

fix(codeql): go/incorrect-integer-conversion in pkg/model/provider/dmr/configure.go:158#3756

Merged
Sayt-0 merged 2 commits into
mainfrom
codeql/go-incorrect-integer-conversion-configure-158
Jul 20, 2026
Merged

fix(codeql): go/incorrect-integer-conversion in pkg/model/provider/dmr/configure.go:158#3756
Sayt-0 merged 2 commits into
mainfrom
codeql/go-incorrect-integer-conversion-configure-158

Conversation

@ronan-thibaut-glitch

Copy link
Copy Markdown
Contributor

An int64 value from strconv.ParseInt (via parseInt64Value) was narrowed directly to int32 in buildConfigureBackendConfig without an upper-bound check, triggering CodeQL rule go/incorrect-integer-conversion. While context window sizes in excess of 2^31−1 are practically unreachable, a value that large would silently wrap to a negative int32, causing model invocation failures. The fix adds an explicit guard — if the int64 exceeds math.MaxInt32, the value is clamped to math.MaxInt32 and a warning is logged; the int32 conversion then operates on a provably in-range value. The now-redundant //nolint:gosec directive is removed (confirmed by running golangci-lint with gosec enabled). A new test subcase covers the clamp path.

…r/configure.go:158

Add an explicit upper-bound check before narrowing the int64 context size
to int32 in buildConfigureBackendConfig. If the value exceeds math.MaxInt32
(unrealistic for model context windows but possible in theory), it is clamped
and a warning is logged rather than silently wrapping to a negative value.
Also removes the //nolint:gosec directive that is no longer needed now that
the conversion is provably safe after the guard.

Fixes CodeQL rule go/incorrect-integer-conversion.
Comment thread pkg/model/provider/dmr/configure.go Fixed
@aheritier aheritier added area/models LLM model integrations and model providers area/providers/docker-model-runner Docker Model Runner (DMR) local inference kind/fix PR fixes a bug (maps to fix:). Use on PRs only. labels Jul 20, 2026
The first iteration added only an upper-bound check (v > math.MaxInt32),
which left the negative range unsanitised — CodeQL go/incorrect-integer-
conversion requires both bounds to be proven before a narrowing cast is
considered safe.

Replace the if/else chain with a switch:
- v > math.MaxInt32: log a warning and clamp to MaxInt32
- 0 ≤ v ≤ MaxInt32: safe range, convert directly (int32(v))
- v < 0: log a warning and skip setting ContextSize (field stays nil)

Treating negative context sizes as absent (nil) rather than clamping to
zero avoids sending a potentially confusing context_size=0 to model-runner.
Also adjusts the test added in the previous commit to match the new semantics.
@ronan-thibaut-glitch
ronan-thibaut-glitch marked this pull request as ready for review July 20, 2026 16:02
@ronan-thibaut-glitch
ronan-thibaut-glitch requested a review from a team as a code owner July 20, 2026 16:02
@Sayt-0
Sayt-0 enabled auto-merge July 20, 2026 16:29
@Sayt-0
Sayt-0 merged commit ff78fcd into main Jul 20, 2026
14 checks passed
@Sayt-0
Sayt-0 deleted the codeql/go-incorrect-integer-conversion-configure-158 branch July 20, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/models LLM model integrations and model providers area/providers/docker-model-runner Docker Model Runner (DMR) local inference kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants