From 49e0e4cf512af82894fdd8acb9d99d0737591bd3 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 16 Jan 2025 17:28:02 +0900 Subject: [PATCH 01/13] fix: validate websocket api request fix: shouldHandle cannot be async wip: copy isHostAllowed chore: move code chore: comment wip: allow only same origin chore: cleanup wip: isApiRequestAllowed chore: cleanup wip: switch to token check chore: unused fix: inject VITEST_API_TOKEN in orchestrator ui docs: add api.allowedHosts fix: require token for __vitest_browser_api__ request chore: cleanup refactor: combine to isWebsocketRequestAllowed --- docs/guide/cli-generated.md | 11 ++ packages/browser/src/client/client.ts | 2 +- .../src/client/public/esm-client-injector.js | 1 + packages/browser/src/node/rpc.ts | 7 +- .../browser/src/node/serverOrchestrator.ts | 1 + packages/browser/src/node/serverTester.ts | 1 + packages/ui/client/constants.ts | 2 +- packages/ui/node/index.ts | 21 +++ packages/vitest/src/api/hostCheck.ts | 148 ++++++++++++++++++ packages/vitest/src/api/setup.ts | 9 +- packages/vitest/src/node/cli/cli-config.ts | 1 + .../vitest/src/node/config/resolveConfig.ts | 4 +- packages/vitest/src/node/types/config.ts | 6 +- packages/vitest/src/public/node.ts | 1 + 14 files changed, 208 insertions(+), 7 deletions(-) create mode 100644 packages/vitest/src/api/hostCheck.ts diff --git a/docs/guide/cli-generated.md b/docs/guide/cli-generated.md index 3a2b078a3e92..68dc2ec5f89f 100644 --- a/docs/guide/cli-generated.md +++ b/docs/guide/cli-generated.md @@ -71,6 +71,17 @@ Specify which IP addresses the server should listen on. Set this to `0.0.0.0` or Set to true to exit if port is already in use, instead of automatically trying the next available port +### api.allowedHosts + +- **Type:** `string[] | true` +- **Default:** `[]` + +The hostnames that Vitest API is allowed to respond to. `localhost` and domains under `.localhost` and all IP addresses are allowed by default. When using HTTPS, this check is skipped. + +If a string starts with `.`, it will allow that hostname without the `.` and all subdomains under the hostname. For example, `.example.com` will allow `example.com`, `foo.example.com`, and `foo.bar.example.com`. + +If set to `true`, the server is allowed to respond to requests for any hosts. This is not recommended as it will be vulnerable to DNS rebinding attacks. + ### silent - **CLI:** `--silent` diff --git a/packages/browser/src/client/client.ts b/packages/browser/src/client/client.ts index 34fff8aea11b..8091874082b4 100644 --- a/packages/browser/src/client/client.ts +++ b/packages/browser/src/client/client.ts @@ -15,7 +15,7 @@ export const RPC_ID const METHOD = getBrowserState().method export const ENTRY_URL = `${ location.protocol === 'https:' ? 'wss:' : 'ws:' -}//${HOST}/__vitest_browser_api__?type=${PAGE_TYPE}&rpcId=${RPC_ID}&sessionId=${getBrowserState().sessionId}&projectName=${getBrowserState().config.name || ''}&method=${METHOD}` +}//${HOST}/__vitest_browser_api__?type=${PAGE_TYPE}&rpcId=${RPC_ID}&sessionId=${getBrowserState().sessionId}&projectName=${getBrowserState().config.name || ''}&method=${METHOD}&token=${(window as any).VITEST_API_TOKEN}` let setCancel = (_: CancelReason) => {} export const onCancel = new Promise((resolve) => { diff --git a/packages/browser/src/client/public/esm-client-injector.js b/packages/browser/src/client/public/esm-client-injector.js index 23d931e2165b..394baa032a4e 100644 --- a/packages/browser/src/client/public/esm-client-injector.js +++ b/packages/browser/src/client/public/esm-client-injector.js @@ -30,6 +30,7 @@ method: { __VITEST_METHOD__ }, providedContext: { __VITEST_PROVIDED_CONTEXT__ }, }; + window.VITEST_API_TOKEN = { __VITEST_API_TOKEN__ }; const config = __vitest_browser_runner__.config; diff --git a/packages/browser/src/node/rpc.ts b/packages/browser/src/node/rpc.ts index 75cf0f779d94..ca7cf48990ee 100644 --- a/packages/browser/src/node/rpc.ts +++ b/packages/browser/src/node/rpc.ts @@ -10,7 +10,7 @@ import { ServerMockResolver } from '@vitest/mocker/node' import { createBirpc } from 'birpc' import { parse, stringify } from 'flatted' import { dirname } from 'pathe' -import { createDebugger, isFileServingAllowed } from 'vitest/node' +import { createDebugger, isFileServingAllowed, isWebsocketRequestAllowed } from 'vitest/node' import { WebSocketServer } from 'ws' const debug = createDebugger('vitest:browser:api') @@ -33,6 +33,11 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject) { return } + if (!isWebsocketRequestAllowed(vitest.config, vite.config, request)) { + socket.destroy() + return + } + const type = searchParams.get('type') const rpcId = searchParams.get('rpcId') const sessionId = searchParams.get('sessionId') diff --git a/packages/browser/src/node/serverOrchestrator.ts b/packages/browser/src/node/serverOrchestrator.ts index b853e9386e21..d265675864bf 100644 --- a/packages/browser/src/node/serverOrchestrator.ts +++ b/packages/browser/src/node/serverOrchestrator.ts @@ -42,6 +42,7 @@ export async function resolveOrchestrator( __VITEST_SESSION_ID__: JSON.stringify(sessionId), __VITEST_TESTER_ID__: '"none"', __VITEST_PROVIDED_CONTEXT__: '{}', + __VITEST_API_TOKEN__: JSON.stringify(globalServer.vitest.config.api.token), }) // disable CSP for the orchestrator as we are the ones controlling it diff --git a/packages/browser/src/node/serverTester.ts b/packages/browser/src/node/serverTester.ts index c6d2687c25c8..c70d91e17ad2 100644 --- a/packages/browser/src/node/serverTester.ts +++ b/packages/browser/src/node/serverTester.ts @@ -68,6 +68,7 @@ export async function resolveTester( __VITEST_SESSION_ID__: JSON.stringify(sessionId), __VITEST_TESTER_ID__: JSON.stringify(crypto.randomUUID()), __VITEST_PROVIDED_CONTEXT__: JSON.stringify(stringify(project.getProvidedContext())), + __VITEST_API_TOKEN__: JSON.stringify(globalServer.vitest.config.api.token), }) const testerHtml = typeof browserProject.testerHtml === 'string' diff --git a/packages/ui/client/constants.ts b/packages/ui/client/constants.ts index 2c3607fda3e3..582860c23f98 100644 --- a/packages/ui/client/constants.ts +++ b/packages/ui/client/constants.ts @@ -4,6 +4,6 @@ export const PORT = import.meta.hot && !browserState ? '51204' : location.port export const HOST = [location.hostname, PORT].filter(Boolean).join(':') export const ENTRY_URL = `${ location.protocol === 'https:' ? 'wss:' : 'ws:' -}//${HOST}/__vitest_api__` +}//${HOST}/__vitest_api__?token=${(window as any).VITEST_API_TOKEN}` export const isReport = !!window.METADATA_PATH export const BASE_PATH = isReport ? import.meta.env.BASE_URL : __BASE_PATH__ diff --git a/packages/ui/node/index.ts b/packages/ui/node/index.ts index 69827c6c3a8f..e07ced386677 100644 --- a/packages/ui/node/index.ts +++ b/packages/ui/node/index.ts @@ -1,5 +1,6 @@ import type { Plugin } from 'vite' import type { Vitest } from 'vitest/node' +import fs from 'node:fs' import { fileURLToPath } from 'node:url' import { toArray } from '@vitest/utils' import { basename, resolve } from 'pathe' @@ -52,6 +53,26 @@ export default (ctx: Vitest): Plugin => { } const clientDist = resolve(fileURLToPath(import.meta.url), '../client') + const clientIndexHtml = fs.readFileSync(resolve(clientDist, 'index.html'), 'utf-8') + + // serve index.html with api token + server.middlewares.use((req, res, next) => { + if (req.url) { + const url = new URL(req.url, 'http://localhost') + if (url.pathname === base) { + const html = clientIndexHtml.replace( + '', + ``, + ) + res.setHeader('content-type', 'text/html') + res.write(html) + res.end() + return + } + } + next() + }) + server.middlewares.use( base, sirv(clientDist, { diff --git a/packages/vitest/src/api/hostCheck.ts b/packages/vitest/src/api/hostCheck.ts new file mode 100644 index 000000000000..3455092e8d50 --- /dev/null +++ b/packages/vitest/src/api/hostCheck.ts @@ -0,0 +1,148 @@ +import type { IncomingMessage } from 'node:http' +import type { ResolvedConfig } from 'vite' +import type { ResolvedConfig as VitestResolvedConfig } from '../node/types/config' +import net from 'node:net' + +// based on +// https://github.com/vitejs/vite-ghsa-vg6x-rcgg-rjx6/pull/1 + +const isFileOrExtensionProtocolRE = /^(?:file|.+-extension):/i + +function getAdditionalAllowedHosts( + resolvedServerOptions: Pick, + resolvedPreviewOptions: Pick, +): string[] { + const list = [] + + // allow host option by default as that indicates that the user is + // expecting Vite to respond on that host + if ( + typeof resolvedServerOptions.host === 'string' + && resolvedServerOptions.host + ) { + list.push(resolvedServerOptions.host) + } + if ( + typeof resolvedServerOptions.hmr === 'object' + && resolvedServerOptions.hmr.host + ) { + list.push(resolvedServerOptions.hmr.host) + } + if ( + typeof resolvedPreviewOptions.host === 'string' + && resolvedPreviewOptions.host + ) { + list.push(resolvedPreviewOptions.host) + } + + // allow server origin by default as that indicates that the user is + // expecting Vite to respond on that host + if (resolvedServerOptions.origin) { + const serverOriginUrl = new URL(resolvedServerOptions.origin) + list.push(serverOriginUrl.hostname) + } + + return list +} + +// Based on webpack-dev-server's `checkHeader` function: https://github.com/webpack/webpack-dev-server/blob/v5.2.0/lib/Server.js#L3086 +// https://github.com/webpack/webpack-dev-server/blob/v5.2.0/LICENSE +function isHostAllowedWithoutCache( + allowedHosts: string[], + additionalAllowedHosts: string[], + host: string, +): boolean { + if (isFileOrExtensionProtocolRE.test(host)) { + return true + } + + // We don't care about malformed Host headers, + // because we only need to consider browser requests. + // Non-browser clients can send any value they want anyway. + // + // `Host = uri-host [ ":" port ]` + const trimmedHost = host.trim() + + // IPv6 + if (trimmedHost[0] === '[') { + const endIpv6 = trimmedHost.indexOf(']') + if (endIpv6 < 0) { + return false + } + // DNS rebinding attacks does not happen with IP addresses + return net.isIP(trimmedHost.slice(1, endIpv6)) === 6 + } + + // uri-host does not include ":" unless IPv6 address + const colonPos = trimmedHost.indexOf(':') + const hostname + = colonPos === -1 ? trimmedHost : trimmedHost.slice(0, colonPos) + + // DNS rebinding attacks does not happen with IP addresses + if (net.isIP(hostname) === 4) { + return true + } + + // allow localhost and .localhost by default as they always resolve to the loopback address + // https://datatracker.ietf.org/doc/html/rfc6761#section-6.3 + if (hostname === 'localhost' || hostname.endsWith('.localhost')) { + return true + } + + for (const additionalAllowedHost of additionalAllowedHosts) { + if (additionalAllowedHost === hostname) { + return true + } + } + + for (const allowedHost of allowedHosts) { + if (allowedHost === hostname) { + return true + } + + // allow all subdomains of it + // e.g. `.foo.example` will allow `foo.example`, `*.foo.example`, `*.*.foo.example`, etc + if ( + allowedHost[0] === '.' + && (allowedHost.slice(1) === hostname || hostname.endsWith(allowedHost)) + ) { + return true + } + } + + return false +} + +/** + * @param vitestConfig + * @param viteConfig resolved config + * @param host the value of host header. See [RFC 9110 7.2](https://datatracker.ietf.org/doc/html/rfc9110#name-host-and-authority). + */ +function isHostAllowed(vitestConfig: VitestResolvedConfig, viteConfig: ResolvedConfig, host: string): boolean { + const apiAllowedHosts = vitestConfig.api.allowedHosts ?? [] + if (apiAllowedHosts === true) { + return true + } + // Vitest only validates websocket upgrade request, so caching won't probably matter. + return isHostAllowedWithoutCache( + apiAllowedHosts, + getAdditionalAllowedHosts(viteConfig.server, viteConfig.preview), + host, + ) +} + +export function isWebsocketRequestAllowed(vitestConfig: VitestResolvedConfig, viteConfig: ResolvedConfig, req: IncomingMessage): boolean { + const url = new URL(req.url ?? '', 'http://localhost') + + // validate token. token is injected in ui/tester/orchestrator html, which is cross origin proteced. + if (url.searchParams.get('token') !== vitestConfig.api.token) { + return false + } + + // host check to prevent DNS rebinding attacks + if (!req.headers.host || !isHostAllowed(vitestConfig, viteConfig, req.headers.host)) { + return false + } + + return true +} diff --git a/packages/vitest/src/api/setup.ts b/packages/vitest/src/api/setup.ts index addd52af2791..d23bfde74a67 100644 --- a/packages/vitest/src/api/setup.ts +++ b/packages/vitest/src/api/setup.ts @@ -1,5 +1,6 @@ import type { File, TaskResultPack } from '@vitest/runner' +import type { IncomingMessage } from 'node:http' import type { ViteDevServer } from 'vite' import type { WebSocket } from 'ws' import type { Vitest } from '../node/core' @@ -21,6 +22,7 @@ import { API_PATH } from '../constants' import { getModuleGraph } from '../utils/graph' import { stringifyReplace } from '../utils/serialization' import { parseErrorStacktrace } from '../utils/source-map' +import { isWebsocketRequestAllowed } from './hostCheck' export function setup(ctx: Vitest, _server?: ViteDevServer) { const wss = new WebSocketServer({ noServer: true }) @@ -29,7 +31,7 @@ export function setup(ctx: Vitest, _server?: ViteDevServer) { const server = _server || ctx.server - server.httpServer?.on('upgrade', (request, socket, head) => { + server.httpServer?.on('upgrade', (request: IncomingMessage, socket, head) => { if (!request.url) { return } @@ -39,6 +41,11 @@ export function setup(ctx: Vitest, _server?: ViteDevServer) { return } + if (!isWebsocketRequestAllowed(ctx.config, server.config, request)) { + socket.destroy() + return + } + wss.handleUpgrade(request, socket, head, (ws) => { wss.emit('connection', ws, request) setupClient(ws) diff --git a/packages/vitest/src/node/cli/cli-config.ts b/packages/vitest/src/node/cli/cli-config.ts index 725a6750130a..8780371595f3 100644 --- a/packages/vitest/src/node/cli/cli-config.ts +++ b/packages/vitest/src/node/cli/cli-config.ts @@ -52,6 +52,7 @@ const apiConfig: (port: number) => CLIOptions = (port: number) => ({ 'Set to true to exit if port is already in use, instead of automatically trying the next available port', }, middlewareMode: null, + allowedHosts: null, }) const poolThreadsCommands: CLIOptions = { diff --git a/packages/vitest/src/node/config/resolveConfig.ts b/packages/vitest/src/node/config/resolveConfig.ts index 4255f9a12c35..117f95f69bb3 100644 --- a/packages/vitest/src/node/config/resolveConfig.ts +++ b/packages/vitest/src/node/config/resolveConfig.ts @@ -9,6 +9,7 @@ import type { } from '../types/config' import type { BaseCoverageOptions, CoverageReporterWithOptions } from '../types/coverage' import type { BuiltinPool, ForksOptions, PoolOptions, ThreadsOptions } from '../types/pool-options' +import crypto from 'node:crypto' import { toArray } from '@vitest/utils' import { resolveModule } from 'local-pkg' import { normalize, relative, resolve } from 'pathe' @@ -624,7 +625,8 @@ export function resolveConfig( } // the server has been created, we don't need to override vite.server options - resolved.api = resolveApiServerConfig(options, defaultPort) + const api = resolveApiServerConfig(options, defaultPort) + resolved.api = { ...api, token: crypto.randomUUID() } if (options.related) { resolved.related = toArray(options.related).map(file => diff --git a/packages/vitest/src/node/types/config.ts b/packages/vitest/src/node/types/config.ts index 0f556b99fce2..8b759f81a600 100644 --- a/packages/vitest/src/node/types/config.ts +++ b/packages/vitest/src/node/types/config.ts @@ -44,7 +44,9 @@ export type CSSModuleScopeStrategy = 'stable' | 'scoped' | 'non-scoped' export type ApiConfig = Pick< ServerOptions, 'port' | 'strictPort' | 'host' | 'middlewareMode' -> +> & { + allowedHosts?: string[] | true +} export type { EnvironmentOptions, HappyDOMOptions, JSDOMOptions } @@ -1013,7 +1015,7 @@ export interface ResolvedConfig defines: Record - api?: ApiConfig + api: ApiConfig & { token: string } cliExclude?: string[] benchmark?: Required< diff --git a/packages/vitest/src/public/node.ts b/packages/vitest/src/public/node.ts index a0031b64801d..911f72bdd3af 100644 --- a/packages/vitest/src/public/node.ts +++ b/packages/vitest/src/public/node.ts @@ -5,6 +5,7 @@ import { TestModule as _TestFile } from '../node/reporters/reported-tasks' export const version = Vitest.version +export { isWebsocketRequestAllowed } from '../api/hostCheck' export { parseCLI } from '../node/cli/cac' export type { CliParseOptions } from '../node/cli/cac' export { startVitest } from '../node/cli/cli-api' From 7c812cb77c4f4544c3f33784f10cd91710cfd710 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Mon, 20 Jan 2025 16:03:09 +0900 Subject: [PATCH 02/13] docs: fix --- docs/config/index.md | 13 ++++++++++++- docs/guide/cli-generated.md | 11 ----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/config/index.md b/docs/config/index.md index 01a5efe5f5ad..6b75fbc37d9a 100644 --- a/docs/config/index.md +++ b/docs/config/index.md @@ -1749,12 +1749,23 @@ Open Vitest UI (WIP) ### api -- **Type:** `boolean | number` +- **Type:** `boolean | number | ApiConfig` - **Default:** `false` - **CLI:** `--api`, `--api.port`, `--api.host`, `--api.strictPort` Listen to port and serve API. When set to true, the default port is 51204 +### api.allowedHosts + +- **Type:** `string[] | true` +- **Default:** `[]` + +The hostnames that Vitest API is allowed to respond to. `localhost` and domains under `.localhost` and all IP addresses are allowed by default. When using HTTPS, this check is skipped. + +If a string starts with `.`, it will allow that hostname without the `.` and all subdomains under the hostname. For example, `.example.com` will allow `example.com`, `foo.example.com`, and `foo.bar.example.com`. + +If set to `true`, the server is allowed to respond to requests for any hosts. This is not recommended as it will be vulnerable to DNS rebinding attacks. + ### browser experimental {#browser} - **Default:** `{ enabled: false }` diff --git a/docs/guide/cli-generated.md b/docs/guide/cli-generated.md index 68dc2ec5f89f..3a2b078a3e92 100644 --- a/docs/guide/cli-generated.md +++ b/docs/guide/cli-generated.md @@ -71,17 +71,6 @@ Specify which IP addresses the server should listen on. Set this to `0.0.0.0` or Set to true to exit if port is already in use, instead of automatically trying the next available port -### api.allowedHosts - -- **Type:** `string[] | true` -- **Default:** `[]` - -The hostnames that Vitest API is allowed to respond to. `localhost` and domains under `.localhost` and all IP addresses are allowed by default. When using HTTPS, this check is skipped. - -If a string starts with `.`, it will allow that hostname without the `.` and all subdomains under the hostname. For example, `.example.com` will allow `example.com`, `foo.example.com`, and `foo.bar.example.com`. - -If set to `true`, the server is allowed to respond to requests for any hosts. This is not recommended as it will be vulnerable to DNS rebinding attacks. - ### silent - **CLI:** `--silent` From 9f5988d7a2061ad321039ac9c1b3f0480a3f4816 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Jan 2025 11:44:21 +0900 Subject: [PATCH 03/13] chore: comment --- packages/vitest/src/api/hostCheck.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vitest/src/api/hostCheck.ts b/packages/vitest/src/api/hostCheck.ts index 3455092e8d50..6a950d6181d9 100644 --- a/packages/vitest/src/api/hostCheck.ts +++ b/packages/vitest/src/api/hostCheck.ts @@ -4,7 +4,7 @@ import type { ResolvedConfig as VitestResolvedConfig } from '../node/types/confi import net from 'node:net' // based on -// https://github.com/vitejs/vite-ghsa-vg6x-rcgg-rjx6/pull/1 +// https://github.com/vitejs/vite/blob/9654348258eaa0883171533a2b74b4e2825f5fb6/packages/vite/src/node/server/middlewares/hostCheck.ts const isFileOrExtensionProtocolRE = /^(?:file|.+-extension):/i From 66f34b88900d3437fe824dcbd949db3bdf0e3edb Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Jan 2025 11:49:52 +0900 Subject: [PATCH 04/13] test: fix token --- test/config/test/override.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/config/test/override.test.ts b/test/config/test/override.test.ts index 0b6dc673657e..ceb4158aaaa5 100644 --- a/test/config/test/override.test.ts +++ b/test/config/test/override.test.ts @@ -249,6 +249,7 @@ describe('correctly defines api flag', () => { expect(c.server.config.server.middlewareMode).toBe(true) expect(c.config.api).toEqual({ middlewareMode: true, + token: expect.any(String), }) }) @@ -262,6 +263,7 @@ describe('correctly defines api flag', () => { expect(c.server.config.server.port).toBe(4321) expect(c.config.api).toEqual({ port: 4321, + token: expect.any(String), }) }) }) From af2e96c14e5a76e2aaf1781489d389df59b7d966 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Jan 2025 12:24:53 +0900 Subject: [PATCH 05/13] fix: timing safe equal + handle invalid server.origin --- packages/vitest/rollup.config.js | 1 + packages/vitest/src/api/hostCheck.ts | 21 ++++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/vitest/rollup.config.js b/packages/vitest/rollup.config.js index 5218a10769e0..8221b4cda508 100644 --- a/packages/vitest/rollup.config.js +++ b/packages/vitest/rollup.config.js @@ -72,6 +72,7 @@ const external = [ 'node:os', 'node:stream', 'node:vm', + 'node:http', 'inspector', 'vite-node/source-map', 'vite-node/client', diff --git a/packages/vitest/src/api/hostCheck.ts b/packages/vitest/src/api/hostCheck.ts index 6a950d6181d9..48a6c4a53472 100644 --- a/packages/vitest/src/api/hostCheck.ts +++ b/packages/vitest/src/api/hostCheck.ts @@ -1,6 +1,7 @@ import type { IncomingMessage } from 'node:http' import type { ResolvedConfig } from 'vite' import type { ResolvedConfig as VitestResolvedConfig } from '../node/types/config' +import crypto from 'node:crypto' import net from 'node:net' // based on @@ -38,8 +39,11 @@ function getAdditionalAllowedHosts( // allow server origin by default as that indicates that the user is // expecting Vite to respond on that host if (resolvedServerOptions.origin) { - const serverOriginUrl = new URL(resolvedServerOptions.origin) - list.push(serverOriginUrl.hostname) + try { + const serverOriginUrl = new URL(resolvedServerOptions.origin) + list.push(serverOriginUrl.hostname) + } + catch {} } return list @@ -135,11 +139,22 @@ export function isWebsocketRequestAllowed(vitestConfig: VitestResolvedConfig, vi const url = new URL(req.url ?? '', 'http://localhost') // validate token. token is injected in ui/tester/orchestrator html, which is cross origin proteced. - if (url.searchParams.get('token') !== vitestConfig.api.token) { + try { + const token = url.searchParams.get('token') + if (!token || !crypto.timingSafeEqual( + Buffer.from(token), + Buffer.from(vitestConfig.api.token), + )) { + return false + } + } + catch { + // an error is thrown when the length is incorrect return false } // host check to prevent DNS rebinding attacks + // (websocket upgrade request cannot be http2 even on `wss`, so `host` header is guaranteed.) if (!req.headers.host || !isHostAllowed(vitestConfig, viteConfig, req.headers.host)) { return false } From 44d2b5c5a6cbf72b53fc27e4db4a22d769f0d300 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Jan 2025 16:01:05 +0900 Subject: [PATCH 06/13] fix: add cache-control no cache --- packages/ui/node/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ui/node/index.ts b/packages/ui/node/index.ts index e07ced386677..66e86ce143af 100644 --- a/packages/ui/node/index.ts +++ b/packages/ui/node/index.ts @@ -64,7 +64,8 @@ export default (ctx: Vitest): Plugin => { '', ``, ) - res.setHeader('content-type', 'text/html') + res.setHeader('Cache-Control', 'no-cache, max-age=0, must-revalidate') + res.setHeader('Content-Type', 'text/html; charset=utf-8') res.write(html) res.end() return From ed50f3da25cc95fa3da2cf5034bddadf6c9477c1 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Jan 2025 17:38:12 +0900 Subject: [PATCH 07/13] docs: Update docs/config/index.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ari Perkkiö --- docs/config/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/config/index.md b/docs/config/index.md index 6b75fbc37d9a..a29f3c73fc2e 100644 --- a/docs/config/index.md +++ b/docs/config/index.md @@ -1755,7 +1755,7 @@ Open Vitest UI (WIP) Listen to port and serve API. When set to true, the default port is 51204 -### api.allowedHosts +#### api.allowedHosts - **Type:** `string[] | true` - **Default:** `[]` From 835e05575204751b3cec9f7a4ad203d22fdd883b Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 22 Jan 2025 11:52:14 +0900 Subject: [PATCH 08/13] wip: remove host check --- docs/config/index.md | 13 +- packages/browser/src/node/rpc.ts | 2 +- packages/vitest/src/api/hostCheck.ts | 157 ++------------------- packages/vitest/src/api/setup.ts | 2 +- packages/vitest/src/node/cli/cli-config.ts | 1 - packages/vitest/src/node/types/config.ts | 4 +- 6 files changed, 12 insertions(+), 167 deletions(-) diff --git a/docs/config/index.md b/docs/config/index.md index a29f3c73fc2e..01a5efe5f5ad 100644 --- a/docs/config/index.md +++ b/docs/config/index.md @@ -1749,23 +1749,12 @@ Open Vitest UI (WIP) ### api -- **Type:** `boolean | number | ApiConfig` +- **Type:** `boolean | number` - **Default:** `false` - **CLI:** `--api`, `--api.port`, `--api.host`, `--api.strictPort` Listen to port and serve API. When set to true, the default port is 51204 -#### api.allowedHosts - -- **Type:** `string[] | true` -- **Default:** `[]` - -The hostnames that Vitest API is allowed to respond to. `localhost` and domains under `.localhost` and all IP addresses are allowed by default. When using HTTPS, this check is skipped. - -If a string starts with `.`, it will allow that hostname without the `.` and all subdomains under the hostname. For example, `.example.com` will allow `example.com`, `foo.example.com`, and `foo.bar.example.com`. - -If set to `true`, the server is allowed to respond to requests for any hosts. This is not recommended as it will be vulnerable to DNS rebinding attacks. - ### browser experimental {#browser} - **Default:** `{ enabled: false }` diff --git a/packages/browser/src/node/rpc.ts b/packages/browser/src/node/rpc.ts index ca7cf48990ee..33a32df8b1a7 100644 --- a/packages/browser/src/node/rpc.ts +++ b/packages/browser/src/node/rpc.ts @@ -33,7 +33,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject) { return } - if (!isWebsocketRequestAllowed(vitest.config, vite.config, request)) { + if (!isWebsocketRequestAllowed(vitest.config, request)) { socket.destroy() return } diff --git a/packages/vitest/src/api/hostCheck.ts b/packages/vitest/src/api/hostCheck.ts index 48a6c4a53472..fa816ec4d2d5 100644 --- a/packages/vitest/src/api/hostCheck.ts +++ b/packages/vitest/src/api/hostCheck.ts @@ -1,163 +1,22 @@ import type { IncomingMessage } from 'node:http' -import type { ResolvedConfig } from 'vite' -import type { ResolvedConfig as VitestResolvedConfig } from '../node/types/config' +import type { ResolvedConfig } from '../node/types/config' import crypto from 'node:crypto' -import net from 'node:net' -// based on -// https://github.com/vitejs/vite/blob/9654348258eaa0883171533a2b74b4e2825f5fb6/packages/vite/src/node/server/middlewares/hostCheck.ts - -const isFileOrExtensionProtocolRE = /^(?:file|.+-extension):/i - -function getAdditionalAllowedHosts( - resolvedServerOptions: Pick, - resolvedPreviewOptions: Pick, -): string[] { - const list = [] - - // allow host option by default as that indicates that the user is - // expecting Vite to respond on that host - if ( - typeof resolvedServerOptions.host === 'string' - && resolvedServerOptions.host - ) { - list.push(resolvedServerOptions.host) - } - if ( - typeof resolvedServerOptions.hmr === 'object' - && resolvedServerOptions.hmr.host - ) { - list.push(resolvedServerOptions.hmr.host) - } - if ( - typeof resolvedPreviewOptions.host === 'string' - && resolvedPreviewOptions.host - ) { - list.push(resolvedPreviewOptions.host) - } - - // allow server origin by default as that indicates that the user is - // expecting Vite to respond on that host - if (resolvedServerOptions.origin) { - try { - const serverOriginUrl = new URL(resolvedServerOptions.origin) - list.push(serverOriginUrl.hostname) - } - catch {} - } - - return list -} - -// Based on webpack-dev-server's `checkHeader` function: https://github.com/webpack/webpack-dev-server/blob/v5.2.0/lib/Server.js#L3086 -// https://github.com/webpack/webpack-dev-server/blob/v5.2.0/LICENSE -function isHostAllowedWithoutCache( - allowedHosts: string[], - additionalAllowedHosts: string[], - host: string, -): boolean { - if (isFileOrExtensionProtocolRE.test(host)) { - return true - } - - // We don't care about malformed Host headers, - // because we only need to consider browser requests. - // Non-browser clients can send any value they want anyway. - // - // `Host = uri-host [ ":" port ]` - const trimmedHost = host.trim() - - // IPv6 - if (trimmedHost[0] === '[') { - const endIpv6 = trimmedHost.indexOf(']') - if (endIpv6 < 0) { - return false - } - // DNS rebinding attacks does not happen with IP addresses - return net.isIP(trimmedHost.slice(1, endIpv6)) === 6 - } - - // uri-host does not include ":" unless IPv6 address - const colonPos = trimmedHost.indexOf(':') - const hostname - = colonPos === -1 ? trimmedHost : trimmedHost.slice(0, colonPos) - - // DNS rebinding attacks does not happen with IP addresses - if (net.isIP(hostname) === 4) { - return true - } - - // allow localhost and .localhost by default as they always resolve to the loopback address - // https://datatracker.ietf.org/doc/html/rfc6761#section-6.3 - if (hostname === 'localhost' || hostname.endsWith('.localhost')) { - return true - } - - for (const additionalAllowedHost of additionalAllowedHosts) { - if (additionalAllowedHost === hostname) { - return true - } - } - - for (const allowedHost of allowedHosts) { - if (allowedHost === hostname) { - return true - } - - // allow all subdomains of it - // e.g. `.foo.example` will allow `foo.example`, `*.foo.example`, `*.*.foo.example`, etc - if ( - allowedHost[0] === '.' - && (allowedHost.slice(1) === hostname || hostname.endsWith(allowedHost)) - ) { - return true - } - } - - return false -} - -/** - * @param vitestConfig - * @param viteConfig resolved config - * @param host the value of host header. See [RFC 9110 7.2](https://datatracker.ietf.org/doc/html/rfc9110#name-host-and-authority). - */ -function isHostAllowed(vitestConfig: VitestResolvedConfig, viteConfig: ResolvedConfig, host: string): boolean { - const apiAllowedHosts = vitestConfig.api.allowedHosts ?? [] - if (apiAllowedHosts === true) { - return true - } - // Vitest only validates websocket upgrade request, so caching won't probably matter. - return isHostAllowedWithoutCache( - apiAllowedHosts, - getAdditionalAllowedHosts(viteConfig.server, viteConfig.preview), - host, - ) -} - -export function isWebsocketRequestAllowed(vitestConfig: VitestResolvedConfig, viteConfig: ResolvedConfig, req: IncomingMessage): boolean { +export function isWebsocketRequestAllowed(config: ResolvedConfig, req: IncomingMessage): boolean { const url = new URL(req.url ?? '', 'http://localhost') // validate token. token is injected in ui/tester/orchestrator html, which is cross origin proteced. try { const token = url.searchParams.get('token') - if (!token || !crypto.timingSafeEqual( + if (token && crypto.timingSafeEqual( Buffer.from(token), - Buffer.from(vitestConfig.api.token), + Buffer.from(config.api.token), )) { - return false + return true } } - catch { - // an error is thrown when the length is incorrect - return false - } + // an error is thrown when the length is incorrect + catch {} - // host check to prevent DNS rebinding attacks - // (websocket upgrade request cannot be http2 even on `wss`, so `host` header is guaranteed.) - if (!req.headers.host || !isHostAllowed(vitestConfig, viteConfig, req.headers.host)) { - return false - } - - return true + return false } diff --git a/packages/vitest/src/api/setup.ts b/packages/vitest/src/api/setup.ts index d23bfde74a67..c79a96bc3023 100644 --- a/packages/vitest/src/api/setup.ts +++ b/packages/vitest/src/api/setup.ts @@ -41,7 +41,7 @@ export function setup(ctx: Vitest, _server?: ViteDevServer) { return } - if (!isWebsocketRequestAllowed(ctx.config, server.config, request)) { + if (!isWebsocketRequestAllowed(ctx.config, request)) { socket.destroy() return } diff --git a/packages/vitest/src/node/cli/cli-config.ts b/packages/vitest/src/node/cli/cli-config.ts index 8780371595f3..725a6750130a 100644 --- a/packages/vitest/src/node/cli/cli-config.ts +++ b/packages/vitest/src/node/cli/cli-config.ts @@ -52,7 +52,6 @@ const apiConfig: (port: number) => CLIOptions = (port: number) => ({ 'Set to true to exit if port is already in use, instead of automatically trying the next available port', }, middlewareMode: null, - allowedHosts: null, }) const poolThreadsCommands: CLIOptions = { diff --git a/packages/vitest/src/node/types/config.ts b/packages/vitest/src/node/types/config.ts index 8b759f81a600..5db3cee7c295 100644 --- a/packages/vitest/src/node/types/config.ts +++ b/packages/vitest/src/node/types/config.ts @@ -44,9 +44,7 @@ export type CSSModuleScopeStrategy = 'stable' | 'scoped' | 'non-scoped' export type ApiConfig = Pick< ServerOptions, 'port' | 'strictPort' | 'host' | 'middlewareMode' -> & { - allowedHosts?: string[] | true -} +> export type { EnvironmentOptions, HappyDOMOptions, JSDOMOptions } From 9fb5f801d70d71557abf1bd16badc002611e3ea8 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 22 Jan 2025 11:56:29 +0900 Subject: [PATCH 09/13] refactor: rename --- packages/browser/src/node/rpc.ts | 4 ++-- packages/vitest/src/api/{hostCheck.ts => check.ts} | 2 +- packages/vitest/src/api/setup.ts | 4 ++-- packages/vitest/src/public/node.ts | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) rename packages/vitest/src/api/{hostCheck.ts => check.ts} (85%) diff --git a/packages/browser/src/node/rpc.ts b/packages/browser/src/node/rpc.ts index 33a32df8b1a7..7c80b3862e05 100644 --- a/packages/browser/src/node/rpc.ts +++ b/packages/browser/src/node/rpc.ts @@ -10,7 +10,7 @@ import { ServerMockResolver } from '@vitest/mocker/node' import { createBirpc } from 'birpc' import { parse, stringify } from 'flatted' import { dirname } from 'pathe' -import { createDebugger, isFileServingAllowed, isWebsocketRequestAllowed } from 'vitest/node' +import { createDebugger, isFileServingAllowed, isValidApiRequest } from 'vitest/node' import { WebSocketServer } from 'ws' const debug = createDebugger('vitest:browser:api') @@ -33,7 +33,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject) { return } - if (!isWebsocketRequestAllowed(vitest.config, request)) { + if (!isValidApiRequest(vitest.config, request)) { socket.destroy() return } diff --git a/packages/vitest/src/api/hostCheck.ts b/packages/vitest/src/api/check.ts similarity index 85% rename from packages/vitest/src/api/hostCheck.ts rename to packages/vitest/src/api/check.ts index fa816ec4d2d5..13eb5bfcc6ad 100644 --- a/packages/vitest/src/api/hostCheck.ts +++ b/packages/vitest/src/api/check.ts @@ -2,7 +2,7 @@ import type { IncomingMessage } from 'node:http' import type { ResolvedConfig } from '../node/types/config' import crypto from 'node:crypto' -export function isWebsocketRequestAllowed(config: ResolvedConfig, req: IncomingMessage): boolean { +export function isValidApiRequest(config: ResolvedConfig, req: IncomingMessage): boolean { const url = new URL(req.url ?? '', 'http://localhost') // validate token. token is injected in ui/tester/orchestrator html, which is cross origin proteced. diff --git a/packages/vitest/src/api/setup.ts b/packages/vitest/src/api/setup.ts index c79a96bc3023..903dcfad653b 100644 --- a/packages/vitest/src/api/setup.ts +++ b/packages/vitest/src/api/setup.ts @@ -22,7 +22,7 @@ import { API_PATH } from '../constants' import { getModuleGraph } from '../utils/graph' import { stringifyReplace } from '../utils/serialization' import { parseErrorStacktrace } from '../utils/source-map' -import { isWebsocketRequestAllowed } from './hostCheck' +import { isValidApiRequest } from './check' export function setup(ctx: Vitest, _server?: ViteDevServer) { const wss = new WebSocketServer({ noServer: true }) @@ -41,7 +41,7 @@ export function setup(ctx: Vitest, _server?: ViteDevServer) { return } - if (!isWebsocketRequestAllowed(ctx.config, request)) { + if (!isValidApiRequest(ctx.config, request)) { socket.destroy() return } diff --git a/packages/vitest/src/public/node.ts b/packages/vitest/src/public/node.ts index 911f72bdd3af..416128d66aaf 100644 --- a/packages/vitest/src/public/node.ts +++ b/packages/vitest/src/public/node.ts @@ -5,7 +5,7 @@ import { TestModule as _TestFile } from '../node/reporters/reported-tasks' export const version = Vitest.version -export { isWebsocketRequestAllowed } from '../api/hostCheck' +export { isValidApiRequest } from '../api/check' export { parseCLI } from '../node/cli/cac' export type { CliParseOptions } from '../node/cli/cac' export { startVitest } from '../node/cli/cli-api' From 983b462cefeab602e102eb885feb0cfa24614865 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 22 Jan 2025 12:38:26 +0900 Subject: [PATCH 10/13] test: add ui test --- test/ui/test/ui.spec.ts | 30 ++++++++++++++++++++++++++++++ test/ui/tsconfig.json | 1 + 2 files changed, 31 insertions(+) diff --git a/test/ui/test/ui.spec.ts b/test/ui/test/ui.spec.ts index 92267f1d90f6..9cd0036a3ff7 100644 --- a/test/ui/test/ui.spec.ts +++ b/test/ui/test/ui.spec.ts @@ -30,6 +30,36 @@ test.describe('ui', () => { await vitest?.close() }) + test('security', async ({ page }) => { + await page.goto('https://example.com/') + + // request html + const htmlResult = await page.evaluate(async (pageUrl) => { + try { + const res = await fetch(pageUrl) + return res.status + } + catch (e) { + return e instanceof Error ? e.message : e + } + }, pageUrl) + expect(htmlResult).toBe('Failed to fetch') + + // request websocket + const wsResult = await page.evaluate(async (pageUrl) => { + const ws = new WebSocket(new URL('/__vitest_api__', pageUrl)) + return new Promise((resolve) => { + ws.addEventListener('open', () => { + resolve('open') + }) + ws.addEventListener('error', () => { + resolve('error') + }) + }) + }, pageUrl) + expect(wsResult).toBe('error') + }) + test('basic', async ({ page }) => { const pageErrors: unknown[] = [] page.on('pageerror', error => pageErrors.push(error)) diff --git a/test/ui/tsconfig.json b/test/ui/tsconfig.json index accaf3ba63ee..21c54a461ab5 100644 --- a/test/ui/tsconfig.json +++ b/test/ui/tsconfig.json @@ -1,6 +1,7 @@ { "extends": "../../tsconfig.base.json", "compilerOptions": { + "lib": ["ESNext", "DOM"], "baseUrl": "../..", "paths": { "vitest/node": [ From 3604f2ebe81a743cfd8424904aa1314c677ebc8a Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 22 Jan 2025 13:15:01 +0900 Subject: [PATCH 11/13] chore: name middleware --- packages/ui/node/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ui/node/index.ts b/packages/ui/node/index.ts index 66e86ce143af..2e9e0146e20e 100644 --- a/packages/ui/node/index.ts +++ b/packages/ui/node/index.ts @@ -56,7 +56,8 @@ export default (ctx: Vitest): Plugin => { const clientIndexHtml = fs.readFileSync(resolve(clientDist, 'index.html'), 'utf-8') // serve index.html with api token - server.middlewares.use((req, res, next) => { + // eslint-disable-next-line prefer-arrow-callback + server.middlewares.use(function vitestUiHtmlMiddleware(req, res, next) { if (req.url) { const url = new URL(req.url, 'http://localhost') if (url.pathname === base) { From 7f918281ef8c57d9bb33e9ee17cf3691175cf86c Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 22 Jan 2025 16:21:53 +0900 Subject: [PATCH 12/13] fix: add vitestUiHtmlMiddleware after viteHostCheckMiddleware --- packages/ui/node/index.ts | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/ui/node/index.ts b/packages/ui/node/index.ts index 2e9e0146e20e..e52e077a264c 100644 --- a/packages/ui/node/index.ts +++ b/packages/ui/node/index.ts @@ -55,6 +55,14 @@ export default (ctx: Vitest): Plugin => { const clientDist = resolve(fileURLToPath(import.meta.url), '../client') const clientIndexHtml = fs.readFileSync(resolve(clientDist, 'index.html'), 'utf-8') + server.middlewares.use( + base, + sirv(clientDist, { + dev: true, + extensions: [], // don't serve index.html + }), + ) + // serve index.html with api token // eslint-disable-next-line prefer-arrow-callback server.middlewares.use(function vitestUiHtmlMiddleware(req, res, next) { @@ -75,13 +83,15 @@ export default (ctx: Vitest): Plugin => { next() }) - server.middlewares.use( - base, - sirv(clientDist, { - single: true, - dev: true, - }), - ) + return () => { + // move vitestUiHtmlMiddleware after viteHostCheckMiddleware if exists + const i1 = server.middlewares.stack.findIndex(h => 'name' in h.handle && h.handle.name === 'vitestUiHtmlMiddleware') + const i2 = server.middlewares.stack.findIndex(h => 'name' in h.handle && h.handle.name === 'viteHostCheckMiddleware') + if (i1 !== -1 && i2 !== -1 && i1 < i2) { + const [item] = server.middlewares.stack.splice(i1, 1) + server.middlewares.stack.splice(i2, 0, item) + } + } }, }, } From 4e5a50c150b74bc777383085bf4cd19fa1958f1c Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 22 Jan 2025 16:34:23 +0900 Subject: [PATCH 13/13] Revert "fix: add vitestUiHtmlMiddleware after viteHostCheckMiddleware" This reverts commit 7f918281ef8c57d9bb33e9ee17cf3691175cf86c. --- packages/ui/node/index.ts | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/packages/ui/node/index.ts b/packages/ui/node/index.ts index e52e077a264c..2e9e0146e20e 100644 --- a/packages/ui/node/index.ts +++ b/packages/ui/node/index.ts @@ -55,14 +55,6 @@ export default (ctx: Vitest): Plugin => { const clientDist = resolve(fileURLToPath(import.meta.url), '../client') const clientIndexHtml = fs.readFileSync(resolve(clientDist, 'index.html'), 'utf-8') - server.middlewares.use( - base, - sirv(clientDist, { - dev: true, - extensions: [], // don't serve index.html - }), - ) - // serve index.html with api token // eslint-disable-next-line prefer-arrow-callback server.middlewares.use(function vitestUiHtmlMiddleware(req, res, next) { @@ -83,15 +75,13 @@ export default (ctx: Vitest): Plugin => { next() }) - return () => { - // move vitestUiHtmlMiddleware after viteHostCheckMiddleware if exists - const i1 = server.middlewares.stack.findIndex(h => 'name' in h.handle && h.handle.name === 'vitestUiHtmlMiddleware') - const i2 = server.middlewares.stack.findIndex(h => 'name' in h.handle && h.handle.name === 'viteHostCheckMiddleware') - if (i1 !== -1 && i2 !== -1 && i1 < i2) { - const [item] = server.middlewares.stack.splice(i1, 1) - server.middlewares.stack.splice(i2, 0, item) - } - } + server.middlewares.use( + base, + sirv(clientDist, { + single: true, + dev: true, + }), + ) }, }, }