fix(dev): denied requests overly (vitejs#20410)

Author: sapphi-red

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

@@ -13,7 +13,7 @@ import {
   isImportRequest,
   isInternalRequest,
   isParentDirectory,
-  isSameFileUri,
+  isSameFilePath,
   normalizePath,
   removeLeadingSlash,
   urlRE,
@@ -262,10 +262,22 @@ export function isFileServingAllowed(
   return isFileLoadingAllowed(config, filePath)
 }
 
-function isUriInFilePath(uri: string, filePath: string) {
-  return isSameFileUri(uri, filePath) || isParentDirectory(uri, filePath)
+/**
+ * Warning: parameters are not validated, only works with normalized absolute paths
+ *
+ * @param targetPath - normalized absolute path
+ * @param filePath - normalized absolute path
+ */
+function isFileInTargetPath(targetPath: string, filePath: string) {
+  return (
+    isSameFilePath(targetPath, filePath) ||
+    isParentDirectory(targetPath, filePath)
+  )
 }
 
+/**
+ * Warning: parameters are not validated, only works with normalized absolute paths
+ */
 export function isFileLoadingAllowed(
   config: ResolvedConfig,
   filePath: string,
@@ -278,7 +290,7 @@ export function isFileLoadingAllowed(
 
   if (config.safeModulePaths.has(filePath)) return true
 
-  if (fs.allow.some((uri) => isUriInFilePath(uri, filePath))) return true
+  if (fs.allow.some((uri) => isFileInTargetPath(uri, filePath))) return true
 
   return false
 }
@@ -298,27 +310,12 @@ export function checkLoadingAccess(
   return 'fallback'
 }
 
-export function checkServingAccess(
-  url: string,
-  server: ViteDevServer,
-): 'allowed' | 'denied' | 'fallback' {
-  if (isFileServingAllowed(url, server)) {
-    return 'allowed'
-  }
-  if (isFileReadable(cleanUrl(url))) {
-    return 'denied'
-  }
-  // if the file doesn't exist, we shouldn't restrict this path as it can
-  // be an API call. Middlewares would issue a 404 if the file isn't handled
-  return 'fallback'
-}
-
 export function respondWithAccessDenied(
-  url: string,
+  id: string,
   server: ViteDevServer,
   res: ServerResponse,
 ): void {
-  const urlMessage = `The request url "${url}" is outside of Vite serving allow list.`
+  const urlMessage = `The request id "${id}" is outside of Vite serving allow list.`
   const hintMessage = `
 ${server.config.server.fs.allow.map((i) => `- ${i}`).join('\n')}
 

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

@@ -34,12 +34,11 @@ import {
   ERR_OUTDATED_OPTIMIZED_DEP,
   NULL_BYTE_PLACEHOLDER,
 } from '../../../shared/constants'
-import { checkServingAccess, respondWithAccessDenied } from './static'
+import { checkLoadingAccess, respondWithAccessDenied } from './static'
 
 const debugCache = createDebugger('vite:cache')
 
 const knownIgnoreList = new Set(['/', '/favicon.ico'])
-const trailingQuerySeparatorsRE = /[?&]+$/
 
 // TODO: consolidate this regex pattern with the url, raw, and inline checks in plugins
 const urlRE = /[?&]url\b/
@@ -48,20 +47,15 @@ const inlineRE = /[?&]inline\b/
 const svgRE = /\.svg\b/
 
 function deniedServingAccessForTransform(
-  url: string,
+  id: string,
   server: ViteDevServer,
   res: ServerResponse,
   next: Connect.NextFunction,
 ) {
-  if (
-    rawRE.test(url) ||
-    urlRE.test(url) ||
-    inlineRE.test(url) ||
-    svgRE.test(url)
-  ) {
-    const servingAccessResult = checkServingAccess(url, server)
+  if (rawRE.test(id) || urlRE.test(id) || inlineRE.test(id) || svgRE.test(id)) {
+    const servingAccessResult = checkLoadingAccess(server.config, id)
     if (servingAccessResult === 'denied') {
-      respondWithAccessDenied(url, server, res)
+      respondWithAccessDenied(id, server, res)
       return true
     }
     if (servingAccessResult === 'fallback') {
@@ -207,22 +201,6 @@ export function transformMiddleware(
         warnAboutExplicitPublicPathInUrl(url)
       }
 
-      const urlWithoutTrailingQuerySeparators = url.replace(
-        trailingQuerySeparatorsRE,
-        '',
-      )
-      if (
-        !url.startsWith('/@id/\0') &&
-        deniedServingAccessForTransform(
-          urlWithoutTrailingQuerySeparators,
-          server,
-          res,
-          next,
-        )
-      ) {
-        return
-      }
-
       if (
         req.headers['sec-fetch-dest'] === 'script' ||
         isJSRequest(url) ||

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

@@ -21,7 +21,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,
@@ -268,7 +268,7 @@ async function loadAndTransform(
     // like /service-worker.js or /api/users
     if (
       environment.config.consumer === 'server' ||
-      isFileLoadingAllowed(environment.getTopLevelConfig(), file)
+      isFileLoadingAllowed(environment.getTopLevelConfig(), slash(file))
     ) {
       try {
         code = await fsp.readFile(file, 'utf-8')

packages/vite/src/node/utils.ts

@@ -285,7 +285,7 @@ export function isParentDirectory(dir: string, file: string): boolean {
  * @param file2 - normalized absolute path
  * @returns true if both files url are identical
  */
-export function isSameFileUri(file1: string, file2: string): boolean {
+export function isSameFilePath(file1: string, file2: string): boolean {
   return (
     file1 === file2 ||
     (isCaseInsensitiveFS && file1.toLowerCase() === file2.toLowerCase())

playground/fs-serve/__tests__/fs-serve.spec.ts

@@ -208,7 +208,7 @@ describe.runIf(isServe)('main', () => {
       .poll(() =>
         page.textContent('.unsafe-fs-fetch-relative-path-after-query-status'),
       )
-      .toBe('403')
+      .toBe('404')
   })
 
   test('nested entry', async () => {