-
Notifications
You must be signed in to change notification settings - Fork 288
CASB MCP Server Support #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
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 adds support for the CASB MCP Server by introducing new schemas for integrations and assets along with corresponding API handlers and tool definitions. Key changes include:
- Definition of new Zod schemas for integrations, assets, and asset categories.
- Implementation of API calls and handlers to support various CASB operations.
- Registration of integration tool definitions and updated configuration for testing and deployment.
Reviewed Changes
Copilot reviewed 11 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mcp-common/src/schemas/cf1-integrations.ts | Added Zod schemas for integrations, assets, and asset categories. |
| packages/mcp-common/src/api/cf1-integration.ts | Implemented API call functions and handlers for integrations and assets. |
| apps/cloudflare-one-casb/vitest.config.ts | Configured Vitest for Workers. |
| apps/cloudflare-one-casb/src/tools/integrations.ts | Defined and registered tool handlers for integration-related operations. |
| apps/cloudflare-one-casb/src/index.ts | Updated entry point with OAuth provider and tool registration. |
| apps/cloudflare-one-casb/README.md | Documented MCP server setup and deployment instructions. |
| apps/cloudflare-one-casb/.eslintrc.cjs | Added ESLint configuration for project consistency. |
Files not reviewed (4)
- apps/cloudflare-one-casb/.dev.vars.example: Language not supported
- apps/cloudflare-one-casb/package.json: Language not supported
- apps/cloudflare-one-casb/tsconfig.json: Language not supported
- apps/cloudflare-one-casb/wrangler.jsonc: Language not supported
| export type State = { activeAccountId: string | null } | ||
| export class MyMCP extends McpAgent<Env, State, Props> { | ||
| // @ts-ignore | ||
| server = new McpServer({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmsparks recently implemented a nifty CloudflareMCPServer which gives us automatic metrics for all tool calls, and changed a bunch of things in our server's index.ts #69
You'll likely want to rebase ontop of latest staging and pull in the changes to index.ts that @cmsparks made to existing apps like workers-observability for ex! , you'll also want to add the analytics binding to your wrangler.jsonc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I’ll write a quick guide on this tomorrow!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok i think i have this hooked up (mostly)
| } | ||
|
|
||
| // Helper function to handle common error cases and account ID checks | ||
| const withAccountCheck = <T extends Record<string, any>>(agent: MyMCP, handler: ToolHandler<T>) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pretty cool!!! Wonder if there's any way we could make these types not "any"/ more strict, maybe using typescript generics or something; Also would be pretty cool if we could move this to mcp-common and accept the CloudflareMcpAgent interface, so we can reuse this across other mcp servers!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this was a "vibe coding" addition, i can explore how to add some type strictness around this.
Connect Metrics
| * For more details on how to configure Wrangler, refer to: | ||
| * https://developers.cloudflare.com/workers/wrangler/configuration/ | ||
| */ | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmsparks @Maximo-Guk this should be re-factored to have all the metrics bindings included now 👍
|
|
||
| export type State = { activeAccountId: string | null } | ||
| export class CASBMCP extends McpAgent<Env, State, Props> { | ||
| server = new CloudflareMCPServer(undefined, env.MCP_METRICS, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ty for figuring this out without my write up!
One small thing, could you move this into the init function and pass in the userId with this.props.user.id like we do in the observability server: https://github.com/cloudflare/mcp-server-cloudflare/blob/main/apps/workers-observability/src/index.ts#L34-L59
Otherwise, lgtm!
| version: this.env.MCP_SERVER_VERSION, | ||
| }) | ||
|
|
||
| registerAccountTools(this as unknown as CloudflareMcpAgent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why you're having to cast this as unknown? It looks like your CASBMCP agent should line up with the CloudflareMCPAgent interface 🤔
MCP Inspector
What
Open questions