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:
+14
-1
@@ -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 };
|
||||||
|
|||||||
@@ -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
@@ -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');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user