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

Config Protection #30

Open
0x4007 opened this issue Jul 26, 2024 · 5 comments
Open

Config Protection #30

0x4007 opened this issue Jul 26, 2024 · 5 comments

Comments

@0x4007
Copy link
Member

0x4007 commented Jul 26, 2024

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.

@0x4007
Copy link
Member Author

0x4007 commented Jul 26, 2024

@Keyrxng this might be easy for you given the similar logic.

@Keyrxng
Copy link
Member

Keyrxng commented Jul 26, 2024

This should be a standalone plugin? or should I include it in assistive-pricing?

@gentlementlegen
Copy link
Member

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.

@Keyrxng
Copy link
Member

Keyrxng commented Jul 26, 2024

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

@gentlementlegen
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants