Migrate TREK 3 to NestJS + React 19 (shared Zod contracts) (#1087)

* Migrate TREK 3 to NestJS + React 19 with a shared Zod contract layer Brownfield strangler migration of the backend onto NestJS modules (auth, trips, days, places, assignments, packing, todo, budget, reservations, collab, files, photos, journey, share, settings, backup, oidc, oauth, admin, atlas, vacay, weather, airports, maps, categories, tags, notifications, system-notices) served through a per-prefix dispatcher, keeping the existing SQLite/better-sqlite3 DB and JWT httpOnly cookie auth, with behavioural parity for every route. Client: React 19 upgrade, "page = wiring container + data hook" pattern across all pages, per-domain Zustand stores bound to @trek/shared contracts, and decomposition of the large components (DayPlanSidebar, PackingListPanel, CollabNotes, FileManager, MemoriesPanel, PlacesSidebar, CollabChat, SystemNoticeModal, BudgetPanel, PlaceFormModal, ...) into focused render units backed by in-file hooks. Apply the shared global request pipeline (helmet/CSP, CORS, HSTS, forced HTTPS, the global MFA policy and request logging) to the NestJS instance as well, so a migrated route is protected identically to the legacy fallback rather than bypassing it. * Finish the NestJS migration — drop the legacy Express app NestJS now serves the whole surface: every /api domain plus the platform routes (uploads, /mcp, the OAuth/MCP SDK + /.well-known metadata and the production SPA fallback). Removed server/src/app.ts, all of server/src/routes/* and the strangler dispatcher; index.ts and the integration suite share a single buildApp() bootstrap so prod and tests can't drift. - Platform/transport routes extracted to nest/platform/platform.routes.ts and mounted before app.init() — Nest's router answers an unmatched request with a 404, so a route registered after init is never reached. The SPA fallback is a NotFoundException filter and the catch-all uses a RegExp (Express 5's path-to-regexp rejects a bare '*'). - New modules: memories (/api/integrations/memories — the Journey gallery's Immich/Synology proxy), addons (GET /api/addons) and the cross-trip GET /api/reservations/upcoming. - TrekExceptionFilter reproduces the old multer / err.statusCode handling so upload rejections keep their 400/413 { error } body and non-ASCII filenames survive (defParamCharset). - addTripToJourney and the MCP get_journey_share_link tool gained the trip-access check they were missing. - Re-pointed the 34 integration tests + the websocket test onto the Nest app; removed the now-meaningless Express-vs-Nest parity tests and a few orphaned client components. * Restore the reset-password rate limit and fix copyTrip reservation links Two correctness/security gaps the NestJS migration introduced: - POST /api/auth/reset-password lost its per-IP rate limiter. Restore it (5 attempts / 15 min on a dedicated bucket, same as the old resetLimiter) so reset tokens can't be brute-forced unthrottled. Covered by AUTH-019. - copyTripById did not copy reservations.end_day_id (a day reference — now remapped through dayMap like day_id) or needs_review, so a duplicated trip lost multi-day transport end-day links and reset the review flag. * Clean up dead code, dedupe helpers, fix the reset-password contract - Remove server exports orphaned by the Express removal: the immich album-link helpers, seven route-only service exports, getFileByIdFull; de-export internal-only helpers (utcSuffix). - De-duplicate verifyTripAccess (9 identical copies -> services/tripAccess.ts) and avatarUrl (3 -> services/avatarUrl.ts); name the bcrypt cost (BCRYPT_COST) and the email regex (EMAIL_REGEX). Public API unchanged. - resetPasswordRequestSchema declared `password`, but the client sends and the service reads `new_password` — rename it so the contract matches and the client types resolve. - Make ATLAS-013 deterministic: stub the admin-1 GeoJSON download instead of fetching ~4600 features from GitHub during the test (it hung the suite). * Make the client typecheck runnable (vitest/vite ambient types) The client had no `typecheck` script and tsc couldn't even start (the baseUrl deprecation errored out, same as server/shared already silence). Add `ignoreDeprecations: "6.0"` to match the other workspaces, a `typecheck` npm script, and a src/vite-env.d.ts referencing vite/client + vitest/globals so tsc knows the test globals (describe/it/expect/vi). This turns ~3600 phantom "Cannot find name" errors into a real, measurable count (~590 actual type errors remain, to be worked down). Type-only; no runtime change. * Derive client domain types from the shared schema contracts Add entity/response Zod schemas to @trek/shared (place, trip, assignment, day, budget, packing, reservation), each matched against the producing server service, and re-export them from client types.ts instead of the hand-written duplicates that had drifted (name/title, amount/total_price, owner_id/user_id, cover_url/cover_image, ...). Updates the call sites and test fixtures the corrected types surfaced; type-only, no runtime behaviour change. * chore(db): log swallowed errors in addon-disable migration + guard against destructive migrations The migration that disables the legacy "memories" addon swallowed any error in an empty catch, as did ~30 other catch blocks in the migration runner (column adds, the journey rebuild, index probes). Replace each silent catch with the existing console.warn('[migrations] ...') log so failures are visible. Control flow is unchanged: every step stays non-fatal, nothing new is thrown. Add a static guardrail test that scans the migration source and fails when a new destructive statement (DROP TABLE / DROP COLUMN / TRUNCATE / DELETE FROM / ALTER ... DROP) appears outside a reviewed allowlist, and when an empty/silent catch block is reintroduced. The existing destructive statements are all legitimate table rebuilds or bounded cleanups and are recorded in the allowlist with a reason. * Re-check SSRF on every redirect hop when resolving short links Replace the one-shot checkSsrf + fetch(redirect:'follow') in the maps and place short-link resolvers with safeFetchFollow, which follows redirects manually and re-runs checkSsrf against the DNS-pinned IP of each hop (max 5). A redirect to an internal/loopback address is now blocked even when the initial URL is public, while legitimate cross-host redirects (goo.gl -> maps.google.com) still resolve. * Reject WebSocket tokens minted before a password change Stamp the user's password_version onto the ephemeral ws token and verify it on connect, closing the socket (4001) when it no longer matches, so a token issued before a password reset can't be replayed. Tokens minted without a version are treated as version 0, matching the JWT pv-claim semantics. * fix(i18n): guard locale key parity and finish the OAuth consent page strings Every non-en locale now exposes the exact same flat key set as en. Keys that had drifted out of sync are backfilled with the English source value (tagged en-fallback) so t() resolves a real string instead of relying on the silent runtime fallback; no existing translation was touched and no key was removed. Add a parity test that imports each aggregated locale bundle and asserts its key set matches en, with a diagnostic listing of any missing/extra keys. This complements the file-level check in shared/scripts by guarding the merged export the app actually serves. Finish internationalising OAuthAuthorizePage: the ~15 remaining hardcoded English chrome strings now go through oauth.authorize.* keys (English source in en, en-fallback placeholders elsewhere). Markup and behaviour are unchanged. * Add semantic theme color tokens to Tailwind Map the CSS theme variables from src/index.css (:root light / .dark dark) to named Tailwind utilities — bg-surface, text-content, border-edge, bg-accent and their variants. This gives components a Tailwind-native target for the theme colors so we can replace inline `style={{ ... 'var(--...)' }}` with utility classes without changing the rendered values. * Surface silent store failures to the user and validate API responses in dev Reservation toggle, todo/packing toggle and budget reorder were swallowing API errors after rolling back, so the user saw the change silently snap back with no explanation. Route those failures through the existing toast channel (new store/notify.ts bridges to window.__addToast, the same channel SystemNoticeBanner uses); the reservation toggle re-throws so ReservationsPanel's own translated toast finally fires. Also wire the existing parseInDev/checkInDev response validation into the maps and notification-test endpoints to catch contract drift in dev. * Migrate static theme inline styles to Tailwind utilities and extract page sub-components Replace the static, color-only inline `style={{ ... 'var(--bg-primary)' ... }}` props with the new semantic Tailwind utilities (bg-surface, text-content, border-edge, ...) wherever the result is byte-identical; dynamic/conditional theme styles and hardcoded status colors are left inline. Extract the Atlas country-search autocomplete, the Admin update banner, and two Journey dialogs into their own presentational components to shrink the oversized page files, keeping behaviour and markup identical. * Remove the unrouted photos page and its dead photo components PhotosPage was never wired into the router and its usePhotos hook read a tripStore photos slice that was never implemented; the Photos gallery, lightbox and upload components were only reachable through it. Per-trip photos now live in the Journey gallery (Immich/Synology). Removed the dead page, hook and components — the live Journey PhotoLightbox is a separate component and stays. * Resolve the remaining client type errors and the trip.title navbar bug Drive the client typecheck to zero without any/ts-ignore: convert the tripId route param to a number once at the page boundary so it matches the numeric props and store actions it feeds, fix trip.name -> trip.title (the wire field is title, so the old read rendered blank in the files/offline views), and tighten the scattered handler-arity, DOM-cast and untyped-payload sites. No runtime behaviour change. * Convert the remaining dynamic and hardcoded inline styles to Tailwind utilities Second styling pass over the components and pages: move conditional theme colors into className ternaries (bg-accent / bg-surface-hover etc.), turn reused CSSProperties constants into className constants, and express static hardcoded hex/rgba colors as Tailwind arbitrary values so the exact rendered colour is preserved. Truly dynamic styling (computed geometry, gradients, multi-part shadows, data-driven colours, the undefined --sidebar/--nav layout vars) stays inline as it cannot be expressed as a static class. Updated three component tests that asserted the old inline active-state styles to assert the equivalent utility class instead. Verified: client typecheck 0, full client suite green, and a live light/dark render check in the dev server confirms the semantic theme tokens resolve correctly (the earlier 'transparent popups' were a stale dev server that pre-dated the tailwind.config token addition, not a code issue). * Add eslint flat-config for client and server and gate typecheck, lint and pages in CI client and server had lint scripts but no eslint config (only shared was linted in CI). Add flat configs mirroring shared's stack (js + typescript-eslint recommended + eslint-config-prettier) plus the client's react-hooks/react-refresh plugins. Pre-existing patterns in this never-linted code (explicit any, require() in the CommonJS server, empty catches, exhaustive-deps) are set to 'warn' rather than 'error' so the gate passes at 0 errors without a repo-wide reformat — these can be ratcheted to errors over time. Wire blocking typecheck + lint + lint:pages steps into the client and server CI jobs (now that both typechecks are clean) and promote the server typecheck from informational to blocking. * Decompose the remaining God Components into hooks, helpers and sub-components FE6: split the oversized page and panel components into thin layout shells plus colocated use<Component> hooks, .constants.ts, .helpers.ts (with tests) and presentational sub-components, following the established 'logic in a hook, render in slices' pattern. Behaviour, markup, classes and effect order are unchanged. Largest reductions: PackingListPanel 1598->42, FileManager 1055->36, AdminPage 1525->167, BudgetPanel 1266->146, JourneyDetailPage 2822->547, PlacesSidebar 945->66, CollabChat 861->106, CollabNotes 1417->532. DayPlanSidebar's drag-and-drop render body was left intact (ref-identity sensitive) and only its toolbar/modals/constants were extracted. * Fix duplicate React keys in the file-assign place list When a place is assigned to the same day more than once it appeared twice in a day's list, so the place-button key={p.id} collided and React warned about duplicate keys. Key by place id + render index so siblings stay unique. Pre-existing in the old FileManager; behaviour unchanged. * Format the shared package and drop an unused import to satisfy the lint gate The i18n and schema changes added code that wasn't prettier-formatted, and place.schema.ts imported categorySchema without using it. Run prettier over shared and remove the import so 'npm run lint' + 'format:check' pass. * Install all workspaces in the server CI job so SWC's native binary is present The server vitest config transforms via unplugin-swc, which needs @swc/core's platform-specific native binary. A workspace-scoped 'npm ci --workspace server' skips that optional dependency, so vitest failed to load the config on the Linux runner. Use a full 'npm ci'. * Re-resolve dependencies with npm install in the server CI job for SWC Full 'npm ci' still skipped @swc/core's Linux native binary because the committed lockfile was generated on Windows and lacks the Linux optional-dep install metadata. 'npm install' re-resolves and fetches the platform-matching binary, which the server's unplugin-swc transform needs to load vitest.config.ts. * Install @swc/core's Linux binary explicitly in the server CI job Neither npm ci nor npm install fetched @swc/core-linux-x64-gnu on the Linux runner because the lockfile was generated on Windows and lacks the Linux optional-dep metadata. Add a step that installs the matching @swc/core-linux-x64-gnu version (no-save, no-lockfile) so unplugin-swc can load the server's vitest config. * Use legacy-peer-deps when installing the SWC Linux binary in CI The explicit @swc/core-linux-x64-gnu install re-resolved the tree and hit the pre-existing lucide-react/react-19 peer conflict that the lockfile was generated around. Add --legacy-peer-deps so the step matches the project's resolution and installs the binary. * Keep the lockfile when installing the SWC binary so other deps stay pinned Dropping --no-package-lock made npm re-resolve the whole tree and upgrade eslint, whose newer recommended config flagged no-useless-assignment as an error in the server lint step. Keep the lockfile so only @swc/core-linux-x64-gnu is added and every other dependency (incl. eslint) stays at its locked version.
2026-06-21 06:11:45 +00:00 · 2026-05-31 21:10:00 +02:00
parent 6d2dd37414
commit 20791a29a7
721 changed files with 44416 additions and 31919 deletions
@@ -16,7 +16,10 @@ import { resolveAuthToggles } from './authService';

 // ── Helpers ────────────────────────────────────────────────────────────────

-export function utcSuffix(ts: string | null | undefined): string | null {
+// bcrypt cost factor for user passwords — kept in sync with authService.
+const BCRYPT_COST = 12;
+
+function utcSuffix(ts: string | null | undefined): string | null {
  if (!ts) return null;
  return ts.endsWith('Z') ? ts : ts.replace(' ', 'T') + 'Z';
 }
@@ -94,7 +97,7 @@ export function createUser(data: { username: string; email: string; password: st
  const existingEmail = db.prepare('SELECT id FROM users WHERE email = ?').get(email);
  if (existingEmail) return { error: 'Email already taken', status: 409 };

-  const passwordHash = bcrypt.hashSync(password, 12);
+  const passwordHash = bcrypt.hashSync(password, BCRYPT_COST);

  const result = db.prepare(
    'INSERT INTO users (username, email, password_hash, role) VALUES (?, ?, ?, ?)'
@@ -136,7 +139,7 @@ export function updateUser(id: string, data: { username?: string; email?: string
    const pwCheck = validatePassword(password);
    if (!pwCheck.ok) return { error: pwCheck.reason, status: 400 };
  }
-  const passwordHash = password ? bcrypt.hashSync(password, 12) : null;
+  const passwordHash = password ? bcrypt.hashSync(password, BCRYPT_COST) : null;

  db.prepare(`
    UPDATE users SET
@@ -12,7 +12,7 @@ async function loadAdmin1Geo(): Promise<any> {
  admin1GeoLoading = fetch(
    'https://raw.githubusercontent.com/nvkelso/natural-earth-vector/master/geojson/ne_10m_admin_1_states_provinces.geojson',
    { headers: { 'User-Agent': 'TREK Travel Planner' } }
-  ).then(r => r.json()).then(geo => {
+  ).then(r => r.json()).then((geo: any) => {
    admin1GeoCache = geo;
    admin1GeoLoading = null;
    console.log(`[Atlas] Cached admin-1 GeoJSON: ${geo.features?.length || 0} features`);
@@ -20,6 +20,9 @@ import { getFlightDistanceKm } from './distanceService';
 import { verifyJwtAndLoadUser } from '../middleware/auth';
 import { User } from '../types';
 import { DEMO_EMAIL_PRIMARY, isDemoEmail } from './demo';
+import { avatarUrl } from './avatarUrl';
+
+export { avatarUrl };

 // ---------------------------------------------------------------------------
 // Constants
@@ -27,10 +30,16 @@ import { DEMO_EMAIL_PRIMARY, isDemoEmail } from './demo';

 authenticator.options = { window: 1 };

+// bcrypt cost factor for user passwords. Shared by register/changePassword/
+// resetPassword and the dummy-hash timing equaliser below — must stay in sync.
+const BCRYPT_COST = 12;
+
+// Shape check for email input on register and profile update.
+const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+
 // Pre-computed bcrypt hash to equalise timing of "unknown email" and
 // "OIDC-only account" branches with the real verification path (CWE-208).
-// Cost factor 12 matches register/changePassword/resetPassword — must stay in sync.
-const DUMMY_PASSWORD_HASH = bcrypt.hashSync('__trek_no_such_user__', 12);
+const DUMMY_PASSWORD_HASH = bcrypt.hashSync('__trek_no_such_user__', BCRYPT_COST);

 const MFA_SETUP_TTL_MS = 15 * 60 * 1000;
 const mfaSetupPending = new Map<number, { secret: string; exp: number }>();
@@ -114,10 +123,6 @@ export function mask_stored_api_key(key: string | null | undefined): string | nu
  return maskKey(plain);
 }

-export function avatarUrl(user: { avatar?: string | null }): string | null {
-  return user.avatar ? `/uploads/avatars/${user.avatar}` : null;
-}
-
 export function resolveAuthToggles(): {
  password_login: boolean;
  password_registration: boolean;
@@ -377,8 +382,7 @@ export function registerUser(body: {
  const pwCheck = validatePassword(password);
  if (!pwCheck.ok) return { error: pwCheck.reason, status: 400 };

-  const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
-  if (!emailRegex.test(email)) {
+  if (!EMAIL_REGEX.test(email)) {
    return { error: 'Invalid email format', status: 400 };
  }

@@ -387,7 +391,7 @@ export function registerUser(body: {
    return { error: 'Registration failed. Please try different credentials.', status: 409 };
  }

-  const password_hash = bcrypt.hashSync(password, 12);
+  const password_hash = bcrypt.hashSync(password, BCRYPT_COST);
  const isFirstUser = userCount === 0;
  const role = isFirstUser ? 'admin' : 'user';

@@ -494,13 +498,15 @@ export function loginUser(body: {
 // Session
 // ---------------------------------------------------------------------------

-export function getCurrentUser(userId: number) {
+export function getCurrentUser(
+  userId: number
+): (Record<string, unknown> & Pick<User, 'id' | 'username' | 'email' | 'role'> & { avatar_url: string }) | null {
  const user = db.prepare(
    'SELECT id, username, email, role, avatar, oidc_issuer, created_at, mfa_enabled, must_change_password FROM users WHERE id = ?'
  ).get(userId) as User | undefined;
  if (!user) return null;
  const base = stripUserForClient(user as User) as Record<string, unknown>;
-  return { ...base, avatar_url: avatarUrl(user) };
+  return { ...base, id: user.id, username: user.username, email: user.email, role: user.role, avatar_url: avatarUrl(user) };
 }

 // ---------------------------------------------------------------------------
@@ -531,7 +537,7 @@ export function changePassword(
    return { error: 'Current password is incorrect', status: 401 };
  }

-  const hash = bcrypt.hashSync(new_password, 12);
+  const hash = bcrypt.hashSync(new_password, BCRYPT_COST);
  db.prepare('UPDATE users SET password_hash = ?, must_change_password = 0, updated_at = CURRENT_TIMESTAMP WHERE id = ?').run(hash, userId);
  return { success: true };
 }
@@ -606,8 +612,7 @@ export function updateSettings(

  if (email !== undefined) {
    const trimmed = email.trim();
-    const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
-    if (!trimmed || !emailRegex.test(trimmed)) {
+    if (!trimmed || !EMAIL_REGEX.test(trimmed)) {
      return { error: 'Invalid email format', status: 400 };
    }
    const conflict = db.prepare('SELECT id FROM users WHERE LOWER(email) = LOWER(?) AND id != ?').get(trimmed, userId);
@@ -1247,7 +1252,7 @@ export function resetPassword(body: {
    }
  }

-  const newHash = bcrypt.hashSync(new_password, 12);
+  const newHash = bcrypt.hashSync(new_password, BCRYPT_COST);
  const newPv = (user.password_version ?? 0) + 1;

  db.transaction(() => {
@@ -1330,7 +1335,10 @@ export function deleteMcpToken(userId: number, tokenId: string): { error?: strin
 // ---------------------------------------------------------------------------

 export function createWsToken(userId: number): { error?: string; status?: number; token?: string } {
-  const token = createEphemeralToken(userId, 'ws');
+  // Bind the ws-token to the user's current password_version so a token minted
+  // before a password reset is rejected on connect (defence-in-depth session gate).
+  const pv = (db.prepare('SELECT password_version FROM users WHERE id = ?').get(userId) as { password_version?: number } | undefined)?.password_version ?? 0;
+  const token = createEphemeralToken(userId, 'ws', { pv });
  if (!token) return { error: 'Service unavailable', status: 503 };
  return { token };
 }
@@ -0,0 +1,3 @@
+export function avatarUrl(user: { avatar?: string | null }): string | null {
+  return user.avatar ? `/uploads/avatars/${user.avatar}` : null;
+}
@@ -185,6 +185,7 @@ export interface RestoreResult {

 export async function restoreFromZip(zipPath: string): Promise<RestoreResult> {
  const extractDir = path.join(dataDir, `restore-${Date.now()}`);
+  let reinitFailed: unknown = null;
  try {
    await fs.createReadStream(zipPath)
      .pipe(unzipper.Extract({ path: extractDir }))
@@ -246,7 +247,16 @@ export async function restoreFromZip(zipPath: string): Promise<RestoreResult> {
        fs.cpSync(extractedUploads, uploadsDir, { recursive: true, force: true });
      }
    } finally {
-      reinitialize();
+      // Reopening the DB must always run (even if the copy above threw) so the
+      // process is never left without a connection. Capture a reopen failure
+      // instead of letting it propagate as a generic error — a backup whose
+      // files already landed on disk but whose connection failed to reopen
+      // needs to be reported as "restart required", not swallowed.
+      try {
+        reinitialize();
+      } catch (reinitErr) {
+        reinitFailed = reinitErr;
+      }
      // The restored DB has different permission-override rows from
      // the pre-restore DB, but our process-local permissions cache
      // still holds the pre-restore state. Any request using a cached
@@ -256,6 +266,10 @@ export async function restoreFromZip(zipPath: string): Promise<RestoreResult> {
    }

    fs.rmSync(extractDir, { recursive: true, force: true });
+    if (reinitFailed) {
+      console.error('Restore: database reopen failed after file swap:', reinitFailed);
+      return { success: false, error: 'Backup files were restored but the database connection could not be reopened. Restart the server to finish the restore.', status: 500 };
+    }
    return { success: true };
  } catch (err: unknown) {
    console.error('Restore error:', err);
@@ -1,17 +1,13 @@
-import { db, canAccessTrip } from '../db/database';
+import { db } from '../db/database';
 import { BudgetItem, BudgetItemMember } from '../types';
+import { avatarUrl } from './avatarUrl';

 // ---------------------------------------------------------------------------
 // Helpers
 // ---------------------------------------------------------------------------

-export function avatarUrl(user: { avatar?: string | null }): string | null {
-  return user.avatar ? `/uploads/avatars/${user.avatar}` : null;
-}
-
-export function verifyTripAccess(tripId: string | number, userId: number) {
-  return canAccessTrip(tripId, userId);
-}
+export { avatarUrl };
+export { verifyTripAccess } from './tripAccess';

 function loadItemMembers(itemId: number | string) {
  const rows = db.prepare(`
@@ -1,8 +1,9 @@
 import path from 'path';
 import fs from 'fs';
-import { db, canAccessTrip } from '../db/database';
+import { db } from '../db/database';
 import { CollabNote, CollabPoll, CollabMessage, TripFile } from '../types';
 import { checkSsrf, createPinnedDispatcher } from '../utils/ssrfGuard';
+import { avatarUrl } from './avatarUrl';

 /* ------------------------------------------------------------------ */
 /*  Internal row types                                                 */
@@ -48,13 +49,8 @@ export interface LinkPreviewResult {
 /*  Helpers                                                            */
 /* ------------------------------------------------------------------ */

-export function avatarUrl(user: { avatar?: string | null }): string | null {
-  return user.avatar ? `/uploads/avatars/${user.avatar}` : null;
-}
-
-export function verifyTripAccess(tripId: string | number, userId: number) {
-  return canAccessTrip(tripId, userId);
-}
+export { avatarUrl };
+export { verifyTripAccess } from './tripAccess';

 /* ------------------------------------------------------------------ */
 /*  Reactions                                                          */
@@ -1,9 +1,7 @@
-import { db, canAccessTrip } from '../db/database';
+import { db } from '../db/database';
 import { DayNote } from '../types';

-export function verifyTripAccess(tripId: string | number, userId: number) {
-  return canAccessTrip(tripId, userId);
-}
+export { verifyTripAccess } from './tripAccess';

 export function listNotes(dayId: string | number, tripId: string | number) {
  return db.prepare(
@@ -1,10 +1,8 @@
-import { db, canAccessTrip } from '../db/database';
+import { db } from '../db/database';
 import { loadTagsByPlaceIds, loadParticipantsByAssignmentIds, formatAssignmentWithPlace } from './queryHelpers';
 import { AssignmentRow, Day, DayNote } from '../types';

-export function verifyTripAccess(tripId: string | number, userId: number) {
-  return canAccessTrip(tripId, userId);
-}
+export { verifyTripAccess } from './tripAccess';

 // ---------------------------------------------------------------------------
 // Day assignment helpers
@@ -11,15 +11,31 @@ interface TokenEntry {
  userId: number;
  purpose: string;
  expiresAt: number;
+  /**
+   * Snapshot of the user's `password_version` at mint time, used for the
+   * defence-in-depth session gate on WebSocket connects. `undefined` for
+   * tokens minted without a version (legacy/other purposes), which callers
+   * treat as version 0 — mirroring the JWT `pv` claim semantics.
+   */
+  pv?: number;
+}
+
+export interface EphemeralTokenMeta {
+  /** Bind the token to the user's current password_version (session gate). */
+  pv?: number;
 }

 const store = new Map<string, TokenEntry>();

-export function createEphemeralToken(userId: number, purpose: string): string | null {
+export function createEphemeralToken(
+  userId: number,
+  purpose: string,
+  meta?: EphemeralTokenMeta,
+): string | null {
  if (store.size >= MAX_STORE_SIZE) return null;
  const token = crypto.randomBytes(32).toString('hex');
  const ttl = TTL[purpose] ?? 60_000;
-  store.set(token, { userId, purpose, expiresAt: Date.now() + ttl });
+  store.set(token, { userId, purpose, expiresAt: Date.now() + ttl, pv: meta?.pv });
  return token;
 }

@@ -31,6 +47,22 @@ export function consumeEphemeralToken(token: string, purpose: string): number |
  return entry.userId;
 }

+/**
+ * Like `consumeEphemeralToken`, but also returns the `password_version` the
+ * token was minted with. Used by the WebSocket handshake so a token issued
+ * before a password change can be rejected even within its short TTL.
+ */
+export function consumeEphemeralTokenWithMeta(
+  token: string,
+  purpose: string,
+): { userId: number; pv?: number } | null {
+  const entry = store.get(token);
+  if (!entry) return null;
+  store.delete(token);
+  if (entry.purpose !== purpose || Date.now() > entry.expiresAt) return null;
+  return { userId: entry.userId, pv: entry.pv };
+}
+
 let cleanupInterval: ReturnType<typeof setInterval> | null = null;

 export function startTokenCleanup(): void {
@@ -1,7 +1,7 @@
 import path from 'path';
 import fs from 'fs';
 import type { Request } from 'express';
-import { db, canAccessTrip } from '../db/database';
+import { db } from '../db/database';
 import { consumeEphemeralToken } from './ephemeralTokens';
 import { verifyJwtAndLoadUser } from '../middleware/auth';
 import { TripFile } from '../types';
@@ -30,9 +30,7 @@ export const filesDir = path.join(__dirname, '../../uploads/files');
 // Helpers
 // ---------------------------------------------------------------------------

-export function verifyTripAccess(tripId: string | number, userId: number) {
-  return canAccessTrip(tripId, userId);
-}
+export { verifyTripAccess } from './tripAccess';

 export function getAllowedExtensions(): string {
  try {
@@ -48,7 +46,7 @@ const FILE_SELECT = `
  LEFT JOIN users u ON f.uploaded_by = u.id
 `;

-export function formatFile(file: TripFile & { trip_id?: number }) {
+export function formatFile(file: TripFile & { trip_id?: number; uploaded_by_avatar?: string | null }) {
  const tripId = file.trip_id;
  return {
    ...file,
@@ -113,10 +111,6 @@ export function getFileById(id: string | number, tripId: string | number): TripF
  return db.prepare('SELECT * FROM trip_files WHERE id = ? AND trip_id = ?').get(id, tripId) as TripFile | undefined;
 }

-export function getFileByIdFull(id: string | number): TripFile {
-  return db.prepare(`${FILE_SELECT} WHERE f.id = ?`).get(id) as TripFile;
-}
-
 export function getDeletedFile(id: string | number, tripId: string | number): TripFile | undefined {
  return db.prepare('SELECT * FROM trip_files WHERE id = ? AND trip_id = ? AND deleted_at IS NOT NULL').get(id, tripId) as TripFile | undefined;
 }
@@ -1,4 +1,4 @@
-import { db } from '../db/database';
+import { db, canAccessTrip } from '../db/database';
 import { broadcastToUser } from '../websocket';
 import type { Journey, JourneyEntry, JourneyPhoto, JourneyContributor } from '../types';
 import { getOrCreateTrekPhoto, getOrCreateLocalTrekPhoto, setTrekPhotoProvider, deleteTrekPhotoIfOrphan } from './memories/photoResolverService';
@@ -254,6 +254,11 @@ export function deleteJourney(journeyId: number, userId: number): boolean {
 // ── Trip management ──────────────────────────────────────────────────────

 export function addTripToJourney(journeyId: number, tripId: number, userId: number): boolean {
+  // Only attach a trip the caller can actually access — otherwise a journey
+  // owner could pull an arbitrary trip's places + photos into their journey
+  // (cross-tenant leak). Mirrors the trip-access gate every other trip-scoped
+  // path enforces.
+  if (!canAccessTrip(tripId, userId)) return false;
  const now = ts();
  try {
    db.prepare(
@@ -501,6 +506,7 @@ export function createEntry(journeyId: number, userId: number, data: {
  tags?: string[];
  pros_cons?: { pros: string[]; cons: string[] };
  visibility?: string;
+  sort_order?: number;
 }, sid?: string): JourneyEntry | null {
  if (!canEdit(journeyId, userId)) return null;

@@ -1,15 +1,12 @@
 import { db } from '../db/database';
 import { decrypt_api_key } from './apiKeyCrypto';
-import { checkSsrf } from '../utils/ssrfGuard';
+import { safeFetchFollow, SsrfBlockedError } from '../utils/ssrfGuard';
 import { getAppUrl } from './notifications';

 // ── Google API call counter ───────────────────────────────────────────────────

 let googleApiCallCount = 0;

-export function getGoogleApiCallCount(): number { return googleApiCallCount; }
-export function resetGoogleApiCallCount(): void { googleApiCallCount = 0; }
-
 function googleFetch(endpoint: string, label: string, init?: RequestInit): Promise<Response> {
  googleApiCallCount++;
  console.debug(`[Google API] #${googleApiCallCount} ${label} → ${endpoint}`);
@@ -637,10 +634,10 @@ export async function getPlacePhoto(
        try {
          const wiki = await fetchWikimediaPhoto(lat, lng, name);
          if (wiki) {
-            // Wikimedia photos: fetch bytes and cache to disk
-            const ssrf = await checkSsrf(wiki.photoUrl, true);
-            if (!ssrf.allowed) throw Object.assign(new Error('Photo URL blocked'), { status: 403 });
-            const imgRes = await fetch(wiki.photoUrl);
+            // Wikimedia photos: fetch bytes and cache to disk. Follow redirects
+            // manually so each hop (the image URL can 3xx to a CDN host) is
+            // re-validated against the SSRF guard, not just the first URL.
+            const imgRes = await safeFetchFollow(wiki.photoUrl, undefined, { bypassInternalIpAllowed: true });
            if (imgRes.ok) {
              const bytes = Buffer.from(await imgRes.arrayBuffer());
              const cached = await placePhotoCache.put(placeId, bytes, wiki.attribution);
@@ -749,13 +746,25 @@ export async function reverseGeocode(lat: string, lng: string, lang?: string): P
 export async function resolveGoogleMapsUrl(url: string): Promise<{ lat: number; lng: number; name: string | null; address: string | null }> {
  let resolvedUrl = url;

-  // Follow redirects for short URLs (goo.gl, maps.app.goo.gl) with SSRF protection
+  // Follow redirects for short URLs (goo.gl, maps.app.goo.gl) with SSRF protection.
+  // Redirects are followed manually so every hop is re-checked — a short link
+  // that 302s to an internal IP is blocked, while a legitimate cross-host
+  // redirect (goo.gl → maps.google.com) still resolves.
  const parsed = new URL(url);
  if (['goo.gl', 'maps.app.goo.gl'].includes(parsed.hostname)) {
-    const ssrf = await checkSsrf(url, true);
-    if (!ssrf.allowed) throw Object.assign(new Error('URL blocked by SSRF check'), { status: 403 });
-    const redirectRes = await fetch(url, { redirect: 'follow', signal: AbortSignal.timeout(10000) });
-    resolvedUrl = redirectRes.url;
+    try {
+      const redirectRes = await safeFetchFollow(
+        url,
+        { signal: AbortSignal.timeout(10000) },
+        { bypassInternalIpAllowed: true },
+      );
+      resolvedUrl = redirectRes.url;
+    } catch (err) {
+      if (err instanceof SsrfBlockedError) {
+        throw Object.assign(new Error('URL blocked by SSRF check'), { status: 403 });
+      }
+      throw err;
+    }
  }

  // Extract coordinates from Google Maps URL patterns:
@@ -349,42 +349,6 @@ export async function getAlbumPhotos(
  }
 }

-export function listAlbumLinks(tripId: string) {
-  return db.prepare(`
-    SELECT tal.*, u.username
-    FROM trip_album_links tal
-    JOIN users u ON tal.user_id = u.id
-    WHERE tal.trip_id = ?
-    ORDER BY tal.created_at ASC
-  `).all(tripId);
-}
-
-export function createAlbumLink(
-  tripId: string,
-  userId: number,
-  albumId: string,
-  albumName: string
-): { success: boolean; error?: string } {
-  try {
-    db.prepare(
-      "INSERT OR IGNORE INTO trip_album_links (trip_id, user_id, album_id, album_name, provider) VALUES (?, ?, ?, ?, 'immich')"
-    ).run(tripId, userId, albumId, albumName || '');
-    return { success: true };
-  } catch {
-    return { success: false, error: 'Album already linked' };
-  }
-}
-
-export function deleteAlbumLink(linkId: string, tripId: string, userId: number) {
-  db.transaction(() => {
-    const link = db.prepare('SELECT id FROM trip_album_links WHERE id = ? AND trip_id = ? AND user_id = ?').get(linkId, tripId, userId);
-    if (link) {
-      db.prepare('DELETE FROM trip_photos WHERE trip_id = ? AND album_link_id = ?').run(tripId, linkId);
-      db.prepare('DELETE FROM trip_album_links WHERE id = ?').run(linkId);
-    }
-  })();
-}
-
 export async function syncAlbumAssets(
  tripId: string,
  linkId: string,
@@ -230,10 +230,3 @@ export function deleteTrekPhotoIfOrphan(photoId: number): void {
  db.prepare("DELETE FROM trek_photos WHERE id = ? AND provider != 'local'").run(photoId);
 }

-// ── Delete local file for a trek_photo ──────────────────────────────────
-
-export function getTrekPhotoFilePath(photoId: number): string | null {
-  const photo = resolveTrekPhoto(photoId);
-  if (!photo || photo.provider !== 'local' || !photo.file_path) return null;
-  return path.join(__dirname, '../../../uploads', photo.file_path);
-}
@@ -458,11 +458,12 @@ export async function listSynologyAlbums(userId: number): Promise<ServiceResult<

    const addAlbums = (result: PromiseSettledResult<ServiceResult<any[]>>, extractPassphrase: (a: any) => string | undefined) => {
        if (result.status === 'rejected') return;
-        if (!result.value.success) {
-            console.warn('[Synology] album list partial failure:', (result.value as any).error?.message);
+        const value = result.value;
+        if ('error' in value) {
+            console.warn('[Synology] album list partial failure:', value.error.message);
            return;
        }
-        for (const album of result.value.data ?? []) {
+        for (const album of value.data ?? []) {
            const id = String(album.id);
            const passphrase = extractPassphrase(album);
            map.set(id, { id, albumName: album.name || '', assetCount: album.item_count || 0, passphrase });
@@ -299,7 +299,7 @@ async function _notifySharedTripPhotos(
  actorUserId: number,
  added: number,
 ): Promise<ServiceResult<void>> {
-  if (added <= 0) return fail('No photos shared, skipping notifications', 200);
+  if (added <= 0) return success(undefined);

  try {
    const actorRow = db.prepare('SELECT username, email FROM users WHERE id = ?').get(actorUserId) as { username: string | null, email: string | null };
@@ -1,3 +1,4 @@
+import type Database from 'better-sqlite3';
 import { db } from '../db/database';
 import { decrypt_api_key } from './apiKeyCrypto';

@@ -187,8 +188,8 @@ function setAdminGlobalPref(event: NotifEventType, channel: 'email' | 'webhook'
 function applyUserChannelPrefs(
  userId: number,
  prefs: Partial<Record<string, Partial<Record<string, boolean>>>>,
-  upsert: ReturnType<typeof db.prepare>,
-  del: ReturnType<typeof db.prepare>
+  upsert: Database.Statement<unknown[]>,
+  del: Database.Statement<unknown[]>
 ): void {
  for (const [eventType, channels] of Object.entries(prefs)) {
    if (!channels) continue;
@@ -432,15 +432,6 @@ export function resolveNtfyUrl(adminCfg: NtfyConfig, userCfg: NtfyConfig | null)
  return `${base}/${encodeURIComponent(topic)}`;
 }

-export function isNtfyConfiguredForUser(userId: number): boolean {
-  const cfg = getUserNtfyConfig(userId);
-  return !!(cfg?.topic);
-}
-
-export function isNtfyConfiguredAdmin(): boolean {
-  return !!(getAppSetting('admin_ntfy_topic'));
-}
-
 function encodeHeaderValue(value: string): string {
  for (let i = 0; i < value.length; i++) {
    if (value.charCodeAt(i) > 0xFF) {
@@ -118,16 +118,6 @@ export function listOAuthClients(userId: number): Record<string, unknown>[] {
  }));
 }

-/** Returns true if the URI is a valid OAuth redirect target (HTTPS or localhost). */
-export function isValidRedirectUri(uri: string): boolean {
-  try {
-    const url = new URL(uri);
-    return url.protocol === 'https:' || url.hostname === 'localhost' || url.hostname === '127.0.0.1';
-  } catch {
-    return false;
-  }
-}
-
 export function createOAuthClient(
  userId: number | null,
  name: string,
@@ -1,11 +1,9 @@
-import { db, canAccessTrip } from '../db/database';
+import { db } from '../db/database';
 import { avatarUrl } from './authService';

 const BAG_COLORS = ['#6366f1', '#ec4899', '#f97316', '#10b981', '#06b6d4', '#8b5cf6', '#ef4444', '#f59e0b'];

-export function verifyTripAccess(tripId: string | number, userId: number) {
-  return canAccessTrip(tripId, userId);
-}
+export { verifyTripAccess } from './tripAccess';

 // ── Items ──────────────────────────────────────────────────────────────────

@@ -2,7 +2,7 @@ import { XMLParser, XMLValidator } from 'fast-xml-parser';
 import unzipper from 'unzipper';
 import { db, getPlaceWithTags } from '../db/database';
 import { loadTagsByPlaceIds } from './queryHelpers';
-import { checkSsrf } from '../utils/ssrfGuard';
+import { checkSsrf, safeFetchFollow, SsrfBlockedError } from '../utils/ssrfGuard';
 import { Place } from '../types';
 import {
  buildCategoryNameLookup,
@@ -587,10 +587,18 @@ export async function importGoogleList(tripId: string, url: string) {
  const ssrf = await checkSsrf(url);
  if (!ssrf.allowed) return { error: 'URL is not allowed', status: 400 };

-  // Follow redirects for short URLs (maps.app.goo.gl, goo.gl)
+  // Follow redirects for short URLs (maps.app.goo.gl, goo.gl). Redirects are
+  // followed manually so every hop is re-checked against the SSRF guard — a
+  // short link that 302s to an internal IP is blocked even though the initial
+  // host is public.
  if (url.includes('goo.gl') || url.includes('maps.app')) {
-    const redirectRes = await fetch(url, { redirect: 'follow', signal: AbortSignal.timeout(10000) });
-    resolvedUrl = redirectRes.url;
+    try {
+      const redirectRes = await safeFetchFollow(url, { signal: AbortSignal.timeout(10000) });
+      resolvedUrl = redirectRes.url;
+    } catch (err) {
+      if (err instanceof SsrfBlockedError) return { error: 'URL is not allowed', status: 400 };
+      throw err;
+    }
  }

  // Pattern: /placelists/list/{ID}
@@ -683,7 +691,7 @@ export async function importGoogleList(tripId: string, url: string) {
 export async function importNaverList(
  tripId: string,
  url: string,
-): Promise<{ places: any[]; listName: string } | { error: string; status: number }> {
+): Promise<{ places: any[]; listName: string; skipped: number } | { error: string; status: number }> {
  let resolvedUrl = url;
  const limit = 20;

@@ -692,11 +700,18 @@ export async function importNaverList(
  if (!ssrf.allowed) return { error: 'URL is not allowed', status: 400 };

  // Resolve naver.me short links to the canonical map.naver.com folder URL.
+  // Redirects are followed manually so each hop is re-validated against the
+  // SSRF guard (a short link could otherwise 302 to an internal address).
  let parsedUrl: URL;
  try { parsedUrl = new URL(url); } catch { return { error: 'Invalid URL', status: 400 }; }
  if (parsedUrl.hostname === 'naver.me') {
-    const redirectRes = await fetch(url, { redirect: 'follow', signal: AbortSignal.timeout(10000) });
-    resolvedUrl = redirectRes.url;
+    try {
+      const redirectRes = await safeFetchFollow(url, { signal: AbortSignal.timeout(10000) });
+      resolvedUrl = redirectRes.url;
+    } catch (err) {
+      if (err instanceof SsrfBlockedError) return { error: 'URL is not allowed', status: 400 };
+      throw err;
+    }
  }

  const folderMatch = resolvedUrl.match(/favorite\/myPlace\/folder\/([A-Za-z0-9_-]+)/i);
@@ -1,6 +1,8 @@
-import { db, canAccessTrip } from '../db/database';
+import { db } from '../db/database';
 import { Reservation } from '../types';

+export { verifyTripAccess } from './tripAccess';
+
 export interface ReservationEndpoint {
  id?: number;
  reservation_id?: number;
@@ -17,10 +19,6 @@ export interface ReservationEndpoint {

 type EndpointInput = Omit<ReservationEndpoint, 'id' | 'reservation_id' | 'sequence'> & { sequence?: number };

-export function verifyTripAccess(tripId: string | number, userId: number) {
-  return canAccessTrip(tripId, userId);
-}
-
 function loadEndpointsByTrip(tripId: string | number): Map<number, ReservationEndpoint[]> {
  const rows = db.prepare(`
    SELECT e.* FROM reservation_endpoints e
@@ -298,15 +296,6 @@ export function updatePositions(tripId: string | number, positions: { id: number
  }
 }

-export function getDayPositions(tripId: string | number, dayId: number | string) {
-  return db.prepare(`
-    SELECT rdp.reservation_id, rdp.position
-    FROM reservation_day_positions rdp
-    JOIN reservations r ON rdp.reservation_id = r.id
-    WHERE r.trip_id = ? AND rdp.day_id = ?
-  `).all(tripId, dayId) as { reservation_id: number; position: number }[];
-}
-
 export function getReservation(id: string | number, tripId: string | number) {
  return db.prepare('SELECT * FROM reservations WHERE id = ? AND trip_id = ?').get(id, tripId) as Reservation | undefined;
 }
@@ -1,8 +1,6 @@
-import { db, canAccessTrip } from '../db/database';
+import { db } from '../db/database';

-export function verifyTripAccess(tripId: string | number, userId: number) {
-  return canAccessTrip(tripId, userId);
-}
+export { verifyTripAccess } from './tripAccess';

 // ── Items ──────────────────────────────────────────────────────────────────

@@ -0,0 +1,9 @@
+import { canAccessTrip } from '../db/database';
+
+/**
+ * Returns the trip row if the user is the owner or a member, otherwise undefined.
+ * Shared by the domain services so each one exposes the same access check.
+ */
+export function verifyTripAccess(tripId: string | number, userId: number) {
+  return canAccessTrip(tripId, userId);
+}
@@ -1,6 +1,6 @@
 import path from 'path';
 import fs from 'fs';
-import { db, canAccessTrip, isOwner } from '../db/database';
+import { db, isOwner } from '../db/database';
 import { Trip, User } from '../types';
 import { listDays, listAccommodations } from './dayService';
 import { listBudgetItems } from './budgetService';
@@ -25,10 +25,7 @@ export const TRIP_SELECT = `

 // ── Access helpers ────────────────────────────────────────────────────────

-export function verifyTripAccess(tripId: string | number, userId: number) {
-  return canAccessTrip(tripId, userId);
-}
-
+export { verifyTripAccess } from './tripAccess';
 export { isOwner };

 // ── Day generation ────────────────────────────────────────────────────────
@@ -189,7 +186,7 @@ export function getTrip(tripId: string | number, userId: number) {
    ${TRIP_SELECT}
    LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = :userId
    WHERE t.id = :tripId AND (t.user_id = :userId OR m.user_id IS NOT NULL)
-  `).get({ userId, tripId });
+  `).get({ userId, tripId }) as Trip | undefined;
 }

 interface UpdateTripData {
@@ -636,19 +633,22 @@ export function copyTripById(sourceTripId: string | number, newOwnerId: number,

    const oldReservations = db.prepare('SELECT * FROM reservations WHERE trip_id = ?').all(sourceTripId) as any[];
    const insertReservation = db.prepare(`
-      INSERT INTO reservations (trip_id, day_id, place_id, assignment_id, accommodation_id, title, reservation_time, reservation_end_time,
-        location, confirmation_number, notes, status, type, metadata, day_plan_position)
-      VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+      INSERT INTO reservations (trip_id, day_id, end_day_id, place_id, assignment_id, accommodation_id, title, reservation_time, reservation_end_time,
+        location, confirmation_number, notes, status, type, metadata, day_plan_position, needs_review)
+      VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
    `);
    for (const r of oldReservations) {
      insertReservation.run(newTripId,
        r.day_id ? (dayMap.get(r.day_id) ?? null) : null,
+        // end_day_id is a day reference too (multi-day transport) — remap it like
+        // day_id, otherwise the duplicated trip loses the reservation's end-day link.
+        r.end_day_id ? (dayMap.get(r.end_day_id) ?? null) : null,
        r.place_id ? (placeMap.get(r.place_id) ?? null) : null,
        r.assignment_id ? (assignmentMap.get(r.assignment_id) ?? null) : null,
        r.accommodation_id ? (accomMap.get(r.accommodation_id) ?? null) : null,
        r.title, r.reservation_time, r.reservation_end_time,
        r.location, r.confirmation_number, r.notes, r.status, r.type,
-        r.metadata, r.day_plan_position);
+        r.metadata, r.day_plan_position, r.needs_review ?? 0);
    }

    const oldBudget = db.prepare('SELECT * FROM budget_items WHERE trip_id = ?').all(sourceTripId) as any[];