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

Author: sapphi-red

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

@@ -10,6 +10,7 @@ import {
   getLocalhostAddressIfDiffersFromDNS,
   injectQuery,
   isFileReadable,
+  isParentDirectory,
   posToNumber,
   processSrcSetSync,
   resolveHostname,
@@ -35,6 +36,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 { indexHtmlMiddleware } from './server/middlewares/indexHtml'
 import { notFoundMiddleware } from './server/middlewares/notFound'
 import { proxyMiddleware } from './server/middlewares/proxy'
 import {
+  normalizePath,
   resolveHostname,
   resolveServerUrls,
   setupSIGTERMListener,
@@ -248,7 +249,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

@@ -34,6 +34,7 @@ import {
   injectQuery,
   isDevServer,
   isJSRequest,
+  isParentDirectory,
   joinUrlSegments,
   normalizePath,
   processSrcSetSync,
@@ -44,6 +45,7 @@ import { checkPublicFile } from '../../publicDir'
 import { isCSSRequest } from '../../plugins/css'
 import { getCodeWithSourcemap, injectSourcesContent } from '../sourcemap'
 import { cleanUrl, unwrapId, wrapId } from '../../../shared/utils'
+import { checkLoadingAccess, respondWithAccessDenied } from './static'
 
 interface AssetNode {
   start: number
@@ -431,7 +433,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, 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 (fsUtils.existsSync(filePath)) {

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

@@ -247,7 +247,7 @@ export function isFileServingAllowed(
   return isFileLoadingAllowed(server, 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

@@ -62,6 +62,15 @@ describe.runIf(isServe)('main', () => {
     expect(await page.textContent('.unsafe-fetch-status')).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 () => {
     expect(await page.textContent('.unsafe-fetch-8498')).toBe('')
     expect(await page.textContent('.unsafe-fetch-8498-status')).toBe('404')
@@ -478,4 +487,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>
@@ -142,6 +146,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) => {
@@ -239,6 +256,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>