fix(importAnalysis): strip url base before passing as safeModulePaths (#13712)

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>

Author: 3 people

Diff for: packages/vite/src/node/plugins/importAnalysis.ts

@@ -557,7 +557,11 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
             }
 
             // record as safe modules
-            server?.moduleGraph.safeModulesPath.add(fsPathFromUrl(url))
+            // safeModulesPath should not include the base prefix.
+            // See https://github.com/vitejs/vite/issues/9438#issuecomment-1465270409
+            server?.moduleGraph.safeModulesPath.add(
+              fsPathFromUrl(stripBase(url, base)),
+            )
 
             if (url !== specifier) {
               let rewriteDone = false

Diff for: playground/fs-serve/__tests__/base/fs-serve-base.spec.ts

@@ -0,0 +1,104 @@
+import fetch from 'node-fetch'
+import { beforeAll, describe, expect, test } from 'vitest'
+import testJSON from '../../safe.json'
+import { isServe, page, viteTestUrl } from '~utils'
+
+const stringified = JSON.stringify(testJSON)
+
+describe.runIf(isServe)('main', () => {
+  beforeAll(async () => {
+    const srcPrefix = viteTestUrl.endsWith('/') ? '' : '/'
+    await page.goto(viteTestUrl + srcPrefix + 'src/')
+  })
+
+  test('default import', async () => {
+    expect(await page.textContent('.full')).toBe(stringified)
+  })
+
+  test('named import', async () => {
+    expect(await page.textContent('.named')).toBe(testJSON.msg)
+  })
+
+  test('safe fetch', async () => {
+    expect(await page.textContent('.safe-fetch')).toMatch('KEY=safe')
+    expect(await page.textContent('.safe-fetch-status')).toBe('200')
+  })
+
+  test('safe fetch with query', async () => {
+    expect(await page.textContent('.safe-fetch-query')).toMatch('KEY=safe')
+    expect(await page.textContent('.safe-fetch-query-status')).toBe('200')
+  })
+
+  test('safe fetch with special characters', async () => {
+    expect(
+      await page.textContent('.safe-fetch-subdir-special-characters'),
+    ).toMatch('KEY=safe')
+    expect(
+      await page.textContent('.safe-fetch-subdir-special-characters-status'),
+    ).toBe('200')
+  })
+
+  test('unsafe fetch', async () => {
+    expect(await page.textContent('.unsafe-fetch')).toMatch('403 Restricted')
+    expect(await page.textContent('.unsafe-fetch-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')
+  })
+
+  test('unsafe fetch with special characters 2 (#8498)', async () => {
+    expect(await page.textContent('.unsafe-fetch-8498-2')).toBe('')
+    expect(await page.textContent('.unsafe-fetch-8498-2-status')).toBe('404')
+  })
+
+  test('safe fs fetch', async () => {
+    expect(await page.textContent('.safe-fs-fetch')).toBe(stringified)
+    expect(await page.textContent('.safe-fs-fetch-status')).toBe('200')
+  })
+
+  test('safe fs fetch', async () => {
+    expect(await page.textContent('.safe-fs-fetch-query')).toBe(stringified)
+    expect(await page.textContent('.safe-fs-fetch-query-status')).toBe('200')
+  })
+
+  test('safe fs fetch with special characters', async () => {
+    expect(await page.textContent('.safe-fs-fetch-special-characters')).toBe(
+      stringified,
+    )
+    expect(
+      await page.textContent('.safe-fs-fetch-special-characters-status'),
+    ).toBe('200')
+  })
+
+  test('unsafe fs fetch', async () => {
+    expect(await page.textContent('.unsafe-fs-fetch')).toBe('')
+    expect(await page.textContent('.unsafe-fs-fetch-status')).toBe('403')
+  })
+
+  test('unsafe fs fetch with special characters (#8498)', async () => {
+    expect(await page.textContent('.unsafe-fs-fetch-8498')).toBe('')
+    expect(await page.textContent('.unsafe-fs-fetch-8498-status')).toBe('404')
+  })
+
+  test('unsafe fs fetch with special characters 2 (#8498)', async () => {
+    expect(await page.textContent('.unsafe-fs-fetch-8498-2')).toBe('')
+    expect(await page.textContent('.unsafe-fs-fetch-8498-2-status')).toBe('404')
+  })
+
+  test('nested entry', async () => {
+    expect(await page.textContent('.nested-entry')).toBe('foobar')
+  })
+
+  test('denied', async () => {
+    expect(await page.textContent('.unsafe-dotenv')).toBe('404')
+  })
+})
+
+describe('fetch', () => {
+  test('serve with configured headers', async () => {
+    const res = await fetch(viteTestUrl + '/src/')
+    expect(res.headers.get('x-served-by')).toBe('vite')
+  })
+})

Diff for: playground/fs-serve/__tests__/fs-serve.spec.ts

@@ -7,7 +7,8 @@ const stringified = JSON.stringify(testJSON)
 
 describe.runIf(isServe)('main', () => {
   beforeAll(async () => {
-    await page.goto(viteTestUrl + '/src/')
+    const srcPrefix = viteTestUrl.endsWith('/') ? '' : '/'
+    await page.goto(viteTestUrl + srcPrefix + 'src/')
   })
 
   test('default import', async () => {
@@ -66,7 +67,9 @@ describe.runIf(isServe)('main', () => {
     expect(await page.textContent('.safe-fs-fetch-special-characters')).toBe(
       stringified,
     )
-    expect(await page.textContent('.safe-fs-fetch-status')).toBe('200')
+    expect(
+      await page.textContent('.safe-fs-fetch-special-characters-status'),
+    ).toBe('200')
   })
 
   test('unsafe fs fetch', async () => {
@@ -88,10 +91,6 @@ describe.runIf(isServe)('main', () => {
     expect(await page.textContent('.nested-entry')).toBe('foobar')
   })
 
-  test('nested entry', async () => {
-    expect(await page.textContent('.nested-entry')).toBe('foobar')
-  })
-
   test('denied', async () => {
     expect(await page.textContent('.unsafe-dotenv')).toBe('404')
   })

Diff for: playground/fs-serve/package.json

@@ -7,6 +7,9 @@
     "dev": "vite root",
     "build": "vite build root",
     "debug": "node --inspect-brk ../../packages/vite/bin/vite",
-    "preview": "vite preview root"
+    "preview": "vite preview root",
+    "dev:base": "vite root --config ./root/vite.config-base.js",
+    "build:base": "vite build root --config ./root/vite.config-base.js",
+    "preview:base": "vite preview root --config ./root/vite.config-base.js"
   }
 }

Diff for: playground/fs-serve/root/src/index.html

@@ -50,11 +50,26 @@ <h2>Denied</h2>
   import '../../entry'
   import json, { msg } from '../../safe.json'
 
+  function joinUrlSegments(a, b) {
+    if (!a || !b) {
+      return a || b || ''
+    }
+    if (a[a.length - 1] === '/') {
+      a = a.substring(0, a.length - 1)
+    }
+    if (b[0] !== '/') {
+      b = '/' + b
+    }
+    return a + b
+  }
+
   text('.full', JSON.stringify(json))
   text('.named', msg)
 
+  const base = typeof BASE !== 'undefined' ? BASE : ''
+
   // inside allowed dir, safe fetch
-  fetch('/src/safe.txt')
+  fetch(joinUrlSegments(base, '/src/safe.txt'))
     .then((r) => {
       text('.safe-fetch-status', r.status)
       return r.text()
@@ -64,7 +79,7 @@ <h2>Denied</h2>
     })
 
   // inside allowed dir with query, safe fetch
-  fetch('/src/safe.txt?query')
+  fetch(joinUrlSegments(base, '/src/safe.txt?query'))
     .then((r) => {
       text('.safe-fetch-query-status', r.status)
       return r.text()
@@ -74,7 +89,7 @@ <h2>Denied</h2>
     })
 
   // inside allowed dir, safe fetch
-  fetch('/src/subdir/safe.txt')
+  fetch(joinUrlSegments(base, '/src/subdir/safe.txt'))
     .then((r) => {
       text('.safe-fetch-subdir-status', r.status)
       return r.text()
@@ -84,7 +99,12 @@ <h2>Denied</h2>
     })
 
   // inside allowed dir, with special characters, safe fetch
-  fetch('/src/special%20characters%20%C3%A5%C3%A4%C3%B6/safe.txt')
+  fetch(
+    joinUrlSegments(
+      base,
+      '/src/special%20characters%20%C3%A5%C3%A4%C3%B6/safe.txt',
+    ),
+  )
     .then((r) => {
       text('.safe-fetch-subdir-special-characters-status', r.status)
       return r.text()
@@ -94,7 +114,7 @@ <h2>Denied</h2>
     })
 
   // outside of allowed dir, treated as unsafe
-  fetch('/unsafe.txt')
+  fetch(joinUrlSegments(base, '/unsafe.txt'))
     .then((r) => {
       text('.unsafe-fetch-status', r.status)
       return r.text()
@@ -107,7 +127,7 @@ <h2>Denied</h2>
     })
 
   // outside of allowed dir with special characters #8498
-  fetch('/src/%2e%2e%2funsafe%2etxt')
+  fetch(joinUrlSegments(base, '/src/%2e%2e%2funsafe%2etxt'))
     .then((r) => {
       text('.unsafe-fetch-8498-status', r.status)
       return r.text()
@@ -120,7 +140,7 @@ <h2>Denied</h2>
     })
 
   // outside of allowed dir with special characters 2 #8498
-  fetch('/src/%252e%252e%252funsafe%252etxt')
+  fetch(joinUrlSegments(base, '/src/%252e%252e%252funsafe%252etxt'))
     .then((r) => {
       text('.unsafe-fetch-8498-2-status', r.status)
       return r.text()
@@ -133,7 +153,7 @@ <h2>Denied</h2>
     })
 
   // imported before, should be treated as safe
-  fetch('/@fs/' + ROOT + '/safe.json')
+  fetch(joinUrlSegments(base, joinUrlSegments('/@fs/', ROOT) + '/safe.json'))
     .then((r) => {
       text('.safe-fs-fetch-status', r.status)
       return r.json()
@@ -143,7 +163,9 @@ <h2>Denied</h2>
     })
 
   // imported before with query, should be treated as safe
-  fetch('/@fs/' + ROOT + '/safe.json?query')
+  fetch(
+    joinUrlSegments(base, joinUrlSegments('/@fs/', ROOT) + '/safe.json?query'),
+  )
     .then((r) => {
       text('.safe-fs-fetch-query-status', r.status)
       return r.json()
@@ -153,7 +175,7 @@ <h2>Denied</h2>
     })
 
   // not imported before, outside of root, treated as unsafe
-  fetch('/@fs/' + ROOT + '/unsafe.json')
+  fetch(joinUrlSegments(base, joinUrlSegments('/@fs/', ROOT) + '/unsafe.json'))
     .then((r) => {
       text('.unsafe-fs-fetch-status', r.status)
       return r.json()
@@ -166,7 +188,13 @@ <h2>Denied</h2>
     })
 
   // outside root with special characters #8498
-  fetch('/@fs/' + ROOT + '/root/src/%2e%2e%2f%2e%2e%2funsafe%2ejson')
+  fetch(
+    joinUrlSegments(
+      base,
+      joinUrlSegments('/@fs/', ROOT) +
+        '/root/src/%2e%2e%2f%2e%2e%2funsafe%2ejson',
+    ),
+  )
     .then((r) => {
       text('.unsafe-fs-fetch-8498-status', r.status)
       return r.json()
@@ -177,7 +205,11 @@ <h2>Denied</h2>
 
   // outside root with special characters 2 #8498
   fetch(
-    '/@fs/' + ROOT + '/root/src/%252e%252e%252f%252e%252e%252funsafe%252ejson',
+    joinUrlSegments(
+      base,
+      joinUrlSegments('/@fs/', ROOT) +
+        '/root/src/%252e%252e%252f%252e%252e%252funsafe%252ejson',
+    ),
   )
     .then((r) => {
       text('.unsafe-fs-fetch-8498-2-status', r.status)
@@ -189,9 +221,11 @@ <h2>Denied</h2>
 
   // not imported before, inside root with special characters, treated as safe
   fetch(
-    '/@fs/' +
-      ROOT +
-      '/root/src/special%20characters%20%C3%A5%C3%A4%C3%B6/safe.json',
+    joinUrlSegments(
+      base,
+      joinUrlSegments('/@fs/', ROOT) +
+        '/root/src/special%20characters%20%C3%A5%C3%A4%C3%B6/safe.json',
+    ),
   )
     .then((r) => {
       text('.safe-fs-fetch-special-characters-status', r.status)
@@ -202,7 +236,7 @@ <h2>Denied</h2>
     })
 
   // .env, denied by default
-  fetch('/@fs/' + ROOT + '/root/.env')
+  fetch(joinUrlSegments(base, joinUrlSegments('/@fs/', ROOT) + '/root/.env'))
     .then((r) => {
       text('.unsafe-dotenv', r.status)
     })

Diff for: playground/fs-serve/root/vite.config-base.js

@@ -0,0 +1,36 @@
+import path from 'node:path'
+import { defineConfig } from 'vite'
+
+const BASE = '/base/'
+
+export default defineConfig({
+  base: BASE,
+  build: {
+    rollupOptions: {
+      input: {
+        main: path.resolve(__dirname, 'src/index.html'),
+      },
+    },
+  },
+  server: {
+    fs: {
+      strict: true,
+      allow: [path.resolve(__dirname, 'src')],
+    },
+    hmr: {
+      overlay: false,
+    },
+    headers: {
+      'x-served-by': 'vite',
+    },
+  },
+  preview: {
+    headers: {
+      'x-served-by': 'vite',
+    },
+  },
+  define: {
+    ROOT: JSON.stringify(path.dirname(__dirname).replace(/\\/g, '/')),
+    BASE: JSON.stringify(BASE),
+  },
+})