Skip to content

[SECURITY] [DOC] - Vite environment variables in defineConfig()#19510

Closed
qu35t-code wants to merge 1 commit intovitejs:mainfrom
qu35t-code:env-security
Closed

[SECURITY] [DOC] - Vite environment variables in defineConfig()#19510
qu35t-code wants to merge 1 commit intovitejs:mainfrom
qu35t-code:env-security

Conversation

@qu35t-code
Copy link

Description

Hello,

As a security auditor (pentester), I have repeatedly encountered cases where client applications expose all environment variables directly in the application's JavaScript code. This often leads to a complete compromise of the application and even its underlying infrastructure.

A common thread in these incidents is that all of these applications use Vite to build their code. After conducting some research in the official documentation, I realized that the code example provided in the defineConfig() code snippet could be misleading and potentially lead to this critical security issue.

Here's an example of vulnerable code:

import { defineConfig, loadEnv } from 'vite'

export default defineConfig(({ mode }) => {
  const env = loadEnv(mode, process.cwd(), '')
  return {
    // Vite configuration
    define: {
      'process.env': env
    },
  }
})

By default, the documentation suggests using an empty string ('') as the third argument of the loadEnv() function, which results in loading all environment variables.

To improve the documentation and security, I suggest a small but impactful change to the documentation snippet: by default, only variables with a specific prefix (VITE_ and APP_) should be included (rather than all environment variables). Additionally, a warning should be added about the risks of passing the env variable directly in the return statement of the function when using '' as the third argument.

image

@qu35t-code qu35t-code changed the title [SECURITY] - Vite environment variables in defineConfig() [SECURITY] [DOC] - Vite environment variables in defineConfig() Feb 25, 2025
@bluwy
Copy link
Member

bluwy commented Feb 25, 2025

I'm open to adding a info block warning against doing something like 'process.env': env, but otherwise this isn't a security issue. It has been discussed at #16686. We shouldn't avoid passing '' in the docs.

@qu35t-code
Copy link
Author

@bluwy,

I never said that what is in the current documentation is a security issue, only that it could become one if developers copy the code snippet and return the variable that is defined !
I think it's less risky and just as valid to pass an array of prefixes as the third argument. It doesn't change anything else; the comment in the code still explains that it's possible to use '' instead :)

@qu35t-code
Copy link
Author

I also add that it's never a good practice to retrieve all environment variables into a single variable.

@sapphi-red
Copy link
Member

@sapphi-red While I believe this is a step in the right direction but I do believe that changing the doc would be more beneficial, even with a warning some people will still ignore it or don't read it. The best way to save those people is to not have a problematic code that they can copy and paste.

I believe that @qu35t-code suggestion provides a safer snippet that will not be problematic for users that don't read the documentation or the warning and would sill allow conscious developer to achieve what they want if required.
#19517 (comment)

I don't think the current code is problematic. The problem is that people are passing sensitive values to define. Even if we removed '', I assume people would merge the values with process.env.

The reason why this section exists is to tell users how to use arbitrary env vars (including values from .env files). If they only need env vars limited to a specific prefix, they won't need to use any of this code. It'll be available automatically. If we remove '', then this whole section doesn't make sense.

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

Successfully merging this pull request may close these issues.

3 participants