Enable MCP for all API endpoints - #160
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
Add mcp=True to 38 remaining endpoints across 10 modules. Only WebSocket endpoints (agent dock, event/activity tail, event ingest) are excluded as they use a different protocol incompatible with MCP. Newly enabled endpoints by module: - agents: create_agent, delete_agent, get_agent, get_scan_status - activity: list_activities, query_activities, count_activities - assets: query_assets, count_assets - emails: get_emails - events: insert_event, get_event, list_events, query_events, count_events, archive_old_events - findings: query_findings, count_findings, set_risk - presets: create_preset, update_preset, delete_preset - scans: get_scans, query_scans, count_scans, cancel_scan - targets: count_targets, set_default_target, create_target, update_target, copy_target, delete_target, is_blacklisted, query_targets, list_ids - technologies: list_technologies, query_technologies, count_technologies Total MCP coverage: 69/73 (94.5%)
|
@R1ckyH thanks for the PR! Have you got a chance to test these out? Do they behave well? |
I am doing some tests on it, currently not yet finished all, but I think MCP is just another way to call the http api. |
fastapi-mcp 0.3.3 only forwarded the Authorization header, causing 401 errors when using custom auth headers like X-API-Key. Upgrade to 0.4.0 which adds a headers allowlist parameter, and pass ["x-api-key"] to forward the API key from MCP client requests to internal API calls. Co-Authored-By: R1ckyH <R1ckyH@users.noreply.github.com>
|
@TheTechromancer, I have added a new commit. This should fix the MCP API key issue mentioned in the README. |
|
I have also added some bug fixes here |
…rror HTTPStreamRoute's wrapper used @functools.wraps(), which copies attributes like __wrapped__ and __annotations__ from the original async generator function onto the wrapper. This caused FastAPI to detect the wrapper as an async generator and attempt to serialize its output via orjson.dumps(), resulting in: TypeError: Type is not JSON serializable: async_generator The wrapper is NOT an async generator — it's a regular async def that returns a StreamingResponse. Replace @functools.wraps with manual __name__, __qualname__, and __signature__ copying to give FastAPI the metadata it needs for routing without the async generator markers. This fixes streaming endpoints (list_assets, list_scans, etc.) for both direct HTTP access and MCP tool calls via fastapi_mcp.
ca927c1 — Fix streaming endpoints crashing with
|
…pe checks - Add None guards to all query_* endpoints (query_scans, query_assets, query_events, query_findings, query_targets, query_technologies) matching the pattern already applied to count_* endpoints - Fix get_emails: AssetFields now inherits CustomAssetFields so the emails field is properly registered on the Asset model, plus use getattr() fallback for safe attribute access - Fix count_activities/query_activities: make query parameter optional with ActivityQuery | None = None and add None guard - Fix in_scope/is_blacklisted/is_in_target: return False instead of crashing when no default target exists in the database All 45 testable MCP endpoints now pass (0 failures).
|
Hey @TheTechromancer, after testing the MCP endpoints against a live server with a full scan workflow (create target → start scan → agent runs → query results), I found a few issues. Wanted to flag before this goes further: 1.
|
|
One more thing to talk about here, when I am trying to do the MCP testing, I face many issues, such as the function not yet implemented. I want to know your attitude before I continue. |
There was a problem hiding this comment.
@R1ckyH, answering 05-08: fix both here. get_scan_status and the seeds scope bug break the MCP surface this PR ships. Same for unimplemented endpoints, implement or drop mcp=True.
Pulled the branch, ran the suite on real mongo/redis (122 passed, 6 skipped), diffed OpenAPI against dev. Blockers inline, plus two with no line to hang on:
- No CI has ever run here. All 8 pushes show CLAAssistant only.
- PR body says 38 endpoints, branch has 69.
🟢 ca927c1: traceback, mechanism, why the narrower copy is right. I reproduced it from that comment alone. That is the standard for bug fixes here.
🟢 You ran a live workflow and reported what broke instead of pushing the green button. Three findings here are yours.
Approach is fine. Fix and I will re-review.
| # that cause FastAPI to mishandle the StreamingResponse | ||
| wrapper.__name__ = self.orig_function.__name__ | ||
| wrapper.__qualname__ = self.orig_function.__qualname__ | ||
| wrapper.__signature__ = inspect.signature(self.orig_function) |
There was a problem hiding this comment.
🔴 Blocking. Your diagnosis is right and I reproduced it: under fastapi 0.136.1 a @functools.wraps-decorated stream wrapper raises TypeError: 'async for' requires an object with __aiter__ method; without it, 200. Under 0.128.7 (what dev pins) both work, so this breakage arrives with the fastapi bump in this same lockfile.
But this copies __name__, __qualname__, and __signature__ and not __doc__. OpenAPI descriptions drop 21 -> 13, and 8 of the losses are MCP tools this PR adds: list_assets, query_assets, query_events, query_findings, query_scans, query_targets, query_technologies, query_activities. Those descriptions are the tool prompts the model reads, so this quietly degrades the feature the PR exists to deliver.
One line fixes it, verified still 200:
wrapper.__doc__ = self.orig_function.__doc__| mcp_endpoints = MCP_ENDPOINTS | ||
| log.debug(f"Creating MCP server with endpoints: {','.join(mcp_endpoints)}") | ||
| mcp = FastApiMCP(fastapi_app, include_operations=list(mcp_endpoints)) | ||
| mcp = FastApiMCP(fastapi_app, include_operations=list(mcp_endpoints), headers=["x-api-key"]) |
There was a problem hiding this comment.
🔴 Blocking. config.py:77 already owns this as auth_header. Read it from config instead of pasting the literal.
Also note this replaces the library default of ["authorization"] rather than extending it. Confirm nothing depends on Authorization passthrough.
| return agent_status | ||
|
|
||
| @api_endpoint("/scan_status", methods=["GET"], summary="Get the status of an agent's scan") | ||
| @api_endpoint("/scan_status", methods=["GET"], summary="Get the status of an agent's scan", mcp=True) |
There was a problem hiding this comment.
🔴 Blocking. VALID_AGENT_COMMANDS is {cancel_scan, finish_scan, get_agent_status, get_file, kill_module, start_scan}. No get_scan_status, so line 98 always returns Invalid command, exactly as you found. Implement the agent-side command or drop mcp=True. Shipping it as a tool in this state is not an option.
While you are here, switch id from UUID to str per your own finding about clients rejecting format: uuid. get_scan already uses str.
| @api_endpoint("/in_scope", methods=["GET"], summary="Check if a host or URL is in scope", mcp=True) | ||
| async def in_scope(self, host: str, target_id: UUID = None) -> bool: | ||
| bbot_target = await self._get_bbot_target(target_id) | ||
| if bbot_target is None: |
There was a problem hiding this comment.
🔴 Blocking, two things on this block and the two below it.
Fails open. Answering "is this host blacklisted?" with False because the target was not found is the wrong direction for a safety check. Neither has any other caller in the tree, so raising BBOTServerNotFoundError costs nothing.
Seeds-only targets. Your 05-08 finding belongs in this PR, since these three are mcp=True here and this is what exposes the bug to agents. BBOTTarget.in_scope() consults only the target list, so target=[] + seeds=["evilcorp.com"] reports its own seed out of scope. Fix at targets_models.py:110-111 and add a test pinning both shapes.
| from bbot_server.applets.base import BaseApplet, api_endpoint, Field | ||
|
|
||
|
|
||
| class EmailsFields(CustomAssetFields): |
There was a problem hiding this comment.
🔴 Blocking. This is a correct fix. The nested class AssetFields(BaseModel) was dead code, since the AST preloader in modules/__init__.py only collects module-level CustomAssetFields subclasses. Verified the effect: emails is absent from Asset.model_fields on dev, present here.
That is a change to the stored asset schema. It is not mentioned in the PR body and no test covers it. Call it out and add one.
| model = Asset | ||
|
|
||
| @api_endpoint("/list", methods=["GET"], type="http_stream", response_model=Asset, summary="Stream all assets") | ||
| @api_endpoint("/list", methods=["GET"], type="http_stream", response_model=Asset, summary="Stream all assets", mcp=True) |
There was a problem hiding this comment.
🔴 Blocking. uv run ruff format --diff exits 1 on this branch, 0 on dev. Adding mcp=True pushes this and 4 other files' decorators past the 119 char limit (activity, events, findings, technologies). CI runs this in the Lint step. uv run ruff format fixes it.
| "fastapi-mcp>=0.4.0", | ||
| "click==8.1.8", | ||
| "mcp==1.7.0", | ||
| "mcp==1.27.0", |
There was a problem hiding this comment.
🔴 Blocking. uv lock --check fails on this branch and passes on dev. The merge resolved uv.lock backward:
bbot3.0.0 (rev=dev) -> 0.0.0 (rev=3.0, d8f056e), while this file still declaresrev = "dev"blastdnsdropped entirelydeepdiff9.0.0 -> 8.6.2bbot-serverself-version 0.3.1 -> 0.2.0, contradicting line 3 of this file
Re-merge dev and regenerate with uv lock instead of hand-resolving the conflict.
|
@R1ckyH I don't have access to push to your PR. Can you resolve the conflicts and address the above feedback? |
Summary
mcp=Trueto 38 remaining API endpoints across 10 modulesNew MCP endpoints by module
create_agent,delete_agent,get_agent,get_scan_statuslist_activities,query_activities,count_activitiesquery_assets,count_assetsget_emailsinsert_event,get_event,list_events,query_events,count_events,archive_old_eventsquery_findings,count_findings,set_riskcreate_preset,update_preset,delete_presetget_scans,query_scans,count_scans,cancel_scancount_targets,set_default_target,create_target,update_target,copy_target,delete_target,is_blacklisted,query_targets,list_idslist_technologies,query_technologies,count_technologies