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/files.py b/app/api/routers/files.py index e00bf8b..0c96f8c 100644 --- a/app/api/routers/files.py +++ b/app/api/routers/files.py @@ -51,9 +51,12 @@ async def api_read_file( @router.post("/admin/api/files/summarize") async def api_summarize_file(payload: FileSummarizePayload): try: + raw_path = payload.path.strip() if payload.path else "" + if not raw_path or "\x00" in raw_path or any(part == ".." for part in raw_path.replace("\\", "/").split("/")): + return JSONResponse(status_code=400, content={"error": "Path traversal or invalid path detected."}) summarizer = get_summarizer_service() summary_text = summarizer.get_or_create_summary( - filepath=payload.path, + filepath=raw_path, repo=payload.repo, force_refresh=payload.force_refresh ) diff --git a/app/api/routers/repositories.py b/app/api/routers/repositories.py index 0fa94d4..69f59ff 100644 --- a/app/api/routers/repositories.py +++ b/app/api/routers/repositories.py @@ -198,7 +198,10 @@ async def api_get_paths(): @router.post("/admin/api/paths") async def api_add_path(config: LocalPathConfig): try: - resolved = os.path.abspath(config.path) + raw_path = config.path.strip() if config.path else "" + if not raw_path or "\x00" in raw_path or any(part == ".." for part in raw_path.replace("\\", "/").split("/")): + return JSONResponse(status_code=400, content={"error": "Path traversal or invalid path detected."}) + resolved = os.path.normpath(os.path.abspath(raw_path)) if not os.path.exists(resolved): return JSONResponse(status_code=400, content={"error": f"Path '{resolved}' does not exist on disk."}) @@ -279,7 +282,10 @@ async def api_test_search(payload: SearchRequest): @router.get("/admin/api/browse") async def api_browse_dir(path: str = "/"): - resolved = os.path.abspath(path) + cleaned = path.strip() if path else "/" + if "\x00" in cleaned or any(part in ("..", ".") for part in cleaned.replace("\\", "/").split("/") if part): + cleaned = "/" + resolved = os.path.normpath(os.path.abspath(cleaned)) if not os.path.exists(resolved): resolved = "/" try: 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/adr.py b/app/services/adr.py index af7abb7..98fd9a6 100644 --- a/app/services/adr.py +++ b/app/services/adr.py @@ -155,9 +155,10 @@ def sync_adr_file(filepath: str, repo: str, content: Optional[str] = None) -> Op """Reads or receives ADR markdown file content and syncs it to SQLite database.""" try: if content is None: - if not os.path.exists(filepath): + norm_fp = os.path.normpath(os.path.abspath(filepath)) + if not os.path.exists(norm_fp): return None - with open(filepath, "r", encoding="utf-8", errors="ignore") as f: + with open(norm_fp, "r", encoding="utf-8", errors="ignore") as f: content = f.read() parsed = parse_adr_markdown(content, filepath, repo) 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/file_reader.py b/app/services/file_reader.py index 10c8a55..3c6a0cb 100644 --- a/app/services/file_reader.py +++ b/app/services/file_reader.py @@ -37,6 +37,10 @@ def _is_within_root(self, target: str, root: str) -> bool: and os.path.realpath(target_abs) == os.path.realpath(root_abs) ) + root_prefix = root_abs if root_abs.endswith(os.sep) else root_abs + os.sep + if not target_abs.startswith(root_prefix) and target_abs != root_abs: + return False + try: if os.path.commonpath([target_abs, root_abs]) != root_abs: return False @@ -46,6 +50,10 @@ def _is_within_root(self, target: str, root: str) -> bool: # Verify symlink containment to prevent symlink breakouts target_real = os.path.realpath(target_abs) root_real = os.path.realpath(root_abs) + root_real_prefix = root_real if root_real.endswith(os.sep) else root_real + os.sep + if not target_real.startswith(root_real_prefix) and target_real != root_real: + return False + try: if os.path.commonpath([target_real, root_real]) != root_real: return False diff --git a/app/services/indexing/processor.py b/app/services/indexing/processor.py index 6664214..ee87a99 100644 --- a/app/services/indexing/processor.py +++ b/app/services/indexing/processor.py @@ -380,7 +380,8 @@ def process_file_content( metadata={"kind": chunk.get("kind", "code")} )) - mtime = os.path.getmtime(filepath) if os.path.exists(filepath) else 0.0 + filepath_norm = os.path.normpath(os.path.abspath(filepath)) + mtime = os.path.getmtime(filepath_norm) if os.path.exists(filepath_norm) else 0.0 summary_tuple = ( filepath, repo, 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/app/services/local_storage.py b/app/services/local_storage.py index 41c9dd6..5131e41 100644 --- a/app/services/local_storage.py +++ b/app/services/local_storage.py @@ -42,13 +42,19 @@ def resolve_safe_path(self, rel_path: str) -> str: if any(part == ".." for part in parts): raise ValueError("Path traversal or invalid path detected") - target = os.path.abspath(os.path.join(self.storage_root, cleaned)) + norm_root = os.path.normpath(os.path.abspath(self.storage_root)) + target = os.path.normpath(os.path.abspath(os.path.join(norm_root, cleaned))) + + prefix = norm_root if norm_root.endswith(os.sep) else norm_root + os.sep + if not target.startswith(prefix) and target != norm_root: + raise ValueError("Path traversal or invalid path detected") + try: - common = os.path.commonpath([target, self.storage_root]) + common = os.path.commonpath([target, norm_root]) except ValueError: raise ValueError("Path traversal or invalid path detected") - if common != self.storage_root: + if common != norm_root: raise ValueError("Path traversal or invalid path detected") return target @@ -61,6 +67,8 @@ def save_file_content( category: Optional[str] = None ) -> Dict[str, Any]: target_path = self.resolve_safe_path(rel_path) + if not target_path.startswith(self.storage_root): + raise ValueError("Path traversal or invalid path detected") os.makedirs(os.path.dirname(target_path), exist_ok=True) if isinstance(content, bytes): @@ -97,6 +105,8 @@ def index_file( category: Optional[str] = None ) -> Dict[str, Any]: abs_path = self.resolve_safe_path(rel_path) + if not abs_path.startswith(self.storage_root): + raise ValueError("Path traversal or invalid path detected") if not os.path.exists(abs_path) or not os.path.isfile(abs_path): raise FileNotFoundError(f"File '{rel_path}' not found on disk for indexing.") @@ -206,6 +216,8 @@ def save_file( def delete_file(self, rel_path: str, repo: str = "local_storage") -> Dict[str, Any]: abs_path = self.resolve_safe_path(rel_path) + if not abs_path.startswith(self.storage_root): + raise ValueError("Path traversal or invalid path detected") self.delete_file_disk(rel_path) try: @@ -232,6 +244,8 @@ def delete_file(self, rel_path: str, repo: str = "local_storage") -> Dict[str, A def read_file_content(self, rel_path: str) -> Dict[str, Any]: target_path = self.resolve_safe_path(rel_path) + if not target_path.startswith(self.storage_root): + raise ValueError("Path traversal or invalid path detected") if not os.path.exists(target_path) or not os.path.isfile(target_path): raise FileNotFoundError(f"File '{rel_path}' does not exist in local storage.") @@ -269,6 +283,8 @@ def read_file_content(self, rel_path: str) -> Dict[str, Any]: def delete_file_disk(self, rel_path: str) -> bool: target_path = self.resolve_safe_path(rel_path) + if not target_path.startswith(self.storage_root): + raise ValueError("Path traversal or invalid path detected") if not os.path.exists(target_path): return False @@ -280,6 +296,8 @@ def delete_file_disk(self, rel_path: str) -> bool: def get_file_tree(self, subfolder: Optional[str] = None) -> Dict[str, Any]: scan_root = self.resolve_safe_path(subfolder) if subfolder else self.storage_root + if not scan_root.startswith(self.storage_root): + raise ValueError("Path traversal or invalid path detected") if not os.path.exists(scan_root): return {"root": self.storage_root, "current_folder": subfolder or "", "directories": [], "files": []} diff --git a/app/services/pdf_extractor.py b/app/services/pdf_extractor.py index f8660d7..3281630 100644 --- a/app/services/pdf_extractor.py +++ b/app/services/pdf_extractor.py @@ -85,11 +85,12 @@ def extract_pdf_pages( raise ValueError(f"PDF exceeds size limit of {MAX_PDF_SIZE_BYTES // (1024*1024)}MB") doc = pymupdf.open(stream=source, filetype="pdf") else: - if not os.path.exists(source): + norm_source = os.path.normpath(os.path.abspath(source)) + if not os.path.exists(norm_source): raise FileNotFoundError(f"PDF file not found: {source}") - if os.path.getsize(source) > MAX_PDF_SIZE_BYTES: + if os.path.getsize(norm_source) > MAX_PDF_SIZE_BYTES: raise ValueError(f"PDF exceeds size limit of {MAX_PDF_SIZE_BYTES // (1024*1024)}MB") - doc = pymupdf.open(source) + doc = pymupdf.open(norm_source) except Exception as e: if isinstance(e, (ValueError, FileNotFoundError)): raise diff --git a/app/services/summarizer.py b/app/services/summarizer.py index f35ac37..4b08372 100644 --- a/app/services/summarizer.py +++ b/app/services/summarizer.py @@ -243,17 +243,21 @@ def get_or_create_summary( content = None if content is None: - if os.path.exists(filepath): + norm_fp = os.path.normpath(os.path.abspath(filepath)) + if os.path.exists(norm_fp) and os.path.isfile(norm_fp): try: - with open(filepath, "r", encoding="utf-8", errors="replace") as f: + with open(norm_fp, "r", encoding="utf-8", errors="replace") as f: content = f.read() except Exception as e: - logger.warning(f"Failed to read file from disk '{filepath}': {e}") + logger.warning(f"Failed to read file from disk '{norm_fp}': {e}") else: try: from app.services.local_storage import get_default_storage_path - storage_cand = os.path.join(get_default_storage_path(), filepath) - if os.path.exists(storage_cand): + storage_root = os.path.normpath(os.path.abspath(get_default_storage_path())) + storage_prefix = storage_root if storage_root.endswith(os.sep) else storage_root + os.sep + cleaned_fp = filepath.strip().replace("\\", "/").lstrip("/") + storage_cand = os.path.normpath(os.path.abspath(os.path.join(storage_root, cleaned_fp))) + if storage_cand.startswith(storage_prefix) and os.path.exists(storage_cand) and os.path.isfile(storage_cand): with open(storage_cand, "r", encoding="utf-8", errors="replace") as f: content = f.read() except Exception: diff --git a/app/services/vector_store/chroma_store.py b/app/services/vector_store/chroma_store.py index d466cad..886b065 100644 --- a/app/services/vector_store/chroma_store.py +++ b/app/services/vector_store/chroma_store.py @@ -113,20 +113,24 @@ def __init__( if target_storage == ":memory:": self.client = chromadb.EphemeralClient() self.mode = "memory" + self.location = target_storage else: - os.makedirs(target_storage, exist_ok=True) - self.client = chromadb.PersistentClient(path=target_storage) + clean_storage = os.path.normpath(os.path.abspath(target_storage)) + os.makedirs(clean_storage, exist_ok=True) + self.client = chromadb.PersistentClient(path=clean_storage) self.mode = "persistent" - self.location = target_storage + self.location = clean_storage else: if target_storage == ":memory:": self.client = chromadb.EphemeralClient() self.mode = "memory" + self.location = target_storage else: - os.makedirs(target_storage, exist_ok=True) - self.client = chromadb.PersistentClient(path=target_storage) + clean_storage = os.path.normpath(os.path.abspath(target_storage)) + os.makedirs(clean_storage, exist_ok=True) + self.client = chromadb.PersistentClient(path=clean_storage) self.mode = "persistent" - self.location = target_storage + self.location = clean_storage logger.info(f"Initialized local Chroma client at {self.location}") if auto_init: diff --git a/app/services/vector_store/qdrant_store.py b/app/services/vector_store/qdrant_store.py index 594e749..f6bb8fd 100644 --- a/app/services/vector_store/qdrant_store.py +++ b/app/services/vector_store/qdrant_store.py @@ -64,21 +64,25 @@ def __init__( if target_storage == ":memory:": self.client = QdrantClient(location=":memory:") self.mode = "memory" + self.location = target_storage else: - os.makedirs(target_storage, exist_ok=True) - self.client = QdrantClient(path=target_storage) + clean_storage = os.path.normpath(os.path.abspath(target_storage)) + os.makedirs(clean_storage, exist_ok=True) + self.client = QdrantClient(path=clean_storage) self.mode = "embedded" - self.location = target_storage + self.location = clean_storage else: target_storage = storage_path or storage_path_env if target_storage == ":memory:": self.client = QdrantClient(location=":memory:") self.mode = "memory" + self.location = target_storage else: - os.makedirs(target_storage, exist_ok=True) - self.client = QdrantClient(path=target_storage) + clean_storage = os.path.normpath(os.path.abspath(target_storage)) + os.makedirs(clean_storage, exist_ok=True) + self.client = QdrantClient(path=clean_storage) self.mode = "embedded" - self.location = target_storage + self.location = clean_storage logger.info(f"Initialized embedded Qdrant client at {self.location}") if auto_init: 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"]