diff --git a/.env.example b/.env.example index 91f9ba7..00f34bc 100644 --- a/.env.example +++ b/.env.example @@ -7,6 +7,16 @@ PORT=3000 NODE_ENV=production +# ── Reverse proxy ────────────────────────────────────────────────────────────── +# Set when running behind a reverse proxy (Docker, nginx, Traefik, Caddy, etc.) +# so req.ip reflects the real client (rate limiting, audit logs, login history) +# and req.secure reflects the original protocol (Secure cookies). +# TRUST_PROXY=true (trust first proxy hop — most common) +# TRUST_PROXY=2 (trust two hops, e.g. CDN + nginx) +# TRUST_PROXY=loopback (trust loopback addresses only) +# Leave unset for direct deployments with no proxy. +# TRUST_PROXY=true + # ── CSRF Cookie httpOnly Setting ────────────────────────────────────────────── # CSRF cookie httpOnly setting (default: true) # The SPA fetches the token from GET /api/auth/csrf-token and stores it in diff --git a/client/api.js b/client/api.js index 602c5c7..c38f20a 100644 --- a/client/api.js +++ b/client/api.js @@ -5,15 +5,30 @@ async function getCsrfToken() { if (!_csrfFetch) { _csrfFetch = fetch('/api/auth/csrf-token', { credentials: 'include' }) .then(r => r.json()) - .then(d => d.token || ''); + .then(d => d.token || '') + .catch(() => { + _csrfFetch = null; // don't cache a failed fetch + return ''; + }); } return _csrfFetch; } -async function _fetch(method, path, body) { +const MUTATING_METHODS = ['POST', 'PUT', 'DELETE', 'PATCH']; + +// Parse a response body without assuming it is JSON. Returns null when the +// body is empty (204) or not valid JSON (e.g. an HTML error page from a proxy). +async function parseJsonSafe(res) { + if (res.status === 204) return null; + const text = await res.text(); + if (!text) return null; + try { return JSON.parse(text); } catch { return null; } +} + +async function _fetch(method, path, body, _retried = false) { const opts = { method, headers: { 'Content-Type': 'application/json' }, credentials: 'include' }; // Add CSRF token header for state-changing methods - if (['POST', 'PUT', 'DELETE', 'PATCH'].includes(method)) { + if (MUTATING_METHODS.includes(method)) { const csrfToken = await getCsrfToken(); if (csrfToken) { opts.headers['x-csrf-token'] = csrfToken; @@ -21,16 +36,22 @@ async function _fetch(method, path, body) { } if (body !== undefined) opts.body = JSON.stringify(body); const res = await fetch('/api' + path, opts); - const data = await res.json(); + const data = await parseJsonSafe(res); if (!res.ok) { - const err = new Error(data.message || data.error || `HTTP ${res.status}`); + // Stale CSRF token (cookie rotated/expired since first fetch): refresh the + // cached token and retry the request once instead of forcing a page reload. + if (!_retried && res.status === 403 && data?.code === 'CSRF_INVALID' && MUTATING_METHODS.includes(method)) { + _csrfFetch = null; + return _fetch(method, path, body, true); + } + const err = new Error(data?.message || data?.error || `HTTP ${res.status}`); err.status = res.status; - err.data = data; - err.details = data.details || []; - err.code = data.code; + err.data = data || {}; + err.details = data?.details || []; + err.code = data?.code; throw err; } - return data; + return data ?? {}; } function queryString(params = {}) { diff --git a/db/database.js b/db/database.js index 233f427..0ba2312 100644 --- a/db/database.js +++ b/db/database.js @@ -641,22 +641,19 @@ function assertWritableDbPath() { } } -function sleep(ms) { - Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); -} - function getDb() { // already ready if (db) return db; - // wait if another init is happening - while (initializing) { - sleep(50); + // Node is single-threaded and initialization below is fully synchronous, so + // the only way to observe `initializing === true` here is a re-entrant call + // from inside init itself (e.g. a migration requiring a module that calls + // getDb() at load time). Blocking/spinning can never resolve that — it would + // deadlock the process — so fail fast with a clear message instead. + if (initializing) { + throw new Error('getDb() called re-entrantly during database initialization'); } - // check again after wait - if (db) return db; - initializing = true; try { diff --git a/middleware/csrf.js b/middleware/csrf.js index 3b525e4..7d3bb36 100644 --- a/middleware/csrf.js +++ b/middleware/csrf.js @@ -37,6 +37,18 @@ function generateCsrfToken() { return crypto.randomBytes(32).toString('hex'); } +/** + * Detect HTTPS the same way services/authService.cookieOpts does: + * req.secure (honors trust proxy) with an x-forwarded-proto fallback for + * deployments where TRUST_PROXY is not configured. + */ +function requestLooksHttps(req) { + if (!req) return false; + if (req.secure) return true; + const proto = req.get?.('x-forwarded-proto') || req.headers?.['x-forwarded-proto']; + return String(proto || '').split(',').map(s => s.trim()).includes('https'); +} + /** * Get or create CSRF token for the current session. * In the SPA's double-submit flow, tokens are stored in a readable cookie so @@ -50,7 +62,7 @@ function getCsrfToken(req, res) { res.cookie(CSRF_COOKIE_NAME, token, { httpOnly: CSRF_HTTP_ONLY, sameSite: CSRF_SAME_SITE, - secure: CSRF_SECURE && req.secure, + secure: CSRF_SECURE && requestLooksHttps(req), path: '/', }); } @@ -62,8 +74,9 @@ function getCsrfToken(req, res) { * Validate CSRF token from request. * Tokens can be provided via: * - x-csrf-token header (API clients) - * - csrf_token query parameter (form submissions) * - csrf_token body field (form submissions) + * Query-parameter tokens are deliberately NOT accepted — URLs leak into + * access logs, browser history, and Referer headers. */ function validateCsrfToken(req) { const cookieToken = req.cookies?.[CSRF_COOKIE_NAME]; @@ -72,9 +85,6 @@ function validateCsrfToken(req) { const headerToken = req.headers?.[CSRF_HEADER_NAME]; if (headerToken && headerToken === cookieToken) return true; - const queryToken = req.query?.csrf_token; - if (queryToken && queryToken === cookieToken) return true; - const bodyToken = req.body?.csrf_token; if (bodyToken && bodyToken === cookieToken) return true; @@ -87,9 +97,11 @@ function validateCsrfToken(req) { * Requires token for: POST, PUT, DELETE, PATCH (state-changing) */ function csrfMiddleware(req, res, next) { - // Exempt login endpoint - no session exists yet to hijack - // Check both originalUrl and path for mounted routers - if (req.originalUrl === '/api/auth/login' || req.path === '/login' || req.path === '/api/auth/login') { + // Exempt the login endpoint only — no session exists yet to hijack. + // Compare against originalUrl (sans query string) so a "/login" subpath on + // some other mounted router is NOT accidentally exempted. + const fullPath = (req.originalUrl || '').split('?')[0]; + if (fullPath === '/api/auth/login') { return next(); } diff --git a/middleware/errorFormatter.js b/middleware/errorFormatter.js index 96ba9c5..4791f97 100644 --- a/middleware/errorFormatter.js +++ b/middleware/errorFormatter.js @@ -55,8 +55,15 @@ function errorFormatter(req, res, next) { res.json = function(data) { // If data is an error object (has error property), standardize it - if (data && typeof data === 'object' && data.error && !data.success) { - const standardized = standardizeError(data.error, data.error || 'ERROR', data.field); + if (data && typeof data === 'object' && !Array.isArray(data) && data.error && !data.success) { + // Already standardized (machine-readable code + human message) — pass through + if (typeof data.code === 'string' && data.code && typeof data.message === 'string') { + return originalJson.call(this, data); + } + // Use the human text as the message, never as the machine code + const message = (typeof data.message === 'string' && data.message) ? data.message : data.error; + const code = (typeof data.code === 'string' && data.code) ? data.code : 'ERROR'; + const standardized = standardizeError(message, code, data.field); return originalJson.call(this, standardized); } return originalJson.call(this, data); diff --git a/middleware/securityHeaders.js b/middleware/securityHeaders.js index 4dcf5c0..9955d7e 100644 --- a/middleware/securityHeaders.js +++ b/middleware/securityHeaders.js @@ -15,17 +15,19 @@ function getCspNonce(req) { /** * Applies baseline security response headers on every request. - * - * Content Security Policy (CSP) is now implemented with nonce-based policies - * to support Tailwind/shadcn inline styles and Vite build hashes. + * + * CSP notes: + * - No nonces. index.html is served via sendFile/static, so a per-request nonce + * is never injected into the markup and would accomplish nothing for scripts. + * Worse, per CSP3 the mere presence of a nonce makes 'unsafe-inline' ignored, + * which would silently block the inline styles Tailwind/Radix/framer-motion + * rely on. All scripts are external and covered by 'self'. */ function securityHeaders(req, res, next) { - // CSP Header - nonce-based policy for Tailwind and Vite - const nonce = getCspNonce(req); - const cspPolicy = + const cspPolicy = `default-src 'self'; ` + - `script-src 'self' 'nonce-${nonce}'; ` + - `style-src 'self' 'unsafe-inline' 'nonce-${nonce}'; ` + + `script-src 'self'; ` + + `style-src 'self' 'unsafe-inline'; ` + `img-src 'self' data:; ` + `font-src 'self'; ` + `connect-src 'self'; ` + diff --git a/routes/bills.js b/routes/bills.js index e406b1a..5275f2b 100644 --- a/routes/bills.js +++ b/routes/bills.js @@ -298,10 +298,16 @@ router.put('/:id/monthly-state', (req, res) => { return res.status(400).json(standardizeError('snoozed_until must be an ISO date string (YYYY-MM-DD) or null', 'VALIDATION_ERROR', 'snoozed_until')); } - const amt = actual_amount !== undefined ? (actual_amount === null ? null : parseFloat(actual_amount)) : null; - const noteVal = notes !== undefined ? (notes || null) : null; - const skipVal = is_skipped !== undefined ? (is_skipped ? 1 : 0) : 0; - const snoozeVal = snoozed_until !== undefined ? (snoozed_until || null) : null; + // Partial-update semantics: fields omitted from the request keep their + // existing values instead of being wiped by the upsert. + const existing = db.prepare( + 'SELECT actual_amount, notes, is_skipped, snoozed_until FROM monthly_bill_state WHERE bill_id=? AND year=? AND month=?' + ).get(billId, y, m); + + const amt = actual_amount !== undefined ? (actual_amount === null ? null : parseFloat(actual_amount)) : (existing?.actual_amount ?? null); + const noteVal = notes !== undefined ? (notes || null) : (existing?.notes ?? null); + const skipVal = is_skipped !== undefined ? (is_skipped ? 1 : 0) : (existing?.is_skipped ?? 0); + const snoozeVal = snoozed_until !== undefined ? (snoozed_until || null) : (existing?.snoozed_until ?? null); db.prepare(` INSERT INTO monthly_bill_state (bill_id, year, month, actual_amount, notes, is_skipped, snoozed_until, updated_at) diff --git a/server.js b/server.js index cce6152..569cd4b 100644 --- a/server.js +++ b/server.js @@ -16,6 +16,25 @@ const app = express(); const PORT = process.env.PORT || 3000; const DIST = path.join(__dirname, 'dist'); +// ── Trust proxy ─────────────────────────────────────────────────────────────── +// Required when running behind a reverse proxy (Docker, nginx, Traefik, etc.) so +// req.ip reflects the real client (rate limiting, audit logs) and req.secure +// reflects the original protocol (Secure cookies). Examples: +// TRUST_PROXY=true → trust first proxy hop (most common) +// TRUST_PROXY=2 → trust two hops +// TRUST_PROXY=loopback → trust loopback addresses only +// Unset/false → no proxy trusted (direct deployment). +const TRUST_PROXY = (process.env.TRUST_PROXY || '').trim(); +if (TRUST_PROXY && !/^(false|0|no|off)$/i.test(TRUST_PROXY)) { + if (/^(true|yes|on)$/i.test(TRUST_PROXY)) { + app.set('trust proxy', 1); + } else if (/^\d+$/.test(TRUST_PROXY)) { + app.set('trust proxy', parseInt(TRUST_PROXY, 10)); + } else { + app.set('trust proxy', TRUST_PROXY); // 'loopback', named subnet, or CIDR list + } +} + // ── Security headers (applied to every response) ────────────────────────────── app.use(securityHeaders); @@ -36,6 +55,11 @@ app.use(cookieParser()); // This ensures the CSRF token cookie is always present for API clients app.use(csrfTokenProvider); +// ── Error response formatter ───────────────────────────────────────────────── +// Patches res.json so all error bodies follow the standardized format. +// Must be mounted BEFORE the routes so the patch is in place when they respond. +app.use(errorFormatter); + // ── API ─────────────────────────────────────────────────────────────────────── // Auth — rate limiters applied at middleware level to prevent bypass @@ -113,6 +137,17 @@ app.use('/api/export', csrfMiddleware, requireAuth, requireUser, exportLi app.use('/api/import', csrfMiddleware, requireAuth, requireUser, importLimiter, importRoutes); app.use('/api/imports', csrfMiddleware, requireAuth, requireUser, importLimiter, importRoutes); +// ── API 404 — unknown /api/* routes must return JSON, not the SPA shell ────── +// Without this, the catch-all below serves index.html with HTTP 200 for any +// unrecognized API path, which API clients then fail to parse as JSON. +app.all('/api/*', (req, res) => { + res.status(404).json({ + error: 'NOT_FOUND', + message: 'Unknown API route', + code: 'NOT_FOUND', + }); +}); + // ── Legacy UI ("Remember When" mode) ───────────────────────────────────────── app.use('/legacy', express.static(path.join(__dirname, 'legacy'))); @@ -127,10 +162,6 @@ app.get('*', (req, res) => { res.sendFile(path.join(DIST, 'index.html')); }); -// ── Global error formatter middleware (runs before error handler) ─────────── -// Ensures all error responses follow the standardized format. -app.use(errorFormatter); - // ── Global error handler ────────────────────────────────────────────────────── // Never expose stack traces, internal paths, or raw error objects in responses. app.use((err, req, res, next) => { diff --git a/services/authService.js b/services/authService.js index efc6db4..addff6a 100644 --- a/services/authService.js +++ b/services/authService.js @@ -13,6 +13,12 @@ const COOKIE_NAME = 'bt_session'; const SINGLE_COOKIE_NAME = 'bt_single_session'; const SESSION_DAYS = 7; +// Pre-computed hash used to equalize login timing when the account is unknown, +// inactive, or OIDC-only. Without it those paths skip bcrypt.compare and +// respond measurably faster, letting an attacker enumerate valid usernames. +// Cost factor 12 matches real password hashes. Computed once at module load. +const TIMING_EQUALIZATION_HASH = bcrypt.hashSync(crypto.randomBytes(32).toString('hex'), 12); + function envFlag(name) { const value = process.env[name]; if (value === undefined) return null; @@ -53,11 +59,12 @@ function cookieOpts(req) { async function login(username, password) { const db = getDb(); const user = db.prepare('SELECT * FROM users WHERE username = ?').get(username); - if (!user) return null; - if (user.active === 0) return null; - // Reject OIDC-only accounts from local login - if (user.auth_provider && user.auth_provider !== 'local') { + // Unknown, inactive, or OIDC-only account: still burn a bcrypt comparison so + // the response time is indistinguishable from a wrong password (no timing + // oracle for username enumeration). + if (!user || user.active === 0 || (user.auth_provider && user.auth_provider !== 'local')) { + await bcrypt.compare(password, TIMING_EQUALIZATION_HASH); return null; } diff --git a/utils/dates.js b/utils/dates.js new file mode 100644 index 0000000..ba7bf78 --- /dev/null +++ b/utils/dates.js @@ -0,0 +1,33 @@ +'use strict'; + +/** + * Server-side local-date helpers. + * + * Bill due dates and payment dates are stored as plain YYYY-MM-DD strings in + * the server's local timezone (the client computes "today" the same way in + * client/pages/TrackerPage.jsx → localDateString). Never derive a calendar + * date from Date.toISOString() — that yields the UTC date, which disagrees + * with local time around midnight and month boundaries. + */ + +/** YYYY-MM-DD in local time. */ +function localDateString(date = new Date()) { + const year = date.getFullYear(); + const month = String(date.getMonth() + 1).padStart(2, '0'); + const day = String(date.getDate()).padStart(2, '0'); + return `${year}-${month}-${day}`; +} + +/** { year, month } (1-based month) in local time. */ +function localYearMonth(date = new Date()) { + return { year: date.getFullYear(), month: date.getMonth() + 1 }; +} + +/** YYYY-MM-DD in local time, `days` days before `from`. */ +function localDateStringDaysAgo(days, from = new Date()) { + const d = new Date(from); + d.setDate(d.getDate() - days); + return localDateString(d); +} + +module.exports = { localDateString, localYearMonth, localDateStringDaysAgo }; diff --git a/workers/dailyWorker.js b/workers/dailyWorker.js index 7a8b063..c9dd05c 100644 --- a/workers/dailyWorker.js +++ b/workers/dailyWorker.js @@ -12,6 +12,7 @@ const { markWorkerStarted, markWorkerSuccess, } = require('../services/statusRuntime'); +const { localDateString, localDateStringDaysAgo } = require('../utils/dates'); const DAILY_CRON_HOUR = 6; @@ -27,7 +28,10 @@ async function runDailyTasks() { const now = new Date(); const year = now.getFullYear(); const month = now.getMonth() + 1; - const todayStr = now.toISOString().slice(0, 10); + // Local date — keep consistent with year/month above and with the client's + // notion of "today". toISOString() would give the UTC date, which can be a + // different calendar day and caused autopay marking a day early/late. + const todayStr = localDateString(now); const bills = db.prepare('SELECT * FROM bills WHERE active = 1 AND deleted_at IS NULL').all(); @@ -37,7 +41,7 @@ async function runDailyTasks() { // fit inside a 90-day window for the current month's due-date checks. const billIds = bills.map(b => b.id); const placeholders = billIds.map(() => '?').join(','); - const windowStart = new Date(Date.now() - 90 * 86400000).toISOString().slice(0, 10); + const windowStart = localDateStringDaysAgo(90, now); let allPayments = []; try {