mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-14 23:26:39 +03:00
Fix: harden live overlay detector waivers (#639 follow-up)
Read waiver config from every live root (appRoot, contextRoot, repoRoot), so monorepo projects whose config lives at the repo root reach the overlay; serialize served roots and page identities repo-relative there. Resolve each page URL to its actual serving file via the inject config's resolved page list before applying file-scoped waivers; ambiguous URLs keep the conservative common-ancestor fallback (PR #645 review discussion r3840011436). Honour detector.ignoreFiles: a wholly waived page now scans to zero findings in the overlay, matching the CLI and the edit hook. Guard the resolver call so a throwing resolver degrades to an unfiltered scan instead of breaking the detect toggle. Match design-system-color waivers by color value across hex and rgb() spellings, and stop extracting font values for bounce-easing findings, mirroring extractFindingIgnoreValue. Regenerate the browser bundle. AI-assisted change: reviewed, planned, and implemented with Claude Code under maintainer direction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Abdul Wahab
committed by
Abdul Wahab
Abdul Wahab
co-authored by
Claude Fable 5
Claude Fable 5
parent
09506a9bb5
commit
152d6940b0
@@ -16,6 +16,20 @@
|
||||
* 3. Remaining `ignoreValues` entries match on the finding's own value;
|
||||
* those are forwarded as `disabledValues` for the detector bundle to
|
||||
* apply where the findings are assembled.
|
||||
* 4. `ignoreFiles` globs that name the page waive it wholesale: the
|
||||
* resolver reports `skipScan: true` and the detector answers the scan
|
||||
* with zero findings, mirroring shouldIgnoreDetectionFile in the CLI
|
||||
* and the edit hook's own ignoreFiles gate.
|
||||
*
|
||||
* `pageFiles`, when the server could resolve it, lists the real project
|
||||
* files the inject config serves. A URL that suffix-matches exactly one of
|
||||
* them takes that file as its only project identity; an ambiguous or absent
|
||||
* match falls back to the served-root common ancestor below.
|
||||
*
|
||||
* Known gap, unchanged from PR #645: framework apps inject into source files
|
||||
* (src/routes/about/+page.svelte) while scans see route URLs (/about), so
|
||||
* entries scoped to source or asset paths never match a page candidate and
|
||||
* are dropped. That shows the finding, which is the conservative direction.
|
||||
*
|
||||
* Kept separate from live-browser.js so the glob and page-scope logic can be
|
||||
* unit tested in Node (tests/live-browser-ignores.test.mjs) without the full
|
||||
@@ -101,7 +115,7 @@
|
||||
// alternatives at all, and demanding a waiver match under both stops
|
||||
// prototype/index.html from applying anywhere. When the globs share no
|
||||
// common root, no prefix is asserted and only the URL path itself matches.
|
||||
function pageCandidates(pathname, roots) {
|
||||
function pageCandidates(pathname, roots, pageFiles) {
|
||||
let pagePath = String(pathname || '');
|
||||
try {
|
||||
pagePath = decodeURIComponent(pagePath);
|
||||
@@ -113,6 +127,32 @@
|
||||
// name files. Without this, /news/ never matches prototype/news/index.html.
|
||||
if (pagePath === '' || pagePath.endsWith('/')) pagePath += 'index.html';
|
||||
|
||||
const candidates = new Set();
|
||||
const addSuffixes = (fullPath) => {
|
||||
const parts = fullPath.split('/').filter(Boolean);
|
||||
for (let i = 0; i < parts.length; i++) {
|
||||
candidates.add(parts.slice(i).join('/'));
|
||||
}
|
||||
};
|
||||
addSuffixes(pagePath);
|
||||
|
||||
// The served page list names the real files the inject config serves.
|
||||
// A URL that suffix-matches exactly one of them has an unambiguous
|
||||
// project identity; assert that identity and stop guessing from roots
|
||||
// (PR #645 review: with src/ and public/ both served, /foo.html must not
|
||||
// borrow src/foo.html's waivers while actually serving public/foo.html).
|
||||
// Zero matches or several fall through to the common-ancestor fallback:
|
||||
// ambiguity resolves toward showing the finding.
|
||||
const knownPages = [];
|
||||
for (const entry of Array.isArray(pageFiles) ? pageFiles : []) {
|
||||
if (typeof entry !== 'string' || !entry) continue;
|
||||
if (entry === pagePath || entry.endsWith('/' + pagePath)) knownPages.push(entry);
|
||||
}
|
||||
if (knownPages.length === 1) {
|
||||
addSuffixes(knownPages[0]);
|
||||
return [...candidates];
|
||||
}
|
||||
|
||||
const prefixes = [];
|
||||
for (const entry of Array.isArray(roots) ? roots : []) {
|
||||
if (typeof entry !== 'string') continue;
|
||||
@@ -125,14 +165,6 @@
|
||||
common = common.slice(0, i);
|
||||
}
|
||||
|
||||
const candidates = new Set();
|
||||
const addSuffixes = (fullPath) => {
|
||||
const parts = fullPath.split('/').filter(Boolean);
|
||||
for (let i = 0; i < parts.length; i++) {
|
||||
candidates.add(parts.slice(i).join('/'));
|
||||
}
|
||||
};
|
||||
addSuffixes(pagePath);
|
||||
if (common.length > 0) addSuffixes(common.join('/') + '/' + pagePath);
|
||||
return [...candidates];
|
||||
}
|
||||
@@ -158,12 +190,21 @@
|
||||
* in whatever state it arrived: absent, null, or hand-edited into the
|
||||
* wrong shape. Every read tolerates that and degrades to no filtering.
|
||||
* @param {string} options.pathname location.pathname of the scanned page.
|
||||
* @returns {{ disabledRules: string[], disabledValues: Array<{rule: string, value: string}> }}
|
||||
* @returns {{ disabledRules: string[], disabledValues: Array<{rule: string, value: string}>, skipScan: boolean }}
|
||||
*/
|
||||
function resolveDetectIgnores({ ignores, pathname } = {}) {
|
||||
const config = ignores && typeof ignores === 'object' ? ignores : {};
|
||||
const asArray = (value) => (Array.isArray(value) ? value : []);
|
||||
const candidates = pageCandidates(pathname, config.roots);
|
||||
const candidates = pageCandidates(pathname, config.roots, config.pageFiles);
|
||||
|
||||
// detector.ignoreFiles waives whole files. When any glob names this
|
||||
// page, the scan itself is skipped; rule and value lists are returned
|
||||
// empty because nothing will run.
|
||||
const ignoreFileGlobs = asArray(config.ignoreFiles)
|
||||
.filter((glob) => typeof glob === 'string' && glob.trim());
|
||||
if (ignoreFileGlobs.length > 0 && matchesScope(ignoreFileGlobs, candidates)) {
|
||||
return { disabledRules: [], disabledValues: [], skipScan: true };
|
||||
}
|
||||
|
||||
const disabledRules = new Set(
|
||||
asArray(config.ignoreRules)
|
||||
@@ -191,7 +232,7 @@
|
||||
disabledValues.push({ rule, value });
|
||||
}
|
||||
|
||||
return { disabledRules: [...disabledRules], disabledValues };
|
||||
return { disabledRules: [...disabledRules], disabledValues, skipScan: false };
|
||||
}
|
||||
|
||||
root.__IMPECCABLE_LIVE_IGNORES__ = {
|
||||
|
||||
@@ -11147,20 +11147,32 @@ void main() {
|
||||
// filters the same findings the CLI and the edit hook do (issue #639).
|
||||
// live-browser-ignores.js resolves .impeccable config for this page:
|
||||
// ignoreRules suppress outright, wildcard ignoreValues suppress their
|
||||
// rule in the files they name, and the rest match on the finding's own
|
||||
// value inside the detector. Guarded so a stale cached live.js without
|
||||
// the resolver part still scans, just unfiltered as before.
|
||||
// rule in the files they name, ignoreFiles that name the page skip the
|
||||
// scan wholesale, and the rest match on the finding's own value inside
|
||||
// the detector. Guarded twice: a stale cached live.js without the
|
||||
// resolver part still scans, and a resolver that throws must not brick
|
||||
// the detect toggle; both degrade to an unfiltered scan.
|
||||
const ignoresApi = window.__IMPECCABLE_LIVE_IGNORES__;
|
||||
const ignores = typeof ignoresApi?.resolveDetectIgnores === 'function'
|
||||
? ignoresApi.resolveDetectIgnores({
|
||||
ignores: window.__IMPECCABLE_PROJECT_IGNORES__,
|
||||
pathname: location.pathname,
|
||||
})
|
||||
: { disabledRules: [], disabledValues: [] };
|
||||
let ignores = { disabledRules: [], disabledValues: [], skipScan: false };
|
||||
if (typeof ignoresApi?.resolveDetectIgnores === 'function') {
|
||||
try {
|
||||
ignores = ignoresApi.resolveDetectIgnores({
|
||||
ignores: window.__IMPECCABLE_PROJECT_IGNORES__,
|
||||
pathname: location.pathname,
|
||||
}) || ignores;
|
||||
} catch (e) {
|
||||
ignores = { disabledRules: [], disabledValues: [], skipScan: false };
|
||||
}
|
||||
}
|
||||
window.postMessage({
|
||||
source: 'impeccable-command',
|
||||
action: 'scan',
|
||||
config: { scanId, disabledRules: ignores.disabledRules, disabledValues: ignores.disabledValues },
|
||||
config: {
|
||||
scanId,
|
||||
disabledRules: ignores.disabledRules || [],
|
||||
disabledValues: ignores.disabledValues || [],
|
||||
skipScan: ignores.skipScan === true,
|
||||
},
|
||||
}, '*');
|
||||
}
|
||||
|
||||
|
||||
@@ -22,7 +22,6 @@ import net from 'node:net';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { parseDesignMd } from './lib/design-parser.mjs';
|
||||
import { loadContext } from './context.mjs';
|
||||
import { readConfig as readHookConfig } from './hook-lib.mjs';
|
||||
import {
|
||||
assembleLiveBrowserScript,
|
||||
assertLiveBrowserScriptParts,
|
||||
@@ -46,10 +45,10 @@ import {
|
||||
readLiveServerInfo,
|
||||
removeLiveServerInfo,
|
||||
resolveDesignSidecarPath,
|
||||
resolveLiveConfigPath,
|
||||
writeLiveServerInfo,
|
||||
} from './lib/impeccable-paths.mjs';
|
||||
import { countByPage as countPendingByPage } from './live/manual-edits-buffer.mjs';
|
||||
import { collectProjectDetectorIgnores } from './live/project-ignores.mjs';
|
||||
import {
|
||||
createManualApplyController,
|
||||
summarizeManualApplyFailures,
|
||||
@@ -696,51 +695,6 @@ function isLoopbackOrigin(origin) {
|
||||
// HTTP request handler
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Project detector waivers for the browser overlay (issue #639). The CLI and
|
||||
// the edit hook filter findings through .impeccable/config.json; the overlay
|
||||
// scans in the browser, so the same config rides along in the /live.js
|
||||
// prelude and live-browser-ignores.js applies it per page at scan time.
|
||||
function readProjectDetectorIgnores() {
|
||||
// readConfig merges config.json with the gitignored config.local.json and
|
||||
// type-checks both, exactly as the edit hook reads the same pair.
|
||||
const config = readHookConfig(process.cwd());
|
||||
return {
|
||||
ignoreRules: Array.isArray(config.ignoreRules) ? config.ignoreRules : [],
|
||||
// Serve only what the browser matches on; createdAt/reason stay local.
|
||||
ignoreValues: (Array.isArray(config.ignoreValues) ? config.ignoreValues : []).map((entry) => ({
|
||||
rule: entry.rule,
|
||||
value: entry.value,
|
||||
...(Array.isArray(entry.files) && entry.files.length > 0 ? { files: entry.files } : {}),
|
||||
})),
|
||||
roots: readLiveServedRoots(),
|
||||
};
|
||||
}
|
||||
|
||||
// Where the served pages live inside the project. Ignore globs are
|
||||
// project-relative and the browser only knows its URL path, so it needs the
|
||||
// prefix; the inject config's own `files` globs are the authority on it.
|
||||
// Deriving it from the ignore globs instead fails silently: one entry scoped
|
||||
// to prototype/library/** would lend prototype/library/ as a candidate prefix
|
||||
// to every page, and that rule would suppress site-wide.
|
||||
function readLiveServedRoots() {
|
||||
try {
|
||||
const configPath = resolveLiveConfigPath({ cwd: process.cwd(), scriptsDir: __dirname });
|
||||
const live = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
|
||||
const files = Array.isArray(live?.files) ? live.files : [];
|
||||
return [...new Set(files
|
||||
.filter((glob) => typeof glob === 'string' && glob)
|
||||
.map((glob) => {
|
||||
const wildcardAt = glob.search(/[*?{]/);
|
||||
const head = wildcardAt === -1 ? glob : glob.slice(0, wildcardAt);
|
||||
const cut = head.lastIndexOf('/');
|
||||
return cut > -1 ? head.slice(0, cut + 1) : '';
|
||||
}))];
|
||||
} catch {
|
||||
// No readable inject config: the browser matches URL paths as-is.
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
function createRequestHandler({ detectScript, liveScriptParts }) {
|
||||
return (req, res) => {
|
||||
const url = new URL(req.url, `http://localhost:${state.port}`);
|
||||
@@ -802,8 +756,16 @@ function createRequestHandler({ detectScript, liveScriptParts }) {
|
||||
appRoot: process.cwd(),
|
||||
parts,
|
||||
// Read per request rather than cached, so editing the config and
|
||||
// reloading the tab is enough to pick up a new waiver.
|
||||
projectIgnores: readProjectDetectorIgnores(),
|
||||
// reloading the tab is enough to pick up a new waiver. Config comes
|
||||
// from every root the session spans (appRoot, contextRoot, repoRoot):
|
||||
// in a monorepo the hook and the CLI key it at the repo root, which
|
||||
// is not the appRoot this process chdir'd onto.
|
||||
projectIgnores: collectProjectDetectorIgnores({
|
||||
appRoot: process.cwd(),
|
||||
contextRoot: LIVE_ROOTS?.contextRoot,
|
||||
repoRoot: LIVE_ROOTS?.repoRoot,
|
||||
scriptsDir: __dirname,
|
||||
}),
|
||||
});
|
||||
res.writeHead(200, {
|
||||
'Content-Type': 'application/javascript',
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
/**
|
||||
* Project detector waivers for the live overlay (issue #639, hardened in the
|
||||
* PR #645 follow-up). One place decides what the /live.js prelude serializes
|
||||
* as window.__IMPECCABLE_PROJECT_IGNORES__:
|
||||
*
|
||||
* ignoreRules detector.ignoreRules, unioned across every live root.
|
||||
* ignoreValues detector.ignoreValues entries ({rule, value, files?}),
|
||||
* deduped across roots; createdAt/reason stay local.
|
||||
* ignoreFiles detector.ignoreFiles globs, unioned across roots, so a
|
||||
* wholly waived page scans to zero findings in the overlay
|
||||
* just as it reports nothing through the CLI and the hook.
|
||||
* roots served-root prefixes derived from the inject config's own
|
||||
* `files` globs. Never derived from the ignore globs: one
|
||||
* entry scoped to prototype/library/** would lend
|
||||
* prototype/library/ as a candidate prefix to every page,
|
||||
* and that rule would suppress site-wide (issue #639).
|
||||
* pageFiles the inject config's `files` expanded to real project
|
||||
* files, so the browser can resolve a URL to the one file it
|
||||
* actually serves instead of trying every root (PR #645
|
||||
* review: with src/ and public/ both served, /foo.html must
|
||||
* not borrow src/foo.html's waivers while actually serving
|
||||
* public/foo.html).
|
||||
*
|
||||
* Config is read from every root the live session spans: the appRoot the
|
||||
* server chdir'd onto, plus contextRoot and repoRoot when they differ. The
|
||||
* edit hook keys the same config at the session cwd (the repo root in a
|
||||
* monorepo, via resolveCacheCwd), and `impeccable detect` reads it from its
|
||||
* invocation cwd, so reading only the appRoot silently dropped every waiver
|
||||
* in exactly the monorepo layouts the roots manifest exists for. Reading is
|
||||
* additive across roots, matching readConfig's own union of config.json and
|
||||
* config.local.json.
|
||||
*
|
||||
* In a monorepo, roots and pageFiles are serialized repo-relative (the
|
||||
* appRoot's path inside the repo is prefixed), so waivers spelled from
|
||||
* either root match through the resolver's suffix expansion.
|
||||
*/
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { readConfig } from '../hook-lib.mjs';
|
||||
import { resolveFiles } from '../live-inject.mjs';
|
||||
import { resolveLiveConfigPath } from '../lib/impeccable-paths.mjs';
|
||||
|
||||
// Serializing thousands of page identities into every /live.js response
|
||||
// helps nobody; past this cap pageFiles is omitted and the resolver falls
|
||||
// back to the served-root common ancestor, which is correct, just less
|
||||
// precise about cross-root duplicates.
|
||||
const PAGE_FILES_CAP = 500;
|
||||
|
||||
export function collectProjectDetectorIgnores({ appRoot, contextRoot, repoRoot, scriptsDir } = {}) {
|
||||
const configRoots = [];
|
||||
for (const dir of [appRoot, contextRoot, repoRoot]) {
|
||||
if (typeof dir !== 'string' || !dir) continue;
|
||||
const resolved = path.resolve(dir);
|
||||
if (!configRoots.includes(resolved)) configRoots.push(resolved);
|
||||
}
|
||||
if (configRoots.length === 0) configRoots.push(process.cwd());
|
||||
|
||||
const ignoreRules = new Set();
|
||||
const ignoreFiles = new Set();
|
||||
const valueEntries = new Map();
|
||||
for (const dir of configRoots) {
|
||||
// readConfig merges config.json with the gitignored config.local.json
|
||||
// and type-checks both, exactly as the edit hook reads the same pair.
|
||||
const config = readConfig(dir);
|
||||
for (const rule of Array.isArray(config.ignoreRules) ? config.ignoreRules : []) {
|
||||
if (typeof rule === 'string' && rule.trim()) ignoreRules.add(rule);
|
||||
}
|
||||
for (const glob of Array.isArray(config.ignoreFiles) ? config.ignoreFiles : []) {
|
||||
if (typeof glob === 'string' && glob.trim()) ignoreFiles.add(glob);
|
||||
}
|
||||
for (const entry of Array.isArray(config.ignoreValues) ? config.ignoreValues : []) {
|
||||
if (!entry || typeof entry !== 'object') continue;
|
||||
// readConfig already normalized rule/value and folded `file` into
|
||||
// `files`; serve only what the browser matches on.
|
||||
const serialized = {
|
||||
rule: entry.rule,
|
||||
value: entry.value,
|
||||
...(Array.isArray(entry.files) && entry.files.length > 0 ? { files: entry.files } : {}),
|
||||
};
|
||||
const key = JSON.stringify([serialized.rule, serialized.value,
|
||||
Array.isArray(serialized.files) ? [...serialized.files].sort() : []]);
|
||||
if (!valueEntries.has(key)) valueEntries.set(key, serialized);
|
||||
}
|
||||
}
|
||||
|
||||
const served = readLiveServedPages({ appRoot: configRoots[0], repoRoot, scriptsDir });
|
||||
return {
|
||||
ignoreRules: [...ignoreRules],
|
||||
ignoreValues: [...valueEntries.values()],
|
||||
ignoreFiles: [...ignoreFiles],
|
||||
roots: served.roots,
|
||||
pageFiles: served.pageFiles,
|
||||
};
|
||||
}
|
||||
|
||||
function readLiveServedPages({ appRoot, repoRoot, scriptsDir }) {
|
||||
let live = null;
|
||||
try {
|
||||
const configPath = resolveLiveConfigPath({ cwd: appRoot, scriptsDir });
|
||||
live = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
|
||||
} catch {
|
||||
// No readable inject config: the browser matches URL paths as-is.
|
||||
return { roots: [], pageFiles: [] };
|
||||
}
|
||||
const files = Array.isArray(live?.files)
|
||||
? live.files.filter((glob) => typeof glob === 'string' && glob)
|
||||
: [];
|
||||
|
||||
// A monorepo appRoot serializes identities repo-relative, so waivers
|
||||
// spelled from either root match through the resolver's suffix expansion.
|
||||
let prefix = '';
|
||||
if (typeof repoRoot === 'string' && repoRoot) {
|
||||
const rel = path.relative(path.resolve(repoRoot), path.resolve(appRoot)).split(path.sep).join('/');
|
||||
if (rel && !rel.startsWith('..') && !path.isAbsolute(rel)) prefix = `${rel}/`;
|
||||
}
|
||||
|
||||
const roots = [...new Set(files.map((glob) => {
|
||||
const wildcardAt = glob.search(/[*?{]/);
|
||||
const head = wildcardAt === -1 ? glob : glob.slice(0, wildcardAt);
|
||||
const cut = head.lastIndexOf('/');
|
||||
return prefix + (cut > -1 ? head.slice(0, cut + 1) : '');
|
||||
}))];
|
||||
|
||||
let pageFiles = [];
|
||||
try {
|
||||
pageFiles = resolveFiles(appRoot, { ...live, files })
|
||||
.filter((rel) => {
|
||||
// resolveFiles passes literal entries through even when they do not
|
||||
// exist; a missing file is nobody's identity.
|
||||
try { return fs.statSync(path.join(appRoot, rel)).isFile(); } catch { return false; }
|
||||
})
|
||||
.map((rel) => prefix + rel);
|
||||
} catch {
|
||||
pageFiles = [];
|
||||
}
|
||||
if (pageFiles.length > PAGE_FILES_CAP) pageFiles = [];
|
||||
|
||||
return { roots, pageFiles };
|
||||
}
|
||||
Reference in New Issue
Block a user