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

[qu35t-code]

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.

[bluwy]

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]

@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]

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

[sapphi-red]

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