fix: apply fs.strict check to HTML files (#20736)

Author: sapphi-red

packages/vite/src/node/__tests__/utils.spec.ts

@@ -15,6 +15,7 @@ import {
   getServerUrlByHost,
   injectQuery,
   isFileReadable,
+  isParentDirectory,
   mergeWithDefaults,
   normalizePath,
   numberToPos,
@@ -44,6 +45,33 @@ describe('bareImportRE', () => {
   })
 })
 
+describe('isParentDirectory', () => {
+  const cases = {
+    '/parent': {
+      '/parent': false,
+      '/parenta': false,
+      '/parent/': true,
+      '/parent/child': true,
+      '/parent/child/child2': true,
+    },
+    '/parent/': {
+      '/parent': false,
+      '/parenta': false,
+      '/parent/': true,
+      '/parent/child': true,
+      '/parent/child/child2': true,
+    },
+  }
+
+  for (const [parent, children] of Object.entries(cases)) {
+    for (const [child, expected] of Object.entries(children)) {
+      test(`isParentDirectory("${parent}", "${child}")`, () => {
+        expect(isParentDirectory(parent, child)).toBe(expected)
+      })
+    }
+  }
+})
+
 describe('injectQuery', () => {
   if (isWindows) {
     // this test will work incorrectly on unix systems

packages/vite/src/node/preview.ts

@@ -26,6 +26,7 @@ import { notFoundMiddleware } from './server/middlewares/notFound'
 import { proxyMiddleware } from './server/middlewares/proxy'
 import {
   getServerUrlByHost,
+  normalizePath,
   resolveHostname,
   resolveServerUrls,
   setupSIGTERMListener,
@@ -263,7 +264,8 @@ export async function preview(
 
   if (config.appType === 'spa' || config.appType === 'mpa') {
     // transform index.html
-    app.use(indexHtmlMiddleware(distDir, server))
+    const normalizedDistDir = normalizePath(distDir)
+    app.use(indexHtmlMiddleware(normalizedDistDir, server))
 
     // handle 404s
     app.use(notFoundMiddleware())

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

@@ -0,0 +1,29 @@
+import { describe, expect, test } from 'vitest'
+import { isUriInFilePath } from '../static'
+
+describe('isUriInFilePath', () => {
+  const cases = {
+    '/parent': {
+      '/parent': true,
+      '/parenta': false,
+      '/parent/': true,
+      '/parent/child': true,
+      '/parent/child/child2': true,
+    },
+    '/parent/': {
+      '/parent': false,
+      '/parenta': false,
+      '/parent/': true,
+      '/parent/child': true,
+      '/parent/child/child2': true,
+    },
+  }
+
+  for (const [parent, children] of Object.entries(cases)) {
+    for (const [child, expected] of Object.entries(children)) {
+      test(`isUriInFilePath("${parent}", "${child}")`, () => {
+        expect(isUriInFilePath(parent, child)).toBe(expected)
+      })
+    }
+  }
+})

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

@@ -35,6 +35,7 @@ import {
   isCSSRequest,
   isDevServer,
   isJSRequest,
+  isParentDirectory,
   joinUrlSegments,
   normalizePath,
   processSrcSetSync,
@@ -48,6 +49,7 @@ import {
   BasicMinimalPluginContext,
   basePluginContextMeta,
 } from '../pluginContainer'
+import { checkLoadingAccess, respondWithAccessDenied } from './static'
 
 interface AssetNode {
   start: number
@@ -454,7 +456,26 @@ export function indexHtmlMiddleware(
       if (isDev && url.startsWith(FS_PREFIX)) {
         filePath = decodeURIComponent(fsPathFromId(url))
       } else {
-        filePath = path.join(root, decodeURIComponent(url))
+        filePath = normalizePath(
+          path.resolve(path.join(root, decodeURIComponent(url))),
+        )
+      }
+
+      if (isDev) {
+        const servingAccessResult = checkLoadingAccess(server.config, filePath)
+        if (servingAccessResult === 'denied') {
+          return respondWithAccessDenied(filePath, server, res)
+        }
+        if (servingAccessResult === 'fallback') {
+          return next()
+        }
+        servingAccessResult satisfies 'allowed'
+      } else {
+        // `server.fs` options does not apply to the preview server.
+        // But we should disallow serving files outside the output directory.
+        if (!isParentDirectory(root, filePath)) {
+          return next()
+        }
       }
 
       if (fs.existsSync(filePath)) {

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

@@ -262,7 +262,7 @@ export function isFileServingAllowed(
   return isFileLoadingAllowed(config, filePath)
 }
 
-function isUriInFilePath(uri: string, filePath: string) {
+export function isUriInFilePath(uri: string, filePath: string): boolean {
   return isSameFileUri(uri, filePath) || isParentDirectory(uri, filePath)
 }
 

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

@@ -76,6 +76,15 @@ describe.runIf(isServe)('main', () => {
       .toBe('403')
   })
 
+  test('unsafe HTML fetch', async () => {
+    await expect
+      .poll(() => page.textContent('.unsafe-fetch-html'))
+      .toMatch('403 Restricted')
+    await expect
+      .poll(() => page.textContent('.unsafe-fetch-html-status'))
+      .toBe('403')
+  })
+
   test('unsafe fetch with special characters (#8498)', async () => {
     await expect.poll(() => page.textContent('.unsafe-fetch-8498')).toBe('')
     await expect
@@ -536,4 +545,18 @@ describe.runIf(isServe)('invalid request', () => {
     )
     expect(response).toContain('HTTP/1.1 403 Forbidden')
   })
+
+  test('should deny request to HTML file outside root by default with relative path', async () => {
+    const response = await sendRawRequest(viteTestUrl, '/../unsafe.html')
+    expect(response).toContain('HTTP/1.1 403 Forbidden')
+  })
+})
+
+describe.runIf(!isServe)('preview HTML', () => {
+  test('unsafe HTML fetch', async () => {
+    await expect.poll(() => page.textContent('.unsafe-fetch-html')).toBe('')
+    await expect
+      .poll(() => page.textContent('.unsafe-fetch-html-status'))
+      .toBe('404')
+  })
 })

playground/fs-serve/root/src/index.html

@@ -19,6 +19,8 @@ <h2>Safe Fetch Subdirectory</h2>
 <h2>Unsafe Fetch</h2>
 <pre class="unsafe-fetch-status"></pre>
 <pre class="unsafe-fetch"></pre>
+<pre class="unsafe-fetch-html-status"></pre>
+<pre class="unsafe-fetch-html"></pre>
 <pre class="unsafe-fetch-8498-status"></pre>
 <pre class="unsafe-fetch-8498"></pre>
 <pre class="unsafe-fetch-8498-2-status"></pre>
@@ -39,6 +41,8 @@ <h2>Safe /@fs/ Fetch</h2>
 <h2>Unsafe /@fs/ Fetch</h2>
 <pre class="unsafe-fs-fetch-status"></pre>
 <pre class="unsafe-fs-fetch"></pre>
+<pre class="unsafe-fs-fetch-html-status"></pre>
+<pre class="unsafe-fs-fetch-html"></pre>
 <pre class="unsafe-fs-fetch-raw-status"></pre>
 <pre class="unsafe-fs-fetch-raw"></pre>
 <pre class="unsafe-fs-fetch-raw-query1-status"></pre>
@@ -149,6 +153,19 @@ <h2>Denied</h2>
       console.error(e)
     })
 
+  // outside of allowed dir, treated as unsafe
+  fetch(joinUrlSegments(base, '/unsafe.html'))
+    .then((r) => {
+      text('.unsafe-fetch-html-status', r.status)
+      return r.text()
+    })
+    .then((data) => {
+      text('.unsafe-fetch-html', data)
+    })
+    .catch((e) => {
+      console.error(e)
+    })
+
   // outside of allowed dir with special characters #8498
   fetch(joinUrlSegments(base, '/src/%2e%2e%2funsafe%2etxt'))
     .then((r) => {
@@ -246,6 +263,19 @@ <h2>Denied</h2>
       console.error(e)
     })
 
+  // not imported before, outside of root, treated as unsafe
+  fetch(joinUrlSegments(base, joinUrlSegments('/@fs/', ROOT) + '/unsafe.html'))
+    .then((r) => {
+      text('.unsafe-fs-fetch-html-status', r.status)
+      return r.text()
+    })
+    .then((data) => {
+      text('.unsafe-fs-fetch-html', data)
+    })
+    .catch((e) => {
+      console.error(e)
+    })
+
   // not imported before, outside of root, treated as unsafe
   fetch(
     joinUrlSegments(

playground/fs-serve/root/unsafe.html

@@ -0,0 +1 @@
+<p>unsafe</p>

playground/fs-serve/unsafe.html

@@ -0,0 +1 @@
+<p>unsafe outside root</p>