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

Author: sapphi-red

docs/config/server-options.md

@@ -377,6 +377,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/src/node/server/middlewares/static.ts

@@ -8,7 +8,6 @@ import type { ViteDevServer } from '../../server'
 import type { ResolvedConfig } from '../../config'
 import { FS_PREFIX } from '../../constants'
 import {
-  fsPathFromId,
   fsPathFromUrl,
   isFileReadable,
   isImportRequest,
@@ -27,11 +26,16 @@ import {
 } from '../../../shared/utils'
 
 const knownJavascriptExtensionRE = /\.(?:[tj]sx?|[cm][tj]s)$/
+const ERR_DENIED_FILE = 'ERR_DENIED_FILE'
 
 const sirvOptions = ({
+  config,
   getHeaders,
+  disableFsServeCheck,
 }: {
+  config: ResolvedConfig
   getHeaders: () => OutgoingHttpHeaders | undefined
+  disableFsServeCheck?: boolean
 }): Options => {
   return {
     dev: true,
@@ -53,6 +57,22 @@ const sirvOptions = ({
         }
       }
     },
+    shouldServe: disableFsServeCheck
+      ? undefined
+      : (filePath) => {
+          const servingAccessResult = checkLoadingAccess(config, 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'
+          return true
+        },
   }
 }
 
@@ -64,7 +84,9 @@ export function servePublicMiddleware(
   const serve = sirv(
     dir,
     sirvOptions({
+      config: server.config,
       getHeaders: () => server.config.server.headers,
+      disableFsServeCheck: true,
     }),
   )
 
@@ -105,6 +127,7 @@ export function serveStaticMiddleware(
   const serve = sirv(
     dir,
     sirvOptions({
+      config: server.config,
       getHeaders: () => server.config.server.headers,
     }),
   )
@@ -154,16 +177,20 @@ export function serveStaticMiddleware(
     if (resolvedPathname.endsWith('/') && fileUrl[fileUrl.length - 1] !== '/') {
       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
+    }
   }
 }
 
@@ -172,7 +199,10 @@ export function serveRawFsMiddleware(
 ): Connect.NextHandleFunction {
   const serveFromRoot = sirv(
     '/',
-    sirvOptions({ getHeaders: () => server.config.server.headers }),
+    sirvOptions({
+      config: server.config,
+      getHeaders: () => server.config.server.headers,
+    }),
   )
 
   // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
@@ -184,24 +214,20 @@ export function serveRawFsMiddleware(
     if (req.url!.startsWith(FS_PREFIX)) {
       const url = new URL(req.url!, 'http://example.com')
       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()
     }
@@ -210,14 +236,12 @@ 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(
   config: ResolvedConfig,
   url: string,
 ): boolean
-/**
- * @deprecated Use the `isFileServingAllowed(config, url)` signature instead.
- */
 export function isFileServingAllowed(
   url: string,
   server: ViteDevServer,
@@ -259,33 +283,52 @@ export function isFileLoadingAllowed(
   return false
 }
 
-export function ensureServingAccess(
+export function checkLoadingAccess(
+  config: ResolvedConfig,
+  path: string,
+): 'allowed' | 'denied' | 'fallback' {
+  if (isFileLoadingAllowed(config, 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://vite.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

@@ -41,7 +41,7 @@ import {
   ERR_OUTDATED_OPTIMIZED_DEP,
   NULL_BYTE_PLACEHOLDER,
 } from '../../../shared/constants'
-import { ensureServingAccess } from './static'
+import { checkServingAccess, respondWithAccessDenied } from './static'
 
 const debugCache = createDebugger('vite:cache')
 
@@ -60,13 +60,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
 }
 
 /**

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

@@ -478,4 +478,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