Skip to content

fix(server): DB rate-limiter cleanup worker runs on a request context and never ticks #269

Description

@cristim

Summary

reinitializeAfterConnect starts the DB-backed rate limiter's cleanup goroutine with the context it was called with. That context is the 30-second request context built in handleHTTPRequest, or the Lambda invocation context, because ensureDB runs lazily on the first request. The worker's ticker period is 10 minutes, so the goroutine is always cancelled long before its first tick and cleanup() never runs from it. The documented 02-M2 mitigation, evicting perpetually-denied keys whose count never resets to 1, is therefore inert. Blast radius is narrow: the worker is only wired when IsLambda is true, and the opportunistic count==1 cleanup still runs, so the effect is unbounded rate_limits growth for keys under sustained denial, on Lambda deployments only. A secondary effect is that every failed reinitializeAfterConnect retry starts one more short-lived goroutine.

Location

internal/server/app.go:754 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd

Related: internal/server/http.go:158 (the 30s request context), internal/server/app.go:633 (ensureDB -> reinitializeAfterConnect), internal/api/db_rate_limiter.go:35 and :65-73 (10-minute ticker, exits on ctx.Done()).

Failure scenario

A Lambda deployment serves its first request. ensureDB connects, reinitializeAfterConnect builds the DBRateLimiter and calls StartCleanupWorker(ctx) with the request context. The request finishes, cancel() fires, and the worker's select takes ctx.Done() before the ticker ever fires. An attacker who keeps a login key permanently over its limit has a row whose count never returns to 1, so the opportunistic cleanup never evicts it, and the scheduled cleanup that was meant to catch that case has already exited. The table grows for as long as the abuse lasts.

Evidence

	if app.appConfig.IsLambda {
		dbRL := api.NewDBRateLimiter(dbConn.Pool())
		// Start the scheduled cleanup worker so perpetually-denied keys (whose
		// count never resets to 1) are still evicted on a fixed schedule (02-M2).
		dbRL.StartCleanupWorker(ctx)
		app.RateLimiter = dbRL

Suggested fix

Give Application a process-lifetime context (cancelled in Close) and start the worker with that instead of the caller's context, and start it only once.


Found by the 2026-09-02 codebase audit, finding A06-004, reported by one reviewer and independently
confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions