From 930800eed99c7214cc331c16b6ff7e5abf26866f Mon Sep 17 00:00:00 2001 From: Ulas Kalayci Date: Mon, 4 May 2026 16:51:21 +0200 Subject: [PATCH] fix(cardav): improve router security and test coverage - Remove error message leakage (return generic 'Interner Fehler') - Remove unused imports (str, collectErrors, MAX_TITLE, MAX_URL) - Add _resetTestDatabase() for proper test cleanup - Add test for populated accounts case with shape validation - Import 'after' from node:test for proper teardown Co-Authored-By: Claude Sonnet 4.5 --- server/db.js | 15 ++++++++++++++- server/routes/cardav.js | 6 +----- test-carddav.js | 41 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/server/db.js b/server/db.js index 6275059..3105af3 100644 --- a/server/db.js +++ b/server/db.js @@ -1354,14 +1354,27 @@ function transaction(fn) { return get().transaction(fn)(); } +let _originalDb = null; + /** * ONLY FOR TESTING: Override the internal db instance * @param {import('better-sqlite3').Database} testDb */ function _setTestDatabase(testDb) { + if (!_originalDb) _originalDb = db; db = testDb; } +/** + * ONLY FOR TESTING: Restore the original db instance + */ +function _resetTestDatabase() { + if (_originalDb) { + db = _originalDb; + _originalDb = null; + } +} + init(); // auto-initialise when module is first imported -export { init, get, transaction, currentVersion, getPath, backupToFile, restoreFromFile, MIGRATIONS, _setTestDatabase }; +export { init, get, transaction, currentVersion, getPath, backupToFile, restoreFromFile, MIGRATIONS, _setTestDatabase, _resetTestDatabase }; diff --git a/server/routes/cardav.js b/server/routes/cardav.js index 0cd26a0..971dab4 100644 --- a/server/routes/cardav.js +++ b/server/routes/cardav.js @@ -6,13 +6,9 @@ import { createLogger } from '../logger.js'; import express from 'express'; -import * as db from '../db.js'; import * as CardDAVSync from '../services/cardav-sync.js'; -import { str, collectErrors, MAX_TITLE } from '../middleware/validate.js'; const log = createLogger('CardDAV'); -const MAX_URL = 500; - const router = express.Router(); /** @@ -26,7 +22,7 @@ router.get('/accounts', async (req, res) => { res.json({ data: accounts }); } catch (err) { log.error('Error fetching accounts:', err); - res.status(500).json({ error: err.message || 'Interner Fehler', code: 500 }); + res.status(500).json({ error: 'Interner Fehler', code: 500 }); } }); diff --git a/test-carddav.js b/test-carddav.js index 9699b05..8a6f425 100644 --- a/test-carddav.js +++ b/test-carddav.js @@ -3,7 +3,7 @@ * Purpose: Verify Migration 30 - CardDAV multi-account contacts sync tables */ -import { describe, it, before } from 'node:test'; +import { describe, it, before, after } from 'node:test'; import assert from 'node:assert/strict'; import Database from 'better-sqlite3'; import { MIGRATIONS } from './server/db.js'; @@ -1515,6 +1515,12 @@ describe('CardDAV API Routes', () => { dbModule._setTestDatabase(apiTestDb); }); + after(async () => { + // Restore original database + const dbModule = await import('./server/db.js'); + dbModule._resetTestDatabase(); + }); + describe('Account Management', () => { it('GET /accounts - should return empty array when no accounts', async () => { const cardavRouter = await import('./server/routes/cardav.js'); @@ -1537,5 +1543,38 @@ describe('CardDAV API Routes', () => { assert.ok(Array.isArray(res.data.data)); assert.strictEqual(res.data.data.length, 0); }); + + it('GET /accounts - should return accounts with correct shape', async () => { + // Insert test account + apiTestDb.prepare(` + INSERT INTO carddav_accounts (name, carddav_url, username, password, created_at) + VALUES (?, ?, ?, ?, ?) + `).run('Test iCloud', 'https://contacts.icloud.com', 'test@icloud.com', 'secret', '2026-05-01T10:00:00Z'); + + const cardavRouter = await import('./server/routes/cardav.js'); + const req = { params: {}, query: {}, body: {} }; + const res = { + statusCode: 200, + status(code) { this.statusCode = code; return this; }, + json(data) { this.data = data; return this; }, + }; + + const getHandler = cardavRouter.default.stack.find( + layer => layer.route?.path === '/accounts' && layer.route.methods.get + )?.route?.stack[0]?.handle; + + await getHandler(req, res); + + assert.strictEqual(res.statusCode, 200); + assert.ok(Array.isArray(res.data.data)); + assert.strictEqual(res.data.data.length, 1); + + const account = res.data.data[0]; + assert.strictEqual(account.name, 'Test iCloud'); + assert.strictEqual(account.cardavUrl, 'https://contacts.icloud.com'); + assert.strictEqual(account.username, 'test@icloud.com'); + assert.strictEqual(account.createdAt, '2026-05-01T10:00:00Z'); + assert.ok(!account.password, 'Password should not be exposed'); + }); }); });