Skip to content

Commit

Permalink
fix: validate websocket request (#7317)
Browse files Browse the repository at this point in the history
Co-authored-by: Ari Perkkiö <[email protected]>
  • Loading branch information
hi-ogawa and AriPerkkio authored Feb 2, 2025
1 parent c82387d commit 191ef9e
Show file tree
Hide file tree
Showing 16 changed files with 103 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/browser/src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<CancelReason>((resolve) => {
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/client/public/esm-client-injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
method: { __VITEST_METHOD__ },
providedContext: { __VITEST_PROVIDED_CONTEXT__ },
};
window.VITEST_API_TOKEN = { __VITEST_API_TOKEN__ };

const config = __vitest_browser_runner__.config;

Expand Down
7 changes: 6 additions & 1 deletion packages/browser/src/node/rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, isValidApiRequest } from 'vitest/node'
import { WebSocketServer } from 'ws'

const debug = createDebugger('vitest:browser:api')
Expand All @@ -33,6 +33,11 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject) {
return
}

if (!isValidApiRequest(vitest.config, request)) {
socket.destroy()
return
}

const type = searchParams.get('type')
const rpcId = searchParams.get('rpcId')
const sessionId = searchParams.get('sessionId')
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/node/serverOrchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/node/serverTester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/client/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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__
23 changes: 23 additions & 0 deletions packages/ui/node/index.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -52,6 +53,28 @@ 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
// 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) {
const html = clientIndexHtml.replace(
'<!-- !LOAD_METADATA! -->',
`<script>window.VITEST_API_TOKEN = ${JSON.stringify(ctx.config.api.token)}</script>`,
)
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
}
}
next()
})

server.middlewares.use(
base,
sirv(clientDist, {
Expand Down
1 change: 1 addition & 0 deletions packages/vitest/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ const external = [
'node:os',
'node:stream',
'node:vm',
'node:http',
'inspector',
'vite-node/source-map',
'vite-node/client',
Expand Down
22 changes: 22 additions & 0 deletions packages/vitest/src/api/check.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import type { IncomingMessage } from 'node:http'
import type { ResolvedConfig } from '../node/types/config'
import crypto from 'node:crypto'

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.
try {
const token = url.searchParams.get('token')
if (token && crypto.timingSafeEqual(
Buffer.from(token),
Buffer.from(config.api.token),
)) {
return true
}
}
// an error is thrown when the length is incorrect
catch {}

return false
}
9 changes: 8 additions & 1 deletion packages/vitest/src/api/setup.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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 { isValidApiRequest } from './check'

export function setup(ctx: Vitest, _server?: ViteDevServer) {
const wss = new WebSocketServer({ noServer: true })
Expand All @@ -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
}
Expand All @@ -39,6 +41,11 @@ export function setup(ctx: Vitest, _server?: ViteDevServer) {
return
}

if (!isValidApiRequest(ctx.config, request)) {
socket.destroy()
return
}

wss.handleUpgrade(request, socket, head, (ws) => {
wss.emit('connection', ws, request)
setupClient(ws)
Expand Down
4 changes: 3 additions & 1 deletion packages/vitest/src/node/config/resolveConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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'
Expand Down Expand Up @@ -629,7 +630,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 =>
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/types/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,7 @@ export interface ResolvedConfig

defines: Record<string, any>

api?: ApiConfig
api: ApiConfig & { token: string }
cliExclude?: string[]

project: string[]
Expand Down
1 change: 1 addition & 0 deletions packages/vitest/src/public/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { TestModule as _TestFile } from '../node/reporters/reported-tasks'

export const version = Vitest.version

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'
Expand Down
2 changes: 2 additions & 0 deletions test/config/test/override.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
})

Expand All @@ -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),
})
})
})
Expand Down
30 changes: 30 additions & 0 deletions test/ui/test/ui.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions test/ui/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"lib": ["ESNext", "DOM"],
"baseUrl": "../..",
"paths": {
"vitest/node": [
Expand Down

0 comments on commit 191ef9e

Please sign in to comment.