From 67a794c753f2b07c88ad4042bdd9ebdfc7b29177 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Mon, 14 Mar 2022 13:13:56 -0500 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Introduce=20the=20ability=20to=20ab?= =?UTF-8?q?ort=20requests=20from=20specificed=20hostnames=20(`disallowedHo?= =?UTF-8?q?stnames`)=20(#823)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ✨ Introduce Disallow hostnames * 🏗 Fix typos in flag description & include in per-snapshot config --- packages/cli-command/src/flags.js | 10 +++ packages/cli-command/test/flags.test.js | 1 + packages/cli-exec/README.md | 2 + packages/cli-snapshot/README.md | 1 + packages/core/README.md | 3 +- packages/core/src/config.js | 15 ++++ packages/core/src/discovery.js | 5 +- packages/core/src/snapshot.js | 5 ++ packages/core/test/discovery.test.js | 91 +++++++++++++++++++++++++ 9 files changed, 131 insertions(+), 2 deletions(-) diff --git a/packages/cli-command/src/flags.js b/packages/cli-command/src/flags.js index 1012cf62d..7e86c6960 100644 --- a/packages/cli-command/src/flags.js +++ b/packages/cli-command/src/flags.js @@ -56,6 +56,15 @@ export const allowedHostnames = { short: 'h' }; +export const disallowedHostnames = { + name: 'disallowed-hostname', + description: 'Disallowed hostnames to abort in asset discovery', + percyrc: 'discovery.disallowedHostnames', + type: 'hostname', + multiple: true, + group: 'Percy' +}; + export const networkIdleTimeout = { name: 'network-idle-timeout', description: 'Asset discovery network idle timeout', @@ -95,6 +104,7 @@ export const PERCY = [ export const DISCOVERY = [ allowedHostnames, + disallowedHostnames, networkIdleTimeout, disableCache, debug diff --git a/packages/cli-command/test/flags.test.js b/packages/cli-command/test/flags.test.js index 9f5786af6..356439dd8 100644 --- a/packages/cli-command/test/flags.test.js +++ b/packages/cli-command/test/flags.test.js @@ -64,6 +64,7 @@ describe('Built-in flags:', () => { -c, --config Config file path -d, --dry-run Print snapshot names only -h, --allowed-hostname Allowed hostnames to capture in asset discovery + --disallowed-hostname Disallowed hostnames to abort in asset discovery -t, --network-idle-timeout Asset discovery network idle timeout --disable-cache Disable asset discovery caches --debug Debug asset discovery and do not upload snapshots diff --git a/packages/cli-exec/README.md b/packages/cli-exec/README.md index 9d588bb76..6f4d8ac36 100644 --- a/packages/cli-exec/README.md +++ b/packages/cli-exec/README.md @@ -32,6 +32,7 @@ Percy options: -c, --config Config file path -d, --dry-run Print snapshot names only -h, --allowed-hostname Allowed hostnames to capture in asset discovery + --disallowed-hostname Disallowed hostnames to abort in asset discovery -t, --network-idle-timeout Asset discovery network idle timeout --disable-cache Disable asset discovery caches --debug Debug asset discovery and do not upload snapshots @@ -62,6 +63,7 @@ Percy options: -c, --config Config file path -d, --dry-run Print snapshot names only -h, --allowed-hostname Allowed hostnames to capture in asset discovery + --disallowed-hostname Disallowed hostnames to abort in asset discovery -t, --network-idle-timeout Asset discovery network idle timeout --disable-cache Disable asset discovery caches --debug Debug asset discovery and do not upload snapshots diff --git a/packages/cli-snapshot/README.md b/packages/cli-snapshot/README.md index 1c1f739fe..f51f9f2e1 100644 --- a/packages/cli-snapshot/README.md +++ b/packages/cli-snapshot/README.md @@ -29,6 +29,7 @@ Percy options: -c, --config Config file path -d, --dry-run Print snapshot names only -h, --allowed-hostname Allowed hostnames to capture in asset discovery + --disallowed-hostname Disallowed hostnames to abort in asset discovery -t, --network-idle-timeout Asset discovery network idle timeout --disable-cache Disable asset discovery caches --debug Debug asset discovery and do not upload snapshots diff --git a/packages/core/README.md b/packages/core/README.md index ae212cf86..a02713c71 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -49,6 +49,7 @@ The following options can also be defined within a Percy config file - `enableJavaScript` — Enable JavaScript for screenshots (**default** `false`) - `discovery` — Asset discovery options - `allowedHostnames` — Array of allowed hostnames to capture assets from + - `disallowedHostnames` — Array of hostnames where requests will be aborted - `requestHeaders` — Request headers used when discovering snapshot assets - `authorization` — Basic auth `username` and `password` for protected snapshot assets - `disableCache` — Disable asset caching (**default** `false`) @@ -192,7 +193,7 @@ depending on the provided properties ([see alternate syntaxes below](#alternate- - `name` — Snapshot name - `domSnapshot` — Snapshot DOM string - `discovery` - Limited snapshot specific discovery options - - `allowedHostnames`, `requestHeaders`, `authorization`, `disableCache`, `userAgent` + - `allowedHostnames`, `disallowedHostnames`, `requestHeaders`, `authorization`, `disableCache`, `userAgent` Common snapshot options are also accepted and will override instance snapshot options. [See instance options](#options) for common snapshot and discovery options. diff --git a/packages/core/src/config.js b/packages/core/src/config.js index fc0471038..c3af9aa77 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -46,6 +46,20 @@ export const configSchema = { }] } }, + disallowedHostnames: { + type: 'array', + default: [], + items: { + type: 'string', + allOf: [{ + not: { pattern: '[^/]/' }, + error: 'must not include a pathname' + }, { + not: { pattern: '^([a-zA-Z]+:)?//' }, + error: 'must not include a protocol' + }] + } + }, networkIdleTimeout: { type: 'integer', default: 100, @@ -124,6 +138,7 @@ export const snapshotSchema = { additionalProperties: false, properties: { allowedHostnames: { $ref: '/config/discovery#/properties/allowedHostnames' }, + disallowedHostnames: { $ref: '/config/discovery#/properties/disallowedHostnames' }, requestHeaders: { $ref: '/config/discovery#/properties/requestHeaders' }, authorization: { $ref: '/config/discovery#/properties/authorization' }, disableCache: { $ref: '/config/discovery#/properties/disableCache' }, diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 9d2874366..cb73a8fb0 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -5,7 +5,7 @@ const MAX_RESOURCE_SIZE = 15 * (1024 ** 2); // 15MB const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308]; const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', 'Other']; -export function createRequestHandler(network, { disableCache, getResource }) { +export function createRequestHandler(network, { disableCache, disallowedHostnames, getResource }) { let log = logger('core:discovery'); return async request => { @@ -19,6 +19,9 @@ export function createRequestHandler(network, { disableCache, getResource }) { if (resource?.root) { log.debug('- Serving root resource', meta); await request.respond(resource); + } else if (hostnameMatches(disallowedHostnames, url)) { + log.debug('- Skipping disallowed hostname', meta); + await request.abort(true); } else if (resource && !disableCache) { log.debug('- Resource cache hit', meta); await request.respond(resource); diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index 863c5e559..289374f64 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -179,6 +179,7 @@ export function getSnapshotConfig(percy, options) { // only specific discovery options are used per-snapshot discovery: { allowedHostnames: percy.config.discovery.allowedHostnames, + disallowedHostnames: percy.config.discovery.disallowedHostnames, networkIdleTimeout: percy.config.discovery.networkIdleTimeout, requestHeaders: percy.config.discovery.requestHeaders, authorization: percy.config.discovery.authorization, @@ -194,6 +195,8 @@ export function getSnapshotConfig(percy, options) { case 'execute': // shorthand for execute.beforeSnapshot return (Array.isArray(next) || typeof next !== 'object') ? [path.concat('beforeSnapshot'), next] : [path]; + case 'discovery.disallowedHostnames': // prevent disallowing the root hostname + return [path, (prev ?? []).concat(next).filter(h => !hostnameMatches(h, options.url))]; } // ensure additional snapshots have complete names @@ -238,6 +241,7 @@ function debugSnapshotConfig(snapshot, showInfo) { debugProp(snapshot, 'execute.afterResize'); debugProp(snapshot, 'execute.beforeSnapshot'); debugProp(snapshot, 'discovery.allowedHostnames'); + debugProp(snapshot, 'discovery.disallowedHostnames'); debugProp(snapshot, 'discovery.requestHeaders', JSON.stringify); debugProp(snapshot, 'discovery.authorization', JSON.stringify); debugProp(snapshot, 'discovery.disableCache'); @@ -331,6 +335,7 @@ export async function* discoverSnapshotResources(percy, snapshot, callback) { enableJavaScript: snapshot.enableJavaScript, disableCache: snapshot.discovery.disableCache, allowedHostnames: snapshot.discovery.allowedHostnames, + disallowedHostnames: snapshot.discovery.disallowedHostnames, getResource: u => resources.get(u) || cache.get(u), saveResource: r => resources.set(r.url, r) && cache.set(r.url, r) } diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 88be1284d..8b503c988 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1035,6 +1035,97 @@ describe('Discovery', () => { ); }); + it('does not capture resources from disallowed hostnames', async () => { + // stop current instance to create a new one + await percy.stop(); + + percy = await Percy.start({ + token: 'PERCY_TOKEN', + discovery: { + disallowedHostnames: ['ex.localhost'] + } + }); + + logger.loglevel('debug'); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testExternalDOM, + widths: [1000] + }); + + await percy.idle(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy:core:snapshot] - discovery.disallowedHostnames: ex.localhost' + ])); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy:core:discovery] Handling request: http://ex.localhost:8001/img.gif', + '[percy:core:discovery] - Skipping disallowed hostname' + ])); + + expect(captured[0]).toEqual([ + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': jasmine.stringMatching(/^\/percy\.\d+\.log$/) + }) + }), + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/' + }) + }), + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/style.css' + }) + }) + ]); + }); + + it('ignores root resource URL when passed to disallowedHostnames', async () => { + // stop current instance to create a new one + await percy.stop(); + + percy = await Percy.start({ + token: 'PERCY_TOKEN', + discovery: { + disallowedHostnames: ['localhost'] + } + }); + + logger.loglevel('debug'); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testExternalDOM, + widths: [1000] + }); + + await percy.idle(); + + expect(captured[0]).toEqual([ + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': jasmine.stringMatching(/^\/percy\.\d+\.log$/) + }) + }), + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/' + }) + }), + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/style.css' + }) + }) + ]); + }); + it('does not hang waiting for embedded isolated pages', async () => { server.reply('/', () => [200, { 'Content-Type': 'text/html',