From 5d932f9fbe3b400889e3e4b08edd38d9c36007d1 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Thu, 27 Aug 2026 10:42:58 +0500 Subject: [PATCH 001/108] Fix: safe temp staging and downloadFile error handling (#479) AI assistance: implemented with Cursor Grok 4.6. Co-authored-by: Cursor --- cli/bin/commands/skills.mjs | 86 ++++++++++-------- tests/skills-cli.test.js | 174 +++++++++++++++++++++++++++++++++++- 2 files changed, 221 insertions(+), 39 deletions(-) diff --git a/cli/bin/commands/skills.mjs b/cli/bin/commands/skills.mjs index edbf56790..8ad893d3c 100644 --- a/cli/bin/commands/skills.mjs +++ b/cli/bin/commands/skills.mjs @@ -9,11 +9,10 @@ */ import { execSync } from 'node:child_process'; -import { existsSync, readFileSync, readdirSync, statSync, lstatSync, unlinkSync, mkdirSync, writeFileSync, rmSync, rmdirSync, renameSync, createWriteStream, realpathSync, symlinkSync, readlinkSync, cpSync } from 'node:fs'; +import { existsSync, readFileSync, readdirSync, statSync, lstatSync, unlinkSync, mkdirSync, mkdtempSync, writeFileSync, rmSync, rmdirSync, renameSync, realpathSync, symlinkSync, readlinkSync, cpSync } from 'node:fs'; import { join, resolve, dirname, relative, isAbsolute, sep } from 'node:path'; import { createInterface, emitKeypressEvents } from 'node:readline'; import { fileURLToPath } from 'node:url'; -import { get } from 'node:https'; import { createHash } from 'node:crypto'; import { tmpdir, homedir } from 'node:os'; import { unzipSync } from 'fflate'; @@ -622,13 +621,17 @@ async function downloadAndExtractBundle() { const localBundle = process.env.IMPECCABLE_BUNDLE_PATH; if (localBundle) return copyOrExtractLocalBundle(localBundle); - const tmpZip = join(tmpdir(), `impeccable-update-${Date.now()}.zip`); - const tmpDir = join(tmpdir(), `impeccable-update-${Date.now()}`); - await downloadFile(`${API_BASE}/api/download/bundle/universal`, tmpZip); - mkdirSync(tmpDir, { recursive: true }); - await extractZip(tmpZip, tmpDir); - rmSync(tmpZip, { force: true }); - return tmpDir; + const staging = mkdtempSync(join(tmpdir(), 'impeccable-update-')); + const tmpZip = join(staging, 'bundle.zip'); + try { + await downloadFile(`${API_BASE}/api/download/bundle/universal`, tmpZip); + await extractZip(tmpZip, staging); + rmSync(tmpZip, { force: true }); + return staging; + } catch (e) { + rmSync(staging, { recursive: true, force: true }); + throw e; + } } async function copyOrExtractLocalBundle(sourceValue) { @@ -637,16 +640,18 @@ async function copyOrExtractLocalBundle(sourceValue) { throw new Error(`Local bundle not found: ${source}`); } - const tmpDir = join(tmpdir(), `impeccable-local-bundle-${process.pid}-${Date.now()}`); - mkdirSync(tmpDir, { recursive: true }); - - if (statSync(source).isDirectory()) { - cpSync(source, tmpDir, { recursive: true }); - return tmpDir; + const staging = mkdtempSync(join(tmpdir(), 'impeccable-local-bundle-')); + try { + if (statSync(source).isDirectory()) { + cpSync(source, staging, { recursive: true }); + } else { + await extractZip(source, staging); + } + return staging; + } catch (e) { + rmSync(staging, { recursive: true, force: true }); + throw e; } - - await extractZip(source, tmpDir); - return tmpDir; } /** @@ -2163,26 +2168,29 @@ function getModifiedSkillFiles(root, providerDirs) { return modified; } -function downloadFile(url, dest) { - return new Promise((resolve, reject) => { - const file = createWriteStream(dest); - get(url, (res) => { - if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) { - // Follow redirect - get(res.headers.location, (res2) => { - res2.pipe(file); - file.on('finish', () => { file.close(); resolve(); }); - }).on('error', reject); - return; - } - if (res.statusCode !== 200) { - reject(new Error(`HTTP ${res.statusCode}`)); - return; - } - res.pipe(file); - file.on('finish', () => { file.close(); resolve(); }); - }).on('error', reject); - }); +async function downloadFile(url, dest, { fetchImpl = globalThis.fetch } = {}) { + let current = url; + let hopsLeft = 5; + while (true) { + const parsed = new URL(current); + if (parsed.protocol !== 'https:') { + throw new Error('Refusing non-HTTPS URL'); + } + const res = await fetchImpl(current, { redirect: 'manual' }); + if (res.status >= 300 && res.status < 400) { + const location = res.headers.get('location'); + if (!location) throw new Error(`HTTP ${res.status}`); + if (hopsLeft <= 0) throw new Error('Too many redirects'); + hopsLeft -= 1; + current = new URL(location, current).href; + continue; + } + if (res.status !== 200) { + throw new Error(`HTTP ${res.status}`); + } + writeFileSync(dest, Buffer.from(await res.arrayBuffer()), { flag: 'wx' }); + return; + } } async function update(flags = []) { @@ -2332,6 +2340,8 @@ export { copyProviderHooks, copyProviderSkills, decideHookInstall, + downloadAndExtractBundle, + downloadFile, expectedHookDests, extractZip, formatInstallDetectionLines, diff --git a/tests/skills-cli.test.js b/tests/skills-cli.test.js index 1156d3dc2..5aacf8e2e 100644 --- a/tests/skills-cli.test.js +++ b/tests/skills-cli.test.js @@ -11,7 +11,7 @@ */ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { execSync, execFileSync } from 'child_process'; -import { mkdtempSync, existsSync, readdirSync, readFileSync, mkdirSync, writeFileSync, rmSync, lstatSync, realpathSync, readlinkSync, symlinkSync } from 'fs'; +import { mkdtempSync, existsSync, readdirSync, readFileSync, mkdirSync, writeFileSync, rmSync, lstatSync, realpathSync, readlinkSync, symlinkSync, statSync } from 'fs'; import { join } from 'path'; import { tmpdir } from 'os'; import { @@ -19,6 +19,8 @@ import { copyProviderHooks, copyProviderSkills, decideHookInstall, + downloadAndExtractBundle, + downloadFile, expectedHookDests, formatInstallDetectionLines, mergeHookManifests, @@ -2214,3 +2216,173 @@ describe('hermesGlobalHome resolver (PR #521)', () => { rmSync(home, { recursive: true, force: true }); }, 20000); }); + +describe('downloadAndExtractBundle: safe staging dir (#479)', () => { + test('local bundle uses mkdtemp under tmpdir with 0700 perms', async () => { + const tmp = mkdtempSync(join(tmpdir(), 'imp-test-staging-')); + const bundleRoot = createFakeUniversalBundle(tmp, ['.claude']); + const prev = process.env.IMPECCABLE_BUNDLE_PATH; + let stagingDir; + try { + process.env.IMPECCABLE_BUNDLE_PATH = bundleRoot; + stagingDir = await downloadAndExtractBundle(); + + expect(stagingDir.startsWith(tmpdir())).toBe(true); + const basename = stagingDir.split(/[/\\]/).pop(); + expect(basename.startsWith('impeccable-local-bundle-')).toBe(true); + expect(basename).not.toMatch(/^impeccable-local-bundle-\d+-\d+$/); + + if (process.platform !== 'win32') { + expect(statSync(stagingDir).mode & 0o777).toBe(0o700); + } + + expect(existsSync(join(stagingDir, '.claude', 'skills', 'impeccable', 'SKILL.md'))).toBe(true); + } finally { + if (prev === undefined) delete process.env.IMPECCABLE_BUNDLE_PATH; + else process.env.IMPECCABLE_BUNDLE_PATH = prev; + if (stagingDir) rmSync(stagingDir, { recursive: true, force: true }); + rmSync(tmp, { recursive: true, force: true }); + } + }); +}); + +describe('downloadFile (#479)', () => { + test('200 writes body to dest with wx flag', async () => { + const dir = mkdtempSync(join(tmpdir(), 'imp-dl-')); + const dest = join(dir, 'out.bin'); + try { + const fetchImpl = async () => new Response('hello', { status: 200 }); + await downloadFile('https://example.com/file', dest, { fetchImpl }); + expect(readFileSync(dest, 'utf8')).toBe('hello'); + + await expect(downloadFile('https://example.com/file', dest, { fetchImpl })) + .rejects.toThrow(); + expect(readFileSync(dest, 'utf8')).toBe('hello'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test('404 throws and dest does not exist', async () => { + const dir = mkdtempSync(join(tmpdir(), 'imp-dl-')); + const dest = join(dir, 'out.bin'); + try { + const fetchImpl = async () => new Response('not found', { status: 404 }); + await expect(downloadFile('https://example.com/missing', dest, { fetchImpl })) + .rejects.toThrow(/HTTP 404/); + expect(existsSync(dest)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test('redirect 302 to 200 follows location and writes second body', async () => { + const dir = mkdtempSync(join(tmpdir(), 'imp-dl-')); + const dest = join(dir, 'out.bin'); + try { + let callCount = 0; + const fetchImpl = async (url) => { + callCount++; + if (url === 'https://example.com/start') { + return new Response('', { status: 302, headers: { location: 'https://example.com/final' } }); + } + return new Response('final body', { status: 200 }); + }; + await downloadFile('https://example.com/start', dest, { fetchImpl }); + expect(callCount).toBe(2); + expect(readFileSync(dest, 'utf8')).toBe('final body'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test('redirect 302 to 404 throws and dest does not exist', async () => { + const dir = mkdtempSync(join(tmpdir(), 'imp-dl-')); + const dest = join(dir, 'out.bin'); + try { + const fetchImpl = async (url) => { + if (url.includes('/start')) { + return new Response('', { status: 302, headers: { location: 'https://example.com/bad' } }); + } + return new Response('error', { status: 404 }); + }; + await expect(downloadFile('https://example.com/start', dest, { fetchImpl })) + .rejects.toThrow(/HTTP 404/); + expect(existsSync(dest)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test('redirect to http throws non-HTTPS and dest does not exist', async () => { + const dir = mkdtempSync(join(tmpdir(), 'imp-dl-')); + const dest = join(dir, 'out.bin'); + try { + const fetchImpl = async () => new Response('', { status: 302, headers: { location: 'http://example.com/insecure' } }); + await expect(downloadFile('https://example.com/start', dest, { fetchImpl })) + .rejects.toThrow(/non-HTTPS/i); + expect(existsSync(dest)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test('relative redirect location resolved against current URL', async () => { + const dir = mkdtempSync(join(tmpdir(), 'imp-dl-')); + const dest = join(dir, 'out.bin'); + try { + const fetchImpl = async (url) => { + if (url === 'https://example.com/api/start') { + return new Response('', { status: 302, headers: { location: '/final' } }); + } + expect(url).toBe('https://example.com/final'); + return new Response('ok', { status: 200 }); + }; + await downloadFile('https://example.com/api/start', dest, { fetchImpl }); + expect(readFileSync(dest, 'utf8')).toBe('ok'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test('more than maxRedirects hops throws and dest does not exist', async () => { + const dir = mkdtempSync(join(tmpdir(), 'imp-dl-')); + const dest = join(dir, 'out.bin'); + try { + const fetchImpl = async () => new Response('', { status: 302, headers: { location: 'https://example.com/loop' } }); + await expect(downloadFile('https://example.com/loop', dest, { fetchImpl })) + .rejects.toThrow(/Too many redirects/); + expect(existsSync(dest)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test('fetchImpl rejection leaves dest absent', async () => { + const dir = mkdtempSync(join(tmpdir(), 'imp-dl-')); + const dest = join(dir, 'out.bin'); + try { + const fetchImpl = async () => { throw new Error('network down'); }; + await expect(downloadFile('https://example.com/file', dest, { fetchImpl })) + .rejects.toThrow(/network down/); + expect(existsSync(dest)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test('http initial URL throws without calling fetch', async () => { + const dir = mkdtempSync(join(tmpdir(), 'imp-dl-')); + const dest = join(dir, 'out.bin'); + try { + let called = false; + const fetchImpl = async () => { called = true; return new Response('x', { status: 200 }); }; + await expect(downloadFile('http://example.com/file', dest, { fetchImpl })) + .rejects.toThrow(/non-HTTPS/i); + expect(called).toBe(false); + expect(existsSync(dest)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); From af2e8b3ac34eb01cba53d860eaeda5912f29419b Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Thu, 27 Aug 2026 11:00:47 +0500 Subject: [PATCH 002/108] Fix: stream bundle downloads to disk instead of buffering AI assistance: implemented with Cursor Grok 4.6. Co-authored-by: Cursor --- cli/bin/commands/skills.mjs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/cli/bin/commands/skills.mjs b/cli/bin/commands/skills.mjs index 8ad893d3c..2f851b38b 100644 --- a/cli/bin/commands/skills.mjs +++ b/cli/bin/commands/skills.mjs @@ -9,9 +9,11 @@ */ import { execSync } from 'node:child_process'; -import { existsSync, readFileSync, readdirSync, statSync, lstatSync, unlinkSync, mkdirSync, mkdtempSync, writeFileSync, rmSync, rmdirSync, renameSync, realpathSync, symlinkSync, readlinkSync, cpSync } from 'node:fs'; +import { existsSync, readFileSync, readdirSync, statSync, lstatSync, unlinkSync, mkdirSync, mkdtempSync, writeFileSync, rmSync, rmdirSync, renameSync, createWriteStream, realpathSync, symlinkSync, readlinkSync, cpSync } from 'node:fs'; import { join, resolve, dirname, relative, isAbsolute, sep } from 'node:path'; import { createInterface, emitKeypressEvents } from 'node:readline'; +import { Readable } from 'node:stream'; +import { pipeline } from 'node:stream/promises'; import { fileURLToPath } from 'node:url'; import { createHash } from 'node:crypto'; import { tmpdir, homedir } from 'node:os'; @@ -2188,7 +2190,13 @@ async function downloadFile(url, dest, { fetchImpl = globalThis.fetch } = {}) { if (res.status !== 200) { throw new Error(`HTTP ${res.status}`); } - writeFileSync(dest, Buffer.from(await res.arrayBuffer()), { flag: 'wx' }); + if (!res.body) throw new Error('Empty response body'); + try { + await pipeline(Readable.fromWeb(res.body), createWriteStream(dest, { flags: 'wx' })); + } catch (e) { + if (e.code !== 'EEXIST') rmSync(dest, { force: true }); + throw e; + } return; } } From be87f5eb8683c3ee0b23685e1a7d06bc7429e641 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Thu, 27 Aug 2026 10:38:33 +0500 Subject: [PATCH 003/108] Fix: refuse inert exact ignore-value entries (#662) ignore-value stored exact values for rules that cannot extract one, so the entries never matched. Refuse them and point at "*" --file. AI assistance: implemented with Cursor Grok 4.6. Co-authored-by: Cursor --- cli/bin/commands/ignores.mjs | 4 ++++ skill/scripts/hook-admin.mjs | 5 +++++ tests/cli-ignores.test.js | 7 +++++++ tests/hook.test.mjs | 22 ++++++++++++++++++++++ 4 files changed, 38 insertions(+) diff --git a/cli/bin/commands/ignores.mjs b/cli/bin/commands/ignores.mjs index dc1d6a376..5e0585dfc 100644 --- a/cli/bin/commands/ignores.mjs +++ b/cli/bin/commands/ignores.mjs @@ -7,6 +7,7 @@ import { readDetectionConfig, readRawDetectionConfig, writeDetectionConfig, + extractFindingIgnoreValue, } from '../../lib/impeccable-config.mjs'; const ACTION_ALIASES = new Map([ @@ -235,6 +236,9 @@ function addFile(cwd, args) { function addValue(cwd, args) { const { local, rest } = parseScope(args); const parsed = parseValueArgs(rest); + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use impeccable ignores add-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } const config = readScopeConfig(cwd, local); const key = ignoreValueKey(parsed); const existing = config.ignoreValues.find((entry) => ignoreValueKey(entry) === key); diff --git a/skill/scripts/hook-admin.mjs b/skill/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/skill/scripts/hook-admin.mjs +++ b/skill/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/tests/cli-ignores.test.js b/tests/cli-ignores.test.js index 95f676ed4..8049106fb 100644 --- a/tests/cli-ignores.test.js +++ b/tests/cli-ignores.test.js @@ -115,6 +115,13 @@ describe('impeccable ignores CLI', () => { expect(existsSync(join(root, '.impeccable', 'config.json'))).toBe(false); }); + test('rejects exact values for rules that cannot extract one', () => { + const result = run(['add-value', 'side-tab', 'Inter']); + expect(result.status).not.toBe(0); + expect(result.stderr).toMatch(/side-tab has no extractable ignore value.*add-value side-tab "\*" --file /); + expect(existsSync(join(root, '.impeccable', 'config.json'))).toBe(false); + }); + test('removes an existing broad wildcard value ignore', () => { mkdirSync(join(root, '.impeccable'), { recursive: true }); writeFileSync(join(root, '.impeccable', 'config.json'), JSON.stringify({ diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index 15f549e43..e11d3fb62 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -819,6 +819,28 @@ describe('hook-admin.mjs', () => { assert.equal(fs.existsSync(getConfigPath(cwd)), false, 'a refused ignore must not write config'); }); + it('ignore-value refuses exact values for rules that cannot extract one', () => { + assert.throws( + () => runAdmin(['ignore-value', 'cramped-padding', 'padding: 4px 8px']), + /cramped-padding has no extractable ignore value.*ignore-value cramped-padding "\*" --file /, + ); + assert.throws( + () => runAdmin(['ignore-value', 'side-tab', 'Inter', '--file', 'a.css']), + /side-tab has no extractable ignore value.*ignore-value side-tab "\*" --file /, + ); + assert.equal(fs.existsSync(getConfigPath(cwd)), false, 'a refused ignore must not write config'); + + const out = runAdmin(['ignore-value', 'overused-font', 'Inter']); + assert.match(out, /Added overused-font=inter/); + + runAdmin(['ignore-value', 'cramped-padding', '*', '--file', 'index.html']); + const shared = JSON.parse(fs.readFileSync(getConfigPath(cwd), 'utf-8')).detector; + assert.equal(shared.ignoreValues.filter((e) => e.rule === 'cramped-padding').length, 1); + const entry = shared.ignoreValues.find((e) => e.rule === 'cramped-padding'); + assert.equal(entry.value, '*'); + assert.deepEqual(entry.files, ['index.html']); + }); + it('ignore-value --file requires a glob', () => { assert.throws( () => runAdmin(['ignore-value', 'side-tab', '*', '--file']), From 1df992ade0193a1b293e53bdbc7325a95e82cbd9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 00:44:04 +0000 Subject: [PATCH 004/108] Sync generated provider output --- .agents/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .claude/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .cursor/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .gemini/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .github/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .grok/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .hermes/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .kiro/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .opencode/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .pi/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .qoder/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .rovodev/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .trae-cn/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .trae/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ .vibe/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ plugin/skills/impeccable/scripts/hook-admin.mjs | 5 +++++ 16 files changed, 80 insertions(+) diff --git a/.agents/skills/impeccable/scripts/hook-admin.mjs b/.agents/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.agents/skills/impeccable/scripts/hook-admin.mjs +++ b/.agents/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.claude/skills/impeccable/scripts/hook-admin.mjs b/.claude/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.claude/skills/impeccable/scripts/hook-admin.mjs +++ b/.claude/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.cursor/skills/impeccable/scripts/hook-admin.mjs b/.cursor/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.cursor/skills/impeccable/scripts/hook-admin.mjs +++ b/.cursor/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.gemini/skills/impeccable/scripts/hook-admin.mjs b/.gemini/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.gemini/skills/impeccable/scripts/hook-admin.mjs +++ b/.gemini/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.github/skills/impeccable/scripts/hook-admin.mjs b/.github/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.github/skills/impeccable/scripts/hook-admin.mjs +++ b/.github/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.grok/skills/impeccable/scripts/hook-admin.mjs b/.grok/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.grok/skills/impeccable/scripts/hook-admin.mjs +++ b/.grok/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.hermes/skills/impeccable/scripts/hook-admin.mjs b/.hermes/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.hermes/skills/impeccable/scripts/hook-admin.mjs +++ b/.hermes/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.kiro/skills/impeccable/scripts/hook-admin.mjs b/.kiro/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.kiro/skills/impeccable/scripts/hook-admin.mjs +++ b/.kiro/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.opencode/skills/impeccable/scripts/hook-admin.mjs b/.opencode/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.opencode/skills/impeccable/scripts/hook-admin.mjs +++ b/.opencode/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.pi/skills/impeccable/scripts/hook-admin.mjs b/.pi/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.pi/skills/impeccable/scripts/hook-admin.mjs +++ b/.pi/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.qoder/skills/impeccable/scripts/hook-admin.mjs b/.qoder/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.qoder/skills/impeccable/scripts/hook-admin.mjs +++ b/.qoder/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.rovodev/skills/impeccable/scripts/hook-admin.mjs b/.rovodev/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.rovodev/skills/impeccable/scripts/hook-admin.mjs +++ b/.rovodev/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.trae-cn/skills/impeccable/scripts/hook-admin.mjs b/.trae-cn/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.trae-cn/skills/impeccable/scripts/hook-admin.mjs +++ b/.trae-cn/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.trae/skills/impeccable/scripts/hook-admin.mjs b/.trae/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.trae/skills/impeccable/scripts/hook-admin.mjs +++ b/.trae/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/.vibe/skills/impeccable/scripts/hook-admin.mjs b/.vibe/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/.vibe/skills/impeccable/scripts/hook-admin.mjs +++ b/.vibe/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than diff --git a/plugin/skills/impeccable/scripts/hook-admin.mjs b/plugin/skills/impeccable/scripts/hook-admin.mjs index 0d8cbaf94..28677c483 100644 --- a/plugin/skills/impeccable/scripts/hook-admin.mjs +++ b/plugin/skills/impeccable/scripts/hook-admin.mjs @@ -35,6 +35,7 @@ import { ensureHookGitExcludes, normalizeIgnoreValue, normalizeIgnoreValueEntries, + extractFindingIgnoreValue, } from './hook-lib.mjs'; const ACTIONS = new Set(['status', 'on', 'off', 'ignore-rule', 'ignore-file', 'ignore-value', 'reset']); @@ -713,6 +714,10 @@ function addIgnoreValue(cwd, args) { throw new Error(`Wildcard value ignores must be scoped with --file , e.g. ${IMPECCABLE_COMMAND} hooks ignore-value design-system-font-size "*" --file "src/widget.js". To suppress the rule project-wide use ${projectWide}.`); } + if (parsed.value !== '*' && !extractFindingIgnoreValue({ antipattern: parsed.rule, ignoreValue: parsed.value })) { + throw new Error(`${parsed.rule} has no extractable ignore value. Use ${IMPECCABLE_COMMAND} hooks ignore-value ${parsed.rule} "*" --file to suppress it in matching files.`); + } + const local = parsed.local; const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local })); // Key on the file scope too: the same rule/value legitimately appears more than From d5873ff8eb2d354cbf32dc747f7014f71468ff56 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Wed, 26 Aug 2026 10:26:41 +0500 Subject: [PATCH 005/108] 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); + } + }); +}); From d690349db1f8740b1c2c5c1fab26944cb0de581f Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Wed, 26 Aug 2026 10:36:40 +0500 Subject: [PATCH 006/108] Fix: keep URL basic-auth credentials on the scan origin (#657) page.authenticate is page-wide, so a cross-origin redirect that then 401s would receive the original credentials. Attach Authorization only to requests for the scan origin. Written with AI assistance (Cursor); reviewed by maintainer. Co-authored-by: Cursor --- cli/engine/engines/browser/detect-url.mjs | 34 ++++++- tests/detect-url-launch.test.mjs | 115 +++++++++++++++++++++- 2 files changed, 141 insertions(+), 8 deletions(-) diff --git a/cli/engine/engines/browser/detect-url.mjs b/cli/engine/engines/browser/detect-url.mjs index c2355dea9..f3ff43c84 100644 --- a/cli/engine/engines/browser/detect-url.mjs +++ b/cli/engine/engines/browser/detect-url.mjs @@ -192,6 +192,36 @@ function splitScanUrl(url) { return { href: parsed.href, credentials }; } +function basicAuthHeader(credentials) { + return `Basic ${Buffer.from(`${credentials.username}:${credentials.password}`).toString('base64')}`; +} + +// page.authenticate is page-wide: a cross-origin redirect that then 401s +// would receive these credentials. Attach Authorization only to the scan origin. +async function applyOriginScopedAuth(page, href, credentials) { + if (!credentials) return; + let origin = ''; + try { + origin = new URL(href).origin; + } catch { + return; + } + if (!origin) return; + const header = basicAuthHeader(credentials); + await page.setRequestInterception(true); + page.on('request', (request) => { + let headers; + try { + if (new URL(request.url()).origin === origin) { + headers = { ...request.headers(), authorization: header }; + } + } catch { + // invalid request URL: continue without auth + } + void request.continue(headers ? { headers } : undefined).catch(() => {}); + }); +} + async function detectUrl(rawUrl, options = {}) { const { href: url, credentials } = splitScanUrl(rawUrl); const profile = options?.profile; @@ -269,9 +299,7 @@ async function detectUrl(rawUrl, options = {}) { ruleId: 'set-viewport', target: url, }, () => page.setViewport(viewport)); - if (credentials) { - await page.authenticate(credentials); - } + await applyOriginScopedAuth(page, url, credentials); await profileStepAsync(profile, { engine: 'browser', phase: 'load', diff --git a/tests/detect-url-launch.test.mjs b/tests/detect-url-launch.test.mjs index 31af73bdb..dfa2680b1 100644 --- a/tests/detect-url-launch.test.mjs +++ b/tests/detect-url-launch.test.mjs @@ -1,4 +1,5 @@ import { describe, test, expect, afterEach } from 'bun:test'; +import http from 'node:http'; import { launchBrowser, detectUrl, splitScanUrl } from '../cli/engine/engines/browser/detect-url.mjs'; // launchBrowser prefers the system-installed Chrome on Windows to dodge the @@ -131,10 +132,13 @@ describe('splitScanUrl', () => { }); function makeFakeBrowser() { - const calls = { authenticate: [], goto: [] }; + const calls = { intercept: false, requestHandler: null, authenticate: [], goto: [] }; const page = { - on() {}, + on(event, handler) { + if (event === 'request') calls.requestHandler = handler; + }, async setViewport() {}, + async setRequestInterception() { calls.intercept = true; }, async authenticate(creds) { calls.authenticate.push(creds); }, async goto(url, opts) { calls.goto.push({ url, opts }); }, async evaluate(fn) { @@ -153,25 +157,56 @@ function makeFakeBrowser() { }; } +function fakeRequest(url, calls) { + return { + url: () => url, + headers: () => ({ accept: 'text/html' }), + continue(overrides) { + calls.continues.push({ url, overrides }); + return Promise.resolve(); + }, + }; +} + +function listen(server) { + return new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(0, '127.0.0.1', () => { + server.off('error', reject); + resolve(`http://127.0.0.1:${server.address().port}/`); + }); + }); +} + describe('detectUrl credential redaction', () => { - test('authenticates with stripped URL and redacts findings', async () => { + test('scopes Authorization to the scan origin and redacts findings', async () => { const { calls, browser } = makeFakeBrowser(); + calls.continues = []; 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.authenticate).toEqual([]); + expect(calls.intercept).toBe(true); + expect(typeof calls.requestHandler).toBe('function'); expect(calls.goto).toHaveLength(1); expect(calls.goto[0].url).toBe('https://example.com/path'); + + const expected = `Basic ${Buffer.from('user:p@ss').toString('base64')}`; + await calls.requestHandler(fakeRequest('https://example.com/path', calls)); + await calls.requestHandler(fakeRequest('https://evil.example/steal', calls)); + expect(calls.continues[0].overrides.headers.authorization).toBe(expected); + expect(calls.continues[1].overrides).toBeUndefined(); + 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 () => { + test('does not intercept when URL has no userinfo', async () => { const { calls, browser } = makeFakeBrowser(); const url = 'https://example.com/path'; const findings = await detectUrl(url, { @@ -181,9 +216,79 @@ describe('detectUrl credential redaction', () => { }); expect(calls.authenticate).toEqual([]); + expect(calls.intercept).toBe(false); + expect(calls.requestHandler).toBe(null); expect(findings.length).toBeGreaterThan(0); for (const f of findings) { expect(f.file).toBe(url); } }); }); + +describe('detectUrl origin-scoped basic auth', () => { + test('does not send URL credentials to a cross-origin redirect that challenges', async () => { + const user = 'qa-scanner'; + const pass = 'Hunter2-657-SHOULD-NOT-LEAK'; + const expected = `Basic ${Buffer.from(`${user}:${pass}`).toString('base64')}`; + const seenOnB = []; + + const serverB = http.createServer((req, res) => { + seenOnB.push(req.headers.authorization || ''); + res.writeHead(401, { 'WWW-Authenticate': 'Basic realm="b"' }); + res.end('b'); + }); + const urlB = await listen(serverB); + const serverA = http.createServer((req, res) => { + res.writeHead(302, { Location: urlB }); + res.end(); + }); + const urlA = await listen(serverA); + + try { + try { + await detectUrl(urlA.replace('http://', `http://${user}:${pass}@`), { + visualContrast: false, + contentHidden: false, + waitUntil: 'domcontentloaded', + }); + } catch { + // B's 401 may fail navigation once credentials are withheld. + } + expect(seenOnB.includes(expected)).toBe(false); + } finally { + await Promise.all([ + new Promise((resolve) => serverA.close(resolve)), + new Promise((resolve) => serverB.close(resolve)), + ]); + } + }, { timeout: 30000 }); + + test('still authenticates the original scan origin', async () => { + const user = 'qa-scanner'; + const pass = 'Hunter2-657-SHOULD-NOT-LEAK'; + const expected = `Basic ${Buffer.from(`${user}:${pass}`).toString('base64')}`; + const seen = []; + const server = http.createServer((req, res) => { + seen.push(req.headers.authorization || ''); + if (req.headers.authorization !== expected) { + res.writeHead(401, { 'WWW-Authenticate': 'Basic realm="a"' }); + res.end('no'); + return; + } + res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }); + res.end('

ok

'); + }); + const origin = await listen(server); + + try { + await detectUrl(origin.replace('http://', `http://${user}:${pass}@`), { + visualContrast: false, + contentHidden: false, + waitUntil: 'domcontentloaded', + }); + expect(seen.includes(expected)).toBe(true); + } finally { + await new Promise((resolve) => server.close(resolve)); + } + }, { timeout: 30000 }); +}); From eaaecbd1fe9c69086fcea530f964c33e076a1ff5 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Fri, 14 Aug 2026 23:37:24 +0500 Subject: [PATCH 007/108] Fix: require session key and origin/host checks on serve-question POSTs (#555) Unauthenticated POST /answer copied steer into the agent ANSWER line. The handler now requires the detached session key and rejects foreign Origin and Host. Written with AI assistance under maintainer direction. Co-authored-by: Cursor --- skill/scripts/serve-question.mjs | 49 +++++++++++++--- tests/serve-question.test.mjs | 95 +++++++++++++++++++++++++++++++- 2 files changed, 134 insertions(+), 10 deletions(-) diff --git a/skill/scripts/serve-question.mjs b/skill/scripts/serve-question.mjs index 7c4ff0812..907224c30 100644 --- a/skill/scripts/serve-question.mjs +++ b/skill/scripts/serve-question.mjs @@ -1019,7 +1019,9 @@ ${buildPath?.toggle ? `