Merge commit from fork

[v9.1] do not use eval in RequestDB

Author: chaen

src/DIRAC/RequestManagementSystem/DB/RequestDB.py

@@ -15,7 +15,6 @@
 import datetime
 import errno
 import random
-
 from urllib.parse import quote_plus
 
 from sqlalchemy import (
@@ -32,6 +31,7 @@
     create_engine,
     distinct,
     func,
+    inspect,
 )
 from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.orm import backref, joinedload, registry, relationship, sessionmaker
@@ -187,6 +187,38 @@ class RequestDB:
     db holding requests
     """
 
+    @staticmethod
+    def _get_column(table_name, column_name):
+        """Resolve supported ORM column attributes without evaluating input."""
+
+        models = {"Request": Request, "Operation": Operation}
+        aliases = {"Status": "_Status"}
+
+        model = models.get(table_name)
+        if model is None:
+            raise ValueError(f"Unknown table '{table_name}'")
+
+        resolved_name = aliases.get(column_name, column_name)
+        if resolved_name not in inspect(model).column_attrs:
+            raise ValueError(f"Unknown {table_name} attribute '{column_name}'")
+
+        return getattr(model, resolved_name)
+
+    @classmethod
+    def _apply_web_filter(cls, query, table_name, column_name, value):
+        column = cls._get_column(table_name, column_name)
+        if isinstance(value, list):
+            return query.filter(column.in_(value))
+        return query.filter(column == value)
+
+    @classmethod
+    def _get_order_expression(cls, table_name, column_name, direction):
+        column = cls._get_column(table_name, column_name)
+        normalized_direction = direction.lower()
+        if normalized_direction not in {"asc", "desc"}:
+            raise ValueError(f"Unknown sort direction '{direction}'")
+        return getattr(column, normalized_direction)()
+
     def __getDBConnectionInfo(self, fullname):
         """Collect from the CS all the info needed to connect to the DB.
         This should be in a base class eventually
@@ -704,13 +736,12 @@ def getRequestSummaryWeb(self, selectDict, sortList, startItem, maxItems):
                     elif key == "Status":
                         key = "_Status"
 
-                    if isinstance(value, list):
-                        summaryQuery = summaryQuery.filter(eval(f"{tableName}.{key}.in_({value})"))
-                    else:
-                        summaryQuery = summaryQuery.filter(eval(f"{tableName}.{key}") == value)
+                    summaryQuery = self._apply_web_filter(summaryQuery, tableName, key, value)
 
             if sortList:
-                summaryQuery = summaryQuery.order_by(eval(f"Request.{sortList[0][0]}.{sortList[0][1].lower()}()"))
+                summaryQuery = summaryQuery.order_by(
+                    self._get_order_expression("Request", sortList[0][0], sortList[0][1])
+                )
 
             try:
                 requestLists = summaryQuery.all()
@@ -744,6 +775,8 @@ def getRequestSummaryWeb(self, selectDict, sortList, startItem, maxItems):
             resultDict["TotalRecords"] = nRequests
 
             return S_OK(resultDict)
+        except ValueError as e:
+            return S_ERROR(str(e))
         #
         except Exception as e:
             self.log.exception("getRequestSummaryWeb: unexpected exception", lException=e)
@@ -763,17 +796,15 @@ def getRequestCountersWeb(self, groupingAttribute, selectDict):
 
         session = self.DBSession()
 
-        if groupingAttribute == "Type":
-            groupingAttribute = "Operation.Type"
-        elif groupingAttribute == "Status":
-            groupingAttribute = "Request._Status"
-        else:
-            groupingAttribute = f"Request.{groupingAttribute}"
-
         try:
+            if groupingAttribute == "Type":
+                groupingColumn = self._get_column("Operation", "Type")
+            else:
+                groupingColumn = self._get_column("Request", groupingAttribute)
+
             summaryQuery = session.query(
-                eval(groupingAttribute), func.count(Request.RequestID)  # pylint: disable=not-callable,no-member
-            )
+                groupingColumn, func.count(Request.RequestID)
+            )  # pylint: disable=not-callable,no-member
 
             for key, value in selectDict.items():
                 if key == "ToDate":
@@ -788,12 +819,9 @@ def getRequestCountersWeb(self, groupingAttribute, selectDict):
                     elif key == "Status":
                         key = "_Status"
 
-                    if isinstance(value, list):
-                        summaryQuery = summaryQuery.filter(eval(f"{objectType}.{key}.in_({value})"))
-                    else:
-                        summaryQuery = summaryQuery.filter(eval(f"{objectType}.{key}") == value)
+                    summaryQuery = self._apply_web_filter(summaryQuery, objectType, key, value)
 
-            summaryQuery = summaryQuery.group_by(eval(groupingAttribute))
+            summaryQuery = summaryQuery.group_by(groupingColumn)
 
             try:
                 requestLists = summaryQuery.all()
@@ -805,6 +833,8 @@ def getRequestCountersWeb(self, groupingAttribute, selectDict):
 
             return S_OK(resultDict)
 
+        except ValueError as e:
+            return S_ERROR(str(e))
         except Exception as e:
             self.log.exception("getRequestSummaryWeb: unexpected exception", lException=e)
             return S_ERROR(f"getRequestSummaryWeb: unexpected exception : {e}")
@@ -817,11 +847,11 @@ def getDistinctValues(self, tableName, columnName):
 
         session = self.DBSession()
         distinctValues = []
-        if columnName == "Status":
-            columnName = "_Status"
         try:
-            result = session.query(distinct(eval(f"{tableName}.{columnName}"))).all()
+            result = session.query(distinct(self._get_column(tableName, columnName))).all()
             distinctValues = [dist[0] for dist in result]
+        except ValueError as e:
+            return S_ERROR(str(e))
         except NoResultFound:
             pass
         except Exception as e:

src/DIRAC/RequestManagementSystem/DB/test/Test_RequestDB.py

@@ -12,6 +12,9 @@
 from DIRAC import gLogger, S_OK
 
 from DIRAC.RequestManagementSystem.DB import RequestDB
+from DIRAC.RequestManagementSystem.Client.File import File
+from DIRAC.RequestManagementSystem.Client.Operation import Operation
+from DIRAC.RequestManagementSystem.Client.Request import Request
 
 from DIRAC.RequestManagementSystem.DB.test.RMSTestScenari import (  # pylint: disable=unused-import
     test_dirty,
@@ -39,3 +42,37 @@ def mock_requestDB__init__(self):
         db.createTables()
 
         yield db
+
+
+def test_web_queries_reject_unknown_attributes(reqDB):
+    request = Request({"RequestName": "web-summary"})
+    operation = Operation({"Type": "RemoveReplica", "TargetSE": "CERN-USER"})
+    operation += File({"LFN": "/lhcb/user/c/cibak/web-summary"})
+    request += operation
+
+    put = reqDB.putRequest(request)
+    assert put["OK"], put
+
+    summary = reqDB.getRequestSummaryWeb({"Type": "RemoveReplica"}, [("RequestID", "ASC")], 0, 10)
+    assert summary["OK"], summary
+    assert summary["Value"]["TotalRecords"] == 1, summary
+
+    counters = reqDB.getRequestCountersWeb("Type", {"Status": "Waiting"})
+    assert counters["OK"], counters
+    assert counters["Value"] == {"RemoveReplica": 1}, counters
+
+    distinct = reqDB.getDistinctValues("Operation", "Type")
+    assert distinct["OK"], distinct
+    assert distinct["Value"] == ["RemoveReplica"], distinct
+
+    invalid_summary = reqDB.getRequestSummaryWeb({"__class__": "Request"}, [], 0, 10)
+    assert not invalid_summary["OK"], invalid_summary
+    assert invalid_summary["Message"] == "Unknown Request attribute '__class__'"
+
+    invalid_counters = reqDB.getRequestCountersWeb("__class__", {})
+    assert not invalid_counters["OK"], invalid_counters
+    assert invalid_counters["Message"] == "Unknown Request attribute '__class__'"
+
+    invalid_distinct = reqDB.getDistinctValues("Request", "__class__")
+    assert not invalid_distinct["OK"], invalid_distinct
+    assert invalid_distinct["Message"] == "Unknown Request attribute '__class__'"