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 <noreply@anthropic.com>
This commit is contained in:
Ulas Kalayci
2026-05-04 16:51:21 +02:00
parent cf68bff25f
commit 930800eed9
3 changed files with 55 additions and 7 deletions
+14 -1
View File
@@ -1354,14 +1354,27 @@ function transaction(fn) {
return get().transaction(fn)(); return get().transaction(fn)();
} }
let _originalDb = null;
/** /**
* ONLY FOR TESTING: Override the internal db instance * ONLY FOR TESTING: Override the internal db instance
* @param {import('better-sqlite3').Database} testDb * @param {import('better-sqlite3').Database} testDb
*/ */
function _setTestDatabase(testDb) { function _setTestDatabase(testDb) {
if (!_originalDb) _originalDb = db;
db = testDb; 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 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 };
+1 -5
View File
@@ -6,13 +6,9 @@
import { createLogger } from '../logger.js'; import { createLogger } from '../logger.js';
import express from 'express'; import express from 'express';
import * as db from '../db.js';
import * as CardDAVSync from '../services/cardav-sync.js'; import * as CardDAVSync from '../services/cardav-sync.js';
import { str, collectErrors, MAX_TITLE } from '../middleware/validate.js';
const log = createLogger('CardDAV'); const log = createLogger('CardDAV');
const MAX_URL = 500;
const router = express.Router(); const router = express.Router();
/** /**
@@ -26,7 +22,7 @@ router.get('/accounts', async (req, res) => {
res.json({ data: accounts }); res.json({ data: accounts });
} catch (err) { } catch (err) {
log.error('Error fetching accounts:', 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 });
} }
}); });
+40 -1
View File
@@ -3,7 +3,7 @@
* Purpose: Verify Migration 30 - CardDAV multi-account contacts sync tables * 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 assert from 'node:assert/strict';
import Database from 'better-sqlite3'; import Database from 'better-sqlite3';
import { MIGRATIONS } from './server/db.js'; import { MIGRATIONS } from './server/db.js';
@@ -1515,6 +1515,12 @@ describe('CardDAV API Routes', () => {
dbModule._setTestDatabase(apiTestDb); dbModule._setTestDatabase(apiTestDb);
}); });
after(async () => {
// Restore original database
const dbModule = await import('./server/db.js');
dbModule._resetTestDatabase();
});
describe('Account Management', () => { describe('Account Management', () => {
it('GET /accounts - should return empty array when no accounts', async () => { it('GET /accounts - should return empty array when no accounts', async () => {
const cardavRouter = await import('./server/routes/cardav.js'); const cardavRouter = await import('./server/routes/cardav.js');
@@ -1537,5 +1543,38 @@ describe('CardDAV API Routes', () => {
assert.ok(Array.isArray(res.data.data)); assert.ok(Array.isArray(res.data.data));
assert.strictEqual(res.data.data.length, 0); 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');
});
}); });
}); });