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

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

Closed
7 tasks done
joshmanders opened this issue Jul 17, 2024 · 17 comments · Fixed by #17648
Labels
p2-edge-case Bug, but has workaround or limited in scope (priority)

Comments

@joshmanders
Copy link

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!
  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
Copy link
Contributor

Oh, lol, related: #17701

@joshmanders
Copy link
Author

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

@jnoordsij
Copy link

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:

  • Emit some warning when building a bundle where some environment variable is missing; e.g. scan for any prefix-matching pattern and notify the user if some environment variable is being referenced but not set ("WARNING: Variable VITE_BAR is being referenced but no value found; including environment into output bundle")
  • Allow users to (with some config flag?) automatically replace any (prefixed) env value with undefined if it was not set
  • Update documentation on environment variables to clearly explain and reflect this current behavior, which I find somewhat counterintuitive at points
  • Allow users to list/declare all allowed environment variables in the config file, potentially even with fallback values, and only replace those (as alternative to prefix?)

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

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

bluwy commented Jul 25, 2024

#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.

@bluwy bluwy added p2-edge-case Bug, but has workaround or limited in scope (priority) and removed pending triage labels Jul 25, 2024
@joshmanders
Copy link
Author

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

bluwy commented Jul 25, 2024

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

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

joshmanders commented Jul 25, 2024

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

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

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

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

jnoordsij commented Jul 25, 2024

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

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

@jnoordsij there's a bit of context in this PR as to why this is a DX issue: #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
Copy link

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

yashjainyj commented Jul 30, 2024

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

@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p2-edge-case Bug, but has workaround or limited in scope (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants