fix: let tenant admins manage models of their own tenant - #4043
Merged
Merged
Conversation
#4008 closed a horizontal-privilege hole on /model/manage/* by restricting the endpoints to the SU role. The whitelist did not distinguish a cross-tenant call from one naming the caller's own tenant, so ADMIN users lost the whole Models tab on /resource-manage: manage/list returned 403 and the page rendered an empty table with no error, because the create, update, delete, healthcheck and provider endpoints share the same guard. The page always sends the caller's own tenant_id (UserManageComp falls back to user.tenantId for non-SU), so rejecting ADMIN blocked no cross-tenant access -- it only broke tenant admins managing their own models. Replace the role whitelist with a role + tenant scope check: SU may target any tenant, ADMIN only the tenant its token belongs to, and every other role is rejected as before. ADMIN and SU share the same model:* permission seeds, so the role itself still has to be checked -- permissions alone cannot separate them. The existing ADMIN cross-tenant test kept passing because it used a foreign tenant_id, which is why the regression went uncaught. Add own-tenant coverage for list and update, plus a DEV case proving require(model:read) is not sufficient on its own to reach the manage surface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jeffwu-1999
requested review from
Dallas98,
WMC001,
YehongPan and
hhhhsc701
as code owners
September 29, 2026 13:21
WMC001
approved these changes
Sep 29, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
3 participants
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.
fix: let tenant admins manage models of their own tenant
#4008 restricted the cross-tenant /model/manage/* endpoints to the SU role.
The whitelist could not tell a foreign-tenant call from one naming the
caller's own tenant, so ADMIN users lost the whole Models tab on
/resource-manage: manage/list returned 403 and the page rendered an empty
table without surfacing an error.
tenant, ADMIN only the tenant its bearer token belongs to, and every other
role is rejected as before
separately because ADMIN and SU share the same model:* permission seeds
delete, batch_create, healthcheck, provider/list, provider/create)
tenant_id, which is why losing own-tenant access went uncaught
case proving require(model:read) alone cannot reach the manage surface