withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
46.39k stars 2.46k forks source link

vite file serving allow list doesn't work with yarn berry + global cache #6022

Closed levic closed 1 year ago

levic commented 1 year ago

What version of astro are you using?

2.0.2

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

yarn 3.x

What operating system are you using?

Mac

Describe the Bug

Background: One of the benefits of yarn 2+ (yarn berry) is pnp mode: the ability to avoid installing everything in node_modules. Instead npm modules are downloaded into a shared global cache and the nodejs module loader is patched to load them from the global cache. yarn installs are usually much faster, and there is no massive node_modules directory.

Previously the astro advice (#3450) was to use nodeLinker: node-modules which causes yarn to not use pnp mode, and instead install everything in node_modules like npm.

With #4842 this is no longer required.

However that patch didn't fix everything: when running the dev server vite now issues errors like:

The request url ".../.yarn/berry/cache/astro-npm-2.0.2-e57b5e49dc-8.zip/node_modules/astro/dist/runtime/client/hmr.js" is outside of Vite serving allow list.

It appears that the default vite.server.fs.allow from astro doesn't include the global yarn cache.

(This isn't limited to hmr.js; it also happens with any package whose contents are directly served -- eg using @fontsource as recommended in the docs)

Link to Minimal Reproducible Example

https://gist.github.com/levic/b8a31b70163329617f0338c6af0a975b

Participation

levic commented 1 year ago

Reproduce instructions:

In a clean directory:

yarn set version berry
yarn config set enableGlobalCache true
yarn create astro@latest

# (astro v2.0.2)

# choose defaults for everything except project name:
# - project name: myproject
# - a few best practices
# - install yarn dependencies: yes
# - initialize a new git repository: yes
# - typescript: strict

cd myproject

# these two commands are a workaround for issue #5637
touch yarn.lock
yarn install

yarn dev
levic commented 1 year ago

If you simply add the global yarn cache to vite's allow list then you're also opening up dependencies that were not specified in package.json.

This is a potential security issue though. Imagine a local package that was not published on npm contains sensitive data & someone runs the dev server with --host to open the dev server for someone else on the network to look at their site. It would be very surprising for the dev to discover that their private package contents are now available to anyone on the network.

The "correct" solution seems to be to enumerate every transitive dependency specified in package.json and add that to vite's allow list but unless yarn has an API to expose this already, I expect that is going to messy.

levic commented 1 year ago

After digging through the astro code I can't find anywhere where allow is being manually set -- can a dev with more knowledge confirm that astro is just using the default vite config?

If so this is really a vite issue, not an astro one.

For anyone else who encounters the same issue, a workaround is to put in your astro.config.mjs:

export default defineConfig({
    vite: {
        server: {
            fs: {
               // if you use this with --host anyone on the network can view
               // the contents of your yarn cache (including private packages)
                strict: false 
            }
        }
    }
});
bluwy commented 1 year ago

Astro doesn't change the server.fs.strict config. Vite's default option likely doesn't cover yarn's global .yarn cache. I'd suggest opening this issue in Vite instead (ideally with a Vite-only repro).

I'm not sure how this can be fixed unless yarn exposes the directory under process.env or through pnpapi, but that probably needs more investigation.

Thanks for the report. I'll close this for now as there's not much Astro can do at the moment.