Fold backupService into the backup domain

src/services/backupService.ts moves to src/nest/backup/backup.impl.ts, imported
only by its own domain now that the controller reaches it through BackupService.
src/services/ is down to airtrail/.

It stays a module of free functions rather than becoming methods on the service.
The restore path closes and reinitializes the core DB handle, which is the single
most dangerous sequence in the server, and rewriting its shape in the same step
as the move would make a regression there impossible to bisect. The db-lifecycle
question is the last step of this fold, not part of it.

The move also pulled 512 lines out of ungated src/services/ into a gated domain,
which is how the per-domain ratchet caught that the per-entry extraction loop had
no tests at all: the zip-slip refusal, the running decompressed-byte cap (the
declared size in the central directory is attacker-controlled, so only the
running total catches a lying archive), a corrupt stream not being dressed up as
a size refusal, and the reopen failure that has to report restart-required rather
than success. Six cases now cover it, and backup goes from 84% to 96% statements.

server/src/nest/README.md

@@ -48,14 +48,28 @@ remains as the platform underneath `@nestjs/platform-express`.
   wiring sheet named throughout the log below — is now
   `PluginRpcHostFactory` and injects two things. See
   `plugins/host/rpc-kit/README.md`.
+- **Plugin module split (complete):** `PluginsModule` was 21 controllers, 24 domain
+  imports and 10 providers in one class, so `AdminModule` importing it for one
+  cascade-disable call inherited the whole graph. It is now `PluginsRuntimeModule`
+  (supervisor, capability router, hook contracts — the half with the domain imports),
+  `PluginContributionsModule` (the 14 read-only hook controllers, in
+  `plugins/contributions/`), `PluginOAuthModule` (`plugins/oauth/`) and a composition
+  root holding the install and delivery controllers. `AdminModule` imports the runtime
+  alone. Note the invariant the boot-time coverage check now depends on: importing
+  `PluginsRuntimeModule` must pull in EVERY domain that owns part of the wire surface,
+  which is why `WeatherModule` sits in its import list.
 - **Guards (BE-Phase 2, in progress):** the JWT verify lives in `auth/`, and
   `middleware/auth.ts`, `validate.ts` and `idempotency.ts` are deleted; the three
   addon guards are one `AddonGuard` + `@RequireAddon`; the demo-mode block is one
   condition. Still open: `TripAccessGuard`, default-deny, the MFA policy.
 
-`src/services/` is down to one top-level file (backup) plus the `airtrail/`
-directory. Both wait on an open decision: backup on the plugins module split and
-the db-lifecycle question, airtrail on the credential handling. Note that the trip-access and
+`src/services/` is down to the `airtrail/` directory, which waits on the credential
+decision. Backup has moved in: `services/backupService.ts` is now
+`nest/backup/backup.impl.ts`, imported only by its own domain. It stays a module of
+free functions rather than becoming methods, because the restore path closes and
+reinitializes the core DB handle and rewriting that shape in the same step as the move
+would make a regression there impossible to bisect — the db-lifecycle question is the
+LAST step of that fold, not part of it. Note that the trip-access and
 `canEdit` methods on the domain services are **not** dead weight waiting for a
 guard: their callers are overwhelmingly the `*.mcp.ts` tools, which never pass
 through an HTTP guard. In the five domains piloted for `TripAccessGuard`, 40 of
@@ -95,7 +109,7 @@ Two shapes, and the choice is settled:
 from the app after `buildApp()` — the way `bootstrap.ts` hands the `/mcp`
 handler its registry. The declaration is **structural** (`{ backups: {
 createBackup } }`), never the provider class: importing `BackupService` would
-pull in `src/nest/backup` → `src/services/backupService` → `src/scheduler` and
+pull in `src/nest/backup` → `nest/backup/backup.impl` → `src/scheduler` and
 close a cycle.
 
 **`<domain>.bridge.ts` is the older shape** and still correct for the MCP
@@ -164,6 +178,20 @@ strings. Where the legacy route returns a hand-written error (e.g. weather's
 `{ error: 'Latitude and longitude are required' }`), reproduce that exact body in
 the controller rather than relying on the generic `ZodValidationPipe` envelope.
 
+## Coverage is gated per domain, as a ratchet
+
+`server/vitest.config.ts` carries one threshold entry per `src/nest/<domain>/`, set at
+that domain's measured coverage minus one point. The single repo-wide 80% it replaced
+let a well-covered domain subsidise a thin one — `booking-import` sits at 50% and
+`integrations` at 20%, and the aggregate still cleared the bar, so either could have
+lost another ten points with the build staying green.
+
+Regenerate the block with `node scripts/coverage-thresholds.mjs` after a run that
+RAISED coverage. Raise an entry when you improve a domain; never lower one to make a
+build pass. The script reads `coverage/coverage-summary.json` rather than the text
+reporter on purpose: the text reporter prints one row per DIRECTORY, so a domain with
+subdirectories reads several points higher there than it actually is.
+
 ## How to write the tests
 
 Every module ships two kinds of tests. The coverage gate (`vitest.config.ts`)

server/src/nest/backup/backup.controller.ts

@@ -25,7 +25,7 @@ import { AdminGuard } from '../auth/admin.guard';
 import { CurrentUser } from '../auth/current-user.decorator';
 import { getClientIp } from '../audit/client-ip';
 import { AuditService } from '../audit/audit.service';
-import { getUploadTmpDir, MAX_BACKUP_UPLOAD_SIZE } from '../../services/backupService';
+import { getUploadTmpDir, MAX_BACKUP_UPLOAD_SIZE } from './backup.impl';
 
 const UPLOAD = {
   dest: getUploadTmpDir(),

server/src/nest/backup/backup.impl.ts

@@ -1,23 +1,23 @@
 import archiver from 'archiver';
 import unzipper from 'unzipper';
 import path from 'path';
-import { readEnv } from '../app-config';
+import { readEnv } from '../../app-config';
 import fs from 'fs';
 import Database from 'better-sqlite3';
-import { db, closeDb, reinitialize } from '../db/database';
-import * as scheduler from '../scheduler';
-import { invalidatePermissionsCache } from '../nest/permissions/permissions.bridge';
-import { pluginsCodeRoot, pluginsDataRoot } from '../nest/plugins/paths';
-import { stageExtractedPluginTrees, applyStagedRestoreNow } from '../nest/plugins/plugin-backup';
-import { snapshotAllPluginDataDbs } from '../nest/plugins/host/plugin-data.service';
+import { db, closeDb, reinitialize } from '../../db/database';
+import * as scheduler from '../../scheduler';
+import { invalidatePermissionsCache } from '../permissions/permissions.bridge';
+import { pluginsCodeRoot, pluginsDataRoot } from '../plugins/paths';
+import { stageExtractedPluginTrees, applyStagedRestoreNow } from '../plugins/plugin-backup';
+import { snapshotAllPluginDataDbs } from '../plugins/host/plugin-data.service';
 
 // ---------------------------------------------------------------------------
 // Paths
 // ---------------------------------------------------------------------------
 
-const dataDir = path.join(__dirname, '../../data');
+const dataDir = path.join(__dirname, '../../../data');
 const backupsDir = path.join(dataDir, 'backups');
-const uploadsDir = path.join(__dirname, '../../uploads');
+const uploadsDir = path.join(__dirname, '../../../uploads');
 
 // Compressed upload cap for restore archives. Defaults to 500 MB, raisable via
 // BACKUP_UPLOAD_LIMIT_MB for instances whose backups (uploads/ included) grow

server/src/nest/backup/backup.service.ts

@@ -1,10 +1,15 @@
 import { Injectable } from '@nestjs/common';
-import * as svc from '../../services/backupService';
+import * as svc from './backup.impl';
 
 /**
- * Thin Nest wrapper around the existing backup service. The zip packing/restore,
- * the auto-backup scheduler settings, the filename validation, the rate-limit
- * bookkeeping and the tmp-dir all reuse the legacy code unchanged.
+ * The backup domain's injectable face.
+ *
+ * The implementation moved here from src/services/backupService.ts with the code
+ * unchanged, and stays a module rather than becoming methods on this class: the zip
+ * packing and the restore path close and reinitialize the core DB handle, which is the
+ * single most dangerous sequence in the server. Rewriting its shape and moving it in
+ * one step would make a regression there impossible to bisect. The DB-lifecycle
+ * question is deliberately the LAST step of this fold, not part of the move.
  */
 @Injectable()
 export class BackupService {

server/src/scheduler.ts

@@ -62,7 +62,7 @@ function saveSettings(settings: BackupSettings): void {
  *
  * The shapes are structural on purpose: the scheduler names the capability it
  * wants, never the provider class. Importing BackupService here would drag in
- * src/nest/backup → src/services/backupService → src/scheduler and put the
+ * src/nest/backup → src/nest/backup/backup.impl → src/scheduler and put the
  * cycle straight back — the same cycle the lazy `await import()` in runBackup
  * existed to dodge.
  *

server/tests/e2e/backup.e2e.test.ts

@@ -39,7 +39,7 @@ const { backupSvc } = vi.hoisted(() => ({
     MAX_BACKUP_UPLOAD_SIZE: 1024,
   },
 }));
-vi.mock('../../src/services/backupService', () => backupSvc);
+vi.mock('../../src/nest/backup/backup.impl', () => backupSvc);
 
 import { BackupModule } from '../../src/nest/backup/backup.module';
 import { DatabaseModule } from '../../src/nest/database/database.module';

server/tests/integration/backup.test.ts

@@ -48,8 +48,8 @@ vi.mock('../../src/config', () => ({
 vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.fn() }));
 
 // Mock filesystem-dependent service functions to avoid real disk I/O in tests
-vi.mock('../../src/services/backupService', async () => {
-  const actual = await vi.importActual<typeof import('../../src/services/backupService')>('../../src/services/backupService');
+vi.mock('../../src/nest/backup/backup.impl', async () => {
+  const actual = await vi.importActual<typeof import('../../src/nest/backup/backup.impl')>('../../src/nest/backup/backup.impl');
   return {
     ...actual,
     createBackup: vi.fn().mockResolvedValue({
@@ -81,7 +81,7 @@ import { runMigrations } from '../../src/db/migrations';
 import { resetTestDb, resetRateLimits } from '../helpers/test-db';
 import { createAdmin, createUser } from '../helpers/factories';
 import { authCookie } from '../helpers/auth';
-import * as backupService from '../../src/services/backupService';
+import * as backupService from '../../src/nest/backup/backup.impl';
 import fs from 'fs';
 import path from 'path';
 import os from 'os';

server/tests/unit/auto-backup.test.ts

@@ -63,7 +63,7 @@ vi.mock('../../src/config', () => ({
 
 import path from 'node:path';
 import { start, setSchedulerDeps } from '../../src/scheduler';
-import { createBackup } from '../../src/services/backupService';
+import { createBackup } from '../../src/nest/backup/backup.impl';
 
 const liveDb = path.join(__dirname, '../../data', 'travel.db');
 

server/tests/unit/nest/backup.controller.test.ts

@@ -7,7 +7,7 @@ vi.mock('../../../src/nest/audit/audit-log.logger', () => ({ LOG_LEVEL: 'error',
 // The controller imports the tmp-dir + size cap at module load. The thin
 // BackupService wrapper forwards every call straight into this module, so the
 // mock also stubs the delegated functions for the wrapper tests below.
-vi.mock('../../../src/services/backupService', () => ({
+vi.mock('../../../src/nest/backup/backup.impl', () => ({
   getUploadTmpDir: () => '/tmp',
   MAX_BACKUP_UPLOAD_SIZE: 1024,
   BACKUP_RATE_WINDOW: 3600000,
@@ -28,7 +28,7 @@ import { BackupService as RealBackupService } from '../../../src/nest/backup/bac
 import { AdminGuard } from '../../../src/nest/auth/admin.guard';
 import type { BackupService } from '../../../src/nest/backup/backup.service';
 import type { AuditService } from '../../../src/nest/audit/audit.service';
-import * as backupSvc from '../../../src/services/backupService';
+import * as backupSvc from '../../../src/nest/backup/backup.impl';
 import type { User } from '../../../src/types';
 
 const user = { id: 1, role: 'admin', email: 'a@example.test' } as User;

server/tests/unit/nest/backup.impl.test.ts

@@ -92,7 +92,7 @@ import {
   backupFileExists,
   listBackups,
   updateAutoSettings,
-} from '../../../src/services/backupService';
+} from '../../../src/nest/backup/backup.impl';
 
 // ---------------------------------------------------------------------------
 // formatSize
@@ -715,6 +715,167 @@ describe('BACKUP-038 restoreFromZip', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// restoreFromZip — the per-entry extraction loop
+//
+// This is the part of the restore that decides what an attacker-supplied archive is
+// allowed to write, and it had no tests of its own: the zip-slip refusal, the running
+// decompressed-byte cap (the declared size in the central directory is
+// attacker-controlled, so the real guard counts bytes as they land) and the failure
+// path that leaves the process without a reopened DB.
+// ---------------------------------------------------------------------------
+
+/** A minimal unzipper entry: emits `chunks` when its stream is piped. */
+function zipEntry(entryPath: string, chunks: Buffer[] = [Buffer.alloc(8)], type = 'File') {
+  return {
+    path: entryPath,
+    type,
+    uncompressedSize: chunks.reduce((n, c) => n + c.length, 0),
+    stream() {
+      const handlers: Record<string, Array<(arg?: unknown) => void>> = {};
+      return {
+        on(event: string, cb: (arg?: unknown) => void) {
+          (handlers[event] ??= []).push(cb);
+          return this;
+        },
+        destroy: vi.fn(),
+        // The production code pipes AFTER registering handlers, so emitting here is
+        // the moment every listener is in place.
+        pipe(out: { emit(event: string): void }) {
+          for (const chunk of chunks) for (const cb of handlers.data ?? []) cb(chunk);
+          out.emit('finish');
+        },
+      };
+    },
+  };
+}
+
+/** A write stream that only has to carry 'finish' back to the awaiting promise. */
+function fakeWriteStream() {
+  const handlers: Record<string, Array<() => void>> = {};
+  return {
+    on(event: string, cb: () => void) {
+      (handlers[event] ??= []).push(cb);
+      return this;
+    },
+    destroy: vi.fn(),
+    emit(event: string) {
+      for (const cb of handlers[event] ?? []) cb();
+    },
+  };
+}
+
+describe('BACKUP-061 restoreFromZip extraction', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    fsMock.createWriteStream.mockImplementation(() => fakeWriteStream());
+    fsMock.existsSync.mockReturnValue(true);
+    fsMock.statSync.mockReturnValue({ size: 10 });
+  });
+
+  it('BACKUP-061a — refuses an entry whose path escapes the archive root (zip-slip)', async () => {
+    unzipperMock.Open.file.mockResolvedValueOnce({ files: [zipEntry('../../etc/passwd')] });
+
+    const result = await restoreFromZip('/data/tmp/slip.zip');
+
+    expect(result.success).toBe(false);
+    expect(result.status).toBe(400);
+    expect(result.error).toMatch(/escapes the archive root/i);
+    // Nothing of that archive is left behind, and no byte of it was written.
+    expect(fsMock.rmSync).toHaveBeenCalledWith(expect.stringContaining('restore-'), { recursive: true, force: true });
+    expect(fsMock.createWriteStream).not.toHaveBeenCalled();
+  });
+
+  it('BACKUP-061b — an absolute entry path is refused the same way', async () => {
+    unzipperMock.Open.file.mockResolvedValueOnce({ files: [zipEntry('C:/Windows/system32/evil.dll')] });
+
+    const result = await restoreFromZip('/data/tmp/abs.zip');
+
+    expect(result.success).toBe(false);
+    expect(result.status).toBe(400);
+    expect(fsMock.createWriteStream).not.toHaveBeenCalled();
+  });
+
+  it('BACKUP-061c — directory entries are skipped rather than written', async () => {
+    unzipperMock.Open.file.mockResolvedValueOnce({
+      files: [zipEntry('uploads/', [], 'Directory'), zipEntry('travel.db')],
+    });
+
+    // What happens after extraction is BACKUP-042..045's business; this case only
+    // cares that the directory entry never reached the writer.
+    await restoreFromZip('/data/tmp/dirs.zip').catch(() => undefined);
+
+    expect(fsMock.createWriteStream).toHaveBeenCalledTimes(1);
+  });
+
+  it('BACKUP-061d — stops mid-stream once the ACTUAL decompressed bytes cross the cap', async () => {
+    // The declared size is a lie: the central directory claims 8 bytes while the
+    // stream delivers well past the 5 GB cap. Only the running total catches this.
+    const lying = zipEntry('travel.db', [Buffer.alloc(3 * 1024 * 1024 * 1024), Buffer.alloc(3 * 1024 * 1024 * 1024)]);
+    lying.uncompressedSize = 8;
+    unzipperMock.Open.file.mockResolvedValueOnce({ files: [lying] });
+
+    const result = await restoreFromZip('/data/tmp/liar.zip');
+
+    expect(result.success).toBe(false);
+    expect(result.status).toBe(400);
+    expect(result.error).toMatch(/decompressed size/i);
+    expect(fsMock.rmSync).toHaveBeenCalledWith(expect.stringContaining('restore-'), { recursive: true, force: true });
+  });
+
+  it('BACKUP-061e — a stream error is not swallowed as a size refusal', async () => {
+    const entry = zipEntry('travel.db');
+    entry.stream = () => {
+      const handlers: Record<string, Array<(arg?: unknown) => void>> = {};
+      return {
+        on(event: string, cb: (arg?: unknown) => void) {
+          (handlers[event] ??= []).push(cb);
+          return this;
+        },
+        destroy: vi.fn(),
+        pipe() {
+          for (const cb of handlers.error ?? []) cb(new Error('corrupt deflate stream'));
+        },
+      };
+    };
+    unzipperMock.Open.file.mockResolvedValueOnce({ files: [entry] });
+
+    // A corrupt stream is NOT dressed up as a 400 "too large": it leaves the function
+    // as a throw, which is what makes the controller answer 500 rather than telling the
+    // admin their perfectly-sized backup is over the cap.
+    await expect(restoreFromZip('/data/tmp/corrupt.zip')).rejects.toThrow('corrupt deflate stream');
+    expect(fsMock.rmSync).toHaveBeenCalledWith(expect.stringContaining('restore-'), { recursive: true, force: true });
+  });
+
+  it('BACKUP-061f — a reopen failure after the swap reports "restart required", not success', async () => {
+    unzipperMock.Open.file.mockResolvedValueOnce({ files: [zipEntry('travel.db')] });
+    const restored = {
+      prepare: vi
+        .fn()
+        .mockReturnValueOnce({ get: vi.fn().mockReturnValue({ integrity_check: 'ok' }) })
+        .mockReturnValueOnce({
+          all: vi.fn().mockReturnValue([{ name: 'users' }, { name: 'trips' }, { name: 'trip_members' }, { name: 'places' }, { name: 'days' }]),
+        }),
+      close: vi.fn(),
+    };
+    DatabaseMock.mockImplementation(function () {
+      return restored;
+    });
+    fsMock.existsSync.mockImplementation((path: string) => !String(path).includes('uploads'));
+    dbMock.reinitialize.mockImplementationOnce(() => {
+      throw new Error('database is locked');
+    });
+
+    const result = await restoreFromZip('/data/tmp/ok.zip');
+
+    // The files already landed, so this is neither a success nor a plain failure: the
+    // admin has to restart, and the message has to say so.
+    expect(result.success).toBe(false);
+    expect(result.status).toBe(500);
+    expect(result.error).toMatch(/restart the server/i);
+  });
+});
+
 // ---------------------------------------------------------------------------
 // better-sqlite3 mock — hoisted by Vitest regardless of file position
 // ---------------------------------------------------------------------------

server/vitest.config.ts

@@ -64,7 +64,7 @@ export default defineConfig({
         'src/nest/atlas/**/*.ts': { statements: 91, branches: 78, functions: 96, lines: 93 },
         'src/nest/audit/**/*.ts': { statements: 92, branches: 79, functions: 99, lines: 96 },
         'src/nest/auth/**/*.ts': { statements: 93, branches: 84, functions: 95, lines: 96 },
-        'src/nest/backup/**/*.ts': { statements: 97, branches: 91, functions: 99, lines: 97 },
+        'src/nest/backup/**/*.ts': { statements: 96, branches: 92, functions: 99, lines: 98 },
         'src/nest/booking-import/**/*.ts': { statements: 49, branches: 28, functions: 58, lines: 52 },
         'src/nest/budget/**/*.ts': { statements: 91, branches: 77, functions: 97, lines: 95 },
         'src/nest/categories/**/*.ts': { statements: 99, branches: 80, functions: 99, lines: 99 },
@@ -83,7 +83,7 @@ export default defineConfig({
         'src/nest/llm-parse/**/*.ts': { statements: 91, branches: 85, functions: 85, lines: 94 },
         'src/nest/maps/**/*.ts': { statements: 92, branches: 85, functions: 95, lines: 95 },
         'src/nest/memories/**/*.ts': { statements: 88, branches: 80, functions: 95, lines: 90 },
-        'src/nest/notifications/**/*.ts': { statements: 83, branches: 71, functions: 84, lines: 86 },
+        'src/nest/notifications/**/*.ts': { statements: 83, branches: 72, functions: 85, lines: 86 },
         'src/nest/oauth/**/*.ts': { statements: 96, branches: 95, functions: 97, lines: 97 },
         'src/nest/oidc/**/*.ts': { statements: 88, branches: 83, functions: 91, lines: 92 },
         'src/nest/packing/**/*.ts': { statements: 92, branches: 83, functions: 98, lines: 97 },