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

Isomorphic env config is a severe risk of secrets leak due to developer errors #11131

Closed
eric-burel opened this issue Mar 17, 2020 · 3 comments
Closed

Comments

@eric-burel
Copy link
Contributor

eric-burel commented Mar 17, 2020

Feature request

Is your feature request related to a problem? Please describe.

Currently, using Next config env will allow user to define variables that can be consumed both server side and client side using process.env.

This pattern is rather safe, because process.env.FOOBAR is hard-replaced directly in the code. So client-side never have access to server-side env variable.

EXCEPT if you accidentally call them.

Simple example:

  • I have called my stripe keys STRIPE_PUBLIC_SECRET and STRIPE_SECRET
  • I mistakenly think that STRIPE_SECRET is the public key (yeah I am dumb but its 3am and I am tired)
  • User opens up the client-side code and find this variable
  • I am ruined

I insist that the system in itself is sound and safe. The mistake is on the developer side here. But still, we could prevent that to happen at the framework level.

Describe the solution you'd like

Force a different syntax for client side environment variables, never allow to get variables that do not match this syntax.
The current isomorphic pattern seems nice but it's counter-productive regarding end user safety, with potentially an extremely high criticity.

CRA uses a prefix for client side variables. Limitation is that it's harder to use.

Meteor uses only runtime-like settings, with a separate public object in the settings:

public: {
    foo: "I am available client-side and server side through Meteor.settings.public"
},
bar: "I am server only" // in Next, I would be available in the client if the developper explicitely write "process.env.bar", that's where the leak can happen

I would prefer a symmetrical pattern though with "public"/"private"

Describe alternatives you've considered

Since nested objects work fine, I already enforce this pattern locally in my app, by using a public object.

Maybe we could add a build rule (somehow?) to check that only process.env.public are allowed client side.

@timneutkens
Copy link
Member

Covered by #11106 (nice coincidence that I posted it yesterday).

@eric-burel
Copy link
Contributor Author

Excellent!

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants