diff --git a/routes/admin.cts b/routes/admin.cts index fbd379c..12e394b 100644 --- a/routes/admin.cts +++ b/routes/admin.cts @@ -510,9 +510,12 @@ router.put('/bank-sync-config', (req: Req, res: Res) => { if (typeof debug_logging === 'boolean') config = setDebugLogging(debug_logging); res.json(config); } catch (err) { - res - .status(err.status || 500) - .json({ error: err.message || 'Failed to update bank sync config' }); + // 4xx setter validation messages are user-facing; 5xx internals are not. + if (err.status && err.status < 500) { + return res.status(err.status).json({ error: err.message }); + } + log.error('[admin/bank-sync-config]', err); + res.status(500).json({ error: 'Failed to update bank sync config' }); } }); @@ -552,7 +555,9 @@ router.post('/migrations/rollback', async (req: Req, res: Res) => { if (err.code === 'ROLLBACK_NOT_SUPPORTED') { return res.status(422).json({ error: err.message }); } - res.status(500).json({ error: 'Rollback failed', details: err.message }); + // Unknown failure: the audit log (above) keeps err.message; the response must not. + log.error('[admin/migrations/rollback]', err); + res.status(500).json({ error: 'Rollback failed' }); } }); diff --git a/routes/import.cts b/routes/import.cts index c98ccae..4ac11cf 100644 --- a/routes/import.cts +++ b/routes/import.cts @@ -39,7 +39,9 @@ function makeErrorId() { } function sendImportError(res, err, fallback, defaultCode) { - if (err.status) { + // Only 4xx service errors carry a user-facing message; a thrown error with + // an explicit 5xx status must fall through to the masked error-id branch. + if (err.status && err.status < 500) { return res.status(err.status).json({ error: fallback, message: err.message, diff --git a/routes/notifications.cts b/routes/notifications.cts index 6c76f77..1bed038 100644 --- a/routes/notifications.cts +++ b/routes/notifications.cts @@ -9,6 +9,7 @@ const { sendTestEmail } = require('../services/notificationService.cts'); const { sendTestPush } = require('../services/notificationService.cts')._push || {}; const { decryptSecret } = require('../services/encryptionService.cts'); const { encryptSecret } = require('../services/encryptionService.cts'); +const { ApiError, ValidationError } = require('../utils/apiError.cts'); // ── Admin: SMTP configuration ───────────────────────────────────────────────── @@ -74,13 +75,16 @@ router.put('/admin', requireAuth, requireAdmin, (req: Req, res: Res) => { // POST /api/notifications/test — send a test email router.post('/test', requireAuth, requireAdmin, async (req: Req, res: Res) => { const { to } = req.body; - if (!to) return res.status(400).json({ error: 'Recipient address required' }); + if (!to) throw ValidationError('Recipient address required', 'to'); try { await sendTestEmail(to); - res.json({ success: true }); } catch (err) { - res.status(500).json({ error: err.message }); + // Deliberate pass-through: the SMTP error text is the admin's own config + // feedback (bad host/auth/TLS). ApiError messages survive formatError; + // anything not wrapped would be masked as a generic 5xx. + throw new ApiError('NOTIFICATION_TEST_FAILED', err.message || 'Test email failed', 502); } + res.json({ success: true }); }); // ── User: notification preferences ─────────────────────────────────────────── @@ -164,7 +168,7 @@ router.post('/test-push', requireAuth, requireUser, async (req: Req, res: Res) = .get(req.user.id); if (!user?.push_channel || !user?.push_url) { - return res.status(400).json({ error: 'Push notification channel not configured' }); + throw ValidationError('Push notification channel not configured', 'push_channel'); } // Decrypt for sending @@ -185,10 +189,12 @@ router.post('/test-push', requireAuth, requireUser, async (req: Req, res: Res) = try { if (!sendTestPush) throw new Error('Push service not initialised'); await sendTestPush(userForPush); - res.json({ success: true }); } catch (err) { - res.status(500).json({ error: err.message }); + // Deliberate pass-through: the push-provider error is the user's own + // channel-config feedback (bad URL/token) — same rationale as /test above. + throw new ApiError('NOTIFICATION_TEST_FAILED', err.message || 'Test push failed', 502); } + res.json({ success: true }); }); module.exports = router; diff --git a/routes/spending.cts b/routes/spending.cts index 842ae14..0cf8444 100644 --- a/routes/spending.cts +++ b/routes/spending.cts @@ -3,9 +3,9 @@ import type { Req, Res, Next } from '../types/http'; ('use strict'); const express = require('express'); -const { log } = require('../utils/logger.cts'); const router = express.Router(); const { getDb } = require('../db/database.cts'); +const { ValidationError } = require('../utils/apiError.cts'); const { getSpendingSummary, getSpendingTransactions, @@ -18,31 +18,27 @@ const { getIncomeTransactions, } = require('../services/spendingService.cts'); +// Throws ValidationError on bad input; service errors propagate to the +// terminal handler (4xx with .status pass their message through, 5xx are +// masked + logged there — no per-route try/catch, per CODE_STANDARDS.md). function parseYM(source) { const now = new Date(); const year = parseInt(source.year || now.getFullYear(), 10); const month = parseInt(source.month || now.getMonth() + 1, 10); - if (isNaN(year) || year < 2000 || year > 2100) return { error: 'Invalid year' }; - if (isNaN(month) || month < 1 || month > 12) return { error: 'Invalid month' }; + if (isNaN(year) || year < 2000 || year > 2100) throw ValidationError('Invalid year', 'year'); + if (isNaN(month) || month < 1 || month > 12) throw ValidationError('Invalid month', 'month'); return { year, month }; } // GET /api/spending/summary?year=&month= router.get('/summary', (req: Req, res: Res) => { const ym = parseYM(req.query); - if (ym.error) return res.status(400).json({ error: ym.error }); - try { - res.json(getSpendingSummary(getDb(), req.user.id, ym.year, ym.month)); - } catch (err) { - log.error('[spending/summary]', err.message); - res.status(500).json({ error: 'Failed to load spending summary' }); - } + res.json(getSpendingSummary(getDb(), req.user.id, ym.year, ym.month)); }); // GET /api/spending/transactions?year=&month=&category_id=&page=&limit= router.get('/transactions', (req: Req, res: Res) => { const ym = parseYM(req.query); - if (ym.error) return res.status(400).json({ error: ym.error }); const { category_id, page, limit } = req.query; const categoryId = @@ -52,56 +48,38 @@ router.get('/transactions', (req: Req, res: Res) => { ? parseInt(category_id, 10) : undefined; - try { - res.json( - getSpendingTransactions(getDb(), req.user.id, ym.year, ym.month, { - categoryId, - uncategorizedOnly: category_id === 'null', - page: parseInt(page || '1', 10), - limit: Math.min(parseInt(limit || '50', 10), 200), - }), - ); - } catch (err) { - log.error('[spending/transactions]', err.message); - res.status(500).json({ error: 'Failed to load transactions' }); - } + res.json( + getSpendingTransactions(getDb(), req.user.id, ym.year, ym.month, { + categoryId, + uncategorizedOnly: category_id === 'null', + page: parseInt(page || '1', 10), + limit: Math.min(parseInt(limit || '50', 10), 200), + }), + ); }); // PATCH /api/spending/transactions/:id/category router.patch('/transactions/:id/category', (req: Req, res: Res) => { const txId = parseInt(req.params.id, 10); - if (isNaN(txId)) return res.status(400).json({ error: 'Invalid transaction ID' }); + if (isNaN(txId)) throw ValidationError('Invalid transaction ID', 'id'); const { category_id, save_rule } = req.body || {}; const categoryId = category_id === null || category_id === undefined ? null : parseInt(category_id, 10); - try { - categorizeTransaction(getDb(), req.user.id, txId, categoryId, !!save_rule); - res.json({ ok: true }); - } catch (err) { - res - .status(err.status || 500) - .json({ error: err.message || 'Failed to categorize transaction' }); - } + categorizeTransaction(getDb(), req.user.id, txId, categoryId, !!save_rule); + res.json({ ok: true }); }); // GET /api/spending/budgets?year=&month= router.get('/budgets', (req: Req, res: Res) => { const ym = parseYM(req.query); - if (ym.error) return res.status(400).json({ error: ym.error }); - try { - res.json({ budgets: getSpendingBudgets(getDb(), req.user.id, ym.year, ym.month) }); - } catch (err) { - log.error('[spending/budgets GET]', err.message); - res.status(500).json({ error: 'Failed to load budgets' }); - } + res.json({ budgets: getSpendingBudgets(getDb(), req.user.id, ym.year, ym.month) }); }); // POST /api/spending/budgets/copy — copy all budgets from previous month into target month router.post('/budgets/copy', (req: Req, res: Res) => { const ym = parseYM(req.body || {}); - if (ym.error) return res.status(400).json({ error: ym.error }); // Previous month let prevYear = ym.year, @@ -111,25 +89,24 @@ router.post('/budgets/copy', (req: Req, res: Res) => { prevYear--; } - try { - const db = getDb(); - const prev = db - .prepare( - ` + const db = getDb(); + const prev = db + .prepare( + ` SELECT category_id, amount FROM spending_budgets WHERE user_id = ? AND year = ? AND month = ? `, - ) - .all(req.user.id, prevYear, prevMonth); + ) + .all(req.user.id, prevYear, prevMonth); - if (prev.length === 0) { - return res.json({ - copied: 0, - budgets: getSpendingBudgets(db, req.user.id, ym.year, ym.month), - }); - } + if (prev.length === 0) { + return res.json({ + copied: 0, + budgets: getSpendingBudgets(db, req.user.id, ym.year, ym.month), + }); + } - const upsert = db.prepare(` + const upsert = db.prepare(` INSERT INTO spending_budgets (user_id, category_id, year, month, amount, updated_at) VALUES (?, ?, ?, ?, ?, datetime('now')) ON CONFLICT(user_id, category_id, year, month) DO UPDATE SET @@ -137,95 +114,66 @@ router.post('/budgets/copy', (req: Req, res: Res) => { updated_at = datetime('now') `); - db.transaction(() => { - for (const row of prev) - upsert.run(req.user.id, row.category_id, ym.year, ym.month, row.amount); - })(); + db.transaction(() => { + for (const row of prev) upsert.run(req.user.id, row.category_id, ym.year, ym.month, row.amount); + })(); - res.json({ - copied: prev.length, - budgets: getSpendingBudgets(db, req.user.id, ym.year, ym.month), - }); - } catch (err) { - log.error('[spending/budgets/copy]', err.message); - res.status(500).json({ error: 'Failed to copy budgets' }); - } + res.json({ + copied: prev.length, + budgets: getSpendingBudgets(db, req.user.id, ym.year, ym.month), + }); }); // PUT /api/spending/budgets — { category_id, year, month, amount } router.put('/budgets', (req: Req, res: Res) => { const { category_id, year, month, amount } = req.body || {}; - if (!category_id) return res.status(400).json({ error: 'category_id required' }); + if (!category_id) throw ValidationError('category_id required', 'category_id'); const ym = parseYM({ year, month }); - if (ym.error) return res.status(400).json({ error: ym.error }); - try { - setSpendingBudget( - getDb(), - req.user.id, - parseInt(category_id, 10), - ym.year, - ym.month, - amount ?? null, - ); - res.json({ ok: true }); - } catch (err) { - log.error('[spending/budgets PUT]', err.message); - res.status(500).json({ error: 'Failed to save budget' }); - } + setSpendingBudget( + getDb(), + req.user.id, + parseInt(category_id, 10), + ym.year, + ym.month, + amount ?? null, + ); + res.json({ ok: true }); }); // GET /api/spending/category-rules router.get('/category-rules', (req: Req, res: Res) => { - try { - res.json({ rules: getSpendingCategoryRules(getDb(), req.user.id) }); - } catch (err) { - log.error('[spending/category-rules GET]', err.message); - res.status(500).json({ error: 'Failed to load rules' }); - } + res.json({ rules: getSpendingCategoryRules(getDb(), req.user.id) }); }); // POST /api/spending/category-rules — { category_id, merchant } router.post('/category-rules', (req: Req, res: Res) => { const { category_id, merchant } = req.body || {}; if (!category_id || !merchant) - return res.status(400).json({ error: 'category_id and merchant required' }); - try { - addSpendingCategoryRule(getDb(), req.user.id, parseInt(category_id, 10), merchant); - res.json({ ok: true }); - } catch (err) { - log.error('[spending/category-rules POST]', err.message); - res.status(err.status || 500).json({ error: err.message || 'Failed to save rule' }); - } + throw ValidationError( + 'category_id and merchant required', + !category_id ? 'category_id' : 'merchant', + ); + addSpendingCategoryRule(getDb(), req.user.id, parseInt(category_id, 10), merchant); + res.json({ ok: true }); }); // DELETE /api/spending/category-rules/:id router.delete('/category-rules/:id', (req: Req, res: Res) => { - try { - deleteSpendingCategoryRule(getDb(), req.user.id, parseInt(req.params.id, 10)); - res.json({ ok: true }); - } catch (err) { - log.error('[spending/category-rules DELETE]', err.message); - res.status(500).json({ error: 'Failed to delete rule' }); - } + deleteSpendingCategoryRule(getDb(), req.user.id, parseInt(req.params.id, 10)); + res.json({ ok: true }); }); // GET /api/spending/income?year=&month=&page= router.get('/income', (req: Req, res: Res) => { const ym = parseYM(req.query); - if (ym.error) return res.status(400).json({ error: ym.error }); - try { - res.json( - getIncomeTransactions(getDb(), req.user.id, ym.year, ym.month, { - page: parseInt(req.query.page || '1', 10), - limit: Math.min(parseInt(req.query.limit || '50', 10), 200), - includeIgnored: req.query.include_ignored === 'true', - }), - ); - } catch (err) { - log.error('[spending/income]', err.message); - res.status(500).json({ error: 'Failed to load income transactions' }); - } + res.json( + getIncomeTransactions(getDb(), req.user.id, ym.year, ym.month, { + page: parseInt(req.query.page || '1', 10), + limit: Math.min(parseInt(req.query.limit || '50', 10), 200), + includeIgnored: req.query.include_ignored === 'true', + }), + ); }); module.exports = router; diff --git a/routes/transactions.cts b/routes/transactions.cts index 4013b3c..1f5e929 100644 --- a/routes/transactions.cts +++ b/routes/transactions.cts @@ -996,7 +996,10 @@ router.post('/unmatch-bulk', (req: Req, res: Res) => { results.push({ transaction_id: txId, ok: true }); } } catch (err) { - results.push({ transaction_id: txId, ok: false, error: err.message }); + // Per-item feedback: service 4xx messages are user-facing; anything + // else is masked — 500-class internals must not leak into the results. + const msg = err.status && err.status < 500 ? err.message : 'Unmatch failed'; + results.push({ transaction_id: txId, ok: false, error: msg }); } } })();