AI Implementation feature(916): Admin Area System: Database Backup/Restore and Danger Zone 1.00 (#62)

2026-07-23 12:49:37 +00:00
parent 470ddd30c3
commit e611d201d9
5 changed files with 290 additions and 32 deletions
@@ -0,0 +1,155 @@
+# Implementation Plan: Job 916 — Admin System Restore Commit (stagingId stripped from request body)
+
+## Root cause (read-only investigation)
+
+The destructive restore never executes because the Zod schema used to validate
+`POST /api/v1/admin/system/restore/commit` **drops the `stagingId` field** from
+the request body before the controller runs.
+
+`backend/src/modules/admin/system/dto/admin-system.dto.ts:23`:
+
+```ts
+export const AdminRestoreCommitBodySchema = AdminDangerConfirmBodySchema;
+```
+
+`AdminDangerConfirmBodySchema` (line 18) only allows `{ operation, token }`.
+Zod's `safeParse` / the NestJS `ZodValidationPipe` therefore strip every
+unknown field — including `stagingId` — from the parsed body. The controller
+then receives `body.stagingId === undefined`, falls back to the empty string
+(`body.stagingId ?? ''`), and `RestoreService.commitRestore('')` finds no
+matching stage and throws `SYSTEM_RESTORE_STAGE_EXPIRED`
+(`RestoreService.commitRestore` at `restore.service.ts:223-231`).
+
+The frontend already sends the field correctly
+(`frontend/src/app/features/admin/system/system.service.ts:42`):
+
+```ts
+async commitRestore(payload: { operation: 'restore-backup'; token: string; stagingId: string })
+```
+
+so the deviation is purely server-side. The negative path (random JSON) still
+fails with `SYSTEM_RESTORE_VALIDATION_FAILED` because the malformed archive
+rejects at `validate`, never reaching `commit`.
+
+The fix is to give `AdminRestoreCommitBodySchema` its own declaration that
+**includes** `stagingId` (mirroring `AdminReauthBodySchema` at line 12-16), and
+to tighten the controller so a missing `stagingId` returns a precise
+`SYSTEM_RESTORE_STAGE_EXPIRED` instead of relying on the empty-string fallback.
+
+## 1. Architectural Reconnaissance
+
+- **Codebase style & conventions:** TypeScript monorepo (`backend` + `frontend`).
+  Backend is NestJS with `class-validator`/`zod` mixed; the admin-system module
+  uses `ZodValidationPipe` per-route. Strict async/await, error handling via
+  `ApiError` + `ERROR_CODES` (`backend/src/common/errors/`).
+- **Data Layer:** SQLite via `better-sqlite3` + TypeORM. Restore streams are
+  in-memory (`RestoreService.stages: Map<stagingId, StagedRestore>`) plus a
+  sibling directory tree under `<DATA_DIR>/.system-staging/restore-<id>/`.
+- **Test Framework & Structure:** Jest with two projects in
+  `tests/jest.config.js`. Tests live in `tests/backend/*.spec.ts` and
+  `tests/frontend/*.spec.ts`. Single command from the root:
+  `npm test` (already wired in `package.json`).
+- **Required Tools & Dependencies:** No new tools. Existing: Node 20, NestJS,
+  better-sqlite3, zod, Jest, ts-jest. `setup.sh` already builds the app and
+  rebuilds the native binding — no updates needed.
+
+## 2. Impacted Files
+
+- **To Modify:**
+  - `backend/src/modules/admin/system/dto/admin-system.dto.ts` — replace the
+    `AdminRestoreCommitBodySchema` alias with a proper schema that accepts
+    `stagingId` (required for `restore-backup`).
+  - `backend/src/modules/admin/system/admin-system.controller.ts` — validate
+    `stagingId` is present before consuming the token, and stop feeding an
+    empty string into `commitRestore`.
+- **To Create:**
+  - `tests/backend/admin-system-restore-commit.spec.ts` — focused e2e test
+    that the `restore/commit` endpoint forwards `stagingId` to
+    `RestoreService.commitRestore` and atomically swaps the live DB.
+
+## 3. Proposed Changes
+
+1. **DTO fix** — `backend/src/modules/admin/system/dto/admin-system.dto.ts`
+   - Remove the `AdminRestoreCommitBodySchema = AdminDangerConfirmBodySchema`
+     alias (line 23).
+   - Add a dedicated schema that requires `stagingId` for `restore-backup`
+     while still letting the safe schema accept the other two operations in
+     shared code paths:
+
+     ```ts
+     export const AdminRestoreCommitBodySchema = z.object({
+       operation: z.enum(ADMIN_OPERATION_KINDS),
+       token: z.string().min(1).max(512),
+       stagingId: z.string().min(1).max(128).optional(),
+     });
+     ```
+
+   - Keep the `AdminOperationKind` re-export and the
+     `AdminRestoreCommitBody` `z.infer` type aligned so the controller
+     signature remains `body: { operation; token; stagingId? }`.
+
+2. **Controller hardening** — `backend/src/modules/admin/system/admin-system.controller.ts:95-121`
+   - After the `body.operation === 'restore-backup'` check, throw
+     `SYSTEM_RESTORE_STAGE_EXPIRED` with HTTP 400 if `body.stagingId` is
+     falsy. This produces a precise error instead of the empty-string
+     fallback that currently slips through to `RestoreService`.
+   - Pass `body.stagingId` directly to `commitRestore` (no `?? ''`).
+   - Existing behavior for `reset-scores` / `wipe-challenges` (which use
+     `AdminDangerConfirmBodySchema`) is unchanged.
+
+3. **No frontend changes required.** `system.service.ts` already POSTs
+   `{ operation, token, stagingId }`; the modal already calls it after
+   `confirmations`. Once the server schema preserves `stagingId` the happy
+   path works end-to-end.
+
+4. **No DB migration needed.** `admin_operation_token` already exists;
+   `stagingId` is bound through the in-memory `RestoreService.stages` map.
+
+## 4. Test Strategy
+
+- **Target Unit Test File:** `tests/backend/admin-system-restore-commit.spec.ts` (new).
+  Companion lightweight spec additions to
+  `tests/backend/admin-system-restore-validation.spec.ts` are optional; the
+  new file is sufficient and keeps the diff small.
+- **Mocking Strategy:**
+  - Build a minimal `Test.createTestingModule` with just
+    `AdminSystemController`, `RestoreService`, `ConfirmationTokenService`,
+    `AuthService`, `BackupService`, `DangerZoneService`, the
+    `AdminOperationTokenEntity` repository, the `RestoreArchiveSchema` DTO,
+    and a `Role`-aware stub for `AdminGuard` (re-use the approach from
+    `tests/backend/admin-system-authorization.spec.ts`).
+  - Stub `ConfirmationTokenService.consume` to record the
+    `{ userId, operation, stagingId }` argument and resolve `{ id: 'tok' }`.
+  - Stub `RestoreService.commitRestore` to record its argument and resolve
+    `{ restoresPerformed: true, revokeUserId: userId }`.
+  - Stub `AuthService.reauthenticateAdmin` to resolve an admin user, and
+    `AuthService.revokeAllRefreshSessions` to a no-op.
+  - Stub `RestoreService.stageArchive` to return a fixed
+    `{ stagingId: 'stage-1', expiresAt, summary }` so the validate→confirm
+    →commit pipeline can be exercised when needed.
+
+- **Cases covered (minimal, focused):**
+  1. `commitRestore` forwards `stagingId` to `RestoreService.commitRestore`
+     and returns `{ ok: true, operation: 'restore-backup' }` when the body
+     contains `{ operation, token, stagingId }`.
+  2. `commitRestore` returns
+     `SYSTEM_RESTORE_STAGE_EXPIRED` (HTTP 400) when `stagingId` is omitted
+     **before** consuming the token (i.e. the fix returns the error early
+     rather than reaching `RestoreService`).
+  3. `commitRestore` returns `SYSTEM_TOKEN_MISMATCH` when `operation` is
+     `reset-scores` (regression guard for the existing controller branch).
+
+- **Run command:** `npm test` from the repo root (already wired in
+  `package.json:test`).
+
+## 5. Verification
+
+- Manual smoke test against the dev stack (`npm run dev`):
+  1. POST `/api/v1/admin/system/restore/validate` with a valid backup → expect
+     `stagingId`.
+  2. POST `/api/v1/admin/system/confirmations` with `{ operation: 'restore-backup', password, stagingId }` → expect `token`.
+  3. POST `/api/v1/admin/system/restore/commit` with `{ operation: 'restore-backup', token, stagingId }`
+     → expect `{ ok: true, operation: 'restore-backup' }`, the live DB swapped,
+     and the admin session revoked (redirect to `/login`).
+- The error path must still surface
+  `SYSTEM_RESTORE_VALIDATION_FAILED` for malformed JSON bodies.