fix: prevent serving unrestricted files (fix #2820) #2850

antfu · 2021-04-04T06:14:22Z

Description

fix #2820

Additional context

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

Before submitting the PR, please make sure you do the following

Read the Contributing Guidelines.
Read the Pull Request Guidelines and follow the Commit Convention.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Ideally, include relevant tests that fail without this PR but pass with it.

vitejs#2820)

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

packages/vite/src/node/server/index.ts

Co-authored-by: Shinigami <chrissi92@hotmail.de>

patak-dev · 2021-04-04T18:45:00Z

Thanks for pushing for this change @antfu. Some initial thoughts.

@marvinhagemeister said they are planning to support an array of folders, instead of only one fsServeRoot. #2820 (comment). I don't know exactly what would be the use case for this? Maybe in a monorepo, you only would like to allow for a few internal packages and not all of them?

I think that it would be good to have an option to support common monorepos, maybe behind an option. But this to me should be pushed to another PR, as it is not related to the security issue at hand.

docs/config/index.md

packages/vite/src/node/server/index.ts

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

patak-dev · 2021-04-04T20:36:09Z

About support for an array of serve roots, that could be added later if needed and we can merge this PR without it. We could think if fsServeRoot is a good name for string | string[] or if we should name it fsServeRoots to match other options like entries (that can also be string | string[]). I don't have a strong position on this one.

Co-authored-by: patak <matias.capeletto@gmail.com>

patak-dev · 2021-04-05T08:26:43Z

@antfu tests are failing because of missing imports from utils normalizePath and fsPathFromId, GitHub didn't allow me to add them in the suggestion

antfu · 2021-04-05T08:55:30Z

Added

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

antfu · 2021-04-10T01:46:54Z

Converting to draft for now as we need to have some rework on this to provide better DX for users to migrate this change.

patak-dev · 2021-05-05T18:35:47Z

Tested locally on Windows and is working as expected 👍🏼

Co-authored-by: patak <matias.capeletto@gmail.com>

Co-authored-by: Shinigami <chrissi92@hotmail.de>

Shinigami92

thoughts (non-blocking): Okay, so if understand everything correct, we are currently provide a new option to set the root and no upper files are allowed to be accessed.

In Mono-Repos we have some strategies to find the root automatically, but for non-Mono-Repos we don't have such.

But the fsServeRoot is optional and therefore this fix doesn't apply at all for non-Mono-Repos, does it?
If so, I would highly suggest to search for just e.g. package.json or .git and let the user/developer define its own fsServeRoot to override it and give the responsibility to them (the user).
So we are strict, but it's possible to relax the strictness.

What are your thoughts about this?

antfu · 2021-05-07T07:34:51Z

@Shinigami92 Sorry wdym? For non-monorepos, it's limited to the project root.

Shinigami92 · 2021-05-07T08:06:41Z

@Shinigami92 Sorry wdym? For non-monorepos, it's limited to the project root.

Ah, so this is this: [...] otherwise will fallback to the [project root](/guide/#index-html-and-project-root)

Okay 🙂

antfu · 2021-05-07T08:19:04Z

packages/vite/src/node/server/index.ts

+   * Accepts absolute path or a path relative to project root.
+   * Will try to search up for workspace root by default.
+   */
+  root?: string


As discussion with @patak-js, we are likely to add more options to configure /@fs/ to enhance further security in the future. For example, disabling dotfiles (.env) by default (e.g. sirv). Changed this to an object for further extensions.

…ng Vite (> 2.2.4) Although it shouldn't cause problems for most Vite Ruby projects, after vitejs/vite#2850 files outside the Vite root would not be served by the dev server middleware. By explicitly setting `server.fsServe.root`, we ensure that every file in the project can be served raw if needed.

Co-authored-by: Shinigami <chrissi92@hotmail.de> Co-authored-by: patak <matias.capeletto@gmail.com>

aleclarson · 2022-11-14T22:22:13Z

packages/vite/src/node/server/searchRoot.ts

@@ -4,7 +4,7 @@ import { join } from 'path'

 // https://github.com/vitejs/vite/issues/2820#issuecomment-812495079
 const ROOT_FILES = [
-  '.git',
+  // '.git',


What's the explanation for this?

fix: new options fsServeRoot to prevent serving unrestricted files (fix

948d493

vitejs#2820)

antfu changed the title ~~fix: new options fsServeRoot to prevent serving unrestricted files (f…~~ fix: new options fsServeRoot to prevent serving unrestricted files (f… Apr 4, 2021

chore: update jsdoc

d905918

antfu added p5-urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) security labels Apr 4, 2021

antfu changed the title ~~fix: new options fsServeRoot to prevent serving unrestricted files (f…~~ fix!: prevent serving unrestricted files (fix #2820) Apr 4, 2021

Shinigami92 reviewed Apr 4, 2021

View reviewed changes

packages/vite/src/node/server/middlewares/static.ts Outdated Show resolved Hide resolved

packages/vite/src/node/server/index.ts Outdated Show resolved Hide resolved

antfu and others added 2 commits April 4, 2021 18:35

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

d4dd106

Co-authored-by: Shinigami <chrissi92@hotmail.de>

chore: update docs

f47345b

antfu marked this pull request as ready for review April 4, 2021 12:08

Shinigami92 reviewed Apr 4, 2021

View reviewed changes

docs/config/index.md Outdated Show resolved Hide resolved

packages/vite/src/node/server/index.ts Outdated Show resolved Hide resolved

Apply suggestions from code review

a0e0221

Shinigami92 previously approved these changes Apr 4, 2021

View reviewed changes

patak-dev requested changes Apr 4, 2021

View reviewed changes

packages/vite/src/node/server/middlewares/static.ts Outdated Show resolved Hide resolved

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

7066730

Co-authored-by: patak <matias.capeletto@gmail.com>

antfu dismissed Shinigami92’s stale review via 7066730 April 5, 2021 07:29

chore: fix imports

34334d9

patak-dev previously approved these changes Apr 5, 2021

View reviewed changes

Shinigami92 previously approved these changes Apr 5, 2021

View reviewed changes

patak-dev mentioned this pull request Apr 5, 2021

Unrestricted directory traversal with @fs #2820

Closed

mohe2015 reviewed Apr 8, 2021

View reviewed changes

packages/vite/src/node/server/middlewares/static.ts Outdated Show resolved Hide resolved

antfu marked this pull request as draft April 10, 2021 01:45

antfu added p4-important Violate documented behavior or significantly improves performance (priority) and removed p5-urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) labels Apr 10, 2021

patak-dev mentioned this pull request Apr 16, 2021

test: revert Prevent regressions in static file serving of files outside the root (#2358) #3015

Merged

9 tasks

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

93cf900

Co-authored-by: patak <matias.capeletto@gmail.com>

antfu dismissed Shinigami92’s stale review via 93cf900 May 5, 2021 18:47

antfu and others added 2 commits May 6, 2021 02:48

Update packages/vite/src/node/server/searchRoot.ts

0501f3d

Co-authored-by: Shinigami <chrissi92@hotmail.de>

chore: remove detection for .git

099ce97

antfu requested review from patak-dev and Shinigami92 May 7, 2021 06:06

patak-dev previously approved these changes May 7, 2021

View reviewed changes

Shinigami92 previously approved these changes May 7, 2021

View reviewed changes

chore: change options to fsServe object

f771868

antfu dismissed stale reviews from Shinigami92 and patak-dev via f771868 May 7, 2021 08:15

antfu commented May 7, 2021

View reviewed changes

Merge branch 'main' into fix/fs-root

8bed0d8

antfu requested review from patak-dev and Shinigami92 May 7, 2021 08:19

fix: expose type

7b7bef9

patak-dev approved these changes May 7, 2021

View reviewed changes

Shinigami92 approved these changes May 7, 2021

View reviewed changes

antfu changed the title ~~fix!: prevent serving unrestricted files (fix #2820)~~ fix: prevent serving unrestricted files (fix #2820) May 7, 2021

antfu merged commit 792a6e1 into vitejs:main May 7, 2021

antfu deleted the fix/fs-root branch May 7, 2021 10:36

This was referenced May 7, 2021

fix: disable dotfiles serving via /@fs/ by default #3298

Closed

path traversal vulnerability #3281

Closed

meteorlxy mentioned this pull request May 12, 2021

[Feature Request] Internal allowlist of @fs serving files #3373

Closed

fi3ework pushed a commit to fi3ework/vite that referenced this pull request May 22, 2021

fix: prevent serving unrestricted files (fix vitejs#2820) (vitejs#2850)

d632567

Co-authored-by: Shinigami <chrissi92@hotmail.de> Co-authored-by: patak <matias.capeletto@gmail.com>

aleclarson reviewed Nov 14, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prevent serving unrestricted files (fix #2820) #2850

fix: prevent serving unrestricted files (fix #2820) #2850

antfu commented Apr 4, 2021 •

edited

Loading

patak-dev commented Apr 4, 2021

patak-dev commented Apr 4, 2021

patak-dev commented Apr 5, 2021

antfu commented Apr 5, 2021

antfu commented Apr 10, 2021

patak-dev commented May 5, 2021

Shinigami92 left a comment

antfu commented May 7, 2021

Shinigami92 commented May 7, 2021

antfu May 7, 2021

aleclarson Nov 14, 2022

fix: prevent serving unrestricted files (fix #2820) #2850

fix: prevent serving unrestricted files (fix #2820) #2850

Conversation

antfu commented Apr 4, 2021 • edited Loading

Description

Additional context

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

patak-dev commented Apr 4, 2021

patak-dev commented Apr 4, 2021

patak-dev commented Apr 5, 2021

antfu commented Apr 5, 2021

antfu commented Apr 10, 2021

patak-dev commented May 5, 2021

Shinigami92 left a comment

Choose a reason for hiding this comment

antfu commented May 7, 2021

Shinigami92 commented May 7, 2021

antfu May 7, 2021

Choose a reason for hiding this comment

aleclarson Nov 14, 2022

Choose a reason for hiding this comment

antfu commented Apr 4, 2021 •

edited

Loading