fix(routes): stop leaking raw err.message on 5xx responses

Ten call sites returned internal error text to clients on server failures.
Intentionally user-facing messages survive, but only through controlled
paths: 4xx service errors (err.status < 500) and explicit ApiError wraps
(notification test-send SMTP/push feedback, coded rollback errors). All
5xx bodies are now generic; internals go to the server log / audit trail.

- notifications.cts, spending.cts: full throw-pattern conversion
  (CODE_STANDARDS exemplar shape; terminal handler formats + masks)
- transactions.cts: bulk-unmatch per-item results mask 500-class messages
- import.cts: sendImportError user-facing branch tightened to 4xx-only
- admin.cts: bank-sync-config + migration-rollback mask unknown failures

(QA-B13-02; full conversion of the remaining routes continues in the
standards-unification phase)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
null 2026-07-10 16:36:27 -05:00
parent 9efa74e12b
commit a8f8446743
5 changed files with 93 additions and 129 deletions

View File

@ -510,9 +510,12 @@ router.put('/bank-sync-config', (req: Req, res: Res) => {
if (typeof debug_logging === 'boolean') config = setDebugLogging(debug_logging); if (typeof debug_logging === 'boolean') config = setDebugLogging(debug_logging);
res.json(config); res.json(config);
} catch (err) { } catch (err) {
res // 4xx setter validation messages are user-facing; 5xx internals are not.
.status(err.status || 500) if (err.status && err.status < 500) {
.json({ error: err.message || 'Failed to update bank sync config' }); 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') { if (err.code === 'ROLLBACK_NOT_SUPPORTED') {
return res.status(422).json({ error: err.message }); 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' });
} }
}); });

View File

@ -39,7 +39,9 @@ function makeErrorId() {
} }
function sendImportError(res, err, fallback, defaultCode) { 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({ return res.status(err.status).json({
error: fallback, error: fallback,
message: err.message, message: err.message,

View File

@ -9,6 +9,7 @@ const { sendTestEmail } = require('../services/notificationService.cts');
const { sendTestPush } = require('../services/notificationService.cts')._push || {}; const { sendTestPush } = require('../services/notificationService.cts')._push || {};
const { decryptSecret } = require('../services/encryptionService.cts'); const { decryptSecret } = require('../services/encryptionService.cts');
const { encryptSecret } = require('../services/encryptionService.cts'); const { encryptSecret } = require('../services/encryptionService.cts');
const { ApiError, ValidationError } = require('../utils/apiError.cts');
// ── Admin: SMTP configuration ───────────────────────────────────────────────── // ── Admin: SMTP configuration ─────────────────────────────────────────────────
@ -74,13 +75,16 @@ router.put('/admin', requireAuth, requireAdmin, (req: Req, res: Res) => {
// POST /api/notifications/test — send a test email // POST /api/notifications/test — send a test email
router.post('/test', requireAuth, requireAdmin, async (req: Req, res: Res) => { router.post('/test', requireAuth, requireAdmin, async (req: Req, res: Res) => {
const { to } = req.body; const { to } = req.body;
if (!to) return res.status(400).json({ error: 'Recipient address required' }); if (!to) throw ValidationError('Recipient address required', 'to');
try { try {
await sendTestEmail(to); await sendTestEmail(to);
res.json({ success: true });
} catch (err) { } 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 ─────────────────────────────────────────── // ── User: notification preferences ───────────────────────────────────────────
@ -164,7 +168,7 @@ router.post('/test-push', requireAuth, requireUser, async (req: Req, res: Res) =
.get(req.user.id); .get(req.user.id);
if (!user?.push_channel || !user?.push_url) { 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 // Decrypt for sending
@ -185,10 +189,12 @@ router.post('/test-push', requireAuth, requireUser, async (req: Req, res: Res) =
try { try {
if (!sendTestPush) throw new Error('Push service not initialised'); if (!sendTestPush) throw new Error('Push service not initialised');
await sendTestPush(userForPush); await sendTestPush(userForPush);
res.json({ success: true });
} catch (err) { } 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; module.exports = router;

View File

@ -3,9 +3,9 @@ import type { Req, Res, Next } from '../types/http';
('use strict'); ('use strict');
const express = require('express'); const express = require('express');
const { log } = require('../utils/logger.cts');
const router = express.Router(); const router = express.Router();
const { getDb } = require('../db/database.cts'); const { getDb } = require('../db/database.cts');
const { ValidationError } = require('../utils/apiError.cts');
const { const {
getSpendingSummary, getSpendingSummary,
getSpendingTransactions, getSpendingTransactions,
@ -18,31 +18,27 @@ const {
getIncomeTransactions, getIncomeTransactions,
} = require('../services/spendingService.cts'); } = 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) { function parseYM(source) {
const now = new Date(); const now = new Date();
const year = parseInt(source.year || now.getFullYear(), 10); const year = parseInt(source.year || now.getFullYear(), 10);
const month = parseInt(source.month || now.getMonth() + 1, 10); const month = parseInt(source.month || now.getMonth() + 1, 10);
if (isNaN(year) || year < 2000 || year > 2100) return { error: 'Invalid year' }; if (isNaN(year) || year < 2000 || year > 2100) throw ValidationError('Invalid year', 'year');
if (isNaN(month) || month < 1 || month > 12) return { error: 'Invalid month' }; if (isNaN(month) || month < 1 || month > 12) throw ValidationError('Invalid month', 'month');
return { year, month }; return { year, month };
} }
// GET /api/spending/summary?year=&month= // GET /api/spending/summary?year=&month=
router.get('/summary', (req: Req, res: Res) => { router.get('/summary', (req: Req, res: Res) => {
const ym = parseYM(req.query); 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)); 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' });
}
}); });
// GET /api/spending/transactions?year=&month=&category_id=&page=&limit= // GET /api/spending/transactions?year=&month=&category_id=&page=&limit=
router.get('/transactions', (req: Req, res: Res) => { router.get('/transactions', (req: Req, res: Res) => {
const ym = parseYM(req.query); const ym = parseYM(req.query);
if (ym.error) return res.status(400).json({ error: ym.error });
const { category_id, page, limit } = req.query; const { category_id, page, limit } = req.query;
const categoryId = const categoryId =
@ -52,7 +48,6 @@ router.get('/transactions', (req: Req, res: Res) => {
? parseInt(category_id, 10) ? parseInt(category_id, 10)
: undefined; : undefined;
try {
res.json( res.json(
getSpendingTransactions(getDb(), req.user.id, ym.year, ym.month, { getSpendingTransactions(getDb(), req.user.id, ym.year, ym.month, {
categoryId, categoryId,
@ -61,47 +56,30 @@ router.get('/transactions', (req: Req, res: Res) => {
limit: Math.min(parseInt(limit || '50', 10), 200), 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' });
}
}); });
// PATCH /api/spending/transactions/:id/category // PATCH /api/spending/transactions/:id/category
router.patch('/transactions/:id/category', (req: Req, res: Res) => { router.patch('/transactions/:id/category', (req: Req, res: Res) => {
const txId = parseInt(req.params.id, 10); 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 { category_id, save_rule } = req.body || {};
const categoryId = const categoryId =
category_id === null || category_id === undefined ? null : parseInt(category_id, 10); category_id === null || category_id === undefined ? null : parseInt(category_id, 10);
try {
categorizeTransaction(getDb(), req.user.id, txId, categoryId, !!save_rule); categorizeTransaction(getDb(), req.user.id, txId, categoryId, !!save_rule);
res.json({ ok: true }); res.json({ ok: true });
} catch (err) {
res
.status(err.status || 500)
.json({ error: err.message || 'Failed to categorize transaction' });
}
}); });
// GET /api/spending/budgets?year=&month= // GET /api/spending/budgets?year=&month=
router.get('/budgets', (req: Req, res: Res) => { router.get('/budgets', (req: Req, res: Res) => {
const ym = parseYM(req.query); 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) }); 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' });
}
}); });
// POST /api/spending/budgets/copy — copy all budgets from previous month into target month // POST /api/spending/budgets/copy — copy all budgets from previous month into target month
router.post('/budgets/copy', (req: Req, res: Res) => { router.post('/budgets/copy', (req: Req, res: Res) => {
const ym = parseYM(req.body || {}); const ym = parseYM(req.body || {});
if (ym.error) return res.status(400).json({ error: ym.error });
// Previous month // Previous month
let prevYear = ym.year, let prevYear = ym.year,
@ -111,7 +89,6 @@ router.post('/budgets/copy', (req: Req, res: Res) => {
prevYear--; prevYear--;
} }
try {
const db = getDb(); const db = getDb();
const prev = db const prev = db
.prepare( .prepare(
@ -138,28 +115,21 @@ router.post('/budgets/copy', (req: Req, res: Res) => {
`); `);
db.transaction(() => { db.transaction(() => {
for (const row of prev) for (const row of prev) upsert.run(req.user.id, row.category_id, ym.year, ym.month, row.amount);
upsert.run(req.user.id, row.category_id, ym.year, ym.month, row.amount);
})(); })();
res.json({ res.json({
copied: prev.length, copied: prev.length,
budgets: getSpendingBudgets(db, req.user.id, ym.year, ym.month), 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' });
}
}); });
// PUT /api/spending/budgets — { category_id, year, month, amount } // PUT /api/spending/budgets — { category_id, year, month, amount }
router.put('/budgets', (req: Req, res: Res) => { router.put('/budgets', (req: Req, res: Res) => {
const { category_id, year, month, amount } = req.body || {}; 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 }); const ym = parseYM({ year, month });
if (ym.error) return res.status(400).json({ error: ym.error });
try {
setSpendingBudget( setSpendingBudget(
getDb(), getDb(),
req.user.id, req.user.id,
@ -169,52 +139,34 @@ router.put('/budgets', (req: Req, res: Res) => {
amount ?? null, amount ?? null,
); );
res.json({ ok: true }); res.json({ ok: true });
} catch (err) {
log.error('[spending/budgets PUT]', err.message);
res.status(500).json({ error: 'Failed to save budget' });
}
}); });
// GET /api/spending/category-rules // GET /api/spending/category-rules
router.get('/category-rules', (req: Req, res: Res) => { router.get('/category-rules', (req: Req, res: Res) => {
try {
res.json({ rules: getSpendingCategoryRules(getDb(), req.user.id) }); 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' });
}
}); });
// POST /api/spending/category-rules — { category_id, merchant } // POST /api/spending/category-rules — { category_id, merchant }
router.post('/category-rules', (req: Req, res: Res) => { router.post('/category-rules', (req: Req, res: Res) => {
const { category_id, merchant } = req.body || {}; const { category_id, merchant } = req.body || {};
if (!category_id || !merchant) if (!category_id || !merchant)
return res.status(400).json({ error: 'category_id and merchant required' }); throw ValidationError(
try { 'category_id and merchant required',
!category_id ? 'category_id' : 'merchant',
);
addSpendingCategoryRule(getDb(), req.user.id, parseInt(category_id, 10), merchant); addSpendingCategoryRule(getDb(), req.user.id, parseInt(category_id, 10), merchant);
res.json({ ok: true }); 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' });
}
}); });
// DELETE /api/spending/category-rules/:id // DELETE /api/spending/category-rules/:id
router.delete('/category-rules/:id', (req: Req, res: Res) => { router.delete('/category-rules/:id', (req: Req, res: Res) => {
try {
deleteSpendingCategoryRule(getDb(), req.user.id, parseInt(req.params.id, 10)); deleteSpendingCategoryRule(getDb(), req.user.id, parseInt(req.params.id, 10));
res.json({ ok: true }); res.json({ ok: true });
} catch (err) {
log.error('[spending/category-rules DELETE]', err.message);
res.status(500).json({ error: 'Failed to delete rule' });
}
}); });
// GET /api/spending/income?year=&month=&page= // GET /api/spending/income?year=&month=&page=
router.get('/income', (req: Req, res: Res) => { router.get('/income', (req: Req, res: Res) => {
const ym = parseYM(req.query); const ym = parseYM(req.query);
if (ym.error) return res.status(400).json({ error: ym.error });
try {
res.json( res.json(
getIncomeTransactions(getDb(), req.user.id, ym.year, ym.month, { getIncomeTransactions(getDb(), req.user.id, ym.year, ym.month, {
page: parseInt(req.query.page || '1', 10), page: parseInt(req.query.page || '1', 10),
@ -222,10 +174,6 @@ router.get('/income', (req: Req, res: Res) => {
includeIgnored: req.query.include_ignored === 'true', includeIgnored: req.query.include_ignored === 'true',
}), }),
); );
} catch (err) {
log.error('[spending/income]', err.message);
res.status(500).json({ error: 'Failed to load income transactions' });
}
}); });
module.exports = router; module.exports = router;

View File

@ -996,7 +996,10 @@ router.post('/unmatch-bulk', (req: Req, res: Res) => {
results.push({ transaction_id: txId, ok: true }); results.push({ transaction_id: txId, ok: true });
} }
} catch (err) { } 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 });
} }
} }
})(); })();