diff --git a/packages/cli-doctor/package.json b/packages/cli-doctor/package.json index 0d3235f5a..7029fb587 100644 --- a/packages/cli-doctor/package.json +++ b/packages/cli-doctor/package.json @@ -42,7 +42,6 @@ "@percy/env": "1.32.5", "@percy/logger": "1.32.5", "@percy/monitoring": "1.32.5", - "minimatch": "^9.0.0", "ws": "^8.17.1" } } diff --git a/packages/cli-doctor/src/checks/pac.js b/packages/cli-doctor/src/checks/pac.js index 1c889420d..98fc25ccf 100644 --- a/packages/cli-doctor/src/checks/pac.js +++ b/packages/cli-doctor/src/checks/pac.js @@ -3,7 +3,6 @@ import fs from 'fs'; import path from 'path'; import os from 'os'; import vm from 'vm'; -import { minimatch } from 'minimatch'; // Test URL used when evaluating PAC scripts const PAC_TEST_URL = 'https://percy.io/'; @@ -318,6 +317,47 @@ export function findInObject(obj, key, depth = 0) { return null; } +/** + * PAC `shExpMatch` shell-expression match. + * + * The PAC spec defines exactly two wildcards — `*` (any sequence) and `?` + * (exactly one character); every other character is a literal. We match them + * directly rather than delegating to a general glob library: brace expansion, + * character classes and extglobs are not part of shExpMatch, and expanding + * them on PAC content — which is untrusted, attacker-influenced input — is a + * memory-exhaustion vector (see GHSA-mh99-v99m-4gvg in brace-expansion). The + * `vm` timeout below cannot save us there, since it does not interrupt native + * allocation inside a host library. + * + * Greedy single-star backtracking, so worst case is O(str * shexp) with no + * regex construction and no catastrophic backtracking. + */ +export function shExpMatch(str, shexp) { + let s = 0; + let p = 0; + let star = -1; + let resume = 0; + + while (s < str.length) { + if (p < shexp.length && (shexp[p] === '?' || shexp[p] === str[s])) { + s++; p++; + } else if (p < shexp.length && shexp[p] === '*') { + star = p++; + resume = s; + } else if (star !== -1) { + // Backtrack: let the last `*` consume one more character. + p = star + 1; + s = ++resume; + } else { + return false; + } + } + + // Trailing `*`s may match the empty string. + while (p < shexp.length && shexp[p] === '*') p++; + return p === shexp.length; +} + /** * Evaluate a PAC script using Node's built-in `vm` module. * Provides minimal shims for the standard PAC helper functions. @@ -356,8 +396,7 @@ export function runPacScript(script, url, host) { value: (str, shexp) => { if (typeof str !== 'string' || typeof shexp !== 'string') return false; if (shexp.length > 256) return false; - // minimatch handles * and ? exactly as PAC shExpMatch requires. - return minimatch(str, shexp, { dot: true, nocase: false }); + return shExpMatch(str, shexp); }, enumerable: true }, diff --git a/packages/cli-doctor/test/pac.test.js b/packages/cli-doctor/test/pac.test.js index 7949e73e3..75a8b01c8 100644 --- a/packages/cli-doctor/test/pac.test.js +++ b/packages/cli-doctor/test/pac.test.js @@ -6,7 +6,7 @@ * full fetch → evaluate → classify pipeline without system config access. */ -import { runPacScript, findInObject, PACDetector } from '../src/checks/pac.js'; +import { runPacScript, findInObject, PACDetector, shExpMatch } from '../src/checks/pac.js'; import { createPacServer, createHttpServer, withEnv, buildPacScript } from './helpers.js'; import childProcess from 'child_process'; import fsMod from 'fs'; @@ -81,6 +81,68 @@ describe('runPacScript — argument passing', () => { }); }); +// ─── shExpMatch — PAC shell-expression semantics ────────────────────────────── + +describe('shExpMatch', () => { + it('treats * as any sequence and ? as exactly one character', () => { + expect(shExpMatch('dev.percy.io', '*.percy.io')).toBe(true); + expect(shExpMatch('percy.io', '*.percy.io')).toBe(false); + expect(shExpMatch('percy.io', 'percy.i?')).toBe(true); + expect(shExpMatch('percy.ioo', 'percy.i?')).toBe(false); + expect(shExpMatch('percy.io', 'percy.io')).toBe(true); + }); + + it('matches the empty string against * and the empty pattern', () => { + expect(shExpMatch('', '*')).toBe(true); + expect(shExpMatch('', '')).toBe(true); + expect(shExpMatch('percy.io', '')).toBe(false); + expect(shExpMatch('percy.io', '***')).toBe(true); + }); + + it('is case-sensitive, per PAC semantics', () => { + expect(shExpMatch('Percy.IO', 'percy.io')).toBe(false); + }); + + // The PAC spec defines only * and ?. Brace expansion, character classes and + // extglobs are NOT shExpMatch syntax, so they must match literally rather + // than being expanded — expanding attacker-supplied patterns is the + // brace-expansion OOM vector (GHSA-mh99-v99m-4gvg). + it('treats brace, bracket and extglob syntax as literal characters', () => { + expect(shExpMatch('{a,b}', '{a,b}')).toBe(true); + expect(shExpMatch('a', '{a,b}')).toBe(false); + expect(shExpMatch('b', '{a,b}')).toBe(false); + expect(shExpMatch('[abc]', '[abc]')).toBe(true); + expect(shExpMatch('a', '[abc]')).toBe(false); + expect(shExpMatch('percy.io', '!(percy.io)')).toBe(false); + expect(shExpMatch('a+(b)', 'a+(b)')).toBe(true); + }); + + it('treats regex metacharacters as literals', () => { + expect(shExpMatch('a.c', 'a.c')).toBe(true); + expect(shExpMatch('abc', 'a.c')).toBe(false); + expect(shExpMatch('a$^(b)', 'a$^(b)')).toBe(true); + }); + + it('returns promptly for brace-bomb and backtracking-heavy patterns', () => { + // Nested braces well under the 256-char sandbox cap: expanding these would + // exhaust memory, and `vm`'s timeout cannot interrupt native allocation. + const target = 'a'.repeat(200); + const braceBomb = '{a,b}'.repeat(40); + const starBomb = '*a'.repeat(100); + const start = process.hrtime.bigint(); + + // Literal braces, so no expansion and no match. + expect(shExpMatch(target, braceBomb)).toBe(false); + // 100 stars still resolve correctly, both matching and — the worst case for + // backtracking — failing only at the very last pattern character. + expect(shExpMatch(target, starBomb)).toBe(true); + expect(shExpMatch(target, starBomb + 'b')).toBe(false); + + const elapsedMs = Number(process.hrtime.bigint() - start) / 1e6; + expect(elapsedMs).toBeLessThan(1000); + }); +}); + // ─── runPacScript — shExpMatch wildcard matching ────────────────────────────── describe('runPacScript — shExpMatch', () => { diff --git a/packages/core/package.json b/packages/core/package.json index 05d19cd18..e949bd9ba 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -57,12 +57,11 @@ "content-disposition": "^0.5.4", "cross-spawn": "^7.0.3", "fast-glob": "^3.2.11", - "fast-xml-parser": "^4.4.1", + "fast-xml-parser": "^4.5.7", "micromatch": "^4.0.8", "mime-types": "^2.1.34", "pako": "^2.1.0", "path-to-regexp": "^6.3.0", - "rimraf": "^3.0.2", "ws": "^8.17.1", "yaml": "^2.4.1" }, diff --git a/packages/core/src/browser.js b/packages/core/src/browser.js index fd526c4b4..fad72151e 100644 --- a/packages/core/src/browser.js +++ b/packages/core/src/browser.js @@ -5,7 +5,6 @@ import { execFileSync } from 'child_process'; import spawn from 'cross-spawn'; import EventEmitter from 'events'; import WebSocket from 'ws'; -import rimraf from 'rimraf'; import logger from '@percy/logger'; import install from './install.js'; import { MAX_CDP_PAYLOAD } from './network.js'; @@ -189,9 +188,14 @@ export class Browser extends EventEmitter { /* istanbul ignore next: * this might fail on some systems but ultimately it is just a temp file */ if (this.profile) { - // attempt to clean up the profile directory - return new Promise((resolve, reject) => { - rimraf(this.profile, e => e ? reject(e) : resolve()); + // attempt to clean up the profile directory. maxRetries mirrors the + // busy-retry behaviour rimraf gave us: on Windows the profile dir is + // often still locked for a moment after the browser process exits. + return fs.promises.rm(this.profile, { + recursive: true, + force: true, + maxRetries: 3, + retryDelay: 100 }).catch(error => { this.log.debug('Could not clean up temporary browser profile directory.'); this.log.debug(error); diff --git a/scripts/test.js b/scripts/test.js index 64cde2f8a..68e44ce73 100644 --- a/scripts/test.js +++ b/scripts/test.js @@ -75,14 +75,15 @@ async function main({ let testBrowsers = browsers != null ? browsers : (!node && pkg.browser); if (coverage) { - // $ rimraf /{.nyc_output,coverage} || true && + // $ rm -rf /{.nyc_output,coverage} || true && // nyc --silent --no-clean node /test.js ... && // nyc report --reporter let flags = flagify({ node, browsers }); let nycbin = path.resolve(filename, '../../node_modules/.bin/nyc'); - let { default: rimraf } = await import('rimraf'); - await new Promise(r => rimraf(path.join(cwd, '{.nyc_output,coverage}'), r)); + for (let dir of ['.nyc_output', 'coverage']) { + await fs.promises.rm(path.join(cwd, dir), { recursive: true, force: true }); + } await child('spawn', nycbin, ['--silent', '--no-clean', 'node', filename, ...flags]); await child('spawn', nycbin, ['report', '--check-coverage', ...flagify({ reporter })]); } else if (!process.send) { diff --git a/yarn.lock b/yarn.lock index 09117dd1a..943b5ecb1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2961,13 +2961,6 @@ brace-expansion@^2.0.1: dependencies: balanced-match "^1.0.0" -brace-expansion@^2.0.2: - version "2.0.2" - resolved "https://registry.yarnpkg.com/brace-expansion/-/brace-expansion-2.0.2.tgz#54fc53237a613d854c7bd37463aad17df87214e7" - integrity sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ== - dependencies: - balanced-match "^1.0.0" - braces@^3.0.2, braces@^3.0.3, braces@~3.0.2: version "3.0.3" resolved "https://registry.npmjs.org/braces/-/braces-3.0.3.tgz" @@ -4362,10 +4355,10 @@ fast-uri@^3.0.1: resolved "https://registry.npmjs.org/fast-uri/-/fast-uri-3.0.2.tgz" integrity sha512-GR6f0hD7XXyNJa25Tb9BuIdN0tdr+0BMi6/CJPH3wJO1JjNG3n/VsSw38AwRdKZABm8lGbPfakLRkYzx2V9row== -fast-xml-parser@^4.4.1: - version "4.5.6" - resolved "https://registry.yarnpkg.com/fast-xml-parser/-/fast-xml-parser-4.5.6.tgz#4ff57d4aca13a2d11aa42ad460495cf00f32b655" - integrity sha512-Yd4vkROfJf8AuJrDIVMVmYfULKmIJszVsMv7Vo71aocsKgFxpdlpSHXSaInvyYfgw2PRuObQSW2GFpVMUjxu9A== +fast-xml-parser@^4.5.7: + version "4.5.7" + resolved "https://registry.yarnpkg.com/fast-xml-parser/-/fast-xml-parser-4.5.7.tgz#ba3479a1eca8abf2cc3e54af254fae6f1f115021" + integrity sha512-a6Qh1RMCNbSrU1+sAyAAZH3rTe+OaWJbNZIq0S+ifZciUUOQtlVxBJwoTUE2bYhysmG/RYyI5WJFIKdBahJdrQ== dependencies: strnum "^1.0.5" @@ -6203,13 +6196,6 @@ minimatch@^8.0.2: dependencies: brace-expansion "^2.0.1" -minimatch@^9.0.0: - version "9.0.9" - resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-9.0.9.tgz#9b0cb9fcb78087f6fd7eababe2511c4d3d60574e" - integrity sha512-OBwBN9AL4dqmETlpS2zasx+vTeWclWzkblfZk7KTA5j3jeOONz/tRCnZomUyvNg83wL5Zv9Ss6HMJXAgL8R2Yg== - dependencies: - brace-expansion "^2.0.2" - minimist-options@4.1.0: version "4.1.0" resolved "https://registry.npmjs.org/minimist-options/-/minimist-options-4.1.0.tgz"