diff --git a/packages/core/src/api.js b/packages/core/src/api.js index 6a4ec27e6..5e3372a88 100644 --- a/packages/core/src/api.js +++ b/packages/core/src/api.js @@ -3,7 +3,7 @@ import path, { dirname, resolve } from 'path'; import logger from '@percy/logger'; import { normalize } from '@percy/config/utils'; import { getPackageJSON, Server, percyAutomateRequestHandler, percyBuildEventHandler, computeResponsiveWidths } from './utils.js'; -import { ServerError } from './server.js'; +import { ServerError, isLoopbackOrigin } from './server.js'; import WebdriverUtils from '@percy/webdriver-utils'; import { handleSyncJob } from './snapshot.js'; import { dump as maestroDump, firstMatch as maestroFirstMatch, SELECTOR_KEYS_WHITELIST, getMaestroHierarchyDrift } from './maestro-hierarchy.js'; @@ -91,6 +91,19 @@ function stripBlockedConfigFields(body, log) { return _applyHttpReadOnlyStripping(body, findHttpReadOnlyPaths(body, ROOT_CONFIG_SCHEMA), log); } +// Reject state-changing requests that carry a cross-origin (non-loopback) Origin +// header. The local server is unauthenticated by design (SDKs post to it), so +// this blocks a malicious website the user is visiting from driving sensitive +// endpoints — live config mutation or stopping the build — via the browser +// (CWE-352 / CWE-306, PER-8600/8601). Node SDK clients send no Origin header and +// are unaffected; loopback browser tooling (any localhost port) is allowed. +function assertNotCrossOrigin(req) { + let origin = req.headers.origin; + if (origin && !isLoopbackOrigin(origin)) { + throw new ServerError(403, 'Cross-origin requests are not allowed on this endpoint'); + } +} + // 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 @@ -344,6 +357,8 @@ export function createPercyServer(percy, port) { }) // get or set config options .route(['get', 'post'], '/percy/config', async (req, res) => { + // mutating the live config is only allowed from same-host callers + if (req.body) assertNotCrossOrigin(req); let body = req.body && stripBlockedConfigFields(req.body, logger('core:server')); return res.json(200, { config: body ? percy.set(body) : percy.config, @@ -905,6 +920,7 @@ export function createPercyServer(percy, port) { }) // stops percy at the end of the current event loop .route('/percy/stop', (req, res) => { + assertNotCrossOrigin(req); setImmediate(() => percy.stop()); return res.json(200, { success: true }); }); diff --git a/packages/core/src/server.js b/packages/core/src/server.js index 65d8e88cb..45e45bbdb 100644 --- a/packages/core/src/server.js +++ b/packages/core/src/server.js @@ -9,6 +9,24 @@ import { compile as makeToPath } from 'path-to-regexp'; +// Returns true when an Origin header value resolves to a loopback host. The +// local Percy server is only ever legitimately reached by Node SDK clients +// (which send no Origin) or by loopback browser tooling (e.g. the Storybook +// manager on localhost:). Treating non-loopback origins as untrusted lets +// us scope CORS and reject cross-origin state changes (CWE-942 / CWE-352). +export function isLoopbackOrigin(origin) { + if (!origin || typeof origin !== 'string') return false; + try { + // URL.hostname strips the brackets from an IPv6 literal, so `[::1]` + // normalises to `::1` here — no need to match the bracketed form. + let host = new URL(origin).hostname.toLowerCase(); + return host === 'localhost' || host === '127.0.0.1' || + host === '::1' || host.endsWith('.localhost'); + } catch { + return false; + } +} + // custom incoming message adds a `url` and `body` properties containing the parsed URL and message // buffer respectively; both available after the 'end' event is emitted export class IncomingMessage extends http.IncomingMessage { @@ -218,19 +236,29 @@ export class Server extends http.Server { #routes = [{ priority: -1, handle: (req, res, next) => { - res.setHeader('Access-Control-Allow-Origin', '*'); + // Only echo a loopback Origin back as Access-Control-Allow-Origin. The + // previous wildcard `*` let any website the user visited read responses + // from the local Percy server (build data, config) cross-origin (CWE-942). + let origin = req.headers.origin; + let originAllowed = isLoopbackOrigin(origin); + if (originAllowed) { + res.setHeader('Access-Control-Allow-Origin', origin); + res.setHeader('Vary', 'Origin'); + } if (req.method === 'OPTIONS') { - let allowHeaders = req.headers['access-control-request-headers'] || '*'; - let allowMethods = [...new Set(this.#routes.flatMap(route => ( - (!route.match || route.match(req.url.pathname)) && route.methods - ) || []))].join(', '); - - res.setHeader('Access-Control-Allow-Headers', allowHeaders); - res.setHeader('Access-Control-Allow-Methods', allowMethods); + if (originAllowed) { + let allowHeaders = req.headers['access-control-request-headers'] || '*'; + let allowMethods = [...new Set(this.#routes.flatMap(route => ( + (!route.match || route.match(req.url.pathname)) && route.methods + ) || []))].join(', '); + + res.setHeader('Access-Control-Allow-Headers', allowHeaders); + res.setHeader('Access-Control-Allow-Methods', allowMethods); + } res.writeHead(204).end(); } else { - res.setHeader('Access-Control-Expose-Headers', '*'); + if (originAllowed) res.setHeader('Access-Control-Expose-Headers', '*'); return next(); } } diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index e22a4c2c7..a5c45dcd7 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -162,6 +162,44 @@ describe('API Server', () => { ])); }); + it('rejects /config POST carrying a cross-origin Origin header (PER-8601)', async () => { + await percy.start(); + let before = percy.config; + + await expectAsync(request('/percy/config', { + method: 'POST', + body: { snapshot: { widths: [1234] } }, + headers: { Origin: 'https://evil.example' } + })).toBeRejected(); + + // live config was not mutated by the cross-origin request + expect(percy.config).toEqual(before); + }); + + it('allows /config POST from a loopback origin', async () => { + await percy.start(); + + await expectAsync(request('/percy/config', { + method: 'POST', + body: { snapshot: { widths: [1000] } }, + headers: { Origin: 'http://localhost:6006' } + })).toBeResolved(); + + expect(percy.config.snapshot.widths).toEqual([1000]); + }); + + it('rejects /stop carrying a cross-origin Origin header (PER-8600)', async () => { + await percy.start(); + let stopSpy = spyOn(percy, 'stop').and.resolveTo(); + + await expectAsync(request('/percy/stop', { + method: 'POST', + headers: { Origin: 'https://evil.example' } + })).toBeRejected(); + + expect(stopSpy).not.toHaveBeenCalled(); + }); + it('has an /idle endpoint that calls #idle()', async () => { spyOn(percy, 'idle').and.resolveTo(); await percy.start(); diff --git a/packages/core/test/unit/server.test.js b/packages/core/test/unit/server.test.js index f0393c3a2..60651ad1b 100644 --- a/packages/core/test/unit/server.test.js +++ b/packages/core/test/unit/server.test.js @@ -1,5 +1,5 @@ import { fs, mockfs } from '../helpers/index.js'; -import Server from '../../src/server.js'; +import Server, { isLoopbackOrigin } from '../../src/server.js'; describe('Unit / Server', () => { let server; @@ -241,28 +241,54 @@ describe('Unit / Server', () => { }); }); - it('handles CORS preflight requests', async () => { + it('handles CORS preflight requests for loopback origins', async () => { server.route(['get', 'post'], '/1', (req, res) => res.send(200)); server.route(['put', 'delete'], '/2', (req, res) => res.text(200)); - let res1 = await request('/1', 'OPTIONS', false); + // CORS is scoped to loopback origins (CWE-942): Access-Control-Allow-Origin + // is the echoed loopback origin, not a wildcard `*`. + let origin = server.address(); // http://localhost:8000 + + let res1 = await request('/1', { + method: 'OPTIONS', + headers: { Origin: origin } + }, false); expect(res1.statusCode).toBe(204); - expect(res1.headers).toHaveProperty('access-control-allow-origin', '*'); + expect(res1.headers).toHaveProperty('access-control-allow-origin', origin); expect(res1.headers).toHaveProperty('access-control-allow-headers', '*'); expect(res1.headers).toHaveProperty('access-control-allow-methods', 'GET, POST'); let res2 = await request('/2', { method: 'OPTIONS', - headers: { 'Access-Control-Request-Headers': 'Content-Type' } + headers: { Origin: origin, 'Access-Control-Request-Headers': 'Content-Type' } }, false); expect(res2.statusCode).toBe(204); - expect(res2.headers).toHaveProperty('access-control-allow-origin', '*'); + expect(res2.headers).toHaveProperty('access-control-allow-origin', origin); expect(res2.headers).toHaveProperty('access-control-allow-headers', 'Content-Type'); expect(res2.headers).toHaveProperty('access-control-allow-methods', 'PUT, DELETE'); }); + it('omits CORS headers for missing or non-loopback origins', async () => { + server.route(['get', 'post'], '/1', (req, res) => res.send(200)); + + // No Origin header (e.g. a Node SDK client) — still a valid 204 preflight, + // but no CORS headers are emitted. + let res1 = await request('/1', 'OPTIONS', false); + expect(res1.statusCode).toBe(204); + expect(res1.headers).not.toHaveProperty('access-control-allow-origin'); + expect(res1.headers).not.toHaveProperty('access-control-allow-methods'); + + // Cross-origin website must not receive permissive CORS headers. + let res2 = await request('/1', { + method: 'OPTIONS', + headers: { Origin: 'https://evil.example.com' } + }, false); + expect(res2.statusCode).toBe(204); + expect(res2.headers).not.toHaveProperty('access-control-allow-origin'); + }); + it('handles server errors', async () => { server.route('/e/foo', () => { throw new Error('foo'); }); server.route('/e/bar', () => { throw new Server.Error(418); }); @@ -290,6 +316,25 @@ describe('Unit / Server', () => { }); }); + describe('isLoopbackOrigin', () => { + it('returns true for loopback origins', () => { + expect(isLoopbackOrigin('http://localhost:8000')).toBe(true); + expect(isLoopbackOrigin('http://127.0.0.1')).toBe(true); + expect(isLoopbackOrigin('http://app.localhost')).toBe(true); + }); + + it('returns false for non-loopback and missing origins', () => { + expect(isLoopbackOrigin('https://evil.example.com')).toBe(false); + expect(isLoopbackOrigin('')).toBe(false); + expect(isLoopbackOrigin(undefined)).toBe(false); + expect(isLoopbackOrigin(42)).toBe(false); + }); + + it('returns false for a malformed origin that cannot be parsed as a URL', () => { + expect(isLoopbackOrigin('not a valid origin')).toBe(false); + }); + }); + describe('#serve([pathname], directory[, options])', () => { beforeEach(async () => { await server.listen();