Fix CodeQL code scanning alerts by silverwind · Pull Request #36858 · go-gitea/gitea

silverwind · 2026-03-08T06:57:16Z

Fixes 10 CodeQL code scanning alerts:

Change NewPagination/SetLinkHeader to accept int64 for total count, clamping internally to fix incorrect-integer-conversion alerts (#110, #114, #115, #116)
Use strconv.Atoi() in htmlrenderer.go to avoid int64 intermediate (#105, #106)
Clamp regex match indices in escape_stream.go to fix allocation-size-overflow (#161, #162, #163)
Cap slice pre-allocation in GetIssueDependencies (#181)

Copilot

Pull request overview

Addresses multiple CodeQL alerts across the codebase by hardening integer conversions and bounding allocations, improving safety around pagination/count handling, slice preallocation, and URL joining.

Changes:

Introduces util.Int64ToInt (clamped int64→int conversion) and applies it in pagination, template error rendering, and XORM BeforeSet enum/type mapping.
Bounds/adjusts slice preallocation to avoid allocation-size overflow and uncontrolled allocation size flags.
Adds a guard in util.URLJoin to avoid a potential out-of-range slice on empty joined URLs.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
routers/web/repo/wiki.go	Uses safe int64→int conversion for wiki revision pagination.
routers/web/repo/commit.go	Uses safe int64→int conversion for commit graph and file history pagination.
routers/api/v1/repo/issue_dependency.go	Caps preallocated slice capacity for dependency list responses.
routers/api/v1/repo/commits.go	Uses safe int64→int conversion when setting pagination link headers.
modules/util/util.go	Adds `Int64ToInt` helper with clamping.
modules/util/url.go	Guards against empty `joinedURL` before slicing in `URLJoin`.
modules/util/sanitize.go	Removes potentially overflowing capacity calculation in `SanitizeCredentialURLs`.
modules/templates/htmlrenderer.go	Uses safe conversion for template error line/pos extraction.
modules/charset/escape_stream.go	Bounds indexes and avoids potentially problematic capacity calculations.
models/repo/repo_unit.go	Uses safe conversion when mapping DB `type` to `unit.Type`.
models/auth/source.go	Uses safe conversion when mapping DB `type` to auth `Type`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

modules/util/util.go

wxiaoguang · 2026-03-08T07:15:55Z

I do not think it's right to introduce a global Int64ToInt function.

It should either correctly be handled by the callee function (e.g.: NewPagination), or use generic (Cell2Int64), or rewrite the "parsing" code (util.ToInt64)

wxiaoguang · 2026-03-08T07:24:33Z

By the way, the "fix" for bad-redirect-check is not complete

I have told you all the details in Prevent redirect bypasses via backslash-encoded paths #36660

wxiaoguang

need rewrite

silverwind · 2026-03-08T07:34:10Z

I do not think it's right to introduce a global Int64ToInt function.

It should either correctly be handled by the callee function (e.g.: NewPagination), or use generic (Cell2Int64), or rewrite the "parsing" code (util.ToInt64)

This will blow up the diff significantly, but I can try.

wxiaoguang · 2026-03-08T07:37:56Z

This will blow up the diff significantly,

It won't

but I can try.

It is simple task with less than 20 minutes without AI.

silverwind · 2026-03-08T08:19:33Z

By the way, the "fix" for bad-redirect-check is not complete

I have told you all the details in Prevent redirect bypasses via backslash-encoded paths #36660

It's not quite the same scenario as in #36660 and it's a false-positive, I've reverted the change.

silverwind · 2026-03-08T08:22:57Z

NewPagination and related funcs now takes int64. I'm not totally sold on the idea of functions taking a mix of int and int64 but maybe you like that more.

wxiaoguang · 2026-03-08T08:34:52Z

NewPagination and related funcs now takes int64. I'm not totally sold on the idea of functions taking a mix of int and int64 but maybe you like that more.

"Generic"

wxiaoguang · 2026-03-08T08:35:07Z

NewPagination and related funcs now takes int64. I'm not totally sold on the idea of functions taking a mix of int and int64 but maybe you like that more.

"Generic"

Or fix the "callers"

wxiaoguang · 2026-03-08T08:36:40Z

URLJoin should be completely fixed (avoided) like this Deprecated URLJoin #36861

silverwind · 2026-03-08T08:40:08Z

"Generic"

Can't make SetLinkHeader generic yet because generic methods are not yet supported in golang, but the other cases can be fixed. I will try.

Address CodeQL alerts by handling integer conversions at callee level using generics and bounding allocations: - Make paginator.New and NewPagination generic [T ~int | ~int64] to safely accept both int and int64, clamping internally - Change SetLinkHeader to accept int64 (methods can't be generic in Go) - Use strconv.Atoi(fmt.Sprint(...)) in htmlrenderer to avoid int64 intermediate - Clamp regex match indices in escape_stream to prevent allocation-size-overflow - Fix over-allocation in SanitizeCredentialURLs - Cap slice pre-allocation in GetIssueDependencies Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>

# Conflicts: # routers/web/repo/projects.go

wxiaoguang · 2026-03-08T08:58:42Z

I would suggest to spend more time on studying the legacy code.

There are many bugs. We can't just only add patches to the bugs

wxiaoguang · 2026-03-08T09:02:00Z

Don't use "Generic" for "count". I will fix.

silverwind · 2026-03-08T09:06:08Z

Ok go ahead.

silverwind · 2026-03-08T10:51:39Z

PR description updated, this will resolve 10 CodeQL issues.

TheFox0x7

lgtm overall safe for one thing.

TheFox0x7 · 2026-03-08T13:53:55Z

routers/web/repo/projects.go

-	}
-
-	pager := context.NewPagination(total, setting.UI.IssuePagingNum, page, numPages)
+	pager := context.NewPagination(count, setting.UI.IssuePagingNum, page, 5)


Because it is the final answer, just like 42 .....

The previous calculation was buggy and 5 is the value that every other caller passes to this function, essentially it's a useless argument because no caller changes it.

And for what this number does:

It controls how many page number buttons appear in the pagination (e.g. ... 2 3 4 5 6 ...)

* origin/main: Optimize Docker build with dependency layer caching (go-gitea#36864) Fix URLJoin, markup render link reoslving, sign-in/up/linkaccount page common data (go-gitea#36861) Fix CodeQL code scanning alerts (go-gitea#36858) Refactor auth middleware (go-gitea#36848) Update Nix flake (go-gitea#36857) Update JS deps (go-gitea#36850) Load `mentionValues` asynchronously (go-gitea#36739) [skip ci] Updated translations via Crowdin

silverwind · 2026-03-08T17:53:49Z

This PR fixed 10 issues but brought up 6 new ones:

CodeQL feels like whack-a-mole 😆.

wxiaoguang · 2026-03-08T18:40:51Z

That's why I didn't continue after Address some CodeQL security concerns #35572

silverwind · 2026-03-08T18:42:52Z

I think we need some make codeql to run these checks locally so we can be sure none regress.

* main: (26 commits) Clean up `refreshViewedFilesSummary` (go-gitea#36868) Remove `util.URLJoin` and replace all callers with direct path concatenation (go-gitea#36867) Optimize Docker build with dependency layer caching (go-gitea#36864) Fix URLJoin, markup render link reoslving, sign-in/up/linkaccount page common data (go-gitea#36861) Fix CodeQL code scanning alerts (go-gitea#36858) Refactor auth middleware (go-gitea#36848) Update Nix flake (go-gitea#36857) Update JS deps (go-gitea#36850) Load `mentionValues` asynchronously (go-gitea#36739) [skip ci] Updated translations via Crowdin Fix dbfs error handling (go-gitea#36844) Fix OAuth2 authorization code expiry and reuse handling (go-gitea#36797) Fix org permission API visibility checks for hidden members and private orgs (go-gitea#36798) Fix non-admins unable to automerge PRs from forks (go-gitea#36833) upgrade to github.com/cloudflare/circl 1.6.3, svgo 4.0.1, markdownlint-cli 0.48.0 (go-gitea#36837) Fix dump release asset bug (go-gitea#36799) build(deps): update material-icon-theme v5.32.0 (go-gitea#36832) Fix bug to check whether user can update pull request branch or rebase branch (go-gitea#36465) Fix forwarded proto handling for public URL detection (go-gitea#36810) Fix artifacts v4 backend upload problems (go-gitea#36805) ... # Conflicts: # pnpm-lock.yaml

* giteaofficial/main: Update minimum go version to 1.26.1, golangci-lint to 2.11.2, fix test style (go-gitea#36876) Add render cache for SVG icons (go-gitea#36863) Fix incorrect viewed files counter if reverted change was viewed (go-gitea#36819) [skip ci] Updated translations via Crowdin Clean up `refreshViewedFilesSummary` (go-gitea#36868) Remove `util.URLJoin` and replace all callers with direct path concatenation (go-gitea#36867) Optimize Docker build with dependency layer caching (go-gitea#36864) Fix URLJoin, markup render link reoslving, sign-in/up/linkaccount page common data (go-gitea#36861) Fix CodeQL code scanning alerts (go-gitea#36858) Refactor auth middleware (go-gitea#36848) Update Nix flake (go-gitea#36857) Update JS deps (go-gitea#36850) Load `mentionValues` asynchronously (go-gitea#36739) [skip ci] Updated translations via Crowdin Fix dbfs error handling (go-gitea#36844) Fix OAuth2 authorization code expiry and reuse handling (go-gitea#36797) Fix org permission API visibility checks for hidden members and private orgs (go-gitea#36798)

silverwind · 2026-03-10T19:45:53Z

I've gone through all 50 open ones with Claude and most were false positives, they are all dismissed now with appropriate comments. The remaining 12 might be real:

https://github.com/go-gitea/gitea/security/code-scanning

silverwind · 2026-03-10T19:57:05Z

Upon further analysis, all but #36884 were false positives, I dismissed them all. Once that one is merged CodeQL will be clean.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 8, 2026

github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Mar 8, 2026

silverwind added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 8, 2026

silverwind requested a review from Copilot March 8, 2026 07:01

Copilot started reviewing on behalf of silverwind March 8, 2026 07:01 View session

Copilot AI reviewed Mar 8, 2026

View reviewed changes

modules/util/util.go Outdated Show resolved Hide resolved

wxiaoguang marked this pull request as draft March 8, 2026 07:16

wxiaoguang previously requested changes Mar 8, 2026

View reviewed changes

GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 8, 2026

silverwind force-pushed the codeql1 branch from 4639253 to 7efebc3 Compare March 8, 2026 08:52

fix incorrect logic

f57901d

# Conflicts: # routers/web/repo/projects.go

silverwind force-pushed the codeql1 branch from f57901d to d6e9f05 Compare March 8, 2026 08:58

clarify

1ba590f

wxiaoguang force-pushed the codeql1 branch from eeee873 to 1ba590f Compare March 8, 2026 09:03

GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Mar 8, 2026

wxiaoguang approved these changes Mar 8, 2026

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 8, 2026

silverwind marked this pull request as ready for review March 8, 2026 10:42

wxiaoguang added 2 commits March 8, 2026 21:43

fix

2ed036e

fix lint

7a51f09

TheFox0x7 approved these changes Mar 8, 2026

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 8, 2026

wxiaoguang added 2 commits March 8, 2026 21:57

Merge branch 'main' into codeql1

b1fd085

fix another NewPagination bug caused by copy-paste

6b9175e

wxiaoguang enabled auto-merge (squash) March 8, 2026 14:22

wxiaoguang disabled auto-merge March 8, 2026 14:22

wxiaoguang enabled auto-merge (squash) March 8, 2026 14:23

wxiaoguang merged commit 0724344 into go-gitea:main Mar 8, 2026
26 checks passed

GiteaBot added this to the 1.26.0 milestone Mar 8, 2026

wxiaoguang deleted the codeql1 branch March 8, 2026 14:35

bircni mentioned this pull request Mar 8, 2026

Feature: Add per-runner “Disable/Pause” #36776

Merged

silverwind mentioned this pull request Mar 10, 2026

feat(api): add REST API for repository project boards #36831

Draft

Uh oh!

Conversation

silverwind commented Mar 8, 2026 • edited by wxiaoguang Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

wxiaoguang commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang left a comment

Choose a reason for hiding this comment

Uh oh!

silverwind commented Mar 8, 2026

Uh oh!

wxiaoguang commented Mar 8, 2026

Uh oh!

silverwind commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Mar 8, 2026

Uh oh!

wxiaoguang commented Mar 8, 2026

Uh oh!

wxiaoguang commented Mar 8, 2026

Uh oh!

silverwind commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Mar 8, 2026

Uh oh!

wxiaoguang commented Mar 8, 2026

Uh oh!

silverwind commented Mar 8, 2026

Uh oh!

silverwind commented Mar 8, 2026

Uh oh!

TheFox0x7 left a comment

Choose a reason for hiding this comment

Uh oh!

TheFox0x7 Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

wxiaoguang Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

TheFox0x7 Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

silverwind Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

silverwind Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

silverwind commented Mar 8, 2026

Uh oh!

wxiaoguang commented Mar 8, 2026

Uh oh!

silverwind commented Mar 8, 2026

Uh oh!

silverwind commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Mar 10, 2026

Uh oh!

Reviewers

silverwind commented Mar 8, 2026 •

edited by wxiaoguang

Loading

wxiaoguang commented Mar 8, 2026 •

edited

Loading

wxiaoguang commented Mar 8, 2026 •

edited

Loading

silverwind commented Mar 8, 2026 •

edited

Loading

silverwind commented Mar 8, 2026 •

edited

Loading

silverwind commented Mar 8, 2026 •

edited

Loading

silverwind Mar 8, 2026 •

edited

Loading

silverwind Mar 8, 2026 •

edited

Loading

silverwind commented Mar 10, 2026 •

edited

Loading