Require permission for namespace plugin sync#6384
Open
Aias00 wants to merge 12 commits into
Open
Conversation
The merge brings in upstream workflow hardening while preserving this branch's existing mvnd-based build flow and k8s test changes. Conflicts were limited to Java setup action selection and the new integrated-test Docker image pre-pull step, so the resolution keeps the retrying local setup-java wrapper and retains the pre-pull retries. Constraint: Merge was already in progress against origin/master at 86e544b Rejected: Keep direct actions/setup-java@v4 | would drop upstream retry behavior for transient setup-java failures Confidence: high Scope-risk: moderate Directive: Keep workflow Java setup calls on ./actions/setup-java-with-retry unless upstream removes the retry action Tested: No conflict markers remain in conflicted workflows; Ruby YAML parse for the three edited workflows; git diff --check for the three edited workflows Not-tested: Full GitHub Actions execution; full repository build/test suite
The namespace plugin sync endpoint can publish uploaded plugin JAR data to connected gateway nodes, so it needs the same plugin modify permission already required by the bulk sync path. This adds the missing method-level Shiro permission and locks the contract with a focused controller regression test. Constraint: Existing Shiro method authorization depends on @RequiresPermissions annotations. Rejected: Add a new permission name | existing syncPluginAll already uses system:plugin:modify for the same operation family. Confidence: high Scope-risk: narrow Directive: Keep single-plugin and bulk plugin sync endpoints on equivalent plugin sync permissions. Tested: ./mvnw -pl shenyu-admin -Dskip.checkstyle=true -DskipLicense=true -Dspotless.check.skip=true -Djacoco.skip=true -Dtest=NamespacePluginControllerTest test -q Tested: ./mvnw -pl shenyu-admin -Dskip.checkstyle=true -DskipLicense=true -Dspotless.check.skip=true -Djacoco.skip=true -Dtest=NamespacePluginControllerTest,DataPermissionControllerTest,SelectorControllerTest test -q Tested: ./mvnw -pl shenyu-admin -Dskip.checkstyle=true -DskipLicense=true -Dspotless.check.skip=true -Djacoco.skip=true test -q Tested: ./mvnw -pl shenyu-admin -DskipTests -Djacoco.skip=true verify -q Not-tested: End-to-end admin login and WebSocket sync against a running bootstrap.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR closes an authorization gap in the shenyu-admin namespace plugin sync API by enforcing the same system:plugin:modify permission on the single-plugin sync endpoint as is already required for the bulk sync endpoint.
Changes:
- Added
@RequiresPermissions("system:plugin:modify")toNamespacePluginController#syncPluginData. - Added a unit test that asserts the permission annotation is present for
syncPluginData.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/NamespacePluginController.java | Enforces system:plugin:modify permission on the single namespace plugin sync endpoint. |
| shenyu-admin/src/test/java/org/apache/shenyu/admin/controller/NamespacePluginControllerTest.java | Adds a reflection-based test to prevent regressions in the permission requirement for syncPluginData. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yu199195
approved these changes
Jun 17, 2026
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
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.
This PR adds the missing
system:plugin:modifypermission check to the single namespace plugin sync endpoint.The bulk plugin sync endpoint already requires
system:plugin:modify; the single-plugin sync path can publish plugin data, including uploaded plugin JAR content, to connected gateway nodes and should enforce the same permission boundary.Make sure that:
./mvnw clean install -Dmaven.javadoc.skip=true.Tested:
./mvnw -pl shenyu-admin -Dskip.checkstyle=true -DskipLicense=true -Dspotless.check.skip=true -Djacoco.skip=true -Dtest=NamespacePluginControllerTest test -q./mvnw -pl shenyu-admin -Dskip.checkstyle=true -DskipLicense=true -Dspotless.check.skip=true -Djacoco.skip=true -Dtest=NamespacePluginControllerTest,DataPermissionControllerTest,SelectorControllerTest test -q./mvnw -pl shenyu-admin -Dskip.checkstyle=true -DskipLicense=true -Dspotless.check.skip=true -Djacoco.skip=true test -q./mvnw -pl shenyu-admin -DskipTests -Djacoco.skip=true verify -q