Add server validation against whitelist (#93)

* Add Whitelist validation by reading the whitelist file on origin/main

Author: olliesilvester

pyproject.toml

@@ -66,10 +66,13 @@ addopts = """
     """
 markers = """
     requires_local_server: mark a test which requires locally hosting the config server
+    use_threading: mark a test which will kickoff the background thread in WhitelistFetcher
 """
 
 # https://iscinumpy.gitlab.io/post/bound-version-constraints/#watch-for-warnings
-filterwarnings = "error"
+filterwarnings = ["error",
+"ignore:Exception ignored in.*sqlite3.Connection:pytest.PytestUnraisableExceptionWarning"] #See https://github.com/pytest-dev/pytest-cov/issues/694
+
 # Doctest python code in docs, python code in src docstrings, test functions in tests
 testpaths = "docs src tests"
 

src/daq_config_server/app.py

@@ -9,7 +9,10 @@
 from fastapi.responses import JSONResponse, Response
 from starlette import status
 
-from daq_config_server.constants import ENDPOINTS
+from daq_config_server.constants import (
+    ENDPOINTS,
+)
+from daq_config_server.whitelist import get_whitelist
 
 app = FastAPI(
     title="DAQ config server",
@@ -33,6 +36,13 @@ class ValidAcceptHeaders(StrEnum):
     RAW_BYTES = "application/octet-stream"
 
 
+def path_is_whitelisted(file_path: Path) -> bool:
+    whitelist = get_whitelist()
+    return file_path in whitelist.whitelist_files or any(
+        file_path.is_relative_to(dir) for dir in whitelist.whitelist_dirs
+    )
+
+
 @app.get(
     ENDPOINTS.CONFIG + "/{file_path:path}",
     responses={
@@ -69,7 +79,24 @@ def get_configuration(
 ):
     """
     Read a file and return its contents in a format specified by the accept header.
+
+    Check the file against the whitelist on the current main branch on GitHub.
     """
+
+    if not file_path.is_absolute():
+        raise HTTPException(
+            status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+            detail=(f"Requested filepath {file_path} must be an absolute path"),
+        )
+
+    if not path_is_whitelisted(file_path):
+        raise HTTPException(
+            status_code=status.HTTP_403_FORBIDDEN,
+            detail=f"{file_path} is not a whitelisted file. Please make sure it \
+            exists in https://raw.githubusercontent.com/DiamondLightSource/\
+            daq-config-server/refs/heads/main/whitelist.yaml",
+        )
+
     if not file_path.is_file():
         raise HTTPException(
             status_code=status.HTTP_404_NOT_FOUND,

src/daq_config_server/constants.py

@@ -5,3 +5,7 @@
 class ENDPOINTS:
     CONFIG = "/config"
     HEALTH = "/healthz"
+
+
+WHITELIST_REFRESH_RATE_S = 300
+WHITELIST_URL = "https://raw.githubusercontent.com/DiamondLightSource/daq-config-server/refs/heads/main/whitelist.yaml"

src/daq_config_server/log.py

@@ -0,0 +1,24 @@
+import logging
+
+
+# See https://github.com/DiamondLightSource/daq-config-server/issues/73
+# for making the logging configurable
+def get_default_logger(name: str = __name__) -> logging.Logger:
+    logger = logging.getLogger(name)
+    logger.setLevel(logging.INFO)
+
+    # Prevent adding handlers multiple times
+    if not logger.handlers:
+        console_handler = logging.StreamHandler()
+        formatter = logging.Formatter(
+            "%(asctime)s - %(levelname)s - %(message)s", datefmt="%Y-%m-%d %H:%M:%S"
+        )
+        console_handler.setFormatter(formatter)
+        logger.addHandler(console_handler)
+
+    return logger
+
+
+# For now use a basic console-writing logger. Integrate properly with kubernetes in the
+# future
+LOGGER = get_default_logger("daq-config-server")

src/daq_config_server/testing/__init__.py

@@ -0,0 +1,3 @@
+from ._utils import make_test_response
+
+__all__ = ["make_test_response"]

src/daq_config_server/testing/_utils.py

@@ -0,0 +1,28 @@
+from unittest.mock import MagicMock
+
+from httpx import Response
+from requests import RequestException
+
+from daq_config_server.app import ValidAcceptHeaders
+
+
+def make_test_response(
+    content: str,
+    status_code: int = 200,
+    raise_exc: type[RequestException] | None = None,
+    json_value: str | None = None,
+    content_type: ValidAcceptHeaders = ValidAcceptHeaders.PLAIN_TEXT,
+):
+    r = Response(
+        json=json_value,
+        status_code=status_code,
+        headers={"content-type": content_type},
+        content=content,
+    )
+    r.raise_for_status = MagicMock()
+
+    if raise_exc:
+        r.raise_for_status.side_effect = raise_exc
+    else:
+        r.raise_for_status.return_value = None
+    return r

src/daq_config_server/whitelist.py

@@ -0,0 +1,63 @@
+import atexit
+import time
+from functools import cache
+from pathlib import Path
+from threading import Event, Thread
+
+import requests
+import yaml
+
+from daq_config_server.constants import (
+    WHITELIST_REFRESH_RATE_S,
+    WHITELIST_URL,
+)
+from daq_config_server.log import LOGGER
+
+
+class WhitelistFetcher:
+    """Read the whitelist from the main branch of this repo from github, and check for
+    updates every 5 minutes. This lets the deployed server see updates to the whitelist
+    without requiring a new release or a restart"""
+
+    def __init__(self):
+        self._initial_load()
+        self._stop = Event()
+        self.update_in_background_thread = Thread(
+            target=self._periodically_update_whitelist, daemon=True
+        )
+        self.update_in_background_thread.start()
+
+    def _fetch_and_update(self):
+        response = requests.get(WHITELIST_URL)
+        response.raise_for_status()
+        data = yaml.safe_load(response.text)
+        self.whitelist_files = {Path(p) for p in data.get("whitelist_files")}
+        self.whitelist_dirs = {Path(p) for p in data.get("whitelist_dirs")}
+
+    def _initial_load(self):
+        try:
+            self._fetch_and_update()
+            LOGGER.info("Successfully read whitelist from GitHub.")
+        except Exception as e:
+            LOGGER.error(f"Initial whitelist load failed: {e}")
+            raise RuntimeError("Failed to load whitelist during initialization.") from e
+
+    def _periodically_update_whitelist(self):
+        while not self._stop.is_set():
+            time.sleep(WHITELIST_REFRESH_RATE_S)
+            try:
+                self._fetch_and_update()
+                LOGGER.info("Whitelist updated successfully.")
+            except Exception as e:
+                LOGGER.error(f"Failed to update whitelist: {e}")
+
+    def stop(self):
+        self._stop.set()
+        self.update_in_background_thread.join(timeout=1)
+
+
+@cache
+def get_whitelist() -> WhitelistFetcher:
+    fetcher = WhitelistFetcher()
+    atexit.register(fetcher.stop)
+    return fetcher

tests/constants.py

@@ -7,3 +7,24 @@
 TEST_BAD_JSON_PATH = TEST_DATA_DIR_PATH.joinpath("test_bad_json")
 
 TEST_GOOD_JSON_PATH = TEST_DATA_DIR_PATH.joinpath("test_good_json.json")
+
+TEST_FILE_NOT_ON_WHITELIST_PATH = TEST_DATA_DIR_PATH.joinpath(
+    "test_file_not_on_whitelist.json"
+)
+
+TEST_FILE_IN_GOOD_DIR = TEST_DATA_DIR_PATH.joinpath("good_dir/test_file")
+
+TEST_FILE_IN_BAD_DIR = TEST_DATA_DIR_PATH.joinpath("bad_dir/test_file")
+
+TEST_INVALID_FILE_PATH = TEST_DATA_DIR_PATH.joinpath("invalid_file")
+
+TEST_WHITELIST_RESPONSE = f"""\
+whitelist_files:
+  - {TEST_GOOD_JSON_PATH}
+  - {TEST_BAD_JSON_PATH}
+  - {TEST_BEAMLINE_PARAMETERS_PATH}
+  - {TEST_INVALID_FILE_PATH}
+
+whitelist_dirs:
+  - {TEST_DATA_DIR_PATH.joinpath("good_dir")}
+"""

tests/system_tests/test_client.py

@@ -90,3 +90,27 @@ def test_bad_json_gives_http_error_with_details(server: ConfigServer):
             dict[Any, Any],
         )
     server._log.error.assert_called_once_with(expected_detail)
+
+
+# See https://github.com/DiamondLightSource/daq-config-server/issues/96 for writing
+# these tests
+
+
+@pytest.mark.xfail
+@pytest.mark.requires_local_server
+def test_request_with_file_not_on_whitelist(): ...
+
+
+@pytest.mark.xfail
+@pytest.mark.requires_local_server
+def test_request_with_file_on_whitelist(): ...
+
+
+@pytest.mark.xfail
+@pytest.mark.requires_local_server
+def test_request_with_file_in_whitelist_dirs(): ...
+
+
+@pytest.mark.xfail
+@pytest.mark.requires_local_server
+def test_request_with_file_not_in_whitelist_dirs(): ...