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

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

Closed
1 task
levic opened this issue Jan 29, 2023 · 4 comments
Closed
1 task

Comments

@levic
Copy link

levic commented Jan 29, 2023

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

  • I am willing to submit a pull request for this issue.
@levic
Copy link
Author

levic commented Jan 29, 2023

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
Copy link
Author

levic commented Jan 29, 2023

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
Copy link
Author

levic commented Jan 29, 2023

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
Copy link
Member

bluwy commented Jan 30, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants