refactor!: move go-chi/session into Gitea by silverwind · Pull Request #39504 · go-gitea/gitea

silverwind · 2026-09-30T15:59:25Z

The gitea.com/go-chi/session package only exists for Gitea, so it moves into modules/session to fix its bugs directly. Fixes the flake in https://github.com/go-gitea/gitea/actions/runs/36726154500/job/109923538400.

Sessions are only written back when changed, so a read-only request can't revert a concurrent change or restore a logged-out session, like go-macaron/session@ae808a4
The session cookie is only set once a session holds data
Every backend refreshes the expiry on load and file sessions are written atomically
Also fix Nondescriptive cookie headers #36176

⚠️ BREAKING ⚠️

the mysql, postgres, couchbase and memcache session providers are removed, use file, db or redis instead
login-related cookies are renamed to gitea_session and gitea_remember, if you'd like to use the old names, set COOKIE_NAME and COOKIE_REMEMBER_NAME in app.ini

Every request wrote its own copy of the session back on release, so a request that only read the session reverted changes a concurrent request saved in the meantime, e.g. the WebAuthn registration state when the page's background polling overlapped it. Session stores now track changes and only refresh the expiry of unchanged sessions. The file store fix is in https://gitea.com/go-chi/session/pulls/14, pinned via replace until it is merged. Assisted-by: Claude Code:claude-opus-5-5

wxiaoguang · 2026-09-30T16:52:01Z

I was also working on the "session" package ....

Details

I believe it's not worth to make any patch to the chi-session package.

Either, we fully rewrite it, or, we write our own and completely drop it.

Gitea side already has a "virtual" wrapper for it, it really doesn't make sense to hack it more .......

Also, we only need to support a few providers, including: memory, redis, file, db. All other providers can be removed (breaking change, but worth)

silverwind · 2026-09-30T16:54:14Z

I'll move the session package first-party. Much better then having this single external dependency with only gitea as the only real consumer.

wxiaoguang · 2026-09-30T16:56:01Z

All legacy chi-* packages have various problems and bugs, and are only used by Gitea.

silverwind · 2026-09-30T16:59:18Z

Ok, migrating all.

These packages only exist for Gitea, and their bugs and the workarounds for them are simpler to fix in first-party code. Sessions are only written back when changed and the cookie is only set once a session holds data, so a read-only request can no longer revert a concurrent change or bring back a logged-out session. Captcha answers are consumed atomically by the cache, and OpenID registration stops after a failed captcha. The removed mysql, postgres, couchbase and memcache session providers fail at startup with a hint to use db or redis. Assisted-by: Claude Code:claude-opus-5-5

silverwind · 2026-09-30T22:05:10Z

Move done, fixed numerous bugs while doing so. The change is flagged breaking because of the session provider removals.

silverwind · 2026-10-01T12:02:12Z

CI failure is unrelated and needs #39512.

Copilot

Copilot review overview

🟡 Changes recommended

Four critical and two moderate findings remain unresolved.

Review effort: Lite
Findings: 4 · 2

Open (6)

GetAndDelete is non-atomic, allowing CAPTCHA answer reuse · New
Keys iteration races with concurrent cache mutations · New
Unauthenticated reload IDs enable unbounded cache memory growth · New
Shared session lock allows logout and save to race · New
Negative session lifetime causes immediate expiration · New
64-bit parsing silently truncates narrower integer fields · New

What changed in this PR

This PR moves Gitea-specific binding, cache, CAPTCHA, and session implementations into internal modules while updating related behavior, configuration, and tests.

Changes:

Adds internal binding, cache, CAPTCHA, and session implementations.
Updates CAPTCHA reload/single-use behavior and session persistence.
Removes obsolete providers and dependencies.

File	Reviewed changes / final status
`web_src/js/features/captcha.ts`	Adds CAPTCHA reload handling.
`tests/integration/signup_test.go`	Updates signup CAPTCHA coverage.
`tests/integration/session_test.go`	Updates session persistence coverage.
`tests/integration/repo_test.go`	Updates authenticated repository coverage.
`tests/integration/create_no_session_test.go`	Tests session cookie creation.
`templates/user/auth/signin_inner.tmpl`	Adjusts CAPTCHA display for linked accounts.
`templates/user/auth/captcha.tmpl`	Adds image CAPTCHA and reload controls.
`templates/admin/config.tmpl`	Displays cache configuration.
`services/forms/user_form.go`	Updates binding rules.
`services/forms/repo_form.go`	Updates repository form validation.
`services/forms/org.go`	Updates organization validation.
`services/forms/admin.go`	Updates administrative validation.
`services/contexttest/context_tests.go`	Updates session test setup.
`services/context/context_test.go`	Removes obsolete cookie tests.
`services/context/context_cookie.go`	Removes obsolete session cookie helper.
`services/context/captcha.go`	Uses internal CAPTCHA verification.
`services/context/base.go`	Removes redirect cookie workaround.
`services/context/base_test.go`	Removes obsolete redirect tests.
`services/auth/source/oauth2/store.go`	Uses the internal session store.
`services/auth/auth.go`	Updates session regeneration.
`routers/web/web.go`	Registers the internal CAPTCHA handler.
`routers/web/auth/openid.go`	Stops registration after failed CAPTCHA.
`routers/web/auth/linkaccount.go`	Updates CAPTCHA verification flow.
`routers/web/auth/auth.go`	Integrates internal CAPTCHA and sessions.
`routers/web/auth/auth_test.go`	Tests OpenID CAPTCHA rejection.
`routers/web/admin/config.go`	Removes virtual session configuration.
`routers/common/middleware.go`	Initializes internal sessions.
`routers/common/maintenancemode.go`	Updates CAPTCHA path documentation.
`options/locale/locale_en-US.json`	Adds CAPTCHA accessibility strings.
`modules/web/middleware/binding.go`	Switches to internal binding.
`modules/web/binding/binding.go`	Adds internal request binding; moderate width-validation issue remains.
`modules/web/binding/binding_test.go`	Tests internal binding behavior.
`modules/validation/binding.go`	Registers internal validation rules.
`modules/validation/binding_test.go`	Tests binding rule registration.
`modules/structs/user.go`	Updates optional URL validation.
`modules/structs/repo.go`	Updates repository name validation.
`modules/structs/org_team.go`	Updates team visibility validation.
`modules/structs/form.go`	Uses internal binding types.
`modules/structs/admin_user.go`	Updates optional URL validation.
`modules/setting/session.go`	Updates session configuration; negative lifetime handling issue remains.
`modules/setting/session_test.go`	Tests session lifetime defaults.
`modules/setting/cache.go`	Updates cache TTL conversion.
`modules/setting/cache_test.go`	Tests cache TTL behavior.
`modules/session/virtual.go`	Removes the virtual provider.
`modules/session/store.go`	Implements session store lifecycle.
`modules/session/session.go`	Adds internal session middleware.
`modules/session/session_test.go`	Tests session backends and concurrency.
`modules/session/redis.go`	Implements Redis sessions.
`modules/session/mem.go`	Implements memory sessions.
`modules/session/main_test.go`	Initializes session tests.
`modules/session/file.go`	Implements file sessions; concurrent logout resurrection issue remains.
`modules/session/db.go`	Implements database sessions.
`modules/imagecaptcha/imagecaptcha.go`	Implements CAPTCHA storage/serving; unbounded arbitrary refresh issue remains.
`modules/imagecaptcha/imagecaptcha_test.go`	Tests CAPTCHA lifecycle and rendering.
`modules/imagecaptcha/image.go`	Implements CAPTCHA image generation.
`modules/git/last_commit_cache.go`	Honors disabled cache TTLs.
`modules/cache/string_cache.go`	Adds the cache abstraction.
`modules/cache/cache.go`	Handles disabled item caching.
`modules/cache/cache_twoqueue.go`	Implements two-queue caching; concurrent key iteration race remains.
`modules/cache/cache_test.go`	Tests cache adapters and expiration.
`modules/cache/cache_redis.go`	Implements Redis caching.
`modules/cache/cache_memory.go`	Implements memory caching.
`modules/cache/cache_memcache.go`	Implements memcache caching; non-atomic CAPTCHA consumption issue remains.
`models/auth/session.go`	Updates session persistence APIs.
`go.sum`	Removes obsolete dependency checksums.
`go.mod`	Replaces external packages and adds memcache dependency.
`custom/conf/app.example.ini`	Updates cache and session documentation.
`cmd/dump.go`	Updates file-session dump handling.
`assets/go-licenses.json`	Updates dependency licenses.
`.golangci.yml`	Removes obsolete dependency restrictions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

wxiaoguang · 2026-10-01T12:18:56Z

Move done, fixed numerous bugs while doing so. The change is flagged breaking because of the session provider removals.

Can we migrate the packages one by one? It is difficult to review the mixed changes .......

silverwind · 2026-10-01T12:26:53Z

Yeah I will split into 1 PR per package I guess after this review round is done.

Every session backend now refreshes the expiry when a session is loaded, so a request outliving the session, like a websocket, can no longer revive a DB session that expired in the meantime. JSON request bodies are decoded as a stream again instead of being buffered whole. Integer form fields are parsed at their own width, so out-of-range values are rejected instead of wrapping, and a negative SESSION_LIFE_TIME falls back to the default like GC_INTERVAL_TIME. Assisted-by: Claude Code:claude-opus-5-5

Binding, cache and captcha move into Gitea in their own pull requests, so this one only replaces go-chi/session. Assisted-by: Claude Code:claude-opus-5-5

A websocket opened by reverse proxy or SSPI sign-in only released its session when the connection closed, so the next request signed in again under a new ID and a later logout missed the open connection. Assisted-by: Claude Code:claude-opus-5-5

Assisted-by: Claude Code:claude-opus-5-5

silverwind · 2026-10-01T14:58:08Z

Split done, this PR now has the session module removal.

The `gitea.com/go-chi/binding` package only exists for Gitea, so it moves into `modules/web/binding` to fix its bugs directly. Split out of #39504. - GET and HEAD always bind the query - JSON `null` slice elements and nested `TrimSpace` fields bind correctly - Integer fields reject out-of-range values instead of wrapping - An empty JSON body binds nothing and an unknown binding rule is an error Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: bircni <bircni@icloud.com>

wxiaoguang · 2026-10-02T16:41:50Z

Maybe also a good chance to rename CookieName: "i_like_gitea", what do you think?

silverwind · 2026-10-02T16:42:48Z

Sure, call it out as a breaking change.

wxiaoguang · 2026-10-02T17:04:04Z

Sure, call it out as a breaking change.

Renamed to "gitea_session" and "gitea_remember" 3015299

silverwind · 2026-10-02T17:28:40Z

Sure, call it out as a breaking change.

Renamed to "gitea_session" and "gitea_remember" 3015299

Added reference to #36176 in OP.

silverwind · 2026-10-02T19:10:02Z

docs: https://gitea.com/gitea/docs/pulls/572

* origin/main: fix(git): tolerate concurrent repacks in go-git storage (go-gitea#39536) [skip ci] Updated translations via Crowdin chore(lint): apply the main eslint config to vue files (go-gitea#39545) fix(markup): use installed math fonts for MathML in Chromium (go-gitea#39491) refactor: only update sync status columns when syncing push mirror (go-gitea#39517) refactor!: move go-chi/session into Gitea (go-gitea#39504) refactor: move go-chi/cache into Gitea (go-gitea#39530) refactor: move go-chi/captcha into Gitea (go-gitea#39529) refactor: move go-chi/binding into Gitea (go-gitea#39528) fix: use READ COMMITTED transactions on MySQL and MariaDB (go-gitea#39506) fix(git): avoid unnecessary timers during language stats (go-gitea#39531) perf(git): speed up activity top authors and subdirectory listings (go-gitea#39526) Assisted-by: Claude Code:claude-opus-5-5 # Conflicts: # go.sum # modules/git/repo_base_gogit.go

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

github-actions Bot added the type/bug label Sep 30, 2026

silverwind marked this pull request as draft September 30, 2026 17:24

github-actions Bot added topic/code-linting docs-update-needed The document needs to be updated synchronously labels Sep 30, 2026

silverwind changed the title ~~fix: don't write back unchanged sessions~~ refactor!: move go-chi binding, cache, captcha and session into Gitea Sep 30, 2026

silverwind marked this pull request as ready for review September 30, 2026 22:06

silverwind added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Sep 30, 2026

github-actions Bot added type/refactoring Existing code has been cleaned up. There should be no new functionality. and removed type/bug labels Sep 30, 2026

Merge branch 'main' into session-race

97243b8

silverwind requested a lite review from Copilot October 1, 2026 12:03

Copilot started reviewing on behalf of silverwind October 1, 2026 12:04 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

silverwind added 3 commits October 1, 2026 14:47

refactor: limit this change to the session package

e463da4

Binding, cache and captcha move into Gitea in their own pull requests, so this one only replaces go-chi/session. Assisted-by: Claude Code:claude-opus-5-5

github-actions Bot removed the topic/code-linting label Oct 1, 2026

This was referenced Oct 1, 2026

refactor: move go-chi/binding into Gitea #39528

Merged

refactor: move go-chi/captcha into Gitea #39529

Merged

silverwind mentioned this pull request Oct 1, 2026

refactor: move go-chi/cache into Gitea #39530

Merged

silverwind changed the title ~~refactor!: move go-chi binding, cache, captcha and session into Gitea~~ refactor!: move go-chi/session into Gitea Oct 1, 2026

refactor: simplify session tests and drop restating comments

236b8da

Assisted-by: Claude Code:claude-opus-5-5

wxiaoguang marked this pull request as draft October 2, 2026 16:22

wxiaoguang added 2 commits October 3, 2026 00:25

Merge branch 'main' into session-race

cbf1613

fix session id generation

a04fc60

wxiaoguang added 2 commits October 3, 2026 00:57

fix session id

8455cca

update cookie name

3015299

wxiaoguang added 3 commits October 3, 2026 01:12

clean up

28f6230

fine tune

20291f2

fine tune

13cb95e

wxiaoguang marked this pull request as ready for review October 2, 2026 17:30

wxiaoguang approved these changes Oct 2, 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 Oct 2, 2026

bircni approved these changes Oct 2, 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 Oct 2, 2026

silverwind merged commit cf89ecd into go-gitea:main Oct 2, 2026
37 checks passed

GiteaBot added this to the 29.0.0 milestone Oct 2, 2026

silverwind deleted the session-race branch October 2, 2026 19:08

Uh oh!

Conversation

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

Uh oh!

⚠️ BREAKING ⚠️

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

wxiaoguang commented Sep 30, 2026

Uh oh!

silverwind commented Sep 30, 2026

Uh oh!

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

Uh oh!

Uh oh!

silverwind commented Oct 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wxiaoguang commented Oct 1, 2026

Uh oh!

silverwind commented Oct 1, 2026

Uh oh!

silverwind commented Oct 1, 2026

Uh oh!

wxiaoguang commented Oct 2, 2026

Uh oh!

silverwind commented Oct 2, 2026

Uh oh!

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

Uh oh!

Uh oh!

silverwind commented Oct 2, 2026

Uh oh!

Uh oh!

silverwind commented Oct 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

silverwind commented Sep 30, 2026 •

edited by wxiaoguang

Loading

wxiaoguang commented Sep 30, 2026 •

edited

Loading

silverwind commented Sep 30, 2026 •

edited

Loading

silverwind commented Sep 30, 2026 •

edited

Loading

wxiaoguang commented Oct 2, 2026 •

edited

Loading