fix(backup): restore from Docker, fail-fast on shadowed /app, bundle encryption key (#1193) (#1197)

* fix(backup): restore uploads through symlinked dir and bundle encryption key (#1193)

Restoring a backup inside Docker threw ERR_FS_CP_DIR_TO_NON_DIR because
/app/server/uploads is a symlink to the mounted /app/uploads volume and
cpSync (dereference:false) refuses to overwrite the symlink node with a
directory. The DB was swapped before this failing copy, so users saw
restored data but missing upload files (trip covers). Resolve the symlink
with realpathSync before copying so the merge targets the real directory;
no-op on a plain dir, so non-Docker behavior is unchanged.

Also bundle the at-rest encryption key (data/.encryption_key) into the
backup so a restore onto a different install can decrypt stored secrets
(API keys, MFA, SMTP/OIDC). Skipped when ENCRYPTION_KEY is provided via
env (the file is not the source of truth then). On restore the key is
swapped back if the archive carries one; a restart is required for the
in-memory key to take effect.

* fix(docker): fail fast when a volume shadows /app (#1193)

Mounting an old volume at /app hides the image's node_modules and dist,
so startup crashed with a cryptic "Cannot find module
'tsconfig-paths/register'". Add a CMD preflight that detects the missing
app files and exits with actionable guidance. Document in the README that
only /app/data and /app/uploads should be mounted, never /app.

* fix: ssrf test

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"]

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.
+
 <h3>Rotating the Encryption Key</h3>
 
 If you need to rotate `ENCRYPTION_KEY` (e.g. upgrading from a version that derived encryption from `JWT_SECRET`):

server/src/services/backupService.ts

@@ -155,6 +155,17 @@ export async function createBackup(): Promise<BackupInfo> {
         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<RestoreResult> {
       }
       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<RestoreResult> {
             }
           }
         }
-        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

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<string, Function> = {};
+      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<string, Function> = {};
+    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 }
+    );
+  });
 });
 
 // ---------------------------------------------------------------------------

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');
     });
   });