Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: fs-serve import graph awareness #3784

Merged
merged 11 commits into from
Jun 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions docs/config/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -488,12 +488,12 @@ createServer()

Restrict serving files outside of workspace root.

### server.fsServe.root
### server.fsServe.allow

- **Experimental**
- **Type:** `string`
- **Type:** `string[]`

Restrict files that could be served via `/@fs/`. When `server.fsServe.strict` is set to `true`, accessing files outside this directory will result in a 403.
Restrict files that could be served via `/@fs/`. When `server.fsServe.strict` is set to `true`, accessing files outside this directory list will result in a 403.

Vite will search for the root of the potential workspace and use it as default. A valid workspace met the following conditions, otherwise will fallback to the [project root](/guide/#index-html-and-project-root).

Expand All @@ -508,7 +508,9 @@ createServer()
server: {
fsServe: {
// Allow serving files from one level up to the project root
root: '..'
allow: [
'..'
]
}
}
}
Expand Down
32 changes: 32 additions & 0 deletions packages/playground/fs-serve/__tests__/fs-serve.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { isBuild } from '../../testUtils'

const json = require('../safe.json')
const stringified = JSON.stringify(json)

if (!isBuild) {
test('default import', async () => {
expect(await page.textContent('.full')).toBe(stringified)
})

test('named import', async () => {
expect(await page.textContent('.named')).toBe(json.msg)
})

test('safe fetch', async () => {
expect(await page.textContent('.safe-fetch')).toBe(stringified)
expect(await page.textContent('.safe-fetch-status')).toBe('200')
})

test('unsafe fetch', async () => {
expect(await page.textContent('.unsafe-fetch')).toBe('')
expect(await page.textContent('.unsafe-fetch-status')).toBe('403')
})

test('nested entry', async () => {
expect(await page.textContent('.nested-entry')).toBe('foobar')
})
} else {
test('dummy test to make jest happy', async () => {
// Your test suite must contain at least one test.
})
}
5 changes: 5 additions & 0 deletions packages/playground/fs-serve/entry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { msg } from './nested/foo'

export const fullmsg = msg + 'bar'

document.querySelector('.nested-entry').textContent = fullmsg
1 change: 1 addition & 0 deletions packages/playground/fs-serve/nested/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const msg = 'foo'
11 changes: 11 additions & 0 deletions packages/playground/fs-serve/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "test-fs-serve",
"private": true,
"version": "0.0.0",
"scripts": {
"dev": "vite root",
"build": "vite build root",
"debug": "node --inspect-brk ../../vite/bin/vite",
"serve": "vite preview"
}
}
49 changes: 49 additions & 0 deletions packages/playground/fs-serve/root/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<h2>Normal Import</h2>
<pre class="full"></pre>
<pre class="named"></pre>

<h2>Safe Fetch</h2>
<pre class="safe-fetch-status"></pre>
<pre class="safe-fetch"></pre>

<h2>Unsafe Fetch</h2>
<pre class="unsafe-fetch-status"></pre>
<pre class="unsafe-fetch"></pre>

<h2>Nested Entry</h2>
<pre class="nested-entry"></pre>

<script type="module">
import '../entry'
import json, { msg } from '../safe.json'

text('.full', JSON.stringify(json))
text('.named', msg)

// imported before, should be treated as safe
fetch('/@fs/' + ROOT + '/safe.json')
.then((r) => {
text('.safe-fetch-status', r.status)
return r.json()
})
.then((data) => {
text('.safe-fetch', JSON.stringify(data))
})

// not imported before, outside of root, treated as unsafe
fetch('/@fs/' + ROOT + '/unsafe.json')
.then((r) => {
text('.unsafe-fetch-status', r.status)
return r.json()
})
.then((data) => {
text('.unsafe-fetch', JSON.stringify(data))
})
.catch((e) => {
console.error(e)
})

function text(sel, text) {
document.querySelector(sel).textContent = text
}
</script>
19 changes: 19 additions & 0 deletions packages/playground/fs-serve/root/vite.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
const path = require('path')

/**
* @type {import('vite').UserConfig}
*/
module.exports = {
server: {
fsServe: {
root: __dirname,
strict: true
},
hmr: {
overlay: false
}
},
define: {
ROOT: JSON.stringify(path.dirname(__dirname).replace(/\\/g, '/'))
}
}
3 changes: 3 additions & 0 deletions packages/playground/fs-serve/safe.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"msg": "safe"
}
3 changes: 3 additions & 0 deletions packages/playground/fs-serve/unsafe.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"msg": "unsafe"
}
5 changes: 5 additions & 0 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,11 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
)
let url = normalizedUrl

// record as safe modules
server?.moduleGraph.safeModulesPath.add(
cleanUrl(url).slice(4 /* '/@fs'.length */)
)

// rewrite
if (url !== specifier) {
// for optimized cjs deps, support named imports by rewriting named
Expand Down
51 changes: 37 additions & 14 deletions packages/vite/src/node/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
import { timeMiddleware } from './middlewares/time'
import { ModuleGraph, ModuleNode } from './moduleGraph'
import { Connect } from 'types/connect'
import { createDebugger, normalizePath } from '../utils'
import { createDebugger, ensureLeadingSlash, normalizePath } from '../utils'
import { errorMiddleware, prepareError } from './middlewares/error'
import { handleHMRUpdate, HmrOptions, handleFileAddUnlink } from './hmr'
import { openBrowser } from './openBrowser'
Expand All @@ -53,6 +53,7 @@ import { createMissingImporterRegisterFn } from '../optimizer/registerMissing'
import { printServerUrls } from '../logger'
import { resolveHostname } from '../utils'
import { searchForWorkspaceRoot } from './searchRoot'
import { CLIENT_DIR } from '../constants'

export interface ServerOptions {
host?: string | boolean
Expand Down Expand Up @@ -129,28 +130,37 @@ export interface ServerOptions {
}

export interface ResolvedServerOptions extends ServerOptions {
fsServe: Required<FileSystemServeOptions>
fsServe: Required<Omit<FileSystemServeOptions, 'root'>>
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved
}

export interface FileSystemServeOptions {
/**
* Strictly restrict file accessing outside of allowing paths.
*
* Set to `false` to disable the warning
* Default to false at this moment, will enabled by default in the future versions.
*
* @expiremental
* @default undefined
Comment on lines 141 to +144
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the default false or undefined?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undefined. false to disable the warning

Copy link
Member

@Shinigami92 Shinigami92 Jun 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we should change the descriptive text in line 141 to something less confusing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point

*/
strict?: boolean | undefined
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved

/**
*
* @expiremental
* @default false
* @deprecated use `fsServe.allow` instead
*/
strict?: boolean
root?: string

/**
* Restrict accessing files outside this directory will result in a 403.
* Restrict accessing files outside the allowed directories.
*
* Accepts absolute path or a path relative to project root.
* Will try to search up for workspace root by default.
*
* @expiremental
*/
root?: string
allow?: string[]
}

/**
Expand Down Expand Up @@ -484,7 +494,7 @@ export async function createServer(
middlewares.use(transformMiddleware(server))

// serve static files
middlewares.use(serveRawFsMiddleware(config))
middlewares.use(serveRawFsMiddleware(server))
middlewares.use(serveStaticMiddleware(root, config))

// spa fallback
Expand Down Expand Up @@ -695,19 +705,32 @@ function createServerCloseFn(server: http.Server | null) {
})
}

function resolvedAllowDir(root: string, dir: string): string {
return ensureLeadingSlash(normalizePath(path.resolve(root, dir)))
}

export function resolveServerOptions(
root: string,
raw?: ServerOptions
): ResolvedServerOptions {
const server = raw || {}
const fsServeRoot = normalizePath(
path.resolve(root, server.fsServe?.root || searchForWorkspaceRoot(root))
)
// TODO: make strict by default
const fsServeStrict = server.fsServe?.strict ?? false
let allowDirs = server.fsServe?.allow

if (!allowDirs) {
allowDirs = [server.fsServe?.root || searchForWorkspaceRoot(root)]
}
allowDirs = allowDirs.map((i) => resolvedAllowDir(root, i))

// only push client dir when vite itself is outside-of-root
const resolvedClientDir = resolvedAllowDir(root, CLIENT_DIR)
if (!allowDirs.some((i) => resolvedClientDir.startsWith(i))) {
allowDirs.push(resolvedClientDir)
}

server.fsServe = {
root: fsServeRoot,
strict: fsServeStrict
// TODO: make strict by default
strict: server.fsServe?.strict,
allow: allowDirs
}
return server as ResolvedServerOptions
}
2 changes: 1 addition & 1 deletion packages/vite/src/node/server/middlewares/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function errorMiddleware(
}

export class AccessRestrictedError extends Error {
constructor(msg: string, public url: string, public serveRoot: string) {
constructor(msg: string) {
super(msg)
}
}
Expand Down
Loading