diff --git a/Dockerfile b/Dockerfile index 70afd237..d6463fd7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -105,5 +105,8 @@ HEALTHCHECK --interval=30s --timeout=5s --start-period=15s --retries=3 \ CMD wget -qO- http://localhost:3000/api/health || exit 1 ENTRYPOINT ["dumb-init", "--"] +# Preflight: if the app code is missing, a volume was almost certainly mounted +# over /app (it hides the image's node_modules + dist). Fail with actionable +# guidance instead of a cryptic "Cannot find module 'tsconfig-paths/register'". # cd into server/ so tsconfig-paths/register finds tsconfig.json and ../node_modules resolves correctly. -CMD ["sh", "-c", "chown -R node:node /app/data /app/uploads 2>/dev/null || true; cd /app/server && exec gosu node node --require tsconfig-paths/register dist/index.js"] +CMD ["sh", "-c", "if [ ! -f /app/server/dist/index.js ] || [ ! -d /app/node_modules/tsconfig-paths ]; then echo 'FATAL: TREK application files are missing from the image.'; echo 'A volume is likely mounted over /app, which hides the app code.'; echo 'Mount ONLY your data and uploads dirs: -v ./data:/app/data -v ./uploads:/app/uploads'; echo 'Do NOT mount a volume at /app. See the Troubleshooting section of the README.'; exit 1; fi; chown -R node:node /app/data /app/uploads 2>/dev/null || true; cd /app/server && exec gosu node node --require tsconfig-paths/register dist/index.js"] diff --git a/README.md b/README.md index 6c3a6f67..881e5c7d 100644 --- a/README.md +++ b/README.md @@ -311,6 +311,9 @@ docker run -d --name trek -p 3000:3000 -v ./data:/app/data -v ./uploads:/app/upl Your data stays in the mounted `data` and `uploads` volumes — updates never touch it. +> [!IMPORTANT] +> Mount **only** the data and uploads directories — `-v ./data:/app/data -v ./uploads:/app/uploads`. **Never mount a volume at `/app`.** Doing so hides the application code shipped in the image and the container fails to start with `Cannot find module 'tsconfig-paths/register'`. If you previously mounted `/app`, switch to the two mounts above; your data in `data/` and `uploads/` is preserved. +

Rotating the Encryption Key

If you need to rotate `ENCRYPTION_KEY` (e.g. upgrading from a version that derived encryption from `JWT_SECRET`): diff --git a/server/src/services/backupService.ts b/server/src/services/backupService.ts index 5128458a..282c61ac 100644 --- a/server/src/services/backupService.ts +++ b/server/src/services/backupService.ts @@ -155,6 +155,17 @@ export async function createBackup(): Promise { archive.file(dbPath, { name: 'travel.db' }); } + // Bundle the at-rest encryption key so the backup is self-contained: the + // DB stores secrets (API keys, MFA, SMTP/OIDC) encrypted with this key, so + // a restore onto a different install would otherwise be unable to decrypt + // them. NOTE: this makes the backup file as sensitive as the key itself — + // store/transfer it securely. Skipped when ENCRYPTION_KEY is provided via + // env, since in that case the file is not the source of truth. + const encKeyPath = path.join(dataDir, '.encryption_key'); + if (!process.env.ENCRYPTION_KEY && fs.existsSync(encKeyPath)) { + archive.file(encKeyPath, { name: '.encryption_key' }); + } + if (fs.existsSync(uploadsDir)) { // Exclude the place-photo and trek-memory caches: both are re-derivable // (re-fetched on demand, keyed on stable ids) and would otherwise dominate @@ -252,6 +263,16 @@ export async function restoreFromZip(zipPath: string): Promise { } fs.copyFileSync(extractedDb, dbDest); + // Restore the bundled at-rest encryption key (if the archive carries one) + // so the restored DB's encrypted secrets can be decrypted. Only the file + // is swapped here; the in-memory key was read at startup, so a restart is + // required for it to take effect (and an explicit ENCRYPTION_KEY env var + // still overrides the file). + const extractedEncKey = path.join(extractDir, '.encryption_key'); + if (fs.existsSync(extractedEncKey)) { + fs.copyFileSync(extractedEncKey, path.join(dataDir, '.encryption_key')); + } + const extractedUploads = path.join(extractDir, 'uploads'); if (fs.existsSync(extractedUploads)) { for (const sub of fs.readdirSync(uploadsDir)) { @@ -262,7 +283,12 @@ export async function restoreFromZip(zipPath: string): Promise { } } } - fs.cpSync(extractedUploads, uploadsDir, { recursive: true, force: true }); + // Copy into the real directory behind uploadsDir. In Docker, uploadsDir + // (/app/server/uploads) is a symlink to the mounted /app/uploads volume; + // cpSync(dereference:false) would otherwise try to overwrite the symlink + // node with a directory and throw ERR_FS_CP_DIR_TO_NON_DIR. realpathSync + // is a no-op when uploadsDir is a plain directory (dev/non-Docker). + fs.cpSync(extractedUploads, fs.realpathSync(uploadsDir), { recursive: true, force: true }); } } finally { // Reopening the DB must always run (even if the copy above threw) so the diff --git a/server/tests/unit/services/backupService.test.ts b/server/tests/unit/services/backupService.test.ts index 2263e3d3..381e05af 100644 --- a/server/tests/unit/services/backupService.test.ts +++ b/server/tests/unit/services/backupService.test.ts @@ -19,6 +19,9 @@ const fsMock = vi.hoisted(() => ({ rmSync: vi.fn(), copyFileSync: vi.fn(), cpSync: vi.fn(), + // Identity by default: when uploadsDir is a plain directory, realpathSync + // returns it unchanged. Tests that exercise the symlink case override this. + realpathSync: vi.fn((p: string) => p), })); const archiverInstanceMock = vi.hoisted(() => ({ @@ -479,6 +482,71 @@ describe('BACKUP-036 createBackup', () => { // The re-derivable caches must not be archived verbatim. expect(archiverInstanceMock.directory).not.toHaveBeenCalled(); }); + + it('BACKUP-036f — bundles .encryption_key when present and ENCRYPTION_KEY env is unset', async () => { + const prevEnvKey = process.env.ENCRYPTION_KEY; + delete process.env.ENCRYPTION_KEY; + try { + fsMock.existsSync.mockImplementation((p: string) => String(p).endsWith('.encryption_key')); + fsMock.mkdirSync.mockReturnValue(undefined); + + const writableEvents: Record = {}; + const fakeWriteStream = { + on: vi.fn((event: string, cb: Function) => { + writableEvents[event] = cb; + }), + }; + fsMock.createWriteStream.mockReturnValue(fakeWriteStream); + + archiverInstanceMock.on.mockImplementation((_e: string, _cb: Function) => {}); + archiverInstanceMock.pipe.mockReturnValue(undefined); + archiverInstanceMock.finalize.mockImplementation(() => { + if (writableEvents['close']) writableEvents['close'](); + }); + archiverMock.mockReturnValue(archiverInstanceMock); + + fsMock.statSync.mockReturnValue({ size: 1024, birthtime: new Date('2026-04-06T12:00:00Z') }); + + await createBackup(); + + expect(archiverInstanceMock.file).toHaveBeenCalledWith( + expect.stringContaining('.encryption_key'), + { name: '.encryption_key' }, + ); + } finally { + process.env.ENCRYPTION_KEY = prevEnvKey; + } + }); + + it('BACKUP-036g — does NOT bundle .encryption_key when ENCRYPTION_KEY env is set', async () => { + // setup.ts sets process.env.ENCRYPTION_KEY, so the env is the source of truth. + fsMock.existsSync.mockImplementation((p: string) => String(p).endsWith('.encryption_key')); + fsMock.mkdirSync.mockReturnValue(undefined); + + const writableEvents: Record = {}; + const fakeWriteStream = { + on: vi.fn((event: string, cb: Function) => { + writableEvents[event] = cb; + }), + }; + fsMock.createWriteStream.mockReturnValue(fakeWriteStream); + + archiverInstanceMock.on.mockImplementation((_e: string, _cb: Function) => {}); + archiverInstanceMock.pipe.mockReturnValue(undefined); + archiverInstanceMock.finalize.mockImplementation(() => { + if (writableEvents['close']) writableEvents['close'](); + }); + archiverMock.mockReturnValue(archiverInstanceMock); + + fsMock.statSync.mockReturnValue({ size: 1024, birthtime: new Date('2026-04-06T12:00:00Z') }); + + await createBackup(); + + expect(archiverInstanceMock.file).not.toHaveBeenCalledWith( + expect.stringContaining('.encryption_key'), + expect.anything(), + ); + }); }); // --------------------------------------------------------------------------- @@ -856,6 +924,53 @@ describe('BACKUP-045 restoreFromZip — full success path (no uploads)', () => { expect(dbMock.reinitialize).toHaveBeenCalled(); }); + + it('BACKUP-045d — restores bundled .encryption_key when the archive carries one', async () => { + setupSuccessfulExtraction(); + setupAllTablesPresent(); + + fsMock.existsSync.mockImplementation((p: string) => { + if (String(p).endsWith('travel.db')) return true; + if (String(p).endsWith('.encryption_key')) return true; // extracted key present + if (String(p).includes('uploads')) return false; + return true; + }); + fsMock.unlinkSync.mockReturnValue(undefined); + fsMock.copyFileSync.mockReturnValue(undefined); + fsMock.rmSync.mockReturnValue(undefined); + + const result = await restoreFromZip('/data/tmp/upload.zip'); + + expect(result).toEqual({ success: true }); + // Key copied from the extract dir into the live data dir. + expect(fsMock.copyFileSync).toHaveBeenCalledWith( + expect.stringContaining('.encryption_key'), + expect.stringContaining('.encryption_key'), + ); + }); + + it('BACKUP-045e — skips key restore when the archive has no .encryption_key', async () => { + setupSuccessfulExtraction(); + setupAllTablesPresent(); + + fsMock.existsSync.mockImplementation((p: string) => { + if (String(p).endsWith('travel.db')) return true; + if (String(p).endsWith('.encryption_key')) return false; // no key in archive + if (String(p).includes('uploads')) return false; + return true; + }); + fsMock.unlinkSync.mockReturnValue(undefined); + fsMock.copyFileSync.mockReturnValue(undefined); + fsMock.rmSync.mockReturnValue(undefined); + + const result = await restoreFromZip('/data/tmp/upload.zip'); + + expect(result).toEqual({ success: true }); + expect(fsMock.copyFileSync).not.toHaveBeenCalledWith( + expect.stringContaining('.encryption_key'), + expect.stringContaining('.encryption_key'), + ); + }); }); describe('BACKUP-046 restoreFromZip — with uploads directory', () => { @@ -912,6 +1027,64 @@ describe('BACKUP-046 restoreFromZip — with uploads directory', () => { { recursive: true, force: true } ); }); + + it('BACKUP-046b — copies into the symlink target, not the symlink itself (#1193)', async () => { + // In Docker, uploadsDir (/app/server/uploads) is a symlink to the mounted + // /app/uploads volume. cpSync(dereference:false) would throw + // ERR_FS_CP_DIR_TO_NON_DIR overwriting the symlink node with a directory. + // The fix resolves the symlink with realpathSync first, so the copy targets + // the real directory behind it. + setupSuccessfulExtraction(); + + const fakeDbInstance = { + prepare: vi.fn() + .mockReturnValueOnce({ + get: vi.fn().mockReturnValue({ integrity_check: 'ok' }), + }) + .mockReturnValueOnce({ + all: vi.fn().mockReturnValue([ + { name: 'users' }, + { name: 'trips' }, + { name: 'trip_members' }, + { name: 'places' }, + { name: 'days' }, + ]), + }), + close: vi.fn(), + }; + DatabaseMock.mockReturnValue(fakeDbInstance); + + fsMock.existsSync.mockImplementation((p: string) => { + if (String(p).endsWith('travel.db')) return true; + if (String(p).includes('uploads')) return true; + return true; + }); + fsMock.readdirSync.mockImplementation((p: string) => { + if (String(p).includes('uploads') && !String(p).includes('restore-')) { + return ['photos'] as any; + } + if (String(p).includes('photos')) return ['img1.jpg'] as any; + return [] as any; + }); + fsMock.statSync.mockReturnValue({ isDirectory: () => true } as any); + fsMock.unlinkSync.mockReturnValue(undefined); + fsMock.copyFileSync.mockReturnValue(undefined); + fsMock.cpSync.mockReturnValue(undefined); + fsMock.rmSync.mockReturnValue(undefined); + // Resolve the uploads symlink to a distinct real target directory. + const REAL_TARGET = '/app/uploads'; + fsMock.realpathSync.mockReturnValueOnce(REAL_TARGET); + + const result = await restoreFromZip('/data/tmp/upload.zip'); + + expect(result).toEqual({ success: true }); + // The copy destination must be the resolved real path, never the symlink. + expect(fsMock.cpSync).toHaveBeenCalledWith( + expect.stringContaining('uploads'), + REAL_TARGET, + { recursive: true, force: true } + ); + }); }); // --------------------------------------------------------------------------- diff --git a/server/tests/unit/utils/ssrfGuard.test.ts b/server/tests/unit/utils/ssrfGuard.test.ts index 691c24c2..7e4ca272 100644 --- a/server/tests/unit/utils/ssrfGuard.test.ts +++ b/server/tests/unit/utils/ssrfGuard.test.ts @@ -163,7 +163,7 @@ describe('checkSsrf', () => { const result = await checkSsrf('http://nxdomain.example.com'); expect(result.allowed).toBe(false); expect(result.isPrivate).toBe(false); - expect(result.error).toBe('Could not resolve hostname'); + expect(result.error).toContain('Could not resolve hostname'); }); });