From bc4d70986febbf0d364a9ff23a907537e034d290 Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 19 Aug 2026 20:27:50 +0200 Subject: [PATCH] =?UTF-8?q?feat(server):=20TREK=5FS3=5F*=20env=20wiring=20?= =?UTF-8?q?=E2=80=94=20schema,=20s3=20namespace,=20all-or-nothing=20boot?= =?UTF-8?q?=20rule?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- server/src/app-config/README.md | 4 +- server/src/app-config/derive.ts | 27 +++++++++++ server/src/app-config/env.schema.ts | 10 ++++ server/src/app-config/env.ts | 46 ++++++++++++++++++- server/src/app-config/index.ts | 1 + server/src/app-config/parsers.ts | 7 +++ server/tests/unit/app-config/derive.test.ts | 25 +++++++++- server/tests/unit/app-config/parsers.test.ts | 14 ++++++ server/tests/unit/app-config/validate.test.ts | 39 ++++++++++++++++ 9 files changed, 170 insertions(+), 3 deletions(-) diff --git a/server/src/app-config/README.md b/server/src/app-config/README.md index 229256c66..a68ba4ae6 100644 --- a/server/src/app-config/README.md +++ b/server/src/app-config/README.md @@ -69,7 +69,9 @@ 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_*, TRANSIT_API_URL, LOG_LEVEL*, ALLOW_INTERNAL_NETWORK*, DEFAULT_LANGUAGE, -TREK_DB_FILE, TREK_DB_JOURNAL_MODE, TREK_DB_SYNCHRONOUS, ENCRYPTION_KEY. +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). (* frozen today because the consuming module captures it at import; tests that override these set them at file top, before the SUT import.) diff --git a/server/src/app-config/derive.ts b/server/src/app-config/derive.ts index 674af530a..f504e86d1 100644 --- a/server/src/app-config/derive.ts +++ b/server/src/app-config/derive.ts @@ -18,6 +18,7 @@ import { SUPPORTED_LANGUAGE_CODES } from '@trek/shared'; import { csvList, csvListFiltered, + nonNegativeIntOr, numberOr, parseBool, parseDurationMs, @@ -268,6 +269,31 @@ 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), @@ -286,6 +312,7 @@ 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 f2c89ee67..0dc8ac782 100644 --- a/server/src/app-config/env.schema.ts +++ b/server/src/app-config/env.schema.ts @@ -128,6 +128,16 @@ export const envSchema = z.object({ 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 8e3d0f415..aebd55417 100644 --- a/server/src/app-config/env.ts +++ b/server/src/app-config/env.ts @@ -1,5 +1,10 @@ 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 }; @@ -30,7 +35,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)); + lines.push(...managedPreconditions(raw), ...s3Preconditions(raw)); if (lines.length === 0) return; console.error(`Invalid environment configuration:\n${lines.join('\n')}`); throw new Error( @@ -68,3 +73,42 @@ function managedPreconditions(raw: RawEnv): string[] { return problems; } + +const S3_VARS = [ + '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', +] as const; +const S3_REQUIRED = S3_VARS.slice(0, 4); + +/** + * 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: string): 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; +} diff --git a/server/src/app-config/index.ts b/server/src/app-config/index.ts index 211f078a4..78b15a7bd 100644 --- a/server/src/app-config/index.ts +++ b/server/src/app-config/index.ts @@ -20,6 +20,7 @@ export { deriveDb, derivePaths, deriveNet, + deriveS3, } from './derive'; export * from './parsers'; export { envSchema } from './env.schema'; diff --git a/server/src/app-config/parsers.ts b/server/src/app-config/parsers.ts index d4709c244..2d75e4db3 100644 --- a/server/src/app-config/parsers.ts +++ b/server/src/app-config/parsers.ts @@ -46,6 +46,13 @@ export function positiveIntOr(raw: string | undefined, fallback: number): number return Number.isFinite(n) && n > 0 ? n : fallback; } +/** Integer ≥ 0 (zero is meaningful — e.g. retries disabled), else the fallback. */ +export function nonNegativeIntOr(raw: string | undefined, fallback: number): number { + if (raw === undefined || raw.trim() === '') return fallback; + const n = Number.parseInt(raw, 10); + return Number.isFinite(n) && n >= 0 ? n : fallback; +} + /** Comma-split + trim, null when unset — the websocket ALLOWED_ORIGINS variant (no empty-entry filter). */ export function csvList(raw: string | undefined): string[] | null { return raw ? raw.split(',').map((o) => o.trim()) : null; diff --git a/server/tests/unit/app-config/derive.test.ts b/server/tests/unit/app-config/derive.test.ts index 2934ee8fe..b6b1453c0 100644 --- a/server/tests/unit/app-config/derive.test.ts +++ b/server/tests/unit/app-config/derive.test.ts @@ -13,6 +13,7 @@ import { deriveBackup, deriveNet, derivePaths, + deriveS3, deriveAll, } from '../../../src/app-config/derive'; @@ -233,6 +234,28 @@ describe('derivePaths', () => { }); }); +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' }); @@ -240,7 +263,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', + 'plugins', 'webauthn', 'integrations', 'backup', 'db', 'paths', 'net', 's3', ] as const) { expect(env[ns]).toBeDefined(); } diff --git a/server/tests/unit/app-config/parsers.test.ts b/server/tests/unit/app-config/parsers.test.ts index 21b02dfb1..8070e11e5 100644 --- a/server/tests/unit/app-config/parsers.test.ts +++ b/server/tests/unit/app-config/parsers.test.ts @@ -3,6 +3,7 @@ import { describe, it, expect } from 'vitest'; import { csvList, csvListFiltered, + nonNegativeIntOr, numberOr, parseBool, parseDurationMs, @@ -80,6 +81,19 @@ describe('positiveIntOr', () => { }); }); +describe('nonNegativeIntOr', () => { + it('accepts zero (0 disables retries) and positive integers', () => { + expect(nonNegativeIntOr('0', 1)).toBe(0); + expect(nonNegativeIntOr('3', 1)).toBe(3); + }); + it('falls back on unset, blank, negative, and non-numeric', () => { + expect(nonNegativeIntOr(undefined, 1)).toBe(1); + expect(nonNegativeIntOr(' ', 1)).toBe(1); + expect(nonNegativeIntOr('-1', 1)).toBe(1); + expect(nonNegativeIntOr('nope', 1)).toBe(1); + }); +}); + describe('csvList / csvListFiltered', () => { it('returns null when unset or empty', () => { expect(csvList(undefined)).toBeNull(); diff --git a/server/tests/unit/app-config/validate.test.ts b/server/tests/unit/app-config/validate.test.ts index d009f268a..aeeda636c 100644 --- a/server/tests/unit/app-config/validate.test.ts +++ b/server/tests/unit/app-config/validate.test.ts @@ -156,6 +156,45 @@ describe('validateEnvAtBoot — centrally administered preconditions', () => { }); }); +describe('validateEnvAtBoot — S3 all-or-nothing preconditions', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + 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('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\)/, + ); + }); +}); + describe('readEnv', () => { it('reads process.env live — a runtime mutation is visible on the next call', () => { const before = process.env.DEMO_MODE;