-
Notifications
You must be signed in to change notification settings - Fork 150
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: Don't allow serialization of firestore settings #1742
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Thanks for your contribution @abhishekwebcode . If I understand correctly, you are concerned about users seeing the name of your Firestore host from the Settings object, is that correct? |
@ehsannas no i am worried about developers with log access seeing the firestore private key ( the one that initialises firebase-admin library nodejs ) |
hey @ehsannas i have signed the agreement now |
When logging any firestore object like WriteBatch,Transaction,etc the settings object also gets logged / exposed This can be seen by running JSON.stringify on any firestore object even a document reference Many developers log firestore objects to help them debug testing/prod issues, this leaking of entire firestore key via this._settings is a bad practice as per me We can also use Object.defineProperty to make it non-enumerable or any other technique that you like
There could be scenarios where the serialized settings object is useful. Could we a) make this behavior optional, b) redact only the key, or c) make the key non-enumerable? |
Thanks for the suggestion. I have updated the code to only redact the credentials part of the settings object. PTAL @MarkDuckworth |
When logging any firestore object like WriteBatch,Transaction,etc the settings object also gets logged / exposed
This can be seen by running JSON.stringify on any firestore object even a document reference
Many developers log firestore objects to help them debug testing/prod issues, this leaking of entire firestore key via this._settings is a bad practice as per me
We can also use Object.defineProperty to make it non-enumerable or any other technique that you like
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕