feature/deployment-readiness #3

Open

marti wants to merge 15 commits from feature/deployment-readiness into dev

pull from: feature/deployment-readiness

merge into: wysi:dev

wysi:main

wysi:audit

wysi:dev

wysi:feature/add-skins

Conversation 0 Commits 15 Files changed 51 +2209 -235

marti commented 2026-10-01 17:23:48 +02:00

Owner

• Copy link

No description provided.

marti added 11 commits 2026-10-01 17:23:48 +02:00

fix(auth): serialise the osu! grant exchange on its row ... eeb923255e

osu! invalidates a refresh token the moment it is exchanged. Two concurrent
exchanges for one user raced: the loser errored, and its write could land last
and overwrite the newer refresh token with the older one, leaving the user
holding a token osu! will never accept again and forcing them back through the
OAuth flow.

The exchange and the write now share one transaction holding a row lock, so a
late request reads the winner's fresh grant instead of exchanging the stale one.

fix(auth): make refresh-token rotation atomic ... 598262019d

Rotation was a read-then-write across two statements with no lock. Two
concurrent refreshes carrying the same token both read it as unconsumed and
both minted a successor: one was orphaned, and the replay the whole design
rests on never fired.

The check and the rotation now run in one transaction with the token row locked,
and RotateRefreshToken carries a revoked_at guard so a zero row count means the
token was already consumed. That zero is treated as a replay, which revokes the
family rather than leaving two live sessions.

A loser is indistinguishable from a leak, so it still revokes everything. That
is the documented behaviour, and it is why the frontend shares one in-flight
refresh; it must not rely on losing the race being harmless.

fix(auth): prune expired refresh tokens on a timer ... fb1f3e299b

DeleteExpiredRefreshTokens was generated and never called, so the table grew by
one row per refresh forever. A revoked row must be kept -- its presence is how a
replay is recognised -- which leaves expiry as the only way a token ever leaves
the table.

A failure is logged and retried on the next tick: a missed sweep costs disk, not
correctness.

feat(utils): validate the values that reach an upstream URL ... 1578b7bb63

The mode, score category, and beatmapset category all arrive from a path or
query parameter and are interpolated straight into an osu! API URL that is then
fetched with a real osu! token attached. Accepting anything else lets a caller
aim that request at a different path on the osu! API.

Each validator is an explicit allowlist rather than a character check, so a new
upstream mode has to be added deliberately.

fix(security): validate path and query parameters before building a URL ... cdc96e82b4

HandleGetUser passed :userId straight through, so /users/..%2f..%2fapi%2fv2%2fme
was forwarded to osu! with a bearer token attached. Every route now validates
the user id as a positive integer and the mode and category against their
allowlists.

Path parameters are checked before the pagination query, so a request with a
bad path reports the path problem rather than complaining about a missing
offset. The OpenAPI annotations gain the 400 they can now return.

fix(security): escape upstream query strings ... 7df19b8e3f

getUrl concatenated parameters with no escaping, so a value containing "&"
introduced parameters of its own into a request we sign. It now uses
url.Values, which also sorts the keys -- Go randomises map iteration, so the
previous ordering produced a different cache key per call and the five minute
response cache never hit.

fix(perf): give the HTML scrape a timeout ... f24ccb23d6

GetHTML used http.Get, which has none, and it runs inside a goroutine the
profile handler waits on: a hung osu! response held the whole request open. It
now goes through HTTPClient like every other outbound call, and identifies
itself, since osu! throttles clients it cannot name.

feat(ratelimit): send RateLimit headers the frontend can act on ... defb4adcd2

Echo's limiter sets no X-RateLimit headers, so the frontend had nothing to read
and fell back to a fixed guess that stalled every request it made. Responses
now carry the limit, the remaining count, and -- on a 429 -- Retry-After.

fix(db): bound the pool and stop a query from hanging forever ... b488287ecf

Every query runs on db.Ctx, a background context with no deadline, so a
statement that never returned would hold a connection indefinitely. The pool
also took pgx's defaults, which on a small container are more connections than
the memory behind them justifies.

Setting statement_timeout on the connection covers every query in one place,
which is why this is a pool setting rather than a context threaded through
every handler -- the per-request context is the better fix and is still open.

ci: audit reachable vulnerabilities ... b1194d9dea

The frontend ran bun audit and the server did not, so a Go advisory reached
main unnoticed. govulncheck is now a make target, part of make check, and a CI
step; it reports only vulnerabilities this code actually calls, which keeps it
honest rather than noisy.

docs: record the rate limit headers, the pool bounds, and the open gaps ... f3ac8eb5b4

The known-limitations list is the useful half here: a query still runs on a
background context rather than the request's, and GET /users/{id} still writes
to the database on every view. Both are bounded but neither is fixed, and the
next person to read this should not have to rediscover them.

marti added 4 commits 2026-10-01 19:39:29 +02:00

fix(db): run every request's queries on the request's own context ... dfa7217438

db.Ctx is a package-level context.Background, so no query was ever cancelled:
a caller who navigated away left the work running, and a slow statement held a
connection for as long as it cared to.

Each handler now passes c.Request().Context() down through the services to the
query set, and the osu! client carries the context on the Client so an outbound
call inherits the same deadline and cancellation. Three package-level functions
that have no receiver -- GetClientAuth, GetUserAuth, GetUserRefresh -- take one
explicitly.

The connection-level statement_timeout stays as a backstop for the paths that
genuinely run outside a request: the token sweep and the application-token
refresh.

perf(users): stop rewriting a profile on every view ... 58ecaad626

UpdateUser is reached from a GET and ran on every one of them: four inserts
plus a synchronous scrape of the osu! team leaderboard and an osuworld call for
the region rank. The inserts conflict-do-nothing, so repeat traffic bought a
slower response and a dependency on two upstreams answering -- nothing else.

One indexed read now answers "is today already recorded", and the whole write
is skipped when it is. A failed check falls through to the full path rather
than silently dropping someone's history.

The test counts outbound calls rather than rows, since a no-op insert is
invisible to a row count. Forcing the write back on makes it fail.

feat(ratelimit): count in Valkey so replicas share one budget ... 60e2ae5c27

Echo's in-memory store is per process, so N replicas allow N times the
configured rate. That is fine with one instance and quietly wrong with two,
which is the kind of thing that only shows up after a scale event.

The counter is one Lua script doing INCR and EXPIRE together: as two round
trips, a process dying between them leaves a key with no TTL, and that client
is locked out until someone notices.

The memory store stays underneath as a fallback, because the cache contract is
already that losing Valkey must make the service slower, never worse -- failing
requests because the cache is down would take the API with it. The tests cover
both halves against a real Valkey, and forcing the fallback back on makes the
sharing test fail.

feat(auth): seal the stored osu! refresh token ... 8ff59621cf

The osu! refresh token is unlike ours: it cannot be revoked, so anyone holding
it can mint osu! access tokens indefinitely. Stored in plaintext, a database
dump -- or a backup, or a replica -- is a full compromise of every user who has
ever signed in.

AES-256-GCM under a new required OSU_TOKEN_KEY. GCM authenticates as well as
encrypts, so a tampered row fails to open rather than decrypting to garbage, and
the nonce rides in the ciphertext, so there is no nonce column to migrate.

Open passes through a value that does not decode as base64, so rows written
before the key existed keep working rather than locking their owner out.
Rotating the key does invalidate every stored grant, which is recoverable --
users sign in again through osu!.

Some checks failed

Hide checks

CI / check (pull_request) Failing after 9m55s

Details

CI / openapi-drift (pull_request) Successful in 9m43s

Details

CI / sqlc-drift (pull_request) Failing after 5m28s

Details

CI / integration (pull_request) Failing after 5m41s

Details

CI / docker (pull_request) Failing after 4s

Details

CI / nix (pull_request) Failing after 8s

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'feature/deployment-readiness:feature/deployment-readiness'

git switch 'feature/deployment-readiness'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'dev'

git merge --no-ff 'feature/deployment-readiness'

git switch 'feature/deployment-readiness'

git rebase 'dev'

git switch 'dev'

git merge --ff-only 'feature/deployment-readiness'

git switch 'feature/deployment-readiness'

git rebase 'dev'

git switch 'dev'

git merge --no-ff 'feature/deployment-readiness'

git switch 'dev'

git merge --squash 'feature/deployment-readiness'

git switch 'dev'

git merge --ff-only 'feature/deployment-readiness'

git switch 'dev'

git merge 'feature/deployment-readiness'

git push origin 'dev'

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

wysi/server!3

No description provided.