Skip to content

Security, performance and stability quick wins - #183

Open
Sparksx wants to merge 1 commit into
mainfrom
claude/focused-curie-ebhov1
Open

Security, performance and stability quick wins#183
Sparksx wants to merge 1 commit into
mainfrom
claude/focused-curie-ebhov1

Conversation

@Sparksx

@Sparksx Sparksx commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

Full codebase audit across security, performance, stability, and functionality — followed by implementation of the 16 highest-priority quick wins (high impact, low difficulty).

Security (7 fixes)

  • Ban enforcement on HTTP routes — banned users could still save state, fight PvP, join clans, and buy gold via API. Added requireNotBanned middleware on game/clans/pvp/payment routes
  • JWT algorithm restrictionjwt.verify() now explicitly requires HS256 on all 3 verification points (access, refresh, socket) to prevent algorithm confusion attacks
  • Distinct dev JWT secrets — access and refresh secrets shared the same dev fallback, letting a 30-day refresh token work as an access token
  • Content Security Policy — CSP was explicitly disabled in Helmet; enabled with directives for Google OAuth, Stripe, and WebSocket
  • PvP rate limiting — added 20 fights/min per IP to prevent rating farming
  • Username character validation — restricted to [A-Za-z0-9_\- ] to prevent impersonation ([ADMIN] System) and control characters
  • Settings key whitelistPUT /api/auth/settings now only accepts known keys (theme, language, sfx, music)

Performance (4 fixes)

  • HTTP compression — added compression middleware (gzip/brotli)
  • Cache-Control headers — Vite hashed assets get max-age=1y, immutable; shell files get 10 min
  • Missing DB indexesUser.updatedAt (PvP opponent pool), Mission(clanId, type, status), Purchase.stripePaymentId (refund lookup)
  • Parallel DB queriesloadActorAndTarget now uses Promise.all instead of sequential queries

Stability (3 fixes)

  • Race conditions in registration & username change — replaced check-then-act pattern with direct create/update + P2002 catch
  • Competing shutdown handlers — removed duplicate SIGINT/SIGTERM handler in prisma.js that raced with index.js shutdown
  • Token cleanup interval — stored interval ref and cleared on shutdown to prevent post-disconnect DB queries

Functionality (2 fixes)

  • PvP defender ELO — both attacker and defender ratings now update (was attacker-only, breaking the ranking system)
  • Monster Hunt missionCOMBAT_MONSTER_DEFEATED event was never emitted, making the defeat_enemies mission (target: 1500) impossible to complete

Test plan

  • All 524 existing tests pass (npm test)
  • Lint clean — no new warnings (npm run lint)
  • Production build succeeds (npm run build)
  • Verify banned user gets 403 on gameplay API routes
  • Verify PvP fight rate limit triggers after 20 fights/min
  • Verify username with special characters is rejected at registration
  • Verify defender ELO changes after a PvP fight
  • Verify defeat_enemies mission progresses after arena wins

Generated by Claude Code

Security:
- Add ban enforcement on HTTP API routes (game, clans, pvp, payment)
- Restrict JWT verification to HS256 algorithm (access, refresh, socket)
- Differentiate dev JWT secrets for access vs refresh tokens
- Enable Content Security Policy in Helmet with proper directives
- Add per-IP rate limiting on PvP fights (20/min)
- Add username character validation (letters, numbers, spaces, hyphens, underscores)
- Whitelist settings keys to prevent storage bloat

Performance:
- Add compression middleware (gzip/brotli)
- Add Cache-Control headers (1y immutable for hashed assets, 10m for shell)
- Add missing DB indexes (User.updatedAt, Mission type+status, Purchase.stripePaymentId)
- Parallelize loadActorAndTarget queries with Promise.all

Stability:
- Fix race condition in registration and username change (catch P2002 instead of check-then-act)
- Remove competing shutdown handlers (prisma.js vs index.js)
- Store and clear token cleanup interval on shutdown

Functionality:
- Update defender ELO in PvP (both players' ratings now change)
- Emit COMBAT_MONSTER_DEFEATED event so defeat_enemies missions can progress

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xc2Hvgc65no84vRm33PUnt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants