Skip to content

fix(deps): clear audit advisories by dropping rimraf/minimatch and bumping fast-xml-parser #2356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rishigupta1599 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(deps): clear audit advisories by dropping rimraf/minimatch and bumping fast-xml-parser #2356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e13e3b
fix(deps): clear audit advisories by dropping rimraf/minimatch and bu…
rishigupta1599 Jul 28, 2026
b7f4131
fix(core): keep fast-xml-parser on 4.x — v5 pulls Node-16-only transi…
rishigupta1599 Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/cli-doctor/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
45 changes: 42 additions & 3 deletions packages/cli-doctor/src/checks/pac.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/';
Expand Down Expand Up @@ -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) {

@rishigupta1599 rishigupta1599 Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Low] Behaviour change vs minimatch worth a CHANGELOG note

This is not a like-for-like replacement. Verified against the installed minimatch:

  • minimatch('http://home.netscape.com/people/ari/index.html', '*/ari/*', { dot: true, nocase: false }) → false (minimatch will not let * cross /); the new shExpMatch → true, which is what the PAC spec specifies.
  • minimatch('', '*') → false; the new implementation → true.

Both are corrections — the old code silently mis-evaluated any shExpMatch(url, ...) call using a path pattern — but they are functional changes riding in under a dependency-hygiene commit, and they can flip a PAC script between DIRECT and PROXY.

Suggestion: no code change; mention in the CHANGELOG and PR description that removing minimatch also fixes full-URL and empty-string matching.

Reviewer: stack-code-reviewer

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.
Expand Down Expand Up @@ -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
},
Expand Down
64 changes: 63 additions & 1 deletion packages/cli-doctor/test/pac.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

@rishigupta1599 rishigupta1599 Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] No shExpMatch coverage for a match target containing /

No test in the suite calls shExpMatch (directly or via runPacScript) with a target containing / — verified, zero such call sites. Every case uses hostname-like targets.

That gap lands on the one input class whose result actually changed in this PR: minimatch treated / as a path-segment boundary that * would not cross, the new matcher lets * cross it (which is what the PAC spec requires). It is also the only example in the canonical Netscape PAC documentation. So the behaviour change is untested in both directions.

Suggestion: add the spec example, plus one end-to-end case through runPacScript with a full-URL pattern:

it('matches wildcards across "/" when matching a full URL (spec: */ari/*)', () => {
  expect(shExpMatch('http://home.netscape.com/people/ari/index.html', '*/ari/*')).toBe(true);
  expect(shExpMatch('http://home.netscape.com/people/montulli/index.html', '*/ari/*')).toBe(false);
});

Reviewer: stack-code-reviewer

});

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', () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
12 changes: 8 additions & 4 deletions packages/core/src/browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 4 additions & 3 deletions scripts/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,15 @@ async function main({
let testBrowsers = browsers != null ? browsers : (!node && pkg.browser);

if (coverage) {
// $ rimraf <cwd>/{.nyc_output,coverage} || true &&
// $ rm -rf <cwd>/{.nyc_output,coverage} || true &&
// nyc --silent --no-clean node <root>/test.js ... &&
// nyc report --reporter <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) {
Expand Down
22 changes: 4 additions & 18 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"

Expand Down Expand Up @@ -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"
Expand Down
Loading