Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion packages/core/src/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 });
});
Expand Down
46 changes: 37 additions & 9 deletions packages/core/src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@
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:<port>). 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 {
Expand Down Expand Up @@ -218,19 +236,29 @@
#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);

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: javascript.express.security.cors-misconfiguration.cors-misconfiguration Warning

By letting user input control CORS parameters, there is a risk that software does not properly verify that the source of data or communication is valid. Use literal values for CORS settings.
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();
}
}
Expand Down
38 changes: 38 additions & 0 deletions packages/core/test/api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
36 changes: 31 additions & 5 deletions packages/core/test/unit/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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); });
Expand Down
Loading