diff --git a/backend/src/services/__tests__/databaseBackup.test.js b/backend/src/services/__tests__/databaseBackup.test.js index 3d7ff47d..0f0bd387 100644 --- a/backend/src/services/__tests__/databaseBackup.test.js +++ b/backend/src/services/__tests__/databaseBackup.test.js @@ -215,6 +215,7 @@ describe('DatabaseBackupService', () => { await expect(service.backup({})).rejects.toThrow(stop.message); expect(mkdirSpy).toHaveBeenCalledWith('/data/db-backups', { recursive: true }); + mkdirSpy.mockRestore(); }); it('falls back to /backup/database only when nothing is configured', async () => { @@ -229,6 +230,7 @@ describe('DatabaseBackupService', () => { await expect(service.backup({})).rejects.toThrow(stop.message); expect(mkdirSpy).toHaveBeenCalledWith('/backup/database', { recursive: true }); + mkdirSpy.mockRestore(); }); }); @@ -288,6 +290,40 @@ describe('DatabaseBackupService', () => { await expect(service.backup({})).rejects.toThrow('publicly served directory'); expect(mkdirSpy).not.toHaveBeenCalled(); + mkdirSpy.mockRestore(); + }); + + it('flags FRONTEND_DIR — the all-in-one image serves its built SPA unauthenticated', () => { + const originalFrontendDir = process.env.FRONTEND_DIR; + process.env.FRONTEND_DIR = '/app/frontend/dist'; + try { + expect(isUnderPubliclyServableRoot('/app/frontend/dist')).toBe(true); + expect(isUnderPubliclyServableRoot(path.join('/app/frontend/dist', 'assets'))).toBe(true); + } finally { + if (originalFrontendDir === undefined) delete process.env.FRONTEND_DIR; + else process.env.FRONTEND_DIR = originalFrontendDir; + } + }); + + it('resolves a symlinked alias of a public root to the same real directory (all-in-one /app/storage -> /data/storage)', async () => { + const os = require('os'); + const realRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'picpeak-real-')); + const linkRoot = path.join(os.tmpdir(), `picpeak-link-${process.pid}-${Date.now()}`); + await fs.mkdir(path.join(realRoot, 'uploads', 'logos'), { recursive: true }); + await fs.symlink(realRoot, linkRoot, 'dir'); + + try { + // STORAGE_PATH (what the guard's roots are built from) is the real + // path; the attacker-supplied destination goes through the symlink + // — exactly the all-in-one image's /app/storage -> /data/storage. + process.env.STORAGE_PATH = realRoot; + const aliased = path.join(linkRoot, 'uploads', 'logos'); + + expect(isUnderPubliclyServableRoot(aliased)).toBe(true); + } finally { + await fs.unlink(linkRoot); + await fs.rm(realRoot, { recursive: true, force: true }); + } }); }); diff --git a/backend/src/services/databaseBackup.js b/backend/src/services/databaseBackup.js index 0c8eaee6..4572f420 100644 --- a/backend/src/services/databaseBackup.js +++ b/backend/src/services/databaseBackup.js @@ -4,7 +4,7 @@ const crypto = require('crypto'); const { spawnAsync, spawnToFile } = require('../utils/safeExec'); const zlib = require('zlib'); const { pipeline } = require('stream/promises'); -const { createReadStream, createWriteStream } = require('fs'); +const { createReadStream, createWriteStream, realpathSync } = require('fs'); const { db } = require('../database/db'); const knexConfig = require('../../knexfile'); const logger = require('../utils/logger'); @@ -51,18 +51,49 @@ function getPubliclyServableRoots() { // on overlap but express.static falls through to this one on a miss). // COPY --chown=nodejs:nodejs in the Dockerfile makes this nodejs-owned // and therefore writable at runtime, not just a read-only image layer. - path.resolve(__dirname, '../../assets/fonts') + path.resolve(__dirname, '../../assets/fonts'), + // The all-in-one image's built frontend bundle (Dockerfile.aio ships it + // nodejs-owned) — server.js serves it unauthenticated as the SPA itself. + process.env.FRONTEND_DIR || path.resolve(__dirname, '../../../frontend/dist') ]; } +// Resolves symlinks in whatever prefix of candidatePath currently exists, +// then re-appends any not-yet-created remainder literally. A plain +// fs.realpathSync would throw ENOENT for the common case where the backup +// destination doesn't exist yet; a plain path.resolve() would miss the +// all-in-one image's `/app/storage -> /data/storage` symlink (Dockerfile.aio), +// which lets `/app/storage/uploads/logos` alias the real public logos +// directory under a name that never lexically matches it. +function resolveRealish(candidatePath) { + let current = path.resolve(candidatePath); + const remainder = []; + for (;;) { + try { + const real = realpathSync(current); + return remainder.length ? path.join(real, ...remainder) : real; + } catch (error) { + if (error.code !== 'ENOENT') { + return path.resolve(candidatePath); + } + const parent = path.dirname(current); + if (parent === current) { + return path.resolve(candidatePath); + } + remainder.unshift(path.basename(current)); + current = parent; + } + } +} + function isUnderPubliclyServableRoot(candidatePath) { // Lowercased comparison: on a case-insensitive-but-preserving filesystem // (default macOS APFS, NTFS, and Docker Desktop's bind-mount passthrough // of either) `STORAGE_PATH/UPLOADS/logos` and `.../uploads/logos` name the // same directory on disk even though path.resolve() never folds case. - const resolved = path.resolve(candidatePath).toLowerCase(); + const resolved = resolveRealish(candidatePath).toLowerCase(); return getPubliclyServableRoots().some((root) => { - const resolvedRoot = path.resolve(root).toLowerCase(); + const resolvedRoot = resolveRealish(root).toLowerCase(); return resolved === resolvedRoot || resolved.startsWith(resolvedRoot + path.sep); }); }