Skip to content

Commit 6f4f735

Browse files
committed
test: close three real coverage gaps surfaced by the review
Three small, well-scoped additions filling gaps the parallel-agent review flagged: 1. cgateConnectionPool: end-to-end cascade-exhaustion test. The pool's execute() path that walks every healthy connection, marks each one bad, and ultimately surfaces "No healthy connections available in pool" wasn't exercised end-to-end (existing tests covered the pre-emptied-set path and the single-failure-then-skip path, but not the drain). Test also advances fake timers so the deferred health-check actually prunes the healthy set, proving the cleanup path runs. 2. webServer CORS: OPTIONS preflight from a disallowed origin. Existing tests cover GET-from-disallowed and OPTIONS-from-allowed; this fills in the most security-relevant remaining cell of the matrix - the preflight denial path that browsers actually rely on. 3. webServer rate-limit: GET / read traffic must NOT be rate-limited even when the mutation budget is exhausted. Also tightens the 429 assertion to verify the error body shape, not just status.
1 parent 223adf5 commit 6f4f735

2 files changed

Lines changed: 97 additions & 0 deletions

File tree

tests/cgateConnectionPool.test.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,37 @@ describe('CgateConnectionPool', () => {
482482
await expect(pool.execute('cmd\n')).rejects.toThrow();
483483
expect(markSpy).toHaveBeenCalled();
484484
});
485+
486+
it('drains the pool when every connection fails during a single execute()', async () => {
487+
// Walk the cascading-failure path end-to-end: start fully healthy,
488+
// make every connection refuse the send, assert execute() rejects
489+
// and every healthy connection was marked unhealthy in turn.
490+
// _markConnectionUnhealthy defers the actual removal via setTimeout;
491+
// we advance fake timers so _checkConnectionHealth then prunes the
492+
// unwritable/destroyed connections from the healthy set, and a
493+
// follow-up execute surfaces the canonical "No healthy connections"
494+
// error - proving the cascade path drains the pool, not just the
495+
// first failed connection.
496+
const connections = await startWithConnections();
497+
const poolSize = connections.length;
498+
expect(poolSize).toBeGreaterThan(0);
499+
for (const c of connections) {
500+
c.sendWithBackpressure = jest.fn().mockResolvedValue(false);
501+
c.isWritable = false;
502+
c.isDestroyed = true;
503+
}
504+
const markSpy = jest.spyOn(pool, '_markConnectionUnhealthy');
505+
506+
await expect(pool.execute('cmd\n')).rejects.toThrow();
507+
508+
expect(markSpy).toHaveBeenCalledTimes(poolSize);
509+
510+
// Drive the deferred health checks queued by _markConnectionUnhealthy.
511+
jest.advanceTimersByTime(1100);
512+
expect(pool.healthyConnections.size).toBe(0);
513+
514+
await expect(pool.execute('cmd\n')).rejects.toThrow('No healthy connections available in pool');
515+
});
485516
});
486517

487518
describe('Health monitoring', () => {

tests/webServer.test.js

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,40 @@ describe('WebServer', () => {
385385
expect(res.status).toBe(204);
386386
expect(res.headers['access-control-allow-origin']).toBe('https://ha.local');
387387
});
388+
389+
it('should omit Access-Control-Allow-Origin on OPTIONS preflight from a disallowed origin', async () => {
390+
const corsServer = new WebServer({
391+
port: 0,
392+
labelLoader,
393+
allowedOrigins: ['https://ha.local'],
394+
getStatus: () => ({})
395+
});
396+
await corsServer.start();
397+
const corsPort = corsServer._server.address().port;
398+
399+
try {
400+
const res = await new Promise((resolve, reject) => {
401+
const req = http.request({
402+
hostname: '127.0.0.1',
403+
port: corsPort,
404+
path: '/api/labels',
405+
method: 'OPTIONS',
406+
headers: { Origin: 'https://evil.example' }
407+
}, (response) => {
408+
resolve({ status: response.statusCode, headers: response.headers });
409+
});
410+
req.on('error', reject);
411+
req.end();
412+
});
413+
414+
// Browsers treat a missing Allow-Origin header on the preflight
415+
// response as a hard CORS denial. Server should NEVER reflect an
416+
// origin that is not in the allowlist.
417+
expect(res.headers['access-control-allow-origin']).toBeUndefined();
418+
} finally {
419+
await corsServer.close();
420+
}
421+
});
388422
});
389423

390424
describe('API key protection', () => {
@@ -556,6 +590,38 @@ describe('WebServer', () => {
556590

557591
const second = await doPatch();
558592
expect(second.status).toBe(429);
593+
expect(second.body).toEqual({ error: 'Too many requests' });
594+
});
595+
596+
it('does not rate-limit GET / read traffic regardless of frequency', async () => {
597+
// Mutation budget is 1 per window, but reads must never be capped -
598+
// a noisy dashboard polling /api/labels shouldn't lock itself out.
599+
limitedServer = new WebServer({
600+
port: 0,
601+
labelLoader,
602+
allowUnauthenticatedMutations: true,
603+
maxMutationRequestsPerWindow: 1,
604+
getStatus: () => ({})
605+
});
606+
await limitedServer.start();
607+
limitedPort = limitedServer._server.address().port;
608+
609+
const doGet = () => new Promise((resolve, reject) => {
610+
http.get({
611+
hostname: '127.0.0.1',
612+
port: limitedPort,
613+
path: '/api/labels'
614+
}, (res) => {
615+
res.on('data', () => {});
616+
res.on('end', () => resolve({ status: res.statusCode }));
617+
}).on('error', reject);
618+
});
619+
620+
// Fire well past the mutation budget on GET - none should 429.
621+
for (let i = 0; i < 5; i++) {
622+
const r = await doGet();
623+
expect(r.status).toBe(200);
624+
}
559625
});
560626

561627
it('removes dormant source entries after the rate limit window passes', () => {

0 commit comments

Comments
 (0)