-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix: restrict static middleware fs access #5361
Conversation
Thank you so much for this fix! As a higher-level note, having both the public and static middlewares means every public file is accessible via two different URLs, which isn't necessarily desirable. I wonder if it would be good to try to have just a single middleware for statically serving files. (This is all assuming the public directory lives within the project workspace, which is true for SvelteKit, but I'm not sure about Vite projects generally) |
Guess we could release this as a part of 2.6 before we flip the default in 2.7 |
@antfu sooner the better. |
Description
Fix #5345
This PR modifies the fs-serve playground so the allowed folder is not the same as the root, so we can also test the static middleware file access.
@antfu there are two
TODO:
in the code:We should use
ensureServingAccess
so anAccessRestrictedError
is thrown and catched later to respond with a 403, but when I tried this the program was terminating unexpectedly and I couldn't figure out why. The current code is skipping the url if it isn't allowed, so the response is a 404. We could evaluate merging as is, since this is a security vulnerability. And improve later, but maybe you see what I missed.Maybe it isn't a bad idea to enqueue this change for the 2.7 beta if we are going to start the process soon.
What is the purpose of this pull request?