fix(storage): write business documents under STORAGE_PATH, not the cwd (#1070)

* fix(storage): write business documents under STORAGE_PATH, not the cwd

persistDocPdf, the invoice sending and reminder writers and both contract
signature writers built their target from
`path.join(process.cwd(), 'storage', 'business-docs', ...)` and never
consulted STORAGE_PATH. docker-compose.yml and
docker-compose.production.yml both pin STORAGE_PATH=/app/storage and the
image's WORKDIR is /app, so on a stock deployment the two expressions
name the same directory and nothing looked wrong.

Point STORAGE_PATH anywhere else and quotes, invoices, Mahnungen and
contract PDFs land outside the configured storage root: missed by the
backup walker, invisible to the storage accounting, and gone when the
container is replaced. It also fails outright where the working
directory is not writable by the runtime user.

Routed all six writers through getStoragePath(), the resolver the rest
of the app already uses. Two read-side sites of the same class came
along: the custom PDF font lookup now checks the storage root before the
legacy cwd path (a font under STORAGE_PATH/fonts was simply never found,
and the document silently fell back to the built-in face), and the
dev-test scratch directory follows the same root.

Left alone deliberately: resolveLogoFile and adminBusinessProfile
already try both roots, so their cwd reference is a legacy fallback
rather than a miss.

No migration needed — the persisted path is stored absolute, so rows
written before this keep resolving to where those files actually are.

Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc

* fix(storage): allow the configured contract root, and move signature images too

Two holes in the previous commit, both found by review.

Contract downloads would have broken. assertContractPdfPath() guards the
admin unsigned/signed PDF routes and GET /api/public/contracts/:token/pdf,
and it listed only <cwd>/storage/business-docs/contract. Moving the
writers to STORAGE_PATH without moving that root meant every newly
generated contract was refused with PATH_OUTSIDE_STORAGE — a worse
failure than the bug being fixed, and only on the installs the fix was
for. The configured root is now allowed alongside the cwd one, which
stays for contracts written before the move; their absolute paths are in
the database and still resolve. Note the sibling root on the next line
already honoured STORAGE_PATH, so the helper was half-migrated already.

persistSignatureImage() still wrote customer and admin signature PNGs
under process.cwd(). It was missed because its path.join is spread over
seven lines while the others are single-line — and the regression test
compared against the single-line literal, so it reported green over a
live bug. The test now collapses whitespace before matching, which is
the only reason a formatting difference ever hid this. A sweep of the
whole of src/ with the same normalisation confirms the remaining
process.cwd()/storage references are all deliberate
`STORAGE_PATH || cwd` fallbacks, not misses.

Added a case that drives assertContractPdfPath against real files on
disk — the guard realpaths both the file and its roots, so a test using
imaginary paths proves nothing. It fails without the fix.

Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc

* fix(storage): resolve the contract guard's root through the shared resolver

The guard still built its own `STORAGE_PATH || <cwd>/storage`. That
matches getStoragePath() only while STORAGE_PATH is set — with it unset
the shared resolver falls back module-relative to <repo>/storage while
this fell back to <cwd>/storage, and the backend is normally started
from backend/, so the two name different directories. Writers and guard
then disagreed about where contracts live and the download routes
refused them, which is the same failure the previous commit fixed for
the configured case, reappearing in the fallback case.

One resolver on both sides now, which is the point of the whole change.
Docblock updated to describe the three roots as they actually are.

Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc

* fix(storage): make the fallback test safe, and align the backup diagnostics

The test added in the previous commit was dangerous. To exercise the
STORAGE_PATH-unset case it deleted process.env.STORAGE_PATH and then, in
cleanup, recursively removed `<resolved root>/business-docs` — which
with the variable unset resolves to the developer's real, gitignored
<repo>/storage. Running `npm test` in a working checkout would have
destroyed local business documents. This checkout has 65 MB there,
including a populated business-docs tree.

Rewritten to mock the shared resolver instead. That is both safe (every
path stays in the tmpdir) and a sharper assertion: if the guard consumes
getStoragePath() the mock moves its root, and if it went back to rolling
its own expression the mock would have no effect and the test fails —
which is exactly the regression being pinned.

backupCoverageService and backupIntegrityService kept their own
`STORAGE_PATH || cwd` roots. The backup walker itself already falls back
module-relative, so with the variable unset the two diagnostics
inspected a directory neither the walker nor the writers use and would
report the business-docs tree as missing while it was in fact being
backed up. Both now use the shared resolver.

No regression: the same jest invocation over contract/quote/invoice/pdf/
backup suites gives an identical 11 failed, 24 passed before and after —
those failures are a locally missing cron-parser dependency and
reproduce on an unmodified tree.

Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc

---------

Co-authored-by: Paul Nothaft <[email protected]>

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', {

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

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

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`);

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

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');
@@ -114,7 +115,7 @@ async function sendInvoice(id, adminId, options = {}) {
   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);
@@ -389,7 +390,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);

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');
 
@@ -1361,8 +1362,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);

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');
@@ -1065,7 +1066,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);

backend/src/utils/safePath.js

@@ -35,10 +35,15 @@
  *
  * **What the contract surface uses**
  *
- * Two roots:
- *   1. `<cwd>/storage/business-docs/contract/<year>/` — system-stamped
- *      PDFs (immutable as-sent + signed copies).
- *   2. `<STORAGE_PATH or cwd/storage>/uploads/contracts/signed/` —
+ * Three roots:
+ *   1. `<storage root>/business-docs/contract/` — system-stamped PDFs
+ *      (immutable as-sent + signed copies) and the signature images
+ *      below them. This is where the writers persist.
+ *   2. `<cwd>/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. `<storage root>/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 (<repo>/storage) while this file used
+  // to fall back to <cwd>/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 <cwd>/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'),
   ]);