From 721f58a068c6fbf9bc86c75a292d22ffc6f78306 Mon Sep 17 00:00:00 2001 From: OpenVelo Agent Date: Tue, 21 Jul 2026 17:18:52 +0000 Subject: [PATCH 1/2] docs: update documentation to OKF v0.1 format --- docs/api/setup.md | 42 ++++++++++++++++++++++------ docs/architecture/backend-modules.md | 4 +-- docs/guides/bootstrap.md | 13 +++++---- docs/index.md | 2 +- 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/docs/api/setup.md b/docs/api/setup.md index c54867a..0a8ad8b 100644 --- a/docs/api/setup.md +++ b/docs/api/setup.md @@ -3,7 +3,7 @@ type: api title: Setup Endpoint description: Dedicated POST /api/v1/setup/create-admin endpoint used only while no administrator exists. tags: [api, setup, bootstrap, first-admin] -timestamp: 2026-07-21T16:48:20Z +timestamp: 2026-07-21T17:18:08Z --- # Endpoint @@ -38,13 +38,13 @@ refreshing without storing the raw token in JS. # Error envelope -| HTTP | `code` | When | -|------|---------------------|----------------------------------------------------------------------| -| 400 | `VALIDATION_FAILED` | zod body validation failed (length, regex, or `passwordConfirm`). | -| 400 | `WEAK_PASSWORD` | Argon2 policy rejects the password (length / mixed-case requirement). | -| 404 | `NOT_FOUND` | An admin already exists — the route is hidden once initialized. | -| 409 | `USERNAME_TAKEN` | Username collision (reachable only during a race before init). | -| 429 | `RATE_LIMITED` | `RegistrationRateLimitService` blocked the IP. | +| HTTP | `code` | When | +|------|-----------------------|----------------------------------------------------------------------| +| 400 | `VALIDATION_FAILED` | zod body validation failed (length, regex, or `passwordConfirm`). | +| 400 | `WEAK_PASSWORD` | Argon2 policy rejects the password (length / mixed-case requirement). | +| 409 | `SYSTEM_INITIALIZED` | An admin already exists. Returned to any request that arrives after bootstrap completes (also returned to the loser of a concurrent race — see *Concurrency* below). | +| 409 | `USERNAME_TAKEN` | Username collision (defensive: reachable only if two concurrent requests with the *same* username somehow slip past the in-process lock). | +| 429 | `RATE_LIMITED` | `RegistrationRateLimitService` blocked the IP. | # Wiring @@ -59,6 +59,32 @@ refreshing without storing the raw token in JS. (before any cookie exists) is not blocked. The middleware still mints a `csrf` cookie on the response so subsequent calls are covered. +# Concurrency + +`SetupService` serializes bootstrap attempts inside a single Nest process +via an in-process promise chain (`#bootstrapChain` + private +`runBootstrap()`). Every caller runs its transaction exactly once; the +chain never short-circuits, so validation and rate-limit semantics stay +identical to the single-request case. + +Behavior under contention: + +1. Two simultaneous first-admin POSTs enter `runBootstrap()`. +2. The first request runs its transaction and creates the admin row. +3. The second request waits on the first, then runs its own + transaction; the `adminCount > 0` guard inside the transaction + fires and it throws `SYSTEM_INITIALIZED` (HTTP 409). +4. Exactly one admin row and one refresh-token row exist afterwards. + +The lock is **per-process**; it does not coordinate multiple Nest +instances behind a load balancer. Multi-replica deployments still rely +on the unique-index/transaction guard, not on this chain. + +The matching test lives at +`tests/backend/setup-create-admin.spec.ts` ("only one of two concurrent +first-admin requests succeeds; the other receives a controlled +conflict"). + # Frontend consumer `frontend/src/app/features/setup/setup-create-admin.service.ts` calls diff --git a/docs/architecture/backend-modules.md b/docs/architecture/backend-modules.md index 7840ad3..dbe44ac 100644 --- a/docs/architecture/backend-modules.md +++ b/docs/architecture/backend-modules.md @@ -25,7 +25,7 @@ also registers two global providers: | `SettingsModule` | `backend/src/modules/settings/settings.module.ts` | Exposes `SettingsService` (get/set/getAll over `setting` table). | | `AuthModule` | `backend/src/modules/auth/auth.module.ts` | `AuthController` (`/api/v1/auth/*`) + `AuthService` (login/refresh/logout/register-first-admin).| | `UsersModule` | `backend/src/modules/users/users.module.ts` | `UsersController` (`/api/v1/auth/register-first-admin`) + `UsersService` (last-admin invariant). | -| `SetupModule` | `backend/src/modules/setup/setup.module.ts` | `SetupController` (`POST /api/v1/setup/create-admin`) + `SetupService`. Returns 404 once any admin exists (route hidden). | +| `SetupModule` | `backend/src/modules/setup/setup.module.ts` | `SetupController` (`POST /api/v1/setup/create-admin`) + `SetupService`. Returns 409 `SYSTEM_INITIALIZED` once any admin exists; serializes concurrent first-admin attempts via an in-process promise chain. | | `SystemModule` | `backend/src/modules/system/system.module.ts` | `SystemController` (`/api/v1/bootstrap`, `/event/status`, SSE streams) + `SystemService`. | | `AdminModule` | `backend/src/modules/admin/admin.module.ts` | `AdminController` (`/api/v1/admin/users*`) + `AdminService`. Gated by `AdminGuard` + `@Roles('admin')`. | | `UploadsModule` | `backend/src/modules/uploads/uploads.module.ts` | `UploadsController` (`/api/v1/uploads/*`). Admin-only multipart. | @@ -37,7 +37,7 @@ also registers two global providers: |------------------------|-------------------------|--------------|--------------------------------------------------------------| | `AuthController` | `/api/v1/auth` | Mostly public (login, refresh, logout, csrf) | `backend/src/modules/auth/auth.controller.ts` | | `UsersController` | `/api/v1/auth/register-first-admin` | Public (bootstrap-only) | `backend/src/modules/users/users.controller.ts` | -| `SetupController` | `/api/v1/setup/create-admin` | Public (bootstrap-only; 404 once an admin exists) | `backend/src/modules/setup/setup.controller.ts` | +| `SetupController` | `/api/v1/setup/create-admin` | Public (bootstrap-only; 409 `SYSTEM_INITIALIZED` once an admin exists) | `backend/src/modules/setup/setup.controller.ts` | | `AdminController` | `/api/v1/admin` | Admin only | `backend/src/modules/admin/admin.controller.ts` | | `SystemController` | `/api/v1` | Public | `backend/src/modules/system/system.controller.ts` | | `UploadsController` | `/api/v1/uploads` | Admin only | `backend/src/modules/uploads/uploads.controller.ts` | diff --git a/docs/guides/bootstrap.md b/docs/guides/bootstrap.md index 9ece7bd..cc5e404 100644 --- a/docs/guides/bootstrap.md +++ b/docs/guides/bootstrap.md @@ -3,7 +3,7 @@ type: guide title: First-Run Bootstrap description: How a fresh HIPCTF instance is initialized by the very first administrator. tags: [guide, bootstrap, first-admin, onboarding, tester] -timestamp: 2026-07-21T16:48:20Z +timestamp: 2026-07-21T17:18:08Z --- # When this flow runs @@ -15,12 +15,15 @@ to create the very first admin user. This is gated by the | `initialized` | User experience | |---------------|---------------------------------------------------------------| | `false` | Every route is redirected to `/bootstrap` until an admin is created. | -| `true` | The setup route becomes a 404 and the modal can never appear again. | +| `true` | The setup endpoint becomes a 409 `SYSTEM_INITIALIZED` and the modal can never appear again. | The `authGuard` enforces the redirect on the client side; the backend -independently rejects `/api/v1/setup/create-admin` with `404 NOT_FOUND` -once any admin row exists, so the flow is safe even if the SPA is -bypassed. +independently rejects `/api/v1/setup/create-admin` with +`409 SYSTEM_INITIALIZED` once any admin row exists, so the flow is safe +even if the SPA is bypassed. Concurrent first-admin requests are +serialized inside the Nest process so that two simultaneous submissions +cannot both succeed — the loser receives the same `SYSTEM_INITIALIZED` +409 (see [Setup Endpoint — Concurrency](/api/setup.md)). # How to access (tester steps) diff --git a/docs/index.md b/docs/index.md index 3c58bc8..52a3e5d 100644 --- a/docs/index.md +++ b/docs/index.md @@ -11,7 +11,7 @@ scoreboard, an event window with a public countdown, theming, and admin controls. The docs below are organized by purpose so agents can pull just the slice -they need. Last regenerated 2026-07-21T16:48:20Z. +they need. Last regenerated 2026-07-21T17:18:08Z. # Architecture -- 2.52.0 From 941f125995a72ee5cb8771af513c8f6b0210b4fa Mon Sep 17 00:00:00 2001 From: OpenVelo Agent Date: Tue, 21 Jul 2026 17:18:52 +0000 Subject: [PATCH 2/2] feat: First-Start Create Admin Modal 1.05 --- .kilo/plans/845.md | 147 --------------------- .kilo/plans/846.md | 52 ++++++++ backend/src/modules/setup/setup.service.ts | 96 +++++++++----- tests/backend/setup-create-admin.spec.ts | 55 ++++++-- 4 files changed, 163 insertions(+), 187 deletions(-) delete mode 100644 .kilo/plans/845.md create mode 100644 .kilo/plans/846.md diff --git a/.kilo/plans/845.md b/.kilo/plans/845.md deleted file mode 100644 index da73886..0000000 --- a/.kilo/plans/845.md +++ /dev/null @@ -1,147 +0,0 @@ -# Implementation Plan: Job 845 — First-Start Create Admin Modal 1.04 (surface field validation) - -## 1. Architectural Reconnaissance - -- **Codebase style & conventions:** - - NestJS API + Angular 17 SPA monorepo (workspaces in `package.json`). - - Frontend: **Standalone components**, `ChangeDetectionStrategy.OnPush`, Angular **signals** (`signal`/`computed`), no NgModules. Style guide mandates `OnPush` and signals for state. - - Reactive Forms via `ReactiveFormsModule` with `fb.nonNullable.group(...)` (non-nullable typed controls). - - All services are `providedIn: 'root'`. Two HTTP interceptors wired in `frontend/src/main.ts` (`csrfInterceptor`, then `authInterceptor`). - - Frontend tests are **pure-logic / no TestBed**: `tests/frontend/**/*.spec.ts` import pure functions (validators, session-storage helpers, shell decision helpers) and rely on `jest` (`testEnvironment: 'jsdom'`) runnable with the root command `npm test` (which uses `jest --config tests/jest.config.js --selectProjects frontend`). - - Strictly-typed reactive forms per `angular-forms` skill (`FormGroup`, `fb.nonNullable`). - - Data attributes / `data-testid` selectors are the stable DOM hooks (see `docs/guides/bootstrap.md`). -- **Data Layer:** N/A for this Job — purely a frontend UI/validation correctness fix. The backend already returns a structured `400 VALIDATION_FAILED` envelope; the bug is solely that the SPA short-circuits on `form.invalid` without ever surfacing per-field error messages. -- **Test Framework & Structure:** - - Jest with `ts-jest`, jsdom environment for frontend. - - Tests live **only** in `tests/frontend/` (never co-located with source). - - Root command: `npm test` (runs all projects via `jest --config tests/jest.config.js`). - - Existing precedent at `tests/frontend/setup-create-admin.spec.ts` is a pure-logic suite (mirrors component logic in standalone helper functions). **We will follow that pattern** — no TestBed, no DOM rendering required, no visual confirmation. -- **Required Tools & Dependencies:** No new dependencies, system tools, or `setup.sh` changes needed. The fix is a localized source-code change in the standalone component + its template + a pure-helper module that mirrors the per-field error messages (for parity testing). The npm workspace already has `@angular/forms ^17.3` and `jest`. - -## 2. Impacted Files - -### To Modify -- **`frontend/src/app/features/setup/setup-create-admin.component.ts`** — fix the reactivity of the per-field error calculations and expose `passwordErrors()`. Convert the `computed(...)`s whose bodies read `FormControl` properties (which are NOT signals in Angular 17) into plain **methods** so the template re-evaluates them on every change-detection cycle (cheap, OnPush-friendly, no extra subscriptions). Alternatively keep them as computed but reflect `touched` into a local `signal()` driven by `statusChanges`/`valueChanges` + an effect; **plain methods are the minimal, idiomatic fix** and are used elsewhere in the same component style. Keep all existing public surface (`form`, `submitting`, `serverError`, `serverErrorCode`, `canRetry`, `policyDescription`, `submit`, `swallowBackdrop`, `swallowEscape`) intact. -- **`frontend/src/app/features/setup/setup-create-admin.component.html`** — render the new Password field-error block, mirror `aria-invalid` on the Password and Confirm Password inputs (Username already has it), and keep the `data-testid` hooks the bootstrap guide documents (`setup-username-error`, add `setup-password-error`, keep `setup-confirm-error`). -- **`frontend/src/app/features/setup/setup-create-admin.component.css`** — no functional change required; `.field-error` styling already exists. -- **`tests/frontend/setup-create-admin.spec.ts`** — extend the existing pure-logic suite to cover the new password-error helper and to assert that, given an all-empty (or partially-filled) post-submit state, the per-field error strings the template renders are correct. - -### To Create -- **`frontend/src/app/features/setup/setup-create-admin.field-errors.ts`** — pure helper functions that return the per-field error string (or `null`) for a given `(value, touched)` snapshot. Keeping them as pure functions (no Angular DI, no signals) is deliberate: it lets the component call them from methods **and** lets the pure-logic test suite assert them without spinning up Angular. Mirrors the pattern already used by `setup-create-admin.validators.ts` and tested in `setup-create-admin.spec.ts`. - -## 3. Proposed Changes - -### Root cause -Angular 17 `FormControl.touched`, `.valid`, and `.errors` are plain class properties, **not** signals. The existing `computed(() => … usernameErrors …)` (component file lines 70–78 and 80–87) reads those plain properties and so registers **zero signal dependencies**. Even after `submit()` calls `form.markAllAsTouched()`, the computed is not re-evaluated — its cached `null` is returned and the `@if (usernameErrors()) { … }` block stays hidden. The same applies to `confirmErrors()` for the `passwordConfirm` control. There is also no `passwordErrors()` at all and no `aria-invalid` binding on the Password/Confirm inputs, which matches the user's observation. - -### 3.1 New pure helper module - -`frontend/src/app/features/setup/setup-create-admin.field-errors.ts`: - -```ts -export type FieldErrorKey = - | 'required' - | 'minlength' - | 'maxlength' - | 'pattern' - | 'passwordMismatch'; - -export interface FieldErrorContext { - value: string; - touched: boolean; - errors: Partial> | null; - groupMismatch?: boolean; -} - -export function usernameError(ctx: FieldErrorContext): string | null { … } -export function passwordError(ctx: FieldErrorContext): string | null { … } // currently required-only; password POLICY is server-enforced -export function confirmError(ctx: FieldErrorContext): string | null { … } -``` - -Rules encoded (single source of truth, identical to current component intent): - -- **Username:** - - not touched or no errors → `null` - - `required` → `'Username is required'` - - `minlength` → `'Username must be at least 3 characters'` - - `maxlength` → `'Username must be at most 32 characters'` - - `pattern` → `'Username may only contain letters, digits, dot, dash and underscore'` - - else → `'Invalid username'` -- **Password:** - - not touched or no errors → `null` - - `required` → `'Password is required'` - - else → `null` (policy is intentionally server-side only; this component never surfaces a "weak password" field error pre-submit) -- **Confirm:** - - not touched and no `passwordMismatch` group error → `null` - - `required` → `'Please confirm your password'` - - `passwordMismatch` (group-level) → `'Passwords do not match'` - - else → `'Invalid value'` - -These signatures accept the small slice of state they need so the component can pass `c.value`, `c.touched`, and `c.errors` (cast through `Partial>`) and so the test suite can call them directly without touching Angular. - -### 3.2 Component changes - -`setup-create-admin.component.ts`: - -1. **Replace the two `computed(...)`s with plain methods** so the template re-renders them on every CD pass: - - `usernameError(): string | null { return usernameError({ value: this.form.controls.username.value, touched: this.form.controls.username.touched, errors: this.form.controls.username.errors }); }` - - `passwordError(): string | null { return passwordError({ value: this.form.controls.password.value, touched: this.form.controls.password.touched, errors: this.form.controls.password.errors }); }` ← **new** - - `confirmError(): string | null { return confirmError({ value: this.form.controls.passwordConfirm.value, touched: this.form.controls.passwordConfirm.touched, errors: this.form.controls.passwordConfirm.errors, groupMismatch: !!this.form.errors?.['passwordMismatch'] }); }` -2. **Delete the orphaned `usernameErrors` and `confirmErrors` signal properties** (template rewires to methods). -3. Leave `submit()` exactly as-is: `markAllAsTouched()` then early-return on invalid. The early return is now correct and observable (because `…Error()` is re-evaluated by the next CD cycle that runs after `markAllAsTouched` synchronously mutates `touched` on every control). No new `effect()` needed; this is the minimal fix. -4. Keep OnPush (`ChangeDetectionStrategy.OnPush`), keep all existing imports/decorators/HostListener for `escape` and `swallowBackdrop`. The host binding keyboard handler stays so the modal remains non-dismissible. - -> Optional, NOT recommended for this Job (kept out of scope to honor "minimal changes"): a reactive alternative would be to subscribe to `statusChanges` / `valueChanges` via `toSignal()` in the constructor and gate rendering on those signals. The plain-method approach above is the minimum-surface fix that fully resolves the bug and matches the component's current style. - -### 3.3 Template changes - -`setup-create-admin.component.html`: - -1. Username `` — keep existing `[attr.aria-invalid]="!!usernameError()"` and existing `` block, but rebind to the new method name. -2. Password `