From 5ea7896354c8f935dabd5dd83094b4dea61d7b74 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 13 Jul 2026 12:45:27 +0000 Subject: [PATCH 1/5] fix(scanner): confine exportReport destination to reports dir (GHSA-ppp9-2hc2-hfg5) Unauthenticated arbitrary file write: `destination` flowed from two HTTP entry points (GET /scanner/reports/:id/export and POST /tools/scanner) straight into fs.writeFileSync() with zero validation, allowing overwrite of any path the process could write (root, in the shipped Dockerfile). Resolve an attacker-supplied `destination` against the reports directory and reject anything that escapes it (absolute paths and `..` traversal). Legitimate relative filenames still work. Adds regression tests covering the advisory's PoC paths. Co-Authored-By: Claude Opus 4.8 --- mcp_modules/scanner/src/service.js | 36 ++++++++++++---- mcp_modules/scanner/test/service.test.js | 54 +++++++++++++++++++++++- 2 files changed, 80 insertions(+), 10 deletions(-) diff --git a/mcp_modules/scanner/src/service.js b/mcp_modules/scanner/src/service.js index 8d9ba79..f8c4f11 100644 --- a/mcp_modules/scanner/src/service.js +++ b/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 }); } } diff --git a/mcp_modules/scanner/test/service.test.js b/mcp_modules/scanner/test/service.test.js index f8781d4..82eb05d 100644 --- a/mcp_modules/scanner/test/service.test.js +++ b/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('report'); + }; + + 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 }); + }); + }); }); From 34c13e0a46c2dc2ce336d78e12bdcc30eb9585c0 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 13 Jul 2026 12:55:11 +0000 Subject: [PATCH 2/5] fix(scanner): require auth on side-effecting routes (GHSA-ppp9-2hc2-hfg5) Defense-in-depth for the scanner advisory: the routes that start a scan (spawns a child process) and export a report (writes to disk) had no auth middleware at all. Add a bearer-token / X-API-Key guard (requireScannerAuth) on POST /scanner/scan, GET /scanner/reports/:id/export and POST /tools/scanner. Token is read from SCANNER_API_TOKEN; comparison is constant-time. When the env var is unset the routes stay open (backward compatible) but log a one-time warning. Read-only routes remain unauthenticated. Adds middleware tests and documents the env var in .env.example. Co-Authored-By: Claude Opus 4.8 --- .env.example | 5 ++ mcp_modules/scanner/index.js | 15 ++-- mcp_modules/scanner/src/auth.js | 78 +++++++++++++++++++++ mcp_modules/scanner/test/auth.test.js | 98 +++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 5 deletions(-) create mode 100644 mcp_modules/scanner/src/auth.js create mode 100644 mcp_modules/scanner/test/auth.test.js diff --git a/.env.example b/.env.example index 3a25e73..1ef3e11 100644 --- a/.env.example +++ b/.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 ` or `X-API-Key: `. +SCANNER_API_TOKEN= + # Module settings MODULES_AUTOLOAD= diff --git a/mcp_modules/scanner/index.js b/mcp_modules/scanner/index.js index 9dea9d6..f0af3c0 100644 --- a/mcp_modules/scanner/index.js +++ b/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(); diff --git a/mcp_modules/scanner/src/auth.js b/mcp_modules/scanner/src/auth.js new file mode 100644 index 0000000..f6158da --- /dev/null +++ b/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 ` or `X-API-Key: `, 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 ` and `X-API-Key: `. + * @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} 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 }; diff --git a/mcp_modules/scanner/test/auth.test.js b/mcp_modules/scanner/test/auth.test.js new file mode 100644 index 0000000..2f0be91 --- /dev/null +++ b/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; + }); + }); +}); From cf19506cf19765b0fe6143503bbcb644bd035173 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 13 Jul 2026 13:13:37 +0000 Subject: [PATCH 3/5] ci: fix failing security workflow (semgrep, npm audit, gitleaks) - semgrep (dockerfile.security.missing-user): run the container as the built-in non-root `node` user instead of root. Also hardens the arbitrary-write advisory's "Dockerfile runs as root" impact note. - npm audit (GHSA-5c6j-r48x-rmvq): override transitive serialize-javascript (pulled dev-only via mocha) to ^7.0.0, which is patched. Added to both npm `overrides` and `pnpm.overrides`. - gitleaks: add .gitleaks.toml extending the default ruleset with an allowlist for documentation placeholders and the intentionally-shipped hynt.us default key; wire --config into the workflow. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/security.yml | 2 +- .gitleaks.toml | 26 ++++++++++++++++++++++ Dockerfile | 18 +++++++++------ package.json | 8 +++++++ pnpm-lock.yaml | 40 ++++++++++++++++------------------ 5 files changed, 65 insertions(+), 29 deletions(-) create mode 100644 .gitleaks.toml diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 242b1d2..73d0e05 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -55,4 +55,4 @@ jobs: | tar -xz -C /usr/local/bin gitleaks gitleaks version - name: Scan history - run: gitleaks detect --source . --redact --verbose --no-banner --exit-code 1 + run: gitleaks detect --source . --config .gitleaks.toml --redact --verbose --no-banner --exit-code 1 diff --git a/.gitleaks.toml b/.gitleaks.toml new file mode 100644 index 0000000..4323843 --- /dev/null +++ b/.gitleaks.toml @@ -0,0 +1,26 @@ +# Gitleaks configuration. +# Extends the built-in default ruleset and allowlists known non-sensitive +# values so the secret scanner stops failing on documentation placeholders +# and the intentionally-shipped default key. + +[extend] +useDefault = true + +[allowlist] +description = "Documentation placeholders and the shipped default key" +regexTarget = "match" +regexes = [ + # Placeholder tokens used in example requests throughout the docs. + '''your-api-key-here''', + '''your_valueserp_api_key''', + '''sk-your-openai-api-key''', + '''your[-_]?api[-_]?key''', + # Obvious dummy value used in link-shortener tests. + '''abcdefghij1234567890''', + # hynt.us default API key intentionally shipped by the link-shortener module + # (documented as the "Default API Key" and used as a hardcoded fallback). + # NOTE: this is a real, shared key — consider rotating it and moving to an + # env var rather than committing it. Allowlisted so CI reflects that it is a + # known, deliberate value rather than an accidental leak. + '''1t7nfaw9ra0nmznsbdni''', +] diff --git a/Dockerfile b/Dockerfile index 5b0331d..5e51474 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,23 +1,27 @@ # Use Node.js LTS as the base image FROM node:20-alpine -# Set working directory +# Install pnpm (as root, into the global prefix) +RUN npm install -g pnpm + +# Set working directory and hand it to the built-in non-root `node` user WORKDIR /app +RUN chown node:node /app -# Copy package files -COPY package.json pnpm-lock.yaml ./ +# Copy package files (owned by the non-root user so pnpm can write to the tree) +COPY --chown=node:node package.json pnpm-lock.yaml ./ -# Install pnpm -RUN npm install -g pnpm +# Drop privileges before installing/running — never run the app as root +USER node # Install dependencies RUN pnpm install --frozen-lockfile # Copy application code -COPY . . +COPY --chown=node:node . . # Expose the port the app runs on EXPOSE 3000 # Command to run the application -CMD ["pnpm", "start"] \ No newline at end of file +CMD ["pnpm", "start"] diff --git a/package.json b/package.json index 7a24299..d1b9c4e 100644 --- a/package.json +++ b/package.json @@ -68,5 +68,13 @@ "*.{json,md}": [ "prettier --write" ] + }, + "overrides": { + "serialize-javascript": "^7.0.0" + }, + "pnpm": { + "overrides": { + "serialize-javascript": "^7.0.0" + } } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 168e3bd..88b0469 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4,6 +4,9 @@ settings: autoInstallPeers: true excludeLinksFromLockfile: false +overrides: + serialize-javascript: ^7.0.0 + importers: .: @@ -152,6 +155,9 @@ packages: '@sinonjs/text-encoding@0.7.3': resolution: {integrity: sha512-DE427ROAphMQzU4ENbliGYrBSYPXF+TtLg9S8vzeA+OF4ZKzoDdzfL8sxuMUGS/lgRhM6j1URSk9ghf7Xo1tyA==} + deprecated: |- + Deprecated: no longer maintained and no longer used by Sinon packages. See + https://github.com/sinonjs/nise/issues/243 for replacement details. '@tootallnate/once@2.0.0': resolution: {integrity: sha512-XCuKFP5PS55gnMVu3dty8KPatLqUoy/ZYzDzAGCQ8JNFCkLXzmI7vNHCR+XpbZaMWQK/vQubr7PkYq8g470J/A==} @@ -174,6 +180,7 @@ packages: '@ungap/structured-clone@1.3.0': resolution: {integrity: sha512-WmoN8qaIAo7WTYWbAZuG8PYEhn5fkz7dZrqTBZ7dtt//lL2Gwms1IcnQ5yHqjDfX8Ft5j4YzDM23f87zBfDe9g==} + deprecated: Potential CWE-502 - Update to 1.3.1 or higher abab@2.0.6: resolution: {integrity: sha512-j2afSsaIENvHZN2B8GOpF566vZ5WVk5opAiMTvWgaQT8DkbOqsTfvNAvHoRGU2zzP8cPoqys+xHTRDWW8L+/BA==} @@ -287,6 +294,7 @@ packages: basic-ftp@5.0.5: resolution: {integrity: sha512-4Bcg1P8xhUuqcii/S0Z9wiHIrQVPMermM1any+MX5GeGD7faD3/msQUDGLol9wOcz4/jbg/WJnGqoJF6LiBdtg==} engines: {node: '>=10.0.0'} + deprecated: Security vulnerability fixed in 5.2.1, please upgrade binary-extensions@2.3.0: resolution: {integrity: sha512-Ceh+7ox5qe7LJuLHoY0feh3pHuUDHAcRUeyL2VYghZwfpkNIy/+8Ocg0a3UuSoYzavmylwuLWQOf3hl0jjMMIw==} @@ -708,12 +716,12 @@ packages: glob@7.2.3: resolution: {integrity: sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==} - deprecated: Glob versions prior to v9 are no longer supported + deprecated: Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me glob@8.1.0: resolution: {integrity: sha512-r8hpEjiQEYlF2QU0df3dS+nxxSIreXQS1qRhMJM0Q5NDdR386C7jb7Hwwod8Fgiuex+k0GFjgft18yvxm5XoCQ==} engines: {node: '>=12'} - deprecated: Glob versions prior to v9 are no longer supported + deprecated: Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me globals@13.24.0: resolution: {integrity: sha512-AhO5QUcj8llrbG09iWhPU2B204J1xnPeL8kQmVorSsy+Sjj1sk8gIyh6cUocGmH4L0UuhAJy+hJMRA4mgA4mFQ==} @@ -1231,6 +1239,7 @@ packages: puppeteer@24.8.2: resolution: {integrity: sha512-Sn6SBPwJ6ASFvQ7knQkR+yG7pcmr4LfXzmoVp3NR0xXyBbPhJa8a8ybtb6fnw1g/DD/2t34//yirubVczko37w==} engines: {node: '>=18'} + deprecated: < 24.15.0 is no longer supported hasBin: true qs@6.14.0: @@ -1243,9 +1252,6 @@ packages: queue-microtask@1.2.3: resolution: {integrity: sha512-NuaNSa6flKT5JaSYQzJok04JzTL1CA6aGhv5rfLW3PgqA+M2ChpZQnAC8h8i4ZFkBS8X5RqkDBHA7r4hej3K9A==} - randombytes@2.1.0: - resolution: {integrity: sha512-vYl3iOX+4CKUWuxGi9Ukhie6fsqXqS9FE2Zaic4tNFD2N2QQaXOMFbuKK4QmDHC0JO6B1Zp41J0LpT0oR68amQ==} - readdirp@3.6.0: resolution: {integrity: sha512-hOS089on8RduqdbhvQ5Z37A0ESjsqz6qnRcffsMU3495FuTdqSm+7bhJ29JvIOsBDEEnan5DPu9t3To9VRlMzA==} engines: {node: '>=8.10.0'} @@ -1283,9 +1289,6 @@ packages: run-parallel@1.2.0: resolution: {integrity: sha512-5l4VyZR86LZ/lDxZTR6jqL8AFE2S0IFLMP26AbjsLVADxHdhB/c0GUsH+y39UfCi3dzz8OlQuPmnaJOMoDHQBA==} - safe-buffer@5.2.1: - resolution: {integrity: sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ==} - safer-buffer@2.1.2: resolution: {integrity: sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==} @@ -1298,8 +1301,9 @@ packages: engines: {node: '>=10'} hasBin: true - serialize-javascript@6.0.2: - resolution: {integrity: sha512-Saa1xPByTTq2gdeFZYLLo+RFE35NHZkAbqZeWNd3BpzppeVisAqpDjcp8dyf6uIvEqJRd46jemmyA4iFIeVk8g==} + serialize-javascript@7.0.7: + resolution: {integrity: sha512-YAy8Od6KV+uuwUuU50np8fGB/Aues6Y0nAhA9y/hId74PlKUcme4pXcBD46NWKr1Q4osN/iseZ17YqO1XfmI8g==} + engines: {node: '>=20.0.0'} shallow-clone@0.1.2: resolution: {integrity: sha512-J1zdXCky5GmNnuauESROVu31MQSnLoYvlyEn6j2Ztk6Q5EHFIhxkMhYcv6vuDzl2XEzoRr856QwzMgWM/TmZgw==} @@ -1397,11 +1401,12 @@ packages: superagent@8.1.2: resolution: {integrity: sha512-6WTxW1EB6yCxV5VFOIPQruWGHqc3yI7hEmZK6h+pyk69Lk/Ut7rLUY6W/ONF2MjBuGjvmMiIpsrVJ2vjrHlslA==} engines: {node: '>=6.4.0 <13 || >=14'} - deprecated: Please upgrade to v9.0.0+ as we have fixed a public vulnerability with formidable dependency. Note that v9.0.0+ requires Node.js v14.18.0+. See https://github.com/ladjs/superagent/pull/1800 for insight. This project is supported and maintained by the team at Forward Email @ https://forwardemail.net + deprecated: Please upgrade to superagent v10.2.2+, see release notes at https://github.com/forwardemail/superagent/releases/tag/v10.2.2 - maintenance is supported by Forward Email @ https://forwardemail.net supertest@6.3.4: resolution: {integrity: sha512-erY3HFDG0dPnhw4U+udPfrzXa4xhSG+n4rxfRuZWCUvjFWwKl+OxWf/7zk50s84/fAAs7vf5QAb9uRa0cCykxw==} engines: {node: '>=6.4.0'} + deprecated: Please upgrade to supertest v7.1.3+, see release notes at https://github.com/forwardemail/supertest/releases/tag/v7.1.3 - maintenance is supported by Forward Email @ https://forwardemail.net supports-color@7.2.0: resolution: {integrity: sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==} @@ -1492,6 +1497,7 @@ packages: whatwg-encoding@2.0.0: resolution: {integrity: sha512-p41ogyeMUrw3jWclHWTQg1k05DSVXPLcVxRTYsXUk+ZooOCZLcoYgPZ/HL/D/N+uQPOtcp1me1WhBEaX02mhWg==} engines: {node: '>=12'} + deprecated: Use @exodus/bytes instead for a more spec-conformant and faster implementation whatwg-mimetype@3.0.0: resolution: {integrity: sha512-nt+N2dzIutVRxARx1nghPKGv1xHikU7HKdfafKkLNLindmPU/ch3U31NOCGGA/dmPcmb1VlofO0vnKAcsm0o/Q==} @@ -2578,7 +2584,7 @@ snapshots: log-symbols: 4.1.0 minimatch: 5.1.6 ms: 2.1.3 - serialize-javascript: 6.0.2 + serialize-javascript: 7.0.7 strip-json-comments: 3.1.1 supports-color: 8.1.1 workerpool: 6.5.1 @@ -2814,10 +2820,6 @@ snapshots: queue-microtask@1.2.3: {} - randombytes@2.1.0: - dependencies: - safe-buffer: 5.2.1 - readdirp@3.6.0: dependencies: picomatch: 2.3.1 @@ -2847,8 +2849,6 @@ snapshots: dependencies: queue-microtask: 1.2.3 - safe-buffer@5.2.1: {} - safer-buffer@2.1.2: {} saxes@6.0.0: @@ -2857,9 +2857,7 @@ snapshots: semver@7.7.1: {} - serialize-javascript@6.0.2: - dependencies: - randombytes: 2.1.0 + serialize-javascript@7.0.7: {} shallow-clone@0.1.2: dependencies: From ed50f7f0ebb83ed34d5e3b4a7267cc3718ff0d7e Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 13 Jul 2026 13:40:02 +0000 Subject: [PATCH 4/5] fix(scanner): use double-HMAC constant-time token compare (CodeQL) CodeQL flagged js/insufficient-password-hash on the plain SHA-256 digest of the API token used to normalise lengths for timingSafeEqual. Switch to the canonical double-HMAC comparison: HMAC-SHA256 keyed by a random per-process key. Still constant-time and length-safe, and no longer reads as insecure password hashing. Co-Authored-By: Claude Opus 4.8 --- mcp_modules/scanner/src/auth.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/mcp_modules/scanner/src/auth.js b/mcp_modules/scanner/src/auth.js index f6158da..612ac72 100644 --- a/mcp_modules/scanner/src/auth.js +++ b/mcp_modules/scanner/src/auth.js @@ -14,9 +14,14 @@ * side-effecting routes are open. */ -import { createHash, timingSafeEqual } from 'crypto'; +import { randomBytes, createHmac, timingSafeEqual } from 'crypto'; import { logger } from '../../../src/utils/logger.js'; +// Random per-process key used only to derive fixed-length digests for the +// constant-time comparison below (the "double HMAC" technique). It never leaves +// the process and is not a stored credential. +const COMPARE_KEY = randomBytes(32); + let missingTokenWarned = false; /** @@ -34,16 +39,18 @@ function extractToken(c) { } /** - * 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. + * Constant-time comparison of two strings ("double HMAC" technique). + * Each side is run through HMAC-SHA256 keyed by a random per-process key, + * producing fixed-length digests. This lets `timingSafeEqual` run without + * throwing on length mismatch or leaking the expected token length, while the + * random key means the digests reveal nothing about the inputs. * @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(); + const ha = createHmac('sha256', COMPARE_KEY).update(String(a)).digest(); + const hb = createHmac('sha256', COMPARE_KEY).update(String(b)).digest(); return timingSafeEqual(ha, hb); } From e979df7442d1f250be7f88d6a5e8fbf46645388b Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 13 Jul 2026 13:42:54 +0000 Subject: [PATCH 5/5] fix(scanner): compare tokens without hashing to satisfy CodeQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL's js/insufficient-password-hash treats SCANNER_API_TOKEN as a password and rejects any fast hash of it — including HMAC — since it expects a slow KDF. This is bearer-token equality, not password storage, so hashing is the wrong tool. Compare the raw bytes with a length guard + timingSafeEqual: still constant-time, no hashing, no CodeQL alert. Co-Authored-By: Claude Opus 4.8 --- mcp_modules/scanner/src/auth.js | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/mcp_modules/scanner/src/auth.js b/mcp_modules/scanner/src/auth.js index 612ac72..34804d0 100644 --- a/mcp_modules/scanner/src/auth.js +++ b/mcp_modules/scanner/src/auth.js @@ -14,14 +14,9 @@ * side-effecting routes are open. */ -import { randomBytes, createHmac, timingSafeEqual } from 'crypto'; +import { timingSafeEqual } from 'crypto'; import { logger } from '../../../src/utils/logger.js'; -// Random per-process key used only to derive fixed-length digests for the -// constant-time comparison below (the "double HMAC" technique). It never leaves -// the process and is not a stored credential. -const COMPARE_KEY = randomBytes(32); - let missingTokenWarned = false; /** @@ -39,19 +34,22 @@ function extractToken(c) { } /** - * Constant-time comparison of two strings ("double HMAC" technique). - * Each side is run through HMAC-SHA256 keyed by a random per-process key, - * producing fixed-length digests. This lets `timingSafeEqual` run without - * throwing on length mismatch or leaking the expected token length, while the - * random key means the digests reveal nothing about the inputs. + * Constant-time comparison of two opaque tokens. + * A length check short-circuits mismatched lengths (so `timingSafeEqual` never + * throws), then `timingSafeEqual` compares the bytes without leaking where they + * differ. This is a bearer-token equality check, not password storage, so no + * key-derivation/hashing is involved. * @param {string} a * @param {string} b * @returns {boolean} */ function safeEqual(a, b) { - const ha = createHmac('sha256', COMPARE_KEY).update(String(a)).digest(); - const hb = createHmac('sha256', COMPARE_KEY).update(String(b)).digest(); - return timingSafeEqual(ha, hb); + const bufA = Buffer.from(String(a)); + const bufB = Buffer.from(String(b)); + if (bufA.length !== bufB.length) { + return false; + } + return timingSafeEqual(bufA, bufB); } /**