diff --git a/.env.example b/.env.example index 33e4d6f5..86e63fe2 100644 --- a/.env.example +++ b/.env.example @@ -56,8 +56,9 @@ DB_NAME=picpeak_prod # Admin Account (initial setup) — OPTIONAL # Leave these unset (default) to create your admin IN THE BROWSER on first run: # open /admin and PicPeak shows a setup screen. The one-time setup token is -# printed to the backend logs (`docker compose logs backend | grep -i "setup token"`) -# and saved to data/SETUP_TOKEN. +# written to data/SETUP_TOKEN with mode 0600 — read it with +# `docker compose exec backend cat /app/data/SETUP_TOKEN`. It is NOT logged +# unless that write fails, so it never sits in `docker logs`. # Set ADMIN_PASSWORD to auto-create the admin on first boot instead (legacy; # credentials written to data/ADMIN_CREDENTIALS.txt). #ADMIN_USERNAME=admin diff --git a/README.md b/README.md index 11348bf6..5abea197 100644 --- a/README.md +++ b/README.md @@ -111,10 +111,15 @@ docker compose up -d On first start with no `ADMIN_PASSWORD` set, PicPeak has **no admin account yet** and greets you with an in-browser setup screen — no credentials in `.env`: 1. Open **http://localhost:3000/admin** — you'll be redirected to `/setup`. -2. Grab the **one-time setup token** from the backend logs (it's also saved to `data/SETUP_TOKEN`): +2. Read the **one-time setup token** from the 0600 file the backend writes it to + (it is deliberately *not* printed to the logs — that would leave a live + bootstrap credential in `docker logs`): ```bash - docker compose logs backend | grep -i "setup token" + docker compose exec backend cat /app/data/SETUP_TOKEN ``` + It is bind-mounted, so `sudo cat data/SETUP_TOKEN` on the host works too. Only + if that file could not be written does the backend fall back to logging the + token (`docker compose logs backend | grep -i "setup token"`). 3. Paste the token, set your admin **email + password**, and you're in. The token is single-use, and the setup screen closes permanently once an admin exists. > Prefer the old behaviour? Set `ADMIN_PASSWORD` in `.env` and PicPeak auto-creates the admin on first boot instead (credentials written to `data/ADMIN_CREDENTIALS.txt`). diff --git a/SIMPLE_SETUP.md b/SIMPLE_SETUP.md index 9b6d3da8..242a032f 100644 --- a/SIMPLE_SETUP.md +++ b/SIMPLE_SETUP.md @@ -170,10 +170,12 @@ If you installed with `picpeak-setup.sh` and gave an `--admin-password`, your ad If you started PicPeak **without** setting `ADMIN_PASSWORD` (e.g. a plain `docker compose up`), there's **no admin yet** and you create it in the browser: 1. Open `http://your-server:3000/admin` — you'll land on a setup screen. -2. Get the **one-time setup token** from the backend logs (also saved to `data/SETUP_TOKEN`): +2. Read the **one-time setup token** from the 0600 file the backend writes it to + (it is not logged — that would leave a live credential in `docker logs`): ```bash - docker compose logs backend | grep -i "setup token" + docker compose exec backend cat /app/data/SETUP_TOKEN ``` + Only if that write fails does the backend log the token instead. 3. Paste it, set your admin email + password. The token is single-use and the screen closes once an admin exists. ## 🌐 Access Methods diff --git a/backend/__tests__/services/emailIntakeCaps.test.js b/backend/__tests__/services/emailIntakeCaps.test.js new file mode 100644 index 00000000..cca9587a --- /dev/null +++ b/backend/__tests__/services/emailIntakeCaps.test.js @@ -0,0 +1,127 @@ +/** + * Inbound-mail resource caps (GHSA-2qf9). + * + * emailIntakeService downloaded, parsed and persisted every message with no + * size, attachment-count or attachment-byte limit. Anyone who can email the + * operator's mailbox reaches this path unauthenticated. + * + * The teeth were in the dedup key: on failure the service wrote an error row + * keyed `err--`, which can never match the envelope-derived + * `messageId` the dedup pass compares against. So the same oversized message + * was re-downloaded every poll interval forever — and an OOM-kill/restart just + * resumed the loop. This pins that an over-limit message is (a) never + * downloaded and (b) recorded under its REAL message id so it dedups. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +process.env.NODE_ENV = 'test'; +process.env.TEST_DATABASE_PATH = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-intake-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'intake-test-secret'; +process.env.EMAIL_INTAKE_MAX_MESSAGE_BYTES = '1000'; + +const OVERSIZED_UID = 11; +const NORMAL_UID = 12; +const OVERSIZED_MSGID = ''; + +const fetchOneCalls = []; + +jest.mock('imapflow', () => ({ + ImapFlow: class { + async connect() {} + async logout() {} + async getMailboxLock() { return { release() {} }; } + async search() { return [OVERSIZED_UID, NORMAL_UID]; } + // Envelope pass now also returns `size`. + async *fetch() { + yield { uid: OVERSIZED_UID, size: 50_000, envelope: { messageId: OVERSIZED_MSGID } }; + yield { uid: NORMAL_UID, size: 500, envelope: { messageId: '' } }; + } + async fetchOne(uid) { + fetchOneCalls.push(String(uid)); + return { source: Buffer.from('Subject: ok\r\n\r\nbody') }; + } + async messageFlagsAdd() { return true; } + }, +})); + +jest.mock('mailparser', () => ({ + simpleParser: async () => ({ + messageId: '', + subject: 'ok', + date: new Date(), + attachments: [], + text: 'body', + html: null, + }), +})); + +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +describe('email intake caps (GHSA-2qf9)', () => { + let db; let cleanup; let intake; + + let pollResult; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + + // pollOnce short-circuits unless the feature flag is on AND an IMAP + // account is configured — without both, this suite would pass vacuously. + await db('feature_flags') + .insert({ key: 'incomingMail', value: 1 }) + .onConflict('key').merge({ value: 1 }); + // getImapConfig() reads email_configs.first() — seedMinimal may already + // have inserted a row, so update that one rather than adding a second + // (the first row would win and report "unconfigured"). + const imapFields = { + imap_host: 'imap.example.com', + imap_user: 'intake@example.com', + imap_pass: 'x', + imap_folder: 'INBOX', + }; + const existingCfg = await db('email_configs').first(); + if (existingCfg) { + await db('email_configs').where({ id: existingCfg.id }).update(imapFields); + } else { + await db('email_configs').insert({ + smtp_host: 'smtp.example.com', + smtp_port: 587, + from_email: 'intake@example.com', + ...imapFields, + }); + } + + intake = require('../../src/services/emailIntakeService'); + pollResult = await intake.pollOnce().catch((e) => ({ thrown: e.message })); + }, 120000); + + afterAll(async () => { if (cleanup) await cleanup(); }); + + it('actually ran the poll (guards against a vacuous suite)', () => { + expect(pollResult).toBeDefined(); + expect(pollResult.skipped).toBeUndefined(); + }); + + it('never downloads a message whose envelope size exceeds the cap', () => { + // The oversized uid must never reach fetchOne (the source download) — + // that download is the DoS. The normal one must still be processed. + expect(fetchOneCalls).not.toContain(String(OVERSIZED_UID)); + expect(fetchOneCalls).toContain(String(NORMAL_UID)); + }); + + it('records the skip under the REAL message id so it dedups next poll', async () => { + const row = await db('received_emails').where({ message_id: OVERSIZED_MSGID }).first(); + expect(row).toBeTruthy(); + expect(row.status).toBe('error'); + expect(String(row.error)).toMatch(/too large/i); + // The whole point: keyed by messageId, NOT err--, which + // could never match the dedup pass and so looped forever. + expect(row.message_id).not.toMatch(/^err-/); + }); +}); diff --git a/backend/__tests__/utils/sanitizeForLog.test.js b/backend/__tests__/utils/sanitizeForLog.test.js new file mode 100644 index 00000000..ec0dfc3c --- /dev/null +++ b/backend/__tests__/utils/sanitizeForLog.test.js @@ -0,0 +1,96 @@ +/** + * Credential redaction for log payloads (GHSA-pgmp / GHSA-r794). + * + * Event create/update logged the whole request body. Beyond the plaintext + * gallery password named in the advisories, the update path also logged + * `client_share_token` — a LIVE bearer credential for client gallery access, + * freshly minted by `regenerate_client_token` — and `client_password_hash`. + */ + +const { sanitizeForLog, isSensitiveKey } = require('../../src/utils/sanitizeForLog'); + +describe('sanitizeForLog', () => { + it('redacts the credentials an event body actually carries', () => { + const out = sanitizeForLog({ + event_name: 'Wedding', + password: 'FAKE-PLAINTEXT-PASSWORD', + client_password: 'FAKE-CLIENT-PASSWORD', + client_password_hash: 'FAKE-BCRYPT-HASH-PLACEHOLDER', + client_share_token: 'FAKE-CLIENT-SHARE-TOKEN', + share_token: 'FAKE-SHARE-TOKEN', + }); + + expect(out.event_name).toBe('Wedding'); + for (const key of ['password', 'client_password', 'client_password_hash', + 'client_share_token', 'share_token']) { + expect(out[key]).toBe('[redacted]'); + } + expect(JSON.stringify(out)).not.toContain('FAKE-PLAINTEXT-PASSWORD'); + expect(JSON.stringify(out)).not.toContain('FAKE-CLIENT-SHARE-TOKEN'); + }); + + it('redacts nested and array-nested secrets', () => { + const out = sanitizeForLog({ + smtp: { host: 'mail.example.com', smtp_password: 'p' }, + users: [{ name: 'a', api_key: 'k' }], + }); + expect(out.smtp.host).toBe('mail.example.com'); + expect(out.smtp.smtp_password).toBe('[redacted]'); + expect(out.users[0].name).toBe('a'); + expect(out.users[0].api_key).toBe('[redacted]'); + }); + + it('passes non-objects through and survives cycles', () => { + expect(sanitizeForLog('plain')).toBe('plain'); + expect(sanitizeForLog(42)).toBe(42); + expect(sanitizeForLog(null)).toBeNull(); + + const cyclic = { name: 'x' }; + cyclic.self = cyclic; + expect(() => sanitizeForLog(cyclic)).not.toThrow(); + expect(sanitizeForLog(cyclic).self).toBe('[circular]'); + }); + + it('matches key names case-insensitively and by fragment', () => { + expect(isSensitiveKey('Authorization')).toBe(true); + expect(isSensitiveKey('CLIENT_SHARE_TOKEN')).toBe(true); + expect(isSensitiveKey('event_name')).toBe(false); + }); +}); + +/** + * Codex round 2: sanitizing req.body was not enough. express-validator's + * errors.array() embeds the SUBMITTED value per field, so a password rejected + * for being too short was still logged in plaintext. + */ +describe('sanitizeValidationErrors', () => { + const { sanitizeValidationErrors } = require('../../src/utils/sanitizeForLog'); + + it('redacts the submitted value for a password field', () => { + const out = sanitizeValidationErrors([ + { type: 'field', path: 'password', msg: 'too short', value: 'FAKE-PLAINTEXT-PASSWORD' }, + { type: 'field', path: 'event_name', msg: 'required', value: '' }, + ]); + expect(out[0].value).toBe('[redacted]'); + expect(out[0].msg).toBe('too short'); + expect(JSON.stringify(out)).not.toContain('FAKE-PLAINTEXT-PASSWORD'); + expect(out[1].value).toBe(''); + }); + + it('handles the legacy `param` field name', () => { + const out = sanitizeValidationErrors([{ param: 'client_password', value: 'FAKE-SECRET' }]); + expect(out[0].value).toBe('[redacted]'); + }); + + it('recurses into object values on non-sensitive fields', () => { + const out = sanitizeValidationErrors([ + { path: 'config', value: { host: 'h', api_key: 'k' } }, + ]); + expect(out[0].value.host).toBe('h'); + expect(out[0].value.api_key).toBe('[redacted]'); + }); + + it('passes non-arrays through untouched', () => { + expect(sanitizeValidationErrors(undefined)).toBeUndefined(); + }); +}); diff --git a/backend/server.js b/backend/server.js index 346a71f2..f56c4cfc 100644 --- a/backend/server.js +++ b/backend/server.js @@ -1005,8 +1005,15 @@ async function startServer() { // Runs AFTER install-from-backup so a restored instance (which repopulates // admin_users) never prints a throwaway token. Best-effort — never blocks boot. let setupToken = null; + let setupTokenFile = null; try { - setupToken = await require('./src/services/setupService').ensureSetupToken(); + const setupSvc = require('./src/services/setupService'); + setupToken = await setupSvc.ensureSetupToken(); + // The path the write ACTUALLY produced (null when it failed). existsSync + // on the candidate answered a different question and reported success + // for a stale, read-only or directory-shaped SETUP_TOKEN — suppressing + // the token here while pointing the operator at content that is not it. + setupTokenFile = setupSvc.writtenSetupTokenFile(); } catch (err) { logger.warn(`[setup] ensureSetupToken skipped: ${err.message}`); } @@ -1026,12 +1033,17 @@ async function startServer() { logger.info(`Server running on port ${PORT}`); logger.info(`Admin interface: ${process.env.ADMIN_URL || 'http://localhost:3000'}`); logger.info(`Frontend: ${process.env.FRONTEND_URL || 'http://localhost:3001'}`); - // First-run: print the one-time setup token to STDOUT (the file logger - // doesn't reach `docker logs`), as the last + most visible thing at boot. + // First-run banner. Print the TOKEN ITSELF only when the 0600 token file + // could not be written — otherwise this lands a live first-admin + // credential in `docker logs` / journald, which is the leak GHSA-r794's + // sweep turned up. When the file exists we point at it instead. if (setupToken) { const url = `${process.env.ADMIN_URL || 'http://localhost:3000'}/admin`; const line = '='.repeat(64); - console.log(`\n${line}\n PicPeak first-run setup — no admin account yet.\n Open: ${url}\n One-time setup token: ${setupToken}\n (also saved to data/SETUP_TOKEN)\n${line}\n`); + const secretLine = setupTokenFile + ? ` Setup token saved to: ${setupTokenFile}\n (read it there — deliberately not printed)` + : ` One-time setup token: ${setupToken}\n (could not write the token file, so it is shown here)`; + console.log(`\n${line}\n PicPeak first-run setup — no admin account yet.\n Open: ${url}\n${secretLine}\n${line}\n`); } }); } catch (error) { diff --git a/backend/src/routes/adminEvents/crud.js b/backend/src/routes/adminEvents/crud.js index f3579f7b..6d5df4c2 100644 --- a/backend/src/routes/adminEvents/crud.js +++ b/backend/src/routes/adminEvents/crud.js @@ -16,6 +16,7 @@ const path = require('path'); const { escapeLikePattern } = require('../../utils/sqlSecurity'); const { validatePasswordInContext, getBcryptRounds } = require('../../utils/passwordValidation'); const logger = require('../../utils/logger'); +const { sanitizeForLog, sanitizeValidationErrors } = require('../../utils/sanitizeForLog'); const { errorResponse } = require('../../utils/routeHelpers'); const { buildShareLinkVariants } = require('../../services/shareLinkService'); const { parseBooleanInput } = require('../../utils/parsers'); @@ -126,10 +127,13 @@ module.exports = (router) => { body('customer_account_ids.*').optional().isInt({ min: 1 }) ], async (req, res) => { try { - logger.debug('Create event request body', { body: req.body }); + // Redact credentials — the body carries the gallery password (GHSA-r794). + logger.debug('Create event request body', { body: sanitizeForLog(req.body) }); const errors = validationResult(req); if (!errors.isEmpty()) { - logger.error('Validation errors:', errors.array()); + // errors.array() embeds the SUBMITTED value per field — including a + // rejected plaintext password (GHSA-r794). + logger.error('Validation errors:', sanitizeValidationErrors(errors.array())); return res.status(400).json({ errors: errors.array() }); } @@ -1263,7 +1267,8 @@ module.exports = (router) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - logger.debug('Update event validation errors', { errors: errors.array(), body: req.body }); + // Redact credentials — an invalid update still logs the whole body (GHSA-pgmp). + logger.debug('Update event validation errors', { errors: sanitizeValidationErrors(errors.array()), body: sanitizeForLog(req.body) }); return res.status(400).json({ errors: errors.array() }); } @@ -1441,9 +1446,12 @@ module.exports = (router) => { } // Log the update request for debugging + // `updates` no longer holds the plaintext password (stripped above), but + // it still carries client_password_hash and — when regenerate_client_token + // was passed — a LIVE client_share_token bearer credential. logger.debug('Update event request', { id, - updates, + updates: sanitizeForLog(updates), color_theme_length: updates.color_theme ? updates.color_theme.length : 0, color_theme_type: typeof updates.color_theme, hero_photo_id: updates.hero_photo_id, diff --git a/backend/src/services/emailIntakeService.js b/backend/src/services/emailIntakeService.js index 955fc1cf..a4834d08 100644 --- a/backend/src/services/emailIntakeService.js +++ b/backend/src/services/emailIntakeService.js @@ -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-- 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-- 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); diff --git a/backend/src/services/setupService.js b/backend/src/services/setupService.js index 19cf55ef..2322700b 100644 --- a/backend/src/services/setupService.js +++ b/backend/src/services/setupService.js @@ -15,11 +15,23 @@ const { formatBoolean } = require('../utils/dbCompat'); // First-run bootstrap. The app boots with NO admin account and no // ADMIN_PASSWORD in the environment; the first browser visit creates the admin. // That create call is guarded by a one-time setup token, generated at boot -// while no admin exists and printed to the logs (+ a best-effort data/SETUP_TOKEN -// file). The token is ALWAYS required and burned on first use, so the endpoint -// is permanently closed once setup is done — safe even on a public IP. +// while no admin exists and written to a 0600 data/SETUP_TOKEN file — and only +// echoed to the logs when that write fails (see ensureSetupToken). The token is +// ALWAYS required and burned on first use, so the endpoint is permanently +// closed once setup is done — safe even on a public IP. const SETUP_TOKEN_KEY = 'setup_token'; +// Path of the token file as ACTUALLY written by the last ensureSetupToken() +// run, or null when that write failed. server.js keys its stdout banner on +// this: it used to re-derive the answer with existsSync(), which reports +// success for a stale, read-only or directory-shaped SETUP_TOKEN that the write +// could not replace — suppressing the token while pointing the operator at +// content that is wrong or unreadable. +let writtenTokenFile = null; +function writtenSetupTokenFile() { + return writtenTokenFile; +} + async function noAdminExists() { const row = await db('admin_users').count({ c: '*' }).first(); return Number(row?.c || 0) === 0; @@ -32,8 +44,8 @@ async function getSetupStatus() { return { needsAdmin, complete: !needsAdmin }; } -// Logs are the source of truth; the file is a convenience for operators who -// reach a shell more easily than the container log view (e.g. `cat data/SETUP_TOKEN`). +// The file is the source of truth (`cat data/SETUP_TOKEN`); the logs only carry +// the token when this file could not be written. function setupTokenFilePath() { const dir = process.env.DATA_DIR || path.join(__dirname, '..', '..', 'data'); return path.join(dir, 'SETUP_TOKEN'); @@ -49,6 +61,7 @@ async function clearSetupToken() { } async function ensureSetupToken() { + writtenTokenFile = null; if (!(await noAdminExists())) { await clearSetupToken(); return null; @@ -60,13 +73,41 @@ async function ensureSetupToken() { // (getAppSetting JSON.parses on read). A raw string is rejected by jsonb. await upsertAppSetting(SETUP_TOKEN_KEY, JSON.stringify(token), 'string'); } - logger.warn(`[setup] No admin account yet — open /admin to finish setup. One-time setup token: ${token}`); + // Write the token to a 0600 file first, and only surface it in the logs / + // stdout when that write FAILED. Previously it was logged unconditionally at + // `warn`, so every default install (LOG_LEVEL=info) wrote a live + // first-admin-bootstrap credential into combined.log and security.log — + // both under the host-bind-mounted ./logs — never rotated out after use. + // The log line remains as the documented last-resort recovery path. + // + // server.js makes the same decision for its stdout banner by reading + // writtenSetupTokenFile() — the outcome recorded here, not a re-derived + // existsSync() guess: printing the token there lands it in `docker logs` / + // journald, which is the very leak this closes, and suppressing it when the + // file is NOT actually current strands the operator with no token at all. + let file = null; + let writeError = null; try { - const file = setupTokenFilePath(); + file = setupTokenFilePath(); fs.mkdirSync(path.dirname(file), { recursive: true }); fs.writeFileSync(file, `${token}\n`, { mode: 0o600 }); + writtenTokenFile = file; } catch (err) { - logger.warn(`[setup] Could not write setup token file (logs still have it): ${err.message}`); + writeError = err; + file = null; + } + + if (writeError) { + logger.warn( + `[setup] Could not write the setup token file (${writeError.message}) — ` + + 'falling back to the log. No admin account yet; open /admin to finish setup. ' + + `One-time setup token: ${token}` + ); + } else { + logger.warn( + '[setup] No admin account yet — open /admin to finish setup. ' + + `The one-time setup token is in ${file} (not logged).` + ); } return token; } @@ -173,4 +214,4 @@ async function createInitialAdmin({ token, email, password, ip }) { }; } -module.exports = { getSetupStatus, ensureSetupToken, verifySetupToken, createInitialAdmin }; +module.exports = { getSetupStatus, ensureSetupToken, setupTokenFilePath, writtenSetupTokenFile, verifySetupToken, createInitialAdmin }; diff --git a/backend/src/utils/sanitizeForLog.js b/backend/src/utils/sanitizeForLog.js new file mode 100644 index 00000000..e2b2dc8a --- /dev/null +++ b/backend/src/utils/sanitizeForLog.js @@ -0,0 +1,84 @@ +/** + * Redact credential-bearing fields before an object reaches the logs + * (GHSA-pgmp / GHSA-r794). + * + * Event create/update routes logged the whole request body. That body can + * carry a gallery `password`, a bcrypt `client_password_hash`, and — when + * `regenerate_client_token` is set — a freshly minted `client_share_token`, + * which is a LIVE bearer credential for client gallery access, not a hash. + * + * Deliberately key-name based rather than value-shaped: a deny-set of names is + * predictable and cheap, whereas guessing at "this looks like a secret" both + * misses and false-positives. Matching is case-insensitive and substring-based + * so `client_password_hash` and `smtp_pass` are caught without enumerating + * every variant. + */ + +const DENY_FRAGMENTS = [ + 'password', + 'passwd', + 'secret', + 'token', + 'api_key', + 'apikey', + 'authorization', + 'credential', + 'private_key', +]; + +const REDACTED = '[redacted]'; + +function isSensitiveKey(key) { + const k = String(key).toLowerCase(); + return DENY_FRAGMENTS.some((fragment) => k.includes(fragment)); +} + +/** + * Return a copy of `value` with sensitive fields replaced by `[redacted]`. + * Non-objects pass through unchanged. Cycles are handled so a caller can't + * turn a log line into an infinite loop. + * + * @param {*} value + * @param {number} [depth] internal recursion guard + * @param {WeakSet} [seen] internal cycle guard + */ +function sanitizeForLog(value, depth = 0, seen = new WeakSet()) { + if (value === null || typeof value !== 'object') return value; + if (depth > 6) return '[truncated]'; + if (seen.has(value)) return '[circular]'; + seen.add(value); + + if (Array.isArray(value)) { + return value.map((v) => sanitizeForLog(v, depth + 1, seen)); + } + + const out = {}; + for (const [key, val] of Object.entries(value)) { + out[key] = isSensitiveKey(key) ? REDACTED : sanitizeForLog(val, depth + 1, seen); + } + return out; +} + +/** + * Redact express-validator's `errors.array()` before logging. + * + * Each entry carries the SUBMITTED value under `value`, keyed by `path`. A + * password that fails the length check therefore lands in the log in plaintext + * — sanitizing only `req.body` does not close that (GHSA-pgmp / r794). + * + * @param {Array} errors output of validationResult(req).array() + */ +function sanitizeValidationErrors(errors) { + if (!Array.isArray(errors)) return errors; + return errors.map((err) => { + if (!err || typeof err !== 'object') return err; + const field = err.path || err.param; + if (field && isSensitiveKey(field)) { + return { ...err, value: REDACTED }; + } + // Even for a non-sensitive field the value may be an object carrying one. + return 'value' in err ? { ...err, value: sanitizeForLog(err.value) } : err; + }); +} + +module.exports = { sanitizeForLog, sanitizeValidationErrors, isSensitiveKey }; diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 87dbf2b0..ea2fb507 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -3619,7 +3619,7 @@ "tokenCommandLabel": "Token nicht gefunden? Führen Sie dies im Projektverzeichnis aus:", "copyCommand": "Befehl kopieren", "tokenRotatedLink": "Logs bereits rotiert? Zur Einrichtungsanleitung", - "tokenLocationHint": "Nicht gefunden? Führen Sie aus: docker compose logs backend | grep -i \"setup token\"", + "tokenLocationHint": "Nicht gefunden? Führen Sie aus: docker compose exec backend cat /app/data/SETUP_TOKEN", "invalidToken": "Dieser Setup-Token ist ungültig.", "passwordRequirements": "Verwenden Sie mindestens 8 Zeichen mit einem Groß- und einem Kleinbuchstaben sowie einer Ziffer.", "emailLabel": "E-Mail-Adresse", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index d0de9977..dfad55bf 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -3515,7 +3515,7 @@ "tokenCommandLabel": "Can't find your token? Run this in the project directory:", "copyCommand": "Copy command", "tokenRotatedLink": "Logs already rotated away? Read the setup guide", - "tokenLocationHint": "Can't find it? Run: docker compose logs backend | grep -i \"setup token\"", + "tokenLocationHint": "Can't find it? Run: docker compose exec backend cat /app/data/SETUP_TOKEN", "invalidToken": "That setup token is not valid.", "passwordRequirements": "Use at least 8 characters with an upper-case letter, a lower-case letter and a number.", "emailLabel": "Email address", diff --git a/frontend/src/pages/SetupPage.tsx b/frontend/src/pages/SetupPage.tsx index 12af1f46..086d6733 100644 --- a/frontend/src/pages/SetupPage.tsx +++ b/frontend/src/pages/SetupPage.tsx @@ -92,7 +92,7 @@ export const SetupPage: React.FC = () => { if (errors[field]) setErrors((prev) => ({ ...prev, [field]: '' })); }; - const recoveryCommand = 'docker compose logs backend | grep -i "setup token"'; + const recoveryCommand = 'docker compose exec backend cat /app/data/SETUP_TOKEN'; const copyRecoveryCommand = async () => { try {