diff --git a/REQUIREMENTS.md b/REQUIREMENTS.md index d480db4..100dcf1 100644 --- a/REQUIREMENTS.md +++ b/REQUIREMENTS.md @@ -2,7 +2,7 @@ > **Note:** This document is automatically generated and verified against the live test suite by `scripts/generate_requirements.py` and `tests/backend/test_requirements_sync.py`. -**Test Verification Baseline:** **968 Automated Tests** (651 Pytest Backend + 271 Vitest Frontend + 46 Playwright E2E). +**Test Verification Baseline:** **969 Automated Tests** (652 Pytest Backend + 271 Vitest Frontend + 46 Playwright E2E). --- @@ -980,7 +980,7 @@ persisting all records and vector points correctly across multiple flushes._ - `test_incremental_pipeline_clone_error_resilience` - _Verifies that a failure during shallow clone records an error in git_repositories and leaves the prior indexed state intact without data loss._ -#### `tests/test_litellm_service.py` (7 tests) +#### `tests/test_litellm_service.py` (8 tests) - `test_discover_models_success` - `test_discover_models_timeout` - `test_discover_models_connect_error` @@ -988,6 +988,7 @@ and leaves the prior indexed state intact without data loss._ - `test_discover_models_http_500_error` - `test_discover_models_url_normalization` - `test_discover_models_default_resolution` +- `test_discover_models_ssrf_rejection` #### `tests/test_local_storage_indexing.py` (4 tests) - `test_incremental_indexing_on_save` diff --git a/app/api/routers/settings.py b/app/api/routers/settings.py index cffa42e..6266be9 100644 --- a/app/api/routers/settings.py +++ b/app/api/routers/settings.py @@ -4,6 +4,7 @@ import sqlite3 import logging from typing import Optional +from urllib.parse import urlsplit from fastapi import APIRouter, Request from fastapi.responses import JSONResponse @@ -326,11 +327,24 @@ def _reindex(): @router.get("/admin/api/models/discover") async def api_discover_models(url: Optional[str] = None, api_key: Optional[str] = None): try: + if url: + parsed = urlsplit(url.strip()) + if parsed.scheme not in ("http", "https"): + return JSONResponse( + status_code=400, + content={"status": "error", "error": "Invalid URL scheme: only http and https are allowed."} + ) + host = (parsed.hostname or "").lower() + if not host or host in ("169.254.169.254", "metadata.google.internal") or host.startswith("169.254."): + return JSONResponse( + status_code=400, + content={"status": "error", "error": "Invalid or restricted target host."} + ) res = await litellm_service.discover_models(url=url, api_key=api_key) return res except Exception as e: logger.error(f"Error discovering models: {e}") - return JSONResponse(status_code=500, content={"status": "error", "error": str(e), "message": str(e)}) + return JSONResponse(status_code=500, content={"status": "error", "error": "Failed to discover models."}) @router.get("/admin/api/settings/embedding") async def api_get_embedding_settings(): diff --git a/app/services/auth/key_service.py b/app/services/auth/key_service.py index 32f1083..3f96181 100644 --- a/app/services/auth/key_service.py +++ b/app/services/auth/key_service.py @@ -41,8 +41,9 @@ def _get_engine(self, engine: Optional[Engine] = None) -> Engine: @staticmethod def hash_key(raw_key: str) -> str: - """Computes deterministic SHA-256 hash of secret key string.""" - return hashlib.sha256(raw_key.encode("utf-8")).hexdigest() + """Computes deterministic PBKDF2-HMAC-SHA256 hash of key string.""" + salt = b"contextcortex_api_key_salt_v1" + return hashlib.pbkdf2_hmac("sha256", raw_key.encode("utf-8"), salt, 50_000).hex() def issue_api_key( self, @@ -94,7 +95,7 @@ def issue_api_key( ).first() inserted_id = row[0] if row else 0 - logger.info(f"Issued new API key '{name}' (id={inserted_id}, prefix={key_prefix}, role={assigned_role.value})") + logger.info(f"Issued new API key '{name}' (id={inserted_id}, role={assigned_role.value})") return ApiKeyOut( id=inserted_id, @@ -305,7 +306,7 @@ def bootstrap_admin_key( group_name="admin", engine=eng, ) - logger.info(f"Auto-bootstrapped initial admin API key (prefix: {key.key_prefix})") + logger.info("Auto-bootstrapped initial admin API key.") return key # Custom explicit secret key specified @@ -319,7 +320,7 @@ def bootstrap_admin_key( ).mappings().fetchone() if row: - logger.info(f"Bootstrap admin key already registered (id={row['id']}, prefix={key_prefix})") + logger.info(f"Bootstrap admin key already registered (id={row['id']})") return ApiKeyOut( id=row["id"], name=row["name"], @@ -355,7 +356,7 @@ def bootstrap_admin_key( ).first() inserted_id = r[0] if r else 0 - logger.info(f"Bootstrapped configured initial admin key (id={inserted_id}, prefix={key_prefix})") + logger.info(f"Bootstrapped configured initial admin key (id={inserted_id})") return ApiKeyOut( id=inserted_id, name=name.strip(), diff --git a/app/services/chunking/api_route_extractor.py b/app/services/chunking/api_route_extractor.py index 74afa3b..6f4ad1c 100644 --- a/app/services/chunking/api_route_extractor.py +++ b/app/services/chunking/api_route_extractor.py @@ -17,7 +17,7 @@ def normalize_path_pattern(path: str) -> str: # Express style :param path = re.sub(r':([a-zA-Z_][a-zA-Z0-9_]*)', r'{\1}', path) # Template literal ${param} - path = re.sub(r'\$\{([^}]+)\}', r'{\1}', path) + path = re.sub(r'\$\{([^{}]+)\}', r'{\1}', path) # Next.js [id] path = re.sub(r'\[([a-zA-Z_][a-zA-Z0-9_]*)\]', r'{\1}', path) @@ -167,8 +167,14 @@ def extract_api_routes_and_calls( if re.search(r'route\.(?:ts|js|tsx|jsx)$', norm_fp): # Infer route path from folder structure # e.g. app/api/users/[id]/route.ts -> /api/users/{id} - app_match = re.search(r'(?:app|pages)(/.*?)/route\.(?:ts|js|tsx|jsx)$', norm_fp) - route_path = normalize_path_pattern(app_match.group(1)) if app_match else "/" + route_base = re.sub(r'/route\.(?:ts|js|tsx|jsx)$', '', norm_fp) + parts = route_base.split('/') + raw_route_path = "/" + for idx, part in enumerate(parts): + if part in ("app", "pages") and idx + 1 < len(parts): + raw_route_path = "/" + "/".join(parts[idx + 1:]) + break + route_path = normalize_path_pattern(raw_route_path) for i, line in enumerate(lines, start=1): m_next = re.search(r'export\s+(?:async\s+)?function\s+(GET|POST|PUT|DELETE|PATCH|HEAD|OPTIONS)\b', line) if m_next: diff --git a/app/services/litellm_service.py b/app/services/litellm_service.py index 692056e..502d4a8 100644 --- a/app/services/litellm_service.py +++ b/app/services/litellm_service.py @@ -1,6 +1,7 @@ import os import logging from typing import Optional, Dict, Any, List +from urllib.parse import urlsplit, urlunsplit import httpx from app.services.database import get_embedding_db_config @@ -38,14 +39,45 @@ async def discover_models( or "dummy" ) - # Normalize URL to target /models endpoint - clean_url = raw_url.strip().rstrip("/") - if clean_url.endswith("/models"): - endpoint = clean_url + # Validate URL against SSRF + parsed = urlsplit(raw_url.strip()) + if parsed.scheme not in ("http", "https"): + error_msg = f"Invalid URL scheme '{parsed.scheme}': only http and https are permitted." + logger.warning(f"LiteLLM model discovery rejected: {error_msg}") + return { + "status": "error", + "message": error_msg, + "total_models": 0, + "models": [], + "embedding_models": [], + "vision_models": [], + "chat_models": [], + } + + host = (parsed.hostname or "").lower() + if not host or host in ("169.254.169.254", "metadata.google.internal") or host.startswith("169.254."): + error_msg = f"Invalid or restricted host '{host}'." + logger.warning(f"LiteLLM model discovery rejected: {error_msg}") + return { + "status": "error", + "message": error_msg, + "total_models": 0, + "models": [], + "embedding_models": [], + "vision_models": [], + "chat_models": [], + } + + # Normalize URL to target /models endpoint safely + clean_path = parsed.path.rstrip("/") + if clean_path.endswith("/models"): + endpoint_path = clean_path else: - if not clean_url.endswith("/v1"): - clean_url = f"{clean_url}/v1" - endpoint = f"{clean_url}/models" + if not clean_path.endswith("/v1"): + clean_path = f"{clean_path}/v1" + endpoint_path = f"{clean_path}/models" + + endpoint = urlunsplit((parsed.scheme, parsed.netloc, endpoint_path, "", "")) headers = {"Authorization": f"Bearer {resolved_api_key}"} diff --git a/frontend/src/SearchInspector.tsx b/frontend/src/SearchInspector.tsx index 05e6ce3..fd8fcb8 100644 --- a/frontend/src/SearchInspector.tsx +++ b/frontend/src/SearchInspector.tsx @@ -89,19 +89,25 @@ export default function SearchInspector() { {p.symbol && {p.symbol}} (Lines {p.start_line}-{p.end_line}) {p.github_url && (() => { + let hostname = ''; + try { + hostname = new URL(p.github_url).hostname.toLowerCase(); + } catch { + // Ignore invalid URL + } const u = p.github_url.toLowerCase(); let label = 'View Source'; let icon = 'fa-solid fa-code-branch'; - if (u.includes('gitlab') || u.includes('/-/blob/')) { + if (hostname === 'gitlab.com' || hostname.endsWith('.gitlab.com') || u.includes('/-/blob/')) { label = 'View on GitLab'; icon = 'fa-brands fa-gitlab'; - } else if (u.includes('gitea') || u.includes('forgejo')) { + } else if (hostname.includes('gitea') || hostname.includes('forgejo')) { label = 'View on Gitea'; icon = 'fa-solid fa-mug-hot'; - } else if (u.includes('bitbucket')) { + } else if (hostname === 'bitbucket.org' || hostname.endsWith('.bitbucket.org')) { label = 'View on Bitbucket'; icon = 'fa-brands fa-bitbucket'; - } else if (u.includes('github.com')) { + } else if (hostname === 'github.com' || hostname.endsWith('.github.com')) { label = 'View on GitHub'; icon = 'fa-brands fa-github'; } diff --git a/tests/test_litellm_service.py b/tests/test_litellm_service.py index 5d510b3..80a3b63 100644 --- a/tests/test_litellm_service.py +++ b/tests/test_litellm_service.py @@ -183,3 +183,16 @@ async def test_discover_models_default_resolution(monkeypatch): args, kwargs = mock_get.call_args assert args[0] == "http://custom-db-litellm:4000/v1/models" assert kwargs["headers"]["Authorization"] == "Bearer db-secret-key" + + +@pytest.mark.asyncio +async def test_discover_models_ssrf_rejection(): + # Test invalid scheme + res_scheme = await discover_models(url="file:///etc/passwd") + assert res_scheme["status"] == "error" + assert "Invalid URL scheme" in res_scheme["message"] + + # Test cloud metadata host + res_metadata = await discover_models(url="http://169.254.169.254/latest/meta-data") + assert res_metadata["status"] == "error" + assert "restricted host" in res_metadata["message"]