Merge branch 'beta' into fix/email-normalization-574

Resolves a conflict with the CRM merge (#555) that landed on beta
between when this branch was cut and now.

Two conflict regions in backend/src/routes/adminCustomers.js:

1. **Require block** — both branches added new requires after
   customerAccountsService. Kept both: this branch's
   emailNormalization import AND beta's customerHoursService +
   invoiceService imports (the CRM merge added the hours-billing +
   invoice-creation paths to this router).

2. **Edit-customer validators** — both branches changed the same set
   of body() validators in the PUT /:id handler. This branch added
   the IDENTITY_PRESERVING_NORMALIZE_EMAIL options arg to
   normalizeEmail; beta changed every body() to optional({ nullable:
   true }) so passive-customer records that store nulls for missing
   profile fields don't reject on save. Kept both: the nullable
   pattern from beta + the email-normalization options from this
   branch. Preserved beta's explanatory comment about the nullable
   choice.

Also patched one NEW normalizeEmail site the CRM merge introduced:

- backend/src/routes/adminCustomers.js:231 — POST /admin/customers
  now exists (CRM-era customer-create endpoint). Same options arg
  applied.

backend/src/routes/adminBusinessProfile.js has an isEmail() WITHOUT
normalizeEmail() on the issuer email — intentional (no normalization
means no risk of the Gmail dot-strip bug for that field), no change
needed.

All 18 normalizeEmail sites now pass IDENTITY_PRESERVING_NORMALIZE_EMAIL.
7/7 regression tests still pass. Lint clean on the merged file.
This commit is contained in:
Paul Nothaft
2026-05-29 21:48:42 +02:00
222 changed files with 48878 additions and 788 deletions
+38
View File
@@ -0,0 +1,38 @@
/**
* Small helper to read app_settings rows.
*
* The picpeak codebase has TWO settings services:
* - `src/services/settingsService.js` queries a `settings` table that
* doesn't actually exist on most deployments (legacy SQLite-era
* name). Calling getSetting() from there raises
* "relation \"settings\" does not exist" on Postgres.
* - The canonical store is `app_settings`, accessed inline by every
* other service (shareLinkService, customerAccountsService,
* authSecurity, dateFormatter, …).
*
* The CRM services use this helper instead of settingsService so the
* crm_* keys seeded by migration 102 are actually readable.
*/
const { db } = require('../database/db');
/**
* Read a single app_settings row by key. Returns the parsed value or
* `defaultValue` when the key doesn't exist.
*
* `setting_value` is always JSON-stringified at write time
* (see migration 102 + the /admin/settings/general route), so we
* JSON.parse on the way out. Falls back to the raw string on
* malformed JSON so legacy text values still work.
*/
async function getAppSetting(key, defaultValue = null) {
const row = await db('app_settings').where({ setting_key: key }).first();
if (!row || row.setting_value == null) return defaultValue;
try {
return JSON.parse(row.setting_value);
} catch (_) {
return row.setting_value;
}
}
module.exports = { getAppSetting };
+35
View File
@@ -0,0 +1,35 @@
/**
* clientIp — resolve the originating client IP for audit-trail
* recording (contract signing, quote responses, payment-check
* actions, etc.).
*
* **Why this helper exists:** the public-facing routes used to read
* `req.headers['x-forwarded-for']` directly and take the first
* comma-segment as the source IP. That bypasses Express's `trust
* proxy` safety net entirely — any direct (non-proxied) POST to the
* signing endpoint can spoof the audit IP by setting the header,
* which defeats the legal-evidence promise of the contract feature.
*
* **Correct path:** trust ONLY `req.ip`, and rely on
* `app.set('trust proxy', ...)` in `server.js` to populate it
* correctly. Express's trust-proxy machinery is the only thing that
* knows which upstream hops are trustworthy. The default in
* `server.js` (`'loopback, linklocal, uniquelocal'`) is correct for
* picpeak's standard deployment (nginx in front, Docker network);
* operators with unusual topologies override via `TRUST_PROXY` env.
*
* **Returns:** the resolved IPv4/IPv6 string, or `null` when Express
* couldn't determine one (very rare — happens with abusive raw
* sockets / malformed connections).
*
* **Storage:** call sites still gate persistence on a separate
* privacy setting (e.g. `crm_contracts_store_ip`). This helper only
* concerns itself with *which* IP to record, not *whether* to
* record one.
*/
function clientIpForAudit(req) {
if (!req) return null;
return req.ip || null;
}
module.exports = { clientIpForAudit };
+22 -1
View File
@@ -97,6 +97,27 @@ async function formatDate(date, language = 'en') {
}
}
/**
* Sync DD.MM.YYYY formatter used by quote / invoice / contract render
* contexts. Unlike `formatDate` above, this never consults app_settings
* — it's intended for fixed-format use inside templates already rendered
* for a specific document type. Three services used to ship a local
* copy each; this is the single source.
*
* - falsy input → empty string (template's {{#if ...}} block hides)
* - invalid date → the original value coerced to String (defensive
* passthrough; matches the prior behaviour of the three local copies)
*/
function formatShortDate(value) {
if (!value) return '';
const d = value instanceof Date ? value : new Date(value);
if (Number.isNaN(d.getTime())) return String(value);
const dd = String(d.getDate()).padStart(2, '0');
const mm = String(d.getMonth() + 1).padStart(2, '0');
return `${dd}.${mm}.${d.getFullYear()}`;
}
module.exports = {
formatDate
formatDate,
formatShortDate,
};
+93
View File
@@ -0,0 +1,93 @@
/**
* documentSequences — atomic gap-free sequence generator for CRM
* document numbers (invoices, quotes, contracts, future doc kinds).
*
* **Contract**: `claimNextSequence(kind, year, [trx])` returns the
* next integer in the (kind, year) series. Atomic against concurrent
* callers: two simultaneous claims for the same row return strictly
* increasing values, no collisions, no gaps.
*
* **How the atomicity works**
*
* Postgres: a single `UPDATE … SET current_value = current_value + 1
* WHERE kind = ? AND year = ? RETURNING current_value` holds a row
* lock for the duration of the statement; concurrent callers
* serialize on the lock.
*
* SQLite: knex does not expose `BEGIN IMMEDIATE` declaratively, but
* SQLite's default journal mode (or WAL) gives us per-row serialization
* via the transaction. We wrap the UPDATE + re-SELECT in a transaction
* which acquires the write lock; concurrent transactions queue.
*
* **First-claim path** (no row yet for the (kind, year))
*
* Migration 132 seeded rows for every existing year via MAX(...)
* backfill. New years need an INSERT on first use. We do an
* INSERT-OR-IGNORE then UPDATE … RETURNING. Both steps are inside
* the same transaction so the year row is guaranteed to exist when
* the UPDATE fires.
*/
const { db } = require('../database/db');
const { AppError } = require('./errors');
/**
* Claim the next sequence value for (kind, year). Returns the new
* integer. Throws AppError on DB failure; caller composes the
* formatted document number from this integer via formatNumberInTemplate.
*
* @param {string} kind 'invoice' | 'quote' | 'contract' | ...
* @param {number} year 4-digit year
* @param {object} [trx] optional knex transaction. When supplied the
* claim joins the caller's transaction so the
* sequence increment and the row INSERT can
* commit-or-roll-back together. Otherwise we
* run our own micro-transaction.
*/
async function claimNextSequence(kind, year, trx) {
if (!kind || typeof kind !== 'string') {
throw new AppError('claimNextSequence: kind required', 500);
}
const yr = parseInt(year, 10);
if (!Number.isFinite(yr)) {
throw new AppError('claimNextSequence: invalid year', 500);
}
const exec = async (q) => {
// Step 1: ensure the (kind, year) row exists. INSERT...ON CONFLICT
// DO NOTHING is the Postgres-native form; SQLite supports the same
// syntax (3.24+). knex's `onConflict('...').ignore()` paves over
// the differences.
await q('document_sequences')
.insert({
kind, year: yr, current_value: 0,
created_at: new Date(), updated_at: new Date(),
})
.onConflict(['kind', 'year']).ignore();
// Step 2: atomic claim. We do UPDATE … (no RETURNING because
// knex's returning() is uneven across drivers) then re-SELECT.
// The transaction wrapper (or the caller's trx) keeps the two
// statements on the same row lock, so concurrent claimers
// serialize.
await q('document_sequences')
.where({ kind, year: yr })
.increment('current_value', 1)
.update({ updated_at: new Date() });
const row = await q('document_sequences')
.where({ kind, year: yr })
.select('current_value')
.first();
if (!row) {
throw new AppError(`claimNextSequence: row vanished for ${kind}/${yr}`, 500);
}
return row.current_value;
};
if (trx) {
return await exec(trx);
}
return await db.transaction(async (innerTrx) => exec(innerTrx));
}
module.exports = { claimNextSequence };
+134
View File
@@ -0,0 +1,134 @@
/**
* IBAN validation per ISO 13616.
*
* Three checks:
* 1. Format — 2 uppercase letters (country) + 2 digits (check) +
* alphanumeric BBAN.
* 2. Length — each country fixes a total IBAN length. We accept any
* country whose ISO code we recognise; unknown country codes
* fall back to a generic 1534 char range (ISO 13616 caps every
* IBAN at 34 chars).
* 3. Mod-97 checksum — rearrange the IBAN so the first four chars
* land at the end, expand letters to digits (A=10..Z=35), the
* result modulo 97 MUST equal 1. Catches single-digit typos and
* digit transpositions with high probability.
*
* The validator is pure (no IO, no DB, no network) and returns a
* structured result so callers can surface a precise reason to the
* user.
*
* What this does NOT do:
* - Confirm the bank itself exists (would require an external
* directory or a bank-routing API — out of scope here).
* - Validate the BBAN's internal structure beyond length + charset
* (country-specific BBAN rules are not enforced).
*
* Usage:
* const { valid, normalized, reason } = validateIban(' ch 93 0076 2011 6238 5295 7 ');
* if (!valid) throw new Error(reason);
* // normalized === 'CH9300762011623852957'
*/
// IBAN length per ISO country code (ISO 13616, public registry).
// Anything not in this table falls through to the 1534 range check.
// Source: SWIFT IBAN Registry. Update when new countries are added.
const IBAN_LENGTHS = {
AD: 24, AE: 23, AL: 28, AT: 20, AZ: 28,
BA: 20, BE: 16, BG: 22, BH: 22, BR: 29, BY: 28,
CH: 21, CR: 22, CY: 28, CZ: 24,
DE: 22, DK: 18, DO: 28,
EE: 20, EG: 29, ES: 24,
FI: 18, FO: 18, FR: 27,
GB: 22, GE: 22, GI: 23, GL: 18, GR: 27, GT: 28,
HR: 21, HU: 28,
IE: 22, IL: 23, IQ: 23, IS: 26, IT: 27,
JO: 30,
KW: 30, KZ: 20,
LB: 28, LC: 32, LI: 21, LT: 20, LU: 20, LV: 21, LY: 25,
MC: 27, MD: 24, ME: 22, MK: 19, MR: 27, MT: 31, MU: 30,
NL: 18, NO: 15,
PK: 24, PL: 28, PS: 29, PT: 25,
QA: 29,
RO: 24, RS: 22,
SA: 24, SC: 31, SE: 24, SI: 19, SK: 24, SM: 27, ST: 25, SV: 28,
TL: 23, TN: 24, TR: 26,
UA: 29,
VA: 22, VG: 24,
XK: 20,
};
/**
* Rearrange + numerify the IBAN per ISO 13616 then take mod 97.
* The whole-string-as-BigInt approach is acceptable here: max IBAN
* length is 34 chars → numerified length is at most ~68 digits.
* Native BigInt is plenty fast for one-off validation.
*/
function mod97(iban) {
const rearranged = iban.slice(4) + iban.slice(0, 4);
let expanded = '';
for (const ch of rearranged) {
if (ch >= '0' && ch <= '9') {
expanded += ch;
} else if (ch >= 'A' && ch <= 'Z') {
// A=10, B=11, ..., Z=35
expanded += String(ch.charCodeAt(0) - 55);
} else {
return -1; // invalid char — caller treats as failed checksum
}
}
// Standard chunked mod-97 to avoid BigInt allocation cost.
let remainder = 0;
for (const digit of expanded) {
remainder = (remainder * 10 + Number(digit)) % 97;
}
return remainder;
}
/**
* Normalise + validate an IBAN string.
*
* @param {unknown} input Raw user-typed value. Spaces and lowercase
* letters are tolerated and stripped/uppercased
* before checking.
* @returns {{
* valid: boolean,
* normalized: string, // empty when input wasn't a string
* reason?: 'EMPTY' // nothing useful supplied
* | 'FORMAT' // failed the structural regex
* | 'LENGTH' // wrong length for the country
* | 'CHECKSUM' // mod-97 didn't equal 1
* }}
*/
function validateIban(input) {
if (input == null) return { valid: false, normalized: '', reason: 'EMPTY' };
const raw = String(input).replace(/\s+/g, '').toUpperCase();
if (!raw) return { valid: false, normalized: '', reason: 'EMPTY' };
// ISO 13616: starts with 2 letters (country) + 2 digits (check) +
// 11..30 chars of alphanumeric BBAN. Total length 15..34.
if (!/^[A-Z]{2}\d{2}[A-Z0-9]{11,30}$/.test(raw)) {
return { valid: false, normalized: raw, reason: 'FORMAT' };
}
const country = raw.slice(0, 2);
const expectedLen = IBAN_LENGTHS[country];
if (expectedLen != null) {
if (raw.length !== expectedLen) {
return { valid: false, normalized: raw, reason: 'LENGTH' };
}
} else if (raw.length < 15 || raw.length > 34) {
return { valid: false, normalized: raw, reason: 'LENGTH' };
}
if (mod97(raw) !== 1) {
return { valid: false, normalized: raw, reason: 'CHECKSUM' };
}
return { valid: true, normalized: raw };
}
module.exports = {
validateIban,
// Exposed for unit tests.
_internal: { mod97, IBAN_LENGTHS },
};
+34
View File
@@ -0,0 +1,34 @@
/**
* Shared numeric coercion helpers used across the CRM services.
*
* Previously: 4 copies of `ensureInt` + 2 copies of `ensureNumber` lived
* across quoteService, invoiceService, contractService, and
* taxReportService. Each copy was identical apart from `Number.isFinite`
* vs `!Number.isNaN` — converging on the same answer in practice
* because `parseInt`/`Number` never produce `Infinity` from string input.
*
* One canonical pair lives here so future numeric coercion concerns
* (e.g. BigInt safety, locale-aware decimals) are addressed in one place.
*/
/**
* Coerce a value to a non-NaN integer, defaulting to 0 on garbage.
* Matches the legacy `ensureInt` semantics across all four services.
*/
function ensureInt(value) {
const n = parseInt(value, 10);
return Number.isFinite(n) ? n : 0;
}
/**
* Coerce a value to a finite Number, defaulting to `fallback` (0) on
* null/undefined/empty string/NaN. Matches the legacy `ensureNumber`
* shape used by quote + invoice line-item math.
*/
function ensureNumber(value, fallback = 0) {
if (value === null || value === undefined || value === '') return fallback;
const n = Number(value);
return Number.isFinite(n) ? n : fallback;
}
module.exports = { ensureInt, ensureNumber };
+79
View File
@@ -0,0 +1,79 @@
/**
* Build a consistent filesystem-safe filename for quote / invoice
* PDFs. Format:
*
* <docNumber>_<customerLabel>.pdf
*
* - docNumber: the invoice/quote number as printed
* - customerLabel: customer.company_name || full person name ||
* display_name || email-local-part || 'customer'
*
* Both segments are sanitised: spaces → '-', non-ASCII letters
* preserved, slashes/colons/quotes stripped, length capped so the
* combined filename stays under the typical 255-byte filesystem
* limit (we cap each side at 80 chars, which is generous for both
* pieces).
*
* Used by:
* - Content-Disposition headers on every admin + customer PDF
* endpoint
* - The PDF's internal `Title` metadata (Chrome's PDF viewer
* uses this as the default name when saving from a blob URL,
* where Content-Disposition can't reach)
*/
function sanitiseSegment(input, maxLen = 80) {
if (!input) return '';
let s = String(input).trim();
// Replace OS-hostile characters with '-'.
s = s.replace(/[/\\:*?"<>|]+/g, '-');
// Collapse whitespace runs into a single '-'.
s = s.replace(/\s+/g, '-');
// Collapse repeat dashes.
s = s.replace(/-+/g, '-');
// Trim leading/trailing dashes + dots.
s = s.replace(/^[-.]+|[-.]+$/g, '');
if (s.length > maxLen) s = s.slice(0, maxLen);
return s;
}
/**
* Resolve a label representing the customer for the filename. Tries
* company name first (most useful for filing), then full person
* name, then display name, then the email's local part, finally
* 'customer' as a generic fallback.
*
* @param {object} customer customer_accounts row (snake_case)
* @returns {string} sanitised label segment
*/
function customerLabel(customer) {
if (!customer) return 'customer';
const company = (customer.company_name || '').trim();
if (company) return sanitiseSegment(company);
const fullName = [customer.first_name, customer.last_name]
.map((v) => (v || '').trim()).filter(Boolean).join(' ');
if (fullName) return sanitiseSegment(fullName);
const display = (customer.display_name || '').trim();
if (display) return sanitiseSegment(display);
const email = (customer.email || '').trim();
if (email) return sanitiseSegment(email.split('@')[0]);
return 'customer';
}
/**
* Build the final filename. Always ends with `.pdf`. When the
* document number is missing (e.g. preview of an unsaved row),
* substitutes a sensible fallback.
*
* @param {object} args
* - docNumber: 'R-2026-0001' / 'Q-2026-0042' / null for previews
* - customer: customer_accounts row
* - fallback: prefix when docNumber is null ('invoice-preview' etc.)
*/
function buildPdfFilename({ docNumber, customer, fallback = 'document' }) {
const numberSeg = sanitiseSegment(docNumber) || sanitiseSegment(fallback) || 'document';
const custSeg = customerLabel(customer);
return `${numberSeg}_${custSeg}.pdf`;
}
module.exports = { buildPdfFilename, sanitiseSegment, customerLabel };
+157
View File
@@ -0,0 +1,157 @@
/**
* publicTokenGuards — shared validators for the public token tables
* (`contract_action_tokens`, `quote_action_tokens`). Centralises the
* checks that every public-facing route MUST run before doing work,
* so future routes can't accidentally skip a guard.
*
* What this enforces:
* 1. **Existence** — 404 when the token doesn't match a row.
* 2. **Expiry** — 410 when `expires_at` is in the past
* OR is NULL (defensive: NULL = expired,
* not "valid forever" — historical bug).
* 3. **One-shot semantics** — when `requireUnused: true`, 409 if
* `used_at` is already set. Prevents
* replay of leaked tokens on the upload
* path. The sign path historically allowed
* re-signing for in-browser flows; opt in
* per call site.
* 4. **Attempt throttling** — non-existent tokens increment a per-IP
* counter; the IP is locked out for 15 min
* after 20 invalid attempts. Mitigates the
* token-prefix brute force route that
* standard rate-limiters don't catch
* (large token space, low miss rate per
* IP, but distributed crawlers add up).
*
* Returns the validated token row on success. Sends the appropriate
* HTTP response and returns `null` on failure — the caller must check
* for null and `return` immediately.
*/
const { db } = require('../database/db');
const { clientIpForAudit } = require('./clientIp');
const logger = require('./logger');
// In-memory bad-attempt counter. Per-process; cleared on restart.
// Keyed by IP. Each entry: { count, firstAt }. We could persist this
// in app_settings or a dedicated table, but in-memory is simpler and
// good enough for the threat (distributed brute force is the only
// case where IP locking helps anyway, and that needs more than one
// IP to be effective).
const BAD_ATTEMPT_LIMIT = 20;
const BAD_ATTEMPT_WINDOW_MS = 15 * 60 * 1000;
const badAttempts = new Map();
function recordBadAttempt(ip) {
if (!ip) return;
const now = Date.now();
const entry = badAttempts.get(ip);
if (!entry || (now - entry.firstAt) > BAD_ATTEMPT_WINDOW_MS) {
badAttempts.set(ip, { count: 1, firstAt: now });
return;
}
entry.count += 1;
}
function isIpLocked(ip) {
if (!ip) return false;
const entry = badAttempts.get(ip);
if (!entry) return false;
if ((Date.now() - entry.firstAt) > BAD_ATTEMPT_WINDOW_MS) {
badAttempts.delete(ip);
return false;
}
return entry.count >= BAD_ATTEMPT_LIMIT;
}
/**
* Validate a public action token. Returns the token row on success,
* sends a response + returns null on failure.
*
* @param {object} req Express request (for IP)
* @param {object} res Express response (to send errors)
* @param {object} opts
* @param {string} opts.tableName 'contract_action_tokens' | 'quote_action_tokens'
* @param {string} opts.token 64-hex token string
* @param {boolean} [opts.requireUnused] refuse when used_at is set (default false)
*/
async function loadActionToken(req, res, opts) {
const { tableName, token, requireUnused = false } = opts;
const ip = clientIpForAudit(req);
if (isIpLocked(ip)) {
res.status(429).json({
error: 'Too many invalid token attempts. Try again in 15 minutes.',
code: 'TOKEN_LOOKUP_LOCKED',
});
return null;
}
const row = await db(tableName).where({ token }).first();
if (!row) {
recordBadAttempt(ip);
res.status(404).json({ error: 'Not found' });
return null;
}
// Defensive: NULL expires_at counts as expired. Historical bug —
// old seed rows could land without an expiry value, granting
// permanent unauthenticated access. We refuse rather than guess.
if (!row.expires_at) {
logger.warn('publicTokenGuards: token has NULL expires_at — refusing', {
tableName, tokenPrefix: token.slice(0, 12),
});
res.status(410).json({ error: 'This link has expired', code: 'TOKEN_NO_EXPIRY' });
return null;
}
if (new Date(row.expires_at).getTime() < Date.now()) {
res.status(410).json({ error: 'This link has expired', code: 'TOKEN_EXPIRED' });
return null;
}
if (requireUnused && row.used_at) {
res.status(409).json({
error: 'This link has already been used',
code: 'TOKEN_ALREADY_USED',
});
return null;
}
return row;
}
/**
* Pre-multer guard for upload routes. Runs the same validation as
* loadActionToken but DOES NOT mutate state — it just rejects bad
* tokens before multer reads the request body and writes to disk.
* Without this, a captured/expired token can DoS disk by spamming
* uploads that get rejected post-write.
*
* Wired in as middleware before `multer.single(...)`.
*/
function preMulterTokenGuard(tableName) {
return async (req, res, next) => {
try {
const token = req.params.token;
if (!token || !/^[a-f0-9]{64}$/i.test(token)) {
return res.status(400).json({ error: 'Invalid token format' });
}
const row = await loadActionToken(req, res, { tableName, token, requireUnused: true });
if (!row) return; // loadActionToken already responded
// Attach for downstream handler — saves a duplicate DB lookup.
req.publicTokenRow = row;
next();
} catch (err) {
logger.error('preMulterTokenGuard: unexpected error', { err: err.message });
return res.status(500).json({ error: 'Internal error' });
}
};
}
module.exports = {
loadActionToken,
preMulterTokenGuard,
// Exported for tests + future routes that need the same lock
// surface (e.g. payment-check actions).
_internal: { recordBadAttempt, isIpLocked, badAttempts },
};
+193
View File
@@ -0,0 +1,193 @@
/**
* resolveLogoFile — resolve a logo to an absolute existing file path.
*
* The CRM PDFs accept a logo from three different sources and each
* source may store the path in a different shape (absolute multer
* path, relative URL, bare filename, etc.). Rather than have the PDF
* renderer guess, we build an exhaustive candidate list HERE, return
* the first existing PNG/JPEG file, and log everything we tried when
* we come up empty.
*
* Why this lives in utils:
* - both invoiceService and quoteService need the same resolution
* - the renderer (pdfService) should NOT touch the DB or the
* filesystem-discovery rules; it just calls doc.image(path)
*
* Sources we accept (in priority order):
* 1. business_profile.logo_path — explicit per-CRM logo
* 2. app_settings.branding_logo_path — absolute multer path from
* Settings → Branding (preferred — already absolute)
* 3. app_settings.branding_logo_url — URL path from the same
* branding upload (fallback for older installs)
*
* For each source we try multiple candidate disk paths:
* - The raw value as an absolute path (if absolute)
* - STORAGE_PATH joined with the value (stripped of leading "/")
* - STORAGE_PATH/uploads/logos/<basename>
* - STORAGE_PATH/branding/<basename>
* - CWD/storage joined with the value (last-ditch for older
* docker-compose configs that didn't set STORAGE_PATH)
*
* Format filter:
* PDFKit only natively decodes PNG + JPEG. SVG/WebP/GIF/TIFF files
* are silently skipped here (with a warn log) so the rest of the
* PDF still renders rather than crashing on an unsupported format.
*/
const fs = require('fs');
const path = require('path');
const crypto = require('crypto');
const { getStoragePath } = require('../config/storage');
const { getAppSetting } = require('./appSettings');
const logger = require('./logger');
const SUPPORTED_EXT = /\.(png|jpe?g)$/i;
// Formats PDFKit can't embed directly but `sharp` can rasterise into
// PNG for us. We transparently convert + cache.
const CONVERTIBLE_EXT = /\.(svg|webp|gif|tif|tiff|avif|heif|heic)$/i;
function generateCandidates(raw, storageRoot) {
const value = String(raw || '').trim();
if (!value) return [];
const stripped = value.replace(/^\/+/, '');
const baseName = path.basename(value);
// Build candidate set; dedup at the end so we don't stat the same
// file twice when the inputs overlap.
const candidates = [
path.isAbsolute(value) ? value : null,
path.join(storageRoot, stripped),
path.join(storageRoot, 'uploads', 'logos', baseName),
path.join(storageRoot, 'branding', baseName),
path.join(process.cwd(), 'storage', stripped),
path.join(process.cwd(), 'storage', 'uploads', 'logos', baseName),
path.join(process.cwd(), 'storage', 'branding', baseName),
].filter(Boolean);
return [...new Set(candidates)];
}
function pickExisting(candidates) {
for (const c of candidates) {
try {
if (fs.existsSync(c) && fs.statSync(c).isFile()) return c;
} catch (_) { /* ignore */ }
}
return null;
}
/**
* Rasterise a non-PNG/JPEG source into PNG so PDFKit can embed it.
* The output is cached under STORAGE_PATH/cache/logo-png/, keyed by
* the source path + mtime + size — re-uploading the SVG invalidates
* the cache automatically without us having to clean up old entries.
*
* Returns the absolute cached PNG path on success, or null when
* `sharp` fails (corrupt SVG, unsupported feature inside the SVG,
* etc.). The caller logs + falls back to the name-only branch.
*/
async function rasteriseToPng(sourcePath, storageRoot) {
let sharp;
try {
sharp = require('sharp');
} catch (err) {
logger.warn('PDF logo rasterisation skipped — sharp not installed', { err: err.message });
return null;
}
try {
const stat = fs.statSync(sourcePath);
const cacheDir = path.join(storageRoot, 'cache', 'logo-png');
fs.mkdirSync(cacheDir, { recursive: true });
// Content-addressed cache: sha1(src path + mtime ns + size).
// Including mtime means re-uploading the source invalidates the
// cache entry naturally.
const key = crypto.createHash('sha1')
.update(`${sourcePath}|${stat.mtimeMs}|${stat.size}`)
.digest('hex');
const cachedPath = path.join(cacheDir, `${key}.png`);
if (fs.existsSync(cachedPath)) {
return cachedPath;
}
// density: 384 gives a crisp render even when the SVG embeds at
// a small intrinsic size (PDFKit's `fit` will downscale, never
// upscale). 512px wide is more than enough for a letterhead
// logo.
await sharp(sourcePath, { density: 384 })
.resize({ width: 512, withoutEnlargement: false })
.png()
.toFile(cachedPath);
logger.info('PDF logo rasterised to PNG', { source: sourcePath, cached: cachedPath });
return cachedPath;
} catch (err) {
logger.warn('PDF logo rasterisation failed', {
source: sourcePath, err: err.message,
});
return null;
}
}
/**
* @param {object} profile the business_profile row (or null)
* @returns {Promise<string|null>} absolute path to a usable PNG/JPEG, or null
*/
async function resolveLogoFile(profile) {
const storageRoot = getStoragePath();
const raws = [];
const profileLogoPath = (profile?.logo_path || '').toString().trim();
if (profileLogoPath) raws.push({ source: 'business_profile.logo_path', value: profileLogoPath });
try {
const brandingDisk = await getAppSetting('branding_logo_path');
if (brandingDisk && typeof brandingDisk === 'string' && brandingDisk.trim()) {
raws.push({ source: 'branding_logo_path', value: brandingDisk.trim() });
}
} catch (_) { /* ignore */ }
try {
const brandingUrl = await getAppSetting('branding_logo_url');
if (brandingUrl && typeof brandingUrl === 'string' && brandingUrl.trim()) {
raws.push({ source: 'branding_logo_url', value: brandingUrl.trim() });
}
} catch (_) { /* ignore */ }
if (raws.length === 0) return null;
for (const { source, value } of raws) {
const candidates = generateCandidates(value, storageRoot);
const found = pickExisting(candidates);
if (!found) continue;
if (SUPPORTED_EXT.test(found)) {
logger.info('Resolved PDF logo', { source, configured: value, resolved: found });
return found;
}
if (CONVERTIBLE_EXT.test(found)) {
// SVG / WebP / GIF / TIFF / AVIF — rasterise to PNG so PDFKit
// can embed it. Cached under STORAGE_PATH/cache/logo-png/ so
// repeated PDF renders don't re-encode the same source.
const rasterised = await rasteriseToPng(found, storageRoot);
if (rasterised) {
logger.info('Resolved PDF logo via rasterisation', {
source, configured: value, original: found, resolved: rasterised,
});
return rasterised;
}
logger.warn('PDF logo rasterisation produced no file; trying next source', {
source, configured: value, found,
});
continue;
}
// Unknown extension — try anyway, PDFKit may still accept it.
logger.warn('PDF logo has unusual extension; attempting to render as-is', {
source, configured: value, found,
});
return found;
}
logger.warn('PDF logo not found on disk after trying all sources', {
storageRoot,
sources: raws.map(({ source, value }) => ({
source, value, candidates: generateCandidates(value, storageRoot),
})),
});
return null;
}
module.exports = { resolveLogoFile };
+124
View File
@@ -0,0 +1,124 @@
/**
* safePath — path-containment helpers for the contract / quote / invoice
* PDF surfaces.
*
* **Why this exists**
*
* The audit (#25, #31) flagged that several routes pipe `fs.createReadStream`
* on a path read directly from the DB (`contracts.pdf_path`,
* `contracts.signed_pdf_path`) and that `attachSignedPdfUpload` accepts
* a route-supplied filePath with no containment assertion. The
* defence-in-depth concern: if a path ever got into the DB pointing
* outside the legitimate storage roots (via a future migration bug,
* a hand-edited row, or a SQL-injection regression elsewhere), the
* stream would happily read /etc/passwd or any other readable file
* for the requesting admin.
*
* Today the DB paths are written by the service layer and never
* accept caller input directly, so the practical exposure is low —
* but a 4-line containment check at the read boundary makes the
* invariant explicit and protects against future drift.
*
* **Approach**
*
* `assertPathInside(absoluteFilePath, allowedRoots)` resolves both
* sides to canonical absolute paths via `fs.realpathSync` and
* verifies the file path starts with one of the allowed root strings
* followed by a path separator (so /storage-evil/ doesn't pass when
* /storage/ is allowed). Throws `AppError 403` on violation.
*
* `realpathSync` resolves symlinks, defeating the obvious attack
* (symlink in storage root → /etc/passwd). It throws on missing
* files, which is fine — callers already exists-check before stream
* via `fs.existsSync`. We re-throw missing-file errors as
* AppError 404 to keep the response shape consistent.
*
* **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/` —
* wet-upload PDFs (admin or customer-supplied).
*
* Both roots are constants from the operator's perspective; legitimate
* paths always live under one of them.
*/
const fs = require('fs');
const path = require('path');
const { AppError } = require('./errors');
/**
* Resolve the canonical (symlink-followed) absolute path. Throws
* AppError 404 when the file is missing on disk; caller handles
* the 404 response.
*/
function realpathOr404(absPath) {
try {
return fs.realpathSync(absPath);
} catch (err) {
if (err && (err.code === 'ENOENT' || err.code === 'ENOTDIR')) {
throw new AppError('File missing on disk', 404, 'FILE_MISSING');
}
throw err;
}
}
/**
* Assert that `filePath` resolves to a location inside one of
* `allowedRoots`. Throws AppError 403 on violation.
*
* Both inputs are resolved through realpath so symlinks in either
* direction are followed before comparison. `allowedRoots` that
* don't themselves exist are silently dropped from the check (a
* deployment with both quote and contract roots may have the
* contract root missing on first boot, for example) — at least one
* root MUST exist for the check to allow the path.
*/
function assertPathInside(filePath, allowedRoots) {
if (!filePath) throw new AppError('No path provided', 400);
const resolvedFile = realpathOr404(filePath);
const resolvedRoots = [];
for (const root of allowedRoots) {
if (!root) continue;
try {
const r = fs.realpathSync(root);
// Append a separator so /storage/foo doesn't match /storage/foo-evil.
resolvedRoots.push(r.endsWith(path.sep) ? r : r + path.sep);
} catch (_) {
// Root doesn't exist yet — fall through. Next iteration may resolve.
}
}
if (resolvedRoots.length === 0) {
// Defensive: refuse rather than allowing free access when no root
// exists. Should only happen on a half-provisioned install.
throw new AppError('No allowed storage roots configured', 500, 'NO_STORAGE_ROOTS');
}
const ok = resolvedRoots.some((root) =>
resolvedFile === root.slice(0, -1) || resolvedFile.startsWith(root)
);
if (!ok) {
throw new AppError('Refusing to serve a file outside the storage roots', 403, 'PATH_OUTSIDE_STORAGE');
}
return resolvedFile;
}
/**
* Convenience helper that builds the standard contract PDF roots
* (system-stamped + wet-upload) and delegates to assertPathInside.
* Use from contract PDF stream / read sites.
*/
function assertContractPdfPath(filePath) {
const cwd = process.cwd();
const storageRoot = process.env.STORAGE_PATH || path.join(cwd, 'storage');
return assertPathInside(filePath, [
path.join(cwd, 'storage', 'business-docs', 'contract'),
path.join(storageRoot, 'uploads', 'contracts', 'signed'),
]);
}
module.exports = {
assertPathInside,
assertContractPdfPath,
};
+96
View File
@@ -0,0 +1,96 @@
/**
* schemaCache — process-local memoisation for `db.schema.hasColumn`.
*
* **Why this exists**
*
* The CRM services on `feat/crm` are riddled with `hasColumn` guards
* because the schema has been drifting fast (every doc-feature
* migration adds a column that older installs may not yet have).
* Each call hits information_schema (Postgres) or sqlite_master
* (SQLite). On hot paths — `recordCustomerSignature`,
* `recordAdminCountersignature`, `getContractById`, the monthly
* billing pass — we issue 48 hasColumn checks per request, all
* for columns whose presence cannot change at runtime.
*
* The audit flagged this as a perf medium. Caching is safe because:
*
* 1. Schema-changing operations (migrations, `ALTER TABLE`) only
* run at boot via `run-migrations-safe.js`, BEFORE any service
* module accepts traffic. The cache is populated lazily after
* boot finishes, so the entries reflect post-migration state.
*
* 2. The Node process is the only schema authority. There's no
* sibling process sneaking in `ALTER TABLE` while we serve
* requests.
*
* 3. If a future migration path needs to run mid-flight, it can
* call `invalidateSchemaCache()` after the schema change.
*
* **What we cache**
*
* Just the boolean answer to `(table, column)`. A miss means the
* column doesn't exist on this install; a hit means it does. The
* cache key is `${table}.${column}`. There's no TTL — the entry is
* valid for the lifetime of the Node process.
*
* **What we DON'T cache**
*
* Negative results from `hasTable` failures (the table simply isn't
* there) — those go through the underlying call each time. That
* scenario is exceptional (table truly missing during a half-applied
* migration window) and we want it to surface, not get masked by a
* stale cache.
*
* **API**
*
* const { hasColumnCached, invalidateSchemaCache } = require('../utils/schemaCache');
* if (await hasColumnCached('contracts', 'signed_pdf_render_failed_at')) {
* ...
* }
*
* Drop-in replacement for `db.schema.hasColumn(...)` calls. The
* existing helper signature returns a Promise<boolean> so async
* call-sites need no shape change.
*/
const { db } = require('../database/db');
const cache = new Map();
async function hasColumnCached(table, column) {
const key = `${table}.${column}`;
if (cache.has(key)) return cache.get(key);
// Resolve via the underlying schema API. We deliberately don't
// catch errors here — if the call throws (e.g. DB connection lost
// mid-boot), the error surfaces to the caller exactly as it would
// have without the cache.
const present = await db.schema.hasColumn(table, column);
cache.set(key, present);
return present;
}
/**
* Drop every cached entry. Call this after a runtime schema change
* (rare — only the dev tooling does this today). Safe to call any
* time; the next hasColumnCached lookup will re-resolve.
*/
function invalidateSchemaCache() {
cache.clear();
}
/**
* Drop entries for a single table. Useful when only one table was
* altered and other tables' caches are still valid.
*/
function invalidateSchemaCacheForTable(table) {
const prefix = `${table}.`;
for (const k of cache.keys()) {
if (k.startsWith(prefix)) cache.delete(k);
}
}
module.exports = {
hasColumnCached,
invalidateSchemaCache,
invalidateSchemaCacheForTable,
};