From fec8de005eef23ffc1d894159cb4391c825d5a39 Mon Sep 17 00:00:00 2001 From: Ninad Sheth Date: Thu, 10 Sep 2026 11:02:44 +0000 Subject: [PATCH] fix(core): allowlist the /percy/log level so request input never drives dynamic dispatch (PER-8625) POST /percy/log used the request-supplied `level` as a property key on the logger group object (`log[level](message, meta)`). On an unauthenticated local endpoint that let any caller invoke arbitrary logger methods, e.g. `{"level":"loglevel","message":"debug"}` flipped the process-wide log level and `deprecated` emitted arbitrary warnings. This is the chain-breaker finding (PER-8606, CWE-1321) for the PER-8625 security chain. Validate `level` against debug/info/warn/error, respond 400 otherwise, and dispatch through an explicit switch so no request input ever reaches a dynamic property lookup. Co-Authored-By: Claude Fable 5.1 --- packages/core/src/api.js | 18 ++++++++++++++++- packages/core/test/api.test.js | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/core/src/api.js b/packages/core/src/api.js index ca92e8902..81ca491e2 100644 --- a/packages/core/src/api.js +++ b/packages/core/src/api.js @@ -97,6 +97,9 @@ function assertNotCrossOrigin(req) { } } +// Log levels an SDK may forward through POST /percy/log. +const SDK_LOG_LEVELS = new Set(['debug', 'info', 'warn', 'error']); + // Create a Percy CLI API server instance export function createPercyServer(percy, port) { let pkg = getPackageJSON(import.meta.url); @@ -319,7 +322,20 @@ export function createPercyServer(percy, port) { const message = req.body.message; const meta = req.body.meta || {}; - log[level](message, meta); + // `level` is attacker-controlled input on an unauthenticated endpoint, so + // it must never be used as a dynamic property key on the logger object + // (CWE-1321 / PER-8606, PER-8625). Only the four log levels are dispatched; + // anything else (`__proto__`, `constructor`, `loglevel`, ...) is rejected. + if (!SDK_LOG_LEVELS.has(level)) { + return res.json(400, { error: 'Invalid log level' }); + } + + switch (level) { + case 'debug': log.debug(message, meta); break; + case 'info': log.info(message, meta); break; + case 'warn': log.warn(message, meta); break; + default: log.error(message, meta); + } res.json(200, { success: true }); }) diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index 5b25f8911..40e7e20f0 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -896,6 +896,41 @@ describe('API Server', () => { expect(sdkLogs[1].meta).toEqual(message2.meta); }); + it('rejects /log requests whose level is not a real log level (PER-8606/8625)', async () => { + await percy.start(); + let before = logger.loglevel(); + + for (let level of ['__proto__', 'constructor', 'loglevel', 'deprecated', 'stdout', '', null, 42]) { + let [data, res] = await request('/percy/log', { + body: { level, message: 'debug', meta: {} }, + method: 'post' + }, true); + + expect(res.statusCode).toBe(400); + expect(data).toEqual({ error: 'Invalid log level' }); + } + + // no dynamic dispatch side effects: global loglevel untouched, nothing logged + expect(logger.loglevel()).toBe(before); + expect(logger.instance.query(log => log.debug === 'sdk').length).toBe(0); + expect(Object.prototype.token).toBeUndefined(); + }); + + it('dispatches every allowed /log level', async () => { + await percy.start(); + logger.loglevel('debug'); + + for (let level of ['debug', 'info', 'warn', 'error']) { + await expectAsync(request('/percy/log', { + body: { level, message: `${level} message` }, + method: 'post' + })).toBeResolvedTo({ success: true }); + } + + const sdkLogs = logger.instance.query(log => log.debug === 'sdk'); + expect(sdkLogs.map(l => l.level)).toEqual(['debug', 'info', 'warn', 'error']); + }); + it('returns a 500 error when an endpoint throws', async () => { spyOn(percy, 'snapshot').and.rejectWith(new Error('test error')); await percy.start();