diff --git a/backend/__tests__/services/businessDocsStoragePath.test.js b/backend/__tests__/services/businessDocsStoragePath.test.js new file mode 100644 index 00000000..7ee0005b --- /dev/null +++ b/backend/__tests__/services/businessDocsStoragePath.test.js @@ -0,0 +1,159 @@ +/** + * Regression test: business documents must be written under STORAGE_PATH. + * + * quoteService.persistDocPdf, the invoice sending/reminder writers and the + * contract signature writers all built their target from + * `path.join(process.cwd(), 'storage', 'business-docs', ...)`. Both compose + * files pin STORAGE_PATH=/app/storage and the image's WORKDIR is /app, so the + * two expressions name the same directory and the bug was invisible on a stock + * deployment. Point STORAGE_PATH anywhere else — a NAS mount, a second disk, + * the single-container image's /data volume — and quotes, invoices, Mahnungen + * and contract PDFs were written outside the configured storage root, so they + * were missed by backups and lost when the container was replaced. + * + * Rather than assert on internals, this drives the module boundary the fix + * changed: getStoragePath() is the one resolver, so a temporary STORAGE_PATH + * must be where the bytes land. + */ + +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +describe('business documents honour STORAGE_PATH', () => { + let tmpRoot; + let originalStoragePath; + + beforeEach(() => { + originalStoragePath = process.env.STORAGE_PATH; + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-storage-')); + process.env.STORAGE_PATH = tmpRoot; + jest.resetModules(); + }); + + afterEach(() => { + if (originalStoragePath === undefined) delete process.env.STORAGE_PATH; + else process.env.STORAGE_PATH = originalStoragePath; + fs.rmSync(tmpRoot, { recursive: true, force: true }); + }); + + it('getStoragePath is the resolver the writers share', () => { + const { getStoragePath } = require('../../src/config/storage'); + expect(getStoragePath()).toBe(tmpRoot); + }); + + it('no business-document writer still targets process.cwd()/storage', () => { + // Whitespace is collapsed before matching on purpose. The first version of + // this test compared against the single-line literal and therefore missed + // persistSignatureImage(), whose identical path.join was simply spread over + // seven lines — it reported green while signature PNGs still wrote outside + // STORAGE_PATH. Formatting must not decide whether a bug is visible. + const writers = [ + 'src/services/quoteService.js', + 'src/services/invoice/sending.js', + 'src/services/invoice/reminders.js', + 'src/services/contract/signatureAssets.js', + 'src/routes/adminDev.js', + ]; + const offenders = writers.filter((rel) => { + const source = fs.readFileSync(path.join(__dirname, '../../', rel), 'utf8'); + return /process\.cwd\(\),'storage'/.test(source.replace(/\s+/g, '')); + }); + expect(offenders).toEqual([]); + }); + + it('generated contract PDFs pass the containment check that serves them', () => { + // assertContractPdfPath guards the admin and public contract download + // routes. It listed only /storage/business-docs/contract, so once the + // writers moved to STORAGE_PATH every freshly generated contract was + // refused with PATH_OUTSIDE_STORAGE — a worse failure than the bug being + // fixed. Both roots must be accepted. + const { assertContractPdfPath } = require('../../src/utils/safePath'); + const { getStoragePath } = require('../../src/config/storage'); + + // assertPathInside realpaths both the file and each root, so the guard only + // means anything against a filesystem that actually has them — write them. + const write = (...segments) => { + const p = path.join(getStoragePath(), 'business-docs', 'contract', ...segments); + fs.mkdirSync(path.dirname(p), { recursive: true }); + fs.writeFileSync(p, 'bytes'); + return p; + }; + + const generated = write('2026', 'C-2026-0001.pdf'); + expect(() => assertContractPdfPath(generated)).not.toThrow(); + + // Signature PNGs live under the same root and are served by the same guard. + const signature = write('signatures', '7', 'customer-1.png'); + expect(() => assertContractPdfPath(signature)).not.toThrow(); + + // And the guard still refuses a real file outside every allowed root. + const foreign = path.join(tmpRoot, 'outside.pdf'); + fs.writeFileSync(foreign, 'bytes'); + expect(() => assertContractPdfPath(foreign)).toThrow(/outside the storage roots/i); + }); + + it('the guard takes its root from the shared resolver, not its own fallback', () => { + // The regression this pins: the guard used to compute + // `STORAGE_PATH || /storage` itself. That agrees with getStoragePath() + // only while STORAGE_PATH is set — unset, the shared resolver falls back + // module-relative to /storage while the guard fell back to + // /storage, and the backend is normally started from backend/. Writers + // and guard then disagreed and contract downloads 403'd. + // + // Mocking the resolver is what makes this provable AND safe. If the guard + // consumes getStoragePath(), the mock moves its root; if it rolled its own + // expression, the mock would have no effect and the assertion fails. It + // also keeps every path inside the tmpdir — an earlier version of this test + // deleted `/business-docs` in cleanup, which with + // STORAGE_PATH unset resolves to a developer's real, gitignored + // /storage and would have destroyed local documents on `npm test`. + jest.resetModules(); + jest.doMock('../../src/config/storage', () => ({ getStoragePath: () => tmpRoot })); + + const { assertContractPdfPath } = require('../../src/utils/safePath'); + + const root = path.join(tmpRoot, 'business-docs', 'contract', '2026'); + fs.mkdirSync(root, { recursive: true }); + const generated = path.join(root, 'C-2026-0002.pdf'); + fs.writeFileSync(generated, 'bytes'); + + expect(() => assertContractPdfPath(generated)).not.toThrow(); + + jest.dontMock('../../src/config/storage'); + }); + + it('writes land under STORAGE_PATH, not the working directory', () => { + const { getStoragePath } = require('../../src/config/storage'); + + // Mirror what persistDocPdf does: derive the root, create it, write. + const root = path.join(getStoragePath(), 'business-docs', 'quote', '2026'); + fs.mkdirSync(root, { recursive: true }); + const filePath = path.join(root, 'Q-2026-0001.pdf'); + fs.writeFileSync(filePath, 'pdf-bytes'); + + expect(fs.existsSync(filePath)).toBe(true); + expect(filePath.startsWith(tmpRoot)).toBe(true); + // And crucially NOT beside the process working directory. + expect(filePath.startsWith(path.join(process.cwd(), 'storage'))).toBe(false); + }); + + it('the PDF font lookup consults the storage root before the legacy path', () => { + // A custom font under STORAGE_PATH/fonts used to be unreachable, so the + // document silently rendered with the built-in face instead. + const fontDir = path.join(tmpRoot, 'fonts'); + fs.mkdirSync(fontDir, { recursive: true }); + const fontPath = path.join(fontDir, 'Brand.ttf'); + fs.writeFileSync(fontPath, 'ttf'); + + const { getStoragePath } = require('../../src/config/storage'); + const raw = 'Brand.ttf'; + const candidates = [ + path.join(getStoragePath(), raw.replace(/^\/+/, '')), + path.join(getStoragePath(), 'fonts', path.basename(raw)), + path.join(process.cwd(), 'storage', 'fonts', path.basename(raw)), + ]; + const found = candidates.find((p) => fs.existsSync(p)); + expect(found).toBe(fontPath); + }); +}); diff --git a/backend/src/routes/adminDev.js b/backend/src/routes/adminDev.js index 69416406..c81c3940 100644 --- a/backend/src/routes/adminDev.js +++ b/backend/src/routes/adminDev.js @@ -28,6 +28,7 @@ */ const express = require('express'); +const { getStoragePath } = require('../config/storage'); const { body } = require('express-validator'); const path = require('path'); const fs = require('fs'); @@ -128,7 +129,7 @@ router.get( ); const FRONTEND_URL_FALLBACK = 'https://app.example.com'; -const DEV_TEST_DIR = () => path.join(process.cwd(), 'storage', 'business-docs', 'dev-test'); +const DEV_TEST_DIR = () => path.join(getStoragePath(), 'business-docs', 'dev-test'); function fakeMoney(major, currency, locale = 'de') { return new Intl.NumberFormat(locale === 'de' ? 'de-CH' : 'en-GB', { diff --git a/backend/src/services/backupCoverageService.js b/backend/src/services/backupCoverageService.js index 4adcd344..fd804fb3 100644 --- a/backend/src/services/backupCoverageService.js +++ b/backend/src/services/backupCoverageService.js @@ -40,11 +40,17 @@ const fs = require('fs').promises; const path = require('path'); +const { getStoragePath } = require('../config/storage'); const { db } = require('../database/db'); const logger = require('../utils/logger'); const backupService = require('./backupService'); -const STORAGE_ROOT = () => process.env.STORAGE_PATH || path.join(process.cwd(), 'storage'); +// The shared resolver, not a second `STORAGE_PATH || cwd` expression. With +// STORAGE_PATH unset the two disagree — getStoragePath() falls back +// module-relative while cwd is normally backend/ — and this diagnostic would +// then report the business-docs tree as missing while the backup walker, which +// uses the module-relative root, was backing it up correctly. +const STORAGE_ROOT = () => getStoragePath(); /** * Top-level subdirectories we expect to find under STORAGE_PATH but diff --git a/backend/src/services/backupIntegrityService.js b/backend/src/services/backupIntegrityService.js index eb273f73..5d516406 100644 --- a/backend/src/services/backupIntegrityService.js +++ b/backend/src/services/backupIntegrityService.js @@ -49,12 +49,18 @@ */ const fs = require('fs'); +const { getStoragePath } = require('../config/storage'); const crypto = require('crypto'); const path = require('path'); const { db } = require('../database/db'); const logger = require('../utils/logger'); -const STORAGE_ROOT = () => process.env.STORAGE_PATH || path.join(process.cwd(), 'storage'); +// The shared resolver, not a second `STORAGE_PATH || cwd` expression. With +// STORAGE_PATH unset the two disagree — getStoragePath() falls back +// module-relative while cwd is normally backend/ — and this diagnostic would +// then report the business-docs tree as missing while the backup walker, which +// uses the module-relative root, was backing it up correctly. +const STORAGE_ROOT = () => getStoragePath(); /** * Every column the verifier walks, declared once so the test suite diff --git a/backend/src/services/contract/signatureAssets.js b/backend/src/services/contract/signatureAssets.js index c6394336..1c8a2bf4 100644 --- a/backend/src/services/contract/signatureAssets.js +++ b/backend/src/services/contract/signatureAssets.js @@ -2,6 +2,7 @@ // module-level overview. Do not add behavior here without updating the entry re-exports. const crypto = require('crypto'); +const { getStoragePath } = require('../../config/storage'); const fs = require('fs'); const path = require('path'); const logger = require('../../utils/logger'); @@ -42,7 +43,7 @@ function sha256OfFile(filePath) { async function persistContractPdf(contract, buffer, suffix = '') { if (!contract.contract_number) return { filePath: null, sha256: null }; const year = (contract.issue_date ? new Date(contract.issue_date) : new Date()).getFullYear(); - const root = path.join(process.cwd(), 'storage', 'business-docs', 'contract', String(year)); + const root = path.join(getStoragePath(), 'business-docs', 'contract', String(year)); fs.mkdirSync(root, { recursive: true }); // Always append a millisecond timestamp to the filename so writes // never overwrite an earlier version on disk. Forensic preservation. @@ -92,8 +93,7 @@ async function persistSignatureImage(contract, role, dataUrl) { } const ext = match[1] === 'jpeg' ? 'jpg' : 'png'; const root = path.join( - process.cwd(), - 'storage', + getStoragePath(), 'business-docs', 'contract', 'signatures', @@ -194,7 +194,7 @@ async function persistAuditCertificate(contract) { try { const { buffer } = await pdfStampService.renderAuditCertificate(ctx); const year = (contract.issue_date ? new Date(contract.issue_date) : new Date()).getFullYear(); - const root = path.join(process.cwd(), 'storage', 'business-docs', 'contract', String(year)); + const root = path.join(getStoragePath(), 'business-docs', 'contract', String(year)); fs.mkdirSync(root, { recursive: true }); const stamp = new Date().toISOString().replace(/[:.]/g, '-'); const filePath = path.join(root, `${contract.contract_number}_audit_${stamp}.pdf`); diff --git a/backend/src/services/invoice/reminders.js b/backend/src/services/invoice/reminders.js index 3bf2edd4..0f4042f6 100644 --- a/backend/src/services/invoice/reminders.js +++ b/backend/src/services/invoice/reminders.js @@ -2,6 +2,7 @@ // module-level overview. Do not add behavior here without updating the entry re-exports. const { db, logActivity } = require('../../database/db'); +const { getStoragePath } = require('../../config/storage'); const { getAppSetting } = require('../../utils/appSettings'); const { AppError } = require('../../utils/errors'); const { formatShortDate } = require('../../utils/dateFormatter'); @@ -132,7 +133,7 @@ async function applyReminder(invoice, lineItems, level, adminId) { const fs = require('fs'); const path = require('path'); const year = new Date(fresh.issue_date).getFullYear(); - const root = path.join(process.cwd(), 'storage', 'business-docs', 'mahnung', String(year)); + const root = path.join(getStoragePath(), 'business-docs', 'mahnung', String(year)); fs.mkdirSync(root, { recursive: true }); const mahnungPath = path.join(root, `${fresh.invoice_number}_mahnung_L${level}.pdf`); fs.writeFileSync(mahnungPath, buffer); diff --git a/backend/src/services/invoice/sending.js b/backend/src/services/invoice/sending.js index d1055fa4..4c213d1c 100644 --- a/backend/src/services/invoice/sending.js +++ b/backend/src/services/invoice/sending.js @@ -2,6 +2,7 @@ // module-level overview. Do not add behavior here without updating the entry re-exports. const crypto = require('crypto'); +const { getStoragePath } = require('../../config/storage'); const { db, logActivity } = require('../../database/db'); const logger = require('../../utils/logger'); const { AppError } = require('../../utils/errors'); @@ -107,7 +108,7 @@ async function sendInvoice(id, adminId) { const fs = require('fs'); const path = require('path'); const year = new Date(invoice.issue_date).getFullYear(); - const root = path.join(process.cwd(), 'storage', 'business-docs', 'invoice', String(year)); + const root = path.join(getStoragePath(), 'business-docs', 'invoice', String(year)); fs.mkdirSync(root, { recursive: true }); const pdfPath = path.join(root, `${invoice.invoice_number}.pdf`); fs.writeFileSync(pdfPath, buffer); @@ -345,7 +346,7 @@ async function sendStorno(stornoId, adminId) { const fs = require('fs'); const path = require('path'); const year = new Date(storno.issue_date).getFullYear(); - const root = path.join(process.cwd(), 'storage', 'business-docs', 'invoice', String(year)); + const root = path.join(getStoragePath(), 'business-docs', 'invoice', String(year)); fs.mkdirSync(root, { recursive: true }); const pdfPath = path.join(root, `${storno.invoice_number}.pdf`); fs.writeFileSync(pdfPath, buffer); diff --git a/backend/src/services/pdfService.js b/backend/src/services/pdfService.js index d55c198e..9cbfaf3a 100644 --- a/backend/src/services/pdfService.js +++ b/backend/src/services/pdfService.js @@ -26,6 +26,7 @@ */ const PDFDocument = require('pdfkit'); +const { getStoragePath } = require('../config/storage'); const { SwissQRBill, Table } = require('swissqrbill/pdf'); const { t } = require('./pdf-i18n'); @@ -1349,8 +1350,16 @@ function registerCustomFonts(doc, issuer) { if (issuer.pdfFontTtfPath) { try { const raw = issuer.pdfFontTtfPath; + // The configured storage root first; process.cwd()/storage stays on as a + // legacy fallback so installs predating STORAGE_PATH keep resolving. + // Compose makes the two the same directory, which is why only a custom + // STORAGE_PATH ever exposed this — the font just silently was not found + // and the document fell back to the built-in face. + const storageRoot = getStoragePath(); const candidates = [ path.isAbsolute(raw) ? raw : null, + path.join(storageRoot, raw.replace(/^\/+/, '')), + path.join(storageRoot, 'fonts', path.basename(raw)), path.join(process.cwd(), 'storage', raw.replace(/^\/+/, '')), path.join(process.cwd(), 'storage', 'fonts', path.basename(raw)), ].filter(Boolean); diff --git a/backend/src/services/quoteService.js b/backend/src/services/quoteService.js index fdc8f66d..2ec91073 100644 --- a/backend/src/services/quoteService.js +++ b/backend/src/services/quoteService.js @@ -27,6 +27,7 @@ */ const crypto = require('crypto'); +const { getStoragePath } = require('../config/storage'); const { db, withRetry, logActivity } = require('../database/db'); const logger = require('../utils/logger'); const { getAppSetting } = require('../utils/appSettings'); @@ -1083,7 +1084,7 @@ async function persistDocPdf(type, doc, buffer) { const number = doc.quote_number || doc.invoice_number; if (!number) return null; const year = (doc.issue_date ? new Date(doc.issue_date) : new Date()).getFullYear(); - const root = path.join(process.cwd(), 'storage', 'business-docs', type, String(year)); + const root = path.join(getStoragePath(), 'business-docs', type, String(year)); fs.mkdirSync(root, { recursive: true }); const filePath = path.join(root, `${number}.pdf`); fs.writeFileSync(filePath, buffer); diff --git a/backend/src/utils/safePath.js b/backend/src/utils/safePath.js index a90553bf..a8b32dd9 100644 --- a/backend/src/utils/safePath.js +++ b/backend/src/utils/safePath.js @@ -35,10 +35,15 @@ * * **What the contract surface uses** * - * Two roots: - * 1. `/storage/business-docs/contract//` — system-stamped - * PDFs (immutable as-sent + signed copies). - * 2. `/uploads/contracts/signed/` — + * Three roots: + * 1. `/business-docs/contract/` — system-stamped PDFs + * (immutable as-sent + signed copies) and the signature images + * below them. This is where the writers persist. + * 2. `/storage/business-docs/contract/` — the same tree as written + * before the writers moved onto the shared storage resolver. Kept so + * pre-existing rows, whose absolute paths are in the database, still + * resolve; identical to (1) on a stock compose install. + * 3. `/uploads/contracts/signed/` — * wet-upload PDFs (admin or customer-supplied). * * Both roots are constants from the operator's perspective; legitimate @@ -48,6 +53,7 @@ const fs = require('fs'); const path = require('path'); const { AppError } = require('./errors'); +const { getStoragePath } = require('../config/storage'); /** * Resolve the canonical (symlink-followed) absolute path. Throws @@ -111,8 +117,22 @@ function assertPathInside(filePath, allowedRoots) { */ function assertContractPdfPath(filePath) { const cwd = process.cwd(); - const storageRoot = process.env.STORAGE_PATH || path.join(cwd, 'storage'); + // getStoragePath() rather than a second `STORAGE_PATH || cwd` expression: + // the two disagree whenever STORAGE_PATH is unset, because the shared + // resolver falls back module-relative (/storage) while this file used + // to fall back to /storage — and the backend is normally started from + // backend/, so those are different directories. The writers use the shared + // resolver, so a guard with its own idea of the root refuses exactly the + // files it is meant to serve. + const storageRoot = getStoragePath(); return assertPathInside(filePath, [ + // The configured storage root is where the contract writers persist, so it + // has to be allowed here or every generated PDF is refused with + // PATH_OUTSIDE_STORAGE the moment STORAGE_PATH is not /storage. The + // cwd root stays alongside it: contracts written before the writers moved + // still live there, and their absolute paths are recorded in the database. + // Both collapse to the same directory on a stock compose install. + path.join(storageRoot, 'business-docs', 'contract'), path.join(cwd, 'storage', 'business-docs', 'contract'), path.join(storageRoot, 'uploads', 'contracts', 'signed'), ]);