Files
minecraft-account-manager/docs/security-review.md
T
dmg 2402e9e42d
CI / validate (push) Successful in 7m26s
Release / release (push) Successful in 9m43s
feat(admin): browse Discord suggestions in the portal
2026-09-10 08:21:08 -04:00

6.7 KiB

Security review

Review timestamp: 2026-09-10T12:20:32Z

Scope

Next.js portal and APIs, Discord bot, PostgreSQL persistence, Keycloak admin authentication, and the Velocity admission plugin.

Automated checks

  • Full source Semgrep auto: 0 findings across 178 scanned files (331 rules; 13 files excluded by ignore patterns).
  • npm audit and npm audit --omit=dev: 0 known vulnerabilities after remediation. Baseline had six affected entries (two moderate, two high, two critical), covering Next.js/Sharp, the NextAuth dependency path, Vitest/mocker, and js-yaml.
  • Resolved patched versions: Next.js and eslint-config-next 16.3.4, Sharp 0.35.4, Vitest/mocker 4.1.11, js-yaml 4.3.2. Retained Next.js's PostCSS override at 8.5.25. No forced major dependency upgrades or database changes.
  • TypeScript, 196 workspace tests, Next.js production build, and Velocity Java tests/shaded plugin build pass with patched dependencies. ESLint exits successfully with two new framework-rule warnings about existing window.location.assign() calls in map-view-toggle.tsx; those unrelated navigation behaviors were not changed.
  • Verification is local source evidence, not a claim that patched artifacts have been deployed. API CI run 1979 passed for 1a01c0ed641f4eda83f81c855c38651aa116933e; dependency-fix CI run 1985 passed for 0af4884f7e16161d0c0ea3a8f9c26b3fa9384cf9.
  • Suggestions UI tests verify both page guards, malformed-ID rejection, literal text rendering (including script-like content), read-only API usage, explicit auth/error states, aborted obsolete reads, and pagination focus. Actual Discord permissions/Message Content access and production browser acceptance remain rollout checks.

Implemented controls

  • Discord login and session tokens use cryptographically secure randomness and are stored only as SHA-256 hashes.
  • Login links expire after ten minutes, are single use, and are rate limited per Discord user with a PostgreSQL advisory lock.
  • Session cookies are httpOnly, sameSite=lax, path-scoped, and secure in production.
  • Admin access uses Keycloak OIDC and a required role.
  • Every suggestions API route independently requires that admin session/role before reading cached or live Discord data. The configured forum is guild/type-checked and requested threads are parent/type-checked before message access. Discord credentials remain server-side; fixed-host, validated-ID requests have bounded caching, concurrency, timeouts, and rate-limit backoff. Responses are uncached and errors use safe RFC 9457 problems. See suggestions API for live Discord permission/intent requirements.
  • User mutations verify ownership server-side.
  • Mojang lookup is server-side and targets a fixed host, avoiding client-forged validation and SSRF.
  • Velocity credentials are high-entropy bearer tokens stored only as hashes.
  • Velocity admission and confirmed-connection requests have a 45-second clock window and database-unique request IDs for cross-instance replay prevention.
  • Velocity and its API fail closed.
  • Registered players require an enabled effective group; explicit assignments replace rather than combine with the protected, disabled-by-default everyone fallback. Disabled access always overrides scheduling.
  • Enabled groups with recurring UTC windows admit only during an active login window. Window boundaries and overlap are validated server-side, malformed persisted policy fails closed, and connected sessions are not re-evaluated.
  • Confirmed VPN, proxy, and Tor game connections are denied unless that same effective group has an explicit exception; everyone and new groups default to no exception.
  • Static admission templates use a fixed allowlist of non-executable variables. Unknown placeholders and control characters are rejected, and rendered values remain plain JSON/text.
  • Group, schedule, VPN-policy, identity, and membership mutations re-check the Keycloak administrator role server-side; registry assignments, policy changes, group edits, creation, deletion, and message settings commit atomically with their audit events.
  • Group identity creation/rename and membership assignment/deletion use compatible PostgreSQL advisory and row locks to prevent duplicate names, stale audit records, or membership/deletion races. The protected default name and deletion restriction are enforced server-side.
  • Every bearer-authenticated Velocity login uses cached IP intelligence before identity resolution, preventing account-creation races from bypassing network policy; malformed provider proxy signals classify as unknown.
  • Event filters accept only event types already present in the ledger, and event detail routes remain role-protected.
  • The administrator-only map defaults to bundled Natural Earth boundaries. OpenStreetMap tile requests begin only after an explicit operator opt-in; marker coordinates are not transmitted as data, but the requested tiles disclose the viewed geographic extent along with the administrator's IP and portal origin.
  • Grouped-map popup labels and links are created with DOM textContent and server-rendered React escaping rather than interpolated HTML.
  • ORM-parameterized queries are used throughout.
  • CSP, clickjacking, MIME-sniffing, referrer, and browser-permission headers are configured.
  • Forwarded IP headers are ignored unless TRUST_PROXY=true is explicitly configured.
  • Private and reserved addresses are not sent to ProxyCheck.io; lookup results are cached to reduce disclosure and API usage.
  • Portal and game login events include approximate network location and VPN/proxy classification when available. The administrator-only location list also exposes enriched network company, ASN, connection type, and the provider's proxy signal.
  • Structured Pino logging redacts credential fields, and secrets are excluded from logs and repository configuration.

Outstanding production requirements

  • Monitor ProxyCheck.io usage, detection quality, and false positives. User account additions fail closed when classification is unavailable; hosting-provider blocking remains optional.
  • Define and automate retention for exact IP addresses, cached provider responses, approximate location, and audit events.
  • Add monitoring and alerts for repeated login denials, plugin authentication failures, and Discord API failures.
  • Use HTTPS for the public application and Velocity API URL. Protect the Velocity configuration file because it contains the one-time-displayed API token.
  • Restrict database credentials so normal application roles cannot update or delete historical event rows outside approved application paths.
  • Validate migrations in staging before production. Local migration application was unavailable during development because the Docker daemon was not running.