HIPCTF2/.kilo/plans/846.md

Implementation Plan: First-Start Create Admin Modal 1.05

1. Architectural Reconnaissance

• Status: The requested race-condition fix is not fully implemented. backend/src/modules/setup/setup.service.ts:58-93 performs the admin count check inside a transaction, but two transactions can both observe zero admins before the Argon2 hash and subsequent insert. Existing tests cover serial creation and post-initialization rejection, but not simultaneous different-user submissions or refresh-token cardinality.
• Codebase style & conventions: TypeScript monorepo using NestJS 10, Angular 17 standalone components, async/await, dependency injection, Zod request validation, TypeORM repositories/transactions, and centralized ApiError/global exception formatting. Controllers are thin; bootstrap business logic belongs in SetupService.
• Data Layer: SQLite through better-sqlite3, accessed with TypeORM and explicit migrations (synchronize: false). The user table has a unique username but no invariant-enforcing uniqueness constraint for the role value. Database startup runs migrations and enforces WAL mode. The setup mutation and refresh-token creation already share one TypeORM transaction.
• Test Framework & Structure: Root Jest multi-project configuration (tests/jest.config.js) runs backend Node tests from tests/backend/**/*.spec.ts and frontend jsdom tests from tests/frontend/**/*.spec.ts; tests are kept outside source directories and run with npm test. Backend integration tests use Nest Test.createTestingModule, the real SQLite TypeORM database, supertest via csrfClient, and cleanup in beforeEach/afterAll.
• Required Tools & Dependencies: No new package or system dependency should be required. Continue using the existing Node/npm toolchain, NestJS, TypeORM, better-sqlite3, Argon2, Jest, and Supertest. setup.sh already installs dependencies and builds both workspaces; do not add infrastructure unless the chosen SQLite locking approach requires an existing package (the preferred plan does not). If a migration is added, it is TypeScript and is included by the existing database migration registration rather than requiring a new CLI.

2. Impacted Files

• To Modify:
  • backend/src/modules/setup/setup.service.ts — serialize or otherwise atomically coordinate first-admin bootstrap attempts so only one request can pass the empty-admin gate and complete user/session creation.
  • tests/backend/setup-create-admin.spec.ts — add one focused concurrent-submission regression test asserting one success, one controlled conflict/business-rule failure, exactly one admin, and exactly one refresh-token row.
  • docs/api/setup.md — document the concurrency guarantee and the structured losing-request response if the public contract changes from the currently documented 404/USERNAME_TAKEN cases.
  • docs/database/users.md and/or docs/guides/bootstrap.md — update the first-admin invariant description to explicitly cover concurrent requests, if documentation is maintained as part of implementation.
  • backend/src/common/errors/error-codes.ts — only if implementation introduces a dedicated bootstrap-race error code rather than reusing the existing SYSTEM_INITIALIZED or NOT_FOUND code.
• To Create:
  • No new application source file is required by the preferred design.
  • A new migration file is not expected unless the implementer chooses a database-level sentinel/lock-row design; if that design is selected, create a timestamped migration under backend/src/database/migrations/ and register it in backend/src/database/database.module.ts.

3. Proposed Changes

Explain the exact implementation logic step-by-step:

1. Database / Schema Migration:

   • Prefer no schema change: the existing transaction and SQLite WAL database are sufficient if the setup critical section is serialized before the admin count check.
   • Implement a process-local, promise-based mutex/critical-section guard owned by SetupService (or an equivalent injected singleton lock service) around the entire first-admin check-to-commit path. The lock must be acquired before dataSource.transaction performs the admin count query and released in a finally block on success, validation failure, database failure, or controlled conflict.
   • Keep the password equality, password-policy validation, rate-limit check, and Argon2 hashing semantics unchanged. The critical requirement is that the second request cannot execute the empty-admin check until the first transaction has either committed the admin and refresh token or rolled back.
   • Once the first transaction commits, the next request rechecks the admin count and exits without hashing, creating a user, or minting a refresh token. Return a controlled business-rule response. The cleanest contract is the existing SYSTEM_INITIALIZED 409 already used by the legacy first-admin endpoint; alternatively retain the setup endpoint's existing NOT_FOUND 404 contract if compatibility is more important. Whichever status/code is selected must be consistent and explicitly tested/documented.
   • Do not rely on the username unique constraint alone: concurrent requests intentionally use different usernames, so that constraint cannot enforce the first-admin invariant. Do not rely on the current unlocked count query or on Argon2's synchronous blocking behavior.
   • If deployment topology may include multiple Node processes/workers, document that an in-process mutex only protects one process. For a true multi-process guarantee, replace it with a database-backed lock/sentinel approach: add one immutable bootstrap lock row/table via migration, acquire it using SQLite's write serialization/transaction semantics before counting admins, then perform the check, hash, insert, and session mint in the same transaction. Validate the chosen approach against the repository's single-process topology before implementation; the minimum Job reproduction is within one Nest process.

2. Backend Logic & APIs:

   • Update SetupService.createAdmin so every path that can mutate bootstrap state is within the same serialized/atomic boundary. Preserve the existing transaction callback ordering: count admins, reject initialized systems, check username, hash password, save enabled admin, record rate limit, and call AuthService.createSession using the transaction manager.
   • Ensure lock release occurs after the transaction promise settles, not immediately after starting it. This prevents the second request from observing an uncommitted or partially completed first request.
   • Normalize the losing request through ApiError with a stable structured code, message, and HTTP status. Do not expose raw SQLite SQLITE_BUSY/constraint errors. If a database-level locking strategy is used, catch/map expected lock contention to the same business-rule error and ensure failed transactions cannot leave a user or refresh-token row behind.
   • Leave SetupController as a thin delegator. It should continue to derive the IP, call the service, set the HttpOnly refresh cookie only on a successful session, and return the existing 201 response shape for the winner.
   • Keep frontend behavior unchanged unless the selected loser code is not already handled. SetupCreateAdminService already preserves status/code from the API, while SetupCreateAdminComponent treats only USERNAME_TAKEN as non-retryable and presents a generic retryable error for other codes. If SYSTEM_INITIALIZED is chosen, update the component/service contract only if UX should stop retrying after another caller wins; otherwise document that a concurrent loser should be redirected/reloaded rather than repeatedly retrying a now-initialized setup route.
   • Update the setup API documentation to state: among simultaneous valid first-admin requests, exactly one may return 201; all others receive the chosen structured conflict/business-rule response; only one enabled admin and one refresh-token row are created.

3. Frontend UI Integration:

   • No visual/modal redesign is required. The modal already disables duplicate clicks within one component instance (submitting signal), but that does not protect independent browser requests; the invariant must remain server-side.
   • If backend returns SYSTEM_INITIALIZED/409 for the losing request, decide and implement the smallest compatible handling: mark the setup flow unavailable and route to login/bootstrap reload, or retain the existing generic failure presentation. Add a frontend logic test only if a changed error-code branch is introduced; do not add visual/browser tests for this backend race fix.

4. Test Strategy

• Target Unit Test File: tests/backend/setup-create-admin.spec.ts.
• Add one minimal regression test using the existing initialized Nest app and real in-memory SQLite database. Clear both users and refresh tokens before the test (the current beforeEach only clears users, so the new test must explicitly clean refresh tokens or use a safe test isolation arrangement).
• Arrange two independent valid requests with different usernames and invoke them concurrently with Promise.all through separate csrfClient instances or equivalent Supertest agents. Avoid mocking Argon2, TypeORM, or the database: the bug is transaction/concurrency behavior and must be verified at the HTTP/service boundary.
• Assert that the result statuses contain exactly one 201 and exactly one expected controlled loser status (normally 409, or 404 if compatibility-preserving behavior is selected). Assert the loser body has the exact structured error code; do not assert a generated token value.
• Assert database postconditions through repositories: exactly one row with role: 'admin', exactly one enabled first-admin user overall, and exactly one row in refresh_token. Verify the winning username is one of the two submitted names and the losing username is absent. This directly guards against both duplicate admins and duplicate sessions.
• Retain the existing focused tests for winner success, serial post-initialization rejection, weak passwords, mismatch, and invalid usernames. Update only assertions/comments that become stale if the selected concurrency error code changes.
• Follow AAA structure, clear mocks if any are introduced, close the Nest app, and ensure no shared persistent data is written. Use the existing DATABASE_PATH=':memory:'; /data is unnecessary because the test does not create successor-job assets or persistent fixtures.
• Run the repository's single root command npm test during implementation, plus the project's available build/type validation commands discovered from package scripts. There is no lint or typecheck script in the inspected root/backend package manifests; do not invent one. setup.sh needs no update for the planned implementation.