fix: address PR review -- error handling, URL encoding, precise validation

4 findings verified and fixed:

1. Wrap GitHub API calls in try/except for httpx.TimeoutException,
   httpx.RequestError, and ValueError (JSON parse). Returns proper
   504/502 instead of unhandled crash.

2. URL-encode branch name with urllib.parse.quote(safe='') so
   branches like 'feature/foo' produce 'feature%2Ffoo' in the
   GitHub Tree API URL instead of breaking the path.

3. Tests now use pytest.raises(ValidationError) instead of broad
   Exception -- catches only Pydantic validation errors as intended.
   Added test_rejects_malformed_github_domain for 'fakegithub.com'.

4. AnalyzeRepoRequest validator uses _GITHUB_URL_RE regex instead of
   substring check -- rejects 'fakegithub.com' and 'notgithub.com.evil'.

25 tests pass.

Author: DevanshuNEU

backend/routes/repos.py

@@ -3,6 +3,7 @@
 from pydantic import BaseModel, field_validator
 from typing import List, Optional
 from pathlib import Path
+from urllib.parse import quote
 import hashlib
 import os
 import re
@@ -185,16 +186,24 @@ async def _fetch_directory_tree(
     """
     from services.repo_validator import RepoValidator
 
-    url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/git/trees/{branch}?recursive=1"
+    # Encode branch for URL safety -- "feature/foo" -> "feature%2Ffoo"
+    encoded_branch = quote(branch, safe="")
+    url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/git/trees/{encoded_branch}?recursive=1"
 
     async def _get(c: httpx.AsyncClient) -> httpx.Response:
         return await c.get(url, headers=_github_headers())
 
-    if client:
-        response = await _get(client)
-    else:
-        async with httpx.AsyncClient(timeout=15.0) as c:
-            response = await _get(c)
+    try:
+        if client:
+            response = await _get(client)
+        else:
+            async with httpx.AsyncClient(timeout=15.0) as c:
+                response = await _get(c)
+    except httpx.TimeoutException:
+        raise HTTPException(status_code=504, detail="GitHub API request timed out")
+    except httpx.RequestError as e:
+        logger.error("GitHub tree API network error", error=str(e))
+        raise HTTPException(status_code=502, detail="Failed to connect to GitHub API")
 
     if response.status_code == 404:
         raise HTTPException(status_code=404, detail="Repository or branch not found")
@@ -203,7 +212,10 @@ async def _get(c: httpx.AsyncClient) -> httpx.Response:
     if response.status_code != 200:
         raise HTTPException(status_code=502, detail=f"GitHub API error: {response.status_code}")
 
-    data = response.json()
+    try:
+        data = response.json()
+    except ValueError:
+        raise HTTPException(status_code=502, detail="Invalid response from GitHub API")
     truncated = data.get("truncated", False)
 
     code_extensions = RepoValidator.CODE_EXTENSIONS
@@ -290,8 +302,10 @@ def validate_url(cls, v: str) -> str:
         v = v.strip().rstrip("/")
         if not v:
             raise ValueError("GitHub URL is required")
-        if "github.com" not in v.lower():
-            raise ValueError("Only GitHub URLs are supported")
+        if not _GITHUB_URL_RE.match(v):
+            raise ValueError(
+                "Invalid GitHub URL. Expected: https://github.com/owner/repo"
+            )
         return v
 
 
@@ -326,25 +340,36 @@ async def analyze_repository(request: AnalyzeRepoRequest) -> dict:
         return cached
 
     # Single httpx client for both GitHub API calls
-    async with httpx.AsyncClient(timeout=15.0) as client:
-        # 1. Fetch repo metadata for default branch and size
-        meta_resp = await client.get(
-            f"{_GITHUB_API_BASE}/repos/{owner}/{repo_name}",
-            headers=_github_headers(),
-        )
+    try:
+        async with httpx.AsyncClient(timeout=15.0) as client:
+            # 1. Fetch repo metadata for default branch and size
+            meta_resp = await client.get(
+                f"{_GITHUB_API_BASE}/repos/{owner}/{repo_name}",
+                headers=_github_headers(),
+            )
 
-        if meta_resp.status_code == 404:
-            raise HTTPException(status_code=404, detail="Repository not found")
-        if meta_resp.status_code == 403:
-            raise HTTPException(status_code=429, detail="GitHub API rate limit exceeded")
-        if meta_resp.status_code != 200:
-            raise HTTPException(status_code=502, detail="Failed to fetch repository metadata")
+            if meta_resp.status_code == 404:
+                raise HTTPException(status_code=404, detail="Repository not found")
+            if meta_resp.status_code == 403:
+                raise HTTPException(status_code=429, detail="GitHub API rate limit exceeded")
+            if meta_resp.status_code != 200:
+                raise HTTPException(status_code=502, detail="Failed to fetch repository metadata")
 
-        metadata = meta_resp.json()
-        default_branch = metadata.get("default_branch", "main")
+            try:
+                metadata = meta_resp.json()
+            except ValueError:
+                raise HTTPException(status_code=502, detail="Invalid response from GitHub API")
+            default_branch = metadata.get("default_branch", "main")
 
-        # 2. Fetch directory tree (reuse same client)
-        tree_data = await _fetch_directory_tree(owner, repo_name, default_branch, client=client)
+            # 2. Fetch directory tree (reuse same client)
+            tree_data = await _fetch_directory_tree(owner, repo_name, default_branch, client=client)
+    except HTTPException:
+        raise
+    except httpx.TimeoutException:
+        raise HTTPException(status_code=504, detail="GitHub API request timed out")
+    except httpx.RequestError as e:
+        logger.error("GitHub API network error", error=str(e))
+        raise HTTPException(status_code=502, detail="Failed to connect to GitHub API")
 
     logger.info(
         "Analyzed repo structure",

backend/tests/test_analyze_repo.py

@@ -1,6 +1,7 @@
 """Tests for POST /repos/analyze -- pre-clone directory analysis (OPE-109)."""
 import pytest
 from unittest.mock import AsyncMock, patch, MagicMock
+from pydantic import ValidationError
 
 # Import after conftest patches external services
 from routes.repos import (
@@ -50,13 +51,17 @@ def test_strips_whitespace_and_slash(self):
         assert req.github_url == "https://github.com/owner/repo"
 
     def test_rejects_empty(self):
-        with pytest.raises(Exception):
+        with pytest.raises(ValidationError):
             AnalyzeRepoRequest(github_url="")
 
     def test_rejects_non_github(self):
-        with pytest.raises(Exception):
+        with pytest.raises(ValidationError):
             AnalyzeRepoRequest(github_url="https://gitlab.com/owner/repo")
 
+    def test_rejects_malformed_github_domain(self):
+        with pytest.raises(ValidationError):
+            AnalyzeRepoRequest(github_url="https://fakegithub.com/owner/repo")
+
 
 # -- IndexConfig validation (from PR #266) ------------------------------------
 
@@ -74,19 +79,19 @@ def test_normalizes_backslashes(self):
         assert cfg.include_paths == ["packages/effect"]
 
     def test_rejects_empty_string(self):
-        with pytest.raises(Exception):
+        with pytest.raises(ValidationError):
             IndexConfig(include_paths=["packages/effect", "  "])
 
     def test_rejects_path_traversal(self):
-        with pytest.raises(Exception):
+        with pytest.raises(ValidationError):
             IndexConfig(include_paths=["../etc/passwd"])
 
     def test_rejects_nested_traversal(self):
-        with pytest.raises(Exception):
+        with pytest.raises(ValidationError):
             IndexConfig(include_paths=["packages/../../etc"])
 
     def test_rejects_non_string(self):
-        with pytest.raises(Exception):
+        with pytest.raises(ValidationError):
             IndexConfig(include_paths=[123])
 
     def test_none_is_valid(self):