-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
fix: avoid leaking defined env vars when trying to access not defined env vars #10030
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - I think this looks great. Nice work @pieh 💪
b5881f4
to
aaf0d05
Compare
@pieh thank you for the security fix. I don't know the best place to make mention of this but the change will seriously impact anyone that is destructuring environment variables.
Will result in undefined values for both. |
Is there a reason that this was not communicated as a breaking change? It's surprising to have an app break after a patch release 🙁 . |
@brimtown this was brought to the attention of the team and from the flow of commits and the issue associated to it, needed to be patched with some urgency to prevent further damage to the people using gatsby. |
@brimtown you're definitely right, though. This probably should have been communicated and pushed out as something beyond a patch release. Minimally, we should have marked it as a breaking change so it shows up in We're sorry for the confusion this has caused, and we'll attempt to improve the documentation and communication on these types of changes going forward! |
Thanks for the response. Appreciate the hard work yall are doing! |
… env vars (gatsbyjs#10030) * fix: avoid leaking defined env vars when trying to access not defined env vars * test: add some testing for env var usage and leaking * add some comments to .env.production - it normally shouldn't be commited to repo
fixes #10021
With current approach when
process.env.NOT_EXISTING_ENV_VAR
is used then bundle will contain:Object({ NODE_ENV: "production", ...otherEnvVars }).NOT_EXISTING_ENV_VAR
- with this approach it will produce:{}.NOT_EXISTING_ENV_VAR
, while still being able to access all defined vars.This is important because not only user code can be responsible for leaks - all imported npm packages can cause this as well.
Limitation - users won't be able to iterate over
process.env
object but this was exactly what was causing potential leak.