fix: apply server.fs check to env transport (#22159) (#22163)

Author: sapphi-red

docs/guide/api-environment-runtimes.md

@@ -304,6 +304,8 @@ function createWorkerEnvironment(name, config, context) {
   const handlerToWorkerListener = new WeakMap()
 
   const workerHotChannel = {
+    // Worker threads post messages are not exposed over the network, skip server.fs checks
+    skipFsCheck: true,
     send: (data) => worker.postMessage(data),
     on: (event, handler) => {
       if (event === 'connection') return

packages/vite/src/node/config.ts

@@ -219,6 +219,7 @@ function defaultCreateClientDevEnvironment(
   return new DevEnvironment(name, config, {
     hot: true,
     transport: context.ws,
+    disableFetchModule: true,
   })
 }
 

packages/vite/src/node/server/environment.ts

@@ -1,6 +1,6 @@
-import type { FetchFunctionOptions, FetchResult } from 'vite/module-runner'
 import type { FSWatcher } from 'dep-types/chokidar'
 import colors from 'picocolors'
+import type { FetchFunctionOptions, FetchResult } from 'vite/module-runner'
 import {
   BaseEnvironment,
   getDefaultResolvedEnvironmentOptions,
@@ -45,6 +45,8 @@ export interface DevEnvironmentContext {
     inlineSourceMap?: boolean
   }
   depsOptimizer?: DepsOptimizer
+  /** @internal used for client environment */
+  disableFetchModule?: boolean
 }
 
 export class DevEnvironment extends BaseEnvironment {
@@ -56,6 +58,10 @@ export class DevEnvironment extends BaseEnvironment {
    * @internal
    */
   _remoteRunnerOptions: DevEnvironmentContext['remoteRunner']
+  /**
+   * @internal
+   */
+  _skipFsCheck: boolean
 
   get pluginContainer(): EnvironmentPluginContainer {
     if (!this._pluginContainer)
@@ -121,6 +127,11 @@ export class DevEnvironment extends BaseEnvironment {
     this._crawlEndFinder = setupOnCrawlEnd()
 
     this._remoteRunnerOptions = context.remoteRunner ?? {}
+    this._skipFsCheck = !!(
+      context.transport &&
+      !(isWebSocketServer in context.transport) &&
+      context.transport.skipFsCheck
+    )
 
     this.hot = context.transport
       ? isWebSocketServer in context.transport
@@ -130,6 +141,9 @@ export class DevEnvironment extends BaseEnvironment {
 
     this.hot.setInvokeHandler({
       fetchModule: (id, importer, options) => {
+        if (context.disableFetchModule) {
+          throw new Error('fetchModule is disabled in this environment')
+        }
         return this.fetchModule(id, importer, options)
       },
     })
@@ -210,12 +224,12 @@ export class DevEnvironment extends BaseEnvironment {
   }
 
   transformRequest(url: string): Promise<TransformResult | null> {
-    return transformRequest(this, url)
+    return transformRequest(this, url, { skipFsCheck: this._skipFsCheck })
   }
 
   async warmupRequest(url: string): Promise<void> {
     try {
-      await this.transformRequest(url)
+      await transformRequest(this, url, { skipFsCheck: true })
     } catch (e) {
       if (
         e?.code === ERR_OUTDATED_OPTIMIZED_DEP ||

packages/vite/src/node/server/hmr.ts

@@ -87,6 +87,11 @@ export type HotChannelListener<T extends string = string> = (
 ) => void
 
 export interface HotChannel<Api = any> {
+  /**
+   * When true, the fs access check is skipped in fetchModule.
+   * Set this for transports that is not exposed over the network.
+   */
+  skipFsCheck?: boolean
   /**
    * Broadcast events to all clients
    */
@@ -1132,6 +1137,7 @@ export function createServerHotChannel(): ServerHotChannel {
   const outsideEmitter = new EventEmitter()
 
   return {
+    skipFsCheck: true,
     send(payload: HotPayload) {
       outsideEmitter.emit('send', payload)
     },

packages/vite/src/node/server/middlewares/transform.ts

@@ -41,7 +41,12 @@ import {
   ERR_OUTDATED_OPTIMIZED_DEP,
   NULL_BYTE_PLACEHOLDER,
 } from '../../../shared/constants'
-import { checkServingAccess, respondWithAccessDenied } from './static'
+import type { ResolvedConfig } from '../../config'
+import {
+  checkLoadingAccess,
+  checkServingAccess,
+  respondWithAccessDenied,
+} from './static'
 
 const debugCache = createDebugger('vite:cache')
 
@@ -80,6 +85,16 @@ function deniedServingAccessForTransform(
   return false
 }
 
+export function isServerAccessDeniedForTransform(
+  config: ResolvedConfig,
+  id: string,
+): boolean {
+  if (rawRE.test(id) || urlRE.test(id) || inlineRE.test(id) || svgRE.test(id)) {
+    return checkLoadingAccess(config, id) !== 'allowed'
+  }
+  return false
+}
+
 /**
  * A middleware that short-circuits the middleware chain to serve cached transformed modules
  */
@@ -266,9 +281,7 @@ export function transformMiddleware(
         // resolve, load and transform using the plugin container
         const result = await transformRequest(environment, url, {
           html: req.headers.accept?.includes('text/html'),
-          allowId(id) {
-            return !deniedServingAccessForTransform(id, server, res, next)
-          },
+          skipFsCheck: environment._skipFsCheck,
         })
         if (result) {
           const depsOptimizer = environment.depsOptimizer
@@ -340,8 +353,24 @@ export function transformMiddleware(
         return next()
       }
       if (e?.code === ERR_DENIED_ID) {
-        // next() is called in ensureServingAccess
-        return
+        const id: string = e.id
+        let servingAccessResult = checkLoadingAccess(
+          server.config,
+          cleanUrl(id),
+        )
+        if (servingAccessResult === 'allowed') {
+          servingAccessResult = checkLoadingAccess(server.config, id)
+        }
+        if (servingAccessResult === 'denied') {
+          respondWithAccessDenied(id, server, res)
+          return true
+        }
+        if (servingAccessResult === 'fallback') {
+          next()
+          return true
+        }
+        servingAccessResult satisfies 'allowed'
+        throw new Error(`Unexpected access result for id ${id}`)
       }
       return next(e)
     }

packages/vite/src/node/server/transformRequest.ts

@@ -20,7 +20,7 @@ import {
 } from '../utils'
 import { ssrTransform } from '../ssr/ssrTransform'
 import { checkPublicFile } from '../publicDir'
-import { cleanUrl, unwrapId } from '../../shared/utils'
+import { cleanUrl, slash, unwrapId } from '../../shared/utils'
 import {
   applySourcemapIgnoreList,
   extractSourcemapFromFile,
@@ -29,6 +29,7 @@ import {
 import { isFileLoadingAllowed } from './middlewares/static'
 import { throwClosedServerError } from './pluginContainer'
 import type { DevEnvironment } from './environment'
+import { isServerAccessDeniedForTransform } from './middlewares/transform'
 
 export const ERR_LOAD_URL = 'ERR_LOAD_URL'
 export const ERR_LOAD_PUBLIC_URL = 'ERR_LOAD_PUBLIC_URL'
@@ -57,9 +58,10 @@ export interface TransformOptions {
    */
   html?: boolean
   /**
+   * Whether to skip the `server.fs` check.
    * @internal
    */
-  allowId?: (id: string) => boolean
+  skipFsCheck?: boolean
 }
 
 // TODO: This function could be moved to the DevEnvironment class.
@@ -253,8 +255,13 @@ async function loadAndTransform(
 
   const moduleGraph = environment.moduleGraph
 
-  if (options.allowId && !options.allowId(id)) {
+  if (
+    !options.skipFsCheck &&
+    id[0] !== '\0' &&
+    isServerAccessDeniedForTransform(config, id)
+  ) {
     const err: any = new Error(`Denied ID ${id}`)
+    err.id = id
     err.code = ERR_DENIED_ID
     throw err
   }
@@ -280,8 +287,8 @@ async function loadAndTransform(
     // only try the fallback if access is allowed, skip for out of root url
     // like /service-worker.js or /api/users
     if (
-      environment.config.consumer === 'server' ||
-      isFileLoadingAllowed(environment.getTopLevelConfig(), file)
+      options.skipFsCheck ||
+      isFileLoadingAllowed(environment.getTopLevelConfig(), slash(file))
     ) {
       try {
         code = await fsp.readFile(file, 'utf-8')

packages/vite/src/node/ssr/__tests__/fixtures/basic/file.js

@@ -0,0 +1 @@
+export default 'ok'

packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts

@@ -376,3 +376,26 @@ test('buildStart before transform', async () => {
     ]
   `)
 })
+
+test('server.fs check is not applied to ssrLoadModule', async () => {
+  const server = await createServer({
+    configFile: false,
+    root,
+    logLevel: 'silent',
+    optimizeDeps: {
+      noDiscovery: true,
+    },
+    server: {
+      fs: {
+        allow: [
+          path.resolve(import.meta.dirname, './fixtures/named-overwrite-all'),
+        ],
+      },
+    },
+  })
+  onTestFinished(() => server.close())
+  await server.environments.ssr.pluginContainer.buildStart({})
+
+  const mod = await server.ssrLoadModule('/fixtures/basic/file.js')
+  expect(mod.default).toBe('ok')
+})

packages/vite/src/node/ssr/runtime/__tests__/fixture-outside.js

@@ -0,0 +1 @@
+export default 'error'

packages/vite/src/node/ssr/runtime/__tests__/server-runtime.spec.ts

@@ -1,5 +1,5 @@
 import { existsSync, readdirSync } from 'node:fs'
-import { posix, win32 } from 'node:path'
+import { posix, resolve, win32 } from 'node:path'
 import { fileURLToPath } from 'node:url'
 import { describe, expect, vi } from 'vitest'
 import { isWindows } from '../../../../shared/utils'
@@ -481,3 +481,18 @@ describe('virtual module hmr', async () => {
     })
   })
 })
+
+describe('server.fs check', async () => {
+  const it = await createModuleRunnerTester({
+    server: {
+      fs: {
+        allow: [resolve(import.meta.dirname, './fixtures/circular')],
+      },
+    },
+  })
+
+  it('it is not applied to the server module runner', async ({ runner }) => {
+    const mod = await runner.import('/fixtures/basic.js')
+    expect(mod.name).toBe('basic')
+  })
+})