fix(security): bound inbound-mail resources, redact secrets from logs (GHSA-2qf9, pgmp, r794) (#959)

* fix(security): bound inbound-mail resources, redact secrets from logs (GHSA-2qf9, pgmp, r794)

GHSA-2qf9 — emailIntakeService downloaded, parsed and persisted every message
with no size, attachment-count or attachment-byte limit, reachable
unauthenticated by anyone who can email the operator's mailbox:
- fetch the envelope with `size` (same cheap pass) and refuse an oversized
  message BEFORE downloading its source;
- cap attachment count and cumulative attachment bytes;
- limits env-overridable, defaults generous for real supplier invoices.

The teeth were in the dedup key. received_emails.message_id is varchar(512)
UNIQUE, and the failure path wrote `err-<uid>-<Date.now()>`, which can never
match the envelope-derived messageId the dedup pass compares against — so an
oversized (or overlong-Message-ID) mail was re-downloaded every poll forever,
and an OOM-kill/restart just resumed the loop. Size-skips are now recorded
under the REAL message id, and overlong ids collapse to a stable sha256 key
that always fits the column.

GHSA-pgmp / r794 — new sanitizeForLog() util (key-name deny-set, recursive,
cycle-safe) applied to the three request-body log sites in adminEvents/crud.js,
plus sanitizeValidationErrors() because express-validator's errors.array()
embeds the SUBMITTED value per field — a rejected plaintext password was still
logged. Scope is wider than filed: the update path also logged
client_password_hash and a LIVE client_share_token bearer credential.

Also: the one-time setup token was logged at warn AND printed to stdout on
every first boot, putting a live first-admin credential in combined.log,
security.log and `docker logs`. It is now written to the 0600 token file and
only surfaced when that write fails — the last-resort path it existed for.

* fix(security): codex round 3 — repair the first-run token recovery flow (GHSA-r794)

Two regressions from keeping the setup token out of the logs.

1. server.js decided whether to print the token by calling existsSync() on the
   candidate path. That answers a different question than "did the write
   succeed": a stale, read-only or directory-shaped SETUP_TOKEN reports as
   present, so the banner suppressed the live token and pointed the operator at
   content that is not it — leaving the current token only in combined.log
   under default production logging. setupService now records the path the
   write actually produced and exposes it via writtenSetupTokenFile().

2. The setup screen, its EN/DE strings, README, SIMPLE_SETUP and .env.example
   all still told first-time users to run
   `docker compose logs backend | grep -i "setup token"`. On the normal path
   that command now returns a path banner and no credential, so the documented
   browser-first onboarding could not be completed. They now point at
   `docker compose exec backend cat /app/data/SETUP_TOKEN`, with the log
   fallback described as what it is — the failure path.

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

---------

Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Paul Nothaft
2026-08-02 21:17:11 +02:00
committed by GitHub
co-authored by Paul Nothaft
parent da855cfef9
commit 1b4e5fee3e
13 changed files with 479 additions and 29 deletions
+76 -4
View File
@@ -20,6 +20,31 @@ const sanitizeHtml = require('sanitize-html');
const { isUniqueViolation } = require('../utils/dbErrors');
const ALLOWED_MIME = ['application/pdf', 'image/jpeg', 'image/png'];
// Resource caps for inbound mail (GHSA-2qf9). Anyone who can email the
// operator's mailbox reaches this code path unauthenticated, and nothing here
// used to bound message size, attachment count or attachment bytes. Defaults
// are generous for real supplier invoices; all three are env-overridable.
const numFromEnv = (name, fallback) => {
const n = Number(process.env[name]);
return Number.isFinite(n) && n > 0 ? n : fallback;
};
const MAX_MESSAGE_BYTES = numFromEnv('EMAIL_INTAKE_MAX_MESSAGE_BYTES', 25 * 1024 * 1024);
// received_emails.message_id is varchar(512) WITH a UNIQUE constraint. A sender
// can legally emit a Message-ID longer than that; the insert then throws, the
// catch path stores a synthetic err-<uid>-<now> key that can never match the
// dedup pass, and every poll re-downloads and re-parses the same message
// forever. Collapse anything overlong to a stable hash so the key always fits
// and always reproduces (GHSA-2qf9).
const MESSAGE_ID_MAX = 512;
const boundedMessageId = (raw, fallback) => {
const value = String(raw || fallback || '').trim() || String(fallback || '');
if (value.length <= MESSAGE_ID_MAX) return value;
return `sha256:${require('crypto').createHash('sha256').update(value).digest('hex')}`;
};
const MAX_ATTACHMENTS = numFromEnv('EMAIL_INTAKE_MAX_ATTACHMENTS', 25);
const MAX_ATTACHMENT_BYTES = numFromEnv('EMAIL_INTAKE_MAX_ATTACHMENT_BYTES', 25 * 1024 * 1024);
let polling = false;
// Fail fast instead of hanging on a wrong host/port (e.g. IMAP pointed at an
@@ -280,8 +305,17 @@ async function pollAccountOnce(cfg, { accountKey = 'accounting', routeToExpenses
const candidates = [];
if (uids.length) {
// eslint-disable-next-line no-restricted-syntax
for await (const m of client.fetch(uids, { uid: true, envelope: true }, { uid: true })) {
candidates.push({ uid: m.uid, messageId: (m.envelope && m.envelope.messageId) || `uid-${cfg.folder}-${m.uid}` });
// `size` rides along in the same cheap envelope pass, so an oversized
// message can be rejected BEFORE its source is downloaded (GHSA-2qf9).
for await (const m of client.fetch(uids, { uid: true, envelope: true, size: true }, { uid: true })) {
candidates.push({
uid: m.uid,
size: Number(m.size) || 0,
messageId: boundedMessageId(
m.envelope && m.envelope.messageId,
`uid-${cfg.folder}-${m.uid}`,
),
});
}
}
@@ -300,10 +334,30 @@ async function pollAccountOnce(cfg, { accountKey = 'accounting', routeToExpenses
let claimKey = null;
let claimed = false;
try {
// Refuse oversized messages before download (GHSA-2qf9). Recorded
// under the REAL message id — not a synthetic err-<uid>-<now> key —
// so the step-3 dedup skips it on the next poll. Without that, the
// same huge message was re-downloaded every poll interval forever,
// and an OOM-kill/restart simply resumed the loop.
if (MAX_MESSAGE_BYTES > 0 && cand.size > MAX_MESSAGE_BYTES) {
logger.warn?.(`emailIntake: skipping uid ${cand.uid}${cand.size} bytes exceeds the ${MAX_MESSAGE_BYTES}-byte limit`);
await db('received_emails').insert({
message_id: cand.messageId,
account_key: accountKey,
status: 'error',
error: `Message too large (${cand.size} bytes); limit is ${MAX_MESSAGE_BYTES}`,
attachment_count: 0,
received_at: new Date(),
created_at: new Date(),
});
await client.messageFlagsAdd(cand.uid, ['\\Seen'], { uid: true });
continue;
}
const one = await client.fetchOne(String(cand.uid), { source: true }, { uid: true });
if (!one || !one.source) continue;
const parsed = await simpleParser(one.source);
messageId = parsed.messageId || cand.messageId;
messageId = boundedMessageId(parsed.messageId, cand.messageId);
// Claim key: a no-Message-ID mail still needs a non-null, per-message
// key so two pollers converge — fall back to the mailbox uid.
claimKey = messageId || `nomsgid-${cand.uid}`;
@@ -347,7 +401,25 @@ async function pollAccountOnce(cfg, { accountKey = 'accounting', routeToExpenses
let count = 0;
const attErrors = [];
if (routeToExpenses) {
const atts = (parsed.attachments || []).filter((a) => ALLOWED_MIME.includes(a.contentType));
const allowed = (parsed.attachments || []).filter((a) => ALLOWED_MIME.includes(a.contentType));
// Cap attachment count AND cumulative bytes (GHSA-2qf9) — a single
// in-limit message can still carry hundreds of attachments, each
// written to disk by saveAttachment().
const atts = [];
let attBytes = 0;
for (const att of allowed) {
if (atts.length >= MAX_ATTACHMENTS) {
attErrors.push(`Attachment limit reached (${MAX_ATTACHMENTS}); remaining attachments skipped`);
break;
}
const size = att.content ? att.content.length : 0;
if (attBytes + size > MAX_ATTACHMENT_BYTES) {
attErrors.push(`Cumulative attachment size limit reached (${MAX_ATTACHMENT_BYTES} bytes); remaining attachments skipped`);
break;
}
attBytes += size;
atts.push(att);
}
for (const att of atts) {
try {
const filePath = await saveAttachment(att);