fix(core): block security-sensitive launchOptions over HTTP API (PER-…

packages/core/src/api.js

@@ -16,6 +16,7 @@ import { Readable } from 'stream';
 // This change ensures better compatibility and avoids relying on Node.js-specific APIs that might cause issues in ESM environments.
 import { fileURLToPath } from 'url';
 import { createRequire } from 'module';
+import { configSchema } from './config.js';
 
 export const getPercyDomPath = (url) => {
   try {
@@ -41,6 +42,55 @@ function encodeURLSearchParams(subj, prefix) {
   )).join('&') : `${prefix}=${encodeURIComponent(subj)}`;
 }
 
+// Walks the config schema and collects dot-paths of any fields marked `httpReadOnly: true`
+// that are present in `body`. Driving this from the schema means new HTTP-blocked fields
+// only need a one-line annotation next to their definition — no list to keep in sync here.
+function findHttpReadOnlyPaths(body, schema, path = '') {
+  if (!body || typeof body !== 'object' || !schema?.properties) return [];
+  let paths = [];
+  for (let [key, propSchema] of Object.entries(schema.properties)) {
+    if (!Object.prototype.hasOwnProperty.call(body, key)) continue;
+    let childPath = path ? `${path}.${key}` : key;
+    if (propSchema?.httpReadOnly) {
+      paths.push(childPath);
+    } else {
+      paths.push(...findHttpReadOnlyPaths(body[key], propSchema, childPath));
+    }
+  }
+  return paths;
+}
+
+// Top-level configSchema is a map of subschemas keyed by top-level config namespace
+// (`discovery`, `snapshot`, …). Wrap it as a single object schema so the walker can recurse
+// uniformly from the root.
+const ROOT_CONFIG_SCHEMA = { type: 'object', properties: configSchema };
+
+// Removes each dot-path's leaf from a deep clone of `body` and logs a warning per path.
+// Returns the original `body` unchanged when `paths` is empty so we don't pay for a clone
+// on every config request. Exported for unit testing: the `?.` chain in the reduce is a
+// defensive guard for paths whose ancestor is absent from `body`. Through the production
+// caller (stripBlockedConfigFields → findHttpReadOnlyPaths) every intermediate is verified
+// present, so the guard is unreachable in normal use — but the explicit paths parameter
+// lets a unit test exercise it without contorting the schema.
+export function _applyHttpReadOnlyStripping(body, paths, log) {
+  if (!paths.length) return body;
+
+  let stripped = JSON.parse(JSON.stringify(body));
+  for (let p of paths) {
+    let parts = p.split('.');
+    let leaf = parts.pop();
+    let parent = parts.reduce((o, k) => o?.[k], stripped);
+    if (parent && typeof parent === 'object') delete parent[leaf];
+    log.warn(`Ignoring \`${p}\` from /percy/config request: this field can only be set via the config file or CLI at startup.`);
+  }
+  return stripped;
+}
+
+// Returns a body with `httpReadOnly` fields removed. Caller guarantees `body` is truthy.
+function stripBlockedConfigFields(body, log) {
+  return _applyHttpReadOnlyStripping(body, findHttpReadOnlyPaths(body, ROOT_CONFIG_SCHEMA), log);
+}
+
 // Parse PNG IHDR chunk for the screenshot's actual rendered dimensions.
 // Returns { width, height } when the buffer is a valid PNG with non-zero
 // dimensions, or null otherwise (non-PNG signature, truncated file, zero
@@ -293,10 +343,13 @@ export function createPercyServer(percy, port) {
       });
     })
   // get or set config options
-    .route(['get', 'post'], '/percy/config', async (req, res) => res.json(200, {
-      config: req.body ? percy.set(req.body) : percy.config,
-      success: true
-    }))
+    .route(['get', 'post'], '/percy/config', async (req, res) => {
+      let body = req.body && stripBlockedConfigFields(req.body, logger('core:server'));
+      return res.json(200, {
+        config: body ? percy.set(body) : percy.config,
+        success: true
+      });
+    })
   // responds once idle (may take a long time)
     .route('get', '/percy/idle', async (req, res) => res.json(200, {
       success: await percy.idle().then(() => true)

packages/core/src/config.js

@@ -489,9 +489,12 @@ export const configSchema = {
         type: 'object',
         additionalProperties: false,
         properties: {
-          executable: { type: 'string' },
+          // httpReadOnly: never settable over /percy/config — accepting these would let any
+          // local process execute arbitrary code as the Percy user via flags like
+          // --renderer-cmd-prefix / --gpu-launcher. Must be set via static config or CLI.
+          executable: { type: 'string', httpReadOnly: true },
           timeout: { type: 'integer' },
-          args: { type: 'array', items: { type: 'string' } },
+          args: { type: 'array', items: { type: 'string' }, httpReadOnly: true },
           headless: { type: 'boolean' },
           closeBrowser: { type: 'boolean', default: true }
         }

packages/core/test/api.test.js

@@ -3,7 +3,7 @@ import PercyConfig from '@percy/config';
 import { logger, setupTest, fs } from './helpers/index.js';
 import Percy from '@percy/core';
 import WebdriverUtils from '@percy/webdriver-utils';
-import { getPercyDomPath } from '../src/api.js';
+import { getPercyDomPath, _applyHttpReadOnlyStripping } from '../src/api.js';
 
 describe('API Server', () => {
   let percy;
@@ -113,6 +113,55 @@ describe('API Server', () => {
     expect(percy.config).toEqual(expected);
   });
 
+  it('does not warn when /config POST contains launchOptions without blocked keys', async () => {
+    await percy.start();
+
+    await request('/percy/config', {
+      method: 'POST',
+      body: { discovery: { launchOptions: { headless: false, closeBrowser: false } } }
+    });
+
+    expect(percy.config.discovery.launchOptions.headless).toBe(false);
+    expect(percy.config.discovery.launchOptions.closeBrowser).toBe(false);
+    expect(logger.stderr).not.toEqual(jasmine.arrayContaining([
+      jasmine.stringMatching(/Ignoring `discovery\.launchOptions/)
+    ]));
+  });
+
+  it('strips security-sensitive launchOptions fields from /config POST', async () => {
+    await percy.start();
+
+    let before = percy.config.discovery.launchOptions;
+    expect(before.executable).toBeUndefined();
+    expect(before.args).toBeUndefined();
+
+    await request('/percy/config', {
+      method: 'POST',
+      body: {
+        discovery: {
+          launchOptions: {
+            executable: '/tmp/evil-binary',
+            args: ['--renderer-cmd-prefix=/tmp/payload'],
+            headless: false,
+            closeBrowser: false
+          }
+        }
+      }
+    });
+
+    // dangerous fields ignored
+    expect(percy.config.discovery.launchOptions.executable).toBeUndefined();
+    expect(percy.config.discovery.launchOptions.args).toBeUndefined();
+    // benign fields still settable
+    expect(percy.config.discovery.launchOptions.headless).toBe(false);
+    expect(percy.config.discovery.launchOptions.closeBrowser).toBe(false);
+
+    expect(logger.stderr).toEqual(jasmine.arrayContaining([
+      jasmine.stringMatching(/Ignoring `discovery\.launchOptions\.executable`/),
+      jasmine.stringMatching(/Ignoring `discovery\.launchOptions\.args`/)
+    ]));
+  });
+
   it('has an /idle endpoint that calls #idle()', async () => {
     spyOn(percy, 'idle').and.resolveTo();
     await percy.start();
@@ -1749,3 +1798,37 @@ describe('API Server', () => {
     });
   });
 });
+
+// Pure unit tests for the stripping helper — kept in their own describe so they don't
+// drag the API Server's beforeEach (Percy instance + Chromium setup). Through the
+// production caller every intermediate of a returned path is verified present, so the
+// `o?.[k]` defensive guard inside _applyHttpReadOnlyStripping is unreachable in normal
+// use. These tests pin that guard directly so a refactor can't silently lose it.
+describe('_applyHttpReadOnlyStripping', () => {
+  it('tolerates paths whose ancestor is absent from body', () => {
+    let log = { warn: jasmine.createSpy('warn') };
+    let body = { unrelated: { keep: true } };
+    let result = _applyHttpReadOnlyStripping(
+      body,
+      ['discovery.launchOptions.executable'],
+      log
+    );
+
+    // Body is deep-cloned (not mutated); the missing path is a no-op delete.
+    expect(result).toEqual({ unrelated: { keep: true } });
+    expect(result).not.toBe(body);
+    // The warning still fires — caller is told the field was rejected.
+    expect(log.warn).toHaveBeenCalledWith(
+      jasmine.stringMatching(/Ignoring `discovery\.launchOptions\.executable`/)
+    );
+  });
+
+  it('returns body unchanged (no clone) when paths is empty', () => {
+    let log = { warn: jasmine.createSpy('warn') };
+    let body = { discovery: { launchOptions: { headless: true } } };
+    let result = _applyHttpReadOnlyStripping(body, [], log);
+
+    expect(result).toBe(body);
+    expect(log.warn).not.toHaveBeenCalled();
+  });
+});