Skip to content

[Feat] MCP: Hosted remote server (streamable-http)#1582

Merged
CREDO23 merged 14 commits into
MODSetter:mainfrom
CREDO23:feature-hosted-mcp-server
Jul 7, 2026
Merged

[Feat] MCP: Hosted remote server (streamable-http)#1582
CREDO23 merged 14 commits into
MODSetter:mainfrom
CREDO23:feature-hosted-mcp-server

Conversation

@CREDO23

@CREDO23 CREDO23 commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Adds a hosted remote MCP server over streamable-HTTP with per-request API-key auth (Bearer ss_pat_…), keeping stdio for self-hosters. Includes /health, a Dockerfile for deploy, and updated docs/web configs pointing at https://mcp.surfsense.com/mcp.

Tested: pytest passes; image builds and serves /health → 200, /mcp → 401 (no key).

High-level PR Summary

This PR adds hosted remote MCP server support over streamable-HTTP with per-request Bearer authentication, alongside the existing stdio transport for self-hosters. A new /health endpoint enables health checks, and the server can be deployed via the included Dockerfile. The authentication system uses contextvars for request-scoped identity isolation across concurrent users. All documentation and web configurations are updated to point at https://mcp.surfsense.com/mcp, and the knowledge-base and scraper tools are reorganized for clarity. As a side improvement, a LangGraph fan-in barrier bug in the video presentation agent is fixed.

⏱️ Estimated Review Time: 3+ hours

💡 Review Order Suggestion
Order File Path
1 surfsense_mcp/Dockerfile
2 surfsense_mcp/.dockerignore
3 surfsense_mcp/pyproject.toml
4 surfsense_mcp/uv.lock
5 surfsense_mcp/src/surfsense_mcp/config.py
6 surfsense_mcp/src/surfsense_mcp/core/auth/identity.py
7 surfsense_mcp/src/surfsense_mcp/core/auth/headers.py
8 surfsense_mcp/src/surfsense_mcp/core/auth/middleware.py
9 surfsense_mcp/src/surfsense_mcp/core/auth/__init__.py
10 surfsense_mcp/src/surfsense_mcp/core/client.py
11 surfsense_mcp/src/surfsense_mcp/core/workspace_context.py
12 surfsense_mcp/src/surfsense_mcp/core/workspace_matching.py
13 surfsense_mcp/src/surfsense_mcp/core/transport/http.py
14 surfsense_mcp/src/surfsense_mcp/core/transport/__init__.py
15 surfsense_mcp/src/surfsense_mcp/__main__.py
16 surfsense_mcp/src/surfsense_mcp/server.py
17 surfsense_mcp/tests/test_auth_headers.py
18 surfsense_mcp/tests/test_request_auth.py
19 surfsense_mcp/tests/test_workspace_context.py
20 surfsense_mcp/tests/test_client_errors.py
21 surfsense_mcp/tests/test_client_params.py
22 surfsense_mcp/src/surfsense_mcp/selfcheck.py
23 surfsense_mcp/src/surfsense_mcp/features/knowledge_base/__init__.py
24 surfsense_mcp/src/surfsense_mcp/features/knowledge_base/annotations.py
25 surfsense_mcp/src/surfsense_mcp/features/knowledge_base/search_tools.py
26 surfsense_mcp/src/surfsense_mcp/features/knowledge_base/document_tools.py
27 surfsense_mcp/src/surfsense_mcp/features/scrapers/__init__.py
28 surfsense_mcp/src/surfsense_mcp/features/scrapers/annotations.py
29 surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/__init__.py
30 surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/web.py
31 surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/google_search.py
32 surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/reddit.py
33 surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/youtube.py
34 surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/google_maps.py
35 surfsense_mcp/src/surfsense_mcp/features/scrapers/run_history.py
36 surfsense_mcp/README.md
37 README.md
38 README.es.md
39 README.hi.md
40 README.pt-BR.md
41 README.zh-CN.md
42 surfsense_web/lib/mcp/clients.ts
43 surfsense_web/components/mcp/agent-setup-tabs.tsx
44 surfsense_web/components/connectors-marketing/api-mcp-tabs.tsx
45 surfsense_web/app/(home)/mcp-server/page.tsx
46 surfsense_web/content/docs/how-to/mcp-server.mdx
47 surfsense_backend/app/agents/video_presentation/graph.py
48 surfsense_backend/tests/unit/agents/test_video_presentation_graph.py
⚠️ Inconsistent Changes Detected
File Path Warning
surfsense_backend/app/agents/video_presentation/graph.py Video presentation graph fan-in barrier fix is unrelated to the MCP remote server feature
surfsense_backend/tests/unit/agents/test_video_presentation_graph.py Test for video presentation graph barrier is unrelated to the MCP remote server feature

Need help? Join our Discord

Summary by CodeRabbit

  • New Features

    • Added hosted and self-host MCP connection support, including remote HTTP access with bearer authentication and stdio setup options.
    • Expanded MCP tooling for knowledge base search, document management, scraping, and run history.
    • Added a /health endpoint and support for running the server over HTTP.
  • Documentation

    • Updated setup guides and README examples across languages to use the new MCP endpoint path.
    • Clarified client configuration steps for hosted vs. self-host deployments.

@vercel

vercel Bot commented Jul 7, 2026

Copy link
Copy Markdown

@CREDO23 is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Refactors the SurfSense MCP server to support hosted (streamable-HTTP) and self-host (stdio) transports with per-request API-key identity, adds ASGI auth middleware, per-identity workspace isolation, splits knowledge-base/scraper tools into dedicated modules, adds a Dockerfile, and updates frontend/docs to reference the hosted /mcp endpoint.

Changes

MCP Server Multi-Transport and Auth Refactor

Layer / File(s) Summary
Transport config, entrypoint, and deployment
surfsense_mcp/src/surfsense_mcp/config.py, __main__.py, server.py, selfcheck.py, Dockerfile, .dockerignore, pyproject.toml
Adds optional API key, host/port settings, transport env var dispatch (stdio vs streamable-http), _run_http uvicorn runner, stateless_http/json_response server flags, a multi-stage Dockerfile, .dockerignore rules, and starlette/uvicorn dependencies.
Per-request identity and API key middleware
core/auth/*.py, core/transport/*.py, tests/test_auth_headers.py, tests/test_request_auth.py
Introduces extract_api_key, ContextVar-based identity bind/unbind helpers, ApiKeyIdentityMiddleware returning 401 on missing keys, a /health public route, CORS wrapping, and corresponding tests.
Client fallback API key resolution
core/client.py, tests/test_client_errors.py, tests/test_client_params.py
Changes SurfSenseClient to resolve the Authorization header per request from bound identity or an optional fallback_api_key, raising ToolError when absent.
Per-identity workspace context isolation
core/workspace_context.py, core/workspace_matching.py, tests/test_workspace_context.py
Tracks active workspace per caller identity in a bounded OrderedDict, extracts matching helpers into workspace_matching.py, and adds isolation tests.
Knowledge base tool modularization
features/knowledge_base/*.py
Splits knowledge-base tools into search_tools.py, document_tools.py, and shared annotations.py, replacing the monolithic __init__.py.
Scraper tool modularization
features/scrapers/*.py, platforms/*.py
Splits scraper tools into per-platform modules (web, google_search, google_maps, reddit, youtube) and run_history.py, wired via a registrar list.
MCP server README rewrite
surfsense_mcp/README.md
Rewrites setup docs to distinguish Hosted vs Self-host (stdio) connection paths.

Estimated code review effort: 4 (Complex) | ~75 minutes

Hosted MCP Endpoint Web and Docs Updates

Layer / File(s) Summary
Client snippet generator remote/stdio split
surfsense_web/lib/mcp/clients.ts
Splits McpClient into remote and stdio snippet builders with REMOTE_URL constant and Bearer helper.
Setup tabs transport toggle
surfsense_web/components/mcp/agent-setup-tabs.tsx
Adds a transport selector and renders transport-specific steps/config.
Marketing page and docs hosted endpoint updates
surfsense_web/app/(home)/mcp-server/page.tsx, components/connectors-marketing/api-mcp-tabs.tsx, content/docs/how-to/mcp-server.mdx
Updates marketing copy, Cursor config snippet, and MDX docs to describe hosted vs self-host setup with the /mcp URL.
README hosted MCP URL updates
README.md, README.es.md, README.hi.md, README.pt-BR.md, README.zh-CN.md
Updates example MCP server URL to include the /mcp path across all localized READMEs.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ApiKeyIdentityMiddleware
  participant identity as identity module
  participant McpApp
  Client->>ApiKeyIdentityMiddleware: HTTP request with Bearer/X-API-Key header
  ApiKeyIdentityMiddleware->>ApiKeyIdentityMiddleware: extract_api_key(headers)
  alt no key
    ApiKeyIdentityMiddleware-->>Client: 401 unauthorized
  else key present
    ApiKeyIdentityMiddleware->>identity: bind_api_key(key)
    ApiKeyIdentityMiddleware->>McpApp: forward request
    McpApp-->>ApiKeyIdentityMiddleware: response
    ApiKeyIdentityMiddleware->>identity: unbind_api_key(token)
    ApiKeyIdentityMiddleware-->>Client: response
  end
Loading

Suggested reviewers: AnishSarkar22, MODSetter

🐰✨
Transports two, both hosted and stdio,
Keys now bound per caller, safe and dry,
Tools split apart, each module its own home,
Docs point to /mcp, no longer to roam,
A carrot for the build that passed review clean!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a hosted remote MCP server over streamable-http.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@CREDO23 CREDO23 force-pushed the feature-hosted-mcp-server branch from 1b455d1 to d09ecd1 Compare July 7, 2026 16:27
@CREDO23 CREDO23 self-assigned this Jul 7, 2026
@CREDO23 CREDO23 merged commit 97f3631 into MODSetter:main Jul 7, 2026
2 of 8 checks passed

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
surfsense_web/app/(home)/mcp-server/page.tsx (1)

53-63: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Hardcoded hero config can drift from lib/mcp/clients.ts.

CURSOR_CONFIG duplicates the Cursor remote snippet manually. The comment even calls out that it "mirrors" clients.ts, but nothing enforces that. clients.ts's header comment states the catalog exists precisely so the marketing page and playground instructions "can never drift apart" — this component bypasses that mechanism.

Generate it from the shared catalog instead:

♻️ Proposed fix
-/* The hosted Cursor config; mirrors lib/mcp/clients.ts. */
-const CURSOR_CONFIG = `{
-  "mcpServers": {
-    "surfsense": {
-      "url": "https://mcp.surfsense.com/mcp",
-      "headers": {
-        "Authorization": "Bearer ss_pat_..."
-      }
-    }
-  }
-}`;
+const CURSOR_CONFIG = MCP_CLIENTS.find((c) => c.id === "cursor")!.remote.build({
+	remoteUrl: REMOTE_URL,
+	apiKey: "ss_pat_...",
+	baseUrl: "https://api.surfsense.com",
+	serverDir: DEFAULT_SERVER_DIR,
+});

Requires importing MCP_CLIENTS, REMOTE_URL, and DEFAULT_SERVER_DIR from @/lib/mcp/clients.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@surfsense_web/app/`(home)/mcp-server/page.tsx around lines 53 - 63, The
hardcoded CURSOR_CONFIG in the mcp-server page duplicates the remote Cursor
snippet and can drift from the shared MCP catalog. Replace the manual string
with a value generated from MCP_CLIENTS using REMOTE_URL and DEFAULT_SERVER_DIR
imported from `@/lib/mcp/clients`, so the marketing page stays in sync with the
canonical config source. Update the relevant page component to derive the
displayed config from the shared catalog instead of maintaining a separate
literal.
surfsense_mcp/Dockerfile (1)

19-33: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Container runs as root — add a non-root USER.

Static analysis flags this correctly: no USER is set, so the image defaults to root. For a publicly hosted, internet-facing service, this is unnecessary privilege exposure.

🔒 Proposed fix to run as a non-root user
 FROM deps AS production

 WORKDIR /app

 COPY . .
 RUN uv pip install --system --no-cache-dir --no-deps -e .

 ENV PYTHONUNBUFFERED=1 \
     SURFSENSE_MCP_TRANSPORT=streamable-http \
     SURFSENSE_MCP_HOST=0.0.0.0 \
     SURFSENSE_MCP_PORT=8080 \
     SURFSENSE_BASE_URL=https://api.surfsense.com

+RUN useradd --create-home --uid 1000 --shell /usr/sbin/nologin appuser \
+    && chown -R appuser:appuser /app
+USER appuser
+
 EXPOSE 8080

 CMD ["python", "-m", "surfsense_mcp"]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@surfsense_mcp/Dockerfile` around lines 19 - 33, The production stage in the
Dockerfile currently runs as root because no USER is set. Update the final image
setup after the environment variables and before CMD to create or switch to a
non-root user for the runtime container, ensuring the app launched by `python -m
surfsense_mcp` runs under that user instead of root.

Source: Linters/SAST tools

🧹 Nitpick comments (2)
surfsense_web/components/connectors-marketing/api-mcp-tabs.tsx (1)

205-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Import REMOTE_URL instead of re-hardcoding the hosted URL.

Same drift risk as the marketing page's CURSOR_CONFIG: this literal duplicates REMOTE_URL from lib/mcp/clients.ts rather than importing it.

♻️ Proposed fix
+import { REMOTE_URL } from "`@/lib/mcp/clients`";
+
 function buildMcp({ mcpTool }: ApiSample): string {
 	const config = {
 		mcpServers: {
 			surfsense: {
-				url: "https://mcp.surfsense.com/mcp",
+				url: REMOTE_URL,
 				headers: { Authorization: "Bearer ${SURFSENSE_API_KEY}" },
 			},
 		},
 	};
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@surfsense_web/components/connectors-marketing/api-mcp-tabs.tsx` around lines
205 - 215, The buildMcp helper is hardcoding the hosted MCP endpoint instead of
reusing the shared source of truth. Update buildMcp in api-mcp-tabs to import
and use REMOTE_URL from lib/mcp/clients.ts when building the surfsense server
config, so the generated MCP sample stays aligned with the client config. Keep
the rest of the JSON shape and mcpTool message unchanged.
surfsense_mcp/Dockerfile (1)

10-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Pin the uv installer version.

uv is installed unpinned; an upstream release could change CLI behavior and silently break the frozen-lockfile export step.

♻️ Proposed pin
-RUN pip install --no-cache-dir uv && \
+RUN pip install --no-cache-dir "uv==0.5.*" && \
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@surfsense_mcp/Dockerfile` around lines 10 - 15, The Dockerfile installs uv
without a fixed version, so the export/install step can change behavior
unexpectedly; update the RUN step to install a pinned uv version before calling
uv export and uv pip install. Keep the existing frozen-lockfile flow intact, and
make sure the pinned installer is easy to locate in the Dockerfile alongside the
pyproject.toml and uv.lock copy step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@surfsense_mcp/src/surfsense_mcp/core/transport/http.py`:
- Around line 30-36: The CORS setup in the HTTP transport is currently too
permissive because `allow_origins=["*"]` lets any browser origin reach the
authenticated `/mcp` endpoint. Update the `CORSMiddleware` configuration in the
transport helper to use an explicit allowlist or a configurable set of trusted
origins instead of a wildcard, and keep the change localized to the middleware
creation logic so the `http.py` transport remains the source of truth for origin
policy.

In `@surfsense_mcp/src/surfsense_mcp/features/knowledge_base/document_tools.py`:
- Around line 151-161: The document update flow in document_tools.update
document handling currently indexes existing["document_type"] and
existing["workspace_id"] directly, which can raise a raw KeyError if the GET
/documents/{document_id} response is missing either field. Update this logic to
validate the required fields before building the PUT payload, and raise a
descriptive ToolError on missing or malformed response data; keep the fix
localized around the existing client.request calls and the return message for
the updated document.
- Around line 82-125: The upload_file tool currently reads a server-local path
via _read_upload and is registered by build_server() for both stdio and
streamable-http, which makes it unsafe in hosted HTTP mode. Update the
registration or tool logic so surfsense_upload_file is disabled for
streamable-http, or change upload_file to accept uploaded bytes/data instead of
a filesystem path; keep the stdio path-based behavior only where local
filesystem access is intended.

---

Outside diff comments:
In `@surfsense_mcp/Dockerfile`:
- Around line 19-33: The production stage in the Dockerfile currently runs as
root because no USER is set. Update the final image setup after the environment
variables and before CMD to create or switch to a non-root user for the runtime
container, ensuring the app launched by `python -m surfsense_mcp` runs under
that user instead of root.

In `@surfsense_web/app/`(home)/mcp-server/page.tsx:
- Around line 53-63: The hardcoded CURSOR_CONFIG in the mcp-server page
duplicates the remote Cursor snippet and can drift from the shared MCP catalog.
Replace the manual string with a value generated from MCP_CLIENTS using
REMOTE_URL and DEFAULT_SERVER_DIR imported from `@/lib/mcp/clients`, so the
marketing page stays in sync with the canonical config source. Update the
relevant page component to derive the displayed config from the shared catalog
instead of maintaining a separate literal.

---

Nitpick comments:
In `@surfsense_mcp/Dockerfile`:
- Around line 10-15: The Dockerfile installs uv without a fixed version, so the
export/install step can change behavior unexpectedly; update the RUN step to
install a pinned uv version before calling uv export and uv pip install. Keep
the existing frozen-lockfile flow intact, and make sure the pinned installer is
easy to locate in the Dockerfile alongside the pyproject.toml and uv.lock copy
step.

In `@surfsense_web/components/connectors-marketing/api-mcp-tabs.tsx`:
- Around line 205-215: The buildMcp helper is hardcoding the hosted MCP endpoint
instead of reusing the shared source of truth. Update buildMcp in api-mcp-tabs
to import and use REMOTE_URL from lib/mcp/clients.ts when building the surfsense
server config, so the generated MCP sample stays aligned with the client config.
Keep the rest of the JSON shape and mcpTool message unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 67c458eb-0855-4216-8239-92e4ac9bd1c9

📥 Commits

Reviewing files that changed from the base of the PR and between fa7075f and d09ecd1.

⛔ Files ignored due to path filters (1)
  • surfsense_mcp/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (45)
  • README.es.md
  • README.hi.md
  • README.md
  • README.pt-BR.md
  • README.zh-CN.md
  • surfsense_mcp/.dockerignore
  • surfsense_mcp/Dockerfile
  • surfsense_mcp/README.md
  • surfsense_mcp/pyproject.toml
  • surfsense_mcp/src/surfsense_mcp/__main__.py
  • surfsense_mcp/src/surfsense_mcp/config.py
  • surfsense_mcp/src/surfsense_mcp/core/auth/__init__.py
  • surfsense_mcp/src/surfsense_mcp/core/auth/headers.py
  • surfsense_mcp/src/surfsense_mcp/core/auth/identity.py
  • surfsense_mcp/src/surfsense_mcp/core/auth/middleware.py
  • surfsense_mcp/src/surfsense_mcp/core/client.py
  • surfsense_mcp/src/surfsense_mcp/core/transport/__init__.py
  • surfsense_mcp/src/surfsense_mcp/core/transport/http.py
  • surfsense_mcp/src/surfsense_mcp/core/workspace_context.py
  • surfsense_mcp/src/surfsense_mcp/core/workspace_matching.py
  • surfsense_mcp/src/surfsense_mcp/features/knowledge_base/__init__.py
  • surfsense_mcp/src/surfsense_mcp/features/knowledge_base/annotations.py
  • surfsense_mcp/src/surfsense_mcp/features/knowledge_base/document_tools.py
  • surfsense_mcp/src/surfsense_mcp/features/knowledge_base/search_tools.py
  • surfsense_mcp/src/surfsense_mcp/features/scrapers/__init__.py
  • surfsense_mcp/src/surfsense_mcp/features/scrapers/annotations.py
  • surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/__init__.py
  • surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/google_maps.py
  • surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/google_search.py
  • surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/reddit.py
  • surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/web.py
  • surfsense_mcp/src/surfsense_mcp/features/scrapers/platforms/youtube.py
  • surfsense_mcp/src/surfsense_mcp/features/scrapers/run_history.py
  • surfsense_mcp/src/surfsense_mcp/selfcheck.py
  • surfsense_mcp/src/surfsense_mcp/server.py
  • surfsense_mcp/tests/test_auth_headers.py
  • surfsense_mcp/tests/test_client_errors.py
  • surfsense_mcp/tests/test_client_params.py
  • surfsense_mcp/tests/test_request_auth.py
  • surfsense_mcp/tests/test_workspace_context.py
  • surfsense_web/app/(home)/mcp-server/page.tsx
  • surfsense_web/components/connectors-marketing/api-mcp-tabs.tsx
  • surfsense_web/components/mcp/agent-setup-tabs.tsx
  • surfsense_web/content/docs/how-to/mcp-server.mdx
  • surfsense_web/lib/mcp/clients.ts

Comment on lines +30 to +36
return CORSMiddleware(
app,
allow_origins=["*"],
allow_methods=["GET", "POST", "DELETE", "OPTIONS"],
allow_headers=["*"],
expose_headers=["Mcp-Session-Id"],
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map relevant files first
git ls-files surfsense_mcp/src/surfsense_mcp/core/transport/http.py \
            surfsense_mcp/src/surfsense_mcp/core/auth/middleware.py \
            surfsense_mcp/src/surfsense_mcp/core/auth \
            surfsense_mcp/src/surfsense_mcp/core/transport \
            | sed 's#^`#FILE` #'

echo
echo "=== ast-grep outline: http.py ==="
ast-grep outline surfsense_mcp/src/surfsense_mcp/core/transport/http.py --view expanded || true

echo
echo "=== ast-grep outline: middleware.py ==="
ast-grep outline surfsense_mcp/src/surfsense_mcp/core/auth/middleware.py --view expanded || true

echo
echo "=== Relevant excerpts ==="
sed -n '1,220p' surfsense_mcp/src/surfsense_mcp/core/transport/http.py
echo
sed -n '1,260p' surfsense_mcp/src/surfsense_mcp/core/auth/middleware.py

Repository: MODSetter/SurfSense

Length of output: 4262


🌐 Web query:

FastMCP streamable HTTP deployment guide CORS allow_origins "*" production lifespan middleware ASGI

💡 Result:

FastMCP supports Streamable HTTP as the recommended transport for production-ready remote MCP services [1][2]. When deploying, you should utilize the ASGI application approach for better control and scalability [1][3]. Production Deployment and ASGI To deploy as an ASGI application, use the.http_app method, which returns a Starlette-compatible ASGI app [1][3]. When mounting this to a parent framework like FastAPI, you must pass the lifespan event handler to ensure the session manager initializes correctly [4][5]: app = FastAPI(lifespan=mcp_app.lifespan) app.mount("/mcp", mcp_app) In production, use an ASGI server like Uvicorn paired with a process manager like Gunicorn (using Uvicorn workers) to handle multiple requests effectively [3][6]. For reverse proxies (e.g., Nginx), ensure proxy_buffering is disabled and proxy_read_timeout is increased (e.g., 300s) to prevent the SSE stream from being terminated or buffered [1][2]. CORS and Security CORS is opt-in and is generally only required for browser-based MCP clients (like the MCP Inspector) [7][8]. - Do not use allow_origins=["*"] in production [8]. Using a wildcard exposes your server to unauthorized cross-origin access and is a significant security risk [8][9]. - If CORS is necessary, explicitly define allowed origins [7][8]: middleware = [ Middleware( CORSMiddleware, allow_origins=["https://your-allowed-domain.com"], allow_methods=["GET", "POST", "DELETE", "OPTIONS"], allow_headers=["mcp-protocol-version", "mcp-session-id", "Authorization"], expose_headers=["mcp-session-id"],) ] app = mcp.http_app(middleware=middleware) The expose_headers=["mcp-session-id"] is critical for browser-based session management [7][8]. If you are mounting multiple apps (e.g., a FastAPI app alongside FastMCP), avoid application-wide CORS middleware; instead, apply specific middleware to the individual sub-apps to prevent routing conflicts [5].

Citations:


Restrict CORS origins here. allow_origins=["*"] on the authenticated /mcp transport lets any browser origin access it; make the allowed origins explicit or configurable instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@surfsense_mcp/src/surfsense_mcp/core/transport/http.py` around lines 30 - 36,
The CORS setup in the HTTP transport is currently too permissive because
`allow_origins=["*"]` lets any browser origin reach the authenticated `/mcp`
endpoint. Update the `CORSMiddleware` configuration in the transport helper to
use an explicit allowlist or a configurable set of trusted origins instead of a
wildcard, and keep the change localized to the middleware creation logic so the
`http.py` transport remains the source of truth for origin policy.

Comment on lines +82 to +125
async def upload_file(
file_path: Annotated[
str,
Field(
description="Path to a local file, e.g. "
"'C:/Users/me/report.pdf' or '~/notes/summary.md'."
),
],
use_vision_llm: Annotated[
bool,
Field(
description="True reads scanned or image-heavy files with a "
"vision model (slower)."
),
] = False,
workspace: WorkspaceParam = None,
) -> str:
"""Upload a local file (PDF, docx, markdown, etc.) into the knowledge base.

Use this to ingest a file from disk so its content becomes searchable;
for text you already have in hand use surfsense_add_document instead.
The file is parsed, chunked, and indexed asynchronously. Duplicate
files are detected and skipped.
Example: file_path='C:/Users/me/report.pdf'.
"""
resolved = await context.resolve(workspace)
payload = _read_upload(file_path)
result = await client.request(
"POST",
"/documents/fileupload",
data={
"workspace_id": str(resolved.id),
"use_vision_llm": str(use_vision_llm).lower(),
"processing_mode": "basic",
},
files=[("files", payload)],
)
pending = (result or {}).get("pending_files", 0)
skipped = (result or {}).get("skipped_duplicates", 0)
note = " (already present, skipped)" if skipped and not pending else ""
return (
f"Uploaded '{Path(file_path).name}' to '{resolved.name}'{note}. "
"It will be searchable once processing completes."
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Files ==\n'
git ls-files | rg '(^|/)(document_tools|server)\.py$|settings\.py$|transport|knowledge_base'

printf '\n== Outline: document_tools.py ==\n'
ast-grep outline surfsense_mcp/src/surfsense_mcp/features/knowledge_base/document_tools.py --view expanded || true

printf '\n== Relevant lines: document_tools.py ==\n'
cat -n surfsense_mcp/src/surfsense_mcp/features/knowledge_base/document_tools.py | sed -n '1,260p'

printf '\n== Outline: server.py ==\n'
ast-grep outline surfsense_mcp/src/surfsense_mcp/server.py --view expanded || true

printf '\n== Relevant lines: server.py ==\n'
cat -n surfsense_mcp/src/surfsense_mcp/server.py | sed -n '1,260p'

printf '\n== Search for transport handling and tool registration ==\n'
rg -n "streamable|stdio|register\\(|knowledge_base|allow_local|workspace" surfsense_mcp/src/surfsense_mcp -g '!**/__pycache__/**'

Repository: MODSetter/SurfSense

Length of output: 38091


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== __main__.py ==\n'
cat -n surfsense_mcp/src/surfsense_mcp/__main__.py | sed -n '1,220p'

printf '\n== core/transport/http.py ==\n'
cat -n surfsense_mcp/src/surfsense_mcp/core/transport/http.py | sed -n '1,220p'

printf '\n== core/auth/identity.py ==\n'
cat -n surfsense_mcp/src/surfsense_mcp/core/auth/identity.py | sed -n '1,220p'

printf '\n== config.py ==\n'
cat -n surfsense_mcp/src/surfsense_mcp/config.py | sed -n '1,220p'

Repository: MODSetter/SurfSense

Length of output: 7855


Arbitrary file read in hosted transport surfsense_upload_file reads a server-local path directly, and build_server() registers it for both stdio and streamable-http. In hosted mode, that makes this an authenticated LFI against the MCP host’s filesystem. Gate it off for HTTP or switch the API to accept file bytes instead of a server path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@surfsense_mcp/src/surfsense_mcp/features/knowledge_base/document_tools.py`
around lines 82 - 125, The upload_file tool currently reads a server-local path
via _read_upload and is registered by build_server() for both stdio and
streamable-http, which makes it unsafe in hosted HTTP mode. Update the
registration or tool logic so surfsense_upload_file is disabled for
streamable-http, or change upload_file to accept uploaded bytes/data instead of
a filesystem path; keep the stdio path-based behavior only where local
filesystem access is intended.

Comment on lines +151 to +161
existing = await client.request("GET", f"/documents/{document_id}")
await client.request(
"PUT",
f"/documents/{document_id}",
json={
"document_type": existing["document_type"],
"workspace_id": existing["workspace_id"],
"content": content,
},
)
return f"Updated document {document_id} ('{existing.get('title', '')}')."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Unhandled KeyError risk on missing fields.

existing["document_type"]/existing["workspace_id"] will raise a raw KeyError if the API response omits either field, unlike the rest of this module which raises descriptive ToolErrors for failure paths.

🛡️ Proposed fix
         existing = await client.request("GET", f"/documents/{document_id}")
+        try:
+            document_type = existing["document_type"]
+            workspace_id = existing["workspace_id"]
+        except KeyError as exc:
+            raise ToolError(f"Document {document_id} is missing '{exc.args[0]}'.") from exc
         await client.request(
             "PUT",
             f"/documents/{document_id}",
             json={
-                "document_type": existing["document_type"],
-                "workspace_id": existing["workspace_id"],
+                "document_type": document_type,
+                "workspace_id": workspace_id,
                 "content": content,
             },
         )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
existing = await client.request("GET", f"/documents/{document_id}")
await client.request(
"PUT",
f"/documents/{document_id}",
json={
"document_type": existing["document_type"],
"workspace_id": existing["workspace_id"],
"content": content,
},
)
return f"Updated document {document_id} ('{existing.get('title', '')}')."
existing = await client.request("GET", f"/documents/{document_id}")
try:
document_type = existing["document_type"]
workspace_id = existing["workspace_id"]
except KeyError as exc:
raise ToolError(f"Document {document_id} is missing '{exc.args[0]}'.") from exc
await client.request(
"PUT",
f"/documents/{document_id}",
json={
"document_type": document_type,
"workspace_id": workspace_id,
"content": content,
},
)
return f"Updated document {document_id} ('{existing.get('title', '')}')."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@surfsense_mcp/src/surfsense_mcp/features/knowledge_base/document_tools.py`
around lines 151 - 161, The document update flow in document_tools.update
document handling currently indexes existing["document_type"] and
existing["workspace_id"] directly, which can raise a raw KeyError if the GET
/documents/{document_id} response is missing either field. Update this logic to
validate the required fields before building the PUT payload, and raise a
descriptive ToolError on missing or malformed response data; keep the fix
localized around the existing client.request calls and the return message for
the updated document.

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.

1 participant