feat(eval): implement sysadmin eval permissions #884
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideThis PR introduces a new ALLOW_SYSADMINS_EVAL config flag and updates both the bot startup logic and the eval command to grant or deny sysadmin eval permissions based on that flag. Sequence Diagram: Bot Startup Owner ID ConfigurationsequenceDiagram
participant AppStartup
participant CONFIG
participant TuxBot
AppStartup->>AppStartup: Bot starting...
AppStartup->>CONFIG: Get BOT_OWNER_ID
CONFIG-->>AppStartup: bot_owner_id
AppStartup->>AppStartup: owner_ids = {bot_owner_id}
AppStartup->>CONFIG: Get ALLOW_SYSADMINS_EVAL
CONFIG-->>AppStartup: allow_sysadmins_eval_flag
alt allow_sysadmins_eval_flag is true
AppStartup->>CONFIG: Get SYSADMIN_IDS
CONFIG-->>AppStartup: sysadmin_ids_list
AppStartup->>AppStartup: owner_ids.update(sysadmin_ids_list)
AppStartup->>AppStartup: Log warning (Sysadmins can use eval)
else allow_sysadmins_eval_flag is false
AppStartup->>AppStartup: Log warning (Sysadmins cannot use eval)
end
AppStartup->>TuxBot: Initialize with dynamic owner_ids
Class Diagram: Update to Config ClassclassDiagram
class Config {
+BOT_OWNER_ID: int
+SYSADMIN_IDS: list[int]
+ALLOW_SYSADMINS_EVAL: bool
+DEFAULT_PROD_PREFIX: str
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements sysadmin eval permissions, enabling sysadmins to execute eval commands conditionally based on a new configuration flag.
- Introduces a new configuration flag (ALLOW_SYSADMINS_EVAL) in the configuration file.
- Updates the eval command logic in the admin cog to handle sysadmin permissions.
- Adjusts the bot startup procedure to include sysadmin IDs as owners when allowed and updates the example settings with detailed instructions.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tux/utils/config.py | Added ALLOW_SYSADMINS_EVAL config entry for toggling eval usage. |
| tux/cogs/admin/eval.py | Modified eval command to check sysadmin permissions based on config. |
| tux/app.py | Updated owner_ids initialization by including sysadmin IDs conditionally. |
| config/settings.yml.example | Updated documentation with warnings and instructions about sysadmin eval usage. |
Contributor
There was a problem hiding this comment.
Hey @electron271 - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
ephemeralflag isn't supported on standard prefix command context.send (link)
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 2 other issues
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Deploying tux with
|
| Latest commit: |
aa96f49
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://249f65d6.tux-afh.pages.dev |
| Branch Preview URL: | https://eval.tux-afh.pages.dev |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
guh
Summary by Sourcery
Implement configurable sysadmin permissions for the eval command, allowing or disallowing sysadmins to execute eval based on a new setting and providing clear warnings at startup and on unauthorized use.
New Features:
Enhancements:
Documentation: