diff --git a/.env.example b/.env.example index a20d114..957f3f0 100644 --- a/.env.example +++ b/.env.example @@ -4,6 +4,8 @@ CORS_ORIGIN="https://minepanel.xyz" # required in production; exact frontend or # Compose defaults to `latest` for the future stable channel; no stable release is published yet. # For current pre-stable deployments, set this explicitly to the `edge` image. MINEPANEL_IMAGE=ghcr.io/minepanelproject/minepanel-backend:latest +# Required managed-Minecraft image identity. The digest is a verified amd64/arm64 manifest. +MINECRAFT_IMAGE=itzg/minecraft-server:2026.8.2@sha256:efa878ddb49cf5251b2e5f2ad71b08fd2f7236c1f7907433f6697258b31d2ce4 # ─── Database ───────────────────────────────────────────────────────────────── POSTGRES_PASSWORD=your-secure-password-here @@ -24,8 +26,6 @@ ENCRYPTION_KEY= SETUP_TOKEN= REQUIRE_ADMIN_APPROVAL=true # true = new users start as PENDING, admin must approve -LOGIN_THROTTLE_LIMIT=5 # max login attempts per window -LOGIN_THROTTLE_TTL_MS=10000 # window duration in ms (default: 10s) # Optional. Enables Google ID-token login; use the trusted frontend OAuth client ID. # The backend derives its frontend origin from CORS_ORIGIN and never accepts an origin from requests. GOOGLE_CLIENT_ID= @@ -65,6 +65,8 @@ MC_PORT_MAX=25665 MIN_FREE_DISK_MB=2048 MAX_MEMORY_RATIO=0.90 STOP_WARN_SECONDS=30 # seconds to warn players before graceful shutdown (0-300, default 30) +MC_CPU_NANO_CPUS=2000000000 # global per-container quota; 2 CPUs +MC_PIDS_LIMIT=512 # finite per-container process limit # ─── Volume / network names (overridable for disposable tests) ─────────────── POSTGRES_VOLUME_NAME=minepanel-postgres-data diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6a42c6a..8332606 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -266,10 +266,59 @@ jobs: severity: HIGH ignore-unfixed: true + trusted-lifecycle: + runs-on: ubuntu-latest + timeout-minutes: 20 + needs: [test, migration, e2e, image] + if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/tags/v')) + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: minepanel + POSTGRES_PASSWORD: minepanel + POSTGRES_DB: minepanel + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U minepanel" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + env: + DATABASE_URL: postgresql://minepanel:minepanel@localhost:5432/minepanel + JWT_SECRET: trusted-lifecycle-jwt-secret-long-enough + JWT_EXPIRES_IN: 15m + JWT_REFRESH_EXPIRES_IN: 7d + ENCRYPTION_KEY: '0000000000000000000000000000000000000000000000000000000000000000' + CORS_ORIGIN: http://127.0.0.1:3000 + DOCKER_SOCKET: /var/run/docker.sock + MINECRAFT_IMAGE: itzg/minecraft-server:2026.8.2@sha256:efa878ddb49cf5251b2e5f2ad71b08fd2f7236c1f7907433f6697258b31d2ce4 + steps: + - name: Check out repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Install Bun + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: 1.3.14 + + - name: Install dependencies (frozen lockfile) + run: bun install --frozen-lockfile + + - name: Apply migrations + run: bun run db:migrate + + - name: Build production application + run: bun run build + + - name: Run trusted real-Docker lifecycle + run: bun run docker:lifecycle + publish: runs-on: ubuntu-latest timeout-minutes: 25 - needs: [test, migration, e2e, image] + needs: [test, migration, e2e, image, trusted-lifecycle] if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/tags/v')) permissions: contents: read diff --git a/README.md b/README.md index 90a4749..0b7a2bf 100644 --- a/README.md +++ b/README.md @@ -41,8 +41,8 @@ docker compose pull && docker compose up -d │ │ │ │ /var/run/docker.sock (root, local) │ │ ▼ │ -│ mc-{id} (itzg/minecraft-server) │ -│ on dedicated managed-MC bridge │ + mc-{id} (itzg/minecraft-server pinned image) │ + on managed bridge (CPU/PIDs capped) │ └──────────────────────────────────────────────┘ ``` @@ -153,6 +153,7 @@ See [`.env.example`](./.env.example) for the full list. Key variables: | `DOMAIN` | Public domain - used by Caddy for HTTPS | required in prod | | `CORS_ORIGIN` | Allowed frontend origin - never derived from `DOMAIN` | required in prod | | `MINEPANEL_IMAGE` | Backend image used by Compose | `ghcr.io/minepanelproject/minepanel-backend:latest` | +| `MINECRAFT_IMAGE` | Required image identity shared by prefetch and managed containers | pinned multi-arch digest in `.env.example` | | `DATABASE_URL` | PostgreSQL connection string | required | | `JWT_SECRET` | Secret for JWT signing | required | | `ENCRYPTION_KEY` | 32 random bytes encoded as 64 hexadecimal characters; generate with `openssl rand -hex 32` | required | @@ -163,6 +164,8 @@ See [`.env.example`](./.env.example) for the full list. Key variables: | `MC_DATA_PATH` | Base path inside the backend; Compose fixes it to `/mc-data` (direct backend execution only) | `/mc-data` | | `MIN_FREE_DISK_MB` | Minimum free disk to allow server creation | `2048` | | `MAX_MEMORY_RATIO` | Max fraction of host RAM allocatable to MC servers | `0.90` | +| `MC_CPU_NANO_CPUS` | Per-container CPU quota in NanoCPU units | `2000000000` | +| `MC_PIDS_LIMIT` | Per-container process limit | `512` | --- diff --git a/SPEC.md b/SPEC.md index 80cd1fd..0ccbf3e 100644 --- a/SPEC.md +++ b/SPEC.md @@ -20,8 +20,8 @@ Normative language, defined once and used consistently: - **SHOULD** — a strong recommendation; a valid exception must be justified in the code or docs. - **MAY** — optional behavior; no default obligation either way. -- **Status: release candidate, not "production-ready".** Phase 1, the per-server authorization spine, requestable-server discovery, transactional setup bootstrap, protocol-1 capability discovery, and challenge-bound Google OAuth are implemented. Hosted-auth compatibility still requires the reserved PKCE fallback, and the remaining release gates in §16 must be resolved before production-ready status. -- **Last verified implementation state:** commit `f638031e737117264455132de0668c0cbb9528c4`, audited 2026-08-27. The audit baseline was clean before this documentation reconciliation. +- **Status: stable-v1 hardening implemented.** Foundation / Next items B-NEXT-1 through B-NEXT-7 are implemented and tested; the conditional PKCE fallback remains future work and does not block this milestone. +- **Last verified implementation state:** this branch's final commit and CI run are the release evidence for the hardening milestone. - **Version truth:** `package.json` says `1.0.0`; `PANEL_VERSION` defaults to `"1.0"`; the Swagger fallback is `"N/A"`; CI sets `PANEL_VERSION` to `1.0.0` in e2e. This inconsistency is tracked as backlog item B-P2-6. - **License:** the repository is MIT-licensed. `package.json` declares `"license": "MIT"` and `LICENSE` is present. `private: true` controls package publication and does not change the license. @@ -52,7 +52,7 @@ Direct browser access from `https://app.minepanel.xyz` to LAN/private-network in **Development phases (canonical numbering — used consistently everywhere):** -- **Foundation / Next:** stabilize the API error envelope and request IDs; resolve password semantics; remove dead environment configuration; add Minecraft CPU/PID isolation; make the Minecraft image reproducible; add trusted real-Docker lifecycle CI. `[ACCEPTED]`. +- **Foundation / Next:** stable-v1 hardening is `[IMPLEMENTED]`: stable API errors/request IDs; explicit password byte semantics; dead login configuration removal; bounded progressive abuse protection; Minecraft CPU/PID isolation; reproducible image identity; and trusted real-Docker lifecycle coverage. - **Phase 1.5 — Identity / Onboarding:** Google OAuth, server visibility/access requests, requestable discovery, and MOD PBAC are `[IMPLEMENTED]`. Remaining GitHub OAuth, Minecraft/Microsoft linking, offline UUID linking, invitation/registration modes, and magic links are explicitly classified in §17.1; none is assumed mandatory for backend feature completion. - **Phase 2A — Platform foundations:** audit log, framework-neutral system-event model, and a scheduler only when first required by a real feature. `[PROPOSED]`. - **Phase 3 — Core operations:** RCON/console broker, real-time server events, backup/restore, scheduled tasks, controlled filesystem writes, file manager, player management, plugins/mods, and notifications. `[PROPOSED]`. @@ -108,7 +108,7 @@ Compose services (`docker-compose.yml`): | `nestjs` | `$MINEPANEL_IMAGE` (default `ghcr.io/minepanelproject/minepanel-backend:latest`) | `pull_policy: missing`; `expose: 3000` only; `user: root`; `security_opt: no-new-privileges`; healthcheck `curl /health`; depends on healthy postgres and completed `minecraft-image` prefetch | | `postgres` | `postgres:16-alpine` | volume `postgres-data`; healthcheck `pg_isready`; no published ports | | `caddy` | `caddy:2-alpine` | publishes 80/443 (+443/udp); auto-HTTPS from `$DOMAIN`; proxies to `nestjs:3000`; serves `./Caddyfile` | -| `minecraft-image` | `itzg/minecraft-server:latest` | one-shot prefetch (`entrypoint: ["/bin/true"]`) so the first server create does not stall on a pull | +| `minecraft-image` | `${MINECRAFT_IMAGE}` (required; `.env.example` pins a verified multi-arch digest) | one-shot prefetch (`entrypoint: ["/bin/true"]`) using the exact identity passed to managed containers | The backend image release contract is explicit: pushes to `master` publish `edge` and an immutable `sha-` tag, never @@ -127,10 +127,9 @@ current revision, so `edge` is the supported pre-stable channel. 5. **MC containers.** Untrusted, modded game code. They run unprivileged, memory-capped, with no added Linux capabilities, on a bridge network. Known gap (backlog B-P2-4): the `mc` bridge allows unrestricted container-to-container traffic; per-server networks are `[PROPOSED]`. 6. **Data volume.** Host directory owned via daemon binds; itzg entrypoint chowns to its runtime user at container start; the backend reads it `:ro`. -### 4.3 Hardening backlog `[ACCEPTED]` +### 4.3 Hardening backlog `[IMPLEMENTED]` -- **B-NEXT-5:** add `NanoCpus` CPU quota and `PidsLimit` to MC container `HostConfig` (one MC server can currently starve backend/postgres; fork-bomb surface). -- **B-NEXT-6:** pin the itzg image with an explicit tag or digest strategy and record resolved identity where needed. +- **B-NEXT-1 through B-NEXT-7:** stable-v1 API errors/request IDs, password semantics, throttle configuration, progressive login abuse protection, CPU/PID isolation, reproducible Minecraft image identity, and trusted lifecycle coverage are implemented below and gated in CI. - **B-P2-4:** document/restrict inter-container traffic on the `mc` network; per-server networks remain `[PROPOSED]`. - **B-P2-5:** run the backend as a non-root user with `group_add` for the Docker group instead of `user: root`. - **B-P2-6:** consider `cap_drop: [ALL]` + `read_only: true` + `tmpfs: /tmp` for the backend container. @@ -527,21 +526,29 @@ No runtime deletion behavior or HTTP status changes are implied by this manual p ## 12. Error contract -### 12.1 Reality today `[IMPLEMENTED]` — not uniform +### 12.1 Stable-v1 API errors `[IMPLEMENTED]` -Four shapes coexist: NestJS default for `HttpException`; `{ message }` for PostgreSQL errors through `DbExceptionFilter` (23505 → 409, 23503 → 400, 42P01/42703 → 500, other → 500); structured `{ error: '…' }` payloads on selected 401/403 paths; and `{ statusCode, error, message, details }` for 422 resource errors. Refresh token failures are normalized to 401 machine codes; other unhandled exceptions still use Nest's generic 500 response. No request ID is generated today. +Every HTTP request receives a server-trusted correlation ID. A bounded safe +`X-Request-Id` value is accepted; malformed or oversized values are replaced +with a UUID. The response always echoes the chosen ID in `X-Request-Id`. -### 12.2 Stable-v1 envelope and request IDs `[ACCEPTED]` — Foundation / Next +Application errors use one envelope: ```json -{ "statusCode": 403, "error": "ACCOUNT_PENDING", "message": "human text", "details": {}, "requestId": "uuid" } +{ "statusCode": 403, "error": "AccountPending", "message": "human text", "details": {}, "requestId": "uuid" } ``` -- `error` is a stable machine code; clients switch on `error`, never `message`. -- `details` is optional structured context; `requestId` is echoed as `X-Request-Id` and included in structured logs. -- Normalize validation, authentication/authorization, not-found, conflict, domain/resource, dependency, and generic errors without leaking internals. -- Implement this as an API/protocol quality task in the current Nest backend. It is not an Elysia migration task. - +`error` is a stable machine code and clients MUST NOT switch on `message`. +Validation errors use `ValidationError` with bounded message details. +Known domain codes (including auth/session and `InsufficientResources`) are +preserved; generic Nest, dependency, database, rate-limit, not-found, +conflict, and unexpected errors map to stable status-based codes. Internal +exception details, SQL errors, Docker errors, stack traces, tokens, secrets, +and filesystem paths are never returned. + +The request middleware logs a structured `http.request` event and the global +filter logs a structured `http.error` event; both include the request ID. +Successful response bodies remain unchanged. --- ## 13. Configuration contract @@ -552,9 +559,15 @@ Four shapes coexist: NestJS default for `HttpException`; `{ message }` for Postg ### 13.2 Environment variables — consumed vs declared -Consumed `[IMPLEMENTED]`: `DATABASE_URL`, `JWT_SECRET`, `JWT_EXPIRES_IN`, `JWT_REFRESH_EXPIRES_IN` (single TTL source for refresh JWT exp, DB `expiresAt`, cookie maxAge — B-P1-5), `GOOGLE_CLIENT_ID` (enables `googleOAuth` capability + issuance), `ENCRYPTION_KEY`, `DOCKER_SOCKET`, `DOCKER_NETWORK`, `MC_DATA_PATH`, `MC_DATA_BIND_SOURCE`, `MC_PORT_MIN`, `MC_PORT_MAX`, `MIN_FREE_DISK_MB`, `MAX_MEMORY_RATIO`, `STOP_WARN_SECONDS`, `REQUIRE_ADMIN_APPROVAL`, `CORS_ORIGIN`, `PORT`, `PANEL_NAME`, `PANEL_DESCRIPTION`, `PANEL_VERSION`, `NODE_ENV` (cookie/preflight behavior). +Consumed `[IMPLEMENTED]`: `DATABASE_URL`, `JWT_SECRET`, `JWT_EXPIRES_IN`, `JWT_REFRESH_EXPIRES_IN` (single TTL source for refresh JWT exp, DB `expiresAt`, cookie maxAge — B-P1-5), `GOOGLE_CLIENT_ID` (enables `googleOAuth` capability + issuance), `ENCRYPTION_KEY`, `DOCKER_SOCKET`, `DOCKER_NETWORK`, `MINECRAFT_IMAGE`, `MC_CPU_NANO_CPUS`, `MC_PIDS_LIMIT`, `MC_DATA_PATH`, `MC_DATA_BIND_SOURCE`, `MC_PORT_MIN`, `MC_PORT_MAX`, `MIN_FREE_DISK_MB`, `MAX_MEMORY_RATIO`, `STOP_WARN_SECONDS`, `REQUIRE_ADMIN_APPROVAL`, `CORS_ORIGIN`, `PORT`, `PANEL_NAME`, `PANEL_DESCRIPTION`, `PANEL_VERSION`, `NODE_ENV` (cookie/preflight behavior). -Declared but **never read** `[CONTRADICTED]`: `LOGIN_THROTTLE_LIMIT` and `LOGIN_THROTTLE_TTL_MS` are dead configuration; `SMTP_*` and `MICROSOFT_*` are reserved for future features. `DOMAIN` is consumed by Caddy only. `PANEL_ASSETS_PATH` is not mounted or consumed; panel/logo endpoints are future work. `MC_DATA_PATH_HOST` is Compose-only and required by `${MC_DATA_PATH_HOST:?}`. Foundation / Next includes deleting or wiring the two dead login-throttle variables; no current implementation should infer behavior from them. +`LOGIN_THROTTLE_LIMIT` and `LOGIN_THROTTLE_TTL_MS` were removed. Login route +throttling remains explicit in code: Nest provides coarse source throttling, +while `LoginAbuseService` adds bounded account/source progressive protection. +`SMTP_*` and `MICROSOFT_*` remain reserved for future features. `DOMAIN` is +consumed by Caddy only. `PANEL_ASSETS_PATH` is not mounted or consumed; +panel/logo endpoints are future work. `MC_DATA_PATH_HOST` is Compose-only and +required by `${MC_DATA_PATH_HOST:?}`. ### 13.3 Reverse-proxy contract `[IMPLEMENTED]` @@ -566,27 +579,37 @@ The only inbound path is Caddy on the `app` network; `trust proxy = 1` is set (` ### 14.1 Unit tests `[IMPLEMENTED]` -The audited repository contains 40 colocated Jest suites and 729 passing unit tests at the revision/date in §2. `DRIZZLE` and `DOCKERODE` tokens are mocked; unit tests do not touch live PostgreSQL or Docker and do not read live secrets. Coverage spans auth, guards, setup, servers, Docker, gateway, admin, and DTO validation. +Unit suites remain colocated and use mocked `DRIZZLE`/`DOCKERODE` providers. +They cover the stable error envelope/request IDs, UTF-8 password boundaries, +progressive login abuse, CPU/PID/image HostConfig guardrails, and existing +auth, setup, lifecycle, gateway, admin, and DTO behavior. ### 14.2 e2e `[IMPLEMENTED]` — real boundary -The repository contains 13 e2e suites with 91 test cases. They run against a **live loopback PostgreSQL** (`TEST_DATABASE_URL`; CI provisions `postgres:16`) with Docker mocked. No e2e suite creates a real Minecraft container. The CI `e2e` job applies migrations and runs `test:e2e`; the only daemon-touching check is the release-only `publish` smoke. +PostgreSQL e2e suites run against live loopback PostgreSQL with Docker mocked. +The release-gated `trusted-lifecycle` job additionally runs +`scripts/docker-lifecycle-smoke.mjs` against a real Docker daemon. | Job | Runs | Gate | |-----|------|------| -| `test` | biome lint:ci, build, jest in-band | PR + master | -| `migration` | full `db:migrate` chain on a fresh Postgres | PR + master | -| `e2e` | migrations + e2e on live PG (no daemon) | PR + master | -| `image` | build amd64, degraded-mode smoke (no socket), migration-before-listen check, image-content assertions, bcrypt load, Trivy CRITICAL + fixed-HIGH | PR + master | -| `publish` | trusted daemon smoke, multi-arch (amd64/arm64) GHCR push with SBOM/provenance; master publishes `edge` + full SHA, `vX.Y.Z` publishes exact/minor/major/`latest` + full SHA | master push / `v*` tags | +| `test` | biome lint:ci, build, anti-slop tests, TOTP smoke, Jest in-band | PR + hardening branch + master | +| `migration` | full `db:migrate` chain on a fresh Postgres | PR + hardening branch + master | +| `e2e` | migrations + e2e on live PG (no daemon) | PR + hardening branch + master | +| `image` | build amd64, degraded-mode smoke, image-content assertions, bcrypt load, Trivy CRITICAL + fixed-HIGH | PR + hardening branch + master | +| `trusted-lifecycle` | build + migration + real create/run/RCON-stop/delete/data-retention lifecycle | hardening branch + master + release tags | +| `publish` | trusted smoke, multi-arch (amd64/arm64) GHCR push with SBOM/provenance | master push / `v*` tags | + +The trusted lifecycle uses an isolated temporary data root, a unique bridge +network, strong cleanup assertions, and proves retained data before cleanup. -The audited local unit run passed (40 suites, 729 tests). The repository CI workflow defines the remaining migration, e2e, image, and trusted publish gates; no claim is made that the full CI workflow or a real Docker lifecycle run was executed during this documentation audit. +### 14.4 Coverage status `[IMPLEMENTED]` -### 14.4 Missing coverage `[ACCEPTED]` — backlog +Setup race and throttling retain live-Postgres coverage; refresh rotation +concurrency retains exactly-one-winner coverage. The trusted lifecycle now +covers real container configuration, readiness, graceful RCON stop, removal, +and retained data semantics. -- Setup race and throttling have live-Postgres coverage in `test/setup-bootstrap.e2e-spec.ts`; refresh rotation concurrency is covered by `test/refresh-rotation.e2e-spec.ts` (exactly-one-winner). Retained-data deletion semantics remain a coverage gap. -- No **real Docker lifecycle integration test** (container create → run → graceful stop → delete → data retention). Foundation / Next tracks release-only real-daemon coverage before tagging. --- @@ -597,6 +620,7 @@ Verified against commit `f638031e737117264455132de0668c0cbb9528c4` on 2026-08-27 | Domain | Feature | Status | Evidence | |--------|---------|--------|----------| | Auth | register / login (timing-equalized, dummy hash) | ✓ | `auth.service.ts`, unit+e2e | +| Auth | progressive account/source login abuse throttling | ✓ | `login-abuse.service.ts`, unit | | Auth | HttpOnly cookie sessions (access 15m / refresh 7d) | ✓ | `auth.controller.ts`, e2e | | Auth | refresh rotation (atomic, jti-keyed, exactly-one-winner) | ✓ | `auth.service.ts`, e2e concurrency | | Auth | Google OAuth (challenge + local JWKS + session reuse) | ✓ | `google-token.service.ts`, `google-oauth.service.ts`, e2e | @@ -617,15 +641,15 @@ Verified against commit `f638031e737117264455132de0668c0cbb9528c4` on 2026-08-27 | Servers | delete: container+row, data retained (202 sync) | ✓ | `servers.service.ts:465-494` | | Access | request/approve/revoke, OPEN/REQUEST/PRIVATE, non-disclosure | ✓ | `server-access.service.ts`, unit | | Docker | socket-only client, degraded mode, guardrail container spec | ✓ | unit | +| Docker | memory/CPU/PID limits, immutable image identity, retained-data lifecycle | ✓ | `docker.service.ts`, trusted lifecycle job | | Docker | host info, statfs disk, free mem | ✓ | unit | | Gateway | WS auth (cookie/event), ADMIN metrics room, `system.stats` 10s | ✓ | `events.gateway.ts`, e2e | | Common | CSRF origin guard, exact-origin CORS, helmet, ValidationPipe | ✓ | unit | -| Errors | PG-code filter (23505/23503/42P01/42703) | ✓ | unit | -| Deploy | compose (nestjs/postgres/caddy/prefetch), preflight, boot migrations | ✓ | image+smoke jobs | -| CI | lint/build/unit/migration/e2e/image/publish, Trivy, multi-arch | ✓ | configured CI gates | +| Errors | stable envelope, request IDs, validation/PG/domain normalization | ✓ | common filter and middleware unit tests | +| Deploy | compose image prefetch, resource config, preflight, boot migrations | ✓ | deployment contract and trusted lifecycle jobs | | API | protocol-1 capability discovery with no-store | ✓ | `app.controller.ts`, unit | -**Not implemented at this audit revision** (older SPECs claimed or implied them as current): `@nestjs/schedule` cron, `nestjs-pino` logging, `/users` controller, `/versions`, `PATCH /servers/:id` (config), `PATCH /servers/:id/version`, server/panel icons, `/panel/logo`, `/system/stats` REST, GitHub OAuth, magic links, Minecraft linking endpoints, invitations, API keys, webhooks, audit log, system events, backups, scheduled tasks, notifications, plugins, file manager, player management, proxies, Bedrock, `Ban` table, and pagination beyond `GET /servers`. +**Deferred or not implemented:** `@nestjs/schedule` cron, `nestjs-pino` logging, `/users` controller, `/versions`, `PATCH /servers/:id` (config), `PATCH /servers/:id/version`, server/panel icons, `/panel/logo`, `/system/stats` REST, GitHub OAuth, magic links, Minecraft linking endpoints, invitations, API keys, webhooks, audit log, system events, backups, scheduled tasks, notifications, plugins, file manager, player management, proxies, Bedrock, `Ban` table, and pagination beyond `GET /servers`. --- @@ -633,16 +657,16 @@ Verified against commit `f638031e737117264455132de0668c0cbb9528c4` on 2026-08-27 This roadmap is reconciled against the audited implementation revision in §2. Items are classified by delivery priority, not by framework preference. The current NestJS backend remains the implementation target until its intended feature set is complete. -### Foundation / Next — stable-v1 and release hardening +### Foundation / Next — stable-v1 and release hardening `[IMPLEMENTED]` -- **B-NEXT-1 Stable API error contract:** implement machine-readable error codes, one consistent response envelope, validation normalization, request IDs, `X-Request-Id`, and correlated structured logs. This is an API/protocol quality task in NestJS, not an Elysia migration task. -- **B-NEXT-2 Password semantics decision:** resolve the bcrypt 72-UTF-8-byte limit against the current 128-character DTO limit; implement the selected migration-safe policy before stable v1. -- **B-NEXT-3 Dead environment cleanup:** delete or wire `LOGIN_THROTTLE_LIMIT` and `LOGIN_THROTTLE_TTL_MS`; do not let declared-but-unused configuration imply runtime behavior. -- **B-NEXT-4 Progressive login-abuse protection:** design throttling that combines normalized account identity with source/network context and avoids hard-locking an account solely because an attacker knows its username. -- **B-NEXT-5 Minecraft resource isolation:** add CPU quota (`NanoCpus` or equivalent) and `PidsLimit` to managed containers. -- **B-NEXT-6 Image reproducibility:** replace blind `itzg/minecraft-server:latest` reliance with an explicit tag or digest strategy and record the resolved image identity where needed. -- **B-NEXT-7 Trusted Docker lifecycle coverage:** add release-gated real-daemon coverage for create → run → graceful stop → delete, including retained data assertions. -- **B-NEXT-8 Hosted-browser compatibility:** implement and verify the reserved PKCE authorization-code fallback if complete hosted cross-origin browser support remains a release requirement. Same-origin deployments are unaffected. +- **B-NEXT-1:** stable error envelope, validation normalization, request IDs, `X-Request-Id`, and structured correlation logs. +- **B-NEXT-2:** strict 72 UTF-8-byte password policy across registration, login, setup, recovery, and password changes; existing bcrypt hashes remain usable when their credential is within policy. +- **B-NEXT-3:** dead `LOGIN_THROTTLE_LIMIT` and `LOGIN_THROTTLE_TTL_MS` variables removed. +- **B-NEXT-4:** bounded progressive account/source abuse protection layered with coarse source throttling; account-wide hard lockout is avoided. +- **B-NEXT-5:** operator-configured global `NanoCpus` and `PidsLimit` guardrails on managed containers. +- **B-NEXT-6:** one required `MINECRAFT_IMAGE` identity shared by Compose prefetch and backend-created containers; the shipped default is a verified amd64/arm64 digest. +- **B-NEXT-7:** trusted CI runs create → ready → graceful RCON stop → delete and proves retained data. +- **B-NEXT-8:** conditional hosted-browser PKCE fallback remains future work and is not implemented. ### Phase 1.5 — Identity / Onboarding @@ -710,7 +734,10 @@ Later consumers of Phase 2A: API keys, outbound webhooks, external integrations, ### 17.5 Later product surfaces `[PROPOSED]` -Creation presets/wizard, mod-loader/mod selection, Velocity/networking, Geyser/Bedrock, mobile/player surfaces, and other deferred product work remain later milestones. No detailed design is normative until its product and security decisions are made. +Creation presets/wizard, mod-loader/mod selection, Velocity/networking, +Geyser/Bedrock, mobile/player surfaces, and other deferred product work +remain later milestones. No detailed design is normative until its product +and security decisions are made. ### 17.6 Backend 2.0 — Elysia 2 `[PROPOSED — FUTURE]` @@ -732,11 +759,14 @@ This is a post-feature-completion migration milestone, not current preparation w ## 18. Security requirements for future features -### 18.1 Password hashing `[DECISION REQUIRED: D-9]` — Foundation / Next +### 18.1 Password hashing `[IMPLEMENTED — D-9 ADOPTED]` — Foundation / Next -- **Reality:** passwords are hashed with bcrypt cost 10; DTO validation allows up to 128 JavaScript characters, while bcrypt accepts only the first 72 UTF-8 bytes. -- **Required decision:** choose and document a migration-safe policy that measures UTF-8 bytes and never silently truncates or treats two passwords sharing a 72-byte prefix as distinct. -- The implementation choice (pepper pre-hash, strict byte limit, or a deliberately planned Argon2id migration) remains open. This is a Foundation / Next hardening item, not a reason to change framework now. +Passwords continue to use bcrypt cost 10. Every password entry point validates +the UTF-8 byte length at most 72 before hashing or credential acceptance; input +is never truncated. This keeps existing bcrypt hashes usable for credentials +within the policy and rejects ambiguous over-limit passwords rather than +creating two interpretations of the same hash prefix. The minimum remains +eight JavaScript characters at the DTO boundary. ### 18.2 File-manager path safety — normative algorithm (accepted, applies to any data-tree file op incl. deletion) @@ -771,7 +801,7 @@ Running server: `save-off` → `save-all flush` → snapshot copy → `save-on` | D-6 | WS auth primary path (§8.6) | **OPEN** | Cookie vs ticket primary after D-1 / when cookies unavailable | Phase 3 real-time | | D-7 | Identity linking policy (§17.1) | **ADOPTED** | Silent email-match linking is forbidden; provider login returns `LinkConfirmationRequired`; linking requires an authenticated session or explicit re-authentication | Complete for implemented Google flow | | D-8 | Write architecture (§10.4) | **OPEN** | sidecar vs rw mount with path module vs per-op exec | Phase 3 write features | -| D-9 | Password hashing (§18.1) | **OPEN** | Pepper pre-hash vs byte-limit vs Argon2id; current bcrypt 72-byte behavior must be resolved | Foundation / Next | +| D-9 | Password hashing (§18.1) | **ADOPTED** | Keep bcrypt cost 10; reject passwords over 72 UTF-8 bytes at every entry point; never truncate; existing in-policy bcrypt hashes remain compatible | Stable v1 | | D-10 | Identity normalization (§8.8) | **ADOPTED** | canonical lowercase at write: trim + lowercase username at registration and provider-generated usernames; login keeps lowercasing; migration adds case-collision preflight (fail loudly on `Bob`+`bob` pairs, manual resolution, no silent data loss) | Stable v1 | | D-11 | License (B-P3-10) | **ADOPTED** | MIT — LICENSE file committed (b5df536); reconcile package.json/SPEC/README/PWA metadata; no license-type change | Any public release | @@ -807,10 +837,10 @@ Password storage: The previous SPEC.md (pre-rewrite) was an ambitious design document that conflated three things. This revision separates them (§1 legend). Notable corrections: 1. **Never existed** (claimed as current in the previous document): `nestjs-pino` logging (actual: `ConsoleLogger`), `@nestjs/schedule` cron (absent), `pendingDeleteAt` + deletion cleanup, `discordWebhook`, future-model tables, `/users` endpoints, `/versions`, version-update and icon endpoints, `/panel/logo`, `/system/stats` REST, GitHub OAuth, magic links, Minecraft-linking endpoints, strict/standard/relaxed rate-limit tiers, per-username brute-force counter, refresh session metadata, 24h sliding refresh renewal, rootless-Docker default, `${XDG_RUNTIME_DIR}` Compose default, and the SvelteKit frontend. -2. **Implemented differently than described**: refresh rotates on every use (not within 24h of expiry); production `sameSite` is `none` (docs claimed `lax`); PKs are `randomUUID` (not cuid); ban deletes sessions (spec claimed it does not); 422 error code is `InsufficientResources` (not per-resource codes); `DbExceptionFilter` emits `{message}` only (spec claimed NestJS default shape); delete returns 202 for a synchronous op; RCON is `docker exec rcon-cli`, not TCP RCON; the `mc-{id}` container name (not `minepanel-mc-{id}`). +2. **Implemented differently than described**: refresh rotates on every use (not within 24h of expiry); production `sameSite` is `none` (docs claimed `lax`); PKs are `randomUUID` (not cuid); ban deletes sessions (spec claimed it does not); 422 error code is `InsufficientResources` (not per-resource codes); the former `DbExceptionFilter` wording described a `{message}`-only response, while the stable-v1 implementation now emits `{statusCode, error, message, details, requestId}`; delete returns 202 for a synchronous op; RCON is `docker exec rcon-cli`, not TCP RCON; the `mc-{id}` container name (not `minepanel-mc-{id}`). 3. **Unsafe examples removed**: `startsWith(serverDir)` path check (§18.2), "128 chars is safe with bcrypt" (§18.1), `SameSite=None; Secure` described as sufficient (§8.5), `tokeninfo` + "optional audience check" OAuth flow (§17.1), silent email-match linking (§17.1), "production-ready"/"100% complete" claims (§2). 4. **Product truth**: frontend stack corrected to React 19 + Vite 7 + Tailwind 4; version strings reconciled as inconsistent (B-P2-6). The repository is MIT-licensed (LICENSE file; D-11 adopted), and `unpublished` context in earlier drafts is obsolete — `package.json` declares `"license": "MIT"`. ## Appendix B — Validation note -This specification was validated against commit `f638031e737117264455132de0668c0cbb9528c4` on 2026-08-27. The audit covered the canonical docs/config, schema and migrations, bootstrap and module composition, auth/identity/guards, admin and access-control paths, Docker and lifecycle services, gateway/adapters, controllers/DTOs, unit and e2e inventories, Compose, Dockerfile, and CI workflow. Local unit verification at this revision reported 40 suites and 729 tests passing; the repository contains 13 e2e suites with 91 test cases, which require the live PostgreSQL setup described in §14.2. No claim of a full CI or real-daemon lifecycle run is made here. +This specification was validated on branch `foundation/stable-v1-hardening` after the stable-v1 hardening implementation. The audit covered the canonical docs/config, schema and migrations, bootstrap and module composition, auth/identity/guards, admin and access-control paths, Docker and lifecycle services, gateway/adapters, controllers/DTOs, unit and e2e inventories, Compose, Dockerfile, and CI workflow. Local verification passed with 44 unit suites and 756 tests, 13 live-PostgreSQL e2e suites and 91 tests, a fresh migration chain, a production build, and a real-Docker create → readiness → graceful stop → delete scenario with retained data. The pushed branch CI run remains the final release gate. diff --git a/docker-compose.yml b/docker-compose.yml index 897e06f..0a85bd9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -28,9 +28,10 @@ services: MIN_FREE_DISK_MB: ${MIN_FREE_DISK_MB:-2048} MAX_MEMORY_RATIO: ${MAX_MEMORY_RATIO:-0.90} STOP_WARN_SECONDS: ${STOP_WARN_SECONDS:-30} + MINECRAFT_IMAGE: ${MINECRAFT_IMAGE:?Set MINECRAFT_IMAGE to a versioned image identity} + MC_CPU_NANO_CPUS: ${MC_CPU_NANO_CPUS:-2000000000} + MC_PIDS_LIMIT: ${MC_PIDS_LIMIT:-512} REQUIRE_ADMIN_APPROVAL: ${REQUIRE_ADMIN_APPROVAL:-true} - LOGIN_THROTTLE_LIMIT: ${LOGIN_THROTTLE_LIMIT:-5} - LOGIN_THROTTLE_TTL_MS: ${LOGIN_THROTTLE_TTL_MS:-10000} PANEL_NAME: ${PANEL_NAME:-MinePanel} PANEL_DESCRIPTION: ${PANEL_DESCRIPTION:-Minecraft server management panel API} PANEL_VERSION: ${PANEL_VERSION:-1.0} @@ -88,7 +89,7 @@ services: condition: service_started minecraft-image: - image: itzg/minecraft-server:latest + image: ${MINECRAFT_IMAGE:?Set MINECRAFT_IMAGE to a versioned image identity} entrypoint: ["/bin/true"] restart: "no" networks: diff --git a/docs/deployment.md b/docs/deployment.md index 29c690d..f088d93 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -46,8 +46,8 @@ docker compose pull && docker compose up -d ## How it works -- `docker compose pull` fetches the configured `MINEPANEL_IMAGE` before any container is recreated. The shipped Compose file uses `pull_policy: missing`: normal `up` reuses a locally cached image, while this explicit pull command refreshes it. -- The default Compose image is `ghcr.io/minepanelproject/minepanel-backend:latest`; `latest` is reserved for a tagged stable release and is not published yet. Use `edge` or a pinned release/SHA tag as described below. +- `docker compose pull` fetches the configured `MINEPANEL_IMAGE` and `MINECRAFT_IMAGE` before any container is recreated. Both services use `pull_policy: missing` semantics through Compose's explicit pull workflow. +- `MINECRAFT_IMAGE` is required and is the single image identity for the prefetch helper and every managed Minecraft container. The template pins a verified amd64/arm64 manifest digest; changing it is an explicit operator decision. - The `nestjs` container runs as `root` (documented runtime) so it can read the local Docker socket. - Database migrations run inside the container **before** the API starts listening on port 3000. - The backend only exposes port 3000 internally; Caddy is the only service reachable from the internet on 80/443. @@ -101,6 +101,9 @@ Key variables: | `DOCKER_NETWORK` | `minepanel_network` | Bridge network used for managed Minecraft containers. | | `MIN_FREE_DISK_MB` | `2048` | Minimum free disk required to create a new server. | | `MAX_MEMORY_RATIO` | `0.90` | Max fraction of host RAM allocatable to MC servers. | +| `MINECRAFT_IMAGE` | pinned in `.env.example` | Required image identity shared by prefetch and managed containers; use a multi-arch tag plus digest for stable releases. | +| `MC_CPU_NANO_CPUS` | `2000000000` | Global per-managed-container CPU quota in Docker NanoCPU units; bounded to 0.1-64 CPUs. | +| `MC_PIDS_LIMIT` | `512` | Finite per-managed-container process limit; bounded to 128-32768. | | `STOP_WARN_SECONDS` | `30` | Seconds to warn players before a graceful shutdown. Integer `0`-`300`. | | `POSTGRES_VOLUME_NAME` | `minepanel-postgres-data` | Named volume for Postgres data. | | `CADDY_DATA_VOLUME_NAME` | `minepanel-caddy-data` | Named volume for Caddy certificates. | @@ -214,10 +217,12 @@ If the `nestjs` container exits during startup, migrations failed before the API ### Socket permission errors If you see Docker daemon unreachable errors, verify the socket path and permissions. Rootless Docker sockets live under `$XDG_RUNTIME_DIR`; set `DOCKER_SOCKET` accordingly. - ### Prefetch service -The `minecraft-image` service pulls the `itzg/minecraft-server` image and creates the managed-MC bridge, then exits cleanly. It has `restart: "no"`. If it fails, the `nestjs` service will not start because of the `service_completed_successfully` dependency. +The `minecraft-image` service pulls the required `MINECRAFT_IMAGE` identity +and creates the managed-Minecraft bridge, then exits cleanly. If it fails, the +`nestjs` service will not start because of the successful-completion +dependency. ### CORS, secure cookies, and CSRF @@ -227,6 +232,16 @@ Authenticated browser requests use HttpOnly cookies. In production the backend e The API therefore rejects every HTTP `POST`, `PUT`, `PATCH`, or `DELETE` request that supplies an `Origin` header other than the canonical `CORS_ORIGIN` (or the API's own origin — same-origin callers such as the Swagger UI at `/docs`), returning `403 {"error":"CsrfOriginForbidden"}`. This protects bodyless form-targetable actions such as session invalidation and server lifecycle operations. Browser requests from the configured frontend must preserve their exact `Origin` header. Origin-less mutating requests remain supported for non-browser automation and CLI clients; do not expose a browser client through a different origin. `OPTIONS` preflights and read-only methods are not subject to this check. Socket.IO/Engine.IO enforces its own matching Origin admission. +### API errors and request IDs + +Every request receives a bounded server-trusted `X-Request-Id`, echoed in +stable error responses as `requestId`. Invalid client IDs are replaced with a +UUID. Errors use `{ statusCode, error, message, details, requestId }`; clients +must switch on `error`, not human text. Login keeps coarse source throttling +and adds bounded account/source progressive penalties in process memory. This +state is intentionally local to one backend instance and expires automatically; +use a shared external limiter only if deployment topology later requires it. + ### Retained Minecraft data cleanup Deleting a server is synchronous at the API layer but deliberately retains world data at `$MC_DATA_PATH_HOST/$SERVER_UUID`. Runtime deletion does not remove files. If cleanup is required, use an exact root and UUID; display and verify the target first. This guarded command has no wildcard and warns that deletion is irreversible: diff --git a/package.json b/package.json index e644fe4..94ee531 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,7 @@ "build": "nest build && bun scripts/postbuild.mjs", "format": "biome format --write .", "smoke:totp": "bun scripts/totp-smoke.mjs", + "docker:lifecycle": "bun scripts/docker-lifecycle-smoke.mjs", "check:scripts": "bun scripts/check-package-scripts.mjs", "check:deployment": "bun scripts/check-deployment-contract.mjs", "start": "nest start", diff --git a/roadmap.json b/roadmap.json index e8a8d27..23fbe13 100644 --- a/roadmap.json +++ b/roadmap.json @@ -1,5 +1,5 @@ { - "updatedAt": "2026-08-27", + "updatedAt": "2026-09-02", "phases": [ { "id": "1", @@ -36,18 +36,18 @@ "id": "next", "label": "Foundation / Next", "name": "Stable-v1 Hardening", - "status": "planned", - "description": "With documentation authority reconciled, stabilize the API protocol, close resource-isolation gaps, and add trusted lifecycle coverage before stable v1.", - "progress": 0, - "progressLabel": "Next milestone", + "status": "completed", + "description": "Stable-v1 hardening is implemented and gated by unit, build, migration, e2e, and trusted real-Docker lifecycle checks. The conditional hosted-browser PKCE fallback remains future work.", + "progress": 100, + "progressLabel": "7 of 8 items complete; PKCE fallback deferred", "items": [ - { "text": "Stable API error envelope, machine codes, and request IDs", "done": false }, - { "text": "Resolve password semantics and bcrypt UTF-8 byte limits", "done": false }, - { "text": "Delete or wire dead login-throttle environment variables", "done": false }, - { "text": "Progressive login-abuse throttling using account + source context", "done": false }, - { "text": "Minecraft CPU quota and PIDs limit", "done": false }, - { "text": "Reproducible Minecraft image strategy", "done": false }, - { "text": "Trusted real-Docker lifecycle CI", "done": false }, + { "text": "Stable API error envelope, machine codes, and request IDs", "done": true }, + { "text": "Resolve password semantics and bcrypt UTF-8 byte limits", "done": true }, + { "text": "Delete or wire dead login-throttle environment variables", "done": true }, + { "text": "Progressive login-abuse throttling using account + source context", "done": true }, + { "text": "Minecraft CPU quota and PIDs limit", "done": true }, + { "text": "Reproducible Minecraft image strategy", "done": true }, + { "text": "Trusted real-Docker lifecycle CI", "done": true }, { "text": "Hosted-browser PKCE fallback if required", "done": false } ] }, @@ -143,8 +143,9 @@ "items": [ { "text": "Complete and freeze the final NestJS feature baseline", "done": false }, { "text": "Document deferred functionality and synchronize all docs", "done": false }, - { "text": "Establish stable API/error contracts and black-box HTTP/WS conformance", "done": false }, - { "text": "Add trusted real-Docker lifecycle coverage", "done": false }, + { "text": "Establish stable API/error contracts", "done": true }, + { "text": "Establish black-box HTTP/WS conformance", "done": false }, + { "text": "Add trusted real-Docker lifecycle coverage", "done": true }, { "text": "Verify Elysia 2 ecosystem reliability on the selected Bun runtime", "done": false }, { "text": "Port with route, status, body, auth, cookie, DB, Docker, and WS parity", "done": false } ] diff --git a/scripts/check-deployment-contract.mjs b/scripts/check-deployment-contract.mjs index 5bf53ef..93d4022 100644 --- a/scripts/check-deployment-contract.mjs +++ b/scripts/check-deployment-contract.mjs @@ -18,15 +18,17 @@ required(workflow, 'type=semver,pattern={{major}}.{{minor}},enable=${{ startsWit required(workflow, 'type=semver,pattern={{major}},enable=${{ startsWith(github.ref, \'refs/tags/v\') }}', 'workflow'); required(workflow, 'type=raw,value=latest,enable=${{ startsWith(github.ref, \'refs/tags/v\') }}', 'workflow'); forbidden(workflow, 'type=raw,value=latest,enable=${{ github.ref == \'refs/heads/master\' }}', 'workflow'); -required(workflow, 'needs: [test, migration, e2e, image]', 'publish gate'); +required(workflow, 'needs: [test, migration, e2e, image, trusted-lifecycle]', 'publish gate'); required(workflow, 'platforms: linux/amd64,linux/arm64', 'publish platforms'); required(workflow, 'provenance: mode=max', 'publish provenance'); required(workflow, 'sbom: true', 'publish SBOM'); required(workflow, 'docker run -d --name minepanel-release-smoke', 'trusted smoke'); +required(workflow, 'Run trusted real-Docker lifecycle', 'trusted lifecycle'); const compose = read('docker-compose.yml'); required(compose, 'image: ${MINEPANEL_IMAGE:-ghcr.io/minepanelproject/minepanel-backend:latest}', 'compose image'); required(compose, 'pull_policy: missing', 'compose pull policy'); +required(compose, 'MINECRAFT_IMAGE:?Set MINECRAFT_IMAGE', 'minecraft image override'); required(compose, 'MINEPANEL_IMAGE', 'compose override'); for (const relativePath of ['README.md', 'docs/deployment.md']) { diff --git a/scripts/check-package-scripts.mjs b/scripts/check-package-scripts.mjs index 8b44979..425e6ab 100755 --- a/scripts/check-package-scripts.mjs +++ b/scripts/check-package-scripts.mjs @@ -11,6 +11,7 @@ const requiredScripts = [ 'build', 'format', 'smoke:totp', + 'docker:lifecycle', 'typecheck', 'lint', 'lint:ci', diff --git a/scripts/docker-lifecycle-smoke.mjs b/scripts/docker-lifecycle-smoke.mjs new file mode 100644 index 0000000..9b38dc5 --- /dev/null +++ b/scripts/docker-lifecycle-smoke.mjs @@ -0,0 +1,264 @@ +import { access, mkdtemp, rm } from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { spawn } from 'node:child_process'; +import Dockerode from 'dockerode'; + +const image = + process.env.MINECRAFT_IMAGE ?? + 'itzg/minecraft-server:2026.8.2@sha256:efa878ddb49cf5251b2e5f2ad71b08fd2f7236c1f7907433f6697258b31d2ce4'; +const port = Number(process.env.MINEPANEL_PORT ?? 3000); +const minecraftPort = 25575; +const networkName = `minepanel-lifecycle-${process.pid}`; +const databaseUrl = + process.env.DATABASE_URL ?? 'postgresql://minepanel:minepanel@127.0.0.1:5432/minepanel'; +const dataRoot = await mkdtemp(path.join(os.tmpdir(), 'minepanel-lifecycle-')); + +if (!path.isAbsolute(dataRoot) || !path.basename(dataRoot).startsWith('minepanel-lifecycle-')) { + throw new Error('Refusing to use an unsafe lifecycle data root'); +} +if (image.includes(':latest') || !image.includes('@sha256:')) { + throw new Error('Trusted lifecycle requires an immutable Minecraft image'); +} + +const docker = new Dockerode({ socketPath: process.env.DOCKER_SOCKET ?? '/var/run/docker.sock' }); +let child; +let network; +let containerId; +let serverId; + +const sleep = (milliseconds) => new Promise((resolve) => setTimeout(resolve, milliseconds)); + +const fetchJson = async (method, route, body, cookie, setupToken) => { + const headers = new Headers(); + if (body !== undefined) headers.set('content-type', 'application/json'); + if (cookie !== undefined) headers.set('cookie', cookie); + if (setupToken !== undefined) headers.set('x-setup-token', setupToken); + const request = { method, headers }; + if (body !== undefined) request.body = JSON.stringify(body); + const response = await fetch(`http://127.0.0.1:${port}${route}`, request); + const text = await response.text(); + const parsed = text ? JSON.parse(text) : null; + if (!response.ok) throw new Error(`${method} ${route} failed: ${response.status} ${text}`); + return { body: parsed, response }; +}; + +const waitFor = async (check, timeoutMs, description) => { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (await check()) return; + await sleep(2000); + } + throw new Error(`Timed out waiting for ${description}`); +}; + +const cookieHeader = (response) => { + const cookies = response.headers.getSetCookie?.() ?? []; + return cookies.map((cookie) => cookie.split(';', 1)[0]).join('; '); +}; + +const isMinecraftReady = async (container) => { + try { + const info = await container.inspect(); + if (info.State?.Running !== true) return false; + const logs = await container.logs({ stderr: true, stdout: true, tail: 200 }); + return /Done \(/u.test(logs.toString()); + } catch { + return false; + } +}; + +const assertDockerConfig = async (container, expectedDataDir) => { + const info = await container.inspect(); + if (info.Config?.Image !== image) throw new Error('Managed container image identity changed'); + if (info.Config?.Labels?.['minepanel.managed'] !== 'true') { + throw new Error('Managed container label is missing'); + } + if (info.Config?.Labels?.['minepanel.server-id'] !== serverId) { + throw new Error('Managed container server label is wrong'); + } + if (info.HostConfig?.Memory !== 512 * 1024 * 1024) throw new Error('Memory limit is wrong'); + if (info.HostConfig?.NanoCpus !== 1_000_000_000) throw new Error('CPU quota is wrong'); + if (info.HostConfig?.PidsLimit !== 512) throw new Error('PIDs limit is wrong'); + if (info.HostConfig?.Privileged !== false) throw new Error('Privileged mode is enabled'); + if ((info.HostConfig?.CapAdd ?? []).length !== 0) throw new Error('Added capabilities are enabled'); + if (info.HostConfig?.NetworkMode !== networkName) throw new Error('Managed network is wrong'); + if (info.HostConfig?.PortBindings?.['25565/tcp']?.[0]?.HostPort !== String(minecraftPort)) { + throw new Error('Minecraft port binding is wrong'); + } + + const mount = info.Mounts?.find((candidate) => candidate.Destination === '/data'); + if (!mount || mount.Source !== expectedDataDir || mount.RW !== true) { + throw new Error('Minecraft data bind mount is wrong'); + } + if (!info.NetworkSettings?.Networks?.[networkName]) throw new Error('Network attachment is missing'); + return info; +}; + +const pullImage = async () => { + const stream = await docker.pull(image); + await new Promise((resolve, reject) => { + docker.modem.followProgress(stream, (error) => (error ? reject(error) : resolve())); + }); +}; + +try { + await pullImage(); + + network = await docker.createNetwork({ + Name: networkName, + Driver: 'bridge', + Labels: { 'minepanel.test': 'trusted-lifecycle' }, + }); + + const environment = { + ...process.env, + NODE_ENV: 'production', + DATABASE_URL: databaseUrl, + JWT_SECRET: 'trusted-lifecycle-jwt-secret-long-enough', + JWT_EXPIRES_IN: '15m', + JWT_REFRESH_EXPIRES_IN: '7d', + ENCRYPTION_KEY: '0000000000000000000000000000000000000000000000000000000000000000', + SETUP_TOKEN: 'trusted-lifecycle-setup-token', + DOCKER_SOCKET: process.env.DOCKER_SOCKET ?? '/var/run/docker.sock', + DOCKER_NETWORK: networkName, + MC_DATA_PATH: dataRoot, + MC_DATA_BIND_SOURCE: dataRoot, + MINECRAFT_IMAGE: image, + MIN_FREE_DISK_MB: '1', + MC_CPU_NANO_CPUS: '1000000000', + MC_PIDS_LIMIT: '512', + MC_PORT_MIN: String(minecraftPort), + MC_PORT_MAX: String(minecraftPort), + MAX_MEMORY_RATIO: '0.9', + STOP_WARN_SECONDS: '0', + REQUIRE_ADMIN_APPROVAL: 'false', + PORT: String(port), + }; + + child = spawn('bun', ['dist/src/main.js'], { env: environment, stdio: ['ignore', 'pipe', 'pipe'] }); + child.stdout.on('data', (chunk) => process.stdout.write(chunk)); + child.stderr.on('data', (chunk) => process.stderr.write(chunk)); + + await waitFor( + async () => { + try { + const response = await fetch(`http://127.0.0.1:${port}/health`); + return response.status === 200 || response.status === 503; + } catch { + return false; + } + }, + 120_000, + 'MinePanel HTTP readiness', + ); + + await fetchJson('POST', '/api/setup/init', { + email: 'trusted-lifecycle@example.com', + username: 'trustedlifecycle', + password: 'TrustedPass123!', + }, undefined, 'trusted-lifecycle-setup-token').then(({ response }) => { + if (response.status !== 201) throw new Error('Bootstrap did not create the admin'); + }); + + const login = await fetchJson('POST', '/api/auth/login', { + identifier: 'trustedlifecycle', + password: 'TrustedPass123!', + }); + const cookie = cookieHeader(login.response); + if (!cookie) throw new Error('Login did not issue session cookies'); + + const created = await fetchJson('POST', '/api/servers', { + name: 'Trusted Lifecycle', + provider: 'PAPER', + version: '1.21.1', + memoryLimitMb: 512, + port: minecraftPort, + }, cookie); + serverId = created.body.id; + const managed = await docker.listContainers({ + all: true, + filters: { label: [`minepanel.server-id=${serverId}`] }, + }); + containerId = managed[0]?.Id; + if (!serverId || !containerId || created.body.status !== 'RUNNING') { + throw new Error('Server creation did not produce a running managed container'); + } + + const container = docker.getContainer(containerId); + const expectedDataDir = path.join(dataRoot, serverId); + await assertDockerConfig(container, expectedDataDir); + await waitFor( + () => isMinecraftReady(container), + 180_000, + 'Minecraft container readiness log', + ); + + await fetchJson('POST', `/api/servers/${serverId}/stop`, undefined, cookie); + await waitFor( + async () => (await container.inspect()).State?.Running === false, + 60_000, + 'graceful Minecraft stop', + ); + + await fetchJson('DELETE', `/api/servers/${serverId}`, undefined, cookie); + await waitFor( + async () => { + try { + await container.inspect(); + return false; + } catch (error) { + return error?.statusCode === 404 || error?.status === 404; + } + }, + 30_000, + 'container removal', + ); + await access(expectedDataDir); + console.log(`trusted Docker lifecycle passed for ${serverId}`); +} finally { + if (child && !child.killed) child.kill('SIGTERM'); + if (containerId) { + try { + const container = docker.getContainer(containerId); + const info = await container.inspect(); + if ( + info.Config?.Labels?.['minepanel.managed'] === 'true' && + info.Config?.Labels?.['minepanel.server-id'] === serverId + ) { + await container.remove({ force: true }); + } + } catch {} + } + if (network) { + try { + await network.remove(); + } catch {} + } + try { + await rm(dataRoot, { recursive: true, force: true }); + } catch (error) { + if (error?.code === 'EACCES') { + const cleaner = await docker.createContainer({ + Image: image, + Entrypoint: ['/bin/sh'], + Cmd: ['-c', 'rm -rf /data'], + HostConfig: { + AutoRemove: true, + Binds: [`${dataRoot}:/data`], + NetworkMode: 'none', + }, + }); + try { + await cleaner.start(); + await cleaner.wait(); + } catch {} + try { + await cleaner.remove({ force: true }); + } catch {} + try { + await rm(dataRoot, { recursive: true, force: true }); + } catch {} + } + } +} diff --git a/setup-mac.sh b/setup-mac.sh index 1445e5f..520dfb8 100755 --- a/setup-mac.sh +++ b/setup-mac.sh @@ -105,6 +105,7 @@ if [[ $SKIP_ENV -eq 0 ]]; then DOMAIN=${DOMAIN} CORS_ORIGIN=${CORS_ORIGIN} MINEPANEL_IMAGE=ghcr.io/minepanelproject/minepanel-backend:edge +MINECRAFT_IMAGE=itzg/minecraft-server:2026.8.2@sha256:efa878ddb49cf5251b2e5f2ad71b08fd2f7236c1f7907433f6697258b31d2ce4 POSTGRES_PASSWORD=${POSTGRES_PASSWORD} DATABASE_URL="postgresql://minepanel:${POSTGRES_PASSWORD}@postgres:5432/minepanel" @@ -115,8 +116,6 @@ JWT_REFRESH_EXPIRES_IN="7d" SETUP_TOKEN=${SETUP_TOKEN} REQUIRE_ADMIN_APPROVAL=true -LOGIN_THROTTLE_LIMIT=5 -LOGIN_THROTTLE_TTL_MS=10000 ENCRYPTION_KEY=${ENCRYPTION_KEY} @@ -134,6 +133,8 @@ MC_PORT_MAX=25665 MIN_FREE_DISK_MB=2048 MAX_MEMORY_RATIO=0.90 STOP_WARN_SECONDS=30 +MC_CPU_NANO_CPUS=2000000000 +MC_PIDS_LIMIT=512 EOF ok ".env generated" fi diff --git a/setup.ps1 b/setup.ps1 index 2e039c1..34e6988 100644 --- a/setup.ps1 +++ b/setup.ps1 @@ -106,6 +106,7 @@ if (-not $skipEnv) { DOMAIN=$domain CORS_ORIGIN=$corsOrigin MINEPANEL_IMAGE=ghcr.io/minepanelproject/minepanel-backend:edge +MINECRAFT_IMAGE=itzg/minecraft-server:2026.8.2@sha256:efa878ddb49cf5251b2e5f2ad71b08fd2f7236c1f7907433f6697258b31d2ce4 POSTGRES_PASSWORD=$pgPassword DATABASE_URL="postgresql://minepanel:$pgPassword@postgres:5432/minepanel" @@ -116,8 +117,6 @@ JWT_REFRESH_EXPIRES_IN="7d" SETUP_TOKEN=$setupToken REQUIRE_ADMIN_APPROVAL=true -LOGIN_THROTTLE_LIMIT=5 -LOGIN_THROTTLE_TTL_MS=10000 ENCRYPTION_KEY=$encryptionKey @@ -134,6 +133,8 @@ MC_PORT_MIN=25565 MC_PORT_MAX=25665 MIN_FREE_DISK_MB=2048 MAX_MEMORY_RATIO=0.90 +MC_CPU_NANO_CPUS=2000000000 +MC_PIDS_LIMIT=512 STOP_WARN_SECONDS=30 "@ diff --git a/setup.sh b/setup.sh index b66e01d..d63f8b9 100755 --- a/setup.sh +++ b/setup.sh @@ -106,8 +106,7 @@ JWT_REFRESH_EXPIRES_IN="7d" SETUP_TOKEN=${SETUP_TOKEN} REQUIRE_ADMIN_APPROVAL=true -LOGIN_THROTTLE_LIMIT=5 -LOGIN_THROTTLE_TTL_MS=10000 +MINECRAFT_IMAGE=itzg/minecraft-server:2026.8.2@sha256:efa878ddb49cf5251b2e5f2ad71b08fd2f7236c1f7907433f6697258b31d2ce4 ENCRYPTION_KEY=${ENCRYPTION_KEY} @@ -125,6 +124,8 @@ MC_PORT_MAX=25665 MIN_FREE_DISK_MB=2048 MAX_MEMORY_RATIO=0.90 STOP_WARN_SECONDS=30 +MC_CPU_NANO_CPUS=2000000000 +MC_PIDS_LIMIT=512 EOF ok ".env generated" fi diff --git a/src/auth/auth.controller.ts b/src/auth/auth.controller.ts index f1b6c46..f6b5a99 100644 --- a/src/auth/auth.controller.ts +++ b/src/auth/auth.controller.ts @@ -17,6 +17,7 @@ import { ApiOperation, ApiTags } from '@nestjs/swagger'; import { Throttle } from '@nestjs/throttler'; import type { Request, Response } from 'express'; import { Public } from 'src/common/decorators/public.decorator'; +import { type RequestWithId } from 'src/common/request-id.middleware'; import { type PublicUser } from 'src/users/public-user'; import { AuthService, requireRefreshToken, type TwoFactorChallenge } from './auth.service'; import { @@ -39,7 +40,7 @@ import { REFRESH_TOKEN_TTL, type RefreshTokenTtl } from './refresh-token-ttl'; type JwtPayload = { id: string; username: string; role: string; temporaryAuth?: boolean }; type AuthCookieJar = { refresh_token?: string }; -type AuthenticatedRequest = Request & { user: JwtPayload; cookies: AuthCookieJar }; +type AuthenticatedRequest = RequestWithId & { user: JwtPayload; cookies: AuthCookieJar }; type RefreshRequest = Request & { cookies: AuthCookieJar }; @ApiTags('auth') @@ -72,9 +73,12 @@ export class AuthController { async login( @Body() loginUser: LoginUserDto, @Res({ passthrough: true }) res: Pick, + @Req() request?: RequestWithId, ): Promise { - const loginResult = await this.authService.loginUser(loginUser); - + const loginResult = await this.authService.loginUser(loginUser, { + identifier: loginUser.identifier, + source: request?.ip ?? request?.socket.remoteAddress ?? 'unknown', + }); if ('requiresTwoFactor' in loginResult) { return loginResult; } diff --git a/src/auth/auth.module.ts b/src/auth/auth.module.ts index 8dd3ca5..148ff44 100644 --- a/src/auth/auth.module.ts +++ b/src/auth/auth.module.ts @@ -15,6 +15,7 @@ import { } from './google-token.service'; import { PreAuthGuard } from './guards/pre-auth.guard'; import { IdentityService } from './identity.service'; +import { LoginAbuseService } from './login-abuse.service'; import { OAuthChallengeService } from './oauth-challenge.service'; import { parseRefreshTokenTtl, REFRESH_TOKEN_TTL } from './refresh-token-ttl'; @@ -55,6 +56,7 @@ import { parseRefreshTokenTtl, REFRESH_TOKEN_TTL } from './refresh-token-ttl'; GoogleTokenService, IdentityService, OAuthChallengeService, + LoginAbuseService, ], exports: [AccessTokenService, IdentityService, OAuthChallengeService], }) diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index 584c531..b4669c2 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -1,5 +1,5 @@ import { createHash } from 'node:crypto'; -import { ForbiddenException, UnauthorizedException } from '@nestjs/common'; +import { BadRequestException, ForbiddenException, UnauthorizedException } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { JwtService } from '@nestjs/jwt'; import * as cryptoUtil from 'src/common/crypto.util'; @@ -265,6 +265,19 @@ describe('AuthService', () => { 'PENDING', ); }); + it('rejects over-limit registration passwords before hashing', async () => { + createService(makeUser()); + usersService.findByIdentifier = jest.fn().mockResolvedValue(null); + + await expect( + service.registerUser({ + email: 'new@example.com', + username: 'newuser', + password: '😀'.repeat(19), + }), + ).rejects.toBeInstanceOf(BadRequestException); + expect(hashSpy).not.toHaveBeenCalled(); + }); it('creates one session for a non-2FA login and sanitizes the returned user', async () => { createService(makeUser({ totpEnabled: false })); diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 7ba5f4a..8fa857d 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -7,6 +7,7 @@ import { HttpStatus, Inject, Injectable, + Optional, UnauthorizedException, } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; @@ -27,6 +28,7 @@ import { EditUserDto } from './dto/editUser.dto'; import { LoginUserDto } from './dto/login.dto'; import { CreateUserDto } from './dto/register.dto'; import { UpdatePasswordDTO } from './dto/updatePw.dto'; +import { LoginAbuseService, type LoginAttemptContext } from './login-abuse.service'; import * as bcrypt from './password'; import { createRefreshTokenId, hashRefreshTokenId } from './refresh-token-id'; import { REFRESH_TOKEN_TTL, type RefreshTokenTtl } from './refresh-token-ttl'; @@ -198,6 +200,7 @@ export class AuthService { @Inject(DRIZZLE) private readonly db: AuthDatabase, private readonly configService: ConfigService, @Inject(REFRESH_TOKEN_TTL) private readonly refreshTokenTtl: RefreshTokenTtl, + @Optional() private readonly loginAbuseService?: LoginAbuseService, ) { const encryptionKey = this.configService.get('ENCRYPTION_KEY'); if (!encryptionKey || !/^[\da-f]{64}$/i.test(encryptionKey)) { @@ -216,6 +219,7 @@ export class AuthService { throw new ConflictException('User already exists'); } + bcrypt.assertPasswordWithinByteLimit(createUser.password); const passwordHash = await bcrypt.hash(createUser.password, 10); const requireApproval = this.configService.get('REQUIRE_ADMIN_APPROVAL') === 'true'; @@ -229,7 +233,10 @@ export class AuthService { return true; } - async loginUser(loginUser: LoginUserDto): Promise { + async loginUser(loginUser: LoginUserDto, context?: LoginAttemptContext): Promise { + if (context && this.loginAbuseService) { + await this.loginAbuseService.assertAllowed(context); + } const user = await this.usersService.findByIdentifier(loginUser.identifier); // Always run exactly two bcrypt comparisons so timing does not reveal @@ -245,7 +252,8 @@ export class AuthService { : DUMMY_PASSWORD_HASH; const tempMatches = await bcrypt.compare(loginUser.password, tempHash); - if (!user || (!primaryMatches && !tempMatches)) { + if (!bcrypt.isPasswordWithinByteLimit(loginUser.password) || !user) { + this.recordLoginFailure(context); throw new UnauthorizedException('Wrong credentials'); } @@ -256,6 +264,7 @@ export class AuthService { // the same generic failure as any other wrong password. if (user.mustChangePassword) { if (!tempMatches) { + this.recordLoginFailure(context); throw new UnauthorizedException('Wrong credentials'); } @@ -273,11 +282,14 @@ export class AuthService { { expiresIn: '5m' }, ); + this.recordLoginSuccess(context); return { requiresTwoFactor: true, preAuthToken }; } await this.consumeTempPassword(user); - return this.issueSession(user, true); + const session = await this.issueSession(user, true); + this.recordLoginSuccess(context); + return session; } if (primaryMatches) { @@ -287,12 +299,16 @@ export class AuthService { { expiresIn: '5m' }, ); + this.recordLoginSuccess(context); return { requiresTwoFactor: true, preAuthToken }; } - return this.issueSession(user); + const session = await this.issueSession(user); + this.recordLoginSuccess(context); + return session; } + this.recordLoginFailure(context); throw new UnauthorizedException('Wrong credentials'); } @@ -651,6 +667,13 @@ export class AuthService { } } + private recordLoginFailure(context: LoginAttemptContext | undefined): void { + if (context && this.loginAbuseService) this.loginAbuseService.recordFailure(context); + } + + private recordLoginSuccess(context: LoginAttemptContext | undefined): void { + if (context && this.loginAbuseService) this.loginAbuseService.recordSuccess(context); + } private assertLoginAllowed(user: User): void { if (user.status === 'PENDING') { throw new ForbiddenException({ error: 'AccountPending' }); diff --git a/src/auth/dto/login.dto.ts b/src/auth/dto/login.dto.ts index 6d6cb52..f73d1bd 100644 --- a/src/auth/dto/login.dto.ts +++ b/src/auth/dto/login.dto.ts @@ -1,6 +1,7 @@ import { ApiProperty } from '@nestjs/swagger'; import { Transform } from 'class-transformer'; import { IsNotEmpty, MaxLength, MinLength } from 'class-validator'; +import { PasswordByteLimit } from './password-byte-limit'; export class LoginUserDto { @ApiProperty() @@ -10,9 +11,9 @@ export class LoginUserDto { @MinLength(3) identifier: string; - @ApiProperty() + @ApiProperty({ maxLength: 72, description: 'At most 72 UTF-8 bytes' }) @IsNotEmpty() @MinLength(8) - @MaxLength(128) + @PasswordByteLimit() password: string; } diff --git a/src/auth/dto/password-byte-limit.spec.ts b/src/auth/dto/password-byte-limit.spec.ts new file mode 100644 index 0000000..8e841eb --- /dev/null +++ b/src/auth/dto/password-byte-limit.spec.ts @@ -0,0 +1,46 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { LoginUserDto } from './login.dto'; +import { UpdatePasswordDTO } from './updatePw.dto'; + +const invalidPasswordErrors = async (password: string) => { + const dto = plainToInstance(LoginUserDto, { identifier: 'player', password }); + return validate(dto); +}; + +const invalidPasswordChangeErrors = async (password: string) => { + const dto = plainToInstance(UpdatePasswordDTO, { + oldPassword: password, + newPassword: password, + }); + return validate(dto); +}; + +describe('password UTF-8 byte limits', () => { + it('accepts an ASCII password at the 72-byte boundary', async () => { + await expect(invalidPasswordErrors('a'.repeat(72))).resolves.toHaveLength(0); + }); + + it('rejects an ASCII password above the 72-byte boundary', async () => { + await expect(invalidPasswordErrors('a'.repeat(73))).resolves.toEqual( + expect.arrayContaining([expect.objectContaining({ property: 'password' })]), + ); + }); + + it('measures multi-byte Unicode passwords as UTF-8 bytes', async () => { + await expect(invalidPasswordErrors('😀'.repeat(18))).resolves.toHaveLength(0); + await expect(invalidPasswordErrors('😀'.repeat(19))).resolves.toEqual( + expect.arrayContaining([expect.objectContaining({ property: 'password' })]), + ); + }); + + it('applies the same byte limit to password changes', async () => { + const errors = await invalidPasswordChangeErrors('é'.repeat(37)); + expect(errors).toEqual( + expect.arrayContaining([ + expect.objectContaining({ property: 'oldPassword' }), + expect.objectContaining({ property: 'newPassword' }), + ]), + ); + }); +}); diff --git a/src/auth/dto/password-byte-limit.ts b/src/auth/dto/password-byte-limit.ts new file mode 100644 index 0000000..fa480a2 --- /dev/null +++ b/src/auth/dto/password-byte-limit.ts @@ -0,0 +1,14 @@ +import { ValidateBy, type ValidationOptions } from 'class-validator'; +import { isPasswordWithinByteLimit } from '../password'; + +export const PasswordByteLimit = (validationOptions?: ValidationOptions): PropertyDecorator => + ValidateBy( + { + name: 'passwordUtf8ByteLimit', + validator: { + validate: (value: string): boolean => isPasswordWithinByteLimit(value), + defaultMessage: () => 'password must not exceed 72 UTF-8 bytes', + }, + }, + validationOptions, + ); diff --git a/src/auth/dto/register.dto.ts b/src/auth/dto/register.dto.ts index 6179159..e8d89d7 100644 --- a/src/auth/dto/register.dto.ts +++ b/src/auth/dto/register.dto.ts @@ -1,6 +1,7 @@ import { ApiProperty } from '@nestjs/swagger'; import { Transform } from 'class-transformer'; import { IsEmail, IsNotEmpty, Matches, MaxLength, MinLength } from 'class-validator'; +import { PasswordByteLimit } from './password-byte-limit'; export class CreateUserDto { @ApiProperty() @@ -18,9 +19,9 @@ export class CreateUserDto { @Matches(/^[a-zA-Z0-9_]+$/) username: string; - @ApiProperty() + @ApiProperty({ maxLength: 72, description: 'At most 72 UTF-8 bytes' }) @IsNotEmpty() @MinLength(8) - @MaxLength(128) + @PasswordByteLimit() password: string; } diff --git a/src/auth/dto/updatePw.dto.ts b/src/auth/dto/updatePw.dto.ts index 97b5dbb..b3a8310 100644 --- a/src/auth/dto/updatePw.dto.ts +++ b/src/auth/dto/updatePw.dto.ts @@ -1,14 +1,16 @@ import { ApiProperty } from '@nestjs/swagger'; -import { IsNotEmpty, MaxLength, MinLength } from 'class-validator'; +import { IsNotEmpty, MinLength } from 'class-validator'; +import { PasswordByteLimit } from './password-byte-limit'; export class UpdatePasswordDTO { - @ApiProperty() + @ApiProperty({ maxLength: 72, description: 'At most 72 UTF-8 bytes' }) @IsNotEmpty() + @PasswordByteLimit() oldPassword: string; - @ApiProperty() + @ApiProperty({ maxLength: 72, description: 'At most 72 UTF-8 bytes' }) @IsNotEmpty() @MinLength(8) - @MaxLength(128) + @PasswordByteLimit() newPassword: string; } diff --git a/src/auth/login-abuse.service.spec.ts b/src/auth/login-abuse.service.spec.ts new file mode 100644 index 0000000..7fa9a09 --- /dev/null +++ b/src/auth/login-abuse.service.spec.ts @@ -0,0 +1,96 @@ +import { HttpException, HttpStatus } from '@nestjs/common'; +import { LoginAbuseService, type LoginAttemptContext } from './login-abuse.service'; + +const attempt = (identifier: string, source: string): LoginAttemptContext => ({ + identifier, + source, +}); + +const expectRateLimited = async (service: LoginAbuseService, context: LoginAttemptContext) => { + await expect(service.assertAllowed(context)).rejects.toMatchObject({ + response: { error: 'LoginRateLimited' }, + status: HttpStatus.TOO_MANY_REQUESTS, + }); +}; + +describe('LoginAbuseService', () => { + it('limits repeated failures for one normalized account and source', async () => { + const service = new LoginAbuseService(); + const context = attempt(' Player ', '198.51.100.10'); + + for (let index = 0; index < 5; index += 1) { + await expect(service.assertAllowed(context)).resolves.toBeUndefined(); + service.recordFailure(context); + } + + await expectRateLimited(service, context); + }); + + it('does not hard-lock an account when failures come from many sources', async () => { + const service = new LoginAbuseService(); + const context = attempt('player', '198.51.100.1'); + + for (let index = 0; index < 20; index += 1) { + const sourceContext = attempt('PLAYER', `198.51.100.${index + 1}`); + await expect(service.assertAllowed(sourceContext)).resolves.toBeUndefined(); + service.recordFailure(sourceContext); + } + + await expect(service.assertAllowed(context)).resolves.toBeUndefined(); + }); + + it('keeps one source attack against many accounts bounded to independent keys', async () => { + const service = new LoginAbuseService(); + + for (let index = 0; index < 20; index += 1) { + const context = attempt(`unknown-${index}`, '198.51.100.10'); + for (let failure = 0; failure < 5; failure += 1) service.recordFailure(context); + await expectRateLimited(service, context); + } + }); + + it('clears account and source penalties after successful authentication', async () => { + const service = new LoginAbuseService(); + const context = attempt('player', '198.51.100.10'); + + for (let index = 0; index < 5; index += 1) service.recordFailure(context); + await expectRateLimited(service, context); + + service.recordSuccess(attempt(' PLAYER ', '198.51.100.10')); + await expect(service.assertAllowed(context)).resolves.toBeUndefined(); + }); + + it('expires failures without requiring unbounded cleanup work', async () => { + jest.useFakeTimers(); + try { + const service = new LoginAbuseService(); + const context = attempt('player', '198.51.100.10'); + for (let index = 0; index < 5; index += 1) service.recordFailure(context); + await expectRateLimited(service, context); + + jest.advanceTimersByTime(15 * 60 * 1000); + await expect(service.assertAllowed(context)).resolves.toBeUndefined(); + } finally { + jest.useRealTimers(); + } + }); + + it('uses the same safe error for rate-limit rejection regardless of account existence', async () => { + const service = new LoginAbuseService(); + const knownLike = attempt('player', '198.51.100.10'); + const unknownLike = attempt('unknown', '198.51.100.10'); + for (let index = 0; index < 5; index += 1) { + service.recordFailure(knownLike); + service.recordFailure(unknownLike); + } + + const errors = await Promise.allSettled([ + service.assertAllowed(knownLike), + service.assertAllowed(unknownLike), + ]); + expect(errors).toEqual([ + expect.objectContaining({ reason: expect.any(HttpException), status: 'rejected' }), + expect.objectContaining({ reason: expect.any(HttpException), status: 'rejected' }), + ]); + }); +}); diff --git a/src/auth/login-abuse.service.ts b/src/auth/login-abuse.service.ts new file mode 100644 index 0000000..3d4983d --- /dev/null +++ b/src/auth/login-abuse.service.ts @@ -0,0 +1,96 @@ +import { createHash } from 'node:crypto'; +import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; + +const FAILURE_WINDOW_MS = 15 * 60 * 1000; +const PAIR_FAILURE_LIMIT = 5; +const MAX_PAIR_ENTRIES = 20_000; +const MAX_ACCOUNT_ENTRIES = 5_000; +const MAX_PROGRESSIVE_DELAY_MS = 100; + +type FailureEntry = { failures: number; firstFailureAt: number; lastFailureAt: number }; + +export type LoginAttemptContext = { identifier: string; source: string }; + +const normalize = (value: string): string => value.trim().toLowerCase().slice(0, 254); + +@Injectable() +export class LoginAbuseService { + private readonly pairFailures = new Map(); + private readonly accountFailures = new Map(); + + async assertAllowed(context: LoginAttemptContext): Promise { + const now = Date.now(); + this.prune(now); + + const pairKey = this.pairKey(context); + const pair = this.pairFailures.get(pairKey); + if (pair && pair.failures >= PAIR_FAILURE_LIMIT) { + throw new HttpException({ error: 'LoginRateLimited' }, HttpStatus.TOO_MANY_REQUESTS); + } + + const account = this.accountFailures.get(normalize(context.identifier)); + if (!account) return; + + const delayMs = Math.min( + MAX_PROGRESSIVE_DELAY_MS, + Math.max(0, account.failures - PAIR_FAILURE_LIMIT) * 10, + ); + if (delayMs > 0) { + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + } + + recordFailure(context: LoginAttemptContext): void { + const now = Date.now(); + this.prune(now); + this.bump(this.pairFailures, this.pairKey(context), now, MAX_PAIR_ENTRIES); + this.bump(this.accountFailures, normalize(context.identifier), now, MAX_ACCOUNT_ENTRIES); + } + + recordSuccess(context: LoginAttemptContext): void { + const accountKey = normalize(context.identifier); + this.accountFailures.delete(accountKey); + + for (const key of this.pairFailures.keys()) { + if (key.startsWith(`${accountKey}\u0000`)) this.pairFailures.delete(key); + } + } + + private pairKey(context: LoginAttemptContext): string { + const source = createHash('sha256').update(context.source.slice(0, 256), 'utf8').digest('hex'); + return `${normalize(context.identifier)}\u0000${source}`; + } + + private bump( + entries: Map, + key: string, + now: number, + maxEntries: number, + ): void { + const previous = entries.get(key); + const entry = + previous && now - previous.firstFailureAt < FAILURE_WINDOW_MS + ? previous + : { failures: 0, firstFailureAt: now, lastFailureAt: now }; + entries.set(key, { + failures: entry.failures + 1, + firstFailureAt: entry.firstFailureAt, + lastFailureAt: now, + }); + + while (entries.size > maxEntries) { + const oldest = entries.keys().next().value; + if (oldest === undefined) break; + entries.delete(oldest); + } + } + + private prune(now: number): void { + for (const [key, entry] of this.pairFailures) { + if (now - entry.lastFailureAt >= FAILURE_WINDOW_MS) this.pairFailures.delete(key); + } + for (const [key, entry] of this.accountFailures) { + if (now - entry.lastFailureAt >= FAILURE_WINDOW_MS) this.accountFailures.delete(key); + } + } +} diff --git a/src/auth/password.ts b/src/auth/password.ts index 79b1b9b..62b0eb4 100644 --- a/src/auth/password.ts +++ b/src/auth/password.ts @@ -1,5 +1,17 @@ +import { BadRequestException } from '@nestjs/common'; import * as bcrypt from 'bcrypt'; +export const MAX_PASSWORD_BYTES = 72; + +export const isPasswordWithinByteLimit = (data: unknown): data is string => + typeof data === 'string' && Buffer.byteLength(data, 'utf8') <= MAX_PASSWORD_BYTES; + +export const assertPasswordWithinByteLimit = (data: string): void => { + if (!isPasswordWithinByteLimit(data)) { + throw new BadRequestException('Password must not exceed 72 UTF-8 bytes'); + } +}; + export const compare = (data: string, encrypted: string): Promise => bcrypt.compare(data, encrypted); diff --git a/src/common/filters/db-exception.filter.spec.ts b/src/common/filters/db-exception.filter.spec.ts new file mode 100644 index 0000000..6a26935 --- /dev/null +++ b/src/common/filters/db-exception.filter.spec.ts @@ -0,0 +1,138 @@ +import { + ArgumentsHost, + BadRequestException, + ForbiddenException, + HttpException, + HttpStatus, + Logger, +} from '@nestjs/common'; +import { DbExceptionFilter } from './db-exception.filter'; + +type HttpTestContext = { + request: { method: string; path: string; requestId?: string }; + response: { setHeader: jest.Mock; status: jest.Mock; json: jest.Mock }; + host: ArgumentsHost; +}; + +const makeContext = (requestId?: string): HttpTestContext => { + const response = { + setHeader: jest.fn(), + status: jest.fn(), + json: jest.fn(), + }; + response.status.mockReturnValue(response); + const request = { method: 'POST', path: '/api/test', requestId }; + const http = { getResponse: () => response, getRequest: () => request }; + // SAFETY: this test host implements the exact switchToHttp methods consumed by the filter. + const host = { switchToHttp: () => http } as ArgumentsHost; + return { request, response, host }; +}; + +describe('DbExceptionFilter', () => { + let filter: DbExceptionFilter; + let errorSpy: jest.SpyInstance; + + beforeEach(() => { + filter = new DbExceptionFilter(); + errorSpy = jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined); + }); + + afterEach(() => errorSpy.mockRestore()); + + it('normalizes validation errors and preserves the request ID', () => { + const context = makeContext('client-request-1'); + + filter.catch( + new BadRequestException({ + statusCode: 400, + error: 'Bad Request', + message: ['email must be an email', 'password is invalid'], + }), + context.host, + ); + + expect(context.response.setHeader).toHaveBeenCalledWith('X-Request-Id', 'client-request-1'); + expect(context.response.status).toHaveBeenCalledWith(400); + expect(context.response.json).toHaveBeenCalledWith({ + statusCode: 400, + error: 'ValidationError', + message: 'Validation failed', + details: { messages: ['email must be an email', 'password is invalid'] }, + requestId: 'client-request-1', + }); + }); + + it('preserves domain machine codes while normalizing the envelope', () => { + const context = makeContext('request-2'); + + filter.catch(new ForbiddenException({ error: 'AccountPending' }), context.host); + + expect(context.response.json).toHaveBeenCalledWith({ + statusCode: 403, + error: 'AccountPending', + message: 'Request failed', + details: {}, + requestId: 'request-2', + }); + }); + + it('maps database failures without exposing database details', () => { + const context = makeContext(); + const databaseError = new Error('password=secret host=/private/db'); + const wrapper = Object.assign(new Error('query failed'), { cause: databaseError }); + Object.assign(databaseError, { code: '23505' }); + + filter.catch(wrapper, context.host); + + const body = context.response.json.mock.calls[0][0]; + expect(body).toMatchObject({ + statusCode: 409, + error: 'CONFLICT', + message: 'Resource already exists', + details: {}, + }); + expect(body.message).not.toContain('secret'); + expect(body.requestId).toMatch(/^[0-9a-f-]{36}$/u); + }); + + it('returns a safe generic envelope for unexpected exceptions and logs correlation data', () => { + const context = makeContext('request-3'); + + filter.catch(new Error('token=secret /private/path'), context.host); + + expect(context.response.status).toHaveBeenCalledWith(HttpStatus.INTERNAL_SERVER_ERROR); + expect(context.response.json).toHaveBeenCalledWith({ + statusCode: 500, + error: 'INTERNAL_SERVER_ERROR', + message: 'Internal server error', + details: {}, + requestId: 'request-3', + }); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('"requestId":"request-3"')); + }); + + it('normalizes rate-limit and resource details without changing their status', () => { + const context = makeContext('request-4'); + + filter.catch( + new HttpException( + { + statusCode: 422, + error: 'InsufficientResources', + message: 'Insufficient memory', + details: { resource: 'memory', availableMb: 1, requiredMb: 2, totalMb: 3 }, + }, + 422, + ), + context.host, + ); + + expect(context.response.json).toHaveBeenCalledWith({ + statusCode: 422, + error: 'InsufficientResources', + message: 'Insufficient memory', + details: { resource: 'memory', availableMb: 1, requiredMb: 2, totalMb: 3 }, + requestId: 'request-4', + }); + }); +}); diff --git a/src/common/filters/db-exception.filter.ts b/src/common/filters/db-exception.filter.ts index 11a2e7d..24fe0e6 100644 --- a/src/common/filters/db-exception.filter.ts +++ b/src/common/filters/db-exception.filter.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'node:crypto'; import { ArgumentsHost, Catch, @@ -6,60 +7,195 @@ import { HttpStatus, Logger, } from '@nestjs/common'; -import { Response } from 'express'; +import type { Request, Response } from 'express'; +import { REQUEST_ID_HEADER, type RequestWithId } from 'src/common/request-id.middleware'; type ExceptionRecord = { - cause?: ExceptionRecord; - code?: string; - message?: string; + cause?: unknown; + code?: unknown; + error?: unknown; + message?: unknown; + details?: unknown; + resource?: unknown; + availableMb?: unknown; + requiredMb?: unknown; + totalMb?: unknown; }; -const isExceptionRecord = (cause: unknown): cause is ExceptionRecord => - typeof cause === 'object' && cause !== null; +type ErrorDetails = { + messages?: string[]; + resource?: string | number; + availableMb?: number; + requiredMb?: number; + totalMb?: number; +}; + +type ApiError = { + statusCode: number; + error: string; + message: string; + details: ErrorDetails; + requestId: string; +}; +const isFiniteNumber = (value: unknown): value is number => Number.isFinite(value); + +const isExceptionRecord = (value: unknown): value is ExceptionRecord => + typeof value === 'object' && value !== null; -const isStringCause = (cause: unknown): cause is string => typeof cause === 'string'; +const isString = (value: unknown): value is string => typeof value === 'string'; + +const isMachineCode = (value: unknown): value is string => + isString(value) && /^[A-Za-z][A-Za-z0-9_]{0,63}$/u.test(value); + +const statusCodeFor = (statusCode: number): string => { + switch (statusCode) { + case HttpStatus.BAD_REQUEST: + return 'BAD_REQUEST'; + case HttpStatus.UNAUTHORIZED: + return 'UNAUTHORIZED'; + case HttpStatus.FORBIDDEN: + return 'FORBIDDEN'; + case HttpStatus.NOT_FOUND: + return 'NOT_FOUND'; + case HttpStatus.CONFLICT: + return 'CONFLICT'; + case HttpStatus.UNPROCESSABLE_ENTITY: + return 'UNPROCESSABLE_ENTITY'; + case HttpStatus.TOO_MANY_REQUESTS: + return 'RATE_LIMITED'; + case HttpStatus.SERVICE_UNAVAILABLE: + return 'SERVICE_UNAVAILABLE'; + default: + return statusCode >= 500 ? 'INTERNAL_SERVER_ERROR' : 'HTTP_ERROR'; + } +}; + +const defaultMessageFor = (statusCode: number): string => + statusCode >= 500 ? 'Internal server error' : 'Request failed'; @Catch() export class DbExceptionFilter implements ExceptionFilter { - catch(cause: unknown, host: ArgumentsHost) { - const ctx = host.switchToHttp(); - const response = ctx.getResponse(); - const code = this.getPostgresCode(cause); - - if (!code) { - if (cause instanceof HttpException) { - return response.status(cause.getStatus()).json(cause.getResponse()); - } - return response.status(500).json({ message: 'Internal server error' }); + private readonly logger = new Logger(DbExceptionFilter.name); + + catch(cause: unknown, host: ArgumentsHost): void { + const context = host.switchToHttp(); + const response = context.getResponse(); + const request = context.getRequest(); + const requestId = this.requestId(request, response); + const error = this.normalize(cause, requestId); + + response.status(error.statusCode).json(error); + this.logger.error( + JSON.stringify({ + event: 'http.error', + error: error.error, + method: request.method, + path: request.path, + requestId, + statusCode: error.statusCode, + }), + ); + } + + private normalize(cause: unknown, requestId: string): ApiError { + const postgresCode = this.getPostgresCode(cause); + if (postgresCode) { + return this.normalizePostgres(postgresCode, requestId); } + if (!(cause instanceof HttpException)) { + return this.error(HttpStatus.INTERNAL_SERVER_ERROR, 'Internal server error', {}, requestId); + } + + const statusCode = cause.getStatus(); + const raw = cause.getResponse(); + const record = isExceptionRecord(raw) ? raw : undefined; + const rawMessage = record?.message ?? raw; + const messages = Array.isArray(rawMessage) + ? rawMessage.filter(isString).slice(0, 50) + : undefined; + const requestedCode = record?.error; + const error = + messages !== undefined + ? 'ValidationError' + : isMachineCode(requestedCode) && + !['Bad Request', 'Unauthorized', 'Forbidden', 'Not Found', 'Conflict'].includes( + requestedCode, + ) + ? requestedCode + : statusCodeFor(statusCode); const message = - isExceptionRecord(cause) && isStringCause(cause.message) ? cause.message : String(cause); - Logger.error(message, code, 'DbExceptionFilter'); - - switch (code) { - case '23505': // unique_violation - response.status(HttpStatus.CONFLICT).json({ message: 'Resource already exists' }); - break; - case '23503': // foreign_key_violation - response.status(HttpStatus.BAD_REQUEST).json({ message: 'Related resource not found' }); - break; - case '42P01': // undefined_table - case '42703': // undefined_column - response - .status(HttpStatus.INTERNAL_SERVER_ERROR) - .json({ message: 'Database schema error' }); - break; + messages !== undefined + ? 'Validation failed' + : isString(rawMessage) + ? rawMessage + : defaultMessageFor(statusCode); + const details = + messages !== undefined + ? { messages } + : error === 'InsufficientResources' && isExceptionRecord(record?.details) + ? this.resourceDetails(record.details) + : {}; + + return this.error(statusCode, message, details, requestId, error); + } + + private normalizePostgres(postgresCode: string, requestId: string): ApiError { + switch (postgresCode) { + case '23505': + return this.error( + HttpStatus.CONFLICT, + 'Resource already exists', + {}, + requestId, + 'CONFLICT', + ); + case '23503': + return this.error( + HttpStatus.BAD_REQUEST, + 'Related resource not found', + {}, + requestId, + 'BAD_REQUEST', + ); + case '42P01': + case '42703': + return this.error(HttpStatus.INTERNAL_SERVER_ERROR, 'Database schema error', {}, requestId); default: - response.status(HttpStatus.INTERNAL_SERVER_ERROR).json({ message: 'Database error' }); + return this.error(HttpStatus.INTERNAL_SERVER_ERROR, 'Database error', {}, requestId); } } - private getPostgresCode(cause: unknown): string | null { - if (!isExceptionRecord(cause) || !isExceptionRecord(cause.cause)) { - return null; + private resourceDetails(details: ExceptionRecord): ErrorDetails { + const result: ErrorDetails = {}; + if (isString(details.resource)) result.resource = details.resource; + for (const key of ['availableMb', 'requiredMb', 'totalMb'] as const) { + const value = details[key]; + if (isFiniteNumber(value)) result[key] = value; } + return result; + } + + private error( + statusCode: number, + message: string, + details: ErrorDetails, + requestId: string, + error = statusCodeFor(statusCode), + ): ApiError { + return { statusCode, error, message, details, requestId }; + } - return isStringCause(cause.cause.code) ? cause.cause.code : null; + private requestId(request: Request, response: Response): string { + // SAFETY: requestIdMiddleware attaches this field before the global filter handles requests. + const requestWithId = request as RequestWithId; + const requestId = requestWithId.requestId ?? randomUUID(); + response.setHeader(REQUEST_ID_HEADER, requestId); + return requestId; + } + + private getPostgresCode(cause: unknown): string | null { + if (!isExceptionRecord(cause) || !isExceptionRecord(cause.cause)) return null; + return isString(cause.cause.code) ? cause.cause.code : null; } } diff --git a/src/common/request-id.middleware.spec.ts b/src/common/request-id.middleware.spec.ts new file mode 100644 index 0000000..6164971 --- /dev/null +++ b/src/common/request-id.middleware.spec.ts @@ -0,0 +1,65 @@ +import { Logger } from '@nestjs/common'; +import type { NextFunction, Request, Response } from 'express'; +import { requestIdMiddleware } from './request-id.middleware'; + +type ResponseDouble = Response & { setHeader: jest.Mock; once: jest.Mock }; + +const makeRequest = (value?: string): Request => { + // SAFETY: this test double implements the Express Request.get boundary consumed by the middleware. + const request = Object.create(null) as Request; + request.get = jest.fn().mockReturnValue(value); + return request; +}; + +const makeResponse = (): ResponseDouble => { + // SAFETY: this test double implements the response methods consumed by the middleware. + const response = Object.create(null) as ResponseDouble; + response.setHeader = jest.fn(); + response.once = jest.fn(); + return response; +}; + +describe('requestIdMiddleware', () => { + let logSpy: jest.SpyInstance; + + beforeEach(() => { + logSpy = jest.spyOn(Logger, 'log').mockImplementation(() => undefined); + }); + + afterEach(() => logSpy.mockRestore()); + + it.each([ + undefined, + '', + 'bad id', + 'x'.repeat(129), + ])('generates a bounded server request ID for invalid input %p', (value) => { + const request = makeRequest(value); + const response = makeResponse(); + // SAFETY: this double implements the Express middleware callback boundary used by the unit test. + const next = jest.fn() as NextFunction; + + requestIdMiddleware(request, response, next); + + const requestId = response.setHeader.mock.calls[0][1]; + expect(requestId).toMatch(/^[0-9a-f-]{36}$/u); + expect(next).toHaveBeenCalledTimes(1); + expect(response.once).toHaveBeenCalledWith('finish', expect.any(Function)); + }); + + it('accepts a bounded safe client correlation ID and logs it on completion', () => { + const request = makeRequest('edge-1:abc'); + const response = makeResponse(); + + // SAFETY: this double implements the Express middleware callback boundary used by the unit test. + requestIdMiddleware(request, response, jest.fn() as NextFunction); + const onFinish = response.once.mock.calls[0][1]; + onFinish(); + + expect(response.setHeader).toHaveBeenCalledWith('X-Request-Id', 'edge-1:abc'); + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('"requestId":"edge-1:abc"'), + 'HttpRequest', + ); + }); +}); diff --git a/src/common/request-id.middleware.ts b/src/common/request-id.middleware.ts new file mode 100644 index 0000000..e47626c --- /dev/null +++ b/src/common/request-id.middleware.ts @@ -0,0 +1,42 @@ +import { randomUUID } from 'node:crypto'; +import { Logger } from '@nestjs/common'; +import type { NextFunction, Request, Response } from 'express'; + +const REQUEST_ID_HEADER = 'X-Request-Id'; +const REQUEST_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:-]{0,127}$/u; + +type RequestWithId = Request & { requestId?: string }; + +const validRequestId = (value: string | undefined): value is string => + value !== undefined && REQUEST_ID_PATTERN.test(value); + +export const requestIdMiddleware = ( + request: Request, + response: Response, + next: NextFunction, +): void => { + // Express normalizes header names and bounds the accepted client value before this check. + const supplied = request.get(REQUEST_ID_HEADER); + const requestId = validRequestId(supplied) ? supplied : randomUUID(); + // SAFETY: Express Request.get is the only request field consumed by this middleware. + const requestWithId = request as RequestWithId; + + requestWithId.requestId = requestId; + response.setHeader(REQUEST_ID_HEADER, requestId); + response.once('finish', () => { + Logger.log( + JSON.stringify({ + event: 'http.request', + method: request.method, + path: request.path, + requestId, + statusCode: response.statusCode, + }), + 'HttpRequest', + ); + }); + + next(); +}; + +export { REQUEST_ID_HEADER, type RequestWithId }; diff --git a/src/docker/docker.service.spec.ts b/src/docker/docker.service.spec.ts index 08a5c7b..5362e40 100644 --- a/src/docker/docker.service.spec.ts +++ b/src/docker/docker.service.spec.ts @@ -164,7 +164,8 @@ describe('DockerService', () => { expect(fakeDocker.createContainer).toHaveBeenCalledWith( expect.objectContaining({ name: 'mc-abc-123', - Image: 'itzg/minecraft-server', + Image: + 'itzg/minecraft-server:2026.8.2@sha256:efa878ddb49cf5251b2e5f2ad71b08fd2f7236c1f7907433f6697258b31d2ce4', ExposedPorts: { '25565/tcp': {} }, Env: expect.arrayContaining([ 'EULA=TRUE', @@ -177,6 +178,8 @@ describe('DockerService', () => { HostConfig: { Binds: ['/mc-data/abc-123:/data'], Memory: 2048 * 1024 * 1024, + NanoCpus: 2_000_000_000, + PidsLimit: 512, PortBindings: { '25565/tcp': [{ HostPort: '25570' }] }, Privileged: false, CapAdd: [], @@ -194,6 +197,35 @@ describe('DockerService', () => { expect(config.HostConfig).not.toHaveProperty('Volumes'); expect(config.HostConfig.Binds).toHaveLength(1); }); + it('uses the configured image and finite isolation limits', async () => { + fakeDocker.createContainer.mockResolvedValue({ id: 'c1' }); + configMock.map.set( + 'MINECRAFT_IMAGE', + `itzg/minecraft-server:2026.8.2@sha256:${'a'.repeat(64)}`, + ); + configMock.map.set('MC_CPU_NANO_CPUS', 1_500_000_000); + configMock.map.set('MC_PIDS_LIMIT', 1024); + + await service.createContainer(makeServer()); + + const config = fakeDocker.createContainer.mock.calls[0][0]; + expect(config.Image).toBe(`itzg/minecraft-server:2026.8.2@sha256:${'a'.repeat(64)}`); + expect(config.HostConfig.NanoCpus).toBe(1_500_000_000); + expect(config.HostConfig.PidsLimit).toBe(1024); + }); + + it.each([ + ['MINECRAFT_IMAGE', 'itzg/minecraft-server:latest'], + ['MC_CPU_NANO_CPUS', 99_999_999], + ['MC_PIDS_LIMIT', 64], + ])('rejects unsafe %s configuration', async (key, value) => { + configMock.map.set(key, value); + + await expect(service.createContainer(makeServer())).rejects.toBeInstanceOf( + BadRequestException, + ); + expect(fakeDocker.createContainer).not.toHaveBeenCalled(); + }); it('maps the env whitelist from the server', async () => { fakeDocker.createContainer.mockResolvedValue({ id: 'c1' }); @@ -452,7 +484,9 @@ describe('DockerService', () => { await service.createContainer(makeServer()); const config = fakeDocker.createContainer.mock.calls[0][0]; - expect(config.Image).toBe('itzg/minecraft-server'); + expect(config.Image).toBe( + 'itzg/minecraft-server:2026.8.2@sha256:efa878ddb49cf5251b2e5f2ad71b08fd2f7236c1f7907433f6697258b31d2ce4', + ); expect(config.HostConfig.NetworkMode).toBe('minepanel_network'); expect(config.HostConfig.NetworkMode).not.toBe('host'); expect(config.HostConfig.Binds).toHaveLength(1); diff --git a/src/docker/docker.service.ts b/src/docker/docker.service.ts index baf29fd..7c9bef9 100644 --- a/src/docker/docker.service.ts +++ b/src/docker/docker.service.ts @@ -26,10 +26,12 @@ import { STOP_TIMEOUT_SECONDS, } from './docker.constants'; -const MINECRAFT_IMAGE = 'itzg/minecraft-server'; +const DEFAULT_MINECRAFT_IMAGE = + 'itzg/minecraft-server:2026.8.2@sha256:efa878ddb49cf5251b2e5f2ad71b08fd2f7236c1f7907433f6697258b31d2ce4'; +const DEFAULT_MC_CPU_NANO_CPUS = 2_000_000_000; +const DEFAULT_MC_PIDS_LIMIT = 512; const CONTAINER_PORT = '25565/tcp'; const MIN_MEMORY_MB = 512; - export type ContainerInspectState = { id: string; name: string; @@ -51,6 +53,9 @@ export type DockerConfig = { mcDataPath: string; mcDataBindSource: string; network: string; + image: string; + cpuNanoCpus: number; + pidsLimit: number; portMin: number; portMax: number; }; @@ -126,13 +131,15 @@ export class DockerService { const config = { name: `mc-${server.id}`, - Image: MINECRAFT_IMAGE, + Image: dockerConfig.image, ExposedPorts: { [CONTAINER_PORT]: {} }, Env: env, Labels: { 'minepanel.server-id': server.id, 'minepanel.managed': 'true' }, HostConfig: { Binds: [`${dataDir}:/data`], Memory: server.memoryLimitMb * 1024 * 1024, + NanoCpus: dockerConfig.cpuNanoCpus, + PidsLimit: dockerConfig.pidsLimit, PortBindings: { [CONTAINER_PORT]: [{ HostPort: String(server.port) }] }, Privileged: false, CapAdd: [], @@ -506,6 +513,37 @@ export class DockerService { if (network === 'host' || network === 'none' || network.startsWith('container:')) { throw new BadRequestException('DOCKER_NETWORK must be a named network'); } + const rawImage = this.configService.get('MINECRAFT_IMAGE', DEFAULT_MINECRAFT_IMAGE); + const image = rawImage.trim(); + if ( + image.endsWith(':latest') || + !/^[a-z0-9]+(?:[._/-][a-z0-9]+)*:[a-z0-9][a-z0-9._-]*(?:@sha256:[a-f0-9]{64})?$/iu.test(image) + ) { + throw new BadRequestException('MINECRAFT_IMAGE must be a versioned tagged image reference'); + } + + const cpuNanoCpus = Number( + this.configService.get('MC_CPU_NANO_CPUS', DEFAULT_MC_CPU_NANO_CPUS), + ); + const pidsLimit = Number( + this.configService.get('MC_PIDS_LIMIT', DEFAULT_MC_PIDS_LIMIT), + ); + if ( + !Number.isSafeInteger(cpuNanoCpus) || + cpuNanoCpus < 100_000_000 || + cpuNanoCpus > 64_000_000_000 || + !Number.isSafeInteger(pidsLimit) || + pidsLimit < 128 || + pidsLimit > 32_768 + ) { + throw new BadRequestException( + 'MC_CPU_NANO_CPUS must be 100000000-64000000000 and MC_PIDS_LIMIT 128-32768', + ); + } + + if (rawImage !== rawImage.trim()) { + throw new BadRequestException('MINECRAFT_IMAGE must not have surrounding whitespace'); + } const portMin = Number(this.configService.get('MC_PORT_MIN', 25565)); const portMax = Number(this.configService.get('MC_PORT_MAX', 25665)); @@ -524,6 +562,9 @@ export class DockerService { mcDataPath: path.resolve(mcDataPath), mcDataBindSource, network, + image, + cpuNanoCpus, + pidsLimit, portMin, portMax, }; diff --git a/src/main.ts b/src/main.ts index f7f40aa..07c5ce4 100644 --- a/src/main.ts +++ b/src/main.ts @@ -9,6 +9,7 @@ import { AppModule } from './app.module'; import { getCanonicalCorsOrigin } from './common/cors-origin'; import { CustomLogger } from './common/custom-logger'; import { DbExceptionFilter } from './common/filters/db-exception.filter'; +import { requestIdMiddleware } from './common/request-id.middleware'; import { runProductionMigrations } from './db/migrate'; import { SocketIoAdapter } from './gateway/socket-io.adapter'; import { SocketReservationService } from './gateway/socket-reservation.service'; @@ -118,6 +119,7 @@ async function bootstrap() { app.use(helmet()); app.use(cookieParser()); + app.use(requestIdMiddleware); app.useGlobalPipes( new ValidationPipe({ whitelist: true, transform: true, forbidNonWhitelisted: true }), diff --git a/src/setup/setup.service.spec.ts b/src/setup/setup.service.spec.ts index b385672..43262e0 100644 --- a/src/setup/setup.service.spec.ts +++ b/src/setup/setup.service.spec.ts @@ -18,7 +18,12 @@ type SetupTransactionFixture = { update: jest.Mock; }; -import { ConflictException, Logger, UnauthorizedException } from '@nestjs/common'; +import { + BadRequestException, + ConflictException, + Logger, + UnauthorizedException, +} from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { Test, type TestingModule } from '@nestjs/testing'; import * as bcrypt from 'bcrypt'; @@ -168,6 +173,12 @@ describe('SetupService', () => { // the token-resolution state read expect(rootInsert).not.toHaveBeenCalled(); }); + it('rejects over-limit admin passwords before opening the transaction', async () => { + await expect( + service.initAdminRegister({ ...dto(), password: '😀'.repeat(19) }, CONFIGURED_TOKEN), + ).rejects.toBeInstanceOf(BadRequestException); + expect(transaction).not.toHaveBeenCalled(); + }); it('rejects a missing setup token before touching the database transaction', async () => { await expect(service.initAdminRegister(dto(), undefined)).rejects.toThrow( diff --git a/src/setup/setup.service.ts b/src/setup/setup.service.ts index 73cfbff..416144c 100644 --- a/src/setup/setup.service.ts +++ b/src/setup/setup.service.ts @@ -11,6 +11,7 @@ import { ConfigService } from '@nestjs/config'; import * as bcrypt from 'bcrypt'; import { eq, sql } from 'drizzle-orm'; import { CreateUserDto } from 'src/auth/dto/register.dto'; +import { assertPasswordWithinByteLimit } from 'src/auth/password'; import { DRIZZLE, type DrizzleDB } from 'src/db/db.module'; import { setupState, users } from 'src/db/schema'; @@ -91,6 +92,7 @@ export class SetupService implements OnModuleInit { // bcrypt runs before the transaction so the advisory lock is never held // during hashing + assertPasswordWithinByteLimit(createUser.password); const passwordHash = await bcrypt.hash(createUser.password, 10); await this.db.transaction(async (tx) => { diff --git a/src/users/users.service.spec.ts b/src/users/users.service.spec.ts index e102882..0a5da55 100644 --- a/src/users/users.service.spec.ts +++ b/src/users/users.service.spec.ts @@ -147,6 +147,25 @@ describe('UsersService', () => { ).rejects.toBeInstanceOf(BadRequestException); expect(update).not.toHaveBeenCalled(); }); + it('rejects over-limit old and new passwords before reading the user', async () => { + await expect( + service.updatePassword( + 'user-1', + { oldPassword: '😀'.repeat(19), newPassword: 'NewPass123!' }, + 'current-refresh', + ), + ).rejects.toBeInstanceOf(BadRequestException); + expect(select).not.toHaveBeenCalled(); + + await expect( + service.updatePassword( + 'user-1', + { oldPassword: 'Password123!', newPassword: '😀'.repeat(19) }, + 'current-refresh', + ), + ).rejects.toBeInstanceOf(BadRequestException); + expect(select).not.toHaveBeenCalled(); + }); it('rejects an OAuth-only account before updating its password', async () => { rows = [makeUser({ passwordHash: null })]; diff --git a/src/users/users.service.ts b/src/users/users.service.ts index b25fbd8..42303b7 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -3,6 +3,7 @@ import * as bcrypt from 'bcrypt'; import { and, eq, isNull, ne, or } from 'drizzle-orm'; import { EditUserDto } from 'src/auth/dto/editUser.dto'; import { UpdatePasswordDTO } from 'src/auth/dto/updatePw.dto'; +import { assertPasswordWithinByteLimit } from 'src/auth/password'; import { hashRefreshTokenId } from 'src/auth/refresh-token-id'; import { DRIZZLE, type DrizzleDB } from 'src/db/db.module'; import { type Role, refreshTokens, type User, type UserStatus, users } from 'src/db/schema'; @@ -78,8 +79,9 @@ export class UsersService { refreshTokenId: string | undefined, temporaryAuth = false, ): Promise { + assertPasswordWithinByteLimit(dto.oldPassword); + assertPasswordWithinByteLimit(dto.newPassword); const userData = await this.findById(userId); - if (!userData) { throw new Error(); }