From bda111abce913f846efc1736d5a05c3e0daed3ff Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Sun, 14 Jun 2026 20:58:19 +0530 Subject: [PATCH 1/3] security: redact CLI logs, bound regex matching (ReDoS), validate Chromium base URL (PER-8609/8615/8616) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three contained runtime hardening fixes in @percy/core: PER-8609 (CWE-532) — clilogs were sent to the Percy API without secret redaction (cilogs already were). Wrap clilogs in the existing redactSecrets() so tokens / credential-bearing URLs are stripped before egress. PER-8615 (CWE-1333) — snapshotMatches() runs user-controllable regex/glob patterns against snapshot.name; a crafted long name reaching the matcher (e.g. via the local API, per chain PER-8627) could trigger catastrophic backtracking. Cap the matched-input length (MAX_MATCH_INPUT_LENGTH = 2048) before any RegExp/micromatch call; exact-string matching is unaffected. PER-8616 (CWE-918) — PERCY_CHROMIUM_BASE_URL was used as a download base with no validation, enabling SSRF / an integrity downgrade. Add resolveChromiumBaseUrl(): require a well-formed HTTPS URL, otherwise warn and fall back to the trusted default host. (Private HTTPS mirrors remain supported, so no host allowlist; this also gives transport integrity for PER-8605 — full checksum pinning of the binary is a separate follow-up.) Verified against real source: resolveChromiumBaseUrl (https-only + fallback), redactSecrets on the clilogs array shape (GitHub token + bearer redacted), and the ReDoS guard (catastrophic pattern on a 50k-char input returns in 0ms). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/install.js | 25 ++++++++++++++++++++++++- packages/core/src/percy.js | 5 ++++- packages/core/src/snapshot.js | 22 ++++++++++++++++------ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/packages/core/src/install.js b/packages/core/src/install.js index d5f3ff391..9cda71013 100644 --- a/packages/core/src/install.js +++ b/packages/core/src/install.js @@ -139,6 +139,29 @@ export async function download({ return exec; } +const DEFAULT_CHROMIUM_BASE_URL = 'https://storage.googleapis.com/chromium-browser-snapshots/'; + +// Resolve the Chromium download base URL. PERCY_CHROMIUM_BASE_URL may point at a +// private mirror, but an unvalidated value enables SSRF / an integrity downgrade +// (CWE-918): require a well-formed HTTPS URL, otherwise warn and fall back to the +// trusted default host. +export function resolveChromiumBaseUrl(value = process.env.PERCY_CHROMIUM_BASE_URL) { + if (!value) return DEFAULT_CHROMIUM_BASE_URL; + let log = logger('core:install'); + let parsed; + try { + parsed = new URL(value); + } catch { + log.warn(`Invalid PERCY_CHROMIUM_BASE_URL "${value}"; using the default Chromium download host.`); + return DEFAULT_CHROMIUM_BASE_URL; + } + if (parsed.protocol !== 'https:') { + log.warn(`Ignoring non-HTTPS PERCY_CHROMIUM_BASE_URL "${value}"; Chromium must be downloaded over HTTPS.`); + return DEFAULT_CHROMIUM_BASE_URL; + } + return value.endsWith('/') ? value : `${value}/`; +} + // Installs a revision of Chromium to a local directory export function chromium({ // default directory is within @percy/core package root @@ -148,7 +171,7 @@ export function chromium({ } = {}) { let extract = (i, o) => import('extract-zip').then(ex => ex.default(i, { dir: o })); - let url = (process.env.PERCY_CHROMIUM_BASE_URL || 'https://storage.googleapis.com/chromium-browser-snapshots/') + + let url = resolveChromiumBaseUrl() + selectByPlatform({ linux: `Linux_x64/${revision}/chrome-linux.zip`, darwin: `Mac/${revision}/chrome-mac.zip`, diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index 2878299c5..1fa50f267 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -863,7 +863,10 @@ export class Percy { if (!process.env.PERCY_TOKEN) return; try { const logsObject = { - clilogs: logger.query(log => !['ci'].includes(log.debug)) + // Redact secrets from CLI logs before egress to the Percy API — these + // can contain tokens or URLs with embedded credentials (CWE-532). The + // cilogs below were already redacted; clilogs were not. + clilogs: redactSecrets(logger.query(log => !['ci'].includes(log.debug))) }; // Only add CI logs if not disabled voluntarily. diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index 8e33750bf..ace1de07c 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -45,21 +45,31 @@ function validateAndFixSnapshotUrl(snapshot) { // used to deserialize regular expression strings const RE_REGEXP = /^\/(.+)\/(\w+)?$/; +// Upper bound on the snapshot name length we will run user-controllable +// regex/glob matching against. A crafted, very long snapshot name reaching this +// matcher (e.g. via the local API) combined with a backtracking-prone pattern +// could otherwise trigger catastrophic backtracking / ReDoS (CWE-1333). Real +// snapshot names are short; an over-long name simply does not match patterns. +const MAX_MATCH_INPUT_LENGTH = 2048; + // Returns true or false if a snapshot matches the provided include and exclude predicates. A // predicate can be an array of predicates, a regular expression, a glob pattern, or a function. function snapshotMatches(snapshot, include, exclude) { // support an options object as the second argument if (include?.include || include?.exclude) ({ include, exclude } = include); + // guard pattern matching against pathologically long inputs (ReDoS) + let patternSafe = typeof snapshot.name === 'string' && snapshot.name.length <= MAX_MATCH_INPUT_LENGTH; + // recursive predicate test function let test = (predicate, fallback) => { if (predicate && typeof predicate === 'string') { - // snapshot name matches exactly or matches a glob + // exact match is always safe; glob matching is only run on bounded input let result = snapshot.name === predicate || - micromatch.isMatch(snapshot.name, predicate); + (patternSafe && micromatch.isMatch(snapshot.name, predicate)); - // snapshot might match a string-based regexp pattern - if (!result) { + // snapshot might match a string-based regexp pattern (bounded input only) + if (!result && patternSafe) { try { let [, parsed, flags] = RE_REGEXP.exec(predicate) || []; result = !!parsed && new RegExp(parsed, flags).test(snapshot.name); @@ -68,8 +78,8 @@ function snapshotMatches(snapshot, include, exclude) { return result; } else if (predicate instanceof RegExp) { - // snapshot matches a regular expression - return predicate.test(snapshot.name); + // snapshot matches a regular expression (bounded input only) + return patternSafe && predicate.test(snapshot.name); } else if (typeof predicate === 'function') { // advanced matching return predicate(snapshot); From 8caf7a89533893edfe34a83e9cc12839c0fa9053 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Tue, 23 Jun 2026 15:17:02 +0530 Subject: [PATCH 2/3] fix(core): memoize secret patterns so redactSecrets does not blow timeouts redactSecrets re-read and re-parsed secretPatterns.yml (~1.7k regexes) and recompiled every RegExp on every recursive call. Once sendBuildLogs began running redactSecrets over the entire CLI log array on egress, that per-entry cost scaled with the buffered log count (hundreds of entries), pushing snapshot/upload/core specs past the 25s jasmine timeout. Compile the pattern list once and reuse it; redaction output is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/utils.js | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index dee1dea6f..5b683d3c4 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -553,10 +553,23 @@ export async function withRetries(fn, { count, onRetry, signal, throwOn }) { } } -export function redactSecrets(data) { - const filepath = path.resolve(url.fileURLToPath(import.meta.url), '../secretPatterns.yml'); - const secretPatterns = YAML.parse(readFileSync(filepath, 'utf-8')); +// Lazily load and compile the secret patterns once. The pattern file holds +// ~1.7k regexes; parsing the YAML and compiling every RegExp on each call made +// redactSecrets O(patterns) per string and re-read the file for every recursive +// call. Since redactSecrets now runs over the full CLI log array on egress +// (sendBuildLogs), that per-call cost is paid hundreds of times and could blow +// past test/runtime timeouts. Compile once and reuse. +let _compiledSecretPatterns; +function getSecretPatterns() { + if (!_compiledSecretPatterns) { + const filepath = path.resolve(url.fileURLToPath(import.meta.url), '../secretPatterns.yml'); + const secretPatterns = YAML.parse(readFileSync(filepath, 'utf-8')); + _compiledSecretPatterns = secretPatterns.patterns.map(p => new RegExp(p.pattern.regex, 'g')); + } + return _compiledSecretPatterns; +} +export function redactSecrets(data) { if (Array.isArray(data)) { // Process each item in the array return data.map(item => redactSecrets(item)); @@ -565,8 +578,8 @@ export function redactSecrets(data) { data.message = redactSecrets(data.message); } if (typeof data === 'string') { - for (const pattern of secretPatterns.patterns) { - data = data.replace(new RegExp(pattern.pattern.regex, 'g'), '[REDACTED]'); + for (const pattern of getSecretPatterns()) { + data = data.replace(pattern, '[REDACTED]'); } } return data; From 534cb410d13606aa028a7633f61c8a9e555023e8 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Tue, 23 Jun 2026 16:40:59 +0530 Subject: [PATCH 3/3] test(core): cover Chromium base URL validation branches Adds unit tests for resolveChromiumBaseUrl: default-host fallback, env default, trailing-slash normalization, and the warn-and-fallback paths for unparseable and non-HTTPS values, restoring 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/test/unit/install.test.js | 40 +++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/core/test/unit/install.test.js b/packages/core/test/unit/install.test.js index d27f15f0e..6c7fe0a84 100644 --- a/packages/core/test/unit/install.test.js +++ b/packages/core/test/unit/install.test.js @@ -291,6 +291,46 @@ describe('Unit / Install', () => { }); } }); + + describe('resolveChromiumBaseUrl', () => { + let defaultUrl = 'https://storage.googleapis.com/chromium-browser-snapshots/'; + + afterEach(() => { + delete process.env.PERCY_CHROMIUM_BASE_URL; + }); + + it('returns the default host when no base URL is set', () => { + delete process.env.PERCY_CHROMIUM_BASE_URL; + expect(install.resolveChromiumBaseUrl()).toEqual(defaultUrl); + }); + + it('reads the base URL from PERCY_CHROMIUM_BASE_URL by default', () => { + process.env.PERCY_CHROMIUM_BASE_URL = 'https://mirror.test.com/'; + expect(install.resolveChromiumBaseUrl()).toEqual('https://mirror.test.com/'); + }); + + it('appends a trailing slash to a valid HTTPS base URL', () => { + expect(install.resolveChromiumBaseUrl('https://mirror.test.com/chromium')) + .toEqual('https://mirror.test.com/chromium/'); + }); + + it('leaves an already slash-terminated base URL unchanged', () => { + expect(install.resolveChromiumBaseUrl('https://mirror.test.com/')) + .toEqual('https://mirror.test.com/'); + }); + + it('falls back to the default host for an unparseable URL', () => { + expect(install.resolveChromiumBaseUrl('not a valid url')).toEqual(defaultUrl); + expect(logger.stderr).toContain( + '[percy] Invalid PERCY_CHROMIUM_BASE_URL "not a valid url"; using the default Chromium download host.'); + }); + + it('rejects a non-HTTPS base URL and falls back to the default host', () => { + expect(install.resolveChromiumBaseUrl('http://mirror.test.com/')).toEqual(defaultUrl); + expect(logger.stderr).toContain( + '[percy] Ignoring non-HTTPS PERCY_CHROMIUM_BASE_URL "http://mirror.test.com/"; Chromium must be downloaded over HTTPS.'); + }); + }); }); describe('Unit / Install in executable', () => {