-
Notifications
You must be signed in to change notification settings - Fork 5
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
Config Protection #30
Comments
@Keyrxng this might be easy for you given the similar logic. |
This should be a standalone plugin? or should I include it in |
The idea is that we want to limit who can access the configuration file, and also avoid having someone overriding it per repo basis (because even though you don't have access to the organization configuration you can always put yours in some repo where you have control) so I'd say it would be a plugin. We were discussing about having protection on branches as well if that was a solution. |
Understood, I have a couple of good ideas for it P.S it was mentioned previously about enforcing tests, I think we should enforce them. Any new plugins should have a test suite which covers at least the basics then any new features should extend those. No set limits or anything just follow the same test structure as other plugins |
We have tests in the template for plugins as well (and I usually don't merge PRs that do not link QA and tests). Feel free to improve the template with new tests if you feel it is required. And let's be careful during reviews as well. |
Not sure if this is fully possible from within the webhook event, but the idea is that only admins or billing managers should be able to modify the config because it affects money flow.
On commit, check if config was modified. If it is unauthorized, rollback the change by immediately committing the previous version as the UbiquiBot.
This should make it near impossible for fraud.
Seems like relevant logic I saw in a pull.
The text was updated successfully, but these errors were encountered: