Skip to content

Commit 5c18dd2

Browse files
hi-ogawaOpenCode (claude-opus-4-8)
andauthored
fix(browser): check fs access in builtin commands [backport to v4] (#10680)
Co-authored-by: Hiroshi Ogawa <4232207+hi-ogawa@users.noreply.github.com> Co-authored-by: OpenCode (claude-opus-4-8) <noreply@opencode.ai>
1 parent bae52b5 commit 5c18dd2

19 files changed

Lines changed: 218 additions & 34 deletions

File tree

packages/browser-playwright/src/commands/screenshot.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { ScreenshotOptions } from 'vitest/browser'
22
import type { BrowserCommandContext } from 'vitest/node'
33
import { mkdir } from 'node:fs/promises'
4-
import { resolveScreenshotPath } from '@vitest/browser'
4+
import { assertBrowserApiWrite, assertBrowserFileAccess, resolveScreenshotPath } from '@vitest/browser'
55
import { dirname, normalize } from 'pathe'
66
import { getDescribedLocator } from './utils'
77

@@ -39,6 +39,9 @@ export async function takeScreenshot(
3939
if (options.save) {
4040
savePath = normalize(path)
4141

42+
assertBrowserApiWrite(context.project, savePath)
43+
assertBrowserFileAccess(context.project, savePath)
44+
4245
await mkdir(dirname(savePath), { recursive: true })
4346
}
4447

packages/browser-playwright/src/commands/trace.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ParsedStack } from 'vitest'
22
import type { BrowserCommand, BrowserCommandContext, BrowserProvider } from 'vitest/node'
33
import type { PlaywrightBrowserProvider } from '../playwright'
44
import { unlink } from 'node:fs/promises'
5+
import { assertBrowserApiWrite, assertBrowserFileAccess } from '@vitest/browser'
56
import { basename, dirname, relative, resolve } from 'pathe'
67
import { getDescribedLocator } from './utils'
78

@@ -51,6 +52,8 @@ export const stopChunkTrace: BrowserCommand<[{ name: string }]> = async (
5152
) => {
5253
if (isPlaywrightProvider(context.provider)) {
5354
const path = resolveTracesPath(context, name)
55+
assertBrowserApiWrite(context.project, path)
56+
assertBrowserFileAccess(context.project, path)
5457
context.provider.pendingTraces.delete(path)
5558
await context.context.tracing.stopChunk({ path })
5659
return { tracePath: path }
@@ -162,6 +165,10 @@ export const deleteTracing: BrowserCommand<[{ traces: string[] }]> = async (
162165
throw new Error(`stopChunkTrace cannot be called outside of the test file.`)
163166
}
164167
if (isPlaywrightProvider(context.provider)) {
168+
for (const trace of traces) {
169+
assertBrowserApiWrite(context.project, trace)
170+
assertBrowserFileAccess(context.project, trace)
171+
}
165172
return Promise.all(
166173
traces.map(trace => unlink(trace).catch((err) => {
167174
if (err.code === 'ENOENT') {
@@ -183,6 +190,8 @@ export const annotateTraces: BrowserCommand<[{ traces: string[]; testId: string
183190
) => {
184191
const vitest = project.vitest
185192
await Promise.all(traces.map((trace) => {
193+
assertBrowserApiWrite(project, trace)
194+
assertBrowserFileAccess(project, trace)
186195
const entity = vitest.state.getReportedEntityById(testId)
187196
const location = entity?.location
188197
? {

packages/browser-playwright/src/commands/upload.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { UserEventUploadOptions } from 'vitest/browser'
22
import type { UserEventCommand } from './utils'
3+
import { assertBrowserFileAccess } from '@vitest/browser'
34
import { resolve } from 'pathe'
45
import { getDescribedLocator } from './utils'
56

@@ -21,7 +22,9 @@ export const upload: UserEventCommand<(element: string, files: Array<string | {
2122

2223
const playwrightFiles = files.map((file) => {
2324
if (typeof file === 'string') {
24-
return resolve(root, file)
25+
const filepath = resolve(root, file)
26+
assertBrowserFileAccess(context.project, filepath)
27+
return filepath
2528
}
2629
return {
2730
name: file.name,

packages/browser-webdriverio/src/commands/screenshot.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { BrowserCommandContext } from 'vitest/node'
33
import crypto from 'node:crypto'
44
import { mkdir, rm } from 'node:fs/promises'
55
import { normalize as platformNormalize } from 'node:path'
6-
import { resolveScreenshotPath } from '@vitest/browser'
6+
import { assertBrowserApiWrite, assertBrowserFileAccess, resolveScreenshotPath } from '@vitest/browser'
77
import { dirname, normalize, resolve } from 'pathe'
88

99
interface ScreenshotCommandOptions extends Omit<ScreenshotOptions, 'element' | 'mask'> {
@@ -41,6 +41,9 @@ export async function takeScreenshot(
4141
if (options.save) {
4242
savePath = normalize(path)
4343

44+
assertBrowserApiWrite(context.project, savePath)
45+
assertBrowserFileAccess(context.project, savePath)
46+
4447
await mkdir(dirname(savePath), { recursive: true })
4548
}
4649

packages/browser-webdriverio/src/commands/upload.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { UserEventUploadOptions } from 'vitest/browser'
22
import type { UserEventCommand } from './utils'
3+
import { assertBrowserFileAccess } from '@vitest/browser'
34
import { resolve } from 'pathe'
45

56
export const upload: UserEventCommand<(element: string, files: Array<string | {
@@ -28,6 +29,7 @@ export const upload: UserEventCommand<(element: string, files: Array<string | {
2829

2930
for (const file of files) {
3031
const filepath = resolve(root, file as string)
32+
assertBrowserFileAccess(context.project, filepath)
3133
const remoteFilePath = await context.browser.uploadFile(filepath)
3234
await element.addValue(remoteFilePath)
3335
}

packages/browser/src/node/commands/fs.ts

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,15 @@
11
import type { BrowserCommands } from 'vitest/browser'
2-
import type { BrowserCommand, TestProject } from 'vitest/node'
2+
import type { BrowserCommand } from 'vitest/node'
33
import fs, { promises as fsp } from 'node:fs'
44
import { basename, dirname, resolve } from 'node:path'
55
import mime from 'mime/lite'
6-
import { isFileLoadingAllowed } from 'vitest/node'
7-
import { slash } from '../utils'
8-
9-
function assertFileAccess(path: string, project: TestProject) {
10-
if (
11-
!isFileLoadingAllowed(project.vite.config, path)
12-
&& !isFileLoadingAllowed(project.vitest.vite.config, path)
13-
) {
14-
throw new Error(
15-
`Access denied to "${path}". See Vite config documentation for "server.fs": https://vitejs.dev/config/server-options.html#server-fs-strict.`,
16-
)
17-
}
18-
}
19-
20-
function assertWrite(path: string, project: TestProject) {
21-
if (!project.config.browser.api.allowWrite || !project.vitest.config.api.allowWrite) {
22-
throw new Error(`Cannot modify file "${path}". File writing is disabled because server is exposed to the internet, see https://vitest.dev/config/browser/api.`)
23-
}
24-
}
6+
import { assertBrowserApiWrite, assertBrowserFileAccess } from '../utils'
257

268
export const readFile: BrowserCommand<
279
Parameters<BrowserCommands['readFile']>
2810
> = async ({ project }, path, options = {}) => {
2911
const filepath = resolve(project.config.root, path)
30-
assertFileAccess(slash(filepath), project)
12+
assertBrowserFileAccess(project, filepath)
3113
// never return a Buffer
3214
if (typeof options === 'object' && !options.encoding) {
3315
options.encoding = 'utf-8'
@@ -38,9 +20,9 @@ export const readFile: BrowserCommand<
3820
export const writeFile: BrowserCommand<
3921
Parameters<BrowserCommands['writeFile']>
4022
> = async ({ project }, path, data, options) => {
41-
assertWrite(path, project)
23+
assertBrowserApiWrite(project, path)
4224
const filepath = resolve(project.config.root, path)
43-
assertFileAccess(slash(filepath), project)
25+
assertBrowserFileAccess(project, filepath)
4426
const dir = dirname(filepath)
4527
if (!fs.existsSync(dir)) {
4628
await fsp.mkdir(dir, { recursive: true })
@@ -51,15 +33,15 @@ export const writeFile: BrowserCommand<
5133
export const removeFile: BrowserCommand<
5234
Parameters<BrowserCommands['removeFile']>
5335
> = async ({ project }, path) => {
54-
assertWrite(path, project)
36+
assertBrowserApiWrite(project, path)
5537
const filepath = resolve(project.config.root, path)
56-
assertFileAccess(slash(filepath), project)
38+
assertBrowserFileAccess(project, filepath)
5739
await fsp.rm(filepath)
5840
}
5941

6042
export const _fileInfo: BrowserCommand<[path: string, encoding: BufferEncoding]> = async ({ project }, path, encoding) => {
6143
const filepath = resolve(project.config.root, path)
62-
assertFileAccess(slash(filepath), project)
44+
assertBrowserFileAccess(project, filepath)
6345
const content = await fsp.readFile(filepath, encoding || 'base64')
6446
return {
6547
content,

packages/browser/src/node/commands/screenshotMatcher/index.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import type { SnapshotUpdateState } from 'vitest'
22
import type { ScreenshotMatcherOptions } from 'vitest/browser'
3-
import type { BrowserCommand, BrowserCommandContext } from 'vitest/node'
3+
import type { BrowserCommand, BrowserCommandContext, TestProject } from 'vitest/node'
44
import type { ScreenshotMatcherArguments, ScreenshotMatcherOutput } from '../../../shared/screenshotMatcher/types'
55
import type { AnyCodec } from './codecs'
66
import type { AnyComparator } from './comparators'
77
import type { TypedArray } from './types'
88
import type { ResolvedOptions } from './utils'
99
import { mkdir, readFile, writeFile } from 'node:fs/promises'
1010
import { basename, dirname } from 'pathe'
11+
import { assertBrowserApiWrite, assertBrowserFileAccess } from '../../utils'
1112
import { asyncTimeout, resolveOptions, takeDecodedScreenshot } from './utils'
1213

1314
/** Decoded image data with dimensions metadata. */
@@ -108,7 +109,7 @@ export const screenshotMatcher: BrowserCommand<ScreenshotMatcherArguments> = asy
108109
comparatorOptions,
109110
})
110111

111-
await performSideEffects(outcome, codec)
112+
await performSideEffects(outcome, codec, context.project)
112113

113114
return buildOutput(outcome, timeout)
114115
}
@@ -230,13 +231,15 @@ async function determineOutcome(
230231
async function performSideEffects(
231232
outcome: MatchOutcome,
232233
codec: AnyCodec,
234+
project: TestProject,
233235
): Promise<void> {
234236
switch (outcome.type) {
235237
case 'missing-reference':
236238
case 'update-reference': {
237239
await writeScreenshot(
238240
outcome.reference.path,
239241
await codec.encode(outcome.reference.image, {}),
242+
project,
240243
)
241244

242245
break
@@ -246,12 +249,14 @@ async function performSideEffects(
246249
await writeScreenshot(
247250
outcome.actual.path,
248251
await codec.encode(outcome.actual.image, {}),
252+
project,
249253
)
250254

251255
if (outcome.diff) {
252256
await writeScreenshot(
253257
outcome.diff.path,
254258
await codec.encode(outcome.diff.image, {}),
259+
project,
255260
)
256261
}
257262

@@ -456,8 +461,10 @@ async function getStableScreenshot({
456461
}
457462

458463
/** Writes encoded images to disk, creating parent directories as needed. */
459-
async function writeScreenshot(path: string, image: TypedArray) {
464+
async function writeScreenshot(path: string, image: TypedArray, project: TestProject) {
460465
try {
466+
assertBrowserApiWrite(project, path)
467+
assertBrowserFileAccess(project, path)
461468
await mkdir(dirname(path), { recursive: true })
462469
await writeFile(path, image)
463470
}

packages/browser/src/node/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export function defineBrowserCommand<T extends unknown[]>(
1818
}
1919

2020
// export type { ProjectBrowser } from './project'
21-
export { parseKeyDef, resolveScreenshotPath } from './utils'
21+
export { assertBrowserApiWrite, assertBrowserFileAccess, parseKeyDef, resolveScreenshotPath } from './utils'
2222

2323
export { asLocator } from 'ivya'
2424

packages/browser/src/node/rpc.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { ServerMockResolver } from '@vitest/mocker/node'
1212
import { extractSourcemapFromFile } from '@vitest/utils/source-map/node'
1313
import { createBirpc } from 'birpc'
1414
import { parse, stringify } from 'flatted'
15-
import { dirname, join } from 'pathe'
15+
import { dirname, join, resolve } from 'pathe'
1616
import { createDebugger, isFileLoadingAllowed, isValidApiRequest } from 'vitest/node'
1717
import { WebSocketServer } from 'ws'
1818

@@ -211,6 +211,19 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject, defaultMocke
211211
)
212212
}
213213
}
214+
else {
215+
// attachment files are copied into `attachmentsDir`, so confine
216+
// client-supplied paths to Vite's `server.fs` boundary
217+
const attachments = artifact.type === 'internal:annotation'
218+
? (artifact.annotation.attachment ? [artifact.annotation.attachment] : [])
219+
: (artifact.attachments ?? [])
220+
for (const attachment of attachments) {
221+
const path = attachment.path
222+
if (path && !path.startsWith('http://') && !path.startsWith('https://')) {
223+
checkFileAccess(resolve(project.config.root, path))
224+
}
225+
}
226+
}
214227

215228
return vitest._testRun.recordArtifact(id, artifact)
216229
},

packages/browser/src/node/utils.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
import { defaultKeyMap } from '@testing-library/user-event/dist/esm/keyboard/keyMap.js'
99
import { parseKeyDef as tlParse } from '@testing-library/user-event/dist/esm/keyboard/parseKeyDef.js'
1010
import { basename, dirname, relative, resolve } from 'pathe'
11+
import { isFileLoadingAllowed } from 'vitest/node'
1112

1213
declare enum DOM_KEY_LOCATION {
1314
STANDARD = 0,
@@ -95,3 +96,23 @@ export async function getBrowserProvider(
9596
export function slash(path: string): string {
9697
return path.replace(/\\/g, '/').replace(/\/+/g, '/')
9798
}
99+
100+
export function assertBrowserFileAccess(project: TestProject, path: string): void {
101+
const normalized = slash(path)
102+
if (
103+
!isFileLoadingAllowed(project.vite.config, normalized)
104+
&& !isFileLoadingAllowed(project.vitest.vite.config, normalized)
105+
) {
106+
throw new Error(
107+
`Access denied to "${path}". See Vite config documentation for "server.fs": https://vitejs.dev/config/server-options.html#server-fs-strict.`,
108+
)
109+
}
110+
}
111+
112+
export function assertBrowserApiWrite(project: TestProject, path: string): void {
113+
if (!project.config.browser.api.allowWrite || !project.vitest.config.api.allowWrite) {
114+
throw new Error(
115+
`Cannot modify file "${path}". File writing is disabled because the server is exposed to the internet, see https://vitest.dev/config/browser/api.`,
116+
)
117+
}
118+
}

0 commit comments

Comments
 (0)