From ceb9af2480bc1d15561c2881ce42b2622dbf110f Mon Sep 17 00:00:00 2001 From: jubnl Date: Mon, 17 Aug 2026 07:34:15 +0200 Subject: [PATCH] refactor(server): boot dir-creation to driver init --- server/src/index.ts | 30 ++++---------- server/src/nest/auth/auth.helpers.ts | 9 ---- server/src/nest/auth/auth.service.ts | 1 - .../nest/storage/storage-registry.service.ts | 5 ++- server/tests/unit/uploads-dirs.test.ts | 41 +++++++++++-------- 5 files changed, 38 insertions(+), 48 deletions(-) diff --git a/server/src/index.ts b/server/src/index.ts index 017f8d52c..f8d458883 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -10,28 +10,16 @@ import http from 'node:http'; import type { INestApplication } from '@nestjs/common'; import { buildApp, getHttpServer } from './bootstrap'; -// Create upload and data directories on startup. -// Every uploads subdir the app writes to must be listed here (#1762): a dir -// created lazily on first upload needs write permission on the uploads mount -// point itself at request time, which fails with EACCES on Docker hosts whose -// bind-mounted uploads dir isn't writable by the runtime user — while every -// feature writing into an already-existing subdir keeps working. Creating them -// at boot turns that into one loud startup failure instead of a stray 500. -// Keep in sync with the `mkdir -p` list in the Dockerfile (guarded by -// tests/unit/uploads-dirs.test.ts). -const uploadsDir = path.join(__dirname, '../uploads'); -const photosDir = path.join(uploadsDir, 'photos'); -const filesDir = path.join(uploadsDir, 'files'); -const coversDir = path.join(uploadsDir, 'covers'); -const avatarsDir = path.join(uploadsDir, 'avatars'); -const journeyDir = path.join(uploadsDir, 'journey'); -const placesDir = path.join(uploadsDir, 'places'); -const backupsDir = path.join(__dirname, '../data/backups'); +// data/tmp is the driver-agnostic global scratch dir (restore-upload spool, +// mirror stream staging) and stays boot-created here. Driver-owned roots — the +// uploads/ category tree and data/backups — are ensured by LocalDriver.init on +// every storage-registry load (boot and reload(), storage-registry.service.ts), +// which keeps the #1762 EACCES guarantee: a non-writable bind mount still +// fails loudly at startup, inside app.init(), instead of as a stray 500 on +// first upload. The Dockerfile `mkdir -p` list is pinned to the registry's +// category prefixes by tests/unit/uploads-dirs.test.ts. const tmpDir = path.join(__dirname, '../data/tmp'); - -[uploadsDir, photosDir, filesDir, coversDir, avatarsDir, journeyDir, placesDir, backupsDir, tmpDir].forEach(dir => { - if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true }); -}); +if (!fs.existsSync(tmpDir)) fs.mkdirSync(tmpDir, { recursive: true }); import { getAppUrl, getMcpSafeUrl, readEnv } from './app-config'; diff --git a/server/src/nest/auth/auth.helpers.ts b/server/src/nest/auth/auth.helpers.ts index 5b34bb11b..e3a76811a 100644 --- a/server/src/nest/auth/auth.helpers.ts +++ b/server/src/nest/auth/auth.helpers.ts @@ -1,7 +1,5 @@ import bcrypt from 'bcryptjs'; import crypto from 'crypto'; -import path from 'path'; -import fs from 'fs'; import { User } from '../../types'; import { decrypt_api_key } from '../common/crypto/apiKeyCrypto'; @@ -38,13 +36,6 @@ export const ADMIN_SETTINGS_KEYS = [ 'passkey_login', 'webauthn_rp_id', 'webauthn_origins', ]; -// One directory deeper than the legacy src/services location, so the extra -// '../' keeps resolving to server/uploads/avatars (same anchor the multer -// diskStorage in auth.controller.ts uses). Import-time mkdir on purpose -// (legacy parity): the upload dir must exist before the first request. -export const avatarDir = path.join(__dirname, '../../../uploads/avatars'); -if (!fs.existsSync(avatarDir)) fs.mkdirSync(avatarDir, { recursive: true }); - // --------------------------------------------------------------------------- // Helpers (exported for route-level use where needed) // --------------------------------------------------------------------------- diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index e09f1a768..6d0141898 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -37,7 +37,6 @@ import { BCRYPT_COST, DUMMY_PASSWORD_HASH, EMAIL_REGEX, - avatarDir, generateBackupCodes, hashBackupCodeBcrypt, mask_stored_api_key, diff --git a/server/src/nest/storage/storage-registry.service.ts b/server/src/nest/storage/storage-registry.service.ts index 6d5b1ef3a..e502110eb 100644 --- a/server/src/nest/storage/storage-registry.service.ts +++ b/server/src/nest/storage/storage-registry.service.ts @@ -45,8 +45,11 @@ const REPLICA_FAILURE_RING_SIZE = 50; * (the backend root IS the backups dir — spec rev 3.1), and `photos-google` * flips to bare keys when it resolves to `place-photos-local` (the relocated * TREK_PLACE_PHOTO_DIR layout, place-photo-cache.service.ts). + * + * Exported for tests/unit/uploads-dirs.test.ts, which pins the Dockerfile's + * `mkdir -p /app/uploads/...` list to these prefixes. */ -const CATEGORY_PREFIXES: Record = { +export const CATEGORY_PREFIXES: Record = { files: 'files/', journey: 'journey/', covers: 'covers/', diff --git a/server/tests/unit/uploads-dirs.test.ts b/server/tests/unit/uploads-dirs.test.ts index 691b36b2a..1f5beab16 100644 --- a/server/tests/unit/uploads-dirs.test.ts +++ b/server/tests/unit/uploads-dirs.test.ts @@ -1,37 +1,46 @@ import { describe, it, expect } from 'vitest'; import fs from 'node:fs'; import path from 'node:path'; +import { CATEGORY_PREFIXES } from '../../src/nest/storage/storage-registry.service'; -// The set of uploads subdirectories is hand-mirrored between the image build and -// the boot-time mkdir in server/src/index.ts. Drift is invisible until a user on -// a host whose bind-mounted uploads dir isn't writable by node hits an EACCES on -// first upload to the missing dir (#1762), so pin the two copies together. +// Driver-owned roots are created by LocalDriver.init on every storage-registry +// load now (boot and reload()) — index.ts no longer mkdirs the uploads tree. +// The Dockerfile still pre-creates the subdirs (and chowns them) at image +// build, so its `mkdir -p` list must stay in sync with the category prefixes +// rooted under uploads/. Drift is invisible until a user on a host whose +// bind-mounted uploads dir isn't writable by node hits an EACCES on first +// upload to the missing dir (#1762), so pin the two lists together. const repoRoot = path.resolve(__dirname, '../../..'); -const readSubdirs = (file: string, pattern: RegExp): Set => { - const source = fs.readFileSync(path.join(repoRoot, file), 'utf8'); +const readDockerfileSubdirs = (): Set => { + const source = fs.readFileSync(path.join(repoRoot, 'Dockerfile'), 'utf8'); const found = new Set(); - for (const m of source.matchAll(pattern)) found.add(m[1]); + for (const m of source.matchAll(/\/app\/uploads\/([A-Za-z0-9_-]+)/g)) found.add(m[1]); return found; }; describe('uploads subdirectory parity', () => { - const fromDockerfile = readSubdirs('Dockerfile', /\/app\/uploads\/([A-Za-z0-9_-]+)/g); - const fromBoot = readSubdirs('server/src/index.ts', /path\.join\(uploadsDir, '([A-Za-z0-9_-]+)'\)/g); + const fromDockerfile = readDockerfileSubdirs(); + // The registry's real values: every non-empty category prefix lives under + // uploads/ (backups' empty prefix is rooted in data/, not uploads/), and the + // nested cache prefixes (photos/google, photos/trek) collapse into their + // top-level dir. + const fromRegistry = new Set( + Object.values(CATEGORY_PREFIXES) + .filter((p) => p !== '') + .map((p) => p.split('/')[0]), + ); - it('finds both lists (guards against the regexes silently matching nothing)', () => { + it('finds the Dockerfile list (guards against the regex silently matching nothing)', () => { expect(fromDockerfile.size).toBeGreaterThan(0); - expect(fromBoot.size).toBeGreaterThan(0); }); - it('creates the same uploads subdirs in the Dockerfile and at boot', () => { - expect([...fromBoot].sort()).toEqual([...fromDockerfile].sort()); + it('creates the same uploads subdirs in the Dockerfile and at registry init', () => { + expect([...fromRegistry].sort()).toEqual([...fromDockerfile].sort()); }); it('covers every uploads subdir the server writes to', () => { - // journey/ and places/ are written by journey.controller.ts and - // services/placeImage.ts; the rest are the long-standing upload stores. - expect([...fromBoot].sort()).toEqual([ + expect([...fromRegistry].sort()).toEqual([ 'avatars', 'covers', 'files',