You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by we...@apache.org on 2025/10/16 07:49:53 UTC

(cloudstack) branch 4.20 updated (eee43e534fa -> 03a4b9f4fd3)

This is an automated email from the ASF dual-hosted git repository.

weizhou pushed a change to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


    from eee43e534fa cloudutils: fix warning, error during kvm agent installation (#11318)
     new c8d44d92a76 api,server: fix entity access
     new 03a4b9f4fd3 server,utils: improve js interpretation functionality

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/com/cloud/server/ManagementService.java   |  13 +-
 .../java/com/cloud/server/ResourceManagerUtil.java |   1 +
 .../apache/cloudstack/quota/QuotaManagerImpl.java  |   4 +-
 .../cloudstack/quota/QuotaManagerImplTest.java     |   4 +-
 .../api/response/QuotaResponseBuilderImpl.java     |  23 ++-
 .../org/apache/cloudstack/quota/QuotaService.java  |  12 +-
 .../apache/cloudstack/quota/QuotaServiceImpl.java  |   9 ++
 .../cloudstack/metrics/MetricsServiceImpl.java     |  76 ++++++++-
 .../cloudstack/metrics/MetricsServiceImplTest.java | 163 ++++++++++++++++----
 .../java/com/cloud/api/query/QueryManagerImpl.java |   2 +-
 .../cloud/network/vpc/NetworkACLServiceImpl.java   |   3 +
 .../com/cloud/resource/ResourceManagerImpl.java    |   6 +
 .../com/cloud/server/ManagementServerImpl.java     |  22 ++-
 .../java/com/cloud/storage/StorageManagerImpl.java |   9 ++
 .../com/cloud/tags/ResourceManagerUtilImpl.java    |  11 ++
 .../utils/jsinterpreter/JsInterpreter.java         | 169 ++++++++++++++-------
 .../utils/jsinterpreter/TagAsRuleHelper.java       |   7 +-
 .../utils/jsinterpreter/JsInterpreterTest.java     |  81 ++++++----
 18 files changed, 473 insertions(+), 142 deletions(-)


(cloudstack) 01/02: api,server: fix entity access

Posted by we...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

weizhou pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit c8d44d92a76efe6dd197128dab8faf5f0f8ac85b
Author: Abhishek Kumar <ab...@gmail.com>
AuthorDate: Wed Sep 17 02:34:31 2025 +0530

    api,server: fix entity access
    
    Added access check for:
    - createNetworkACL
    - listNetworkACLs
    - listResourceDetails
    - listVirtualMachinesUsageHistory
    - listVolumesUsageHistory
    
    Signed-off-by: Abhishek Kumar <ab...@gmail.com>
---
 .../java/com/cloud/server/ResourceManagerUtil.java |   1 +
 .../cloudstack/metrics/MetricsServiceImpl.java     |  76 +++++++++-
 .../cloudstack/metrics/MetricsServiceImplTest.java | 163 +++++++++++++++++----
 .../java/com/cloud/api/query/QueryManagerImpl.java |   2 +-
 .../cloud/network/vpc/NetworkACLServiceImpl.java   |   3 +
 .../com/cloud/tags/ResourceManagerUtilImpl.java    |  11 ++
 6 files changed, 221 insertions(+), 35 deletions(-)

diff --git a/api/src/main/java/com/cloud/server/ResourceManagerUtil.java b/api/src/main/java/com/cloud/server/ResourceManagerUtil.java
index 9a3b51a70d5..f5081cbe307 100644
--- a/api/src/main/java/com/cloud/server/ResourceManagerUtil.java
+++ b/api/src/main/java/com/cloud/server/ResourceManagerUtil.java
@@ -18,6 +18,7 @@ package com.cloud.server;
 
 public interface ResourceManagerUtil {
     long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType);
+    long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType, boolean checkAccess);
     String getUuid(String resourceId, ResourceTag.ResourceObjectType resourceType);
     ResourceTag.ResourceObjectType getResourceType(String resourceTypeStr);
     void checkResourceAccessible(Long accountId, Long domainId, String exceptionMessage);
diff --git a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
index 0ef094d3d4e..c305d9312ba 100644
--- a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
+++ b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
@@ -22,6 +22,7 @@ import static com.cloud.utils.NumbersUtil.toReadableSize;
 import java.lang.reflect.InvocationTargetException;
 import java.text.DecimalFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -32,8 +33,6 @@ import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import com.cloud.dc.ClusterVO;
-import com.cloud.utils.Ternary;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.ListClustersMetricsCmd;
 import org.apache.cloudstack.api.ListDbMetricsCmd;
@@ -100,6 +99,7 @@ import com.cloud.capacity.CapacityManager;
 import com.cloud.capacity.dao.CapacityDao;
 import com.cloud.capacity.dao.CapacityDaoImpl;
 import com.cloud.cluster.dao.ManagementServerHostDao;
+import com.cloud.dc.ClusterVO;
 import com.cloud.dc.DataCenter;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.dc.dao.DataCenterDao;
@@ -112,6 +112,7 @@ import com.cloud.host.Status;
 import com.cloud.host.dao.HostDao;
 import com.cloud.network.router.VirtualRouter;
 import com.cloud.org.Cluster;
+import com.cloud.projects.Project;
 import com.cloud.server.DbStatsCollection;
 import com.cloud.server.ManagementServerHostStats;
 import com.cloud.server.StatsCollector;
@@ -124,6 +125,7 @@ import com.cloud.usage.dao.UsageJobDao;
 import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
 import com.cloud.utils.Pair;
+import com.cloud.utils.Ternary;
 import com.cloud.utils.db.DbProperties;
 import com.cloud.utils.db.DbUtil;
 import com.cloud.utils.db.Filter;
@@ -184,6 +186,10 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements
 
     private static Gson gson = new Gson();
 
+    private final List<Account.Type> AccountTypesWithRecursiveUsageAccess = Arrays.asList(
+            Account.Type.ADMIN, Account.Type.DOMAIN_ADMIN, Account.Type.READ_ONLY_ADMIN
+    );
+
     protected MetricsServiceImpl() {
         super();
     }
@@ -245,17 +251,30 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements
      * @return the list of VMs.
      */
     protected Pair<List<UserVmVO>, Integer> searchForUserVmsInternal(ListVMsUsageHistoryCmd cmd) {
+        final Long id = cmd.getId();
+        Account caller = CallContext.current().getCallingAccount();
+        List<Long> permittedAccounts = new ArrayList<>();
+        boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType());
+        Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, recursive, null);
+        accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false);
+        Long domainId = domainIdRecursiveListProject.first();
+        Boolean isRecursive = domainIdRecursiveListProject.second();
+        Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
+
         Filter searchFilter = new Filter(UserVmVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal());
         List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());
         String name = cmd.getName();
         String keyword = cmd.getKeyword();
 
         SearchBuilder<UserVmVO> sb =  userVmDao.createSearchBuilder();
+        sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); // select distinct
+        accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria);
         sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN);
         sb.and("displayName", sb.entity().getDisplayName(), SearchCriteria.Op.LIKE);
         sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ);
 
         SearchCriteria<UserVmVO> sc = sb.create();
+        accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria);
         if (CollectionUtils.isNotEmpty(ids)) {
             sc.setParameters("idIN", ids.toArray());
         }
@@ -269,7 +288,14 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements
             sc.addAnd("displayName", SearchCriteria.Op.SC, ssc);
         }
 
-        return userVmDao.searchAndCount(sc, searchFilter);
+        Pair<List<UserVmVO>, Integer> uniqueVmPair = userVmDao.searchAndCount(sc, searchFilter);
+        Integer count = uniqueVmPair.second();
+        if (count == 0) {
+            return new Pair<>(new ArrayList<>(), count);
+        }
+        List<Long> vmIds = uniqueVmPair.first().stream().map(UserVmVO::getId).collect(Collectors.toList());
+        List<UserVmVO> vms = userVmDao.listByIds(vmIds);
+        return new Pair<>(vms, count);
     }
 
     /**
@@ -331,17 +357,49 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements
      * @return the list of VMs.
      */
     protected Pair<List<VolumeVO>, Integer> searchForVolumesInternal(ListVolumesUsageHistoryCmd cmd) {
+        final Long id = cmd.getId();
+        Account caller = CallContext.current().getCallingAccount();
+        List<Long> permittedAccounts = new ArrayList<>();
+        boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType());
+        Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, recursive, null);
+        accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false);
+        Long domainId = domainIdRecursiveListProject.first();
+        Boolean isRecursive = domainIdRecursiveListProject.second();
+        Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
+
         Filter searchFilter = new Filter(VolumeVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal());
         List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());
         String name = cmd.getName();
         String keyword = cmd.getKeyword();
 
         SearchBuilder<VolumeVO> sb =  volumeDao.createSearchBuilder();
+        sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); // select distinct
+        accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria);
         sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN);
         sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ);
         sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ);
 
+        boolean shouldListSystemVmVolumes = accountMgr.isRootAdmin(CallContext.current().getCallingAccountId());
+        List<Long> vmIds = new ArrayList<>();
+        if (!shouldListSystemVmVolumes) {
+            SearchBuilder<UserVmVO> vmSearch = userVmDao.createSearchBuilder();
+            vmSearch.select(null, SearchCriteria.Func.DISTINCT, vmSearch.entity().getId());
+            accountMgr.buildACLSearchBuilder(vmSearch, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria);
+            SearchCriteria<UserVmVO> vmSc = vmSearch.create();
+            accountMgr.buildACLSearchCriteria(vmSc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria);
+            List<UserVmVO> vms = userVmDao.search(vmSc, null);
+            vmIds = vms.stream().map(UserVmVO::getId).collect(Collectors.toList());
+            if (vmIds.isEmpty()) {
+                sb.and("instanceIdNull", sb.entity().getInstanceId(), SearchCriteria.Op.NULL);
+            } else {
+                sb.and().op("instanceIdNull", sb.entity().getInstanceId(), SearchCriteria.Op.NULL);
+                sb.or("instanceIds", sb.entity().getInstanceId(), SearchCriteria.Op.IN);
+                sb.cp();
+            }
+        }
+
         SearchCriteria<VolumeVO> sc = sb.create();
+        accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria);
         if (CollectionUtils.isNotEmpty(ids)) {
             sc.setParameters("idIN", ids.toArray());
         }
@@ -354,8 +412,18 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements
             ssc.addOr("state", SearchCriteria.Op.EQ, keyword);
             sc.addAnd("name", SearchCriteria.Op.SC, ssc);
         }
+        if (!shouldListSystemVmVolumes && CollectionUtils.isNotEmpty(vmIds)) {
+            sc.setParameters("instanceIds", vmIds.toArray());
+        }
 
-        return volumeDao.searchAndCount(sc, searchFilter);
+        Pair<List<VolumeVO>, Integer> uniqueVolumePair = volumeDao.searchAndCount(sc, searchFilter);
+        Integer count = uniqueVolumePair.second();
+        if (count == 0) {
+            return new Pair<>(new ArrayList<>(), count);
+        }
+        List<Long> volumeIds = uniqueVolumePair.first().stream().map(VolumeVO::getId).collect(Collectors.toList());
+        List<VolumeVO> volumes = volumeDao.listByIds(volumeIds);
+        return new Pair<>(volumes, count);
     }
 
     /**
diff --git a/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java b/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java
index b37be68b3e4..9184d744410 100644
--- a/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java
+++ b/plugins/metrics/src/test/java/org/apache/cloudstack/metrics/MetricsServiceImplTest.java
@@ -19,13 +19,16 @@ package org.apache.cloudstack.metrics;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import org.apache.cloudstack.api.ListVMsUsageHistoryCmd;
+import org.apache.cloudstack.api.ListVolumesUsageHistoryCmd;
 import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.response.VmMetricsStatsResponse;
 import org.apache.commons.lang3.time.DateUtils;
 import org.junit.Assert;
@@ -35,12 +38,18 @@ import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 import org.mockito.Spy;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
 import com.cloud.utils.Pair;
+import com.cloud.utils.db.Filter;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
 import com.cloud.vm.UserVmVO;
@@ -75,6 +84,12 @@ public class MetricsServiceImplTest {
     @Mock
     VmStatsDao vmStatsDaoMock;
 
+    @Mock
+    AccountManager accountManager;
+
+    @Mock
+    VolumeDao volumeDao;
+
     @Captor
     ArgumentCaptor<String> stringCaptor1, stringCaptor2;
 
@@ -95,12 +110,26 @@ public class MetricsServiceImplTest {
 
     @Mock
     VmStatsVO vmStatsVOMock;
+    @Mock
+    Account mockAccount;
+    @Mock
+    ListVolumesUsageHistoryCmd listVolumesUsageHistoryCmdMock;
+    @Mock
+    VolumeVO volumeVOMock;
+    @Mock
+    SearchBuilder<VolumeVO> volumeSearchBuilderMock;
+    @Mock
+    SearchCriteria<VolumeVO> volumeSearchCriteriaMock;
+    @Mock
+    Filter filterMock;
+
 
     private void prepareSearchCriteriaWhenUseSetParameters() {
         Mockito.doNothing().when(scMock).setParameters(Mockito.anyString(), Mockito.any());
     }
 
     private void preparesearchForUserVmsInternalTest() {
+        Mockito.when(mockAccount.getType()).thenReturn(Account.Type.NORMAL);
         expectedVmListAndCounter = new Pair<>(Arrays.asList(userVmVOMock), 1);
 
         Mockito.doReturn(1L).when(listVMsUsageHistoryCmdMock).getStartIndex();
@@ -111,8 +140,10 @@ public class MetricsServiceImplTest {
         Mockito.doReturn(userVmVOMock).when(sbMock).entity();
         Mockito.doReturn(scMock).when(sbMock).create();
 
-        Mockito.doReturn(new Pair<List<UserVmVO>, Integer>(Arrays.asList(userVmVOMock), 1))
+        Mockito.doReturn(expectedVmListAndCounter)
         .when(userVmDaoMock).searchAndCount(Mockito.any(), Mockito.any());
+        Mockito.doReturn(expectedVmListAndCounter.first())
+                .when(userVmDaoMock).listByIds(Mockito.anyList());
     }
 
     @Test
@@ -124,12 +155,17 @@ public class MetricsServiceImplTest {
         Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getName();
         Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getKeyword();
 
-        Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock);
-
-        Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture());
-        Assert.assertEquals("idIN", stringCaptor1.getValue());
-        Assert.assertEquals(fakeVmId1, objectArrayCaptor.getAllValues().get(0)[0]);
-        Assert.assertEquals(expectedVmListAndCounter, result);
+        try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {
+            CallContext callContextMock = Mockito.mock(CallContext.class);
+            callContextMocked.when(CallContext::current).thenReturn(callContextMock);
+            Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount);
+            Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock);
+
+            Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture());
+            Assert.assertEquals("idIN", stringCaptor1.getValue());
+            Assert.assertEquals(fakeVmId1, objectArrayCaptor.getAllValues().get(0)[0]);
+            Assert.assertEquals(expectedVmListAndCounter, result);
+        }
     }
 
     @Test
@@ -141,13 +177,17 @@ public class MetricsServiceImplTest {
         Mockito.doReturn(expected).when(listVMsUsageHistoryCmdMock).getIds();
         Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getName();
         Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getKeyword();
-
-        Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock);
-
-        Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture());
-        Assert.assertEquals("idIN", stringCaptor1.getValue());
-        Assert.assertArrayEquals(expected.toArray(), objectArrayCaptor.getAllValues().get(0));
-        Assert.assertEquals(expectedVmListAndCounter, result);
+        try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {
+            CallContext callContextMock = Mockito.mock(CallContext.class);
+            callContextMocked.when(CallContext::current).thenReturn(callContextMock);
+            Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount);
+            Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock);
+
+            Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture());
+            Assert.assertEquals("idIN", stringCaptor1.getValue());
+            Assert.assertArrayEquals(expected.toArray(), objectArrayCaptor.getAllValues().get(0));
+            Assert.assertEquals(expectedVmListAndCounter, result);
+        }
     }
 
     @Test
@@ -159,12 +199,17 @@ public class MetricsServiceImplTest {
         Mockito.doReturn("fakeName").when(listVMsUsageHistoryCmdMock).getName();
         Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getKeyword();
 
-        Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock);
-
-        Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture());
-        Assert.assertEquals("displayName", stringCaptor1.getValue());
-        Assert.assertEquals("%fakeName%", objectArrayCaptor.getValue()[0]);
-        Assert.assertEquals(expectedVmListAndCounter, result);
+        try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {
+            CallContext callContextMock = Mockito.mock(CallContext.class);
+            callContextMocked.when(CallContext::current).thenReturn(callContextMock);
+            Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount);
+            Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock);
+
+            Mockito.verify(scMock).setParameters(stringCaptor1.capture(), objectArrayCaptor.capture());
+            Assert.assertEquals("displayName", stringCaptor1.getValue());
+            Assert.assertEquals("%fakeName%", objectArrayCaptor.getValue()[0]);
+            Assert.assertEquals(expectedVmListAndCounter, result);
+        }
     }
 
     @Test
@@ -177,16 +222,21 @@ public class MetricsServiceImplTest {
         Mockito.doReturn(null).when(listVMsUsageHistoryCmdMock).getName();
         Mockito.doReturn("fakeKeyword").when(listVMsUsageHistoryCmdMock).getKeyword();
 
-        Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock);
-
-        Mockito.verify(scMock, Mockito.times(2)).addOr(stringCaptor1.capture(), opCaptor.capture(), objectArrayCaptor.capture());
-        List<String> conditions = stringCaptor1.getAllValues();
-        List<Object[]> params = objectArrayCaptor.getAllValues();
-        Assert.assertEquals("displayName", conditions.get(0));
-        Assert.assertEquals("state", conditions.get(1));
-        Assert.assertEquals("%fakeKeyword%", params.get(0)[0]);
-        Assert.assertEquals("fakeKeyword", params.get(1)[0]);
-        Assert.assertEquals(expectedVmListAndCounter, result);
+        try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {
+            CallContext callContextMock = Mockito.mock(CallContext.class);
+            callContextMocked.when(CallContext::current).thenReturn(callContextMock);
+            Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount);
+            Pair<List<UserVmVO>, Integer> result = spy.searchForUserVmsInternal(listVMsUsageHistoryCmdMock);
+
+            Mockito.verify(scMock, Mockito.times(2)).addOr(stringCaptor1.capture(), opCaptor.capture(), objectArrayCaptor.capture());
+            List<String> conditions = stringCaptor1.getAllValues();
+            List<Object[]> params = objectArrayCaptor.getAllValues();
+            Assert.assertEquals("displayName", conditions.get(0));
+            Assert.assertEquals("state", conditions.get(1));
+            Assert.assertEquals("%fakeKeyword%", params.get(0)[0]);
+            Assert.assertEquals("fakeKeyword", params.get(1)[0]);
+            Assert.assertEquals(expectedVmListAndCounter, result);
+        }
     }
 
     @Test
@@ -317,4 +367,57 @@ public class MetricsServiceImplTest {
 
         spy.createStatsResponse(Arrays.asList(vmStatsVOMock));
     }
+
+    @Test
+    public void searchForVolumesInternalWithValidParameters() {
+        Mockito.doReturn(null).when(listVolumesUsageHistoryCmdMock).getId();
+        Mockito.doReturn(Arrays.asList(1L, 2L)).when(listVolumesUsageHistoryCmdMock).getIds();
+        Mockito.doReturn("volumeName").when(listVolumesUsageHistoryCmdMock).getName();
+        Mockito.doReturn("keyword").when(listVolumesUsageHistoryCmdMock).getKeyword();
+        Mockito.doReturn(volumeSearchBuilderMock).when(volumeDao).createSearchBuilder();
+        Mockito.doReturn(volumeVOMock).when(volumeSearchBuilderMock).entity();
+        SearchBuilder vmSearchBuilderMock = Mockito.mock(SearchBuilder.class);
+        Mockito.doReturn(vmSearchBuilderMock).when(userVmDaoMock).createSearchBuilder();
+        Mockito.doReturn(userVmVOMock).when(vmSearchBuilderMock).entity();
+        Mockito.doReturn(volumeSearchCriteriaMock).when(volumeSearchBuilderMock).create();
+        Mockito.doReturn(volumeSearchCriteriaMock).when(volumeDao).createSearchCriteria();
+        Mockito.doReturn(new Pair<>(Arrays.asList(volumeVOMock), 1)).when(volumeDao).searchAndCount(Mockito.any(), Mockito.any());
+        Mockito.doReturn(Arrays.asList(volumeVOMock)).when(volumeDao).listByIds(Mockito.anyList());
+
+
+        try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {
+            CallContext callContextMock = Mockito.mock(CallContext.class);
+            callContextMocked.when(CallContext::current).thenReturn(callContextMock);
+            Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount);
+            Pair<List<VolumeVO>, Integer> result = spy.searchForVolumesInternal(listVolumesUsageHistoryCmdMock);
+
+            Assert.assertNotNull(result);
+            Assert.assertEquals(1, result.second().intValue());
+            Assert.assertEquals(volumeVOMock, result.first().get(0));
+        }
+    }
+
+    @Test
+    public void searchForVolumesInternalWithValidParametersNoItem() {
+        Mockito.doReturn(1L).when(listVolumesUsageHistoryCmdMock).getId();
+        Mockito.doReturn(volumeSearchBuilderMock).when(volumeDao).createSearchBuilder();
+        Mockito.doReturn(volumeVOMock).when(volumeSearchBuilderMock).entity();
+        Mockito.doReturn(volumeSearchCriteriaMock).when(volumeSearchBuilderMock).create();
+        Mockito.doReturn(new Pair<>(Collections.emptyList(), 0)).when(volumeDao).searchAndCount(Mockito.any(), Mockito.any());
+
+
+        try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {
+            CallContext callContextMock = Mockito.mock(CallContext.class);
+            callContextMocked.when(CallContext::current).thenReturn(callContextMock);
+            Mockito.when(callContextMock.getCallingAccount()).thenReturn(mockAccount);
+            Mockito.when(callContextMock.getCallingAccountId()).thenReturn(1L);
+            Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(true);
+            Mockito.when(mockAccount.getType()).thenReturn(Account.Type.ADMIN);
+            Pair<List<VolumeVO>, Integer> result = spy.searchForVolumesInternal(listVolumesUsageHistoryCmdMock);
+
+            Assert.assertNotNull(result);
+            Assert.assertEquals(0, result.second().intValue());
+        }
+    }
+
 }
diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
index 6fb9ab515cb..6ed2fed9cb6 100644
--- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
+++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
@@ -5335,7 +5335,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
 
         //Validation - 1.3
         if (resourceIdStr != null) {
-            resourceId = resourceManagerUtil.getResourceId(resourceIdStr, resourceType);
+            resourceId = resourceManagerUtil.getResourceId(resourceIdStr, resourceType, true);
         }
 
         List<? extends ResourceDetail> detailList = new ArrayList<>();
diff --git a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
index 0791ca7ecd3..f99ea6b9b8c 100644
--- a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
+++ b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
@@ -483,6 +483,8 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
             throw new InvalidParameterValueException("Cannot create Network ACL Item. ACL Id or network Id is required");
         }
         Network network = networkModel.getNetwork(createNetworkACLCmd.getNetworkId());
+        Account caller = CallContext.current().getCallingAccount();
+        _accountMgr.checkAccess(caller, null, true, network);
         if (network.getVpcId() == null) {
             throw new InvalidParameterValueException("Network: " + network.getUuid() + " does not belong to VPC");
         }
@@ -744,6 +746,7 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
 
         if (networkId != null) {
             final Network network = _networkDao.findById(networkId);
+            _accountMgr.checkAccess(caller, null, true, network);
             aclId = network.getNetworkACLId();
             if (aclId == null) {
                 // No aclId associated with the network.
diff --git a/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java b/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java
index c02f4136863..82d69992cdf 100644
--- a/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java
+++ b/server/src/main/java/com/cloud/tags/ResourceManagerUtilImpl.java
@@ -22,6 +22,7 @@ import java.util.Objects;
 
 import javax.inject.Inject;
 
+import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.api.Identity;
 import org.apache.cloudstack.api.InternalIdentity;
 import org.apache.cloudstack.context.CallContext;
@@ -128,6 +129,11 @@ public class ResourceManagerUtilImpl implements ResourceManagerUtil {
 
     @Override
     public long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType) {
+        return getResourceId(resourceId, resourceType, false);
+    }
+
+    @Override
+    public long getResourceId(String resourceId, ResourceTag.ResourceObjectType resourceType, boolean checkAccess) {
         Class<?> clazz = s_typeMap.get(resourceType);
         Object entity = entityMgr.findByUuid(clazz, resourceId);
         if (entity != null) {
@@ -138,6 +144,11 @@ public class ResourceManagerUtilImpl implements ResourceManagerUtil {
         }
         entity = entityMgr.findById(clazz, resourceId);
         if (entity != null) {
+            if (checkAccess && entity instanceof ControlledEntity) {
+                ControlledEntity controlledEntity = (ControlledEntity)entity;
+                Account caller = CallContext.current().getCallingAccount();
+                accountMgr.checkAccess(caller, null, false, controlledEntity);
+            }
             return ((InternalIdentity)entity).getId();
         }
         throw new InvalidParameterValueException("Unable to find resource by id " + resourceId + " and type " + resourceType);


(cloudstack) 02/02: server,utils: improve js interpretation functionality

Posted by we...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

weizhou pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit 03a4b9f4fd33d69717e2a21e5963e5f26612df90
Author: Abhishek Kumar <ab...@gmail.com>
AuthorDate: Wed Sep 17 01:20:38 2025 +0530

    server,utils: improve js interpretation functionality
    
    Make JS interpretation functionalities configurable via a hidden config
    - js.interpretation.enabled
    Default value is false, making such functionalities disabled, ie, new
    heuristic rules cannot be added or updated.
    
    For JsInterpretor, use --no-java --no-syntax-extensions args and a deny-all ClassFilter.
    Replace string-spliced vars with ENGINE_SCOPE Bindings, use a fresh ScriptContext per run, and compile before eval.
    Use a named daemon worker with hard timeouts and capture stdout.
    
    Signed-off-by: Abhishek Kumar <ab...@gmail.com>
---
 .../java/com/cloud/server/ManagementService.java   |  13 +-
 .../apache/cloudstack/quota/QuotaManagerImpl.java  |   4 +-
 .../cloudstack/quota/QuotaManagerImplTest.java     |   4 +-
 .../api/response/QuotaResponseBuilderImpl.java     |  23 ++-
 .../org/apache/cloudstack/quota/QuotaService.java  |  12 +-
 .../apache/cloudstack/quota/QuotaServiceImpl.java  |   9 ++
 .../com/cloud/resource/ResourceManagerImpl.java    |   6 +
 .../com/cloud/server/ManagementServerImpl.java     |  22 ++-
 .../java/com/cloud/storage/StorageManagerImpl.java |   9 ++
 .../utils/jsinterpreter/JsInterpreter.java         | 169 ++++++++++++++-------
 .../utils/jsinterpreter/TagAsRuleHelper.java       |   7 +-
 .../utils/jsinterpreter/JsInterpreterTest.java     |  81 ++++++----
 12 files changed, 252 insertions(+), 107 deletions(-)

diff --git a/api/src/main/java/com/cloud/server/ManagementService.java b/api/src/main/java/com/cloud/server/ManagementService.java
index 18f3e901cd9..d6d7f7a59c6 100644
--- a/api/src/main/java/com/cloud/server/ManagementService.java
+++ b/api/src/main/java/com/cloud/server/ManagementService.java
@@ -20,7 +20,6 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
-import com.cloud.user.UserData;
 import org.apache.cloudstack.api.command.admin.cluster.ListClustersCmd;
 import org.apache.cloudstack.api.command.admin.config.ListCfgGroupsByCmd;
 import org.apache.cloudstack.api.command.admin.config.ListCfgsByCmd;
@@ -66,6 +65,7 @@ import org.apache.cloudstack.api.command.user.vm.GetVMPasswordCmd;
 import org.apache.cloudstack.api.command.user.vmgroup.UpdateVMGroupCmd;
 import org.apache.cloudstack.config.Configuration;
 import org.apache.cloudstack.config.ConfigurationGroup;
+import org.apache.cloudstack.framework.config.ConfigKey;
 
 import com.cloud.alert.Alert;
 import com.cloud.capacity.Capacity;
@@ -85,6 +85,7 @@ import com.cloud.storage.GuestOSHypervisor;
 import com.cloud.storage.GuestOsCategory;
 import com.cloud.storage.StoragePool;
 import com.cloud.user.SSHKeyPair;
+import com.cloud.user.UserData;
 import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;
 import com.cloud.vm.InstanceGroup;
@@ -98,6 +99,14 @@ import com.cloud.vm.VirtualMachine.Type;
 public interface ManagementService {
     static final String Name = "management-server";
 
+    ConfigKey<Boolean> JsInterpretationEnabled = new ConfigKey<>("Hidden"
+            , Boolean.class
+            , "js.interpretation.enabled"
+            , "false"
+            , "Enable/Disable all JavaScript interpretation related functionalities to create or update Javascript rules."
+            , false
+            , ConfigKey.Scope.Global);
+
     /**
      * returns the a map of the names/values in the configuration table
      *
@@ -481,4 +490,6 @@ public interface ManagementService {
 
     Pair<Boolean, String> patchSystemVM(PatchSystemVMCmd cmd);
 
+    void checkJsInterpretationAllowedIfNeededForParameterValue(String paramName, boolean paramValue);
+
 }
diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java
index c9254814f46..99181f80c29 100644
--- a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java
+++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java
@@ -32,7 +32,6 @@ import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import com.cloud.user.Account;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.quota.activationrule.presetvariables.GenericPresetVariable;
 import org.apache.cloudstack.quota.activationrule.presetvariables.PresetVariableHelper;
@@ -62,6 +61,7 @@ import org.springframework.stereotype.Component;
 
 import com.cloud.usage.UsageVO;
 import com.cloud.usage.dao.UsageDao;
+import com.cloud.user.Account;
 import com.cloud.user.AccountVO;
 import com.cloud.user.dao.AccountDao;
 import com.cloud.utils.DateUtil;
@@ -467,7 +467,7 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager {
 
         }
 
-        jsInterpreter.injectStringVariable("resourceType", presetVariables.getResourceType());
+        jsInterpreter.injectVariable("resourceType", presetVariables.getResourceType());
         jsInterpreter.injectVariable("value", presetVariables.getValue().toString());
         jsInterpreter.injectVariable("zone", presetVariables.getZone().toString());
     }
diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java
index 5dfc12f7ef8..3b2ea54e86d 100644
--- a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java
+++ b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java
@@ -270,7 +270,7 @@ public class QuotaManagerImplTest {
         Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString());
         Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString());
         Mockito.verify(jsInterpreterMock, Mockito.never()).injectVariable(Mockito.eq("project"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectStringVariable(Mockito.eq("resourceType"), Mockito.anyString());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString());
         Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString());
         Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString());
     }
@@ -291,7 +291,7 @@ public class QuotaManagerImplTest {
         Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString());
         Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString());
         Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("project"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectStringVariable(Mockito.eq("resourceType"), Mockito.anyString());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString());
         Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString());
         Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString());
     }
diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java
index 1c486759e43..00110bb0c0a 100644
--- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java
+++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java
@@ -39,7 +39,6 @@ import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 
-import com.cloud.utils.DateUtil;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.command.QuotaBalanceCmd;
@@ -70,8 +69,8 @@ import org.apache.cloudstack.quota.dao.QuotaCreditsDao;
 import org.apache.cloudstack.quota.dao.QuotaEmailConfigurationDao;
 import org.apache.cloudstack.quota.dao.QuotaEmailTemplatesDao;
 import org.apache.cloudstack.quota.dao.QuotaTariffDao;
-import org.apache.cloudstack.quota.vo.QuotaAccountVO;
 import org.apache.cloudstack.quota.dao.QuotaUsageDao;
+import org.apache.cloudstack.quota.vo.QuotaAccountVO;
 import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
 import org.apache.cloudstack.quota.vo.QuotaCreditsVO;
 import org.apache.cloudstack.quota.vo.QuotaEmailConfigurationVO;
@@ -79,26 +78,28 @@ import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
 import org.apache.cloudstack.quota.vo.QuotaTariffVO;
 import org.apache.cloudstack.quota.vo.QuotaUsageVO;
 import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.reflect.FieldUtils;
-import org.apache.commons.lang3.ObjectUtils;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.springframework.stereotype.Component;
 
 import com.cloud.domain.DomainVO;
 import com.cloud.domain.dao.DomainDao;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
 import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.PermissionDeniedException;
 import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
 import com.cloud.user.AccountVO;
 import com.cloud.user.User;
 import com.cloud.user.dao.AccountDao;
 import com.cloud.user.dao.UserDao;
+import com.cloud.utils.DateUtil;
 import com.cloud.utils.Pair;
 import com.cloud.utils.db.Filter;
-import com.cloud.event.ActionEvent;
-import com.cloud.event.EventTypes;
 
 @Component
 public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
@@ -139,6 +140,12 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
     @Inject
     private ApiDiscoveryService apiDiscoveryService;
 
+    protected void checkActivationRulesAllowed(String activationRule) {
+        if (!_quotaService.isJsInterpretationEnabled() && StringUtils.isNotEmpty(activationRule)) {
+            throw new PermissionDeniedException("Quota Tariff Activation Rule cannot be set, as Javascript interpretation is disabled in the configuration.");
+        }
+    }
+
     @Override
     public QuotaTariffResponse createQuotaTariffResponse(QuotaTariffVO tariff, boolean returnActivationRule) {
         final QuotaTariffResponse response = new QuotaTariffResponse();
@@ -440,6 +447,8 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
             throw new InvalidParameterValueException(String.format("There is no quota tariffs with name [%s].", name));
         }
 
+        checkActivationRulesAllowed(activationRule);
+
         Date currentQuotaTariffStartDate = currentQuotaTariff.getEffectiveOn();
 
         currentQuotaTariff.setRemoved(now);
@@ -696,6 +705,8 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
             throw new InvalidParameterValueException(String.format("A quota tariff with name [%s] already exist.", name));
         }
 
+        checkActivationRulesAllowed(activationRule);
+
         if (startDate.compareTo(now) < 0) {
             throw new InvalidParameterValueException(String.format("The value passed as Quota tariff's start date is in the past: [%s]. " +
                     "Please, inform a date in the future or do not pass the parameter to use the current date and time.", startDate));
diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaService.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaService.java
index 8f3c34982c0..f6a34e01be8 100644
--- a/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaService.java
+++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaService.java
@@ -16,15 +16,15 @@
 //under the License.
 package org.apache.cloudstack.quota;
 
-import com.cloud.user.AccountVO;
-import com.cloud.utils.component.PluggableService;
+import java.math.BigDecimal;
+import java.util.Date;
+import java.util.List;
 
 import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
 import org.apache.cloudstack.quota.vo.QuotaUsageVO;
 
-import java.math.BigDecimal;
-import java.util.Date;
-import java.util.List;
+import com.cloud.user.AccountVO;
+import com.cloud.utils.component.PluggableService;
 
 public interface QuotaService extends PluggableService {
 
@@ -40,4 +40,6 @@ public interface QuotaService extends PluggableService {
 
     boolean saveQuotaAccount(AccountVO account, BigDecimal aggrUsage, Date endDate);
 
+    boolean isJsInterpretationEnabled();
+
 }
diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaServiceImpl.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaServiceImpl.java
index 17fa7bd8425..73a8bc30486 100644
--- a/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaServiceImpl.java
+++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaServiceImpl.java
@@ -60,6 +60,7 @@ import com.cloud.configuration.Config;
 import com.cloud.domain.dao.DomainDao;
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.exception.PermissionDeniedException;
+import com.cloud.server.ManagementService;
 import com.cloud.user.Account;
 import com.cloud.user.AccountVO;
 import com.cloud.user.dao.AccountDao;
@@ -86,6 +87,8 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
 
     private TimeZone _usageTimezone;
 
+    private boolean jsInterpretationEnabled = false;
+
     public QuotaServiceImpl() {
         super();
     }
@@ -97,6 +100,8 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
         String timeZoneStr = ObjectUtils.defaultIfNull(_configDao.getValue(Config.UsageAggregationTimezone.toString()), "GMT");
         _usageTimezone = TimeZone.getTimeZone(timeZoneStr);
 
+        jsInterpretationEnabled = ManagementService.JsInterpretationEnabled.value();
+
         return true;
     }
 
@@ -284,4 +289,8 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
         }
     }
 
+    @Override
+    public boolean isJsInterpretationEnabled() {
+        return jsInterpretationEnabled;
+    }
 }
diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
index 2ef7f4b6637..24f89548490 100755
--- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
+++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
@@ -154,6 +154,7 @@ import com.cloud.org.Cluster;
 import com.cloud.org.Grouping;
 import com.cloud.org.Managed;
 import com.cloud.serializer.GsonHelper;
+import com.cloud.server.ManagementService;
 import com.cloud.service.ServiceOfferingVO;
 import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.service.dao.ServiceOfferingDetailsDao;
@@ -271,6 +272,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
     private ServiceOfferingDetailsDao _serviceOfferingDetailsDao;
     @Inject
     private UserVmManager userVmManager;
+    @Inject
+    ManagementService managementService;
 
     private List<? extends Discoverer> _discoverers;
 
@@ -1936,6 +1939,9 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
 
     @Override
     public Host updateHost(final UpdateHostCmd cmd) throws NoTransitionException {
+        managementService.checkJsInterpretationAllowedIfNeededForParameterValue(ApiConstants.IS_TAG_A_RULE,
+                Boolean.TRUE.equals(cmd.getIsTagARule()));
+
         return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(),
                 cmd.getAllocationState(), cmd.getUrl(), cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false);
     }
diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
index 56e8a56f2e2..58ac6891e5f 100644
--- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
@@ -1040,6 +1040,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     protected List<DeploymentPlanner> _planners;
 
+    private boolean jsInterpretationEnabled = false;
+
     private final List<HypervisorType> supportedHypervisors = new ArrayList<>();
 
     public List<DeploymentPlanner> getPlanners() {
@@ -1126,6 +1128,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         supportedHypervisors.add(HypervisorType.KVM);
         supportedHypervisors.add(HypervisorType.XenServer);
 
+        jsInterpretationEnabled = JsInterpretationEnabled.value();
+
         return true;
     }
 
@@ -4022,8 +4026,10 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         cmdList.add(ListGuestVlansCmd.class);
         cmdList.add(AssignVolumeCmd.class);
         cmdList.add(ListSecondaryStorageSelectorsCmd.class);
-        cmdList.add(CreateSecondaryStorageSelectorCmd.class);
-        cmdList.add(UpdateSecondaryStorageSelectorCmd.class);
+        if (jsInterpretationEnabled) {
+            cmdList.add(CreateSecondaryStorageSelectorCmd.class);
+            cmdList.add(UpdateSecondaryStorageSelectorCmd.class);
+        }
         cmdList.add(RemoveSecondaryStorageSelectorCmd.class);
         cmdList.add(ListAffectedVmsForStorageScopeChangeCmd.class);
 
@@ -4066,7 +4072,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {vmPasswordLength, sshKeyLength, humanReadableSizes, customCsIdentifier};
+        return new ConfigKey<?>[] {vmPasswordLength, sshKeyLength, humanReadableSizes, customCsIdentifier,
+                JsInterpretationEnabled};
     }
 
     protected class EventPurgeTask extends ManagedContextRunnable {
@@ -5523,4 +5530,13 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         _lockControllerListener = lockControllerListener;
     }
 
+    @Override
+    public void checkJsInterpretationAllowedIfNeededForParameterValue(String paramName, boolean paramValue) {
+        if (!paramValue || jsInterpretationEnabled) {
+            return;
+        }
+        throw new InvalidParameterValueException(String.format(
+                "The parameter %s cannot be set to true as JS interpretation is disabled",
+                paramName));
+    }
 }
diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
index 323c0eb3ee4..5a66ad502f2 100644
--- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
@@ -213,6 +213,7 @@ import com.cloud.org.Grouping.AllocationState;
 import com.cloud.resource.ResourceState;
 import com.cloud.server.ConfigurationServer;
 import com.cloud.server.ManagementServer;
+import com.cloud.server.ManagementService;
 import com.cloud.server.StatsCollector;
 import com.cloud.service.dao.ServiceOfferingDetailsDao;
 import com.cloud.storage.Storage.ImageFormat;
@@ -398,6 +399,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
     ConfigurationDao configurationDao;
     @Inject
     private ImageStoreDetailsUtil imageStoreDetailsUtil;
+    @Inject
+    ManagementService managementService;
 
     protected List<StoragePoolDiscoverer> _discoverers;
 
@@ -1015,6 +1018,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
             throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone is currently disabled: %s", zone));
         }
 
+        managementService.checkJsInterpretationAllowedIfNeededForParameterValue(ApiConstants.IS_TAG_A_RULE,
+                Boolean.TRUE.equals(cmd.isTagARule()));
+
         Map<String, Object> params = new HashMap<>();
         params.put("zoneId", zone.getId());
         params.put("clusterId", clusterId);
@@ -1197,6 +1203,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
         // Input validation
         Long id = cmd.getId();
 
+        managementService.checkJsInterpretationAllowedIfNeededForParameterValue(ApiConstants.IS_TAG_A_RULE,
+                Boolean.TRUE.equals(cmd.isTagARule()));
+
         StoragePoolVO pool = _storagePoolDao.findById(id);
         if (pool == null) {
             throw new IllegalArgumentException("Unable to find storage pool with ID: " + id);
diff --git a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java
index 1a3d9d843c3..3126da50bca 100644
--- a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java
+++ b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java
@@ -19,31 +19,52 @@ package org.apache.cloudstack.utils.jsinterpreter;
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.io.StringWriter;
+import java.util.Arrays;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
+import javax.script.Bindings;
+import javax.script.Compilable;
+import javax.script.CompiledScript;
+import javax.script.ScriptContext;
+import javax.script.ScriptEngine;
+import javax.script.ScriptException;
+import javax.script.SimpleBindings;
+import javax.script.SimpleScriptContext;
+
 import org.apache.commons.collections.MapUtils;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
-
-import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.logging.log4j.Logger;
+import org.openjdk.nashorn.api.scripting.ClassFilter;
 import org.openjdk.nashorn.api.scripting.NashornScriptEngineFactory;
 
-import javax.script.ScriptEngine;
+import com.cloud.utils.exception.CloudRuntimeException;
 
 /**
- * A class to execute JavaScript scripts, with the possibility to inject context to the scripts.
+ * Executes JavaScript with strong restrictions to mitigate RCE risks.
+ * - Disables Java interop via --no-java AND a deny-all ClassFilter
+ * - Disables Nashorn syntax extensions
+ * - Uses Bindings instead of string-splicing variables
+ * - Fresh ScriptContext per execution, with timeout on a daemon worker
  */
 public class JsInterpreter implements Closeable {
     protected Logger logger = LogManager.getLogger(JsInterpreter.class);
 
+    protected static final List<String> RESTRICTED_TOKENS = Arrays.asList( "engine", "context", "factory",
+            "Java", "java", "Packages"," javax", "load", "loadWithNewGlobal", "print", "factory", "getClass",
+            "runCommand", "Runtime", "exec", "ProcessBuilder", "Thread", "thread", "Threads", "Class", "class");
+
     protected ScriptEngine interpreter;
     protected String interpreterName;
     private final String injectingLogMessage = "Injecting variable [%s] with value [%s] into the JS interpreter.";
@@ -51,21 +72,40 @@ public class JsInterpreter implements Closeable {
     private TimeUnit defaultTimeUnit = TimeUnit.MILLISECONDS;
     private long timeout;
     private String timeoutDefaultMessage;
-    protected Map<String, String> variables = new LinkedHashMap<>();
+
+    // Store variables as Objects; they go into Bindings (no code splicing)
+    protected Map<String, Object> variables = new LinkedHashMap<>();
+
+    /** Deny-all filter: no Java class is visible from scripts. */
+    static final class DenyAllClassFilter implements ClassFilter {
+        @Override public boolean exposeToScripts(String className) { return false; }
+    }
 
     /**
      * Constructor created exclusively for unit testing.
      */
-    protected JsInterpreter() {
-    }
+    protected JsInterpreter() { }
 
     public JsInterpreter(long timeout) {
         this.timeout = timeout;
-        this.timeoutDefaultMessage = String.format("Timeout (in milliseconds) defined in the global setting [quota.activationrule.timeout]: [%s].", this.timeout);
+        this.timeoutDefaultMessage = String.format(
+                "Timeout (in milliseconds) defined in the global setting [quota.activationrule.timeout]: [%s].", this.timeout);
 
-        executor = Executors.newSingleThreadExecutor();
-        NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
+        if (System.getProperty("nashorn.args") == null) {
+            System.setProperty("nashorn.args", "--no-java --no-syntax-extensions");
+        }
+
+        this.executor = new ThreadPoolExecutor(
+                1, 1, 60L, TimeUnit.SECONDS,
+                new LinkedBlockingQueue<>(),
+                r -> {
+                    Thread t = new Thread(r, "JsInterpreter-worker");
+                    t.setDaemon(true);
+                    return t;
+                }
+        );
 
+        NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
         this.interpreterName = factory.getEngineName();
         logger.trace(String.format("Initiating JS interpreter: %s.", interpreterName));
 
@@ -73,49 +113,53 @@ public class JsInterpreter implements Closeable {
     }
 
     protected void setScriptEngineDisablingJavaLanguage(NashornScriptEngineFactory factory) {
-        interpreter = factory.getScriptEngine("--no-java");
+        String[] opts = new String[] {
+                "--no-java",
+                "--no-syntax-extensions",
+        };
+        interpreter = factory.getScriptEngine(
+                opts,
+                JsInterpreter.class.getClassLoader(),
+                new DenyAllClassFilter()
+        );
     }
 
-    /**
-     * Discards the current variables map and create a new one.
-     */
+    /** Discards the current variables map and create a new one. */
     public void discardCurrentVariables() {
         logger.trace("Discarding current variables map and creating a new one.");
         variables = new LinkedHashMap<>();
     }
 
     /**
-     * Adds the parameters to a Map that will be converted to JS variables right before executing the script.
-     * @param key The name of the variable.
-     * @param value The value of the variable.
+     * Adds a variable that will be exposed via ENGINE_SCOPE bindings.
+     * Safe against code injection (no string concatenation).
      */
-    public void injectVariable(String key, String value) {
-        logger.trace(String.format(injectingLogMessage, key, value));
+    public void injectVariable(String key, Object value) {
+        if (key == null) return;
+        logger.trace(String.format(injectingLogMessage, key, String.valueOf(value)));
         variables.put(key, value);
     }
 
     /**
-     * Adds the parameter, surrounded by double quotes, to a Map that will be converted to a JS variable right before executing the script.
-     * @param key The name of the variable.
-     * @param value The value of the variable.
+     * @deprecated Not needed when using Bindings; kept for source compatibility.
+     *             Prefer {@link #injectVariable(String, Object)}.
      */
+    @Deprecated
     public void injectStringVariable(String key, String value) {
         if (value == null) {
             logger.trace(String.format("Not injecting [%s] because its value is null.", key));
             return;
         }
-        value = String.format("\"%s\"", value);
-        logger.trace(String.format(injectingLogMessage, key, value));
-        variables.put(key, value);
+        injectVariable(key, value);
     }
 
     /**
-     * Injects the variables to the script and execute it.
+     * Injects the variables via Bindings and executes the script with a fresh context.
      * @param script Code to be executed.
      * @return The result of the executed script.
      */
     public Object executeScript(String script) {
-        script = addVariablesToScript(script);
+        Objects.requireNonNull(script, "script");
 
         logger.debug(String.format("Executing script [%s].", script));
 
@@ -126,43 +170,60 @@ public class JsInterpreter implements Closeable {
     }
 
     protected Object executeScriptInThread(String script) {
-        Callable<Object> task = () -> interpreter.eval(script);
+        final Callable<Object> task = () -> {
+            final SimpleScriptContext ctx = new SimpleScriptContext();
 
-        Future<Object> future = executor.submit(task);
+            final Bindings engineBindings = new SimpleBindings();
+            if (MapUtils.isNotEmpty(variables)) {
+                engineBindings.putAll(variables);
+            }
+            for (String token : RESTRICTED_TOKENS) {
+                engineBindings.put(token, null);
+            }
+            ctx.setBindings(engineBindings, ScriptContext.ENGINE_SCOPE);
+
+            final StringWriter out = new StringWriter();
+            ctx.setWriter(out);
+
+            try {
+                final CompiledScript compiled = ((Compilable) interpreter).compile(script);
+                Object result = compiled.eval(ctx);
+                if (out.getBuffer().length() > 0) {
+                    logger.info("Script produced output on stdout: [{}]", out);
+                }
+                return result;
+            } catch (ScriptException se) {
+                String msg = se.getMessage() == null ? "Script error" : se.getMessage();
+                throw new ScriptException("Script error: " + msg, se.getFileName(), se.getLineNumber(), se.getColumnNumber());
+            }
+        };
+
+        final Future<Object> future = executor.submit(task);
 
         try {
             return future.get(this.timeout, defaultTimeUnit);
-        } catch (TimeoutException | InterruptedException | ExecutionException e) {
-            String message = String.format("Unable to execute script [%s] due to [%s]", script, e.getMessage());
-
-            if (e instanceof TimeoutException) {
-                message = String.format("Execution of script [%s] took too long and timed out. %s", script, timeoutDefaultMessage);
-            }
-
+        } catch (TimeoutException e) {
+            String message = String.format(
+                    "Execution of script [%s] took too long and timed out. %s", script, timeoutDefaultMessage);
             logger.error(message, e);
             throw new CloudRuntimeException(message, e);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            String message = String.format("Execution of script [%s] was interrupted.", script);
+            logger.error(message, e);
+            throw new CloudRuntimeException(message, e);
+        } catch (ExecutionException e) {
+            Throwable cause = e.getCause() == null ? e : e.getCause();
+            String message = String.format("Unable to execute script [%s] due to [%s]", script, cause.getMessage());
+            logger.error(message, cause);
+            throw new CloudRuntimeException(message, cause);
         } finally {
             future.cancel(true);
         }
     }
 
-    protected String addVariablesToScript(String script) {
-        if (MapUtils.isEmpty(variables)) {
-            logger.trace(String.format("There is no variables to add to script [%s]. Returning.", script));
-            return script;
-        }
-
-        String variablesToString = "";
-        for (Map.Entry<String, String> variable : variables.entrySet()) {
-            variablesToString = String.format("%s %s = %s;", variablesToString, variable.getKey(), variable.getValue());
-        }
-
-        return String.format("%s %s", variablesToString, script);
-    }
-
-
     @Override
     public void close() throws IOException {
-        executor.shutdown();
+        executor.shutdownNow();
     }
 }
diff --git a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java
index 8cf9c13fc19..da3da612a22 100644
--- a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java
+++ b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java
@@ -16,12 +16,12 @@
 //under the License.
 package org.apache.cloudstack.utils.jsinterpreter;
 
-import com.cloud.utils.exception.CloudRuntimeException;
-import org.apache.commons.lang3.StringEscapeUtils;
+import java.io.IOException;
+
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-import java.io.IOException;
+import com.cloud.utils.exception.CloudRuntimeException;
 
 public class TagAsRuleHelper {
 
@@ -32,7 +32,6 @@ public class TagAsRuleHelper {
 
     public static boolean interpretTagAsRule(String rule, String tags, long timeout) {
         String script = PARSE_TAGS + rule;
-        tags = String.format("'%s'", StringEscapeUtils.escapeEcmaScript(tags));
         try (JsInterpreter jsInterpreter = new JsInterpreter(timeout)) {
             jsInterpreter.injectVariable("tags", tags);
             Object scriptReturn = jsInterpreter.executeScript(script);
diff --git a/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java b/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java
index ea8a0247f9c..a78ee7b908e 100644
--- a/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java
+++ b/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java
@@ -20,6 +20,7 @@ package org.apache.cloudstack.utils.jsinterpreter;
 import java.io.IOException;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.UUID;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
@@ -27,7 +28,8 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeoutException;
 
-import com.cloud.utils.exception.CloudRuntimeException;
+import javax.script.ScriptEngine;
+
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -36,9 +38,10 @@ import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.Spy;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.openjdk.nashorn.api.scripting.ClassFilter;
 import org.openjdk.nashorn.api.scripting.NashornScriptEngineFactory;
 
-import javax.script.ScriptEngine;
+import com.cloud.utils.exception.CloudRuntimeException;
 
 @RunWith(MockitoJUnitRunner.class)
 public class JsInterpreterTest {
@@ -61,30 +64,6 @@ public class JsInterpreterTest {
         Assert.assertTrue(executor.isShutdown());
     }
 
-    @Test
-    public void addVariablesToScriptTestVariablesMapIsEmptyReturnScript() {
-        String script = "a + b";
-        jsInterpreterSpy.variables = new LinkedHashMap<>();
-
-        String result = jsInterpreterSpy.addVariablesToScript(script);
-
-        Assert.assertEquals(script, result);
-    }
-
-    @Test
-    public void addVariablesToScriptTestVariablesMapIsNotEmptyInjectVariableToScript() {
-        String script = "a + b";
-        String var1 = "{test: \"test\"}";
-        jsInterpreterSpy.injectVariable("var1", var1);
-        jsInterpreterSpy.injectVariable("var2", var1);
-
-        String expected = String.format(" var1 = %s; var2 = %s; %s", var1, var1, script);
-
-        String result = jsInterpreterSpy.addVariablesToScript(script);
-
-        Assert.assertEquals(expected, result);
-    }
-
     @Test
     public void executeScriptTestReturnResultOfScriptExecution() {
         String script = "5";
@@ -154,7 +133,7 @@ public class JsInterpreterTest {
 
     @Test
     public void discardCurrentVariablesTestInstantiateNewMap() {
-        Map<String, String> variables = new LinkedHashMap<>();
+        Map<String, Object> variables = new LinkedHashMap<>();
         variables.put("a", "b");
         variables.put("b", null);
 
@@ -170,12 +149,14 @@ public class JsInterpreterTest {
         NashornScriptEngineFactory nashornScriptEngineFactoryMock = Mockito.spy(NashornScriptEngineFactory.class);
         ScriptEngine scriptEngineMock = Mockito.mock(ScriptEngine.class);
 
-        Mockito.doReturn(scriptEngineMock).when(nashornScriptEngineFactoryMock).getScriptEngine(Mockito.anyString());
+        Mockito.doReturn(scriptEngineMock).when(nashornScriptEngineFactoryMock).getScriptEngine(Mockito.any(),
+                Mockito.any(ClassLoader.class), Mockito.any(ClassFilter.class));
 
         jsInterpreterSpy.setScriptEngineDisablingJavaLanguage(nashornScriptEngineFactoryMock);
 
         Assert.assertEquals(scriptEngineMock, jsInterpreterSpy.interpreter);
-        Mockito.verify(nashornScriptEngineFactoryMock).getScriptEngine("--no-java");
+        Mockito.verify(nashornScriptEngineFactoryMock).getScriptEngine(Mockito.any(),
+                Mockito.any(ClassLoader.class), Mockito.any(ClassFilter.class));
     }
 
     @Test
@@ -193,6 +174,46 @@ public class JsInterpreterTest {
 
         jsInterpreterSpy.injectStringVariable("a", "b");
 
-        Assert.assertEquals(jsInterpreterSpy.variables.get("a"), "\"b\"");
+        Assert.assertEquals(jsInterpreterSpy.variables.get("a"), "b");
+    }
+
+    @Test
+    public void executeScriptTestValidScriptShouldPassWithMixedVariables() {
+        try (JsInterpreter jsInterpreter = new JsInterpreter(1000)) {
+            jsInterpreter.injectVariable("x", 10);
+            jsInterpreter.injectVariable("y", "hello");
+            jsInterpreter.injectVariable("z", true);
+            String validScript = "var result = x + (z ? 1 : 0); y + '-' + result;";
+            Object result = jsInterpreter.executeScript(validScript);
+            Assert.assertEquals("hello-11", result);
+        } catch (IOException exception) {
+            Assert.fail("IOException not expected here");
+        }
+    }
+
+    private void runMaliciousScriptFileTest(String script, String filename) {
+        try (JsInterpreter jsInterpreter = new JsInterpreter(1000)) {
+            jsInterpreter.executeScript(script);
+        } catch (CloudRuntimeException ex) {
+            Assert.assertTrue(ex.getMessage().contains("Unable to execute script"));
+            java.io.File f = new java.io.File(filename);
+            Assert.assertFalse(f.exists());
+        } catch (IOException exception) {
+            Assert.fail("IOException not expected here");
+        }
+    }
+
+    @Test
+    public void executeScriptTestMaliciousScriptShouldThrowException1() {
+        String filename = "/tmp/hack1-" + UUID.randomUUID();
+        String maliciousScript = "Java.type('java.lang.Runtime').getRuntime().exec('touch " + filename + "')";
+        runMaliciousScriptFileTest(maliciousScript, filename);
+    }
+
+    @Test
+    public void executeScriptTestMaliciousScriptShouldThrowException2() {
+        String filename = "/tmp/hack2-" + UUID.randomUUID();
+        String maliciousScript = "var e=this.engine.getFactory().getScriptEngine('-Dnashorn.args=--no-java=False'); e.eval(\"java.lang.Runtime.getRuntime().exec(['/bin/bash','-c','touch " + filename + "']);\");";
+        runMaliciousScriptFileTest(maliciousScript, filename);
     }
 }