Merge pull request #202 from theta42/fix-nmap-rttvar-false-failure

fix(discovery): recover nmap scans that succeed despite a benign RTTVAR stderr warning

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",

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 {
+          return;
-          reject(error);
         }
+        // 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();

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 = '<nmaprun>...</nmaprun>';
+      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;
+    }
+  });
 });