fix: backport #19965, check static serve file inside sirv (#19967)

Author: sapphi-red

docs/config/server-options.md

@@ -330,6 +330,12 @@ export default defineConfig({
 
 Blocklist for sensitive files being restricted to be served by Vite dev server. This will have higher priority than [`server.fs.allow`](#server-fs-allow). [picomatch patterns](https://github.com/micromatch/picomatch#globbing-features) are supported.
 
+::: tip NOTE
+
+This blocklist does not apply to [the public directory](/guide/assets.md#the-public-directory). All files in the public directory are served without any filtering, since they are copied directly to the output directory during build.
+
+:::
+
 ## server.origin
 
 - **Type:** `string`

packages/vite/rollup.config.ts

@@ -195,7 +195,7 @@ function createCjsConfig(isProduction: boolean) {
       ...Object.keys(pkg.dependencies),
       ...(isProduction ? [] : Object.keys(pkg.devDependencies)),
     ],
-    plugins: [...createNodePlugins(false, false, false), bundleSizeLimit(120)],
+    plugins: [...createNodePlugins(false, false, false), bundleSizeLimit(121)],
   })
 }
 

packages/vite/src/node/publicUtils.ts

@@ -15,5 +15,8 @@ export { normalizePath, mergeConfig, mergeAlias, createFilter } from './utils'
 export { send } from './server/send'
 export { createLogger } from './logger'
 export { searchForWorkspaceRoot } from './server/searchRoot'
-export { isFileServingAllowed } from './server/middlewares/static'
+export {
+  isFileServingAllowed,
+  isFileLoadingAllowed,
+} from './server/middlewares/static'
 export { loadEnv, resolveEnvPrefix } from './env'

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

@@ -668,7 +668,7 @@ export async function _createServer(
   // as-is without transforms.
   if (config.publicDir) {
     middlewares.use(
-      servePublicMiddleware(config.publicDir, config.server.headers),
+      servePublicMiddleware(config.publicDir, server, config.server.headers),
     )
   }
 

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

@@ -8,7 +8,6 @@ import type { ViteDevServer } from '../..'
 import { FS_PREFIX } from '../../constants'
 import {
   cleanUrl,
-  fsPathFromId,
   fsPathFromUrl,
   isFileReadable,
   isImportRequest,
@@ -23,13 +22,18 @@ import {
 } from '../../utils'
 
 const knownJavascriptExtensionRE = /\.[tj]sx?$/
+const ERR_DENIED_FILE = 'ERR_DENIED_FILE'
 
 const sirvOptions = ({
+  server,
   headers,
   shouldServe,
+  disableFsServeCheck,
 }: {
+  server: ViteDevServer
   headers?: OutgoingHttpHeaders
   shouldServe?: (p: string) => void
+  disableFsServeCheck?: boolean
 }): Options => {
   return {
     dev: true,
@@ -50,19 +54,40 @@ const sirvOptions = ({
         }
       }
     },
-    shouldServe,
+    shouldServe: disableFsServeCheck
+      ? shouldServe
+      : (filePath) => {
+          const servingAccessResult = checkLoadingAccess(server, filePath)
+          if (servingAccessResult === 'denied') {
+            const error: any = new Error('denied access')
+            error.code = ERR_DENIED_FILE
+            error.path = filePath
+            throw error
+          }
+          if (servingAccessResult === 'fallback') {
+            return false
+          }
+          servingAccessResult satisfies 'allowed'
+          if (shouldServe) {
+            return shouldServe(filePath)
+          }
+          return true
+        },
   }
 }
 
 export function servePublicMiddleware(
   dir: string,
+  server: ViteDevServer,
   headers?: OutgoingHttpHeaders,
 ): Connect.NextHandleFunction {
   const serve = sirv(
     dir,
     sirvOptions({
+      server,
       headers,
       shouldServe: (filePath) => shouldServeFile(filePath, dir),
+      disableFsServeCheck: true,
     }),
   )
 
@@ -83,6 +108,7 @@ export function serveStaticMiddleware(
   const serve = sirv(
     dir,
     sirvOptions({
+      server,
       headers: server.config.server.headers,
     }),
   )
@@ -132,16 +158,20 @@ export function serveStaticMiddleware(
     ) {
       fileUrl = withTrailingSlash(fileUrl)
     }
-    if (!ensureServingAccess(fileUrl, server, res, next)) {
-      return
-    }
-
     if (redirectedPathname) {
       url.pathname = encodeURI(redirectedPathname)
       req.url = url.href.slice(url.origin.length)
     }
 
-    serve(req, res, next)
+    try {
+      serve(req, res, next)
+    } catch (e) {
+      if (e && 'code' in e && e.code === ERR_DENIED_FILE) {
+        respondWithAccessDenied(e.path, server, res)
+        return
+      }
+      throw e
+    }
   }
 }
 
@@ -150,7 +180,10 @@ export function serveRawFsMiddleware(
 ): Connect.NextHandleFunction {
   const serveFromRoot = sirv(
     '/',
-    sirvOptions({ headers: server.config.server.headers }),
+    sirvOptions({
+      server,
+      headers: server.config.server.headers,
+    }),
   )
 
   // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
@@ -162,24 +195,20 @@ export function serveRawFsMiddleware(
     // searching based from fs root.
     if (url.pathname.startsWith(FS_PREFIX)) {
       const pathname = decodeURI(url.pathname)
-      // restrict files outside of `fs.allow`
-      if (
-        !ensureServingAccess(
-          slash(path.resolve(fsPathFromId(pathname))),
-          server,
-          res,
-          next,
-        )
-      ) {
-        return
-      }
-
       let newPathname = pathname.slice(FS_PREFIX.length)
       if (isWindows) newPathname = newPathname.replace(/^[A-Z]:/i, '')
-
       url.pathname = encodeURI(newPathname)
       req.url = url.href.slice(url.origin.length)
-      serveFromRoot(req, res, next)
+
+      try {
+        serveFromRoot(req, res, next)
+      } catch (e) {
+        if (e && 'code' in e && e.code === ERR_DENIED_FILE) {
+          respondWithAccessDenied(e.path, server, res)
+          return
+        }
+        throw e
+      }
     } else {
       next()
     }
@@ -188,56 +217,85 @@ export function serveRawFsMiddleware(
 
 /**
  * Check if the url is allowed to be served, via the `server.fs` config.
+ * @deprecated Use the `isFileLoadingAllowed` function instead.
  */
 export function isFileServingAllowed(
   url: string,
   server: ViteDevServer,
 ): boolean {
   if (!server.config.server.fs.strict) return true
 
-  const file = fsPathFromUrl(url)
+  const filePath = fsPathFromUrl(url)
+  return isFileLoadingAllowed(server, filePath)
+}
+
+function isUriInFilePath(uri: string, filePath: string) {
+  return isSameFileUri(uri, filePath) || isParentDirectory(uri, filePath)
+}
+
+export function isFileLoadingAllowed(
+  server: ViteDevServer,
+  filePath: string,
+): boolean {
+  const { fs } = server.config.server
 
-  if (server._fsDenyGlob(file)) return false
+  if (!fs.strict) return true
 
-  if (server.moduleGraph.safeModulesPath.has(file)) return true
+  if (server._fsDenyGlob(filePath)) return false
 
-  if (
-    server.config.server.fs.allow.some(
-      (uri) => isSameFileUri(uri, file) || isParentDirectory(uri, file),
-    )
-  )
-    return true
+  if (server.moduleGraph.safeModulesPath.has(filePath)) return true
+
+  if (fs.allow.some((uri) => isUriInFilePath(uri, filePath))) return true
 
   return false
 }
 
-export function ensureServingAccess(
+export function checkLoadingAccess(
+  server: ViteDevServer,
+  path: string,
+): 'allowed' | 'denied' | 'fallback' {
+  if (isFileLoadingAllowed(server, slash(path))) {
+    return 'allowed'
+  }
+  if (isFileReadable(path)) {
+    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 checkServingAccess(
   url: string,
   server: ViteDevServer,
-  res: ServerResponse,
-  next: Connect.NextFunction,
-): boolean {
+): 'allowed' | 'denied' | 'fallback' {
   if (isFileServingAllowed(url, server)) {
-    return true
+    return 'allowed'
   }
   if (isFileReadable(cleanUrl(url))) {
-    const urlMessage = `The request url "${url}" is outside of Vite serving allow list.`
-    const hintMessage = `
+    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,
+  server: ViteDevServer,
+  res: ServerResponse,
+): void {
+  const urlMessage = `The request url "${url}" is outside of Vite serving allow list.`
+  const hintMessage = `
 ${server.config.server.fs.allow.map((i) => `- ${i}`).join('\n')}
 
 Refer to docs https://vitejs.dev/config/server-options.html#server-fs-allow for configurations and more details.`
 
-    server.config.logger.error(urlMessage)
-    server.config.logger.warnOnce(hintMessage + '\n')
-    res.statusCode = 403
-    res.write(renderRestrictedErrorHTML(urlMessage + '\n' + hintMessage))
-    res.end()
-  } else {
-    // 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
-    next()
-  }
-  return false
+  server.config.logger.error(urlMessage)
+  server.config.logger.warnOnce(hintMessage + '\n')
+  res.statusCode = 403
+  res.write(renderRestrictedErrorHTML(urlMessage + '\n' + hintMessage))
+  res.end()
 }
 
 function renderRestrictedErrorHTML(msg: string): string {

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

@@ -43,7 +43,7 @@ import {
 } from '../../plugins/optimizedDeps'
 import { ERR_CLOSED_SERVER } from '../pluginContainer'
 import { getDepsOptimizer } from '../../optimizer'
-import { ensureServingAccess } from './static'
+import { checkServingAccess, respondWithAccessDenied } from './static'
 
 const debugCache = createDebugger('vite:cache')
 
@@ -62,13 +62,24 @@ function deniedServingAccessForTransform(
   res: ServerResponse,
   next: Connect.NextFunction,
 ) {
-  return (
-    (rawRE.test(url) ||
-      urlRE.test(url) ||
-      inlineRE.test(url) ||
-      svgRE.test(url)) &&
-    !ensureServingAccess(url, server, res, next)
-  )
+  if (
+    rawRE.test(url) ||
+    urlRE.test(url) ||
+    inlineRE.test(url) ||
+    svgRE.test(url)
+  ) {
+    const servingAccessResult = checkServingAccess(url, server)
+    if (servingAccessResult === 'denied') {
+      respondWithAccessDenied(url, server, res)
+      return true
+    }
+    if (servingAccessResult === 'fallback') {
+      next()
+      return true
+    }
+    servingAccessResult satisfies 'allowed'
+  }
+  return false
 }
 
 export function transformMiddleware(

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

@@ -466,4 +466,17 @@ describe.runIf(isServe)('invalid request', () => {
     )
     expect(response).toContain('HTTP/1.1 400 Bad Request')
   })
+
+  test('should deny request to denied file when a request has /.', async () => {
+    const response = await sendRawRequest(viteTestUrl, '/src/dummy.crt/.')
+    expect(response).toContain('HTTP/1.1 403 Forbidden')
+  })
+
+  test('should deny request with /@fs/ to denied file when a request has /.', async () => {
+    const response = await sendRawRequest(
+      viteTestUrl,
+      path.posix.join('/@fs/', root, 'root/src/dummy.crt/') + '.',
+    )
+    expect(response).toContain('HTTP/1.1 403 Forbidden')
+  })
 })

playground/fs-serve/root/src/dummy.crt

@@ -0,0 +1 @@
+secret