add security analysis and fixes (#168)

.gitignore

@@ -6,6 +6,7 @@ __pycache__/
 .eggs/
 .pytest_cache/
 docker-compose.yml
+bandit.txt
 coverage.xml
 build/
 coverage/

HISTORY.rst

@@ -12,8 +12,9 @@ Unreleased
 Features / Changes
 ~~~~~~~~~~~~~~~~~~~~~
 * update code with more linting checks.
+* add ``bandit`` security code analysis and apply some detected issues (#168).
 * prepare travis pipeline for *eventual* docker image smoke test.
-* bump `alembic>=1.3.0` to remove old warnings and receive recent fixes.
+* bump ``alembic>=1.3.0`` to remove old warnings and receive recent fixes.
 
 Bug Fixes
 ~~~~~~~~~~~~~~~~~~~~~
@@ -35,14 +36,14 @@ Features / Changes
 
 Bug Fixes
 ~~~~~~~~~~~~~~~~~~~~~
-* fix 500 error when getting user's services on /users/{user_name}/services
+* fix 500 error when getting user's services on ``/users/{user_name}/services``.
 
 1.7.2 (2019-11-15)
 ---------------------
 
 Bug Fixes
 ~~~~~~~~~~~~~~~~~~~~~
-* fix gunicorn breaking change in 20.0.0 is not compatible with alpine: pin gunicorn==19.9.0
+* fix ``gunicorn>=20.0.0`` breaking change not compatible with alpine: pin ``gunicorn==19.9.0``.
 
 1.7.1 (2019-11-12)
 ---------------------

Makefile

@@ -141,6 +141,12 @@ test-remote: install-dev install	## run only remote tests with the default Pytho
 	@echo "Running remote tests..."
 	bash -c '$(CONDA_CMD) pytest tests -vv -m "remote" --junitxml "$(APP_ROOT)/tests/results.xml"'
 
+.PHONY: test-security
+test-security:						## run security static code analysis
+	@echo "Running security tests..."
+	bash -c '$(CONDA_CMD) bandit "$(APP_ROOT)" --ini "$(APP_ROOT)/setup.cfg" -r \
+		| tee "$(APP_ROOT)/bandit.txt"'
+
 .PHONY: test-docker
 test-docker: docker-test			## synonym for target 'docker-test' - WARNING: could build image if missing
 

magpie/api/schemas.py

@@ -347,8 +347,8 @@ class HeaderRequestSchemaUI(colander.MappingSchema):
 class BaseResponseBodySchema(colander.MappingSchema):
     def __init__(self, code, description, **kw):
         super(BaseResponseBodySchema, self).__init__(**kw)
-        assert isinstance(code, int)
-        assert isinstance(description, six.string_types)
+        assert isinstance(code, int)                        # nosec: B101
+        assert isinstance(description, six.string_types)    # nosec: B101
         self.__code = code
         self.__desc = description
 
@@ -388,7 +388,7 @@ class ErrorVerifyParamBodySchema(colander.MappingSchema):
 class ErrorResponseBodySchema(BaseResponseBodySchema):
     def __init__(self, code, description, **kw):
         super(ErrorResponseBodySchema, self).__init__(code, description, **kw)
-        assert code >= 400
+        assert code >= 400  # nosec: B101
 
     route_name = colander.SchemaNode(
         colander.String(),

magpie/constants.py

@@ -68,13 +68,12 @@ def _get_default_log_level():
     Get default configurations from ini file.
     """
     _default_log_lvl = "INFO"
-    # noinspection PyBroadException
     try:
         import magpie.utils
         _settings = magpie.utils.get_settings_from_config_ini(MAGPIE_INI_FILE_PATH,
                                                               ini_main_section_name="logger_magpie")
         _default_log_lvl = _settings.get("level", _default_log_lvl)
-    except Exception:
+    except Exception:  # noqa # nosec: B110
         pass
     return _default_log_lvl
 

magpie/db.py

@@ -131,12 +131,11 @@ def is_database_ready(db_session=None):
 
     for _, obj in inspect.getmembers(models):
         if inspect.isclass(obj):
-            # noinspection PyBroadException
             try:
                 curr_table_name = obj.__tablename__
                 if curr_table_name not in table_names:
                     return False
-            except Exception:
+            except Exception:  # noqa # nosec: B112
                 continue
     return True
 

magpie/helpers/register_default_users.py

@@ -46,7 +46,6 @@ def register_user_with_group(user_name, group_name, email, password, db_session)
     else:
         print_log("User '{}' already exist".format(user_name), level=logging.DEBUG)
 
-    # noinspection PyBroadException
     try:
         # ensure the reference between user/group exists (user joined the group)
         user_group_refs = BaseService.all(models.UserGroup, db_session=db_session)
@@ -55,7 +54,8 @@ def register_user_with_group(user_name, group_name, email, password, db_session)
             # noinspection PyArgumentList
             group_entry = models.UserGroup(group_id=registered_group.id, user_id=registered_user.id)
             db_session.add(group_entry)
-    except Exception:  # in case reference already exists, avoid duplicate error
+    except Exception:  # noqa
+        # in case reference already exists, avoid duplicate error
         db_session.rollback()
 
 

magpie/helpers/sync_resources.py

@@ -55,9 +55,10 @@ def _merge_resources(resources_local, resources_remote, max_depth=None):
     if not resources_remote:
         return resources_local
 
-    assert is_valid_resource_schema(resources_local)
-    assert is_valid_resource_schema(resources_remote)
-
+    if not is_valid_resource_schema(resources_local):
+        raise ValueError("Invalid 'local' resource schema.")
+    if not is_valid_resource_schema(resources_remote):
+        raise ValueError("Invalid 'remote' resource schema.")
     if not resources_local:
         raise ValueError("The resources must contain at least the service name.")
 
@@ -288,10 +289,9 @@ def fetch_all_services_by_type(service_type, session):
     :param session:
     """
     for service in session.query(models.Service).filter_by(type=service_type):
-        # noinspection PyBroadException
         try:
             fetch_single_service(service, session)
-        except Exception:
+        except Exception:  # noqa # nosec: B110
             if CRON_SERVICE:
                 LOGGER.exception("There was an error when fetching data from the url: %s" % service.url)
                 pass

magpie/helpers/sync_services.py

@@ -83,7 +83,8 @@ def get_resources(self):
         resources = {"geoserver-api": {"children": workspace_tree,
                                        "resource_type": resource_type}}
 
-        assert is_valid_resource_schema(resources), "Error in Interface implementation"
+        if not is_valid_resource_schema(resources):
+            raise ValueError("Error in SyncServiceInterface implementation")
         return resources
 
 
@@ -107,7 +108,8 @@ def get_resources(self):
                     for p in resp.json()}
 
         resources = {self.service_name: {"children": projects, "resource_type": resource_type}}
-        assert is_valid_resource_schema(resources), "Error in Interface implementation"
+        if not is_valid_resource_schema(resources):
+            raise ValueError("Error in SyncServiceInterface implementation")
         return resources
 
 
@@ -144,7 +146,8 @@ def thredds_get_resources(url, depth):
             return tree_item
 
         resources = thredds_get_resources(self.url, self.max_depth)
-        assert is_valid_resource_schema(resources), 'Error in Interface implementation'
+        if not is_valid_resource_schema(resources):
+            raise ValueError("Error in SyncServiceInterface implementation")
         return resources
 
 

magpie/owsrequest.py

@@ -8,7 +8,7 @@
 from magpie.api.requests import get_multiformat_any
 from magpie.utils import get_logger, CONTENT_TYPE_JSON, CONTENT_TYPE_PLAIN, CONTENT_TYPE_FORM, is_json_body, get_header
 from typing import TYPE_CHECKING
-import lxml.etree
+import lxml.etree  # nosec: B410 # module safe but bandit flags it : https://github.com/tiran/defusedxml/issues/38
 
 if TYPE_CHECKING:
     from pyramid.request import Request
@@ -92,8 +92,7 @@ class WPSPost(OWSParser):
 
     def __init__(self, request):
         super(WPSPost, self).__init__(request)
-        # noinspection PyUnresolvedReferences
-        self.document = lxml.etree.fromstring(self.request.body)
+        self.document = lxml.etree.fromstring(self.request.body)  # nosec: B410
         lxml_strip_ns(self.document)
 
     def _get_param_value(self, param):

requirements-dev.txt

@@ -1,5 +1,6 @@
 -r requirements-docs.txt
 autopep8
+bandit
 bump2version
 codacy-coverage
 coverage==4.0

setup.cfg

@@ -44,6 +44,11 @@ exclude =
 	parts,
 	share,
 
+[bandit]
+exclude = *.egg-info,build,dist,env,tests
+targets = .
+#format = custom
+
 [tool:pytest]
 addopts =
 	--strict