diff --git a/nodejs/package.json b/nodejs/package.json index 330ca3b..4556858 100755 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -11,7 +11,7 @@ "scripts": { "start": "node ./bin/www", "dev": "npx nodemon --ignore public/ ./bin/www", - "test": "NODE_ENV=test jest tests/groups.test.js tests/subtypes.test.js tests/site_join.test.js tests/site_config.test.js tests/site_replicate.test.js tests/proxy_client.test.js tests/reconciler.test.js --forceExit" + "test": "NODE_ENV=test jest tests/groups.test.js tests/subtypes.test.js tests/site_join.test.js tests/site_config.test.js tests/site_replicate.test.js tests/proxy_client.test.js tests/reconciler.test.js tests/nmap_plugin.test.js --forceExit" }, "jest": { "testEnvironment": "node", diff --git a/nodejs/plugins/discovery/nmap.js b/nodejs/plugins/discovery/nmap.js index 011b88d..0e3b74e 100644 --- a/nodejs/plugins/discovery/nmap.js +++ b/nodejs/plugins/discovery/nmap.js @@ -88,9 +88,28 @@ module.exports = { var msg = (error && error.message) || String(error); if (/nmap.*not found|command location/i.test(msg)) { reject(new Error('nmap binary not installed in the container image (rebuild with Dockerfile.openldap, which apk-adds nmap)')); - } else { - reject(error); + return; } + // node-nmap (node_modules/node-nmap/index.js) treats ANY stderr + // output from the nmap binary as a fatal scan error -- including + // nmap's own benign RTT timing-calibration warnings ("RTTVAR has + // grown to over N seconds, decreasing to M"), which it prints + // *during* a scan that goes on to complete normally. That means a + // scan that actually succeeded (valid XML already sitting in + // scan.rawData) got thrown away and reported as a failed run with + // zero hosts discovered -- not just a noisy log line. Recover by + // manually re-running node-nmap's own XML-parse-then-complete path + // (rawDataHandler -> scanComplete -> the 'complete' listener above) + // when the "error" is this specific known-benign nmap message and + // there's actually output to parse. A genuine XML parse failure + // re-emits 'error' with a different message, which falls through to + // reject() below same as before -- this only widens the recovery + // path, it doesn't swallow real failures. + if (/RTTVAR has grown/i.test(msg) && scan.rawData) { + scan.rawDataHandler(scan.rawData); + return; + } + reject(error); }); scan.startScan(); diff --git a/nodejs/tests/nmap_plugin.test.js b/nodejs/tests/nmap_plugin.test.js index fc9b178..53ee302 100644 --- a/nodejs/tests/nmap_plugin.test.js +++ b/nodejs/tests/nmap_plugin.test.js @@ -10,6 +10,7 @@ jest.mock('node-nmap', () => { this.targetRange = targetRange; this.customFlags = customFlags; this.command = ['-oX', '-', ...(customFlags || []), targetRange]; + this.rawData = ''; } startScan() { setImmediate(() => { @@ -18,6 +19,15 @@ jest.mock('node-nmap', () => { ]); }); } + // Real node-nmap's rawDataHandler XML-parses this.rawData then calls + // this.scanComplete(results), which emits 'complete' -- the mock skips + // straight to emitting the same shape so the RTTVAR-recovery test below + // exercises the exact call our plugin code makes. + rawDataHandler() { + this.emit('complete', [ + { ip: '192.168.1.20', hostname: 'host-20', openPorts: [] } + ]); + } } return { NmapScan: MockNmapScan, @@ -43,4 +53,42 @@ describe('nmap discovery plugin', () => { expect(result.resources[0].name).toBe('host-10'); expect(result.edges).toHaveLength(1); }); + + test('recovers a scan that completed despite nmap\'s benign RTTVAR stderr warning', async () => { + // Regression: node-nmap treats ANY stderr output as fatal, including + // nmap's own harmless RTT-calibration message -- which discards a scan + // that actually succeeded. Simulate that by emitting 'error' with the + // RTTVAR text instead of 'complete', with rawData present. + const nmapModule = require('node-nmap'); + const originalStartScan = nmapModule.NmapScan.prototype.startScan; + nmapModule.NmapScan.prototype.startScan = function () { + this.rawData = '...'; + setImmediate(() => { + this.emit('error', new Error('RTTVAR has grown to over 2.3 seconds, decreasing to 2.0')); + }); + }; + + try { + const result = await nmapPlugin.discover({ targetRange: '192.168.1.0/24' }); + expect(result.resources.some((r) => r.name === 'host-20')).toBe(true); + } finally { + nmapModule.NmapScan.prototype.startScan = originalStartScan; + } + }); + + test('still rejects a genuine error even when the message differs from RTTVAR', async () => { + const nmapModule = require('node-nmap'); + const originalStartScan = nmapModule.NmapScan.prototype.startScan; + nmapModule.NmapScan.prototype.startScan = function () { + setImmediate(() => { + this.emit('error', new Error('nmap: permission denied')); + }); + }; + + try { + await expect(nmapPlugin.discover({ targetRange: '192.168.1.0/24' })).rejects.toThrow('permission denied'); + } finally { + nmapModule.NmapScan.prototype.startScan = originalStartScan; + } + }); });