fix: restrict files logic (fix #5416)

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

@@ -24,8 +24,8 @@ describe('main', () => {
     })
 
     test('unsafe fetch', async () => {
-      expect(await page.textContent('.unsafe-fetch')).toMatch('403 Restricted')
-      expect(await page.textContent('.unsafe-fetch-status')).toBe('403')
+      expect(await page.textContent('.unsafe-fetch')).toMatch('')
+      expect(await page.textContent('.unsafe-fetch-status')).toBe('404')
     })
 
     test('safe fs fetch', async () => {
@@ -35,7 +35,7 @@ describe('main', () => {
 
     test('unsafe fs fetch', async () => {
       expect(await page.textContent('.unsafe-fs-fetch')).toBe('')
-      expect(await page.textContent('.unsafe-fs-fetch-status')).toBe('403')
+      expect(await page.textContent('.unsafe-fs-fetch-status')).toBe('404')
     })
 
     test('nested entry', async () => {

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

@@ -68,23 +68,12 @@ export function errorMiddleware(
     if (allowNext) {
       next()
     } else {
-      if (err instanceof AccessRestrictedError) {
-        res.statusCode = 403
-        res.write(renderErrorHTML(err.message))
-        res.end()
-      }
       res.statusCode = 500
       res.end()
     }
   }
 }
 
-export class AccessRestrictedError extends Error {
-  constructor(msg: string) {
-    super(msg)
-  }
-}
-
 export function renderErrorHTML(msg: string): string {
   // to have syntax highlighting and autocompletion in IDE
   const html = String.raw

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

@@ -1,4 +1,5 @@
 import path from 'path'
+import fs from 'fs'
 import sirv, { Options } from 'sirv'
 import { Connect } from 'types/connect'
 import { normalizePath, ViteDevServer } from '../..'
@@ -12,7 +13,6 @@ import {
   isWindows,
   slash
 } from '../../utils'
-import { AccessRestrictedError } from './error'
 
 const sirvOptions: Options = {
   dev: true,
@@ -51,11 +51,12 @@ export function serveStaticMiddleware(
 
   // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
   return function viteServeStaticMiddleware(req, res, next) {
-    // only serve the file if it's not an html request
+    // only serve the file if it's not an html request or ends with /
     // so that html requests can fallthrough to our html middleware for
     // special processing
     // also skip internal requests `/@fs/ /@vite-client` etc...
     if (
+      req.url!.endsWith('/') ||
       path.extname(cleanUrl(req.url!)) === '.html' ||
       isInternalRequest(req.url!)
     ) {
@@ -86,8 +87,13 @@ export function serveStaticMiddleware(
     if (resolvedUrl.endsWith('/') && !fileUrl.endsWith('/')) {
       fileUrl = fileUrl + '/'
     }
-    ensureServingAccess(fileUrl, server)
-
+    // If the file exist but is restricted, ignore this request
+    // This will generate a 403 if it isn't handled by other middlewares
+    // We avoid a hard 404 here to avoid leaking file names information
+    if (isFileServingRestricted(fileUrl, server)) {
+      return next()
+    }
+
     if (redirected) {
       req.url = redirected
     }
@@ -109,8 +115,13 @@ export function serveRawFsMiddleware(
     // the paths are rewritten to `/@fs/` prefixed paths and must be served by
     // searching based from fs root.
     if (url.startsWith(FS_PREFIX)) {
-      // restrict files outside of `fs.allow`
-      ensureServingAccess(slash(path.resolve(fsPathFromId(url))), server)
+      // If the file exist but is restricted, ignore this request
+      // This will generate a 403 if it isn't handled by other middlewares
+      // We avoid a hard 404 here to avoid leaking file names information
+      if (isFileServingRestricted(slash(path.resolve(fsPathFromId(url))), server)) {
+        next()
+      }
+
       url = url.slice(FS_PREFIX.length)
       if (isWindows) url = url.replace(/^[A-Z]:/i, '')
 
@@ -122,42 +133,45 @@ export function serveRawFsMiddleware(
   }
 }
 
-export function isFileServingAllowed(
+export function isFileServingRestricted(
   url: string,
   server: ViteDevServer
 ): boolean {
   // explicitly disabled
-  if (server.config.server.fs.strict === false) return true
+  if (server.config.server.fs.strict === false) return false
 
   const file = ensureLeadingSlash(normalizePath(cleanUrl(url)))
+
+  // 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
+  if (fs.existsSync(file)) return false
 
-  if (server.moduleGraph.safeModulesPath.has(file)) return true
+  if (server.moduleGraph.safeModulesPath.has(file)) return false
 
-  if (server.config.server.fs.allow.some((i) => file.startsWith(i + '/')))
-    return true
+  const { allow } = server.config.server.fs
+  if (allow.some((i) => file.startsWith(i + '/')))
+    return false
 
   if (!server.config.server.fs.strict) {
     server.config.logger.warnOnce(`Unrestricted file system access to "${url}"`)
     server.config.logger.warnOnce(
       `For security concerns, accessing files outside of serving allow list will ` +
-        `be restricted by default in the future version of Vite. ` +
-        `Refer to https://vitejs.dev/config/#server-fs-allow for more details.`
+      `be restricted by default in the future version of Vite. ` +
+      `Refer to https://vitejs.dev/config/#server-fs-allow for more details.`
     )
-    return true
+    return false
   }
 
-  return false
-}
-
-export function ensureServingAccess(url: string, server: ViteDevServer): void {
-  if (!isFileServingAllowed(url, server)) {
-    const allow = server.config.server.fs.allow
-    throw new AccessRestrictedError(
-      `The request url "${url}" is outside of Vite serving allow list:
+  // We avoid a hard 403 here to avoid leaking file names information
+  // A warn message is logged to the console instead, and the browser
+  // will end up seeing a 404
+  server.config.logger.warn(
+    `The request url "${url}" is outside of Vite serving allow list:
 
 ${allow.map((i) => `- ${i}`).join('\n')}
 
 Refer to docs https://vitejs.dev/config/#server-fs-allow for configurations and more details.`
-    )
-  }
+  )
+
+  return true
 }

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

@@ -17,7 +17,7 @@ import {
 import { checkPublicFile } from '../plugins/asset'
 import { ssrTransform } from '../ssr/ssrTransform'
 import { injectSourcesContent } from './sourcemap'
-import { isFileServingAllowed } from './middlewares/static'
+import { isFileServingRestricted } from './middlewares/static'
 import { performance } from 'perf_hooks'
 
 const debugLoad = createDebugger('vite:load')
@@ -107,7 +107,7 @@ async function doTransform(
     // as string
     // only try the fallback if access is allowed, skip for out of root url
     // like /service-worker.js or /api/users
-    if (options.ssr || isFileServingAllowed(file, server)) {
+    if (options.ssr || !isFileServingRestricted(file, server)) {
       try {
         code = await fs.readFile(file, 'utf-8')
         isDebug && debugLoad(`${timeFrom(loadStart)} [fs] ${prettyUrl}`)