fix(scanner): patch unauthenticated arbitrary file write (GHSA-ppp9-2hc2-hfg5)

.env.example

@@ -15,6 +15,11 @@ RATE_LIMIT_ENABLED=true
 RATE_LIMIT_MAX=100
 RATE_LIMIT_WINDOW_MS=900000 # 15 minutes
 
+# Scanner module: token required on side-effecting routes (scan/export and the
+# /tools/scanner tool endpoint). If unset, those routes are unauthenticated.
+# Send as `Authorization: Bearer <token>` or `X-API-Key: <token>`.
+SCANNER_API_TOKEN=
+
 # Module settings
 MODULES_AUTOLOAD=
 

mcp_modules/scanner/index.js

@@ -15,6 +15,7 @@ import {
   exportReport,
 } from './src/controller.js';
 import { scannerService } from './src/service.js';
+import { requireScannerAuth } from './src/auth.js';
 
 /**
  * Register this module with the Hono app
@@ -33,13 +34,16 @@ export async function register(app) {
     });
   });
 
-  // Register scan routes
+  // Register scan routes.
+  // Read-only routes stay open; side-effecting routes (start a scan / spawn a
+  // child process, write a report to disk) require authentication when a
+  // SCANNER_API_TOKEN is configured — see GHSA-ppp9-2hc2-hfg5.
   app.get('/scanner/scans', getScanHistory);
   app.get('/scanner/scans/:id', getScanById);
   app.get('/scanner/stats', getScanStats);
-  app.post('/scanner/scan', scanTarget);
+  app.post('/scanner/scan', requireScannerAuth, scanTarget);
   app.get('/scanner/reports/:id', generateReport);
-  app.get('/scanner/reports/:id/export', exportReport);
+  app.get('/scanner/reports/:id/export', requireScannerAuth, exportReport);
 
   // Register MCP tool
   app.get('/tools/scanner/info', c => {
@@ -81,8 +85,9 @@ export async function register(app) {
     });
   });
 
-  // Register MCP tool endpoint
-  app.post('/tools/scanner', async c => {
+  // Register MCP tool endpoint. This dispatches to side-effecting actions
+  // (scan, export), so it is guarded the same way as the direct routes.
+  app.post('/tools/scanner', requireScannerAuth, async c => {
     try {
       const params = await c.req.json();
 

mcp_modules/scanner/src/auth.js

@@ -0,0 +1,78 @@
+/**
+ * Scanner authentication middleware
+ *
+ * Guards the scanner's side-effecting routes (scan, export, the /tools/scanner
+ * tool endpoint) that were previously reachable with no authentication at all
+ * (see GHSA-ppp9-2hc2-hfg5). A bearer token / API key is required when
+ * `SCANNER_API_TOKEN` is configured.
+ *
+ * Behaviour:
+ *   - `SCANNER_API_TOKEN` set   → the token must be supplied as
+ *       `Authorization: Bearer <token>` or `X-API-Key: <token>`, else 401.
+ *   - `SCANNER_API_TOKEN` unset → requests are allowed (preserves out-of-box
+ *       behaviour) but a one-time warning is logged so operators know the
+ *       side-effecting routes are open.
+ */
+
+import { createHash, timingSafeEqual } from 'crypto';
+import { logger } from '../../../src/utils/logger.js';
+
+let missingTokenWarned = false;
+
+/**
+ * Extract a presented credential from the request.
+ * Supports `Authorization: Bearer <token>` and `X-API-Key: <token>`.
+ * @param {import('hono').Context} c
+ * @returns {string}
+ */
+function extractToken(c) {
+  const authz = c.req.header('authorization') || c.req.header('Authorization');
+  if (authz && authz.toLowerCase().startsWith('bearer ')) {
+    return authz.slice(7).trim();
+  }
+  return c.req.header('x-api-key') || c.req.header('X-API-Key') || '';
+}
+
+/**
+ * Constant-time comparison of two strings.
+ * Both sides are hashed to a fixed length first so the comparison neither
+ * throws on length mismatch nor leaks the expected token length.
+ * @param {string} a
+ * @param {string} b
+ * @returns {boolean}
+ */
+function safeEqual(a, b) {
+  const ha = createHash('sha256').update(String(a)).digest();
+  const hb = createHash('sha256').update(String(b)).digest();
+  return timingSafeEqual(ha, hb);
+}
+
+/**
+ * Hono middleware requiring a valid scanner API token on side-effecting routes.
+ * @param {import('hono').Context} c
+ * @param {() => Promise<void>} next
+ */
+export async function requireScannerAuth(c, next) {
+  const token = process.env.SCANNER_API_TOKEN || '';
+
+  if (!token) {
+    if (!missingTokenWarned) {
+      logger.warn(
+        'SCANNER_API_TOKEN is not set — scanner side-effecting routes (scan/export) are UNAUTHENTICATED. ' +
+          'Set SCANNER_API_TOKEN to require a bearer token / X-API-Key.'
+      );
+      missingTokenWarned = true;
+    }
+    return next();
+  }
+
+  const provided = extractToken(c);
+  if (!provided || !safeEqual(provided, token)) {
+    return c.json({ error: 'Unauthorized' }, 401);
+  }
+
+  return next();
+}
+
+// Exposed for tests.
+export const _internal = { safeEqual, extractToken };

mcp_modules/scanner/src/service.js

@@ -6,7 +6,7 @@
  * web application security scanning capabilities.
  */
 
-import { join } from 'path';
+import { join, resolve, sep, dirname } from 'path';
 import { homedir } from 'os';
 import { existsSync, mkdirSync, writeFileSync, readFileSync } from 'fs';
 import { spawn } from 'child_process';
@@ -441,14 +441,32 @@ export class ScannerService {
       // Verify scan exists
       this.getScanById(scanId);
       const format = options.format || 'json';
-      let destination = options.destination;
-
-      if (!destination) {
-        destination = join(this.dataDir, 'reports', `${scanId}-report.${format}`);
-        // Ensure the reports directory exists
-        const reportsDir = join(this.dataDir, 'reports');
-        if (!existsSync(reportsDir)) {
-          mkdirSync(reportsDir, { recursive: true });
+
+      // All reports are confined to this directory. Ensure it exists.
+      const reportsDir = resolve(join(this.dataDir, 'reports'));
+      if (!existsSync(reportsDir)) {
+        mkdirSync(reportsDir, { recursive: true });
+      }
+
+      let destination;
+      if (!options.destination) {
+        destination = join(reportsDir, `${scanId}-report.${format}`);
+      } else {
+        // SECURITY: `destination` is attacker-controllable from unauthenticated
+        // HTTP entry points. Treat it as a path relative to the reports directory
+        // and reject anything that escapes it (absolute paths, `..` traversal,
+        // symlink-style resolution). Prevents arbitrary file write (GHSA-ppp9-2hc2-hfg5).
+        const resolved = resolve(reportsDir, options.destination);
+        if (resolved !== reportsDir && !resolved.startsWith(reportsDir + sep)) {
+          throw new Error(
+            `Invalid destination: path must stay within the reports directory (${reportsDir})`
+          );
+        }
+        destination = resolved;
+        // Create any nested sub-directory the caller asked for (still inside reportsDir).
+        const destDir = dirname(destination);
+        if (!existsSync(destDir)) {
+          mkdirSync(destDir, { recursive: true });
         }
       }
 

mcp_modules/scanner/test/auth.test.js

@@ -0,0 +1,98 @@
+/**
+ * Tests for scanner authentication middleware (GHSA-ppp9-2hc2-hfg5 hardening).
+ */
+
+import { describe, it, beforeEach, afterEach } from 'mocha';
+import { expect, sinon } from './setup.js';
+import { requireScannerAuth } from '../src/auth.js';
+
+/**
+ * Build a fake Hono context with the given request headers.
+ */
+const makeContext = (headers = {}) => {
+  const lower = {};
+  for (const [k, v] of Object.entries(headers)) {
+    lower[k.toLowerCase()] = v;
+  }
+  return {
+    req: {
+      header: name => lower[String(name).toLowerCase()],
+    },
+    json: sinon.stub().callsFake((body, status) => ({ body, status: status || 200 })),
+  };
+};
+
+describe('requireScannerAuth', () => {
+  const originalToken = process.env.SCANNER_API_TOKEN;
+
+  afterEach(() => {
+    sinon.restore();
+    if (originalToken === undefined) {
+      delete process.env.SCANNER_API_TOKEN;
+    } else {
+      process.env.SCANNER_API_TOKEN = originalToken;
+    }
+  });
+
+  describe('when SCANNER_API_TOKEN is not configured', () => {
+    beforeEach(() => {
+      delete process.env.SCANNER_API_TOKEN;
+    });
+
+    it('allows the request through (backward compatible)', async () => {
+      const c = makeContext();
+      const next = sinon.stub().resolves();
+      await requireScannerAuth(c, next);
+      expect(next.calledOnce).to.be.true;
+      expect(c.json.called).to.be.false;
+    });
+  });
+
+  describe('when SCANNER_API_TOKEN is configured', () => {
+    const TOKEN = 'super-secret-token';
+
+    beforeEach(() => {
+      process.env.SCANNER_API_TOKEN = TOKEN;
+    });
+
+    it('rejects a request with no credential (401)', async () => {
+      const c = makeContext();
+      const next = sinon.stub().resolves();
+      await requireScannerAuth(c, next);
+      expect(next.called).to.be.false;
+      expect(c.json.calledWith(sinon.match.any, 401)).to.be.true;
+    });
+
+    it('rejects a wrong bearer token (401)', async () => {
+      const c = makeContext({ Authorization: 'Bearer wrong-token' });
+      const next = sinon.stub().resolves();
+      await requireScannerAuth(c, next);
+      expect(next.called).to.be.false;
+      expect(c.json.calledWith(sinon.match.any, 401)).to.be.true;
+    });
+
+    it('allows a correct bearer token', async () => {
+      const c = makeContext({ Authorization: `Bearer ${TOKEN}` });
+      const next = sinon.stub().resolves();
+      await requireScannerAuth(c, next);
+      expect(next.calledOnce).to.be.true;
+      expect(c.json.called).to.be.false;
+    });
+
+    it('allows a correct X-API-Key', async () => {
+      const c = makeContext({ 'X-API-Key': TOKEN });
+      const next = sinon.stub().resolves();
+      await requireScannerAuth(c, next);
+      expect(next.calledOnce).to.be.true;
+      expect(c.json.called).to.be.false;
+    });
+
+    it('rejects a wrong X-API-Key (401)', async () => {
+      const c = makeContext({ 'X-API-Key': 'nope' });
+      const next = sinon.stub().resolves();
+      await requireScannerAuth(c, next);
+      expect(next.called).to.be.false;
+      expect(c.json.calledWith(sinon.match.any, 401)).to.be.true;
+    });
+  });
+});

mcp_modules/scanner/test/service.test.js

@@ -2,9 +2,12 @@
  * Tests for the scanner module service
  */
 
-import { describe, it, afterEach } from 'mocha';
+import { describe, it, beforeEach, afterEach } from 'mocha';
+import { existsSync, rmSync } from 'fs';
+import { join, resolve, sep } from 'path';
 import { expect, sinon } from './setup.js';
 import * as utils from '../src/utils.js';
+import { ScannerService } from '../src/service.js';
 
 describe('ScannerService', () => {
   afterEach(() => {
@@ -97,4 +100,53 @@ describe('ScannerService', () => {
       expect(result).to.equal('0s');
     });
   });
+
+  // Regression tests for GHSA-ppp9-2hc2-hfg5:
+  // unauthenticated arbitrary file write via unvalidated `destination` in exportReport.
+  describe('exportReport destination confinement', () => {
+    const scanId = 'scan-security-test';
+    let service;
+
+    const stubScan = svc => {
+      sinon.stub(svc, 'getScanById').returns({ id: scanId, status: 'completed' });
+      sinon.stub(svc, 'generateReport').resolves('<html>report</html>');
+    };
+
+    beforeEach(() => {
+      service = new ScannerService();
+      stubScan(service);
+    });
+
+    const maliciousPaths = [
+      '/tmp/pwned.html',
+      '/etc/cron.d/x',
+      '../../../../tmp/pwned.json',
+      '../../.bashrc',
+    ];
+
+    maliciousPaths.forEach(destination => {
+      it(`rejects out-of-bounds destination: ${destination}`, async () => {
+        let threw = false;
+        try {
+          await service.exportReport(scanId, { format: 'html', destination });
+        } catch (err) {
+          threw = true;
+          expect(err.message).to.match(/reports directory|Invalid destination/i);
+        }
+        expect(threw, 'exportReport should reject the malicious destination').to.be.true;
+      });
+    });
+
+    it('allows a plain filename inside the reports directory', async () => {
+      const reportsDir = resolve(join(service.dataDir, 'reports'));
+      const result = await service.exportReport(scanId, {
+        format: 'html',
+        destination: 'safe-report.html',
+      });
+      const written = resolve(result.destination);
+      expect(written === reportsDir || written.startsWith(reportsDir + sep)).to.be.true;
+      expect(existsSync(written)).to.be.true;
+      rmSync(written, { force: true });
+    });
+  });
 });