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 config loadEnv exposing all envs to frontend. #16686

Closed
7 tasks done
pumanitro opened this issue May 15, 2024 · 7 comments
Closed
7 tasks done

Vite config loadEnv exposing all envs to frontend. #16686

pumanitro opened this issue May 15, 2024 · 7 comments

Comments

@pumanitro
Copy link

Describe the bug

This line of code:
const clientEnv = loadEnv(mode, process.cwd(), '');

for vite.config.ts file (entire vite configuration) was the place that caused our CI/CD envs to be exposed to the world.

Of course, some bots that scrape the internet read compiled bundle, got our keys, and used them to cause a massacre.

Yes, we *** up using chatgpt generated config and doing code review.

Still, it should not be so easy to do it and I feel more people could be affected by that.

It is also hard to find it during the code review because these are 2 sings that can cost you and your company a lot of money.

IMO it should show some error at least somewhere, or take a value like 'YES_I_WANT_TO_EXPOSE_ALL_OF_MY_ENVS_TO_THE_WORLD_AND_BOTS'.

Reproduction

https://stackblitz.com/edit/vitejs-vite-e6fzdp?file=vite.config.ts&terminal=dev

Steps to reproduce

No response

System Info

Windows 10,
Chrome 124.0.6367.202
Packages:
    "vite": "~5.0.0",
    "vite-plugin-env-compatible": "^2.0.1",
    "vitest": "^1.3.1",

Used Package Manager

npm

Logs

No response

Validations

Copy link

stackblitz bot commented May 15, 2024

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

@pumanitro pumanitro changed the title Vite config loadEnv exposing all the envs to frontend. Vite config loadEnv exposing all envs to frontend. May 15, 2024
@afriestad
Copy link

https://vitejs.dev/guide/api-javascript.html#loadenv

Hey there, @pumanitro! Here you've manually turned off the guardrails by sending in an empty string as prefix. This is intended and well documented behaviour. I recommend checking the documentation for the functions you call, especially if a plagiarism engine recommends a function you haven't used before.

@pumanitro
Copy link
Author

https://vitejs.dev/guide/api-javascript.html#loadenv

Hey there, @pumanitro! Here you've manually turned off the guardrails by sending in an empty string as prefix. This is intended and well documented behaviour. I recommend checking the documentation for the functions you call, especially if a plagiarism engine recommends a function you haven't used before.

I totally understand it, still I am proposing this to not be so easy to be done. Especially if this can cost companies lives. Empty string is hard to be noticed in comparison to regexp like this: 'YES_I_WANT_TO_EXPOSE_ALL_OF_MY_ENVS_TO_THE_WORLD_AND_BOTS'.
What is more, I got a feeling more and more people will be hacked because of it. Don't we want to protect humans that make mistakes by being it more explicit?

@bluwy
Copy link
Member

bluwy commented May 16, 2024

I don't think there's an issue with loadEnv() either. The problem here isn't loadEnv() but how you use the result of loadEnv(), which in this case is:

    define: {
      'process.env': clientEnv,
    },

loadEnv() is only a utility to load env vars, and it shouldn't warn with the assumption that you're exposing the secret env vars.

In Vite, we do issue an error if you set envPrefix: '' because that directly controls the values of import.meta.env.*. But in the case for define, you're free to replace any value with something else.

@bluwy bluwy closed this as not planned Won't fix, can't repro, duplicate, stale May 16, 2024
@SvizelPritula
Copy link

SvizelPritula commented May 16, 2024

Note that the documentation (kind of) suggests you do this. When explaining how to use loadEnv to access values from .env files, it demonstrates that by feeding everything into define: https://vitejs.dev/config/
Edit: I'm blind, it just extract one value. It's therefore okay.

@pumanitro
Copy link
Author

pumanitro commented May 16, 2024

Note that the documentation (kind of) suggests you do this. When explaining how to use loadEnv to access values from .env files, it demonstrates that by feeding everything into define: https://vitejs.dev/config/

image

Screanshoting till they remove it :)

@lyesh
Copy link

lyesh commented May 16, 2024

loadEnv(mode, process.cwd(), '') (the effective value in the submission) is very different from loadEnv(mode, process.cwd(), '').APP_ENV (the effective value in the documentation). It's like the difference between "rm -rf /opt/" and "rm -rf / opt/" . Just one character makes the difference between removing all of the third-party apps on the system and deleting everything that it can find. As in most professions, you need to know what you are doing to use your tools properly and effectively.

@github-actions github-actions bot locked and limited conversation to collaborators May 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants