Repo param injection, stats error handling, stack trace leakage, dispatch logging

Author: peteroyce

src/github_client.py

@@ -45,7 +45,7 @@ def _client(token: str | None = None) -> Github:
 
 
 def _default_org() -> str | None:
-    return os.getenv("GITHUB_DEFAULT_ORG") or None
+    return os.getenv("GITHUB_DEFAULT_ORG")
 
 
 def _repo(g: Github, owner: str, repo: str) -> Repository:
@@ -262,6 +262,11 @@ def search_code(query: str, repo: str | None = None) -> list[dict[str, Any]]:
 
     Returns a list of result dicts.
     """
+    if repo is not None:
+        _REPO_RE = re.compile(r"^[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+$")
+        if not _REPO_RE.match(repo):
+            raise ValueError("Invalid repo format: must be owner/repo")
+
     g = _client()
     safe_query = _sanitise_query(query)
     full_query = f"{safe_query} repo:{repo}" if repo else safe_query
@@ -420,9 +425,9 @@ def get_weekly_digest(owner: str, repo: str) -> dict[str, Any]:
                 {"login": login, "commits": commits} for login, commits in weekly[:5]
             ]
     except RateLimitExceededException:
-        pass  # contributor stats are best-effort in the digest
+        top_contributors = {"available": False, "reason": "stats_unavailable"}
     except GithubException:
-        pass  # stats may not be ready on newly created repos
+        top_contributors = {"available": False, "reason": "stats_unavailable"}
 
     return {
         "period": {"from": since.isoformat(), "to": now.isoformat()},

src/server.py

@@ -9,6 +9,7 @@
 
 import asyncio
 import json
+import logging
 import traceback
 from typing import Any
 
@@ -461,6 +462,7 @@ def _handle_get_weekly_digest(args: dict[str, Any]) -> str:
 
 def _dispatch(name: str, args: dict[str, Any]) -> str:
     """Synchronous dispatcher — called inside an executor."""
+    logging.info(f"Tool called: {name}")
     handler = _TOOL_HANDLERS.get(name)
     if handler is None:
         known = ", ".join(sorted(_TOOL_HANDLERS))
@@ -484,8 +486,9 @@ async def handle_call_tool(
         error_text = f"Configuration error: {exc}"
     except RuntimeError as exc:
         error_text = f"GitHub API error: {exc}"
-    except Exception:
-        error_text = f"Unexpected error:\n{traceback.format_exc()}"
+    except Exception as exc:
+        logging.exception("Unexpected error in tool dispatch")
+        error_text = f"Error: {type(exc).__name__}: {exc}"
 
     return [types.TextContent(type="text", text=error_text)]
 

tests/test_github_client.py

@@ -523,6 +523,35 @@ def test_rate_limit_exception_raises_runtime_with_reset_info(self):
             with pytest.raises(RuntimeError, match="rate limit"):
                 gh.search_code("test")
 
+    def test_repo_param_injection_raises_value_error(self):
+        """A repo value containing injection payload must raise ValueError, not reach GitHub."""
+        mock_g = MagicMock()
+        mock_g.search_code.return_value = iter([])
+
+        with patch("src.github_client._client", return_value=mock_g):
+            from src import github_client as gh
+
+            with pytest.raises(ValueError, match="Invalid repo format"):
+                gh.search_code("foo", repo="owner/repo repo:evil/bad")
+
+        # The malicious string must never have been forwarded to the GitHub API
+        mock_g.search_code.assert_not_called()
+
+    def test_valid_repo_param_accepted(self):
+        """A well-formed owner/repo string must pass validation without error."""
+        mock_g = MagicMock()
+        mock_g.search_code.return_value = iter([])
+
+        with patch("src.github_client._client", return_value=mock_g):
+            from src import github_client as gh
+
+            # Should not raise
+            gh.search_code("foo", repo="valid-owner/valid.repo_name")
+
+        mock_g.search_code.assert_called_once()
+        sent_query = mock_g.search_code.call_args[0][0]
+        assert "repo:valid-owner/valid.repo_name" in sent_query
+
 
 # ---------------------------------------------------------------------------
 # get_contributor_stats