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

All environment variable names and associated values logged on unhandled env attribute error #292

Closed
mro-rhansen2 opened this issue Jun 6, 2023 · 3 comments · Fixed by #315

Comments

@mro-rhansen2
Copy link

We had a surprise recently in our code base when an attribute error was raised on access to an Env instance. All environment variables and their associated values were logged to stdout which was subsequently scraped and shipped to our log analytics stack. The culprit seems to be that the Env.__repr__ magic method deliberately stringifies the self._values dictionary and includes the results in the string representation of any given Env instance.

Was this a deliberate design decision? Environment variables are often used to handle secrets in many systems. The choice to spit all of those values out without user input seems unwise and doesn't appear to provide any immediate value to the library itself.

There are obviously failings on our end in this. Specifically, our test suite didn't catch the issue because this particular section of the code doesn't have great coverage admittedly. Be that as it may, it doesn't seem like a sound design choice to arbitrarily leak information that is generally assumed to be sensitive, if not downright privileged, in nature.

My workaround (aside from fixing the typo in the method name) was to simply monkeypatch the Env.__repr__ method with something that was safer and wouldn't divulge information that we don't want to appear in plain text logs in case there are any other "oopsies" in the future. If y'all are opposed to removing the stringification of the self._values dictionary altogether, then would y'all consider a safe: bool = False constructor parameter? If that variable is set to True, then Env wouldn't haphazardly report on every value within its scope.

@sloria
Copy link
Owner

sloria commented Jan 11, 2024

This is a good point! I've updated Env.__repr__ to not display parsed envvars and instead show constructor variables, which I believe is a more appropriate usage of __repr__ anyway.

Thanks for bringing this to my attention!

@mro-rhansen2
Copy link
Author

Awesome! We really do appreciate it. Things like this are silent killers for corporate development teams all over the world. Looking forward to the next release.

@sloria
Copy link
Owner

sloria commented Jan 11, 2024

The change is released in 10.3.0 👍

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 a pull request may close this issue.

2 participants