From d5873ff8eb2d354cbf32dc747f7014f71468ff56 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Wed, 26 Aug 2026 10:26:41 +0500 Subject: [PATCH] Fix: redact URL userinfo from detect findings (#657) Strip basic-auth credentials from scan-target URLs before goto and finding output, and pass them to page.authenticate instead. Written with AI assistance (Cursor); reviewed by maintainer. Co-authored-by: Cursor --- cli/engine/engines/browser/detect-url.mjs | 38 +++++++- tests/detect-url-launch.test.mjs | 110 +++++++++++++++++++++- 2 files changed, 145 insertions(+), 3 deletions(-) diff --git a/cli/engine/engines/browser/detect-url.mjs b/cli/engine/engines/browser/detect-url.mjs index 5e3d5446b..c2355dea9 100644 --- a/cli/engine/engines/browser/detect-url.mjs +++ b/cli/engine/engines/browser/detect-url.mjs @@ -162,7 +162,38 @@ async function runVisualContrastFallback(page, serializedGroups, options, profil // Puppeteer detection (for URLs) // --------------------------------------------------------------------------- -async function detectUrl(url, options = {}) { +function decodeUrlComponent(value) { + try { + return decodeURIComponent(value); + } catch { + return value; + } +} + +function splitScanUrl(url) { + let parsed; + try { + parsed = new URL(url); + } catch { + return { href: url, credentials: null }; + } + if (!parsed.username && !parsed.password) { + return { href: url, credentials: null }; + } + const credentials = + parsed.protocol === 'http:' || parsed.protocol === 'https:' + ? { + username: decodeUrlComponent(parsed.username), + password: decodeUrlComponent(parsed.password), + } + : null; + parsed.username = ''; + parsed.password = ''; + return { href: parsed.href, credentials }; +} + +async function detectUrl(rawUrl, options = {}) { + const { href: url, credentials } = splitScanUrl(rawUrl); const profile = options?.profile; const waitUntil = options?.waitUntil || 'networkidle0'; const settleMs = Number.isFinite(options?.settleMs) ? options.settleMs : 0; @@ -238,6 +269,9 @@ async function detectUrl(url, options = {}) { ruleId: 'set-viewport', target: url, }, () => page.setViewport(viewport)); + if (credentials) { + await page.authenticate(credentials); + } await profileStepAsync(profile, { engine: 'browser', phase: 'load', @@ -369,4 +403,4 @@ async function createBrowserDetector(options = {}) { }; } -export { runVisualContrastFallback, detectUrl, createBrowserDetector, launchBrowser }; +export { runVisualContrastFallback, detectUrl, createBrowserDetector, launchBrowser, splitScanUrl }; diff --git a/tests/detect-url-launch.test.mjs b/tests/detect-url-launch.test.mjs index 53a147722..31af73bdb 100644 --- a/tests/detect-url-launch.test.mjs +++ b/tests/detect-url-launch.test.mjs @@ -1,5 +1,5 @@ import { describe, test, expect, afterEach } from 'bun:test'; -import { launchBrowser } from '../cli/engine/engines/browser/detect-url.mjs'; +import { launchBrowser, detectUrl, splitScanUrl } from '../cli/engine/engines/browser/detect-url.mjs'; // launchBrowser prefers the system-installed Chrome on Windows to dodge the // bundled-Chrome GPU crash-loop (issue #372), and keeps the pinned bundled @@ -79,3 +79,111 @@ describe('launchBrowser', () => { expect(p.calls.every(c => c.channel === undefined)).toBe(true); }); }); + +describe('splitScanUrl', () => { + test('strips http(s) userinfo and returns credentials', () => { + expect(splitScanUrl('https://user:pass@example.com')).toEqual({ + href: 'https://example.com/', + credentials: { username: 'user', password: 'pass' }, + }); + expect(splitScanUrl('https://user:p%40ss@example.com/path?q=1')).toEqual({ + href: 'https://example.com/path?q=1', + credentials: { username: 'user', password: 'p@ss' }, + }); + expect(splitScanUrl('https://user@example.com')).toEqual({ + href: 'https://example.com/', + credentials: { username: 'user', password: '' }, + }); + expect(splitScanUrl('http://:secret@host.com/')).toEqual({ + href: 'http://host.com/', + credentials: { username: '', password: 'secret' }, + }); + }); + + test('preserves original string when no userinfo', () => { + expect(splitScanUrl('https://example.com')).toEqual({ + href: 'https://example.com', + credentials: null, + }); + expect(splitScanUrl('https://example.com/path?email=a@b.com')).toEqual({ + href: 'https://example.com/path?email=a@b.com', + credentials: null, + }); + }); + + test('handles IPv6 and non-http(s) URLs', () => { + expect(splitScanUrl('https://user:pass@[::1]:8080/x')).toEqual({ + href: 'https://[::1]:8080/x', + credentials: { username: 'user', password: 'pass' }, + }); + expect(splitScanUrl('file:///tmp/a.html')).toEqual({ + href: 'file:///tmp/a.html', + credentials: null, + }); + }); + + test('returns original string for invalid URLs', () => { + expect(splitScanUrl('not a url')).toEqual({ + href: 'not a url', + credentials: null, + }); + }); +}); + +function makeFakeBrowser() { + const calls = { authenticate: [], goto: [] }; + const page = { + on() {}, + async setViewport() {}, + async authenticate(creds) { calls.authenticate.push(creds); }, + async goto(url, opts) { calls.goto.push({ url, opts }); }, + async evaluate(fn) { + if (typeof fn === 'function' && fn.toString().includes('impeccableDetect')) { + return [{ findings: [{ type: 'low-contrast', detail: 'x', ignoreValue: '', severity: '' }] }]; + } + return []; + }, + async close() {}, + }; + return { + calls, + browser: { + async newPage() { return page; }, + }, + }; +} + +describe('detectUrl credential redaction', () => { + test('authenticates with stripped URL and redacts findings', async () => { + const { calls, browser } = makeFakeBrowser(); + const findings = await detectUrl('https://user:p%40ss@example.com/path', { + browser, + visualContrast: false, + contentHidden: false, + }); + + expect(calls.authenticate).toEqual([{ username: 'user', password: 'p@ss' }]); + expect(calls.goto).toHaveLength(1); + expect(calls.goto[0].url).toBe('https://example.com/path'); + expect(findings.length).toBeGreaterThan(0); + for (const f of findings) { + expect(f.file).toBe('https://example.com/path'); + } + }); + + test('does not authenticate when URL has no userinfo', async () => { + const { calls, browser } = makeFakeBrowser(); + const url = 'https://example.com/path'; + const findings = await detectUrl(url, { + browser, + visualContrast: false, + contentHidden: false, + }); + + expect(calls.authenticate).toEqual([]); + expect(findings.length).toBeGreaterThan(0); + for (const f of findings) { + expect(f.file).toBe(url); + } + }); +});