AI Implementation feature(919): Admin Area System: Database Backup/Restore and Danger Zone 1.02 (#64)
This commit was merged in pull request #64.
This commit is contained in:
@@ -1,32 +0,0 @@
|
||||
# Implementation Plan: Job 918 — Admin Area System Database Backup/Restore and Danger Zone 1.01
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
- **Codebase style & conventions:** Node.js monorepo using strict TypeScript, NestJS controllers/services on the backend, Angular standalone components and signals on the frontend, async/await, dependency injection, Zod request/archive validation, and stable `ApiError` codes. The requested UI and API flow already exists; the defect is isolated to the backend restore rebuild. `RestoreService.cloneAndOverwriteDatabase()` clones the live SQLite file and executes `DELETE` statements while the live schema's `trg_user_last_admin_delete` trigger remains active. `PRAGMA foreign_keys = OFF` does not disable triggers, so deleting the sole live admin raises `LAST_ADMIN` before archived users can be inserted.
|
||||
- **Data Layer:** SQLite through TypeORM's `better-sqlite3` driver. Runtime migrations create a database-level invariant with `trg_user_last_admin_update` and `trg_user_last_admin_delete`. Backups dynamically include application tables but exclude `admin_operation_token`, `migrations`, and SQLite internals. Restore uses a cloned database, direct parameterized `better-sqlite3` statements, and a database/uploads filesystem swap with rollback handles.
|
||||
- **Test Framework & Structure:** Jest 29 with `ts-jest`; backend tests live under `/repo/tests/backend` and are selected by `/repo/tests/jest.config.js`. Root `npm test` runs backend and frontend projects, while `npm run test:backend` runs the focused backend suite. Tests must remain CLI-only and use isolated temporary SQLite/upload/staging paths rather than shared mutable resources.
|
||||
- **Required Tools & Dependencies:** No new package, system tool, global CLI, or persistent `/data` asset is required. Existing Node.js/npm, TypeScript, Jest, Nest testing utilities, TypeORM, and `better-sqlite3` are sufficient. `setup.sh` already installs dependencies and rebuilds the native SQLite binding, so it should not be changed.
|
||||
|
||||
## 2. Impacted Files
|
||||
- **To Modify:**
|
||||
- `backend/src/modules/admin/system/restore.service.ts` — make the offline clone rebuild temporarily suspend the last-admin triggers, restore all archived rows, validate invariants, and reinstate the triggers before the file is eligible for swapping.
|
||||
- `tests/backend/admin-system-restore-commit.spec.ts` — add a focused real-file restore regression covering a backup that contains the same sole admin as the live database, restored table content, uploads, and trigger preservation; retain the existing controller/DTO contract coverage.
|
||||
- **To Create:** None.
|
||||
|
||||
## 3. Proposed Changes
|
||||
1. **Database / Schema Migration:**
|
||||
- Do not add or alter a migration: the `LAST_ADMIN` triggers are correct for ordinary runtime user mutations.
|
||||
- In the private staged database clone only, read the exact SQL definitions of `trg_user_last_admin_update` and `trg_user_last_admin_delete` from `sqlite_master`, fail closed if the expected trigger definitions cannot be captured, and drop those triggers before clearing application tables.
|
||||
- Keep foreign keys disabled during the bulk replacement, clear all known application tables, and insert archived rows in the existing dependency-safe order with parameterized values.
|
||||
- Before closing the staged clone, verify the archive produced at least one admin user so a restore cannot bypass the deployment-wide invariant. Recreate both captured triggers exactly, re-enable foreign keys, run `PRAGMA foreign_key_check`, and treat any violations or trigger-restoration failure as a restore failure. This confines the temporary invariant suspension to an offline candidate file and ensures the candidate has full protections before swap.
|
||||
2. **Backend Logic & APIs:**
|
||||
- Preserve the existing `POST /api/v1/admin/system/restore/validate`, confirmation-token, and `POST /restore/commit` contracts; `stagingId` already survives Zod validation and reaches `RestoreService.commitRestore()`.
|
||||
- Refactor the staged rebuild cleanup so database closure and pragma/trigger restoration are deterministic and original failures are not masked by cleanup. Any rebuild, integrity, swap, or verification error must continue through the existing rollback path and return `SYSTEM_RESTORE_ROLLED_BACK` without changing live data/uploads.
|
||||
- Keep the existing filesystem transaction: build and validate the candidate first, copy staged uploads beside the live upload root, swap database and uploads together, verify the swapped database, then commit rollback artifacts.
|
||||
- Keep post-success session revocation in `AdminSystemController.commitRestore()`. After the restored database is live, revoke refresh sessions for the authenticated admin ID; the frontend's already-wired `forceServerInvalidation()` then clears the access session and redirects all tabs to login.
|
||||
3. **Frontend UI Integration:**
|
||||
- No frontend changes are planned. The file picker, staging summary, reauthentication modal, confirmation phrase, restore commit request, rollback alert, data-cache invalidation, and forced-login behavior are already wired in `frontend/src/app/features/admin/system/system.component.ts`, `system.service.ts`, and the auth/system data-change services.
|
||||
|
||||
## 4. Test Strategy
|
||||
- **Target Unit Test File:** `tests/backend/admin-system-restore-commit.spec.ts`.
|
||||
- **Mocking Strategy:** Keep existing controller dependencies mocked for request forwarding/token/session-revocation assertions. Add one minimal service-level integration regression using a unique temporary directory, a real file-backed `better-sqlite3` database, real `RestoreService`, and real `FilesystemTransactionService`; mock only `ConfigService` and the live `DataSource.query` verification boundary as needed. Arrange a live database with the migrated last-admin triggers, one sole admin, pre-restore challenge/blog/setting rows, and live uploads; stage a valid archive containing that same admin plus different table values and replacement challenge/icon files; commit; then open the swapped file independently and assert archived counts/settings, exact upload replacement, and both last-admin triggers still exist and reject deleting the sole restored admin. Add one focused invalid-archive invariant case with no admin and assert `SYSTEM_RESTORE_ROLLED_BACK` plus unchanged live database/uploads. Clean all temporary files in `afterEach`/`afterAll`; no UI, network, visual checks, shared `/data`, or large mock environment.
|
||||
- **Execution:** Follow red-green-refactor: first run the focused backend spec to confirm the `LAST_ADMIN` failure, implement the smallest restore-service correction, rerun the focused spec, then run `npm run test:backend`, root `npm test`, `npm --workspace backend run build`, and the repository lint/typecheck commands if present (the current package scripts expose build but no dedicated lint/typecheck command).
|
||||
@@ -0,0 +1,69 @@
|
||||
# Implementation Plan: Job 919 — Crash-Safe Admin Database Backup/Restore and Danger Zone Recovery
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
- **Status:** The requested Job is **not fully implemented**. Restore currently has an in-process rollback handle only; a SIGKILL during or after the multi-path swap can leave the database and upload directory in different generations, and startup has no recovery sweep for rollback artifacts. Relevant existing implementation is in `backend/src/modules/admin/system/restore.service.ts` and `backend/src/modules/admin/system/filesystem-transaction.service.ts`.
|
||||
- **Codebase style & conventions:** TypeScript monorepo with a NestJS 10 backend, Angular 17 standalone frontend, TypeORM over `better-sqlite3`, synchronous local filesystem primitives for staged archive files and rename operations, async Nest services/controllers, Zod schemas through `ZodValidationPipe`, and `ApiError`/global exception-envelope handling. `main.ts` explicitly initializes the database before listening. Controllers are thin and delegate destructive work to services.
|
||||
- **Data Layer:** File-backed SQLite configured through `DATABASE_PATH`, TypeORM/`DataSource` for the live application connection, migrations with WAL enabled by `DatabaseInitService`, and direct `better-sqlite3` use in restore for an offline candidate database. Application uploads live below `UPLOAD_DIR`; backups encode the complete upload tree as base64 entries. No database schema migration is required for this fix unless the implementer elects to persist a transaction journal; the preferred design uses a durable filesystem transaction manifest instead of application tables so recovery works before ordinary application access.
|
||||
- **Test Framework & Structure:** Jest 29 with `ts-jest`, two root projects (`backend` Node and `frontend` jsdom), and all tests in the dedicated `/repo/tests` tree. Backend restore coverage is concentrated in `tests/backend/admin-system-restore-commit.spec.ts`; transaction behavior currently has no focused dedicated test. Tests must remain CLI-runnable through the root `npm test`, use isolated per-test filesystem/database roots, and avoid UI/manual verification. Add only focused tests for crash/restart recovery, rollback-artifact cleanup, and coherent post-recovery state.
|
||||
- **Required Tools & Dependencies:** Existing Node/npm toolchain, TypeScript, Jest, `better-sqlite3`, NestJS, TypeORM, and Node `fs`/`path`/`crypto` APIs are sufficient. No new runtime package is required. Do not add a database or external queue. `setup.sh` already creates the normal upload directories, installs dependencies, rebuilds `better-sqlite3`, and builds both workspaces; update it only if the chosen implementation introduces a required OS utility (not expected). If using subprocess-based crash simulation, use the existing Node runtime and Jest rather than adding a global CLI.
|
||||
|
||||
## 2. Impacted Files
|
||||
- **To Modify:**
|
||||
- `backend/src/modules/admin/system/filesystem-transaction.service.ts` — replace the ephemeral, in-memory-only swap state with a durable transaction manifest/state machine; make recovery deterministic across process death, preserve pair ordering, and clean rollback/staged artifacts only after the transaction reaches a durable terminal state.
|
||||
- `backend/src/modules/admin/system/restore.service.ts` — create and finalize a restore transaction through the durable transaction service, avoid treating the database and uploads as independently committed, and ensure candidate/WAL/SHM/staging artifacts are included in recovery and cleanup decisions.
|
||||
- `backend/src/database/database-init.service.ts` — invoke startup recovery before migrations/normal schema verification and fail closed if a transaction cannot be safely resolved; ensure recovery runs before HTTP traffic is accepted.
|
||||
- `backend/src/main.ts` — only if needed to expose the recovery call in the explicit bootstrap sequence; preserve the documented `DatabaseInitService.init()`-before-`listen()` ordering and do not serve uploads before recovery completes.
|
||||
- `backend/src/config/env.schema.ts` — only if a configurable transaction-manifest/stale-transaction retention setting is introduced; otherwise no change is needed.
|
||||
- `backend/src/common/utils/upload.ts` — only if shared helpers are needed for locating the transaction journal/staging root; reuse `resolveSystemStagingDir` and keep system staging outside public uploads.
|
||||
- `tests/backend/admin-system-restore-commit.spec.ts` — extend the existing minimal restore tests for a simulated interruption between pair swaps and verify the database/upload generation is coherent after recovery.
|
||||
- `tests/backend/filesystem-transaction.spec.ts` — create a focused unit/integration test file in the dedicated backend test folder for durable manifest transitions, recovery of interrupted swaps, idempotent cleanup, and refusal to guess when the manifest is incomplete.
|
||||
- `tests/backend/database-init-recovery.spec.ts` — create a focused startup-recovery test that runs the initialization recovery path against isolated file-backed data and checks orphan rollback directories are removed or safely restored before normal startup.
|
||||
- `docs/api/admin-system.md` — update restore semantics/error behavior if the externally observable contract changes, especially recovery guarantees and startup handling.
|
||||
- `docs/guides/admin-system-page.md` — update operational guidance only if restore failure/startup recovery messaging or operator cleanup expectations change.
|
||||
- **To Create:**
|
||||
- `backend/src/modules/admin/system/transaction-recovery.service.ts` (recommended) — a small startup/recovery coordinator if keeping recovery logic out of `FilesystemTransactionService`; it should discover manifests, classify states, resolve only provably safe outcomes, and report/fail closed on ambiguous state.
|
||||
- `tests/backend/filesystem-transaction.spec.ts` — dedicated focused transaction tests.
|
||||
- `tests/backend/database-init-recovery.spec.ts` — dedicated focused startup recovery tests.
|
||||
- A durable manifest file format under the configured system staging/data directory at runtime, not a committed fixture; no persistent repository data should be added outside code/tests. Test-generated databases and upload trees should be created in unique temporary directories, or under `/data` only when a successor job must reuse them.
|
||||
|
||||
## 3. Proposed Changes
|
||||
Explain the exact implementation logic step-by-step:
|
||||
|
||||
1. **Database / Schema Migration:**
|
||||
- Do not add application tables or migrations for the transaction protocol. The failure occurs during filesystem replacement and can happen before a database connection is usable; recovery metadata must therefore be independent of SQLite and durable on the same filesystem as the database/upload paths.
|
||||
- Define a versioned manifest containing a transaction id, operation name, ordered swap pairs, absolute or validated repository-relative live/staged/rollback paths, intended generation, and an explicit phase such as `prepared`, `live-snapshots-created`, `replacements-installed`, `verified`, `committed`, or `rollback-required`.
|
||||
- Write the manifest atomically (`temporary manifest` → flush/close as supported → rename into the canonical manifest path) before the first live path is moved. Update it atomically after every irreversible filesystem step. Use a unique transaction directory under `resolveSystemStagingDir`, never under the public upload tree, and include database sidecars (`-wal`, `-shm`) in the database replacement protocol or explicitly normalize/remove them so an old WAL cannot be paired with a new main DB.
|
||||
- Persist enough information to distinguish these cases after a kill: all old paths available and replacements incomplete means restore the entire old generation; every replacement installed and candidate verification recorded means finish/retain the new generation; any mixed/ambiguous state means do not delete either generation and fail startup closed with an actionable log/error. The implementation must never simply delete an unknown rollback directory.
|
||||
- Ensure durability ordering: manifest exists before mutation; each rename/copy result is recorded before advancing; the terminal commit marker is written only after both database and upload generations are present and verified; only then remove rollback and staged artifacts. Cleanup is best effort after the terminal marker and must not erase the only known recoverable generation.
|
||||
|
||||
2. **Backend Logic & APIs:**
|
||||
- Refactor `FilesystemTransactionService.stageSwap()` into a crash-recoverable protocol while retaining its public rollback/commit behavior for existing callers. Keep pair processing deterministic and perform rollback in reverse order for in-process failures, but make the manifest authoritative so a process restart can continue recovery.
|
||||
- Add a public startup recovery method (either on `FilesystemTransactionService` or the recommended `TransactionRecoveryService`) that scans only the configured transaction root for unfinished manifests and stale restore rollback artifacts. It must be idempotent: repeated startup attempts converge to the same coherent state and do not recreate or delete unrelated files.
|
||||
- For a manifest whose old database and old upload tree are both available, restore both old paths as one generation. For a manifest whose new database and new upload tree are both fully installed and whose post-swap verification marker is durable, finalize the new generation and remove old rollback artifacts. For the observed hybrid case (old DB plus new uploads, or vice versa), select the old generation only when both old paths are present; otherwise select the fully verified new generation only when both new paths are present; otherwise fail closed rather than producing a third hybrid.
|
||||
- Add explicit validation of the recovered database and upload generation before finalization. Database validation should open the selected SQLite file read-only, run the existing foreign-key/integrity checks and required-admin invariant, and account for WAL/SHM sidecars. Upload validation should ensure the selected tree is a complete directory replacement, not a merge; do not copy individual files from one generation into the other. Any orphaned rollback/staged directory associated with a transaction should be removed only after the selected generation is installed and the durable commit/rollback marker is written.
|
||||
- Change `RestoreService.commitRestore()` to use the durable transaction API and remove unused `liveBackupDb` behavior. Candidate DB construction should remain offline and candidate verification should happen before the swap whenever possible; `afterSwap` must verify the selected live DB without relying on an already-open TypeORM connection that may still point at the old inode. The result should report success only after the transaction is durably committed.
|
||||
- Handle process-level concurrency by preventing two destructive filesystem transactions from running simultaneously. Reuse the existing operation-in-progress/error conventions if present; otherwise add a small lock/manifest guard in the transaction root. A stale lock must be recoverable only through manifest inspection, not by blindly deleting it.
|
||||
- Keep controller routes and confirmation-token contracts unchanged. Restore success should continue to revoke refresh sessions; restore rollback errors should continue to map to `SYSTEM_RESTORE_ROLLED_BACK`. If startup recovery cannot establish a coherent state, fail startup before `app.listen()` and log the transaction id/phase without exposing secrets or raw archive contents.
|
||||
- Update backup traversal if necessary to exclude all transaction metadata/rollback directories, not just `.staging`, so an interrupted restore can never be re-archived as application upload content. The exclusion must be based on exact reserved names/prefixes and remain scoped to `UPLOAD_DIR`.
|
||||
- No frontend feature work is required: the existing admin restore UI already reports `SYSTEM_RESTORE_ROLLED_BACK` and logs the user out after a successful restore. Only update frontend pure helpers/tests if the backend introduces a new user-visible error code; prefer retaining existing codes.
|
||||
|
||||
3. **Frontend UI Integration:**
|
||||
- No component or route changes are planned. The existing `SystemService` calls validation and commit endpoints, and the admin system page already handles success/failure. Verify that any recovery failure remains represented by the existing restore-rollback/startup failure behavior; do not add visual tests or UI infrastructure.
|
||||
|
||||
4. **Operational/startup integration:**
|
||||
- Invoke recovery before `DatabaseInitService.ensureWalMode()`, migrations, seed verification, static upload mounting, and `app.listen()`. The ordering is critical because opening the old/new database or serving uploads before resolving the manifest can recreate the mixed state.
|
||||
- Preserve configured path safety: resolve `DATABASE_PATH`, `UPLOAD_DIR`, and staging paths once through `ConfigService`; reject manifests whose paths escape the configured data roots; never follow symlinks or accept arbitrary manifest paths from the request payload.
|
||||
- Document that recovery is automatic on restart, that an ambiguous transaction prevents startup rather than silently deleting data, and that an operator must preserve the transaction directory for forensic/manual recovery.
|
||||
|
||||
## 4. Test Strategy
|
||||
- **Target Unit Test File:** `tests/backend/filesystem-transaction.spec.ts` for the transaction state machine and recovery behavior; `tests/backend/database-init-recovery.spec.ts` for startup integration; extend `tests/backend/admin-system-restore-commit.spec.ts` with one end-to-end restore coherence assertion. Keep all tests in `/repo/tests/backend`, never alongside source.
|
||||
- **Mocking Strategy:** Use unique file-backed temporary directories per test (or `/data` only for intentionally persistent successor-job fixtures), real Node filesystem operations, and small real SQLite databases created with `better-sqlite3`. Mock only unrelated Nest/DataSource boundaries and time/randomness where deterministic manifest names are needed. Do not mock the filesystem for the core recovery tests because the bug is specifically a crash/interrupted-rename condition. Avoid requiring a browser, UI, external service, Docker, or a large seeded database.
|
||||
- **Focused cases:**
|
||||
1. Successful restore writes a terminal commit marker and leaves exactly the restored DB/upload generation with no rollback or stage artifacts.
|
||||
2. Simulated interruption after the database pair is swapped but before uploads are swapped leaves a durable manifest; startup recovery restores the old database and old uploads together, removes both new/staged artifacts, and never leaves the backup upload file in the live tree.
|
||||
3. Simulated interruption after both pairs are swapped but before cleanup allows startup recovery to retain the complete new generation and remove old rollback artifacts.
|
||||
4. The observed hybrid layout (old live DB plus new live uploads plus old rollback upload tree) is resolved to the old generation when both old rollback paths are available; assertions cover DB rows, referenced upload files, absence of backup-only orphan files, and cleanup of rollback directories.
|
||||
5. A manifest with neither a complete old generation nor a verified complete new generation causes startup recovery to fail closed and preserves artifacts for operator recovery.
|
||||
6. Recovery is idempotent when invoked twice and does not alter unrelated uploads or databases.
|
||||
7. Existing no-admin restore rejection and successful restore coverage remain green, proving the new transaction protocol does not weaken the `LAST_ADMIN` invariant or existing API contract.
|
||||
- **CLI execution:** The existing root `npm test` runs both Jest projects in one command. The implementer must run the focused backend tests and then root lint/typecheck commands if defined by the repository; no new dependency or test command should be required. If the test runner executes suites in parallel, each suite must use a unique filesystem root and close all SQLite handles/resources in `afterEach`/`afterAll`.
|
||||
Reference in New Issue
Block a user