Skip to content

Kr/mcp add server mode#389

Closed
rocktavious wants to merge 4 commits intofeature/mcpfrom
kr/mcp-add-server-mode
Closed

Kr/mcp add server mode#389
rocktavious wants to merge 4 commits intofeature/mcpfrom
kr/mcp-add-server-mode

Conversation

@rocktavious
Copy link
Collaborator

Resolves #

Problem

MCP servers can run in 2 modes - stdio and http

Solution

Expose commandline arguments to configure the mode and the port for http mode

Checklist

  • I have run this code, and it appears to resolve the stated issue.
  • This PR has no user interface changes or has already received approval from product management to change the interface.
  • Make a changie entry that explains the customer facing outcome of this change

@rocktavious rocktavious self-assigned this Apr 15, 2025
@rocktavious rocktavious changed the base branch from main to kr/mcp-add-beta April 15, 2025 17:10
@eapache-opslevel
Copy link
Contributor

Do we need to do any of the items in https://modelcontextprotocol.io/docs/concepts/transports#security-warning%3A-dns-rebinding-attacks to protect the security of this mode?

@rocktavious
Copy link
Collaborator Author

@eapache-opslevel we should probably do

  • Implement rate limiting
  • Use appropriate timeouts
  • For local SSE servers, bind only to localhost (127.0.0.1) instead of all interfaces (0.0.0.0)

The first 2 we should cut as future tickets - the 3rd one i'll fix in this PR

Base automatically changed from kr/mcp-add-beta to feature/mcp April 15, 2025 19:12
@rocktavious rocktavious force-pushed the kr/mcp-add-server-mode branch from 3997770 to 03f0c90 Compare April 15, 2025 19:13
@eapache-opslevel
Copy link
Contributor

We are going to let this linger a bit. We don't understand the security implications fully, and all anybody needs right now is the STDIO version which has no security concerns.

@rocktavious
Copy link
Collaborator Author

rocktavious commented Apr 15, 2025

Additional reason to wait on this - client := getClientGQL() is reading the API token from env vars not from headers passed over the wire. So we'll need to modify that aspect when running in HTTP mode.

@rocktavious
Copy link
Collaborator Author

Won't Do. We've decided to switch libraries so this will have to be slightly re-written. going to close for now but keep the branch for when we want SSE with mcp-go

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.

2 participants