From a6fb82833f7e4a7540da13d6726ef802be0bb6de Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 20 Aug 2026 00:41:03 +0200 Subject: [PATCH] =?UTF-8?q?feat(server):=20remove=20TREK=5FS3=5F*/TREK=5FU?= =?UTF-8?q?PLOADS=5FDIR=20=E2=80=94=20tombstone=20boot=20rule,=20built-in?= =?UTF-8?q?=20override=20as=20the=20relocation=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- server/src/app-config/README.md | 13 +- server/src/app-config/derive.ts | 29 --- server/src/app-config/env.schema.ts | 11 - server/src/app-config/env.ts | 55 ++--- server/src/app-config/index.ts | 1 - .../nest/storage/storage-registry.service.ts | 25 +- server/tests/unit/app-config/derive.test.ts | 27 +-- server/tests/unit/app-config/validate.test.ts | 65 ++--- .../storage/storage-registry.service.test.ts | 222 ++++++++---------- 9 files changed, 150 insertions(+), 298 deletions(-) diff --git a/server/src/app-config/README.md b/server/src/app-config/README.md index a68ba4ae6..a708be12b 100644 --- a/server/src/app-config/README.md +++ b/server/src/app-config/README.md @@ -66,15 +66,18 @@ fallthrough and the strip-ALL-slashes quirk (see `parsers.ts` DI in Nest, module-top `readEnv()` consts elsewhere): PORT, HOST, TRUST_PROXY, SESSION_DURATION(_REMEMBER), MCP_SESSION_TTL, MCP_MAX_SESSION_PER_USER, MCP_SSE_KEEPALIVE, TREK_PLUGIN_RPC_*/LOG_*/MAX_RSS_MB, -TREK_PLUGIN_REGISTRY_URL, TREK_WIKI_DIR*, TREK_PLACE_PHOTO_DIR, -TREK_UPLOADS_DIR, BACKUP_*, +TREK_PLUGIN_REGISTRY_URL, TREK_WIKI_DIR*, TREK_PLACE_PHOTO_DIR, BACKUP_*, TRANSIT_API_URL, LOG_LEVEL*, ALLOW_INTERNAL_NETWORK*, DEFAULT_LANGUAGE, -TREK_DB_FILE, TREK_DB_JOURNAL_MODE, TREK_DB_SYNCHRONOUS, ENCRYPTION_KEY, -TREK_S3_* (endpoint/bucket/credentials/region/keyPrefix/retries/timeoutMs — -read once by the storage registry at driver init, not on the request path). +TREK_DB_FILE, TREK_DB_JOURNAL_MODE, TREK_DB_SYNCHRONOUS, ENCRYPTION_KEY. (* frozen today because the consuming module captures it at import; tests that override these set them at file top, before the SUT import.) +**Tombstoned** (boot-aborting since the admin-config slice — configure storage +in the admin UI / `data/storage-config.json` instead): `TREK_S3_ENDPOINT`, +`TREK_S3_BUCKET`, `TREK_S3_ACCESS_KEY_ID`, `TREK_S3_SECRET_ACCESS_KEY`, +`TREK_S3_REGION`, `TREK_S3_KEY_PREFIX`, `TREK_S3_RETRIES`, `TREK_S3_TIMEOUT_MS`, +`TREK_UPLOADS_DIR` (see `storageTombstonePreconditions` in `env.ts`). + **Runtime-toggled** (read live on every access via `readEnv()` / `RuntimeEnvService`; tests mutate these mid-lifetime): TREK_MANAGED, PLACES_API_BASE, PLACES_API_KEY, MAPBOX_ACCESS_TOKEN, DEMO_MODE, NODE_ENV, APP_VERSION, APP_URL, TREK_API_DOCS_ENABLED, diff --git a/server/src/app-config/derive.ts b/server/src/app-config/derive.ts index f504e86d1..58f15fb42 100644 --- a/server/src/app-config/derive.ts +++ b/server/src/app-config/derive.ts @@ -18,7 +18,6 @@ import { SUPPORTED_LANGUAGE_CODES } from '@trek/shared'; import { csvList, csvListFiltered, - nonNegativeIntOr, numberOr, parseBool, parseDurationMs, @@ -258,8 +257,6 @@ export function derivePaths(raw: RawEnv) { return { wikiDir: raw.TREK_WIKI_DIR, placePhotoDir: raw.TREK_PLACE_PHOTO_DIR, - /** Uploads root override; unset keeps the computed `/uploads` default (storage registry). */ - uploadsDir: raw.TREK_UPLOADS_DIR, }; } @@ -269,31 +266,6 @@ export function deriveNet(raw: RawEnv) { }; } -/** - * S3 backend (spec: docs/superpowers/specs/2026-08-17-s3-storage-driver-design.md). - * New namespace — no legacy call site, so semantics are chosen clean: trim - * everything, blank = unset. `configured` is the registry's gate; partial sets - * are refused at boot by s3Preconditions (env.ts), never silently degraded. - */ -export function deriveS3(raw: RawEnv) { - const endpoint = (raw.TREK_S3_ENDPOINT ?? '').trim(); - const bucket = (raw.TREK_S3_BUCKET ?? '').trim(); - const accessKeyId = (raw.TREK_S3_ACCESS_KEY_ID ?? '').trim(); - const secretAccessKey = (raw.TREK_S3_SECRET_ACCESS_KEY ?? '').trim(); - return { - configured: Boolean(endpoint && bucket && accessKeyId && secretAccessKey), - endpoint, - bucket, - accessKeyId, - secretAccessKey, - /** Arbitrary values accepted — non-AWS endpoints ignore it (R2 wants 'auto'). */ - region: (raw.TREK_S3_REGION ?? '').trim() || 'us-east-1', - keyPrefix: (raw.TREK_S3_KEY_PREFIX ?? '').trim(), - retries: nonNegativeIntOr(raw.TREK_S3_RETRIES, 1), - timeoutMs: positiveNumberOr(raw.TREK_S3_TIMEOUT_MS, 30000), - }; -} - export function deriveAll(raw: RawEnv) { return { app: deriveApp(raw), @@ -312,7 +284,6 @@ export function deriveAll(raw: RawEnv) { backup: deriveBackup(raw), db: deriveDb(raw), paths: derivePaths(raw), - s3: deriveS3(raw), net: deriveNet(raw), }; } diff --git a/server/src/app-config/env.schema.ts b/server/src/app-config/env.schema.ts index 0dc8ac782..d2669bc41 100644 --- a/server/src/app-config/env.schema.ts +++ b/server/src/app-config/env.schema.ts @@ -124,20 +124,9 @@ export const envSchema = z.object({ TREK_DB_SYNCHRONOUS: anyString, TREK_WIKI_DIR: anyString, TREK_PLACE_PHOTO_DIR: anyString, - TREK_UPLOADS_DIR: anyString, BACKUP_UPLOAD_LIMIT_MB: positiveNumber, BACKUP_MAX_DECOMPRESSED_MB: positiveNumber, - // S3 storage backend (all-or-nothing set — cross-field rule in env.ts s3Preconditions) - TREK_S3_ENDPOINT: url, - TREK_S3_BUCKET: anyString, - TREK_S3_ACCESS_KEY_ID: anyString, - TREK_S3_SECRET_ACCESS_KEY: anyString, - TREK_S3_REGION: anyString, - TREK_S3_KEY_PREFIX: anyString, - TREK_S3_RETRIES: integer(0, Number.MAX_SAFE_INTEGER, 'must be a non-negative integer (0 disables retries)'), - TREK_S3_TIMEOUT_MS: positiveNumber, - // Admin / demo ADMIN_EMAIL: anyString, ADMIN_PASSWORD: anyString, diff --git a/server/src/app-config/env.ts b/server/src/app-config/env.ts index 124367454..0b0179c3b 100644 --- a/server/src/app-config/env.ts +++ b/server/src/app-config/env.ts @@ -1,10 +1,5 @@ import { deriveAll, type AppEnv, type RawEnv } from './derive'; import { envSchema } from './env.schema'; -// storage-keys is the single source of truth for key/prefix shape — deliberately -// imported here rather than re-implementing the rule for TREK_S3_KEY_PREFIX. It -// is a plain module with no imports back into app-config, so this does not -// create a cycle. -import { isValidPrefix } from '../nest/storage/storage-keys'; export type { AppEnv, RawEnv }; @@ -35,7 +30,7 @@ export function validateEnvAtBoot(raw: RawEnv = process.env as RawEnv): void { const key = String(issue.path[0]); return ` - ${key}=${JSON.stringify(raw[key])}: ${issue.message}`; }); - lines.push(...managedPreconditions(raw), ...s3Preconditions(raw)); + lines.push(...managedPreconditions(raw), ...storageTombstonePreconditions(raw)); if (lines.length === 0) return; console.error(`Invalid environment configuration:\n${lines.join('\n')}`); throw new Error( @@ -74,7 +69,15 @@ function managedPreconditions(raw: RawEnv): string[] { return problems; } -const S3_VARS = [ +/** + * Storage configuration moved wholesale to the admin UI / data/storage-config.json + * (spec: docs/superpowers/specs/2026-08-19-storage-admin-config-design.md). + * These variables are tombstoned rather than ignored: an upgrading operator + * with them set would otherwise silently run local-only — the exact + * half-configured trap the old all-or-nothing rule existed to prevent. + * TREK_PLACE_PHOTO_DIR intentionally survives (documented, prod-supported). + */ +const REMOVED_STORAGE_VARS = [ 'TREK_S3_ENDPOINT', 'TREK_S3_BUCKET', 'TREK_S3_ACCESS_KEY_ID', @@ -83,39 +86,11 @@ const S3_VARS = [ 'TREK_S3_KEY_PREFIX', 'TREK_S3_RETRIES', 'TREK_S3_TIMEOUT_MS', + 'TREK_UPLOADS_DIR', ] as const; -// Explicit, not `S3_VARS.slice(0, 4)` — the required set must not silently -// shift if S3_VARS is ever reordered. -const S3_REQUIRED: (typeof S3_VARS)[number][] = [ - 'TREK_S3_ENDPOINT', - 'TREK_S3_BUCKET', - 'TREK_S3_ACCESS_KEY_ID', - 'TREK_S3_SECRET_ACCESS_KEY', -]; -/** - * All-or-nothing: any TREK_S3_* set means the operator wants off-box storage; - * booting with a partial set would silently run without it (fail closed). - * Prefix shape delegates to storage-keys — the single source of key rules. - */ -function s3Preconditions(raw: RawEnv): string[] { - const present = (key: (typeof S3_VARS)[number]): boolean => Boolean(raw[key]?.trim()); - if (!S3_VARS.some(present)) return []; - const problems: string[] = []; - for (const key of S3_REQUIRED) { - if (!present(key)) { - problems.push( - ` - ${key} is unset: required when any TREK_S3_* variable is set, because a ` + - 'half-configured s3 backend would boot without the off-box storage the operator asked for.', - ); - } - } - const prefix = raw.TREK_S3_KEY_PREFIX?.trim().replace(/^\/+|\/+$/g, ''); - if (prefix && !isValidPrefix(`${prefix}/`)) { - problems.push( - ` - TREK_S3_KEY_PREFIX=${JSON.stringify(raw.TREK_S3_KEY_PREFIX)}: must be plain key segments ` + - '(no dot-segments, backslashes, or control characters).', - ); - } - return problems; +function storageTombstonePreconditions(raw: RawEnv): string[] { + return REMOVED_STORAGE_VARS.filter((key) => Boolean(raw[key]?.trim())).map( + (key) => ` - ${key} was removed — configure storage in the admin UI or data/storage-config.json.`, + ); } diff --git a/server/src/app-config/index.ts b/server/src/app-config/index.ts index 78b15a7bd..211f078a4 100644 --- a/server/src/app-config/index.ts +++ b/server/src/app-config/index.ts @@ -20,7 +20,6 @@ export { deriveDb, derivePaths, deriveNet, - deriveS3, } from './derive'; export * from './parsers'; export { envSchema } from './env.schema'; diff --git a/server/src/nest/storage/storage-registry.service.ts b/server/src/nest/storage/storage-registry.service.ts index e3a8bc18e..bc5c532bb 100644 --- a/server/src/nest/storage/storage-registry.service.ts +++ b/server/src/nest/storage/storage-registry.service.ts @@ -177,34 +177,17 @@ export class StorageRegistryService implements OnModuleInit { private build(settings: { backends: unknown; categories: unknown }, boot: boolean): RegistryState { // 1. Env is read fresh on every load (never snapshotted — RuntimeEnvService rule). - const paths = this.env.env().paths; - const uploadsRoot = paths.uploadsDir || DEFAULT_UPLOADS_ROOT; - const placePhotoDir = paths.placePhotoDir; + const placePhotoDir = this.env.env().paths.placePhotoDir; // 2. Built-in defaults; settings entries with the same name/category override. + // uploads-local's root is the computed default since TREK_UPLOADS_DIR was + // removed — relocation is a settings override row with the built-in's name. const backends = new Map(); - backends.set('uploads-local', { name: 'uploads-local', type: 'local', options: { root: uploadsRoot } }); + backends.set('uploads-local', { name: 'uploads-local', type: 'local', options: { root: DEFAULT_UPLOADS_ROOT } }); backends.set('backups-local', { name: 'backups-local', type: 'local', options: { root: DEFAULT_BACKUPS_ROOT } }); if (placePhotoDir) { backends.set('place-photos-local', { name: 'place-photos-local', type: 'local', options: { root: placePhotoDir } }); } - const s3 = this.env.env().s3; - if (s3.configured) { - backends.set('s3-main', { - name: 's3-main', - type: 's3', - options: { - endpoint: s3.endpoint, - region: s3.region, - bucket: s3.bucket, - keyPrefix: s3.keyPrefix, - accessKeyId: s3.accessKeyId, - secretAccessKey: s3.secretAccessKey, - retries: s3.retries, - timeoutMs: s3.timeoutMs, - }, - }); - } for (const config of parseBackendList(settings.backends)) backends.set(config.name, config); const categoryBackends = new Map(); diff --git a/server/tests/unit/app-config/derive.test.ts b/server/tests/unit/app-config/derive.test.ts index b6b1453c0..7886637ed 100644 --- a/server/tests/unit/app-config/derive.test.ts +++ b/server/tests/unit/app-config/derive.test.ts @@ -13,7 +13,6 @@ import { deriveBackup, deriveNet, derivePaths, - deriveS3, deriveAll, } from '../../../src/app-config/derive'; @@ -227,35 +226,11 @@ describe('derivePaths', () => { it('passes the path vars through raw — defaulting stays at the consumer', () => { expect(derivePaths({ TREK_WIKI_DIR: '/w' }).wikiDir).toBe('/w'); expect(derivePaths({ TREK_PLACE_PHOTO_DIR: '/p' }).placePhotoDir).toBe('/p'); - expect(derivePaths({ TREK_UPLOADS_DIR: '/mnt/uploads' }).uploadsDir).toBe('/mnt/uploads'); - expect(derivePaths({}).uploadsDir).toBeUndefined(); expect(derivePaths({}).wikiDir).toBeUndefined(); expect(derivePaths({}).placePhotoDir).toBeUndefined(); }); }); -describe('deriveS3', () => { - const full = { - TREK_S3_ENDPOINT: 'http://127.0.0.1:9000', - TREK_S3_BUCKET: 'trek', - TREK_S3_ACCESS_KEY_ID: 'ak', - TREK_S3_SECRET_ACCESS_KEY: 'sk', - }; - it('is unconfigured when nothing is set, with defaults intact', () => { - const s3 = deriveS3({}); - expect(s3.configured).toBe(false); - expect(s3).toMatchObject({ region: 'us-east-1', keyPrefix: '', retries: 1, timeoutMs: 30000 }); - }); - it('is configured only when all four required vars are non-blank', () => { - expect(deriveS3(full).configured).toBe(true); - expect(deriveS3({ ...full, TREK_S3_BUCKET: ' ' }).configured).toBe(false); - }); - it('applies overrides and trims values', () => { - const s3 = deriveS3({ ...full, TREK_S3_REGION: ' auto ', TREK_S3_KEY_PREFIX: 'trek/prod', TREK_S3_RETRIES: '0', TREK_S3_TIMEOUT_MS: '5000' }); - expect(s3).toMatchObject({ region: 'auto', keyPrefix: 'trek/prod', retries: 0, timeoutMs: 5000 }); - }); -}); - describe('deriveAll', () => { it('assembles every namespace', () => { const env = deriveAll({ PORT: '4000', DEMO_MODE: 'true' }); @@ -263,7 +238,7 @@ describe('deriveAll', () => { expect(env.demo.enabled).toBe(true); for (const ns of [ 'app', 'http', 'session', 'demo', 'adminBootstrap', 'oidc', 'smtp', 'mcp', - 'plugins', 'webauthn', 'integrations', 'backup', 'db', 'paths', 'net', 's3', + 'plugins', 'webauthn', 'integrations', 'backup', 'db', 'paths', 'net', ] as const) { expect(env[ns]).toBeDefined(); } diff --git a/server/tests/unit/app-config/validate.test.ts b/server/tests/unit/app-config/validate.test.ts index b288e3d93..372ed2439 100644 --- a/server/tests/unit/app-config/validate.test.ts +++ b/server/tests/unit/app-config/validate.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { validateEnvAtBoot, readEnv } from '../../../src/app-config/env'; -import { deriveS3 } from '../../../src/app-config/derive'; describe('validateEnvAtBoot', () => { afterEach(() => { @@ -157,47 +156,35 @@ describe('validateEnvAtBoot — centrally administered preconditions', () => { }); }); -describe('validateEnvAtBoot — S3 all-or-nothing preconditions', () => { - afterEach(() => { - vi.restoreAllMocks(); +describe('validateEnvAtBoot — removed storage variables (tombstones)', () => { + const removed = [ + 'TREK_S3_ENDPOINT', + 'TREK_S3_BUCKET', + 'TREK_S3_ACCESS_KEY_ID', + 'TREK_S3_SECRET_ACCESS_KEY', + 'TREK_S3_REGION', + 'TREK_S3_KEY_PREFIX', + 'TREK_S3_RETRIES', + 'TREK_S3_TIMEOUT_MS', + 'TREK_UPLOADS_DIR', + ] as const; + + it.each(removed)('TOMB-001 %s present at boot aborts, naming the replacement', (key) => { + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => validateEnvAtBoot({ [key]: 'anything' })).toThrow( + /Invalid environment configuration \(1 problem\)/, + ); + const report = error.mock.calls.map((c) => c.join(' ')).join('\n'); + expect(report).toContain(`${key} was removed`); + expect(report).toContain('admin UI or data/storage-config.json'); }); - const full = { - TREK_S3_ENDPOINT: 'http://127.0.0.1:9000', - TREK_S3_BUCKET: 'trek', - TREK_S3_ACCESS_KEY_ID: 'ak', - TREK_S3_SECRET_ACCESS_KEY: 'sk', - }; - it('S3-BOOT-001: refuses a partial TREK_S3_* set', () => { - vi.spyOn(console, 'error').mockImplementation(() => {}); - expect(() => validateEnvAtBoot({ TREK_S3_ENDPOINT: full.TREK_S3_ENDPOINT })).toThrow( - /Invalid environment configuration \(3 problems\)/, - ); + it('TOMB-002 TREK_PLACE_PHOTO_DIR survives (documented, prod-supported)', () => { + expect(() => validateEnvAtBoot({ TREK_PLACE_PHOTO_DIR: '/photos' })).not.toThrow(); }); - it('S3-BOOT-002: names each missing variable and the why', () => { - const error = vi.spyOn(console, 'error').mockImplementation(() => {}); - expect(() => validateEnvAtBoot({ TREK_S3_REGION: 'auto' })).toThrow(); - const report = error.mock.calls.map((c) => c.join(' ')).join('\n'); - expect(report).toContain('TREK_S3_ENDPOINT is unset'); - expect(report).toContain('TREK_S3_SECRET_ACCESS_KEY is unset'); - expect(report).toContain('required when any TREK_S3_'); - }); - it('S3-BOOT-003: passes with the full required set', () => { - expect(() => validateEnvAtBoot(full)).not.toThrow(); - }); - it('S3-BOOT-004: inert when no TREK_S3_* variable is set', () => { - expect(() => validateEnvAtBoot({})).not.toThrow(); - }); - it('S3-BOOT-005: rejects a malformed key prefix alongside schema problems', () => { - vi.spyOn(console, 'error').mockImplementation(() => {}); - expect(() => validateEnvAtBoot({ ...full, TREK_S3_KEY_PREFIX: '../evil', TREK_S3_ENDPOINT: 'not a url' })).toThrow( - /Invalid environment configuration \(2 problems\)/, - ); - }); - it('S3-BOOT-006: a slash-wrapped key prefix passes boot validation (shape-checked stripped of its slashes), while deriveS3 keeps it un-normalized', () => { - const withPrefix = { ...full, TREK_S3_KEY_PREFIX: '/trek/prod/' }; - expect(() => validateEnvAtBoot(withPrefix)).not.toThrow(); - expect(deriveS3(withPrefix).keyPrefix).toBe('/trek/prod/'); + + it('TOMB-003 blank values are unset-equivalent and do not trip the tombstone', () => { + expect(() => validateEnvAtBoot({ TREK_UPLOADS_DIR: ' ', TREK_S3_BUCKET: '' })).not.toThrow(); }); }); diff --git a/server/tests/unit/nest/storage/storage-registry.service.test.ts b/server/tests/unit/nest/storage/storage-registry.service.test.ts index e90f325d0..fb1b39394 100644 --- a/server/tests/unit/nest/storage/storage-registry.service.test.ts +++ b/server/tests/unit/nest/storage/storage-registry.service.test.ts @@ -24,13 +24,12 @@ import { createTables } from '../../../../src/db/schema'; import { runMigrations } from '../../../../src/db/migrations'; import { DatabaseService } from '../../../../src/nest/database/database.service'; import type { RuntimeEnvService } from '../../../../src/nest/app-config/runtime-env.service'; -import type { deriveS3 } from '../../../../src/app-config/derive'; import { encrypt_api_key } from '../../../../src/nest/common/crypto/apiKeyCrypto'; import { StorageRegistryService } from '../../../../src/nest/storage/storage-registry.service'; import { LocalDriver } from '../../../../src/nest/storage/drivers/local.driver'; import { MirrorDriver, type ReplicaFailure } from '../../../../src/nest/storage/drivers/mirror.driver'; import { S3Driver } from '../../../../src/nest/storage/drivers/s3.driver'; -import { GLOBAL_TEMP_DIR, DEFAULT_BACKUPS_ROOT } from '../../../../src/nest/storage/storage-paths'; +import { GLOBAL_TEMP_DIR, DEFAULT_BACKUPS_ROOT, DEFAULT_UPLOADS_ROOT } from '../../../../src/nest/storage/storage-paths'; import { STORAGE_CATEGORIES } from '../../../../src/nest/storage/storage.types'; const db = new DatabaseService(testDb); @@ -50,38 +49,13 @@ function makeTmpDir(): string { } interface EnvPaths { - uploadsDir?: string; placePhotoDir?: string; } -type EnvS3 = ReturnType; -const S3_OFF: EnvS3 = { - configured: false, - endpoint: '', - bucket: '', - accessKeyId: '', - secretAccessKey: '', - region: 'us-east-1', - keyPrefix: '', - retries: 1, - timeoutMs: 30000, -}; -const S3_ON: EnvS3 = { - ...S3_OFF, - configured: true, - endpoint: 'http://127.0.0.1:9000', - bucket: 'trek', - accessKeyId: 'ak', - secretAccessKey: 'sk', -}; - -function makeEnvStub( - initial: EnvPaths, - s3: EnvS3 = S3_OFF, -): { env: RuntimeEnvService; setPaths: (p: EnvPaths) => void } { +function makeEnvStub(initial: EnvPaths): { env: RuntimeEnvService; setPaths: (p: EnvPaths) => void } { let paths = initial; return { - env: { env: () => ({ paths, s3 }) } as unknown as RuntimeEnvService, + env: { env: () => ({ paths }) } as unknown as RuntimeEnvService, setPaths: (p) => { paths = p; }, @@ -92,13 +66,43 @@ function setSetting(key: string, value: string): void { testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run(key, value); } -/** Registry rooted in a fresh temp uploads dir; boots unless told not to. */ -function makeRegistry(paths?: EnvPaths, opts: { boot?: boolean; s3?: EnvS3 } = {}) { - const uploadsRoot = paths?.uploadsDir ?? makeTmpDir(); - const stub = makeEnvStub({ uploadsDir: uploadsRoot, placePhotoDir: paths?.placePhotoDir }, opts.s3); +function uploadsOverride(root: string): unknown { + return { name: 'uploads-local', type: 'local', options: { root } }; +} + +interface RegistryOpts { + uploadsRoot?: string; + placePhotoDir?: string; + boot?: boolean; + /** Extra storage.backends entries, appended after the uploads-local override. */ + backends?: unknown[]; + categories?: Record; +} + +/** + * TREK_UPLOADS_DIR is removed: the uploads-local root is the computed default, + * relocatable only via a settings override row bearing the built-in's name + * (first-class merge-by-name). The helper seeds that override so unit runs + * never write into the real server/uploads. + */ +function makeRegistry(opts: RegistryOpts = {}) { + const uploadsRoot = opts.uploadsRoot ?? makeTmpDir(); + setSetting('storage.backends', JSON.stringify([uploadsOverride(uploadsRoot), ...(opts.backends ?? [])])); + if (opts.categories) setSetting('storage.categories', JSON.stringify(opts.categories)); + const stub = makeEnvStub({ placePhotoDir: opts.placePhotoDir }); const registry = new StorageRegistryService(db, stub.env); if (opts.boot !== false) registry.onModuleInit(); - return { registry, uploadsRoot, setPaths: stub.setPaths }; + return { registry, uploadsRoot, setPaths: stub.setPaths, setUploadsRoot: (root: string) => rewriteUploadsOverride(root) }; +} + +/** For reload tests: swap the override row's root in place. */ +function rewriteUploadsOverride(root: string): void { + const row = testDb.prepare("SELECT value FROM app_settings WHERE key = 'storage.backends'").get() as + | { value: string } + | undefined; + const entries = row?.value ? (JSON.parse(row.value) as Array<{ name: string; options: { root: string } }>) : []; + const next = entries.map((e) => (e.name === 'uploads-local' ? { ...e, options: { root } } : e)); + setSetting('storage.backends', JSON.stringify(next)); } beforeEach(() => { @@ -161,6 +165,25 @@ describe('StorageRegistryService defaults', () => { expect(fs.statSync(GLOBAL_TEMP_DIR).isDirectory()).toBe(true); expect(fs.statSync(path.join(DEFAULT_BACKUPS_ROOT, '.tmp')).isDirectory()).toBe(true); }); + + it('roots uploads-local at the computed default when no override row exists', () => { + const { registry } = makeRegistry({ boot: false }); + testDb.prepare("DELETE FROM app_settings WHERE key = 'storage.backends'").run(); + registry.onModuleInit(); + const files = registry.resolve('files'); + expect(files.backendName).toBe('uploads-local'); + expect(files.driver.getLocalPath!('files/x.pdf')).toBe( + path.join(fs.realpathSync(DEFAULT_UPLOADS_ROOT), 'files/x.pdf'), + ); + }); + + it('a settings row bearing a built-in name replaces it (first-class override)', () => { + const relocated = makeTmpDir(); + const { registry } = makeRegistry({ uploadsRoot: relocated }); + expect(registry.resolve('files').driver.getLocalPath!('files/x.pdf')).toBe( + path.join(fs.realpathSync(relocated), 'files/x.pdf'), + ); + }); }); // ── settings merge + validation ─────────────────────────────────────────────── @@ -168,16 +191,13 @@ describe('StorageRegistryService defaults', () => { describe('StorageRegistryService settings', () => { it('merges app_settings backends/categories over the defaults (backup mirror)', () => { const nasRoot = makeTmpDir(); - setSetting( - 'storage.backends', - JSON.stringify([ + const { registry } = makeRegistry({ + backends: [ { name: 'nas-backups', type: 'local', options: { root: nasRoot } }, { name: 'backup-mirror', type: 'mirror', options: { primary: 'backups-local', replicas: ['nas-backups'] } }, - ]), - ); - setSetting('storage.categories', JSON.stringify({ backups: 'backup-mirror' })); - - const { registry } = makeRegistry(); + ], + categories: { backups: 'backup-mirror' }, + }); const backups = registry.resolve('backups'); expect(backups.backendName).toBe('backup-mirror'); expect(backups.keyPrefix).toBe(''); @@ -213,25 +233,24 @@ describe('StorageRegistryService settings', () => { ['unknown category name', undefined, JSON.stringify({ 'not-a-category': 'uploads-local' })], ['malformed JSON', 'not json at all', undefined], ])('falls back to built-in defaults at boot on invalid settings: %s', (_label, backendsRow, categoriesRow) => { + const { registry } = makeRegistry({ boot: false }); if (backendsRow !== undefined) setSetting('storage.backends', backendsRow); if (categoriesRow !== undefined) setSetting('storage.categories', categoriesRow); + registry.onModuleInit(); - const { registry } = makeRegistry(); expect(registry.resolve('files').backendName).toBe('uploads-local'); expect(registry.resolve('backups').backendName).toBe('backups-local'); }); it('keeps the last-good config (not defaults) when a reload() sees invalid settings', () => { const nasRoot = makeTmpDir(); - setSetting( - 'storage.backends', - JSON.stringify([ + const { registry } = makeRegistry({ + backends: [ { name: 'nas-backups', type: 'local', options: { root: nasRoot } }, { name: 'backup-mirror', type: 'mirror', options: { primary: 'backups-local', replicas: ['nas-backups'] } }, - ]), - ); - setSetting('storage.categories', JSON.stringify({ backups: 'backup-mirror' })); - const { registry } = makeRegistry(); + ], + categories: { backups: 'backup-mirror' }, + }); expect(registry.resolve('backups').backendName).toBe('backup-mirror'); setSetting('storage.categories', 'garbage {'); @@ -246,11 +265,11 @@ describe('StorageRegistryService settings', () => { describe('StorageRegistryService reload', () => { it('swaps the map atomically: new resolves see the new instance, held refs stay usable', async () => { - const { registry, setPaths } = makeRegistry(); + const { registry, setUploadsRoot } = makeRegistry(); const before = registry.resolve('files'); await before.driver.put('files/pre-reload.bin', Readable.from('old root')); - setPaths({ uploadsDir: makeTmpDir() }); + setUploadsRoot(makeTmpDir()); registry.reload(); const after = registry.resolve('files'); @@ -274,7 +293,7 @@ describe('StorageRegistryService reload', () => { // process may be spooling into the same tree — see LocalDriver.init). const old = new Date(Date.now() - 2 * 60 * 60 * 1000); fs.utimesSync(stray, old, old); - const { registry } = makeRegistry({ uploadsDir: uploadsRoot }); + const { registry } = makeRegistry({ uploadsRoot }); expect(fs.existsSync(stray)).toBe(false); // boot reclaimed it fs.writeFileSync(stray, 'in-flight upload'); @@ -304,44 +323,6 @@ describe('StorageRegistryService replica health', () => { }); }); -// ── s3-main (env-declared) ──────────────────────────────────────────────────── - -describe('s3-main (env-declared)', () => { - it('appears only when TREK_S3_* is configured, with no category mapped to it', () => { - const { registry } = makeRegistry({}, { s3: S3_ON }); - expect(() => registry.resolve('backups')).not.toThrow(); - for (const category of STORAGE_CATEGORIES) { - expect(registry.resolve(category).backendName).not.toBe('s3-main'); - } - }); - - it('is absent when the env is unset (zero behavior change)', () => { - setSetting('storage.categories', JSON.stringify({ backups: 's3-main' })); - const { registry } = makeRegistry(); // S3_OFF - // invalid mapping → last-good/defaults kept, per the standing mechanism - expect(registry.resolve('backups').backendName).toBe('backups-local'); - }); - - it('is assignable to any category via settings (S3Driver instance)', () => { - setSetting('storage.categories', JSON.stringify({ covers: 's3-main' })); - const { registry } = makeRegistry({}, { s3: S3_ON }); - const resolved = registry.resolve('covers'); - expect(resolved.backendName).toBe('s3-main'); - expect(resolved.driver).toBeInstanceOf(S3Driver); - expect(resolved.keyPrefix).toBe('covers/'); - }); - - it('serves as a mirror replica (local primary + s3 replica on backups)', () => { - setSetting( - 'storage.backends', - JSON.stringify([{ name: 'backup-mirror', type: 'mirror', options: { primary: 'backups-local', replicas: ['s3-main'] } }]), - ); - setSetting('storage.categories', JSON.stringify({ backups: 'backup-mirror' })); - const { registry } = makeRegistry({}, { s3: S3_ON }); - expect(registry.resolve('backups').backendName).toBe('backup-mirror'); - }); -}); - // ── settings-declared s3 backends (admin-config slice: acceptance) ─────────── describe('settings-declared s3 backends', () => { @@ -357,9 +338,10 @@ describe('settings-declared s3 backends', () => { }; it('accepts a settings s3 backend and assigns it to a category', () => { - setSetting('storage.backends', JSON.stringify([{ name: 'off-box', type: 's3', options: s3Options }])); - setSetting('storage.categories', JSON.stringify({ covers: 'off-box' })); - const { registry } = makeRegistry(); + const { registry } = makeRegistry({ + backends: [{ name: 'off-box', type: 's3', options: s3Options }], + categories: { covers: 'off-box' }, + }); const resolved = registry.resolve('covers'); expect(resolved.backendName).toBe('off-box'); expect(resolved.driver).toBeInstanceOf(S3Driver); @@ -367,51 +349,41 @@ describe('settings-declared s3 backends', () => { }); it('decrypts an enc:v1: secretAccessKey at build (decrypt-at-build)', () => { - setSetting( - 'storage.backends', - JSON.stringify([ - { name: 'off-box', type: 's3', options: { ...s3Options, secretAccessKey: encrypt_api_key('sk-secret') } }, - ]), - ); - setSetting('storage.categories', JSON.stringify({ backups: 'off-box' })); - const { registry } = makeRegistry(); + const { registry } = makeRegistry({ + backends: [{ name: 'off-box', type: 's3', options: { ...s3Options, secretAccessKey: encrypt_api_key('sk-secret') } }], + categories: { backups: 'off-box' }, + }); expect(registry.resolve('backups').driver).toBeInstanceOf(S3Driver); }); it('applies the shared-schema defaults for omitted optional fields', () => { const { endpoint, bucket, accessKeyId, secretAccessKey } = s3Options; - setSetting( - 'storage.backends', - JSON.stringify([{ name: 'off-box', type: 's3', options: { endpoint, bucket, accessKeyId, secretAccessKey } }]), - ); - setSetting('storage.categories', JSON.stringify({ backups: 'off-box' })); - const { registry } = makeRegistry(); + const { registry } = makeRegistry({ + backends: [{ name: 'off-box', type: 's3', options: { endpoint, bucket, accessKeyId, secretAccessKey } }], + categories: { backups: 'off-box' }, + }); expect(registry.resolve('backups').backendName).toBe('off-box'); }); it('serves as a mirror replica (local primary + settings s3 replica on backups)', () => { const nasRoot = makeTmpDir(); - setSetting( - 'storage.backends', - JSON.stringify([ + const { registry } = makeRegistry({ + backends: [ { name: 'off-box', type: 's3', options: s3Options }, { name: 'nas-backups', type: 'local', options: { root: nasRoot } }, { name: 'backup-mirror', type: 'mirror', options: { primary: 'nas-backups', replicas: ['off-box'] } }, - ]), - ); - setSetting('storage.categories', JSON.stringify({ backups: 'backup-mirror' })); - const { registry } = makeRegistry(); + ], + categories: { backups: 'backup-mirror' }, + }); expect(registry.resolve('backups').backendName).toBe('backup-mirror'); expect(registry.resolve('backups').driver).toBeInstanceOf(MirrorDriver); }); it('falls back with the pinned message when s3 options fail the shared schema', () => { const errorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined); - setSetting( - 'storage.backends', - JSON.stringify([{ name: 'off-box', type: 's3', options: { ...s3Options, endpoint: 'not a url' } }]), - ); - const { registry } = makeRegistry(); + const { registry } = makeRegistry({ + backends: [{ name: 'off-box', type: 's3', options: { ...s3Options, endpoint: 'not a url' } }], + }); expect(registry.resolve('backups').backendName).toBe('backups-local'); const logged = errorSpy.mock.calls.map((call) => String(call[0])).join('\n'); expect(logged).toContain("s3 backend 'off-box' has invalid options — endpoint:"); @@ -419,12 +391,10 @@ describe('settings-declared s3 backends', () => { it('falls back with the pinned message when the ciphertext cannot be decrypted', () => { const errorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined); - setSetting( - 'storage.backends', - JSON.stringify([{ name: 'off-box', type: 's3', options: { ...s3Options, secretAccessKey: 'enc:v1:AAAA' } }]), - ); - setSetting('storage.categories', JSON.stringify({ backups: 'off-box' })); - const { registry } = makeRegistry(); + const { registry } = makeRegistry({ + backends: [{ name: 'off-box', type: 's3', options: { ...s3Options, secretAccessKey: 'enc:v1:AAAA' } }], + categories: { backups: 'off-box' }, + }); expect(registry.resolve('backups').backendName).toBe('backups-local'); const logged = errorSpy.mock.calls.map((call) => String(call[0])).join('\n'); expect(logged).toContain("s3 backend 'off-box': could not decrypt 'secretAccessKey'");