Skip to content

Fix 12 quick-win issues: security, stability, performance - #189

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

Fix 12 quick-win issues: security, stability, performance#189
Sparksx wants to merge 1 commit into
mainfrom
claude/focused-curie-8vbnkt

Conversation

@Sparksx

@Sparksx Sparksx commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

Comprehensive audit of the Gear Master codebase across security, stability, performance, and functionality — followed by implementation of the 12 highest-impact, lowest-effort fixes.

Security (3 fixes)

  • Ban enforcement on REST API — banned users could still save game state, fight PvP, manage clans, and make purchases. Added requireNotBanned middleware to game/payment/clan/pvp route groups.
  • XSS in admin sprite modals.file was interpolated into innerHTML without escapeHtml().
  • Username change TOCTOU race — removed check-then-update pattern; now catches Prisma P2002 unique constraint violation directly, returning a proper 409.

Stability (4 fixes)

  • Competing shutdown handlersserver/lib/prisma.js had its own SIGTERM/SIGINT handlers that called process.exit(0) before the HTTP server could drain connections. Removed in favor of the properly sequenced shutdown in index.js.
  • Atomic refund clawback — payment refund status update and gold deduction were separate operations; wrapped in a single prisma.$transaction.
  • Atomic clan leave — member deletion and ownership transfer were not atomic; concurrent leaves could break the ownership chain. Wrapped in an interactive transaction.
  • Startup resilience — migrations/seed now wrapped in try/catch with process.exit(1) on failure; health endpoint verifies DB connectivity via SELECT 1.

Performance (3 fixes)

  • Compression middleware — all HTTP responses were served uncompressed (60-80% payload reduction for text).
  • Missing DB indexes — added indexes on User.pvpWins (leaderboard), User.updatedAt (PvP opponent pool), Purchase.stripePaymentId (refund lookup).
  • Static asset caching — Vite-hashed assets now served with immutable, max-age=1y; app shell gets max-age=1h.

Functionality (2 fixes)

  • Admin reset-state defaults — server used gold: 0, diamonds: 100 while client uses STARTING_GOLD (100) and diamonds are dormant. Fixed to gold: 100, diamonds: 0.
  • Admin forge level prompt — showed stale range "1–12" instead of actual max 1–35.

Remaining findings (not implemented — higher effort)

The full audit identified 20+ additional items documented in the commit. The most impactful remaining work:

  1. Client-authoritative gold (high impact / high effort) — the entire gold economy is client-side; a modified client can set arbitrary values
  2. Server-side caching layer (high impact / medium effort) — zero caching; every request hits the DB
  3. Sprite sheet optimization (high impact / medium effort) — 25 MB of unoptimized PNGs
  4. Code splitting (medium impact / medium effort) — all screens in one bundle

Test plan

  • All 524 tests pass (npm test)
  • ESLint clean — 0 errors, only pre-existing warnings (npm run lint)
  • Production build succeeds (npm run build)
  • Verify banned user gets 403 on game/pvp/clan/payment endpoints
  • Verify refund webhook atomically claws back gold
  • Verify clan ownership transfers correctly when owner leaves

Generated by Claude Code

Security:
- Add requireNotBanned middleware enforcing bans on REST API routes
- Fix XSS in admin sprite modal (escapeHtml on s.file)

Stability:
- Remove competing shutdown handlers in prisma.js (duplicate SIGTERM/SIGINT)
- Wrap payment refund clawback in atomic transaction
- Make clan leave + ownership transfer atomic
- Fix username change TOCTOU race (catch P2002 instead of check-then-update)
- Wrap startup migrations in try/catch with process.exit(1) on failure
- Health endpoint now verifies DB connectivity via SELECT 1

Performance:
- Add compression middleware for HTTP responses
- Add missing DB indexes on User.pvpWins, User.updatedAt, Purchase.stripePaymentId
- Serve hashed static assets with immutable cache headers (1y), app shell with 1h

Functionality:
- Fix admin reset-state defaults (gold: 100, diamonds: 0 to match client)
- Fix admin forge level prompt range (1-35 instead of stale 1-12)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JwKUKsHsJaYvUYUgGtZHmd
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