Merge pull request #2 from CHRISCARLON/v0.1.3

V0.1.3 tasks completed.

Author: CHRISCARLON

.dockerignore

@@ -0,0 +1,86 @@
+# Python artifacts
+__pycache__/
+*.py[cod]
+*$py.class
+*.so
+.Python
+build/
+develop-eggs/
+dist/
+downloads/
+eggs/
+.eggs/
+lib/
+lib64/
+parts/
+sdist/
+var/
+wheels/
+*.egg-info/
+.installed.cfg
+*.egg
+
+# Virtual environments
+.venv/
+venv/
+ENV/
+env/
+.env
+
+# Testing and type checking
+.pytest_cache/
+.mypy_cache/
+.coverage
+htmlcov/
+.tox/
+.hypothesis/
+*.cover
+.coverage.*
+
+# Development tools
+.ruff_cache/
+pyrightconfig.json
+.vscode/
+.idea/
+*.swp
+*.swo
+*~
+
+# OS files
+.DS_Store
+.DS_Store?
+._*
+.Spotlight-V100
+.Trashes
+ehthumbs.db
+Thumbs.db
+
+# Git
+.git/
+.gitignore
+.gitattributes
+
+# Documentation
+*.md
+docs/
+LICENSE
+
+# Build files
+Makefile
+makefile
+dockerfile
+Dockerfile
+
+# Logs
+*.log
+logs/
+
+# Package manager
+uv.lock
+poetry.lock
+Pipfile.lock
+
+# Test files
+*_test.py
+test_*.py
+tests/

.gitignore

@@ -13,3 +13,6 @@ wheels/
 .ruff_cache
 uv.lock
 pyrightconfig.json
+*.log
+.pytest_cache/
+.mypy_cache/

dockerfile

@@ -2,17 +2,19 @@ FROM python:3.11-slim
 
 WORKDIR /app
 
-ENV PYTHONPATH=/app/src
-ENV PYTHONUNBUFFERED=1
+ENV PYTHONPATH=/app/src \
+    PYTHONUNBUFFERED=1 \
+    PIP_NO_CACHE_DIR=1 \
+    PIP_DISABLE_PIP_VERSION_CHECK=1
+
 
 RUN apt-get update && apt-get install -y \
     gcc \
-    && rm -rf /var/lib/apt/lists/*
-
+    && rm -rf /var/lib/apt/lists/* \
+    && apt-get clean
 
 COPY pyproject.toml ./
-
-RUN pip install .
+RUN pip install --no-cache-dir .
 
 COPY src/ ./src/
 

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "os-mcp"
-version = "0.1.2"
+version = "0.1.3"
 description = "A Python MCP server that provides access to Ordnance Survey's DataHub APIs."
 readme = "README.md"
 requires-python = ">=3.11"

src/api_service/os_api.py

@@ -37,8 +37,7 @@ async def _get_open_api_spec(self) -> OpenAPISpecification:
             spec = OpenAPISpecification(spec=response)
             return spec
         except Exception as e:
-            logger.error(f"Error getting OpenAPI spec: {e}")
-            raise e
+            raise ValueError(f"Failed to get OpenAPI spec: {e}")
 
     async def cache_openapi_spec(self) -> OpenAPISpecification:
         """
@@ -111,11 +110,14 @@ async def _get_collections(self) -> CollectionsCache:
             response = await self.make_request("COLLECTIONS")
             collections_list = response.get("collections", [])
             filtered = self._filter_latest_collections(collections_list)
-            logger.debug(f"Filtered collections: {filtered}")
+            logger.debug(
+                f"Filtered collections: {len(filtered)} collections"
+            )  # Don't log the actual data
             return CollectionsCache(collections=filtered, raw_response=response)
         except Exception as e:
-            logger.error(f"Error getting collections: {e}")
-            raise e
+            sanitized_error = self._sanitise_api_key(str(e))
+            logger.error(f"Error getting collections: {sanitized_error}")
+            raise ValueError(f"Failed to get collections: {sanitized_error}")
 
     async def cache_collections(self) -> CollectionsCache:
         """
@@ -132,7 +134,8 @@ async def cache_collections(self) -> CollectionsCache:
                     f"Collections successfully cached - {len(self._cached_collections.collections)} collections after filtering"
                 )
             except Exception as e:
-                raise ValueError(f"Failed to cache collections: {e}")
+                sanitized_error = self._sanitise_api_key(str(e))
+                raise ValueError(f"Failed to cache collections: {sanitized_error}")
         return self._cached_collections
 
     async def initialise(self):
@@ -163,19 +166,59 @@ async def get_api_key(self) -> str:
             raise ValueError("OS_API_KEY environment variable is not set")
         return api_key
 
-    def _sanitize_response(self, data: Any) -> Any:
-        """Remove API keys from response URLs recursively"""
+    def _sanitise_api_key(self, text: Any) -> str:
+        """Remove API keys from any text (URLs, error messages, etc.)"""
+        if not isinstance(text, str):
+            return text
+
+        patterns = [
+            r"[?&]key=[^&\s]*",
+            r"[?&]api_key=[^&\s]*",
+            r"[?&]apikey=[^&\s]*",
+            r"[?&]token=[^&\s]*",
+        ]
+
+        sanitized = text
+        for pattern in patterns:
+            sanitized = re.sub(pattern, "", sanitized, flags=re.IGNORECASE)
+
+        sanitized = re.sub(r"[?&]$", "", sanitized)
+        sanitized = re.sub(r"&{2,}", "&", sanitized)
+        sanitized = re.sub(r"\?&", "?", sanitized)
+
+        return sanitized
+
+    def _sanitise_response(self, data: Any) -> Any:
+        """Remove API keys from response data recursively"""
         if isinstance(data, dict):
+            sanitized_dict = {}
             for key, value in data.items():
-                if key == "href" and isinstance(value, str):
-                    data[key] = re.sub(r'[?&]key=[^&]*', '', value)
-                    data[key] = re.sub(r'[?&]$', '', data[key])
-                    data[key] = re.sub(r'&{2,}', '&', data[key])
+                if isinstance(value, str) and any(
+                    url_indicator in key.lower()
+                    for url_indicator in ["href", "url", "link", "uri"]
+                ):
+                    sanitized_dict[key] = self._sanitise_api_key(value)
                 elif isinstance(value, (dict, list)):
-                    data[key] = self._sanitize_response(value)
+                    sanitized_dict[key] = self._sanitise_response(value)
+                else:
+                    sanitized_dict[key] = value
+            return sanitized_dict
         elif isinstance(data, list):
-            return [self._sanitize_response(item) for item in data]
-        
+            return [self._sanitise_response(item) for item in data]
+        elif isinstance(data, str):
+            if any(
+                indicator in data
+                for indicator in [
+                    "http://",
+                    "https://",
+                    "key=",
+                    "api_key=",
+                    "apikey=",
+                    "token=",
+                ]
+            ):
+                return self._sanitise_api_key(data)
+
         return data
 
     async def make_request(
@@ -217,14 +260,15 @@ async def make_request(
 
         api_key = await self.get_api_key()
         request_params = params or {}
-        request_params["key"] = api_key  
+        request_params["key"] = api_key
 
         headers = {"User-Agent": self.user_agent, "Accept": "application/json"}
 
         client_ip = getattr(self.session, "_source_address", None)
         client_info = f" from {client_ip}" if client_ip else ""
 
-        logger.info(f"Requesting URL: {endpoint_value}{client_info}")
+        sanitized_url = self._sanitise_api_key(endpoint_value)
+        logger.info(f"Requesting URL: {sanitized_url}{client_info}")
 
         for attempt in range(1, max_retries + 1):
             try:
@@ -238,26 +282,29 @@ async def make_request(
                     timeout=timeout,
                 ) as response:
                     if response.status >= 400:
+                        # Sanitize error response text
+                        error_text = await response.text()
+                        sanitized_error = self._sanitise_api_key(error_text)
                         error_message = (
-                            f"HTTP Error: {response.status} - {await response.text()}"
+                            f"HTTP Error: {response.status} - {sanitized_error}"
                         )
                         logger.error(f"Error: {error_message}")
                         raise ValueError(error_message)
 
                     response_data = await response.json()
-                    
-                    return self._sanitize_response(response_data)
+
+                    return self._sanitise_response(response_data)
             except (aiohttp.ClientError, asyncio.TimeoutError) as e:
                 if attempt == max_retries:
-                    error_message = (
-                        f"Request failed after {max_retries} attempts: {str(e)}"
-                    )
+                    sanitized_exception = self._sanitise_api_key(str(e))
+                    error_message = f"Request failed after {max_retries} attempts: {sanitized_exception}"
                     logger.error(f"Error: {error_message}")
                     raise ValueError(error_message)
                 else:
                     await asyncio.sleep(0.7)
             except Exception as e:
-                error_message = f"Request failed: {str(e)}"
+                sanitized_exception = self._sanitise_api_key(str(e))
+                error_message = f"Request failed: {sanitized_exception}"
                 logger.error(f"Error: {error_message}")
                 raise ValueError(error_message)
         raise RuntimeError(

src/mcp_service/guardrails.py

@@ -26,7 +26,7 @@ def __init__(self):
             r"(?i)leak confidential",
             r"(?i)reveal secrets",
             r"(?i)expose secrets",
-            r"(?i)secrets.*contain", 
+            r"(?i)secrets.*contain",
             r"(?i)extract secrets",
         ]