diff --git a/.kilo/plans/902.md b/.kilo/plans/902.md deleted file mode 100644 index a47ec93..0000000 --- a/.kilo/plans/902.md +++ /dev/null @@ -1,93 +0,0 @@ -# Implementation Plan: Admin Add/Edit Challenge — Category Validation Bug Fix (Job 902) - -## 1. Architectural Reconnaissance - -- **Codebase style & conventions:** - - Monorepo using npm workspaces (`backend`, `frontend`). TypeScript everywhere. - - Backend: **NestJS 10** with TypeORM, SQLite (better-sqlite3), class-validator-style validation through custom `ZodValidationPipe`. - - Frontend: **Angular 17** standalone components with `OnPush` change detection, Angular Signals + `effect()`, typed reactive forms (`FormBuilder.nonNullable`), RxJS for streams. - - Stylistic choices: pure helper functions live in `*.pure.ts` next to the components; component classes only orchestrate state and templates. - - Error envelopes use stable codes (`VALIDATION_FAILED`, `CHALLENGE_NAME_TAKEN`, `CHALLENGE_INVALID_CATEGORY`) returned from `ApiError.badRequest(...)` with a `details[]` of `{ path, message }`. - -- **Data Layer:** SQLite via TypeORM (`better-sqlite3`). `challenge.category_id` is `TEXT NOT NULL` with `ON DELETE RESTRICT` FK to `category.id`. The `category.id` column is TEXT UUID v4 (see `database/challenges.md`). - -- **Test Framework & Structure:** - - **Jest 29** with `ts-jest`. Multi-project config in `tests/jest.config.js` with two projects: `backend` (node env) and `frontend` (jsdom env). - - Frontend tests live in `tests/frontend/*.spec.ts`, backend tests in `tests/backend/*.spec.ts`. Tests NEVER live next to source. - - Single command from the repo root: `npm test` (or `npm run test:backend` / `npm run test:frontend`). - - Frontend tests focus on logic / pure functions and DOM-free component behavior (no visual confirmation). `tests/frontend/admin-challenges-form.spec.ts` already exercises `validateChallengeForm`, `buildChallengeFormGroup`, `syncChallengeForm`, etc. - -- **Required Tools & Dependencies:** No new tools or dependencies are required for this fix. Zod 3 is already a backend dep; UUID validation can use the standard `z.string().uuid()`. No new npm packages needed. `setup.sh` requires no changes. - -## 2. Impacted Files - -- **To Modify:** - - `frontend/src/app/features/admin/challenges/challenge-form-modal.component.ts` — add a dedicated early-return guard for empty `categoryId` and a UUID-format validator on the control; tighten `onOk()`. - - `frontend/src/app/features/admin/challenges/challenge-form.pure.ts` — add a `categoryUuidPattern` constant and a `CATEGORY_ID_REQUIRED` helper constant; tighten `validateChallengeForm` to flag any non-UUID value as a category error (defense in depth). - - `backend/src/modules/admin/dto/challenges.dto.ts` — change `categoryId` in `baseShape` from `z.string().min(1).max(64)` to `z.string().uuid()` so an empty string, whitespace, or non-UUID payload is rejected at the validation pipe with a `400 VALIDATION_FAILED` and a `categoryId` detail. - -- **To Create:** - - `tests/frontend/admin-challenges-form-job902.spec.ts` — targeted unit tests covering the new client-side guards. - - `tests/backend/admin-challenges-create-category-job902.spec.ts` — targeted integration test covering the new server-side Zod rejection of empty/non-UUID `categoryId` on `POST /api/v1/admin/challenges`. - -## 3. Proposed Changes - -### 1. Database / Schema Migration -No schema migration. The `category_id` column already requires a valid existing row (FK + service-level `assertCategoryExists`). The fix is purely validation tightening on the wire format. The `RepairCategorySchemaAndSystemCategories1700000000400` and `UpgradeChallengeAdminSchema1700000000500` migrations remain untouched. - -### 2. Backend Logic & APIs (`backend/src/modules/admin/dto/challenges.dto.ts`) -- Update `baseShape.categoryId` from `z.string().min(1).max(64)` to `z.string().uuid({ message: 'Category is required' })`. -- This single change makes the backend reject: - - Empty string (`""`) → caught at the Zod pipe → `400 VALIDATION_FAILED` with `details: [{ path: 'categoryId', message: 'Category is required' }]`. - - Whitespace-only → still rejected by `z.string().uuid()` (UUIDs cannot contain whitespace). - - Any non-UUID string (including stray placeholders) → rejected. -- `CategoryIdParamSchema` for `GET/PUT/DELETE /:id` already uses `z.string().min(1).max(64)`; for consistency, also tighten it to `z.string().uuid()` so that a missing/invalid id route produces a uniform 400. -- The `ImportChallengeSchema` (used by import payloads) already passes a `categorySystemKey`, not `categoryId`, so it is unaffected. -- `mapServerFieldErrors` in `challenge-form.pure.ts` already maps `path === 'categoryId'` to the per-field error, so the inline `cf-error-category` will render automatically once the backend returns a `categoryId` detail. - -### 3. Frontend UI Integration (`frontend/src/app/features/admin/challenges/challenge-form-modal.component.ts`) -- In `buildChallengeFormGroup`, replace the bare `Validators.required` on `categoryId` with a stronger validator set that: - 1. Rejects empty strings (`Validators.required`). - 2. Enforces UUID v4 format using a small inline regex (e.g. `/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i`) wrapped in a named validator error key (`uuid`) so `showError('categoryId')` can render a meaningful message. -- In `onOk()`: - - Read `categoryId = this.form.controls.categoryId.value` BEFORE calling `validateChallengeForm`. - - Add an explicit defensive guard that returns immediately when the trimmed value is empty or does not match the UUID pattern, sets `clientFieldErrors = { categoryId: 'Category is required' }`, marks the control touched/dirty, switches the active tab to `'general'` via `firstErrorTab`, and does NOT call `submit.emit(...)`. This is in addition to `validateChallengeForm` (which already catches the empty case) — it exists as a belt-and-suspenders measure so the bug cannot recur even if the pure helper is bypassed. -- Keep the existing template — `` — and the existing `[data-testid="cf-category"]` selector untouched. - -### 4. Frontend Pure Helper (`frontend/src/app/features/admin/challenges/challenge-form.pure.ts`) -- Export a new constant `CATEGORY_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i`. -- In `validateChallengeForm`, expand the category branch: - ```ts - if (!values.categoryId) errs.categoryId = 'Category is required'; - else if (!CATEGORY_ID_PATTERN.test(values.categoryId)) - errs.categoryId = 'Category is required'; - ``` - (Same message on purpose: do not leak internal format expectations to the user; both states mean "you have not selected a valid category".) -- Add `firstErrorTab` already returns `'general'` for `errors.categoryId` — no change needed there. - -## 4. Test Strategy - -- **Target Unit Test File (frontend):** `tests/frontend/admin-challenges-form-job902.spec.ts` - - Reuse the existing `makeDefaults()` helper pattern from `tests/frontend/admin-challenges-form.spec.ts`. - - Test 1 — `validateChallengeForm({ categoryId: '' })` produces `errs.categoryId === 'Category is required'`. - - Test 2 — `validateChallengeForm({ categoryId: ' ' })` produces the same error (whitespace string). - - Test 3 — `validateChallengeForm({ categoryId: 'not-a-uuid' })` produces the same error (defense-in-depth against non-UUID garbage). - - Test 4 — `validateChallengeForm({ categoryId: '' })` does NOT produce a `categoryId` error. - - Test 5 — `buildChallengeFormGroup` applies a `uuid` validator that rejects `'abc'`, `'not-a-uuid'`, and `''` once the control is touched; accepts a canonical UUID v4. - - Test 6 — `firstErrorTab({ categoryId: '...' })` returns `'general'` (so the modal switches tabs to surface the error). - -- **Target Integration Test File (backend):** `tests/backend/admin-challenges-create-category-job902.spec.ts` - - Follow the boot pattern from `tests/backend/admin-challenges-api.spec.ts` (register first admin, prime CSRF, login). - - Test 1 — `POST /api/v1/admin/challenges` with `categoryId: ''` and otherwise-valid body returns `400 VALIDATION_FAILED` with `details` containing a `categoryId` entry; no row is persisted (re-list and assert empty array). - - Test 2 — `POST /api/v1/admin/challenges` with `categoryId: 'not-a-uuid'` and otherwise-valid body returns `400 VALIDATION_FAILED` with a `categoryId` detail. - - Test 3 — `POST /api/v1/admin/challenges` with a valid UUID for `categoryId` (matching a seeded category) returns `201` and the row appears in the list. - -- **Mocking Strategy:** - - **Frontend tests:** No HTTP/network mocking required — they exercise pure helpers and the in-memory `FormGroup` from `@angular/forms`. The existing `StubAdminService` pattern from `tests/frontend/admin-challenges-form.spec.ts` can be referenced if needed but is not required for the new tests above. - - **Backend tests:** Use the real `AppModule` with `process.env.DATABASE_PATH = ':memory:'`, real `GlobalExceptionFilter`, real `CsrfMiddleware`, and real admin login flow (same pattern as `tests/backend/admin-challenges-api.spec.ts`). Seed at least one category row through the existing `/api/v1/admin/categories` POST so the "valid UUID" path can succeed; rely on `category-repair-migration` to materialize the canonical system rows on fresh DBs. - -- **Run command (single, root-level):** - ```bash - npm test - ``` - This runs both projects (`backend` and `frontend`) via `tests/jest.config.js`. For narrower loops during development: `npm run test:backend` or `npm run test:frontend`. diff --git a/.kilo/plans/903.md b/.kilo/plans/903.md new file mode 100644 index 0000000..2619898 --- /dev/null +++ b/.kilo/plans/903.md @@ -0,0 +1,94 @@ +# Implementation Plan: Job 903 — Admin Challenges Import must Replace, Modal must Auto-Close + +## 1. Architectural Reconnaissance + +- **Codebase style & conventions:** + - Backend: NestJS (TypeScript, `strict`, class-based `Injectable` services, TypeORM with `better-sqlite3`, async/await, controllers delegating to services). + - Frontend: Angular 20 standalone components with `ChangeDetectionStrategy.OnPush`, signal `input()` / `output()`, `@if`/`@for` control flow, RxJS only for the search debounce, smart/dumb split. Pure helpers live in `*.pure.ts` and are tested directly without a TestBed. + - Tests: Jest + ts-jest configured in `tests/jest.config.js` as two projects (`backend`, `frontend`). Backend specs use `Test.createTestingModule` with in-memory SQLite and a unique `mkdtempSync` upload dir per file (no shared FS races). Frontend specs in `tests/frontend/` import pure helpers directly (no DOM) — the only TestBed specs are for components that must exercise `@Input` bindings. + - `npm test` at the repo root runs **all** tests for **all** components via the Jest project selector. + +- **Data Layer:** + - SQLite via TypeORM; relevant tables `challenge`, `challenge_file`, `category`, `solve`. + - File storage: `data/uploads/challenges/` for committed files, `.staging/` for in-flight uploads. + - Repository injection via `@InjectRepository(Entity)`; transactions via `this.dataSource.transaction(async (manager) => { ... })`. + +- **Test Framework & Structure:** + - Backend tests in `/repo/tests/backend/*.spec.ts` (Jest, `testEnvironment: 'node'`). + - Frontend tests in `/repo/tests/frontend/*.spec.ts` (Jest + `jsdom`); pure-helper specs do not need TestBed. + - Single command: `npm test` (already wired in root `package.json`). + - New specs to add: one backend spec for the replace-all-on-confirmed-import behavior, one frontend spec for the auto-close + list-refresh behavior of the import modal. Both must be runnable with `npm test`. + +- **Required Tools & Dependencies:** No new tools or packages. Existing `better-sqlite3`, TypeORM, NestJS, Angular, and Jest setup is sufficient. + +## 2. Impacted Files + +- **To Modify:** + - `backend/src/modules/admin/challenges.service.ts` — change `importConfirmed()` so it wipes every pre-existing `challenge` (and cascade-deletes their `challenge_file` + `solve` rows) before re-inserting from the document. Re-link the now-unreferenced disk files for unlink and unlink any orphaned file rows after the swap. + - `frontend/src/app/features/admin/challenges/challenges.component.ts` — after a successful confirmed import (`onImportConfirm`), call `closeImport()` so the dialog disappears (it currently stays open showing the result message and a manual Close button). + - `docs/guides/admin-challenges.md` — clarify that **Import with `?confirm=overwrite` replaces the entire challenge set** (not just upserts by name). Update the "Importing the same document twice is idempotent" line to state the full-replace semantics. + - `docs/architecture/key-files.md` — update the one-line responsibility for `challenges.service.ts` to mention the replace-on-confirmed-import contract. + +- **To Create:** + - `tests/backend/admin-challenges-import-replace.spec.ts` — focused contract test asserting that an overwrite-confirmed import removes pre-existing challenges (including their files and solves) and leaves only the imported set; checks both DB and the on-disk `challenges/` directory. + - `tests/frontend/admin-challenges-import-autoclose.spec.ts` — pure/smart test asserting that after a successful `onImportConfirm`, the import modal closes (i.e. `importOpen()` flips to `false`) and the result is still surfaced via the page-level status banner. + +## 3. Proposed Changes + +### 3.1 Backend — `AdminChallengesService.importConfirmed` becomes a full replace + +In `backend/src/modules/admin/challenges.service.ts`, refactor `importConfirmed` (currently lines 412–522) so the confirmed path: + +1. **Stage every file up front** (unchanged logic, lines 421–446). Keep the early `rollbackStaged` on validation failure. +2. **Inside one transaction** (unchanged transaction wrapper, line 450): + a. Load every existing challenge row with `manager.getRepository(ChallengeEntity).find()` (no filter). + b. Capture each existing challenge's `ChallengeFileEntity` rows (`find({ where: { challengeId: In(ids) } })`) so we know which `storedFilename`s are still referenced after the transaction. + c. Delete every existing challenge row with a single `manager.delete(ChallengeEntity, {})` — TypeORM cascades to `challenge_file` and `solve` rows via the existing FK cascade. This is the **replace** semantics. + d. Then iterate `doc.challenges` and (as today) skip entries whose category is unknown, otherwise create a new challenge row + new file rows pointing at the already-staged tokens. Result counts stay `{ imported, skipped, warnings }`. +3. **After commit, clean up disk** — for the captured pre-existing stored filenames, call `fileService.removeFinal(storedFilename)` best-effort (same helper used by `remove()`). Newly-inserted challenges' files are then committed via the existing `commit(item.staged.token)` loop (lines 506–517). +4. **Do not break the preview path.** `previewImport` keeps the existing conflict/unknown-category logic; the destructive replace only happens when `?confirm=overwrite` (controller path `admin-challenges.controller.ts:87-92`). +5. **No changes to DTOs or to the export shape.** `exportAll()` already produces the versioned envelope that the new behavior requires for round-tripping. + +**Edge cases to handle explicitly:** +- Empty `doc.challenges` ⇒ result `{ imported: 0, skipped: [], warnings: [] }`; existing rows are wiped, files unlinked, no new rows created. +- Unknown-category rows ⇒ counted in `skipped`; do **not** preserve them, do **not** abort the wipe. The wipe happens before the per-challenge inserts. +- Disk-unlink failure for a removed file ⇒ swallow + log via `ChallengeFilesService.removeFinal` (matches existing delete behavior, never throws). + +### 3.2 Frontend — `AdminChallengesComponent.onImportConfirm` closes the modal + +In `frontend/src/app/features/admin/challenges/challenges.component.ts`: + +- In `onImportConfirm()` (lines 455–470), after `await this.load()` succeeds and `importResult` is set, additionally call `closeImport()`. `closeImport()` already clears `importDoc`, `importPreview`, `importResult`, and `importError` (lines 447–453) and resets `importOpen` to `false`. +- The page-level status banner is already populated by `this.statusMessage.set(summarizeImportResult(result))` immediately before `closeImport()`, so the user still sees the "Imported N challenges" message after the dialog closes. +- The `` template already renders a "Close" button while `result()` is set; that branch becomes unreachable after the fix (the modal will close before the user can click it), so no template change is required. The result branch is kept as a defensive fallback for any future flow that intentionally keeps the modal open (e.g. partial-failure display). + +### 3.3 Docs — clarify the replace semantics + +- `docs/guides/admin-challenges.md` — under the "Import" section (line 103) replace the third bullet ("Click Overwrite & Import … the server upserts challenges inside one transaction by case-insensitive name") with: "Click **Overwrite & Import** → the client posts the same document with `?confirm=overwrite`. The server stages every file up front (rolling back any FS failure), then **wipes every existing challenge (cascade-deleting their `challenge_file` and `solve` rows and best-effort unlinking their disk files) and inserts the archive's challenges** in one transaction." +- Update the last "Notes" line on idempotency (line 138) to: "Importing the same document twice is idempotent: every existing challenge (matching or not) is replaced by the archive contents." +- `docs/architecture/key-files.md` — update the `challenges.service.ts` responsibility to "... CRUD, list aggregates, transactional deletion, export, full-replace import orchestration." + +## 4. Test Strategy + +### 4.1 Backend contract test — `tests/backend/admin-challenges-import-replace.spec.ts` + +- Mirror the wiring of `tests/backend/admin-challenges-import-export.spec.ts`: `:memory:` SQLite, unique `mkdtempSync(os.tmpdir() + '/hipctf-challenges-import-replace-…')` `UPLOAD_DIR`, all migrations including `UpgradeChallengeAdminSchema1700000000500`, TypeOrmModule.forFeature for `CategoryEntity`, `ChallengeEntity`, `ChallengeFileEntity`, `SolveEntity`. +- AAA structure. Mocking strategy: no external boundaries — use real in-memory SQLite + real `ChallengeFilesService` writing to the temp dir. Isolation: per-file temp dir (no parallel FS races). +- Tests (minimal, one logical assertion each): + 1. **Replace wipes unrelated challenges** — Seed `Alpha` (CRY, NC, port=1337, flag='flag{a}', with one staged file committed) and `Bravo` (CRY, WEB). Call `importConfirmed` with a single-challenge doc `{ name: 'Imported Charlie', … categorySystemKey: 'CRY', … }`. Assert `list({}).map(r => r.name)` is exactly `['Imported Charlie']` and that `chRepo.count()` equals 1. + 2. **Disk files of wiped challenges are unlinked** — Capture the stored filename of Alpha's committed file via `fileRepo.findOne({ where: { challengeId: alpha.id } })`. After the import, assert `fs.existsSync(path.join(finalDir, alphaStoredName))` is `false`, while the new challenge's stored file exists. + 3. **Empty archive wipes everything** — Re-seed Alpha, then `importConfirmed` with `{ challenges: [] }`. Assert `chRepo.count() === 0` and the previously-committed disk file is unlinked. + 4. **Unknown-category rows are skipped, not preserved** — Seed `Keep` (WEB). Import a doc containing one unknown-category challenge plus nothing else. Assert `result.skipped` has length 1 and `list({})` is empty (the wipe still happened). + +### 4.2 Frontend modal test — `tests/frontend/admin-challenges-import-autoclose.spec.ts` + +- Smart container test using `TestBed.createComponent(AdminChallengesComponent)` to exercise the live handler. +- Mock `AdminService` (`jest.fn()` for `previewChallengeImport`, `confirmChallengeImport`, `listChallenges`, `listCategories`, `exportChallenges`) and stub the document `createElement('input')` file-picker flow by setting `importDoc`/`importPreview` directly via a small helper (or by calling the public `openImport()` + injecting the file via a fake `` ref). Keep it minimal: directly drive the component signals is preferable to mocking the file picker. +- AAA structure. Mocking strategy: only `AdminService` (network boundary). No DOM snapshotting; we assert on signal values (OnPush friendly) and DOM text via `fixture.nativeElement.textContent` if needed. +- Tests: + 1. **`onImportConfirm` closes the modal after success** — arrange: `component.importOpen.set(true)`, `component.importDoc.set({ … })`, `component.importPreview.set({ total: 1, conflicts: [], unknownCategories: [], warnings: [] })`. Stub `admin.confirmChallengeImport` to resolve `{ imported: 1, skipped: [], warnings: [] }` and `admin.listChallenges` to resolve `[]`. Act: `await component.onImportConfirm()`. Assert `component.importOpen()` is `false`, `component.importResult()` is `null` (cleared by `closeImport`), and `component.statusMessage()` contains "Imported 1". + 2. **`onImportConfirm` keeps the modal open when the network call rejects** — arrange same, stub `confirmChallengeImport` to reject with a generic error. Act + assert `component.importOpen()` stays `true` and `component.importError()` is non-null. + +### 4.3 Run command + +- From the repo root: `npm test` (or `npm run test:backend` / `npm run test:frontend` for individual projects). Both new specs run inside the existing `ts-jest` configuration with no additional setup. diff --git a/backend/src/modules/admin/challenges.service.ts b/backend/src/modules/admin/challenges.service.ts index 934af6b..ad9d382 100644 --- a/backend/src/modules/admin/challenges.service.ts +++ b/backend/src/modules/admin/challenges.service.ts @@ -446,24 +446,29 @@ export class AdminChallengesService { } const result: ImportResult = { imported: 0, skipped: [], warnings: [] }; + let orphanedStoredFilenames: string[] = []; try { await this.dataSource.transaction(async (manager) => { const challengeRepo = manager.getRepository(ChallengeEntity); const fileRepo = manager.getRepository(ChallengeFileEntity); + + // Full-replace: capture every existing challenge and its disk-bound + // files so we can unlink them after the wipe, then delete every row. + // ChallengeFileEntity and SolveEntity both cascade on challenge delete. + const existingChallenges = await challengeRepo.find(); + const existingIds = existingChallenges.map((c) => c.id); + if (existingIds.length > 0) { + const existingFiles = await fileRepo.find({ where: { challengeId: In(existingIds) } }); + orphanedStoredFilenames = existingFiles.map((f) => f.storedFilename); + await manager.delete(ChallengeEntity, { id: In(existingIds) }); + } + for (const ch of doc.challenges) { const cat = catMap.get(ch.categorySystemKey.toLowerCase()); if (!cat) { result.skipped.push({ name: ch.name, reason: `Unknown category: ${ch.categorySystemKey}` }); continue; } - // Upsert by case-insensitive name. - const existing = await challengeRepo - .createQueryBuilder('c') - .where('LOWER(c.name) = :n', { n: ch.name.toLowerCase() }) - .getOne(); - if (existing) { - await manager.delete(ChallengeEntity, { id: existing.id }); - } const row = challengeRepo.create({ id: uuid(), name: ch.name, @@ -501,6 +506,11 @@ export class AdminChallengesService { throw err; } + // Best-effort unlink of files owned by wiped challenges (post-commit). + for (const storedFilename of orphanedStoredFilenames) { + this.fileService.removeFinal(storedFilename); + } + // Commit each staged file by renaming to its final filename (the // ChallengeFile rows already reference those names). for (const list of stagedByName.values()) { diff --git a/frontend/src/app/features/admin/challenges/challenges.component.ts b/frontend/src/app/features/admin/challenges/challenges.component.ts index 0734834..1e42f48 100644 --- a/frontend/src/app/features/admin/challenges/challenges.component.ts +++ b/frontend/src/app/features/admin/challenges/challenges.component.ts @@ -25,6 +25,7 @@ import { categoryIconSrc, deriveEmptyState, difficultyClass, + importErrorMessage, summarizeImportPreview, summarizeImportResult, } from './challenges.pure'; @@ -459,11 +460,11 @@ export class AdminChallengesComponent implements OnInit, OnDestroy { this.importError.set(null); try { const result = await this.admin.confirmChallengeImport(doc); - this.importResult.set(result); this.statusMessage.set(summarizeImportResult(result)); await this.load(); + this.closeImport(); } catch (e: any) { - this.importError.set(e?.error?.message ?? e?.message ?? 'Failed to import challenges'); + this.importError.set(importErrorMessage(e, 'Failed to import challenges')); } finally { this.importing.set(false); } diff --git a/frontend/src/app/features/admin/challenges/challenges.pure.ts b/frontend/src/app/features/admin/challenges/challenges.pure.ts index c178343..88719ea 100644 --- a/frontend/src/app/features/admin/challenges/challenges.pure.ts +++ b/frontend/src/app/features/admin/challenges/challenges.pure.ts @@ -61,6 +61,16 @@ export function summarizeImportResult(r: ImportResultResponse): string { return lines.join(' '); } +export function importErrorMessage(err: unknown, fallback: string): string { + if (err && typeof err === 'object') { + const e = err as { error?: { message?: string }; message?: string }; + if (e.error?.message) return e.error.message; + if (typeof e.message === 'string' && e.message.length > 0) return e.message; + } + return fallback; +} + + export function difficultyClass(difficulty: 'LOW' | 'MEDIUM' | 'HIGH'): string { return `difficulty-badge difficulty-${difficulty.toLowerCase()}`; } diff --git a/tests/backend/admin-challenges-import-replace.spec.ts b/tests/backend/admin-challenges-import-replace.spec.ts new file mode 100644 index 0000000..ebadb72 --- /dev/null +++ b/tests/backend/admin-challenges-import-replace.spec.ts @@ -0,0 +1,249 @@ +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 * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { Test } from '@nestjs/testing'; +import { ConfigModule } from '@nestjs/config'; +import { TypeOrmModule } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { validateEnv } from '../../backend/src/config/env.schema'; +import { AdminChallengesService } from '../../backend/src/modules/admin/challenges.service'; +import { ChallengeFilesService } from '../../backend/src/modules/admin/challenge-files.service'; +import { CategoryEntity } from '../../backend/src/database/entities/category.entity'; +import { ChallengeEntity } from '../../backend/src/database/entities/challenge.entity'; +import { ChallengeFileEntity } from '../../backend/src/database/entities/challenge-file.entity'; +import { SolveEntity } from '../../backend/src/database/entities/solve.entity'; +import { UserEntity } from '../../backend/src/database/entities/user.entity'; +import { SettingEntity } from '../../backend/src/database/entities/setting.entity'; +import { InitSchema1700000000000 } from '../../backend/src/database/migrations/1700000000000-InitSchema'; +import { SeedSystemData1700000000100 } from '../../backend/src/database/migrations/1700000000100-SeedSystemData'; +import { AddCategoryTimestampsAndUniqueAbbrev1700000000200 } from '../../backend/src/database/migrations/1700000000200-AddCategoryTimestampsAndUniqueAbbrev'; +import { UpdateSystemCategoryKeys1700000000300 } from '../../backend/src/database/migrations/1700000000300-UpdateSystemCategoryKeys'; +import { RepairCategorySchemaAndSystemCategories1700000000400 } from '../../backend/src/database/migrations/1700000000400-RepairCategorySchemaAndSystemCategories'; +import { UpgradeChallengeAdminSchema1700000000500 } from '../../backend/src/database/migrations/1700000000500-UpgradeChallengeAdminSchema'; +import { CHALLENGE_FORMAT, CHALLENGE_FORMAT_VERSION } from '../../backend/src/modules/admin/dto/challenges.dto'; + +describe('Job 903: AdminChallengesService - confirmed import replaces the full challenge set', () => { + let service: AdminChallengesService; + let fileService: ChallengeFilesService; + let catRepo: Repository; + let chRepo: Repository; + let fileRepo: Repository; + let cryCategoryId: string; + let uploadDir: string; + let finalDir: string; + + beforeAll(async () => { + uploadDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hipctf-challenges-import-replace-')); + process.env.UPLOAD_DIR = uploadDir; + finalDir = path.join(uploadDir, 'challenges'); + const moduleRef = await Test.createTestingModule({ + imports: [ + ConfigModule.forRoot({ isGlobal: true, validate: validateEnv }), + TypeOrmModule.forRoot({ + type: 'better-sqlite3', + database: ':memory:', + entities: [UserEntity, SettingEntity, CategoryEntity, ChallengeEntity, ChallengeFileEntity, SolveEntity], + migrations: [ + InitSchema1700000000000, + SeedSystemData1700000000100, + AddCategoryTimestampsAndUniqueAbbrev1700000000200, + UpdateSystemCategoryKeys1700000000300, + RepairCategorySchemaAndSystemCategories1700000000400, + UpgradeChallengeAdminSchema1700000000500, + ], + migrationsRun: true, + synchronize: false, + }), + TypeOrmModule.forFeature([CategoryEntity, ChallengeEntity, ChallengeFileEntity, SolveEntity]), + ], + providers: [AdminChallengesService, ChallengeFilesService], + }).compile(); + moduleRef.init(); + service = moduleRef.get(AdminChallengesService); + fileService = moduleRef.get(ChallengeFilesService); + catRepo = moduleRef.get>(getRepositoryToken(CategoryEntity)); + chRepo = moduleRef.get>(getRepositoryToken(ChallengeEntity)); + fileRepo = moduleRef.get>(getRepositoryToken(ChallengeFileEntity)); + const cry = await catRepo.findOne({ where: { systemKey: 'CRY' } }); + if (!cry) throw new Error('expected seeded CRY category'); + cryCategoryId = cry.id; + }); + + afterAll(async () => { + if (uploadDir && fs.existsSync(uploadDir)) { + try { + fs.rmSync(uploadDir, { recursive: true, force: true }); + } catch { + // ignore + } + } + }); + + beforeEach(async () => { + // Clear out any challenges left over from a previous test so the name + // uniqueness constraint does not fire. + const all = await service.list({}); + for (const row of all) await service.remove(row.id); + }); + + async function seedChallenge(name: string): Promise<{ id: string; storedFilename: string | null }> { + const staged = fileService.stage(Buffer.from(name + '-bytes'), `${name}.txt`, 'text/plain'); + const created = await service.create({ + name, + descriptionMd: 'd', + categoryId: cryCategoryId, + difficulty: 'LOW', + initialPoints: 100, + minimumPoints: 50, + decaySolves: 10, + flag: `flag{${name}}`, + protocol: 'WEB', + port: null, + ipAddress: '', + enabled: true, + files: [ + { + stagedToken: staged.token, + originalFilename: staged.originalFilename, + mimeType: staged.mimeType, + sizeBytes: staged.sizeBytes, + }, + ], + } as any); + const fileRow = await fileRepo.findOne({ where: { challengeId: created.id } }); + return { id: created.id, storedFilename: fileRow ? fileRow.storedFilename : null }; + } + + function buildDoc(challenges: any[]): any { + return { + format: CHALLENGE_FORMAT, + version: CHALLENGE_FORMAT_VERSION, + exportedAt: '2026-07-22T00:00:00Z', + challenges, + }; + } + + it('confirmed import wipes unrelated pre-existing challenges', async () => { + await seedChallenge('WipeAlpha'); + await seedChallenge('WipeBravo'); + + const result = await service.importConfirmed( + buildDoc([ + { + name: 'ImportedCharlie', + descriptionMd: 'fresh', + categorySystemKey: 'CRY', + difficulty: 'HIGH', + initialPoints: 250, + minimumPoints: 50, + decaySolves: 10, + flag: 'flag{charlie}', + protocol: 'WEB', + port: null, + ipAddress: '', + enabled: true, + files: [], + }, + ]) as any, + ); + + expect(result.imported).toBe(1); + expect(result.skipped).toEqual([]); + + const names = (await service.list({})).map((r) => r.name).sort(); + expect(names).toEqual(['ImportedCharlie']); + expect(await chRepo.count()).toBe(1); + }); + + it('unlinks the disk files owned by wiped challenges and keeps the imported file', async () => { + const { storedFilename: alphaStored } = await seedChallenge('DiskAlpha'); + + await service.importConfirmed( + buildDoc([ + { + name: 'ImportedDelta', + descriptionMd: 'd', + categorySystemKey: 'CRY', + difficulty: 'LOW', + initialPoints: 100, + minimumPoints: 50, + decaySolves: 10, + flag: 'flag{delta}', + protocol: 'WEB', + port: null, + ipAddress: '', + enabled: true, + files: [ + { + originalFilename: 'delta.txt', + mimeType: 'text/plain', + sizeBytes: Buffer.byteLength('delta-bytes'), + dataBase64: Buffer.from('delta-bytes').toString('base64'), + }, + ], + }, + ]) as any, + ); + + expect(alphaStored).toBeDefined(); + expect(fs.existsSync(path.join(finalDir, alphaStored!))).toBe(false); + + const deltaRow = (await service.list({})).find((r) => r.name === 'ImportedDelta'); + expect(deltaRow).toBeDefined(); + const deltaFiles = await fileRepo.find({ where: { challengeId: deltaRow!.id } }); + expect(deltaFiles).toHaveLength(1); + expect(fs.existsSync(path.join(finalDir, deltaFiles[0].storedFilename))).toBe(true); + }); + + it('empty archive wipes everything and leaves no challenges', async () => { + const { storedFilename: alphaStored } = await seedChallenge('EmptyAlpha'); + + const result = await service.importConfirmed(buildDoc([]) as any); + + expect(result.imported).toBe(0); + expect(result.skipped).toEqual([]); + expect(await chRepo.count()).toBe(0); + expect(await fileRepo.count()).toBe(0); + expect(alphaStored).toBeDefined(); + expect(fs.existsSync(path.join(finalDir, alphaStored!))).toBe(false); + }); + + it('unknown-category rows are reported as skipped but still wipe everything else', async () => { + await seedChallenge('SkipAlpha'); + await seedChallenge('SkipBravo'); + + const result = await service.importConfirmed( + buildDoc([ + { + name: 'Ghost', + descriptionMd: 'd', + categorySystemKey: 'NO_SUCH_CATEGORY', + difficulty: 'LOW', + initialPoints: 100, + minimumPoints: 50, + decaySolves: 10, + flag: 'flag{ghost}', + protocol: 'WEB', + port: null, + ipAddress: '', + enabled: true, + files: [], + }, + ]) as any, + ); + + expect(result.imported).toBe(0); + expect(result.skipped).toEqual([ + { name: 'Ghost', reason: 'Unknown category: NO_SUCH_CATEGORY' }, + ]); + expect(await service.list({})).toEqual([]); + expect(await chRepo.count()).toBe(0); + }); +}); diff --git a/tests/frontend/admin-challenges-import-autoclose.spec.ts b/tests/frontend/admin-challenges-import-autoclose.spec.ts new file mode 100644 index 0000000..28992f1 --- /dev/null +++ b/tests/frontend/admin-challenges-import-autoclose.spec.ts @@ -0,0 +1,98 @@ +import { + importErrorMessage, + summarizeImportResult, +} from '../../frontend/src/app/features/admin/challenges/challenges.pure'; + +describe('Job 903: import error + result helpers', () => { + it('summarizeImportResult reports imported/skipped/warning counts', () => { + const r = { + imported: 2, + skipped: [{ name: 'X', reason: 'Unknown category: Y' }], + warnings: ['1 skipped.'], + }; + const out = summarizeImportResult(r as any); + expect(out).toContain('Imported 2'); + expect(out).toContain('Skipped 1'); + expect(out).toContain('1 warning'); + }); + + it('importErrorMessage prefers the nested error.message envelope', () => { + expect(importErrorMessage({ error: { message: 'Server said no' } }, 'fallback')).toBe('Server said no'); + }); + + it('importErrorMessage falls back to err.message', () => { + expect(importErrorMessage({ message: 'boom' }, 'fallback')).toBe('boom'); + }); + + it('importErrorMessage falls back to the supplied default when no message is available', () => { + expect(importErrorMessage({}, 'fallback')).toBe('fallback'); + expect(importErrorMessage(null, 'fallback')).toBe('fallback'); + }); +}); + +describe('Job 903: confirmed import handler contract', () => { + function makeDoc() { + return { format: 'hipctf-challenges', version: 1, exportedAt: '', challenges: [] }; + } + + it('a successful confirm yields a closed-modal state and a status banner', async () => { + const state = { + importOpen: true as boolean, + importDoc: makeDoc() as any, + importPreview: { total: 2, conflicts: [], unknownCategories: [], warnings: [] } as any, + importResult: null as any, + importError: null as string | null, + importing: false, + statusMessage: null as string | null, + }; + + // Simulate onImportConfirm body using pure helpers. + try { + const result = { imported: 2, skipped: [], warnings: [] }; + state.statusMessage = summarizeImportResult(result as any); + await Promise.resolve(); + // Close-modal branch (the new behavior). + state.importOpen = false; + state.importDoc = null; + state.importPreview = null; + state.importResult = null; + state.importError = null; + } catch (err) { + state.importError = importErrorMessage(err, 'Failed to import challenges'); + } finally { + state.importing = false; + } + + expect(state.importOpen).toBe(false); + expect(state.importDoc).toBeNull(); + expect(state.importPreview).toBeNull(); + expect(state.importResult).toBeNull(); + expect(state.importError).toBeNull(); + expect(state.statusMessage).toContain('Imported 2'); + expect(state.importing).toBe(false); + }); + + it('a rejected confirm keeps the modal open and surfaces the error message', async () => { + const state = { + importOpen: true as boolean, + importDoc: makeDoc() as any, + importPreview: { total: 1, conflicts: [], unknownCategories: [], warnings: [] } as any, + importResult: null as any, + importError: null as string | null, + importing: false, + statusMessage: null as string | null, + }; + + try { + throw { error: { message: 'Server said no' } }; + } catch (err) { + state.importError = importErrorMessage(err, 'Failed to import challenges'); + } finally { + state.importing = false; + } + + expect(state.importOpen).toBe(true); + expect(state.importError).toBe('Server said no'); + expect(state.importing).toBe(false); + }); +});