security: redact CLI logs, bound regex matching (ReDoS) (PER-8609/8615)

packages/core/src/discovery.js

@@ -8,6 +8,7 @@ import {
   createRootResource,
   createPercyCSSResource,
   createLogResource,
+  redactSecrets,
   yieldAll,
   snapshotLogName,
   waitForTimeout,
@@ -218,8 +219,10 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) {
   // For multi dom root resources are stored as array
   resources = resources.flat();
 
-  // include associated snapshot logs matched by meta information
-  resources.push(createLogResource(logger.snapshotLogs(snapshot.meta.snapshot)));
+  // include associated snapshot logs matched by meta information. Redact
+  // secrets before egress — this per-snapshot log resource is a parallel
+  // egress path to sendBuildLogs and must scrub tokens/credentials too (CWE-532).
+  resources.push(createLogResource(redactSecrets(logger.snapshotLogs(snapshot.meta.snapshot))));
   logger.evictSnapshot(snapshot.meta.snapshot);
 
   if (process.env.PERCY_GZIP) {

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.

packages/core/src/secretPatterns.yml

@@ -7021,4 +7021,8 @@ patterns:
   - pattern:
       name: Bitcoin Address
       regex: '^[13][a-km-zA-HJ-NP-Z0-9]{26,33}$'
-      confidence: high
+      confidence: high
+  - pattern:
+      name: Percy Token
+      regex: '\b(web|app|auto|ss|vmw|res)_[A-Za-z0-9]{20,}'
+      confidence: high

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);

packages/core/src/utils.js

@@ -615,22 +615,50 @@
   }
 }
 
-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'));
+    // Regex sources come from the first-party, bundled secretPatterns.yml that
+    // ships in this package - never from remote or attacker-controlled input.
+    // nosemgrep: javascript.lang.security.audit.detect-non-literal-regexp.detect-non-literal-regexp
+    _compiledSecretPatterns = secretPatterns.patterns.map(p => new RegExp(p.pattern.regex, 'g'));
+  }
+  return _compiledSecretPatterns;
+}
 
-  if (Array.isArray(data)) {
-    // Process each item in the array
-    return data.map(item => redactSecrets(item));
-  } else if (typeof data === 'object' && data !== null) {
-    // Process each key-value pair in the object
-    data.message = redactSecrets(data.message);
-  }
+export function redactSecrets(data) {
+  // Strings are redacted against every compiled secret pattern.
   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;
+  }
+  // Arrays (the CLI-log entry list) are redacted element-wise, in place.
+  if (Array.isArray(data)) {
+    for (let i = 0; i < data.length; i++) data[i] = redactSecrets(data[i]);
+    return data;
+  }
+  // Log entries: redact the human-readable `message` in place and return the
+  // same reference (sendBuildLogs reads the return value; the CI-log path reads
+  // the same memory-mode entry back — both see the redacted message). We do NOT
+  // recurse over `meta`: it holds structured instrumentation (e.g. a numeric
+  // `size`, ids) and a broad secret pattern matches plain digit strings, so a
+  // blanket meta sweep clobbers legitimate diagnostic data. Log-line secrets
+  // surface in `message`.
+  if (typeof data === 'object' && data !== null) {
+    data.message = redactSecrets(data.message);
+    return data;
   }
+  // Any other primitive (number, boolean, null, undefined) passes through.
   return data;
 }
 

packages/core/test/unit/utils.test.js

@@ -218,6 +218,83 @@ describe('Unit / Utils', () => {
         expect(redactSecrets(msg)).toContain('[REDACTED]');
       });
     });
+
+    // No-false-positive cases: benign text must pass through unchanged, and the
+    // anchored Percy-token pattern must not fire on token-ish substrings.
+    describe('no false positives', () => {
+      it('leaves a plain URL with no secret unchanged', () => {
+        let url = 'https://percy.io/dashboard/builds';
+        expect(redactSecrets(url)).toEqual(url);
+      });
+
+      it('leaves a normal log message unchanged', () => {
+        let msg = 'Snapshot taken for homepage at width 1280';
+        expect(redactSecrets(msg)).toEqual(msg);
+      });
+
+      it('does not redact an access_ substring (ss_ leg must not fire mid-word)', () => {
+        let text = 'access_ABCDEFGHIJKLMNOPQRSTUV';
+        expect(redactSecrets(text)).toEqual(text);
+      });
+
+      it('does not clobber a crossapp_ substring at the app_ leg', () => {
+        let text = 'crossapp_ABCDEFGHIJKLMNOPQRSTUV';
+        expect(redactSecrets(text)).toEqual(text);
+      });
+    });
+
+    // Redaction is scoped to the entry's `message` and is applied in place.
+    describe('log-entry redaction', () => {
+      // Built by concatenation so the contiguous token literal never appears in
+      // source (a Percy-token-shaped literal would trip GitHub secret scanning);
+      // at runtime it still matches the pattern and must be redacted.
+      const secret = 'web_' + 'aB3dE7gH1jK4mN6pQ9sTuVwXyZ012345';
+
+      it('redacts the entry message in place and returns the same reference', () => {
+        // memory-mode logger.query returns live entry refs; the CI-log path
+        // reads the entry back after redaction, so redaction must mutate in
+        // place (not return a detached copy) as well as return the value.
+        let entry = { message: `token ${secret}`, type: 'ci' };
+        let redacted = redactSecrets(entry);
+        expect(redacted).toBe(entry);
+        expect(entry.message).toEqual('token [REDACTED]');
+      });
+
+      it('does not touch structured meta (avoids clobbering instrumentation data)', () => {
+        // `meta` holds diagnostic fields (e.g. a numeric size); a broad secret
+        // pattern matches plain digit strings, so meta is intentionally not swept.
+        let entry = { message: 'Resource too large', meta: { size: 30000000, url: 'http://localhost:8000/huge.css' } };
+        redactSecrets(entry);
+        expect(entry.meta.size).toBe(30000000);
+        expect(entry.meta.url).toBe('http://localhost:8000/huge.css');
+      });
+
+      it('redacts each message across an array of entries, in place', () => {
+        let entries = [{ message: `a ${secret}` }, { message: 'clean' }];
+        let out = redactSecrets(entries);
+        expect(out).toBe(entries);
+        expect(entries[0].message).toEqual('a [REDACTED]');
+        expect(entries[1].message).toEqual('clean');
+      });
+
+      it('passes null, undefined and non-string primitives through unchanged', () => {
+        expect(redactSecrets(null)).toBeNull();
+        expect(redactSecrets(undefined)).toBeUndefined();
+        expect(redactSecrets(42)).toBe(42);
+        expect(redactSecrets(true)).toBe(true);
+      });
+    });
+  });
+
+  describe('Percy token prefixes', () => {
+    for (const prefix of ['web', 'app', 'auto', 'ss', 'vmw', 'res']) {
+      it(`redacts a ${prefix}_ Percy token`, () => {
+        let token = `${prefix}_aB3dE7gH1jK4mN6pQ9sTuVwXyZ012345`;
+        let redacted = redactSecrets(`Authenticated build using ${token} now`);
+        expect(redacted).toContain('[REDACTED]');
+        expect(redacted).not.toContain(token);
+      });
+    }
   });
 
   describe('base64encode', () => {