feat(mcp): add OAuth-protected server - #297
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 47 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/api/src/mcp/server.ts">
<violation number="1" location="packages/api/src/mcp/server.ts:256">
P2: `execute_code` accepts unbounded code on a route with no body limiter, so an authenticated caller can submit oversized payloads before Zod validation and consume API memory or kernel capacity. Add an MCP request/body limit and a maximum code size.</violation>
</file>
<file name="packages/api/src/mcp/createMcpApp.test.ts">
<violation number="1" location="packages/api/src/mcp/createMcpApp.test.ts:341">
P3: The rate-limit test asserts on the module-level `authorizationBudget` (shared with every other test in this file via `createMcpApp`), so its 429 count and record bound depend on how many authorize calls the other tests in the file already made. It passes today only because the other tests consume few slots. Add `beforeEach`/`afterEach` isolation (e.g. re-import fresh budgets or use an injectable budget) so this test does not silently break when a future test also hits `/authorize`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| project: z.string(), | ||
| session_id: z.string().optional(), | ||
| notebook: z.string().optional(), | ||
| code: z.string(), |
There was a problem hiding this comment.
P2: execute_code accepts unbounded code on a route with no body limiter, so an authenticated caller can submit oversized payloads before Zod validation and consume API memory or kernel capacity. Add an MCP request/body limit and a maximum code size.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/api/src/mcp/server.ts, line 256:
<comment>`execute_code` accepts unbounded code on a route with no body limiter, so an authenticated caller can submit oversized payloads before Zod validation and consume API memory or kernel capacity. Add an MCP request/body limit and a maximum code size.</comment>
<file context>
@@ -0,0 +1,364 @@
+ project: z.string(),
+ session_id: z.string().optional(),
+ notebook: z.string().optional(),
+ code: z.string(),
+ timeout_seconds: z.number().int().min(1).max(300).default(60),
+ kernel_session_id: z.string().optional(),
</file context>
| }).toString(); | ||
|
|
||
| const statuses: number[] = []; | ||
| for (let index = 0; index < 105; index += 1) { |
There was a problem hiding this comment.
P3: The rate-limit test asserts on the module-level authorizationBudget (shared with every other test in this file via createMcpApp), so its 429 count and record bound depend on how many authorize calls the other tests in the file already made. It passes today only because the other tests consume few slots. Add beforeEach/afterEach isolation (e.g. re-import fresh budgets or use an injectable budget) so this test does not silently break when a future test also hits /authorize.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/api/src/mcp/createMcpApp.test.ts, line 341:
<comment>The rate-limit test asserts on the module-level `authorizationBudget` (shared with every other test in this file via `createMcpApp`), so its 429 count and record bound depend on how many authorize calls the other tests in the file already made. It passes today only because the other tests consume few slots. Add `beforeEach`/`afterEach` isolation (e.g. re-import fresh budgets or use an injectable budget) so this test does not silently break when a future test also hits `/authorize`.</comment>
<file context>
@@ -0,0 +1,354 @@
+ }).toString();
+
+ const statuses: number[] = [];
+ for (let index = 0; index < 105; index += 1) {
+ statuses.push((await app.request(authorizeUrl, { redirect: 'manual' })).status);
+ }
</file context>
|
@cubic-dev-ai review |
@mscolnick I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 54 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
aa22235 to
681db36
Compare
1 participant
Adds an OAuth 2.1-protected MCP endpoint for discovering, creating, launching, and executing code in marimohub notebooks.
Includes dynamic public-client registration with PKCE, browser consent and project-scoped PAT grants, exact MCP resource binding, the
mcp:toolsscope, short-lived tokens, client-bound revocation, deployment-wide OAuth rate limits, structured registration auditing, path-prefixed discovery metadata, configuration, and documentation.