Various hardening to REST routes (#1293)

* URL reweriting for static

* request size limits

* improve quotas, make tests work

* remove debugging code

* pass limits to front-end

* fix renaming boggle

server/common/app_config.py

@@ -21,6 +21,18 @@ DEFAULT_SERVER_PORT = int(environ.get("CXG_SERVER_PORT", "5005"))
 # anything bigger than this will generate a special message
 BIG_FILE_SIZE_THRESHOLD = 100 * 2 ** 20  # 100MB
 
+""" Default limits for requests """
+Default_Limits = {
+    # Max number of columns that may be requested for /annotations or /data routes.
+    # This is a simplistic means of preventing excess resource consumption (eg,
+    # requesting the entire X matrix in one request) or other DoS style attacks/errors.
+    # Set to None to disable check.
+    "column_request_max": 32,
+    # Max number of cells that will be accepted for differential expression.
+    # Set to None to disable the check.
+    "diffexp_cellcount_max": None,  # None is disabled
+}
+
 
 class AppFeature(object):
     def __init__(self, path, available=False, method="POST", extra={}):
@@ -78,6 +90,9 @@ class AppConfig(object):
         except KeyError as e:
             raise ConfigurationError(f"Unexpected config: {str(e)}")
 
+        # Used for various limits, eg, size of requests.  Not currently configurable.
+        self.limits = Default_Limits
+
         # The annotation object is created during complete_config and stored here.
         self.user_annotations = None
 
@@ -425,5 +440,12 @@ class AppConfig(object):
         config["library_versions"] = library_versions
         config["links"] = links
         config["parameters"] = parameters
+        config["limits"] = self.limits
 
         return c
+
+    def exceeds_limit(self, limit_name, value):
+        limit_value = self.limits.get(limit_name, None)
+        if limit_value is None:  # disabled
+            return False
+        return value > limit_value

server/common/errors.py

@@ -68,3 +68,11 @@ class ConfigurationError(Exception):
     """
 
     pass
+
+
+class ExceedsLimitError(Exception):
+    """
+    Raised when an HTTP request exceeds a limit/quota
+    """
+
+    pass

server/common/rest.py

@@ -9,6 +9,7 @@ from server.common.errors import (
     JSONEncodingValueError,
     PrepareError,
     DisabledFeatureError,
+    ExceedsLimitError,
 )
 
 import json
@@ -54,9 +55,13 @@ def config_get(app_config, data_adaptor, annotations):
 
 def annotations_obs_get(request, data_adaptor, annotations):
     fields = request.args.getlist("annotation-name", None)
+    num_columns_requested = len(data_adaptor.get_obs_keys()) if len(fields) == 0 else len(fields)
+    if data_adaptor.config.exceeds_limit("column_request_max", num_columns_requested):
+        return abort(HTTPStatus.BAD_REQUEST)
     preferred_mimetype = request.accept_mimetypes.best_match(["application/octet-stream"])
     if preferred_mimetype != "application/octet-stream":
         return abort(HTTPStatus.NOT_ACCEPTABLE)
+
     try:
         labels = None
         if annotations:
@@ -100,9 +105,13 @@ def annotations_obs_put(request, data_adaptor, annotations):
 
 def annotations_var_get(request, data_adaptor, annotations):
     fields = request.args.getlist("annotation-name", None)
+    num_columns_requested = len(data_adaptor.get_var_keys()) if len(fields) == 0 else len(fields)
+    if data_adaptor.config.exceeds_limit("column_request_max", num_columns_requested):
+        return abort(HTTPStatus.BAD_REQUEST)
     preferred_mimetype = request.accept_mimetypes.best_match(["application/octet-stream"])
     if preferred_mimetype != "application/octet-stream":
         return abort(HTTPStatus.NOT_ACCEPTABLE)
+
     try:
         labels = None
         if annotations is not None:
@@ -129,7 +138,7 @@ def data_var_put(request, data_adaptor):
             HTTPStatus.OK,
             {"Content-Type": "application/octet-stream"},
         )
-    except FilterError as e:
+    except (FilterError, ValueError, ExceedsLimitError) as e:
         return abort_and_log(HTTPStatus.BAD_REQUEST, str(e), include_exc_info=True)
 
 
@@ -160,7 +169,7 @@ def diffexp_obs_post(request, data_adaptor):
     try:
         diffexp = data_adaptor.diffexp_topN(set1_filter, set2_filter, count)
         return make_response(diffexp, HTTPStatus.OK, {"Content-Type": "application/json"})
-    except (ValueError, DisabledFeatureError, FilterError) as e:
+    except (ValueError, DisabledFeatureError, FilterError, ExceedsLimitError) as e:
         return abort_and_log(HTTPStatus.BAD_REQUEST, str(e), include_exc_info=True)
     except JSONEncodingValueError:
         # JSON encoding failure, usually due to bad data. Just let it ripple up
@@ -171,13 +180,13 @@ def diffexp_obs_post(request, data_adaptor):
 
 def layout_obs_get(request, data_adaptor):
     preferred_mimetype = request.accept_mimetypes.best_match(["application/octet-stream"])
+    if preferred_mimetype != "application/octet-stream":
+        return abort(HTTPStatus.NOT_ACCEPTABLE)
+
     try:
-        if preferred_mimetype == "application/octet-stream":
+        return make_response(
-            return make_response(
+            data_adaptor.layout_to_fbs_matrix(), HTTPStatus.OK, {"Content-Type": "application/octet-stream"}
-                data_adaptor.layout_to_fbs_matrix(), HTTPStatus.OK, {"Content-Type": "application/octet-stream"}
+        )
-            )
-        else:
-            return abort(HTTPStatus.NOT_ACCEPTABLE)
     except PrepareError:
         return abort_and_log(
             HTTPStatus.NOT_IMPLEMENTED,

server/data_anndata/anndata_adaptor.py

@@ -351,3 +351,11 @@ class AnndataAdaptor(DataAdaptor):
 
     def get_obs_columns(self):
         return self.data.obs.columns
+
+    def get_obs_keys(self):
+        # return list of keys
+        return self.data.obs.keys().to_list()
+
+    def get_var_keys(self):
+        # return list of keys
+        return self.data.var.keys().to_list()

server/data_common/data_adaptor.py

@@ -6,7 +6,7 @@ from os.path import basename, splitext
 
 from server.data_common.fbs.matrix import encode_matrix_fbs
 from server.common.constants import Axis, DEFAULT_TOP_N
-from server.common.errors import FilterError, JSONEncodingValueError
+from server.common.errors import FilterError, JSONEncodingValueError, ExceedsLimitError
 from server.compute.diffexp import diffexp_ttest
 from server.common.utils import jsonify_numpy
 from server.common.app_config import AppFeature, AppConfig
@@ -92,6 +92,16 @@ class DataAdaptor(metaclass=ABCMeta):
     def get_obs_columns(self):
         pass
 
+    @abstractmethod
+    def get_obs_keys(self):
+        # return list of keys
+        pass
+
+    @abstractmethod
+    def get_var_keys(self):
+        # return list of keys
+        pass
+
     @abstractmethod
     def cleanup(self):
         pass
@@ -259,6 +269,10 @@ class DataAdaptor(metaclass=ABCMeta):
         if obs_selector is not None:
             raise FilterError("filtering on obs unsupported")
 
+        num_columns = self.get_shape()[1] if var_selector is None else np.count_nonzero(var_selector)
+        if self.config.exceeds_limit("column_request_max", num_columns):
+            raise ExceedsLimitError("Requested dataframe columns exceed column request limit")
+
         X = self.get_X_array(obs_selector, var_selector)
         col_idx = np.nonzero([] if var_selector is None else var_selector)[0]
         return encode_matrix_fbs(X, col_idx=col_idx, row_idx=None)
@@ -286,6 +300,11 @@ class DataAdaptor(metaclass=ABCMeta):
         if top_n is None:
             top_n = DEFAULT_TOP_N
 
+        if self.config.exceeds_limit(
+            "diffexp_cellcount_max", np.count_nonzero(obs_mask_A) + np.count_nonzero(obs_mask_B)
+        ):
+            raise ExceedsLimitError("Diffexp request exceeds max cell count limit")
+
         result = diffexp_ttest(self, obs_mask_A, obs_mask_B, top_n, self.config.diffexp__lfc_cutoff)
 
         try:

server/data_cxg/cxg_adaptor.py

@@ -234,6 +234,16 @@ class CxgAdaptor(DataAdaptor):
         col_names = [attr.name for attr in schema]
         return pd.Index(col_names)
 
+    def get_obs_keys(self):
+        obs = self.open_array("obs")
+        schema = obs.schema
+        return [attr.name for attr in schema]
+
+    def get_var_keys(self):
+        var = self.open_array("var")
+        schema = var.schema
+        return [attr.name for attr in schema]
+
     # function to get the embedding
     # this function to iterate through embeddings.
     def get_embedding_names(self):

server/eb/.ebextensions/static_rewrite.config

@@ -0,0 +1,11 @@
+# Configure URL rewriting from /<dataset>/static/* to /static/*
+files:
+  "/etc/httpd/conf.d/static_rewrite.conf":
+  	mode: "000644"
+  	owner: root
+  	group: root
+  	content: |
+  		RewriteEngine On
+  		RewriteRule "^(.*)/static/(.*)" "/static/$2" [PT]
+  		RewriteRule "^/favicon.jpg" "/static/img/favicon.jpg" [PT]
+  

server/test/test_anndata_adaptor.py

@@ -44,6 +44,8 @@ class AdaptorTest(unittest.TestCase):
         }
         config = AppConfig()
         config.update(**args)
+        for k in config.limits.keys():
+            config.limits[k] = None
         config.complete_config()
         self.data = AnndataAdaptor(DataLocator(self.data_locator), config)
 

server/test/test_api.py

@@ -193,7 +193,10 @@ class EndPoints(object):
         endpoint = f"data/var"
         url = f"{self.URL_BASE}{endpoint}"
         result = self.session.put(url)
-        self.assertEqual(result.status_code, HTTPStatus.OK)
+        self.assertEqual(result.status_code, HTTPStatus.BAD_REQUEST)
+
+        filter = {"filter": {"var": {"index": [0, 1, 4]}}}
+        result = self.session.put(url, json=filter)
         self.assertEqual(result.headers["Content-Type"], "application/octet-stream")
 
     def test_data_put_fbs(self):
@@ -201,15 +204,7 @@ class EndPoints(object):
         url = f"{self.URL_BASE}{endpoint}"
         header = {"Accept": "application/octet-stream"}
         result = self.session.put(url, headers=header)
-        self.assertEqual(result.status_code, HTTPStatus.OK)
+        self.assertEqual(result.status_code, HTTPStatus.BAD_REQUEST)
-        self.assertEqual(result.headers["Content-Type"], "application/octet-stream")
-        df = decode_fbs.decode_matrix_FBS(result.content)
-        self.assertEqual(df["n_rows"], 2638)
-        self.assertEqual(df["n_cols"], 1838)
-        self.assertIsNotNone(df["columns"])
-        self.assertListEqual(df["col_idx"].tolist(), [])
-        self.assertIsNone(df["row_idx"])
-        self.assertEqual(len(df["columns"]), df["n_cols"])
 
     def test_data_put_filter_fbs(self):
         endpoint = f"data/var"

server/test/test_nan_anndata_adaptor.py

@@ -24,6 +24,8 @@ class NaNTest(unittest.TestCase):
         config.update(**self.args)
         locator = DataLocator("test/test_datasets/nan.h5ad")
         config.update(single_dataset__datapath=locator.path)
+        for k in config.limits.keys():
+            config.limits[k] = None
         config.complete_config()
 
         with warnings.catch_warnings():

server/test/test_nan_rest.py

@@ -47,7 +47,8 @@ class WithNaNs(unittest.TestCase):
     def test_data(self):
         endpoint = "data/var"
         url = f"{URL_BASE}{endpoint}"
-        result = self.session.put(url)
+        filter = {"filter": {"var": {"index": [[0, 20]]}}}
+        result = self.session.put(url, json=filter)
         self.assertEqual(result.status_code, HTTPStatus.OK)
         self.assertEqual(result.headers["Content-Type"], "application/octet-stream")
         df = decode_fbs.decode_matrix_FBS(result.content)