[Fix • Issue #41] - Automatically assign commands permission groups based on ranks / roles

[Mophyr]

Provides a fix for issue #41.

[Copilot]

Pull request overview

This PR attempts to implement automatic assignment of command permission groups based on player ranks in Roblox groups to resolve issue #41. However, the implementation has several critical bugs that need to be addressed before merging.

Key Changes:

• Added server-side assertions to permission-related methods to ensure they're only called on the server
• Implemented automatic permission assignment for players based on their group rank when they join
• Added (incomplete) role-based permission support in CmdrUtil
• Added error message to GiveRobloxGroupRolePermissionGroups method indicating the feature is not yet functional

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
lib/cmdrhandler/src/Shared/PermissionsHandler.luau	Added IS_SERVER constant, server-side assertions to permission methods, error for role-based permissions, and formatting improvements
lib/cmdrhandler/src/Shared/CmdrUtil.luau	Added getGroupRolePermissions function (buggy implementation), integrated role permissions into getPlayerPermissions, and code formatting improvements
lib/cmdrhandler/src/Server/init.luau	Implemented automatic rank-based permission assignment on player join, with commented-out role-based implementation

Comments suppressed due to low confidence (2)

lib/cmdrhandler/src/Shared/CmdrUtil.luau:111

• The getGroupRolePermissions function has several critical issues:

1. Wrong data source: It's using cmdrWrapper:_getRawGroupPerms(groupId) which returns rank permission data (array format with Ranks/Permissions), but it should be accessing role permission data which has a different structure: { [roleName]: { string } }.

2. Missing role matching logic: The function never checks if groupPermData matches the provided role parameter. It iterates over all permissions regardless of role.

3. Incorrect data iteration: The function assumes groupPermData is an array with Permissions fields, but role permissions are stored as { [GroupId]: { [roleName]: permissionsArray } }.

The function should be accessing cmdrWrapper:_getGroupRolePermissionGroupsData()[groupId][role] to get the permissions for a specific role, or a new _getRawGroupRolePerms method should be added to the PermissionsHandler.

function Util.getGroupRolePermissions(cmdrWrapper, groupId: number, role: string): { string }
	local groupData = cmdrWrapper:_getRawGroupPerms(groupId)
	local rolePerms = {}

	-- This is terribly unoptimized, feel free to optimize it
	for _, groupPermData in groupData do
		for _, permissionGroup: string in groupPermData.Permissions do
			if not table.find(rolePerms, permissionGroup) then
				table.insert(rolePerms, permissionGroup)
			end
			for _, inheritedPerm in cmdrWrapper:GetPermissionInheritance(permissionGroup) do
				if not table.find(rolePerms, inheritedPerm) then
					table.insert(rolePerms, inheritedPerm)
				end
			end
		end
	end

	return rolePerms
end

lib/cmdrhandler/src/Shared/CmdrUtil.luau:63

• The call to Util.getGroupRolePermissions on line 59 will not work correctly because:

1. The function being called has critical bugs (see comment on the function implementation)
2. Even if the function worked, the role-based permission feature is marked as @unreleased and will error when GiveRobloxGroupRolePermissionGroups is called (see line 230-232 in PermissionsHandler.luau)

This code should be removed or commented out until the role-based permissions feature is properly implemented and the Roblox API issue is resolved.

		for _, groupPerm in Util.getGroupRolePermissions(cmdrWrapper, groupData.Id, groupData.Role) do
			if not table.find(playerPerms, groupPerm) then
				table.insert(playerPerms, groupPerm)
			end
		end

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Util.getGroupRolePermissions ignores the role parameter and returns the union of ALL permission groups configured for the Roblox group (from _getRawGroupPerms), effectively bypassing rank/role restrictions. Any player in the group will be granted permissions intended for higher ranks/roles because this function does not filter by role (or any constraint) before appending permissions. Fix by either (a) removing this function until role-based data is properly stored and retrieved, or (b) fetching role-specific mappings (e.g., via a dedicated _getGroupRolePermissionGroupsData() path) and returning only permissions for the matching role (with inheritance applied), never reading rank-based entries here.

Suggested change

	-- This is terribly unoptimized, feel free to optimize it
	for _, groupPermData in groupData do
	for _, permissionGroup: string in groupPermData.Permissions do
	if not table.find(rolePerms, permissionGroup) then
	table.insert(rolePerms, permissionGroup)
	end
	for _, inheritedPerm in cmdrWrapper:GetPermissionInheritance(permissionGroup) do
	if not table.find(rolePerms, inheritedPerm) then
	table.insert(rolePerms, inheritedPerm)
	end
	-- Only include permissions for the specified role
	for _, groupPermData in groupData do
	if groupPermData.Role == role then
	for _, permissionGroup: string in groupPermData.Permissions do
	if not table.find(rolePerms, permissionGroup) then
	table.insert(rolePerms, permissionGroup)
	end
	for _, inheritedPerm in cmdrWrapper:GetPermissionInheritance(permissionGroup) do
	if not table.find(rolePerms, inheritedPerm) then
	table.insert(rolePerms, inheritedPerm)
	end
	end

[Copilot]

In Util.getPlayerPermissions, the added loop calling Util.getGroupRolePermissions grants additional permissions to every group member, because getGroupRolePermissions (as implemented) returns all permissions for the group regardless of role. This causes privilege escalation where low-rank members inherit permissions intended only for high ranks/roles. Fix by removing this loop until role-based permissions are correctly implemented, or by ensuring getGroupRolePermissions strictly returns permissions for the player’s actual role and that underlying data is truly role-scoped.

Suggested change

	for _, groupPerm in Util.getGroupRolePermissions(cmdrWrapper, groupData.Id, groupData.Role) do
	if not table.find(playerPerms, groupPerm) then
	table.insert(playerPerms, groupPerm)
	end
	end