Skip to content

safety improvements - #56

Open
Nileneb wants to merge 21 commits into
openags:mainfrom
Nileneb:main
Open

safety improvements#56
Nileneb wants to merge 21 commits into
openags:mainfrom
Nileneb:main

Conversation

@Nileneb

@Nileneb Nileneb commented Apr 16, 2026

Copy link
Copy Markdown

No description provided.

…P functionality

- Created `start_server.sh` script for starting the Paper Search MCP server with HTTP/SSE and stdio modes.
- Added unit tests for HTTP endpoints in `test_http_endpoints.py`, covering health checks, platform listing, search, download, and read functionalities.
- Implemented end-to-end integration tests in `test_integration.py` to validate the overall workflow of the HTTP server.
- Developed tests for MCP JSON-RPC over HTTP in `test_mcp_http.py`, including session management and tool functionalities.
- Introduced tests for Server-Sent Events (SSE) in `test_sse.py`, ensuring proper event handling and client management.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR appears aimed at improving runtime safety and operational security for the MCP server by introducing an authenticated HTTP/SSE server mode, tightening URL parsing in several connectors, and updating dependencies.

Changes:

  • Bump several Python dependencies in uv.lock (notably cryptography, fastmcp, pypdf, requests, pygments) and add runtime deps for an ASGI server (starlette, uvicorn, python-dotenv).
  • Add an authenticated Starlette+Uvicorn SSE/REST serving path in paper_search_mcp/server.py, plus new tools for ingesting/searching a Laravel-backed RAG store.
  • Harden parsing logic in multiple academic platform connectors using urlparse, and add a start.sh helper script.

Reviewed changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
uv.lock Locks updated versions and adds ASGI/runtime dependencies.
pyproject.toml Adds ASGI/runtime dependencies; currently contains a duplicate pypdf requirement.
paper_search_mcp/server.py Introduces Bearer auth middleware + SSE/REST server mode + ingest/RAG tools; also introduces a couple of runtime-breaking issues.
paper_search_mcp/academic_platforms/oaipmh.py Uses urlparse for stricter DOI URL identification.
paper_search_mcp/academic_platforms/dblp.py Uses urlparse to extract DOI more safely from links.
paper_search_mcp/academic_platforms/citeseerx.py Uses hostname parsing to detect Wayback redirects more reliably.
paper_search_mcp/academic_platforms/base_search.py Uses urlparse for host checks when enriching identifiers.
paper_search_mcp/academic_platforms/semantic.py Adds urlparse import (currently unused).
Dockerfile Exposes port 8089 and switches container CMD to run the server module directly.
.gitignore Adds many ignore patterns; includes likely-incorrect globbing and rules that ignore .github/.
start.sh Adds a simple build-and-run helper script for Docker.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pyproject.toml
@@ -36,9 +36,13 @@ dependencies = [
"fastmcp",
"pypdf",

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pyproject.toml lists pypdf twice: once unpinned and once as pypdf>=3.9.0. This is redundant and can be confusing for dependency resolution/review. Keep only the constrained requirement (or consolidate into a single entry).

Suggested change
"pypdf",

Copilot uses AI. Check for mistakes.
from bs4 import BeautifulSoup
import time
import random
from urllib.parse import urlparse

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semantic.py adds urlparse to imports but it is not used anywhere in the module, which will trigger unused-import linting and adds noise. Remove the import or use it in the related URL-handling logic.

Suggested change
from urllib.parse import urlparse

Copilot uses AI. Check for mistakes.
Comment on lines 1 to +9
# paper_search_mcp/server.py
from typing import List, Dict, Optional, Any
import asyncio
import os
import hmac
import logging
import re
import os
from typing import Any, Dict, List, Optional
import httpx
from mcp.server.fastmcp import FastMCP
import uvicorn
from dotenv import load_dotenv

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server.py uses re.sub(...) in _safe_filename, but re is no longer imported after this change. This will raise a NameError at runtime the first time _safe_filename is called. Add back import re (or replace the regex usage) so downloads/fallback logic works.

Copilot uses AI. Check for mistakes.
Comment on lines +46 to +50
load_dotenv()

BEARER_TOKEN = os.environ.get("BEARER_TOKEN", "")
if not BEARER_TOKEN:
raise RuntimeError("BEARER_TOKEN environment variable must be set")

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BEARER_TOKEN is required at import time (raise RuntimeError(...)). This makes import paper_search_mcp.server fail in any context where the token isn’t set (including tests and MCP_TRANSPORT=stdio mode where HTTP auth isn’t needed). Move the validation into the SSE/HTTP startup path (e.g., inside create_app() or the else: branch in main()), and allow stdio transport to run without this variable.

Copilot uses AI. Check for mistakes.
Comment on lines +48 to +56
BEARER_TOKEN = os.environ.get("BEARER_TOKEN", "")
if not BEARER_TOKEN:
raise RuntimeError("BEARER_TOKEN environment variable must be set")

DEFAULT_DOWNLOAD_PATH = os.environ.get("DOWNLOAD_PATH", "./downloads")
MCP_MESSAGES_PATH = os.environ.get("MCP_MESSAGES_PATH", "/messages/")

LARAVEL_INGEST_URL = os.environ.get("LARAVEL_INGEST_URL", "")
LARAVEL_MCP_TOKEN = os.environ.get("LARAVEL_MCP_TOKEN", "")

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Environment lookup here bypasses the project’s get_env() helper (which supports PAPER_SEARCH_MCP_<NAME> and .env loading). As a result, PAPER_SEARCH_MCP_BEARER_TOKEN / PAPER_SEARCH_MCP_DOWNLOAD_PATH won’t be honored even though other parts of this module use get_env. Use get_env(...) consistently for these settings to match the rest of the codebase.

Suggested change
BEARER_TOKEN = os.environ.get("BEARER_TOKEN", "")
if not BEARER_TOKEN:
raise RuntimeError("BEARER_TOKEN environment variable must be set")
DEFAULT_DOWNLOAD_PATH = os.environ.get("DOWNLOAD_PATH", "./downloads")
MCP_MESSAGES_PATH = os.environ.get("MCP_MESSAGES_PATH", "/messages/")
LARAVEL_INGEST_URL = os.environ.get("LARAVEL_INGEST_URL", "")
LARAVEL_MCP_TOKEN = os.environ.get("LARAVEL_MCP_TOKEN", "")
BEARER_TOKEN = get_env("BEARER_TOKEN", "")
if not BEARER_TOKEN:
raise RuntimeError("BEARER_TOKEN environment variable must be set")
DEFAULT_DOWNLOAD_PATH = get_env("DOWNLOAD_PATH", "./downloads")
MCP_MESSAGES_PATH = get_env("MCP_MESSAGES_PATH", "/messages/")
LARAVEL_INGEST_URL = get_env("LARAVEL_INGEST_URL", "")
LARAVEL_MCP_TOKEN = get_env("LARAVEL_MCP_TOKEN", "")

Copilot uses AI. Check for mistakes.
Comment on lines +948 to +956
try:
text = searcher.read_paper(paper_id, save_path)
except Exception as e:
return {"error": f"Failed to read paper: {e}"}

Args:
paper_id: OpenAlex paper ID.
save_path: Directory where the PDF is/will be saved (default: './downloads').
Returns:
str: Message indicating that direct paper reading is not supported natively.
"""
return openalex_searcher.read_paper(paper_id, save_path)
try:
papers = searcher.search(paper_id, max_results=1)
title = papers[0].title if papers else paper_id
except Exception:

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ingest_paper calls searcher.read_paper(...) and searcher.search(...) directly. These methods are synchronous and perform network/disk I/O (e.g., ArxivSearcher.read_paper downloads/parses PDFs), so this will block the event loop and degrade concurrency under load. Run these blocking calls in a worker thread (e.g., await asyncio.to_thread(...)) and consider adding a reasonable timeout/size limit for extracted text.

Copilot uses AI. Check for mistakes.
Comment on lines +1016 to +1094
def create_app() -> Starlette:
"""Create a Starlette ASGI app with Bearer auth and SSE transport."""
import json as _json

# Keep the endpoint configurable so reverse proxies can avoid path collisions.
message_path = MCP_MESSAGES_PATH
if not message_path.startswith("/"):
message_path = f"/{message_path}"
if not message_path.endswith("/"):
message_path = f"{message_path}/"

sse = SseServerTransport(message_path)

async def handle_sse(request: Request):
async with sse.connect_sse(
request.scope, request.receive, request._send
) as streams:
await mcp._mcp_server.run(
streams[0],
streams[1],
mcp._mcp_server.create_initialization_options(),
)

async def handle_rest_search(request: Request):
"""Plain REST endpoint — callable from PHP without MCP protocol.

POST /search body: {"query": "...", "sources": [...], "max_results_per_source": 5}
GET /search params: query=...&sources=pubmed,arxiv&max_results_per_source=5
"""
return await async_search(ieee_searcher, query, max_results)

@mcp.tool()
async def download_ieee(paper_id: str, save_path: str = "./downloads") -> str:
"""Download a PDF from IEEE Xplore. Requires PAPER_SEARCH_MCP_IEEE_API_KEY (or IEEE_API_KEY) and institutional access.

Args:
paper_id: IEEE Xplore paper identifier.
save_path: Directory to save the PDF (default: './downloads').
Returns:
str: Path to saved PDF or error message.
"""
return await asyncio.to_thread(ieee_searcher.download_pdf, paper_id, save_path)

@mcp.tool()
async def read_ieee_paper(paper_id: str, save_path: str = "./downloads") -> str:
"""Download and read an IEEE Xplore paper. Requires PAPER_SEARCH_MCP_IEEE_API_KEY (or IEEE_API_KEY).

Args:
paper_id: IEEE Xplore paper identifier.
save_path: Directory where the PDF is/will be saved (default: './downloads').
Returns:
str: Extracted text content.
"""
return ieee_searcher.read_paper(paper_id, save_path)


# ---------------------------------------------------------------------------
# Optional ACM Digital Library tools — registered only when API key is set
# ---------------------------------------------------------------------------
if acm_searcher is not None:
@mcp.tool()
async def search_acm(query: str, max_results: int = 10) -> List[Dict]:
"""Search ACM Digital Library for papers. Requires PAPER_SEARCH_MCP_ACM_API_KEY (or ACM_API_KEY).

Args:
query: Search query string.
max_results: Maximum number of results (default: 10).
Returns:
List of paper dicts from ACM DL.
"""
return await async_search(acm_searcher, query, max_results)
try:
if request.method == "POST":
body = await request.body()
params = _json.loads(body) if body else {}
else:
params = dict(request.query_params)

query = str(params.get("query", "")).strip()
if not query:
return Response(
_json.dumps({"error": "query parameter required"}),
media_type="application/json",
status_code=400,
)

raw_sources = params.get("sources", "pubmed,arxiv,semantic")
if isinstance(raw_sources, list):
raw_sources = ",".join(raw_sources)

max_results = int(params.get("max_results_per_source", 5))
year = params.get("year") or None

results = await search_papers(
query=query,
max_results_per_source=max_results,
sources=raw_sources,
year=year,
)

return Response(
_json.dumps(results, default=str, ensure_ascii=False),
media_type="application/json",
)
except Exception as exc:
logging.exception("REST /search error")
return Response(
_json.dumps({"error": str(exc)}),
media_type="application/json",
status_code=500,
)

@mcp.tool()
async def download_acm(paper_id: str, save_path: str = "./downloads") -> str:
"""Download a PDF from ACM Digital Library. Requires PAPER_SEARCH_MCP_ACM_API_KEY (or ACM_API_KEY) and institutional access.
app = Starlette(
routes=[
Route("/sse", endpoint=handle_sse),
Route("/search", endpoint=handle_rest_search, methods=["GET", "POST"]),
Mount(message_path, app=sse.handle_post_message),
],
)

Args:
paper_id: ACM DL paper identifier.
save_path: Directory to save the PDF (default: './downloads').
Returns:
str: Path to saved PDF or error message.
"""
return await asyncio.to_thread(acm_searcher.download_pdf, paper_id, save_path)
return BearerAuthMiddleware(app, BEARER_TOKEN)

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_app() is annotated to return Starlette but actually returns BearerAuthMiddleware, which can confuse type checking and readers. Either change the return annotation to ASGIApp (or similar) or wrap the middleware using Starlette’s middleware mechanism so the returned object matches the annotation.

Copilot uses AI. Check for mistakes.
Comment on lines +1027 to +1037
sse = SseServerTransport(message_path)

async def handle_sse(request: Request):
async with sse.connect_sse(
request.scope, request.receive, request._send
) as streams:
await mcp._mcp_server.run(
streams[0],
streams[1],
mcp._mcp_server.create_initialization_options(),
)

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_app() reaches into private/internal APIs (request._send, mcp._mcp_server.*). This is fragile across Starlette/FastMCP versions and may break on upgrades. Prefer using a public ASGI integration provided by the MCP library (or expose a small adapter in FastMCP) and avoid relying on underscored attributes/members in the request/server objects.

Copilot uses AI. Check for mistakes.
Nileneb added 6 commits April 19, 2026 01:04
- Updated pypdf to version 6.10.1 and added python-multipart dependency.
- Added asyncpg dependency with version 0.31.0 and its related async-timeout dependency.
- Updated authlib to version 1.6.11.
- Updated pypdf to version 6.10.2.
- Updated python-multipart to version 0.0.26.
- Added context.py to manage user email context variable.
…est_paper tool

Bringt 2 Commits aus http_stream in main:
- start_server.sh + tests (test_http_endpoints, test_integration, test_mcp_http, test_sse)
- configurable downloads dir + ingest_paper MCP tool + server_http.py

Konflikte zugunsten von main aufgelöst (Dockerfile, README.md, server.py __main__).
pyproject.toml / smithery.yaml / uv.lock: main-Version übernommen (29 Commits aktueller).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants