From dc8d898f2049c1c1bcaefd001c2ae1d15c63b51f Mon Sep 17 00:00:00 2001 From: Ramgopal Nagaboina Date: Thu, 10 Sep 2026 17:54:43 -0400 Subject: [PATCH 1/2] server: compare the root admin resource limit against unlimited as a long The root admin check in updateResourceLimit used max.shortValue() != RESOURCE_UNLIMITED. max is a Long, so shortValue() keeps only the low 16 bits: a finite limit such as 65535 or 131071 truncates to -1, passes the "Only -1 limit is supported for Root Admin accounts" check and is stored on the root admin account. Compare the full value instead. --- .../ResourceLimitManagerImpl.java | 2 +- .../ResourceLimitManagerImplTest.java | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 6d2ec103ca21..5d14a9e2f9f7 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -931,7 +931,7 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege } //only Unlimited value is accepted if account is Root Admin - if (_accountMgr.isRootAdmin(account.getId()) && max.shortValue() != Resource.RESOURCE_UNLIMITED) { + if (_accountMgr.isRootAdmin(account.getId()) && max != Resource.RESOURCE_UNLIMITED) { throw new InvalidParameterValueException("Only " + Resource.RESOURCE_UNLIMITED + " limit is supported for Root Admin accounts"); } diff --git a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java index bdc6620c4904..a8b24251e92d 100644 --- a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java +++ b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java @@ -26,6 +26,7 @@ import org.apache.cloudstack.api.response.AccountResponse; import org.apache.cloudstack.api.response.DomainResponse; import org.apache.cloudstack.api.response.TaggedResourceLimitAndCountResponse; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.reservation.dao.ReservationDao; import org.apache.cloudstack.resourcelimit.Reserver; @@ -57,6 +58,7 @@ import com.cloud.domain.Domain; import com.cloud.domain.DomainVO; import com.cloud.domain.dao.DomainDao; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceAllocationException; import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; @@ -73,8 +75,10 @@ import com.cloud.user.AccountManager; import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; +import com.cloud.user.User; import com.cloud.user.dao.AccountDao; import com.cloud.utils.Pair; +import com.cloud.utils.db.EntityManager; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.UserVmDao; @@ -123,6 +127,8 @@ public class ResourceLimitManagerImplTest extends TestCase { UserVmDao userVmDao; @Mock SnapshotDataStoreDao snapshotDataStoreDao; + @Mock + EntityManager entityManager; private List hostTags = List.of("htag1", "htag2", "htag3"); private List storageTags = List.of("stag1", "stag2"); @@ -1178,4 +1184,22 @@ public void testDecrementVmMemoryResourceCount() { Mockito.verify(resourceLimitManager, Mockito.times(1)) .decrementResourceCountWithTag(accountId, Resource.ResourceType.memory, tag, Long.valueOf(memory)); } + + @Test + public void updateResourceLimitRejectsAFiniteLimitForRootAdminEvenWhenItsLowBitsLookUnlimited() { + long rootAdminAccountId = 2L; + Account rootAdmin = Mockito.mock(Account.class); + Mockito.when(rootAdmin.getId()).thenReturn(rootAdminAccountId); + Mockito.when(entityManager.findById(Account.class, rootAdminAccountId)).thenReturn(rootAdmin); + Mockito.when(accountManager.isRootAdmin(rootAdminAccountId)).thenReturn(true); + + CallContext.register(Mockito.mock(User.class), rootAdmin); + try { + InvalidParameterValueException e = Assert.assertThrows(InvalidParameterValueException.class, () -> + resourceLimitManager.updateResourceLimit(rootAdminAccountId, null, Resource.ResourceType.user_vm.getOrdinal(), 65535L, null)); + Assert.assertEquals("Only -1 limit is supported for Root Admin accounts", e.getMessage()); + } finally { + CallContext.unregister(); + } + } } From 219aeb0db62c96a083e8836a1c82d69a80436206 Mon Sep 17 00:00:00 2001 From: rootnomad Date: Mon, 26 Oct 2026 15:10:35 -0400 Subject: [PATCH 2/2] server: make the long comparison explicit in the root admin limit check Cast RESOURCE_UNLIMITED to long in the root-admin unlimited-only guard, as suggested in review. max is a Long, so the check already compared as long once shortValue() was dropped; the cast makes that intent obvious. --- .../java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 5d14a9e2f9f7..f97ab8cc5359 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -931,7 +931,7 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege } //only Unlimited value is accepted if account is Root Admin - if (_accountMgr.isRootAdmin(account.getId()) && max != Resource.RESOURCE_UNLIMITED) { + if (_accountMgr.isRootAdmin(account.getId()) && max != (long) Resource.RESOURCE_UNLIMITED) { throw new InvalidParameterValueException("Only " + Resource.RESOURCE_UNLIMITED + " limit is supported for Root Admin accounts"); }