feat: Admin Area System: Database Backup/Restore and Danger Zone 1.00

.kilo/plans/871-followup.md

@@ -1,30 +0,0 @@
-# Implementation Plan: Job 871 — follow-up tweaks
-
-## 1. Architectural Reconnaissance
-- The previous implementation introduced a `RestoreService` that takes a `ConfigService` constructor parameter but does not retain it on `this`. Internally `stageArchive()` calls `resolveSystemStagingDir(this.requireConfig())`, where `requireConfig()` returns the parameter via a cast `(this as any).configService`. This works but is a hack — the parameter should simply be retained as a class field.
-- The previous `DangerZoneService.wipeChallenges()` snapshots `<UPLOAD_DIR>/challenges` to a side directory, transactionally deletes the DB rows, and then removes the snapshot. It currently never actually deletes the live `<UPLOAD_DIR>/challenges` directory on success, so the filesystem files persist even though the DB rows are gone. The user wants the live directory physically removed after the DB commit succeeds, with the snapshot retained as the rollback source until the disk delete is complete so that a disk-delete failure can be restored from it.
-
-## 2. Impacted Files
-- **To Modify:**
-  - `backend/src/modules/admin/system/restore.service.ts` — retain `ConfigService` on the instance and stop using the `requireConfig()` shim; use it directly in `stageArchive()`.
-  - `backend/src/modules/admin/system/danger-zone.service.ts` — in `wipeChallenges()`, after the DB transaction commits, physically remove the live `<UPLOAD_DIR>/challenges` directory, and only after that succeeds remove the snapshot; on any failure after the snapshot was created, copy the snapshot back into place so a disk-delete failure can be reversed.
-
-## 3. Proposed Changes
-1. **`restore.service.ts` cleanup:**
-   - Convert the constructor's `config: ConfigService` parameter into `private readonly configService: ConfigService` so it is stored on the instance.
-   - At the top of the constructor body, capture each derived value (`uploadDir`, `databasePath`, `stageTtlMs`, `restoreUploadLimit`) from `configService` once.
-   - Remove the private `requireConfig()` shim method entirely.
-   - In `stageArchive()`, change the call to `resolveSystemStagingDir(this.configService)`.
-2. **`danger-zone.service.ts` physical delete:**
-   - In `wipeChallenges()`, after the `dataSource.transaction(...)` block returns successfully (we already have the row counts in `counts`), add: `FilesystemTransactionService.rmSafe(challengesDir)` to physically remove the live `<UPLOAD_DIR>/challenges` directory and its contents.
-   - If that rm succeeds, then `rmSafe` the `stagingBackup` snapshot.
-   - If the rm of `challengesDir` itself throws, retain the snapshot so the catch branch can restore from it (keep `staged = true`), rethrow the error to enter the catch branch, and from the existing catch branch copy `stagingBackup` back into `challengesDir` exactly as before. Update the in-method narrative so that order of operations matches: stage → db commit → live rm → snapshot rm.
-   - Do not change the DB ordering: the transaction must commit before any filesystem delete so a DB failure does not leave a partial filesystem state.
-
-## 4. Test Strategy
-- **Target Unit Test File:**
-  - Existing `tests/backend/admin-system-danger.spec.ts` already verifies that after `wipe-challenges` the `challenge` table is empty. Extend it with one assertion: after a successful wipe, the live `<UPLOAD_DIR>/challenges` directory is also physically empty/absent.
-  - Existing `tests/backend/admin-system-backup.spec.ts` and `tests/backend/admin-system-restore-validation.spec.ts` already validate `BackupService` and `RestoreService.stageArchive()` behavior; no behavioral changes are expected from the constructor cleanup since `stageArchive()` still resolves the same staging directory. Re-run those tests after the edit to confirm no regressions.
-  - No new frontend tests are required — these are strictly backend concerns.
-- **Mocking Strategy:** Reuse the existing per-suite isolated temp upload directories (`mkdtempSync(path.join(os.tmpdir(), 'hipctf-…'))`) and assert against the live path with `fs.existsSync`/`fs.readdirSync`. No new mocking infrastructure is needed; the danger-zone logic now relies on the same primitives (fs + SQLite transaction) the existing tests already exercise.
-- **Verification:** Run `npm test` to confirm both `admin-system-danger.spec.ts` and the related system suites stay green; also run `npx --workspace backend tsc -p tsconfig.build.json --noEmit` to confirm the `requireConfig()` removal does not break the build.

.kilo/plans/916.md

@@ -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.

backend/src/modules/admin/system/admin-system.controller.ts

@@ -108,13 +108,20 @@ export class AdminSystemController {
         400,
       );
     }
+    if (!body.stagingId) {
+      throw new ApiError(
+        ERROR_CODES.SYSTEM_RESTORE_STAGE_EXPIRED,
+        'Restore stage missing or expired',
+        400,
+      );
+    }
     await this.tokens.consume({
       userId,
       operation: 'restore-backup',
       token: body.token,
       stagingId: body.stagingId,
     });
-    const result = await this.restore.commitRestore(body.stagingId ?? '');
+    const result = await this.restore.commitRestore(body.stagingId);
     void result;
     await this.auth.revokeAllRefreshSessions(userId);
     return { ok: true, operation: 'restore-backup' };

backend/src/modules/admin/system/dto/admin-system.dto.ts

@@ -20,7 +20,11 @@ export const AdminDangerConfirmBodySchema = z.object({
   token: z.string().min(1).max(512),
 });
 
-export const AdminRestoreCommitBodySchema = AdminDangerConfirmBodySchema;
+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(),
+});
 
 export const AdminRestoreValidateResponseSchema = z.object({
   stagingId: z.string(),

tests/backend/admin-system-restore-commit.spec.ts

@@ -0,0 +1,122 @@
+process.env.DATABASE_PATH = ':memory:';
+process.env.THEMES_DIR = './themes';
+process.env.FRONTEND_DIST = './frontend/dist';
+process.env.UPLOAD_SIZE_LIMIT = '5mb';
+process.env.NODE_ENV = 'test';
+
+import { ApiError } from '../../backend/src/common/errors/api-error';
+import { ERROR_CODES } from '../../backend/src/common/errors/error-codes';
+import type { ArgumentMetadata } from '@nestjs/common';
+import { AdminSystemController } from '../../backend/src/modules/admin/system/admin-system.controller';
+import { ZodValidationPipe } from '../../backend/src/common/pipes/zod-validation.pipe';
+import { AdminRestoreCommitBodySchema } from '../../backend/src/modules/admin/system/dto/admin-system.dto';
+
+describe('Job 916: Admin restore commit body passes stagingId through', () => {
+  function makeController(opts: {
+    commitRestore: jest.Mock;
+    consume?: jest.Mock;
+  }) {
+    const consume = opts.consume ?? jest.fn().mockResolvedValue({ id: 'tok' });
+    const reauthenticateAdmin = jest.fn().mockResolvedValue({ id: 'admin-1' });
+    const revokeAllRefreshSessions = jest.fn().mockResolvedValue(undefined);
+    const issue = jest.fn().mockResolvedValue({ token: 't', expiresAt: 'x', id: 'i' });
+    const resetScores = jest.fn();
+    const wipeChallenges = jest.fn();
+    return {
+      controller: new AdminSystemController(
+        { build: jest.fn() } as any,
+        { stageArchive: jest.fn(), commitRestore: opts.commitRestore } as any,
+        { resetScores, wipeChallenges } as any,
+        { consume, issue } as any,
+        { reauthenticateAdmin, revokeAllRefreshSessions } as any,
+      ),
+      consume,
+      commitRestore: opts.commitRestore,
+      issue,
+      reauthenticateAdmin,
+      revokeAllRefreshSessions,
+    };
+  }
+
+  function adminReq(): any {
+    return { user: { sub: 'admin-1', role: 'admin' } };
+  }
+
+  it('forwards stagingId to RestoreService.commitRestore and returns ok', async () => {
+    const commitRestore = jest.fn().mockResolvedValue({ restoresPerformed: true, revokeUserId: 'admin-1' });
+    const { controller, consume, revokeAllRefreshSessions } = makeController({ commitRestore });
+
+    const result = await controller.commitRestore(
+      adminReq(),
+      { operation: 'restore-backup', token: 'raw-token', stagingId: 'stage-xyz' },
+    );
+
+    expect(result).toEqual({ ok: true, operation: 'restore-backup' });
+    expect(consume).toHaveBeenCalledWith({
+      userId: 'admin-1',
+      operation: 'restore-backup',
+      token: 'raw-token',
+      stagingId: 'stage-xyz',
+    });
+    expect(commitRestore).toHaveBeenCalledWith('stage-xyz');
+    expect(revokeAllRefreshSessions).toHaveBeenCalledWith('admin-1');
+  });
+
+  it('returns SYSTEM_RESTORE_STAGE_EXPIRED when stagingId is missing', async () => {
+    const commitRestore = jest.fn();
+    const consume = jest.fn();
+    const { controller } = makeController({ commitRestore, consume });
+
+    await expect(
+      controller.commitRestore(
+        adminReq(),
+        { operation: 'restore-backup', token: 'raw-token' },
+      ),
+    ).rejects.toMatchObject({
+      code: ERROR_CODES.SYSTEM_RESTORE_STAGE_EXPIRED,
+    });
+    expect(consume).not.toHaveBeenCalled();
+    expect(commitRestore).not.toHaveBeenCalled();
+  });
+
+  it('returns SYSTEM_TOKEN_MISMATCH when operation is not restore-backup', async () => {
+    const commitRestore = jest.fn();
+    const consume = jest.fn();
+    const { controller } = makeController({ commitRestore, consume });
+
+    await expect(
+      controller.commitRestore(
+        adminReq(),
+        { operation: 'reset-scores' as any, token: 'raw-token', stagingId: 'stage-xyz' },
+      ),
+    ).rejects.toMatchObject({
+      code: ERROR_CODES.SYSTEM_TOKEN_MISMATCH,
+    });
+    expect(consume).not.toHaveBeenCalled();
+    expect(commitRestore).not.toHaveBeenCalled();
+  });
+
+  it('AdminRestoreCommitBodySchema preserves the stagingId field', () => {
+    const pipe = new ZodValidationPipe(AdminRestoreCommitBodySchema);
+    const meta: ArgumentMetadata = { type: 'body' };
+    const value = pipe.transform({
+      operation: 'restore-backup',
+      token: 'raw-token',
+      stagingId: 'stage-xyz',
+    }, meta);
+    expect(value).toEqual({
+      operation: 'restore-backup',
+      token: 'raw-token',
+      stagingId: 'stage-xyz',
+    });
+  });
+
+  it('AdminRestoreCommitBodySchema keeps optional stagingId absent (not empty-stringed)', () => {
+    const pipe = new ZodValidationPipe(AdminRestoreCommitBodySchema);
+    const meta: ArgumentMetadata = { type: 'body' };
+    const value = pipe.transform({ operation: 'restore-backup', token: 'raw-token' }, meta);
+    expect(value).toEqual({ operation: 'restore-backup', token: 'raw-token' });
+    expect((value as any).stagingId).toBeUndefined();
+    void ApiError;
+  });
+});