From e50cf1c692091be6a876824ad64df60bec5a6124 Mon Sep 17 00:00:00 2001 From: Josh Faigan Date: Tue, 17 Mar 2026 10:10:35 -0400 Subject: [PATCH 1/3] create allowlist for localhost variants and validate host on requests --- .changeset/few-maps-melt.md | 5 + .../theme-environment.test.ts | 190 ++++++++++++------ .../theme-environment/theme-environment.ts | 55 ++++- 3 files changed, 182 insertions(+), 68 deletions(-) create mode 100644 .changeset/few-maps-melt.md diff --git a/.changeset/few-maps-melt.md b/.changeset/few-maps-melt.md new file mode 100644 index 00000000000..e5d2098547c --- /dev/null +++ b/.changeset/few-maps-melt.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Set localhost variants and run host header validations to prevent DNS rebinding vulnerability diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts index ff76c8b24af..816429604de 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts @@ -42,6 +42,46 @@ beforeEach(() => { }) describe('setupDevServer', () => { + const decoder = new TextDecoder() + + const createH3Event = (options: {url: string; headers?: Record}) => { + const req = new IncomingMessage(new Socket()) + req.url = options.url + if (options.headers) req.headers = options.headers + const res = new ServerResponse(req) + return createEvent(req, res) + } + + const dispatchEvent = async ( + server: ReturnType, + url: string, + headers?: Record, + ): Promise<{res: ServerResponse; status: number; body: string | Buffer}> => { + const event = createH3Event({url, headers}) + const {res} = event.node + let body = '' + const resWrite = res.write.bind(res) + res.write = (chunk) => { + body ??= '' + body += decoder.decode(chunk) + return resWrite(chunk) + } + const resEnd = res.end.bind(res) + res.end = (content) => { + if (!body) body = content ?? '' + return resEnd(content) + } + + await server.dispatchEvent(event) + + if (!body && '_data' in res) { + // eslint-disable-next-line require-atomic-updates + body = await new Response(res._data as ReadableStream).text() + } + + return {res, status: res.statusCode, body} + } + const developmentTheme = buildTheme({id: 1, name: 'Theme', role: DEVELOPMENT_THEME_ROLE})! const localFiles = new Map([ ['templates/asset.json', {checksum: '1', key: 'templates/asset.json'}], @@ -206,80 +246,82 @@ describe('setupDevServer', () => { await expect(backgroundJobPromise).rejects.toThrow('Failed to fetch checksums from API') }) - describe('request handling', async () => { + describe('DNS rebinding protection', () => { const context = {...defaultServerContext} const server = setupDevServer(developmentTheme, context) - const html = String.raw - const decoder = new TextDecoder() - - const createH3Event = (options: {url: string; headers?: Record}) => { - const req = new IncomingMessage(new Socket()) - req.url = options.url - if (options.headers) req.headers = options.headers - const res = new ServerResponse(req) - return createEvent(req, res) - } - - const dispatchEvent = async ( - url: string, - headers?: Record, - ): Promise<{res: ServerResponse; status: number; body: string | Buffer}> => { - const event = createH3Event({url, headers}) - const {res} = event.node - let body = '' - const resWrite = res.write.bind(res) - res.write = (chunk) => { - body ??= '' - body += decoder.decode(chunk) - return resWrite(chunk) - } - const resEnd = res.end.bind(res) - res.end = (content) => { - if (!body) body = content ?? '' - return resEnd(content) - } + test.each([ + ['localhost:9292', 'localhost variant'], + ['127.0.0.1:9292', 'IPv4 loopback'], + ['[::1]:9292', 'IPv6 loopback'], + ['LOCALHOST:9292', 'case insensitive'], + ['localhost.:9292', 'trailing dot with port'], + ])('accepts %s (%s)', async (host) => { + const response = await dispatchEvent(server, '/wpm@something', {host}) + expect(response.status).not.toBe(400) + expect(response.status).toBe(204) + }) - await server.dispatchEvent(event) + test.each([ + ['attacker.com:9292', 'attacker domain'], + ['poc.mzero.cloud:9292', 'DNS rebinding domain'], + ['localhost:1234', 'wrong port'], + ])('rejects %s (%s)', async (host) => { + const response = await dispatchEvent(server, '/', {host}) + expect(response.status).toBe(400) + }) - if (!body && '_data' in res) { - // When returning a Response from H3, we get the body here: - // eslint-disable-next-line require-atomic-updates - body = await new Response(res._data as ReadableStream).text() + test('accepts requests when --host flag is uppercase (LOCALHOST)', async () => { + const uppercaseHostContext = { + ...defaultServerContext, + options: {...defaultServerContext.options, host: 'LOCALHOST'}, } + const uppercaseServer = setupDevServer(developmentTheme, uppercaseHostContext) + const response = await dispatchEvent(uppercaseServer, '/wpm@something', {host: 'localhost:9292'}) + expect(response.status).not.toBe(400) + expect(response.status).toBe(204) + }) + }) - return {res, status: res.statusCode, body} - } + describe('request handling', async () => { + const context = {...defaultServerContext} + const server = setupDevServer(developmentTheme, context) + + const html = String.raw + const defaultHost = `${context.options.host}:${context.options.port}` test('mocks known endpoints', async () => { - await expect(dispatchEvent('/wpm@something')).resolves.toHaveProperty('status', 204) - await expect(dispatchEvent('/.well-known/shopify/monorail')).resolves.toHaveProperty('status', 204) + await expect(dispatchEvent(server, '/wpm@something', {host: defaultHost})).resolves.toHaveProperty('status', 204) + await expect(dispatchEvent(server, '/.well-known/shopify/monorail', {host: defaultHost})).resolves.toHaveProperty( + 'status', + 204, + ) expect(vi.mocked(render)).not.toHaveBeenCalled() }) test('CORS allows the theme editor (Online Store Editor) origin so hot reload works in the editor', async () => { - const {res} = await dispatchEvent('/assets/file2.css', {origin: 'https://online-store-web.shopifyapps.com'}) + const {res} = await dispatchEvent(server, '/assets/file2.css', {host: defaultHost, origin: 'https://online-store-web.shopifyapps.com'}) expect(res.getHeader('access-control-allow-origin')).toEqual('https://online-store-web.shopifyapps.com') }) test('CORS allows the storefront origin', async () => { const origin = `https://${defaultServerContext.session.storeFqdn}` - const {res} = await dispatchEvent('/assets/file2.css', {origin}) + const {res} = await dispatchEvent(server, '/assets/file2.css', {host: defaultHost, origin}) expect(res.getHeader('access-control-allow-origin')).toEqual(origin) }) test('CORS does not allow unknown origins', async () => { - const {res} = await dispatchEvent('/assets/file2.css', {origin: 'https://evil.example.com'}) + const {res} = await dispatchEvent(server, '/assets/file2.css', {host: defaultHost, origin: 'https://evil.example.com'}) expect(res.getHeader('access-control-allow-origin')).toBeUndefined() }) test('CORS does not set headers on direct navigation without an Origin header', async () => { - const {res} = await dispatchEvent('/assets/file2.css') + const {res} = await dispatchEvent(server, '/assets/file2.css', {host: defaultHost}) expect(res.getHeader('access-control-allow-origin')).toBeUndefined() }) test('serves proxied local assets', async () => { - const eventPromise = dispatchEvent('/cdn/somepathhere/assets/file1.css') + const eventPromise = dispatchEvent(server, '/cdn/somepathhere/assets/file1.css', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() expect(vi.mocked(render)).not.toHaveBeenCalled() @@ -294,7 +336,7 @@ describe('setupDevServer', () => { test('serves local assets from the root in a backward compatible way', async () => { // Also serves assets from the root, similar to what the old server did: - const eventPromise = dispatchEvent('/assets/file2.css') + const eventPromise = dispatchEvent(server, '/assets/file2.css', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() expect(vi.mocked(render)).not.toHaveBeenCalled() @@ -305,7 +347,7 @@ describe('setupDevServer', () => { }) test('handles non-breaking spaces in files correctly', async () => { - const eventPromise = dispatchEvent('/assets/file-with-nbsp.js') + const eventPromise = dispatchEvent(server, '/assets/file-with-nbsp.js', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() const {res, body} = await eventPromise @@ -393,7 +435,7 @@ describe('setupDevServer', () => { localThemeFileSystem.files.set(snippetFile.key, snippetFile) // Request the compiled CSS - const response = await dispatchEvent('/compiled_assets/styles.css') + const response = await dispatchEvent(server, '/compiled_assets/styles.css', {host: defaultHost}) // Just verify the content-type is set correctly expect(response.res.getHeader('content-type')).toEqual('text/css') @@ -441,7 +483,7 @@ describe('setupDevServer', () => { localThemeFileSystem.files.set(blockFile2.key, blockFile2) localThemeFileSystem.files.set(blockFile3.key, blockFile3) - const eventPromise = dispatchEvent('/cdn/somepath/compiled_assets/block-scripts.js') + const eventPromise = dispatchEvent(server, '/cdn/somepath/compiled_assets/block-scripts.js', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() const {res, body} = await eventPromise @@ -526,7 +568,9 @@ describe('setupDevServer', () => { localThemeFileSystem.files.set(snippetFile2.key, snippetFile2) localThemeFileSystem.files.set(snippetFile3.key, snippetFile3) - const eventPromise = dispatchEvent('/cdn/somepath/compiled_assets/snippet-scripts.js') + const eventPromise = dispatchEvent(server, '/cdn/somepath/compiled_assets/snippet-scripts.js', { + host: defaultHost, + }) await expect(eventPromise).resolves.not.toThrow() const {res, body} = await eventPromise @@ -610,7 +654,7 @@ describe('setupDevServer', () => { localThemeFileSystem.files.set(sectionFile1.key, sectionFile1) localThemeFileSystem.files.set(sectionFile2.key, sectionFile2) localThemeFileSystem.files.set(sectionFile3.key, sectionFile3) - const eventPromise = dispatchEvent('/cdn/somepath/compiled_assets/scripts.js') + const eventPromise = dispatchEvent(server, '/cdn/somepath/compiled_assets/scripts.js', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() const {res, body} = await eventPromise @@ -676,7 +720,9 @@ describe('setupDevServer', () => { localThemeFileSystem.files.set(snippetFile.key, snippetFile) - const {res, body} = await dispatchEvent('/cdn/somepath/compiled_assets/snippet-scripts.js') + const {res, body} = await dispatchEvent(server, '/cdn/somepath/compiled_assets/snippet-scripts.js', { + host: defaultHost, + }) const bodyString = body.toString() const bodyByteLength = Buffer.byteLength(bodyString) @@ -692,7 +738,7 @@ describe('setupDevServer', () => { vi.stubGlobal('fetch', fetchStub) // Request a compiled asset that doesn't exist - await dispatchEvent('/compiled_assets/nonexistent.js') + await dispatchEvent(server, '/compiled_assets/nonexistent.js', {host: defaultHost}) // Should fall back to proxy expect(fetchStub).toHaveBeenCalledOnce() @@ -722,7 +768,7 @@ describe('setupDevServer', () => { ), ) - const eventPromise = dispatchEvent('/', {accept: 'text/html'}) + const eventPromise = dispatchEvent(server, '/', {host: defaultHost, accept: 'text/html'}) await expect(eventPromise).resolves.not.toThrow() const {res, body} = await eventPromise @@ -742,7 +788,7 @@ describe('setupDevServer', () => { vi.stubGlobal('fetch', fetchStub) // --- Unknown endpoint: - const eventPromise = dispatchEvent('/path/to/something-else.js') + const eventPromise = dispatchEvent(server, '/path/to/something-else.js', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() expect(vi.mocked(render)).not.toHaveBeenCalled() @@ -768,7 +814,9 @@ describe('setupDevServer', () => { // --- Unknown assets: fetchStub.mockClear() - await expect(dispatchEvent('/cdn/somepathhere/assets/file42.css')).resolves.not.toThrow() + await expect( + dispatchEvent(server, '/cdn/somepathhere/assets/file42.css', {host: defaultHost}), + ).resolves.not.toThrow() expect(fetchStub).toHaveBeenCalledOnce() expect(fetchStub).toHaveBeenLastCalledWith( new URL( @@ -800,7 +848,7 @@ describe('setupDevServer', () => { vi.stubGlobal('fetch', fetchStub) - const eventPromise = dispatchEvent('/cdn/shop/t/img/assets/file3.css') + const eventPromise = dispatchEvent(server, '/cdn/shop/t/img/assets/file3.css', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() expect(vi.mocked(render)).not.toHaveBeenCalled() @@ -815,7 +863,7 @@ describe('setupDevServer', () => { const now = Date.now() const pathname = '/cdn/shop/t/img/assets/file4.js' - const eventPromise = dispatchEvent(`${pathname}?1234`) + const eventPromise = dispatchEvent(server, `${pathname}?1234`, {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() expect(vi.mocked(render)).not.toHaveBeenCalled() @@ -831,7 +879,7 @@ describe('setupDevServer', () => { fetchStub.mockResolvedValueOnce(new Response(null, {status: 302})) vi.mocked(render).mockResolvedValueOnce(new Response(null, {status: 401})) - const eventPromise = dispatchEvent('/non-renderable-path') + const eventPromise = dispatchEvent(server, '/non-renderable-path', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() expect(vi.mocked(render)).toHaveBeenCalled() @@ -858,7 +906,7 @@ describe('setupDevServer', () => { fetchStub.mockResolvedValueOnce(new Response(null, {status: 404})) vi.mocked(render).mockResolvedValueOnce(new Response(null, {status: 401})) - const eventPromise = dispatchEvent('/non-renderable-path') + const eventPromise = dispatchEvent(server, '/non-renderable-path', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() expect(vi.mocked(render)).toHaveBeenCalled() @@ -885,14 +933,22 @@ describe('setupDevServer', () => { fetchStub.mockResolvedValueOnce(new Response(null, {status: 200})) vi.mocked(render).mockResolvedValue(new Response(null, {status: 404})) - await expect(dispatchEvent('/non-renderable-path?sections=xyz')).resolves.toHaveProperty('status', 404) - await expect(dispatchEvent('/non-renderable-path?section_id=xyz')).resolves.toHaveProperty('status', 404) - await expect(dispatchEvent('/non-renderable-path?app_block_id=xyz')).resolves.toHaveProperty('status', 404) + await expect( + dispatchEvent(server, '/non-renderable-path?sections=xyz', {host: defaultHost}), + ).resolves.toHaveProperty('status', 404) + await expect( + dispatchEvent(server, '/non-renderable-path?section_id=xyz', {host: defaultHost}), + ).resolves.toHaveProperty('status', 404) + await expect( + dispatchEvent(server, '/non-renderable-path?app_block_id=xyz', {host: defaultHost}), + ).resolves.toHaveProperty('status', 404) expect(vi.mocked(render)).toHaveBeenCalledTimes(3) expect(fetchStub).not.toHaveBeenCalled() - await expect(dispatchEvent('/non-renderable-path?unknown=xyz')).resolves.toHaveProperty('status', 200) + await expect( + dispatchEvent(server, '/non-renderable-path?unknown=xyz', {host: defaultHost}), + ).resolves.toHaveProperty('status', 200) expect(fetchStub).toHaveBeenCalledOnce() }) @@ -908,7 +964,7 @@ describe('setupDevServer', () => { vi.stubGlobal('fetch', fetchStub) // When - const event = createH3Event({url: '/compiled_assets/styles.css'}) + const event = createH3Event({url: '/compiled_assets/styles.css', headers: {host: defaultHost}}) await themeExtServer.dispatchEvent(event) // Then @@ -929,7 +985,7 @@ describe('setupDevServer', () => { fetchStub.mockClear() // When requesting the same compiled asset with theme context - const themeEvent = createH3Event({url: '/compiled_assets/styles.css'}) + const themeEvent = createH3Event({url: '/compiled_assets/styles.css', headers: {host: defaultHost}}) await server.dispatchEvent(themeEvent) // Then it should handle it locally, not proxy @@ -941,7 +997,7 @@ describe('setupDevServer', () => { vi.stubGlobal('fetch', fetchStub) vi.mocked(render).mockRejectedValueOnce(new Error('Network error')) - const eventPromise = dispatchEvent('/') + const eventPromise = dispatchEvent(server, '/', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() expect(vi.mocked(render)).toHaveBeenCalled() @@ -956,7 +1012,7 @@ describe('setupDevServer', () => { vi.stubGlobal('fetch', fetchStub) localThemeFileSystem.uploadErrors.set('templates/asset.json', ['Error 1', 'Error 2']) - const eventPromise = dispatchEvent('/') + const eventPromise = dispatchEvent(server, '/', {host: defaultHost}) await expect(eventPromise).resolves.not.toThrow() expect(vi.mocked(render)).not.toHaveBeenCalled() diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts index 422fec583c7..d0465545adb 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts @@ -8,7 +8,15 @@ import {renderTasksToStdErr} from '../theme-ui.js' import {renderThrownError} from '../errors.js' import {promiseWithResolvers} from '../../polyfills/promiseWithResolvers.js' -import {createApp, defineEventHandler, defineLazyEventHandler, toNodeListener, handleCors} from 'h3' +import { + createApp, + defineEventHandler, + defineLazyEventHandler, + toNodeListener, + handleCors, + sendError, + createError, +} from 'h3' import {fetchChecksums} from '@shopify/cli-kit/node/themes/api' import {createServer} from 'node:http' @@ -128,6 +136,31 @@ interface DevelopmentServerInstance { close: () => Promise } +function createAllowedHostsSet(host: string, port: string): Set { + const allowedHosts = new Set() + const portSuffix = `:${port}` + const normalizedHost = host.toLowerCase().trim() + + allowedHosts.add(`${normalizedHost}${portSuffix}`) + + // When binding to localhost variants or 0.0.0.0, allow all localhost forms + const localhostVariants = ['localhost', '127.0.0.1', '::1', '0.0.0.0'] + if (localhostVariants.includes(normalizedHost)) { + allowedHosts.add(`localhost${portSuffix}`) + allowedHosts.add(`127.0.0.1${portSuffix}`) + allowedHosts.add(`[::1]${portSuffix}`) + } + + return allowedHosts +} + +function normalizeHostHeader(hostHeader: string | undefined): string | undefined { + if (!hostHeader) return undefined + // Lowercase, then strip trailing dot before port (or at end for plain hostname). + // IPv6 brackets: trailing dot would be after `]`, e.g. [::1].:9292 + return hostHeader.toLowerCase().replace(/\.(?=:\d|$)/, '') +} + function createDevelopmentServer(theme: Theme, ctx: DevServerContext, initialWork: Promise) { const app = createApp() const allowedOrigins = [ @@ -136,6 +169,26 @@ function createDevelopmentServer(theme: Theme, ctx: DevServerContext, initialWor // Required for HMR with the theme editor 'https://online-store-web.shopifyapps.com', ] + const allowedHosts = createAllowedHostsSet(ctx.options.host, ctx.options.port) + + // Host header validation + app.use( + defineEventHandler((event) => { + const hostHeader = event.node.req.headers.host + const normalizedHost = normalizeHostHeader(hostHeader) + + if (!normalizedHost || !allowedHosts.has(normalizedHost)) { + return sendError( + event, + createError({ + statusCode: 400, + statusMessage: 'Bad Request', + message: 'Invalid Host header', + }), + ) + } + }), + ) app.use( defineLazyEventHandler(async () => { From a1984a960010558e49a1b4a26293bf0dc1c44928 Mon Sep 17 00:00:00 2001 From: Josh Faigan Date: Thu, 4 Jun 2026 16:31:19 -0400 Subject: [PATCH 2/3] fix(theme): validate host header for app extension dev server --- .../theme-environment/host-validation.ts | 54 ++++++++ .../theme-environment/theme-environment.ts | 56 +------- .../theme-ext-server.test.ts | 122 ++++++++++++++++++ .../theme-ext-environment/theme-ext-server.ts | 2 + 4 files changed, 181 insertions(+), 53 deletions(-) create mode 100644 packages/theme/src/cli/utilities/theme-environment/host-validation.ts create mode 100644 packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.test.ts diff --git a/packages/theme/src/cli/utilities/theme-environment/host-validation.ts b/packages/theme/src/cli/utilities/theme-environment/host-validation.ts new file mode 100644 index 00000000000..5d1349dba9a --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-environment/host-validation.ts @@ -0,0 +1,54 @@ +import {createError, defineEventHandler, sendError} from 'h3' + +function createAllowedHostsSet(host: string, port: number): Set { + const allowedHosts = new Set() + const portSuffix = `:${port}` + const normalizedHost = host.toLowerCase().trim() + + allowedHosts.add(`${normalizedHost}${portSuffix}`) + + // When binding to localhost variants or 0.0.0.0, allow all localhost forms + const localhostVariants = ['localhost', '127.0.0.1', '::1', '0.0.0.0'] + if (localhostVariants.includes(normalizedHost)) { + allowedHosts.add(`localhost${portSuffix}`) + allowedHosts.add(`127.0.0.1${portSuffix}`) + allowedHosts.add(`[::1]${portSuffix}`) + } + + return allowedHosts +} + +function normalizeHostHeader(hostHeader: string | undefined): string | undefined { + if (!hostHeader) return undefined + // Lowercase, then strip trailing dot before port (or at end for plain hostname). + // IPv6 brackets: trailing dot would be after `]`, e.g. [::1].:9292 + return hostHeader.toLowerCase().replace(/\.(?=:\d|$)/, '') +} + +/** + * Creates an h3 event handler that validates the request's Host header + * against an allowlist of configured host/port and localhost variants. + * + * Used to mitigate DNS rebinding attacks on local dev servers. + * + * Returns a 400 Bad Request when the Host header is missing or not in the allowlist. + */ +export function createHostValidationHandler(host: string, port: number) { + const allowedHosts = createAllowedHostsSet(host, port) + + return defineEventHandler((event) => { + const hostHeader = event.node.req.headers.host + const normalizedHost = normalizeHostHeader(hostHeader) + + if (!normalizedHost || !allowedHosts.has(normalizedHost)) { + return sendError( + event, + createError({ + statusCode: 400, + statusMessage: 'Bad Request', + message: 'Invalid Host header', + }), + ) + } + }) +} diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts index d0465545adb..5e9e37ac15e 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts @@ -3,20 +3,13 @@ import {getHtmlHandler} from './html.js' import {getAssetsHandler} from './local-assets.js' import {getProxyHandler} from './proxy.js' import {reconcileAndPollThemeEditorChanges} from './remote-theme-watcher.js' +import {createHostValidationHandler} from './host-validation.js' import {uploadTheme} from '../theme-uploader.js' import {renderTasksToStdErr} from '../theme-ui.js' import {renderThrownError} from '../errors.js' import {promiseWithResolvers} from '../../polyfills/promiseWithResolvers.js' -import { - createApp, - defineEventHandler, - defineLazyEventHandler, - toNodeListener, - handleCors, - sendError, - createError, -} from 'h3' +import {createApp, defineEventHandler, defineLazyEventHandler, toNodeListener, handleCors} from 'h3' import {fetchChecksums} from '@shopify/cli-kit/node/themes/api' import {createServer} from 'node:http' @@ -136,31 +129,6 @@ interface DevelopmentServerInstance { close: () => Promise } -function createAllowedHostsSet(host: string, port: string): Set { - const allowedHosts = new Set() - const portSuffix = `:${port}` - const normalizedHost = host.toLowerCase().trim() - - allowedHosts.add(`${normalizedHost}${portSuffix}`) - - // When binding to localhost variants or 0.0.0.0, allow all localhost forms - const localhostVariants = ['localhost', '127.0.0.1', '::1', '0.0.0.0'] - if (localhostVariants.includes(normalizedHost)) { - allowedHosts.add(`localhost${portSuffix}`) - allowedHosts.add(`127.0.0.1${portSuffix}`) - allowedHosts.add(`[::1]${portSuffix}`) - } - - return allowedHosts -} - -function normalizeHostHeader(hostHeader: string | undefined): string | undefined { - if (!hostHeader) return undefined - // Lowercase, then strip trailing dot before port (or at end for plain hostname). - // IPv6 brackets: trailing dot would be after `]`, e.g. [::1].:9292 - return hostHeader.toLowerCase().replace(/\.(?=:\d|$)/, '') -} - function createDevelopmentServer(theme: Theme, ctx: DevServerContext, initialWork: Promise) { const app = createApp() const allowedOrigins = [ @@ -169,26 +137,8 @@ function createDevelopmentServer(theme: Theme, ctx: DevServerContext, initialWor // Required for HMR with the theme editor 'https://online-store-web.shopifyapps.com', ] - const allowedHosts = createAllowedHostsSet(ctx.options.host, ctx.options.port) - // Host header validation - app.use( - defineEventHandler((event) => { - const hostHeader = event.node.req.headers.host - const normalizedHost = normalizeHostHeader(hostHeader) - - if (!normalizedHost || !allowedHosts.has(normalizedHost)) { - return sendError( - event, - createError({ - statusCode: 400, - statusMessage: 'Bad Request', - message: 'Invalid Host header', - }), - ) - } - }), - ) + app.use(createHostValidationHandler(ctx.options.host, ctx.options.port)) app.use( defineLazyEventHandler(async () => { diff --git a/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.test.ts b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.test.ts new file mode 100644 index 00000000000..e365662567d --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.test.ts @@ -0,0 +1,122 @@ +import {createDevelopmentExtensionServer} from './theme-ext-server.js' +import {DevServerContext} from '../theme-environment/types.js' +import {emptyThemeExtFileSystem, emptyThemeFileSystem} from '../theme-fs-empty.js' + +import {DEVELOPMENT_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils' +import {buildTheme} from '@shopify/cli-kit/node/themes/factories' +import {describe, expect, test} from 'vitest' +import {createEvent} from 'h3' + +import {IncomingMessage, ServerResponse} from 'node:http' +import {Socket} from 'node:net' + +describe('createDevelopmentExtensionServer', () => { + const decoder = new TextDecoder() + + const createH3Event = (options: {url: string; headers?: Record}) => { + const req = new IncomingMessage(new Socket()) + req.url = options.url + if (options.headers) req.headers = options.headers + const res = new ServerResponse(req) + return createEvent(req, res) + } + + const dispatchEvent = async ( + server: ReturnType, + url: string, + headers?: Record, + ): Promise<{res: ServerResponse; status: number; body: string | Buffer}> => { + const event = createH3Event({url, headers}) + const {res} = event.node + let body = '' + const resWrite = res.write.bind(res) + res.write = (chunk) => { + body ??= '' + body += decoder.decode(chunk) + return resWrite(chunk) + } + const resEnd = res.end.bind(res) + res.end = (content) => { + if (!body) body = content ?? '' + return resEnd(content) + } + + await server.dispatch(event) + + if (!body && '_data' in res) { + // eslint-disable-next-line require-atomic-updates + body = await new Response(res._data as ReadableStream).text() + } + + return {res, status: res.statusCode, body} + } + + const developmentTheme = buildTheme({id: 1, name: 'Theme', role: DEVELOPMENT_THEME_ROLE})! + + const defaultServerContext: DevServerContext = { + session: { + storefrontToken: 'shptka_test_token_123', + token: '', + storeFqdn: 'my-store.myshopify.com', + sessionCookies: {_shopify_essential: 'test-cookie-value'}, + }, + lastRequestedPath: '', + localThemeFileSystem: emptyThemeFileSystem(), + localThemeExtensionFileSystem: emptyThemeExtFileSystem(), + directory: 'tmp', + type: 'theme-extension', + options: { + ignore: [], + only: [], + noDelete: false, + host: '127.0.0.1', + port: 9293, + liveReload: 'hot-reload', + open: false, + themeEditorSync: false, + errorOverlay: 'default', + }, + } + + describe('DNS rebinding protection', () => { + const context = {...defaultServerContext} + const server = createDevelopmentExtensionServer(developmentTheme, context) + + test.each([ + ['localhost:9293', 'localhost variant'], + ['127.0.0.1:9293', 'IPv4 loopback'], + ['[::1]:9293', 'IPv6 loopback'], + ['LOCALHOST:9293', 'case insensitive'], + ['localhost.:9293', 'trailing dot with port'], + ])('accepts %s (%s)', async (host) => { + const response = await dispatchEvent(server, '/wpm@something', {host}) + expect(response.status).not.toBe(400) + expect(response.status).toBe(204) + }) + + test.each([ + ['attacker.com:9293', 'attacker domain'], + ['poc.mzero.cloud:9293', 'DNS rebinding domain'], + ['localhost:1234', 'wrong port'], + ])('rejects %s (%s)', async (host) => { + const response = await dispatchEvent(server, '/', {host}) + expect(response.status).toBe(400) + }) + + test('rejects requests with missing Host header', async () => { + const response = await dispatchEvent(server, '/') + expect(response.status).toBe(400) + }) + + test('accepts requests when --host flag is uppercase (LOCALHOST)', async () => { + const uppercaseHostContext = { + ...defaultServerContext, + options: {...defaultServerContext.options, host: 'LOCALHOST'}, + } + const uppercaseServer = createDevelopmentExtensionServer(developmentTheme, uppercaseHostContext) + const response = await dispatchEvent(uppercaseServer, '/wpm@something', {host: 'localhost:9293'}) + expect(response.status).not.toBe(400) + expect(response.status).toBe(204) + }) + }) +}) diff --git a/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.ts b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.ts index b305e32b5c5..55477a74de7 100644 --- a/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.ts +++ b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.ts @@ -4,6 +4,7 @@ import {getHtmlHandler} from '../theme-environment/html.js' import {getAssetsHandler} from '../theme-environment/local-assets.js' import {getProxyHandler} from '../theme-environment/proxy.js' import {getHotReloadHandler, triggerHotReload} from '../theme-environment/hot-reload/server.js' +import {createHostValidationHandler} from '../theme-environment/host-validation.js' import {emptyThemeFileSystem} from '../theme-fs-empty.js' import {initializeDevServerSession} from '../theme-environment/dev-server-session.js' import {createApp, toNodeListener} from 'h3' @@ -36,6 +37,7 @@ export async function initializeDevelopmentExtensionServer(theme: Theme, devExt: export function createDevelopmentExtensionServer(theme: Theme, ctx: DevServerContext) { const app = createApp() + app.use(createHostValidationHandler(ctx.options.host, ctx.options.port)) app.use(getHotReloadHandler(theme, ctx)) app.use(getAssetsHandler(theme, ctx)) app.use(getProxyHandler(theme, ctx)) From 74d7fc0f2b260125200183cdb6434566300aca81 Mon Sep 17 00:00:00 2001 From: Josh Faigan Date: Fri, 5 Jun 2026 12:07:39 -0400 Subject: [PATCH 3/3] fix(theme): allow LAN access when theme dev binds to a wildcard host --- .../theme-environment/host-validation.test.ts | 95 +++++++++++++++++++ .../theme-environment/host-validation.ts | 31 +++--- .../theme-environment.test.ts | 10 +- 3 files changed, 118 insertions(+), 18 deletions(-) create mode 100644 packages/theme/src/cli/utilities/theme-environment/host-validation.test.ts diff --git a/packages/theme/src/cli/utilities/theme-environment/host-validation.test.ts b/packages/theme/src/cli/utilities/theme-environment/host-validation.test.ts new file mode 100644 index 00000000000..6b0db7448c7 --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-environment/host-validation.test.ts @@ -0,0 +1,95 @@ +import {createHostValidationHandler} from './host-validation.js' + +import {beforeEach, describe, expect, test, vi} from 'vitest' +import {createEvent} from 'h3' + +import {networkInterfaces} from 'node:os' +import {IncomingMessage, ServerResponse} from 'node:http' +import {Socket} from 'node:net' + +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal() + return {...actual, networkInterfaces: vi.fn()} +}) + +const dispatchHost = async ( + handler: ReturnType, + host?: string, +): Promise => { + const req = new IncomingMessage(new Socket()) + req.url = '/' + if (host !== undefined) req.headers = {host} + const res = new ServerResponse(req) + const event = createEvent(req, res) + + await handler(event) + + return res.statusCode +} + +describe('createHostValidationHandler', () => { + describe('loopback bind', () => { + const handler = createHostValidationHandler('127.0.0.1', 9292) + + test.each([['localhost:9292'], ['127.0.0.1:9292'], ['[::1]:9292'], ['LOCALHOST:9292'], ['localhost.:9292']])( + 'accepts %s', + async (host) => { + const status = await dispatchHost(handler, host) + expect(status).not.toBe(400) + }, + ) + + test.each([['attacker.com:9292'], ['localhost:1234'], ['127.0.0.1']])('rejects %s', async (host) => { + const status = await dispatchHost(handler, host) + expect(status).toBe(400) + }) + + test('rejects missing host header', async () => { + const status = await dispatchHost(handler) + expect(status).toBe(400) + }) + }) + + describe('wildcard bind', () => { + beforeEach(() => { + vi.mocked(networkInterfaces).mockReturnValue({ + en0: [ + { + address: '192.168.1.50', + netmask: '255.255.255.0', + family: 'IPv4', + mac: '00:00:00:00:00:00', + internal: false, + cidr: '192.168.1.50/24', + }, + ], + } as ReturnType) + }) + + const buildHandler = () => createHostValidationHandler('0.0.0.0', 9292) + + test.each([['192.168.1.50:9292'], ['127.0.0.1:9292'], ['[::1]:9292']])('accepts %s', async (host) => { + const status = await dispatchHost(buildHandler(), host) + expect(status).not.toBe(400) + }) + + test.each([['192.168.1.50:1234'], ['attacker.com:9292']])('rejects %s', async (host) => { + const status = await dispatchHost(buildHandler(), host) + expect(status).toBe(400) + }) + }) + + describe('non-wildcard LAN bind', () => { + const handler = createHostValidationHandler('192.168.1.50', 9292) + + test.each([['192.168.1.50:9292']])('accepts %s', async (host) => { + const status = await dispatchHost(handler, host) + expect(status).not.toBe(400) + }) + + test.each([['192.168.1.99:9292']])('rejects %s', async (host) => { + const status = await dispatchHost(handler, host) + expect(status).toBe(400) + }) + }) +}) diff --git a/packages/theme/src/cli/utilities/theme-environment/host-validation.ts b/packages/theme/src/cli/utilities/theme-environment/host-validation.ts index 5d1349dba9a..e9b4ce04b32 100644 --- a/packages/theme/src/cli/utilities/theme-environment/host-validation.ts +++ b/packages/theme/src/cli/utilities/theme-environment/host-validation.ts @@ -1,4 +1,5 @@ import {createError, defineEventHandler, sendError} from 'h3' +import * as os from 'node:os' function createAllowedHostsSet(host: string, port: number): Set { const allowedHosts = new Set() @@ -7,7 +8,6 @@ function createAllowedHostsSet(host: string, port: number): Set { allowedHosts.add(`${normalizedHost}${portSuffix}`) - // When binding to localhost variants or 0.0.0.0, allow all localhost forms const localhostVariants = ['localhost', '127.0.0.1', '::1', '0.0.0.0'] if (localhostVariants.includes(normalizedHost)) { allowedHosts.add(`localhost${portSuffix}`) @@ -15,24 +15,27 @@ function createAllowedHostsSet(host: string, port: number): Set { allowedHosts.add(`[::1]${portSuffix}`) } + // When binding to a wildcard address, the server is also reachable through the machine's + // interface IPs (e.g. a LAN address like 192.168.x.x from another device on the network). + if (normalizedHost === '0.0.0.0' || normalizedHost === '::') { + for (const interfaces of Object.values(os.networkInterfaces())) { + for (const iface of interfaces ?? []) { + const address = iface.address.toLowerCase().split('%')[0] + const isIPv6 = iface.family === 'IPv6' + const formatted = isIPv6 ? `[${address}]` : address + allowedHosts.add(`${formatted}${portSuffix}`) + } + } + } + return allowedHosts } function normalizeHostHeader(hostHeader: string | undefined): string | undefined { if (!hostHeader) return undefined - // Lowercase, then strip trailing dot before port (or at end for plain hostname). - // IPv6 brackets: trailing dot would be after `]`, e.g. [::1].:9292 return hostHeader.toLowerCase().replace(/\.(?=:\d|$)/, '') } -/** - * Creates an h3 event handler that validates the request's Host header - * against an allowlist of configured host/port and localhost variants. - * - * Used to mitigate DNS rebinding attacks on local dev servers. - * - * Returns a 400 Bad Request when the Host header is missing or not in the allowlist. - */ export function createHostValidationHandler(host: string, port: number) { const allowedHosts = createAllowedHostsSet(host, port) @@ -43,11 +46,7 @@ export function createHostValidationHandler(host: string, port: number) { if (!normalizedHost || !allowedHosts.has(normalizedHost)) { return sendError( event, - createError({ - statusCode: 400, - statusMessage: 'Bad Request', - message: 'Invalid Host header', - }), + createError({statusCode: 400, statusMessage: 'Bad Request', message: 'Invalid Host header'}), ) } }) diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts index 816429604de..0175c759eb7 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts @@ -300,7 +300,10 @@ describe('setupDevServer', () => { }) test('CORS allows the theme editor (Online Store Editor) origin so hot reload works in the editor', async () => { - const {res} = await dispatchEvent(server, '/assets/file2.css', {host: defaultHost, origin: 'https://online-store-web.shopifyapps.com'}) + const {res} = await dispatchEvent(server, '/assets/file2.css', { + host: defaultHost, + origin: 'https://online-store-web.shopifyapps.com', + }) expect(res.getHeader('access-control-allow-origin')).toEqual('https://online-store-web.shopifyapps.com') }) @@ -311,7 +314,10 @@ describe('setupDevServer', () => { }) test('CORS does not allow unknown origins', async () => { - const {res} = await dispatchEvent(server, '/assets/file2.css', {host: defaultHost, origin: 'https://evil.example.com'}) + const {res} = await dispatchEvent(server, '/assets/file2.css', { + host: defaultHost, + origin: 'https://evil.example.com', + }) expect(res.getHeader('access-control-allow-origin')).toBeUndefined() })