vitejs / vite

Next generation frontend tooling. It's fast!
http://vitejs.dev
MIT License
67.07k stars 6.02k forks source link

When referencing an environment variable that doesnt exist, vite dumps your whole environment into the bundle as an object #17710

Closed joshmanders closed 1 month ago

joshmanders commented 1 month ago

Describe the bug

When you reference an env var, the idea is that Vite will replace the reference with the value of that env var. So given the following .env

VITE_FOO=foo

And the following code

console.log(import.meta.env.VITE_FOO);

The resulting output should be

console.log("foo");

And this is true and works as expected. But now reference an env var that doesn't exist, you'd expect the replacement to become undefined, like so:

console.log(import.meta.env.VITE_FOO); // 'foo'
console.log(import.meta.env.VITE_BAR); // undefined

What actually happens is the whole "env" object is dumped directly into the bundle, then references the var from that, which because its not defined, in the end gives you undefined

var o = {
  VITE_FOO: "foo",
  BASE_URL: "/",
  MODE: "production",
  DEV: !1,
  PROD: !0,
  SSR: !1,
};

console.log("foo");
console.log(o.VITE_BAR);

I believe this is wrong and can lead to bad security leaks and instead it should output the following

console.log("foo");
console.log(undefined);

Reproduction

https://github.com/joshmanders/vite-env-bug

Steps to reproduce

Run npm install Run npm run build Inspect output in dist/assets/

System Info

System:
    OS: macOS 14.5
    CPU: (10) arm64 Apple M1 Max
    Memory: 1.38 GB / 64.00 GB
    Shell: 3.7.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 20.15.0 - /opt/homebrew/bin/node
    npm: 10.7.0 - /opt/homebrew/bin/npm
    Watchman: 2024.06.24.00 - /opt/homebrew/bin/watchman
  Browsers:
    Brave Browser: 126.1.67.123
    Chrome: 126.0.6478.127
    Edge: 126.0.2592.102
    Safari: 17.5
  npmPackages:
    vite: ^5.3.4 => 5.3.4

Used Package Manager

npm

Logs

Click to expand! ```shell vite:config bundled config file loaded in 11.67ms +0ms vite:config using resolved config: { vite:config root: '/Users/josh/Code/joshmanders/vite-env-bug', vite:config build: { vite:config target: [ 'es2020', 'edge88', 'firefox78', 'chrome87', 'safari14' ], vite:config cssTarget: [ 'es2020', 'edge88', 'firefox78', 'chrome87', 'safari14' ], vite:config outDir: './dist', vite:config assetsDir: 'assets', vite:config assetsInlineLimit: 4096, vite:config cssCodeSplit: true, vite:config sourcemap: false, vite:config rollupOptions: { input: [Array] }, vite:config minify: 'esbuild', vite:config terserOptions: {}, vite:config write: true, vite:config emptyOutDir: null, vite:config copyPublicDir: true, vite:config manifest: false, vite:config lib: false, vite:config ssr: false, vite:config ssrManifest: false, vite:config ssrEmitAssets: false, vite:config reportCompressedSize: true, vite:config chunkSizeWarningLimit: 500, vite:config watch: null, vite:config commonjsOptions: { include: [Array], extensions: [Array] }, vite:config dynamicImportVarsOptions: { warnOnError: true, exclude: [Array] }, vite:config modulePreload: { polyfill: true }, vite:config cssMinify: true vite:config }, vite:config configFile: '/Users/josh/Code/joshmanders/vite-env-bug/vite.config.js', vite:config configFileDependencies: [ '/Users/josh/Code/joshmanders/vite-env-bug/vite.config.js' ], vite:config inlineConfig: { vite:config root: undefined, vite:config base: undefined, vite:config mode: undefined, vite:config configFile: undefined, vite:config logLevel: undefined, vite:config clearScreen: undefined, vite:config build: {} vite:config }, vite:config base: '/', vite:config rawBase: '/', vite:config resolve: { vite:config mainFields: [ 'browser', 'module', 'jsnext:main', 'jsnext' ], vite:config conditions: [], vite:config extensions: [ vite:config '.mjs', '.js', vite:config '.mts', '.ts', vite:config '.jsx', '.tsx', vite:config '.json' vite:config ], vite:config dedupe: [], vite:config preserveSymlinks: false, vite:config alias: [ [Object], [Object] ] vite:config }, vite:config publicDir: '/Users/josh/Code/joshmanders/vite-env-bug/public', vite:config cacheDir: '/Users/josh/Code/joshmanders/vite-env-bug/node_modules/.vite', vite:config command: 'build', vite:config mode: 'production', vite:config ssr: { vite:config target: 'node', vite:config optimizeDeps: { noDiscovery: true, esbuildOptions: [Object] } vite:config }, vite:config isWorker: false, vite:config mainConfig: null, vite:config bundleChain: [], vite:config isProduction: true, vite:config plugins: [ vite:config 'vite:build-metadata', vite:config 'vite:watch-package-data', vite:config 'vite:pre-alias', vite:config 'alias', vite:config 'vite:modulepreload-polyfill', vite:config 'vite:resolve', vite:config 'vite:html-inline-proxy', vite:config 'vite:css', vite:config 'vite:esbuild', vite:config 'vite:json', vite:config 'vite:wasm-helper', vite:config 'vite:worker', vite:config 'vite:asset', vite:config 'vite:wasm-fallback', vite:config 'vite:define', vite:config 'vite:css-post', vite:config 'vite:build-html', vite:config 'vite:worker-import-meta-url', vite:config 'vite:asset-import-meta-url', vite:config 'vite:force-systemjs-wrap-complete', vite:config 'commonjs', vite:config 'vite:data-uri', vite:config 'vite:dynamic-import-vars', vite:config 'vite:import-glob', vite:config 'vite:build-import-analysis', vite:config 'vite:esbuild-transpile', vite:config 'vite:terser', vite:config 'vite:reporter', vite:config 'vite:load-fallback' vite:config ], vite:config css: { lightningcss: undefined }, vite:config esbuild: { jsxDev: false }, vite:config server: { vite:config preTransformRequests: true, vite:config sourcemapIgnoreList: [Function: isInNodeModules$1], vite:config middlewareMode: false, vite:config fs: { vite:config strict: true, vite:config allow: [Array], vite:config deny: [Array], vite:config cachedChecks: undefined vite:config } vite:config }, vite:config preview: { vite:config port: undefined, vite:config strictPort: undefined, vite:config host: undefined, vite:config https: undefined, vite:config open: undefined, vite:config proxy: undefined, vite:config cors: undefined, vite:config headers: undefined vite:config }, vite:config envDir: '/Users/josh/Code/joshmanders/vite-env-bug', vite:config env: { vite:config VITE_FOO: 'foo', vite:config BASE_URL: '/', vite:config MODE: 'production', vite:config DEV: false, vite:config PROD: true vite:config }, vite:config assetsInclude: [Function: assetsInclude], vite:config logger: { vite:config hasWarned: false, vite:config info: [Function: info], vite:config warn: [Function: warn], vite:config warnOnce: [Function: warnOnce], vite:config error: [Function: error], vite:config clearScreen: [Function: clearScreen], vite:config hasErrorLogged: [Function: hasErrorLogged] vite:config }, vite:config packageCache: Map(1) { vite:config 'fnpd_/Users/josh/Code/joshmanders/vite-env-bug' => { vite:config dir: '/Users/josh/Code/joshmanders/vite-env-bug', vite:config data: [Object], vite:config hasSideEffects: [Function: hasSideEffects], vite:config webResolvedImports: {}, vite:config nodeResolvedImports: {}, vite:config setResolvedCache: [Function: setResolvedCache], vite:config getResolvedCache: [Function: getResolvedCache] vite:config }, vite:config set: [Function (anonymous)] vite:config }, vite:config createResolver: [Function: createResolver], vite:config optimizeDeps: { vite:config holdUntilCrawlEnd: true, vite:config esbuildOptions: { preserveSymlinks: false } vite:config }, vite:config worker: { format: 'iife', plugins: '() => plugins', rollupOptions: {} }, vite:config appType: 'spa', vite:config experimental: { importGlobRestoreExtension: false, hmrPartialAccept: false }, vite:config getSortedPlugins: [Function: getSortedPlugins], vite:config getSortedPluginHooks: [Function: getSortedPluginHooks] vite:config } +17ms ```

Validations

innocenzi commented 1 month ago

Oh, lol, related: https://github.com/vitejs/vite/pull/17701

joshmanders commented 1 month ago

@innocenzi seems we both were frustrated with the same issue yesterday haha

jnoordsij commented 1 month ago

I ran into this as well, but more so from the side of being surprised that if VITE_BAR is not defined in your environment, it is actually left in the outputted code as <somevar>.VITE_BAR rather than being replaced with undefined (and then further optimized). Usecase: I have some debug-variable that I can set to a specific value in my local development environment to change behavior, but do not define in (production) build environment, hoping it would just entirely remove the code.

Thinking about this, there's a combination of possible things that can be done in my eyes to improve the current behavior:

As to the issue description: I don't think it dumps the "whole environment" into the build, but rather just it's own values + any value with correct prefix. Building console.info(import.meta.env.VITE_BAR); with VITE_FOO=foo SECRET_VAR=s3cr3t vite build results in

var e={VITE_FOO:"foo",BASE_URL:"/",MODE:"production",DEV:!1,PROD:!0,SSR:!1};console.info(e.VITE_BAR);

Thus the potential security issue is a bit more limited/subtle here.

joshmanders commented 1 month ago

Thus the potential security issue is a bit more limited/subtle here.

FWIW The security risk is because I have a plugin I made that just places the whole contents of process.env into the define, so I don't need a VITE_ prefix on things I want to use in the client side that aren't necessarily ONLY for the client side and because of this bug, it opens up a potential security risk by dumping sensitive secrets that are never referenced and shouldn't ever be included in the bundle.

Basically VITE_ prefix is putting a bulletproof vest on so you don't shoot yourself, and this bug is grabbing the gun and forcing you to point it in your face when you say "Well I don't need to wear a bullet proof vest, I am capable of not shooting myself"

bluwy commented 1 month ago

https://github.com/vitejs/vite/pull/17648 should fix this, but still, you shouldn't put any sensitive information in import.meta.env in the first place that could cause security risks. Only non-secret variables should be kept there.

joshmanders commented 1 month ago

17648 should fix this, but still, you shouldn't put any sensitive information in import.meta.env in the first place that could cause security risks. Only non-secret variables should be kept there.

I should be able to put anything I want in import.meta.env, the security risk is Vite dumping unused env vars into the final bundle because something was referenced that wasn't defined.

If I never use import.meta.env.SUPER_SECRET in my codebase, but it's defined in my env, that shouldn't ever be exposed to the frontend just because a 3rd party package references import.meta.env.DOESNT_EXIST.

And VITE_SECRET doesn't stop you from exposing secrets to the frontend, so again, this security risk is on Vite for not exposing and not end users.

bluwy commented 1 month ago

Vite assumes anything in import.meta.env is safe and public, so it can expose everything if it needs to. It's never meant to hold any secret variables in the first place. In fact, in dev it'll expose everything by default as a perf optimization, so if you ever start dev with --host, you'd be open to attack. If you have a plugin that puts secret variables into import.meta.env, that plugin imposes security risks, not Vite. You can't misuse a feature and blame Vite that it's not doing security right.

jnoordsij commented 1 month ago

I should be able to put anything I want in import.meta.env, the security risk is Vite dumping unused env vars into the final bundle because something was referenced that wasn't defined.

If I never use import.meta.env.SUPER_SECRET in my codebase, but it's defined in my env, that shouldn't ever be exposed to the frontend just because a 3rd party package references import.meta.env.DOESNT_EXIST.

(just a FYI) Even if the above issue is completely remedied, if you expose too much data to import.meta.env, you still run the risk of some (malicious or unknowing) third party package referencing import.meta.env.SUPER_SECRET in their code, silently exposing your secret anyways.

joshmanders commented 1 month ago

You can't misuse a feature and blame Vite that it's not doing security right.

Dumping an object of environment variables whether you explicitly defined them to be ABLE to be referenced or not is the security issue, full stop. Enough with the virtue signaling.

(just a FYI) Even if the above issue is completely remedied, if you expose too much data to import.meta.env, you still run the risk of some (malicious or unknowing) third party package referencing import.meta.env.SUPER_SECRET in their code, silently exposing your secret anyways.

This applies to Vite approved env vars too.

Many I'd really like if people just acknowledge that there's bugs and those bugs can have unintended side effects that could be a security issue and go "yeah we'll work on resolving that" instead of "ya'll big dum dums are not allowed to use env vars we don't approve of!"

Every single one of these arguments can be applied to "the sanctioned way" and still be correct.

Only difference is my report is a legitimate bug and security risk, yours are just placing blame on the victims.

jnoordsij commented 1 month ago

(Preface: I'm not here to deny the behavior is buggy/confusing in any way; just trying to clarify the behavior and the potential impact. As I mentioned above, I also think the behavior is potentially confusing and/or misleading and things could be done to improve it, it's just that any solution here will still have drawbacks and still requires users to be careful.)

I think there might be some confusing regarding the "vite dumps your whole environment" part here, because it does not. By default, Vite will only look at environment variables prefixed with VITE_ (or some other prefix you if you alter your config). By definition, all of these variables will be exposed to your frontend bundle and any code in it, i.e. including third party code, regardless of whether or not it is "dumped" as described by this issue.

By default, import.meta.env.SUPER_SECRET will thus not be exposed in any way with the default config, as it does not have the VITE_ prefix (and clearly shouldn't; that's the exact reason why #17701 was closed). But if you do somehow manage to include it your import.meta.env by bypassing the inplace security check, it is subject to leaking through other means, like third party code, even if the "dump" of this issue would be removed. Therefore the "security issue" at hand is really not tied nor fixed by just removing the "dump".

Like I suggested above, altering the current behavior to not (only) expose variables with the VITE_ prefix would be viable only if you would be able to add to your config a strictly controlled list of variables you'd want to expose. This would put all of the control on exposed variables into the users hands, and allow for easier sharing of non-secret variables between multiple applications/tools (as #17701 was intending to do), while still limiting the exposed variables to just a manually picked set, avoiding security issues.

See also the docs at https://vitejs.dev/guide/env-and-mode.html#env-files:

To prevent accidentally leaking env variables to the client, only variables prefixed with VITE_ are exposed to your Vite-processed code.

joshmanders commented 1 month ago

It's okay, I'm an adult. I know how to handle my environment variables. You don't need to tell me what I can and cannot use in my apps.

Lets skip the bike shedding about the contents of the variables themselves, and focus on the fact that this is a bug and if an undefined variable is referenced it should be replaced with undefined not a JSON object containing all variables in the config.

joshmanders commented 1 month ago

You act as if “secret env var” equates to keys to services and other sensitive stuff and not something that is ACTUALLY needed in the app, such as I don’t know… “secret pass code to unlock hidden chest in level 3 of game that needs to be configurable” and if a 3rd party library somehow manages to sneak a console.log(import.meta.env.NON_EXISTANT) that we should just be okay with Vite just putting all that stuff in there.

jnoordsij commented 1 month ago

I'm very sorry but I just still fail to see how this is a bug nor how it is a security issue. I'm glad that #17648 will land and change the behavior, but I don't think the current behavior is unreasonable in any way, just possibly unexpected.

Yet as stressed above (and just to repeat for anyone reading along), anything on import.meta.env should be considered possibly part of the produced bundle (and thus e.g. exposed to a frontend) and thus should not be used for any sensitive information. Behavior on code like console.log(import.meta.env) will remain the same: it will (and by design should) expose all the contents of import.meta.env. And exactly for that reason the protection of the VITE_ prefix exists.

Edit: just to clarify, I don't intend to keep this discussion going any further, as I think it's already gone too far off-topic for the scope of this issue.

joshmanders commented 1 month ago

k, I've wasted enough time arguing about this. Clearly I'm not the only one who sees a problem with this. But I ain't a maintainer so do whatever ya'll want. Closing this.

innocenzi commented 1 month ago

@jnoordsij there's a bit of context in this PR as to why this is a DX issue: https://github.com/vitejs/vite/pull/17701

And I want to answer to this:

Behavior on code like console.log(import.meta.env) will remain the same: it will (and by design should) expose all the contents of import.meta.env. And exactly for that reason the protection of the VITE_ prefix exists.

Maybe it's by design, but that doesn't mean it's good. That design, if that was actually intended, was such a footgun that we had to have that VITE_ protection.

I just think that code like console.log(import.meta.env) should throw a warning at build-time. We should strive to allow non-prefixed environment variables to be bundled without security risk.

jnoordsij commented 1 month ago

@innocenzi I do agree that the current behavior can be confusing at points; I've created #17765 as follow-up to repeat the suggestions I listed above to improve it. Feel free to put forward any further suggestions you have on the topic there!

Note there already are ways to expose non-prefixed environment variables to the client code (see https://vitejs.dev/config/shared-options#envprefix), but indeed there may be possible improvements on achieving such a thing more easily.

yashjainyj commented 1 month ago

I Found one simple solution to use this with the help of core javascript concept.

To access env. variable you need to define with VITE_ prefix (for ex: VITE_APP_VERSION ) And then in vite.config.ts define config define: { 'process.env': env, global: 'window' }, then in any util file you can directly use process.env.VITE_APP_VERSION

For accessing images in the assets folder, the traditional require() method is not supported in Vite. Instead, you should use the import.meta.url with the URL class. However, a more efficient approach is to create a utility file for managing images. This allows you to import images directly and use them throughout your project.

import imageName from '@/assets/your_image_name'; export class ImageUtil { static imageName }

and then use in any file like this

import { ImageUtil } from './ImageUtil'; const myImage = ImageUtil.imageName;

Hope it is helpful