🔒 Use SafeConstructor for YAML parsing and add fine-grained permissions#248
🔒 Use SafeConstructor for YAML parsing and add fine-grained permissions#248
Conversation
- Replace unsafe Yaml() with Yaml(SafeConstructor()) to prevent deserialization gadget attacks from malicious plugin JARs - Add per-command permissions (mpm.command.add, .remove, .install, etc.) - Register mpm.command as parent permission with all children for backward compatibility (operators with mpm.command get all sub-perms) Closes #234
🚀 Preview of mpmThis preview file are availalble for 7 days. (until 2026-04-04)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8249a019a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| org.bukkit.permissions.PermissionDefault.OP, | ||
| children | ||
| ) | ||
| server.pluginManager.addPermission(parentPermission) |
There was a problem hiding this comment.
Check for existing parent permission before adding it
Calling addPermission() unconditionally here can throw an IllegalArgumentException if mpm.command is already registered (for example after plugin reloads in the same JVM, or when another plugin pre-registers the same node), which will abort onEnable() and keep the plugin disabled. Please guard this with a getPermission("mpm.command") check (or update the existing permission) instead of always adding a new one.
Useful? React with 👍 / 👎.
Summary
Yaml()withYaml(SafeConstructor(LoaderOptions()))in PluginDataUtils to prevent deserialization gadget attacksmpm.command.add,mpm.command.remove,mpm.command.install,mpm.command.uninstall,mpm.command.update,mpm.command.list,mpm.command.backup,mpm.command.lock,mpm.command.init,mpm.command.reload)mpm.commandas parent permission containing all childrenTest plan
./gradlew checkpassesmpm.commandstill grants access to all commandsmpm.command.listwithoutmpm.command.update)Closes #234