feat: plugin option to protect production environment #1231
Conversation
By setting options of the plugin, you can choose to warn or error (or do nothing) when an environment variable for emulator host isn't set. thus by setting one to 'error', a user can make cypress error rather than pointing to production for a firebase service by accident
|
Love this idea! Thanks for sharing what happened in your case leading you to adding the feature 👏 Updating the docs and tests would be awesome - I can try to get to them early next week if you don't get to it before then |
prescottprue
changed the title
also removed some superfluous optional chaining
|
I have added the tests and documentation: b72dec1 Some notes: If optional chaining is indeed a problem, they should be removed here as well, I don't think the optional chaining is worth having a higher min cypress version if that is the case. Currently everything is programmed in a backwards-compatible way.
|
|
|
||
| process.env.FIREBASE_DATABASE_EMULATOR_HOST = ''; | ||
| expect( | ||
| pluginWithProtectProduction({ |
There was a problem hiding this comment.
Nice thanks for adding the tests!
src/firebase-utils.ts
Outdated
| @@ -178,6 +208,20 @@ export function initializeFirebase( | |||
| ); | |||
| /* eslint-enable no-console */ | |||
| adminInstance.firestore().settings(firestoreSettings); | |||
| } else if ( | |||
| protectProduction?.firestore && | |||
There was a problem hiding this comment.
| protectProduction?.firestore && | |
| protectProduction && | |
| protectProduction.firestore && |
|
|
||
| The plugin tries to detect whether or not the firebase emulators are running, based on the respective environment variables being set or not. When the an emulator isn't running, production could be targeted instead which might be dangerous. The `protectProduction` key configures the behaviour when the emulator for a specific firebase service hasn't been detected. The options for the behaviour when an emalator is not running are: | ||
|
|
||
| - `'none'` _(default)_ no protection is granted in this case, nor will a warning be output to the console. Production could be targeted. |
There was a problem hiding this comment.
Do you think there is value in offering this option at all? Since not providing a setting here is the same as default
There was a problem hiding this comment.
Good point. I might have added it for (explicit) completeness of the different behaviours.
I do think it can be useful in case you have the protection turned off for a certain service and turned on for others.
In that case it has some 'warn' and/or 'error' entries. Others or forgetful future selves might quickly assume there is some protection for every service, when there might not be for a service that isn't listed. So being able to set it to 'none' explicitly shows the intent or the possible danger that it is indeed turned off.
Of course it doesn't prevent users from not explicitly marking the protection is turned off, but at least you can do so if you would want to, explaining with a comment why it is turned off.
|
Looks like there is a lint error after I handled a merge conflict, but I don't believe I have access to push to your fork - once you get a chance to update on your end we should be good to release Thanks again for the feature and your diligence in cleanup to prep for release 👏 |
|
Oh yeah, weird my editor wasn't giving me that linting error at first. I thought I checked the box to allow edits from maintainers. |
|
@KantiKuijk thanks again - needed to do some updates to main before I could get it in |
|
🎉 This PR is included in version 4.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
I was accidentally altering production stuff because I didn't notice the environment variables weren't set. I had to revert to a backup, and thought I should have an option to prevent this from happening in the future.
Description
An extra object can be added to cypressFirebasePlugin where options for the plugin can be specified.
The protectProduction option is an object mapping firebase service names to either "none", "warn" or "error".
Depending on what is set, the behaviour will be different when te corresponding emulator host environment variable isn't set.
"none": same as not setting the option at all, same behaviour as before (backwards compatible)
"warn": warns the user in the log but will not error, thus marginally safer for users that actively read the cypress command line logs
"error": throws an error thus preventing cypress from starting, thus protecting the production environment
Similar to other PR, I am willing to change docs and tests, if I know this PR has the possibility of being merged, otherwise it would just be wasted time and effort on my part.
Also similar, open to changes or different implementations.