mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-11 21:57:14 +03:00
Fix: a waiver scoped to one served root must not hide findings on another
Greptile's review found a real bug in the new resolver. When the live config lists pages under more than one folder (src/**/*.html and public/**/*.html), the overlay treated a URL like /foo.html as src/foo.html and public/foo.html at the same time. A waiver written only for src/foo.html could then hide a finding on the page actually served from public/foo.html. That fails in the worst direction: a real finding disappears and nothing says so. The overlay can never look up the right file. The live server does not serve the pages; the project's own dev or static server does, and its URL-to-file mapping is invisible from here. So the fix stops guessing: a file-scoped waiver now applies only when it matches the URL path itself, which is true whichever folder serves the page, or when it matches under every configured folder, so no possible reading disagrees. Anything ambiguous shows the finding, which is also what the CLI reports for the file really being served. With a single configured root, the common case, nothing changes: the new rule reduces to the old behaviour exactly. Multi-root projects keep three ways to write a waiver that still applies: name the file under each folder, use the bare path, or use **/. Two new unit tests pin the ambiguous case and the safe spellings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Abdul Wahab
co-authored by
Claude Fable 5
parent
5330fa358e
commit
ce1c9f8dad
@@ -89,6 +89,13 @@
|
||||
// Each prefixed path also contributes its slash suffixes, mirroring
|
||||
// findingMatchesScopedIgnoreFile in cli/lib/impeccable-config.mjs (which
|
||||
// matches globs against every path suffix of the finding's file).
|
||||
//
|
||||
// With several roots configured, one URL has several possible identities
|
||||
// and the browser cannot tell which root actually serves it. The rooted
|
||||
// groups are therefore kept separate: matchesScope treats a root-prefixed
|
||||
// match as valid only when it holds under every root, so a waiver scoped
|
||||
// to src/foo.html never hides a finding on a page served from
|
||||
// public/foo.html. With a single root this reduces to plain matching.
|
||||
function pageCandidates(pathname, roots) {
|
||||
let pagePath = String(pathname || '');
|
||||
try {
|
||||
@@ -101,33 +108,39 @@
|
||||
// name files. Without this, /news/ never matches prototype/news/index.html.
|
||||
if (pagePath === '' || pagePath.endsWith('/')) pagePath += 'index.html';
|
||||
|
||||
const prefixes = [''];
|
||||
const suffixesOf = (fullPath) => {
|
||||
const parts = fullPath.split('/').filter(Boolean);
|
||||
const out = [];
|
||||
for (let i = 0; i < parts.length; i++) {
|
||||
out.push(parts.slice(i).join('/'));
|
||||
}
|
||||
return out;
|
||||
};
|
||||
|
||||
const rooted = [];
|
||||
for (const entry of Array.isArray(roots) ? roots : []) {
|
||||
if (typeof entry !== 'string') continue;
|
||||
prefixes.push(entry === '' || entry.endsWith('/') ? entry : entry + '/');
|
||||
const prefix = entry === '' || entry.endsWith('/') ? entry : entry + '/';
|
||||
rooted.push(suffixesOf(prefix + pagePath));
|
||||
}
|
||||
|
||||
const candidates = new Set();
|
||||
for (const prefix of prefixes) {
|
||||
const full = prefix + pagePath;
|
||||
const parts = full.split('/').filter(Boolean);
|
||||
for (let i = 0; i < parts.length; i++) {
|
||||
candidates.add(parts.slice(i).join('/'));
|
||||
}
|
||||
}
|
||||
return [...candidates];
|
||||
return { bare: suffixesOf(pagePath), rooted };
|
||||
}
|
||||
|
||||
function matchesScope(globs, candidates) {
|
||||
return globs.some((glob) => {
|
||||
let re;
|
||||
const regexes = [];
|
||||
for (const glob of globs) {
|
||||
try {
|
||||
re = globToRegex(String(glob));
|
||||
regexes.push(globToRegex(String(glob)));
|
||||
} catch {
|
||||
return false;
|
||||
// Malformed glob: skip it, as matchesAnyGlob does in the CLI.
|
||||
}
|
||||
return candidates.some((candidate) => re.test(candidate));
|
||||
});
|
||||
}
|
||||
if (regexes.length === 0) return false;
|
||||
const hits = (paths) => paths.some((path) => regexes.some((re) => re.test(path)));
|
||||
// Matches on the URL path itself hold whichever root serves the page.
|
||||
if (hits(candidates.bare)) return true;
|
||||
// Root-prefixed matches only hold if no possible identity disagrees.
|
||||
return candidates.rooted.length > 0 && candidates.rooted.every(hits);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -183,6 +183,37 @@ describe('live-browser-ignores resolver', () => {
|
||||
assert.deepEqual(out.disabledRules, ['dark-glow']);
|
||||
});
|
||||
|
||||
it('does not apply a waiver scoped to one root when several roots could serve the URL', () => {
|
||||
// With src/**/*.html and public/**/*.html both configured, /foo.html
|
||||
// could be served from either root. A waiver naming only src/foo.html
|
||||
// must not hide a finding on a page actually served from
|
||||
// public/foo.html; ambiguity resolves to showing the finding.
|
||||
const ignores = {
|
||||
roots: ['src/', 'public/'],
|
||||
ignoreValues: [
|
||||
{ rule: 'dark-glow', value: '*', files: ['src/foo.html'] },
|
||||
{ rule: 'gradient-text', value: 'teal', files: ['src/foo.html'] },
|
||||
],
|
||||
};
|
||||
const out = resolve({ ignores, pathname: '/foo.html' });
|
||||
assert.deepEqual(out, EMPTY);
|
||||
});
|
||||
|
||||
it('applies a scoped waiver under several roots when every identity matches', () => {
|
||||
const ignores = {
|
||||
roots: ['src/', 'public/'],
|
||||
ignoreValues: [
|
||||
// Two globs covering both identities.
|
||||
{ rule: 'dark-glow', value: '*', files: ['src/foo.html', 'public/foo.html'] },
|
||||
// A bare-path glob holds whichever root serves the page.
|
||||
{ rule: 'em-dash-overuse', value: '*', files: ['foo.html'] },
|
||||
{ rule: 'gradient-text', value: '*', files: ['**/foo.html'] },
|
||||
],
|
||||
};
|
||||
const out = resolve({ ignores, pathname: '/foo.html' });
|
||||
assert.deepEqual(out.disabledRules.sort(), ['dark-glow', 'em-dash-overuse', 'gradient-text']);
|
||||
});
|
||||
|
||||
it('survives malformed roots and percent-escapes without throwing', () => {
|
||||
const out = resolve({
|
||||
ignores: {
|
||||
|
||||
Reference in New Issue
Block a user