Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate websocket request #7317

Merged
merged 17 commits into from
Feb 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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