Skip to content

Conversation

@auldsyababua
Copy link

  • Implement secure HTTP client with CheckRedirect function that blocks all redirects
  • Add comprehensive test coverage for both redirect blocking and normal operation
  • Prevents SSRF attacks and credential leakage through malicious redirects
  • Uses Go standard library http.ErrUseLastResponse pattern (industry best practice)
  • Zero performance overhead, follows OWASP security guidelines

Security benefits:

  • Blocks redirects to prevent internal/external malicious service access
  • Protects Bearer tokens from leaking to unauthorized redirect targets
  • Ensures MCP connections go only to intended endpoints

Test coverage:

  • TestCreateMcpServerConnBlocksRedirects: Verifies redirect blocking
  • TestCreateMcpServerConnWorksWithoutRedirects: Ensures no regression

- Implement secure HTTP client with CheckRedirect function that blocks all redirects
- Add comprehensive test coverage for both redirect blocking and normal operation
- Prevents SSRF attacks and credential leakage through malicious redirects
- Uses Go standard library http.ErrUseLastResponse pattern (industry best practice)
- Zero performance overhead, follows OWASP security guidelines

Security benefits:
- Blocks redirects to prevent internal/external malicious service access
- Protects Bearer tokens from leaking to unauthorized redirect targets
- Ensures MCP connections go only to intended endpoints

Test coverage:
- TestCreateMcpServerConnBlocksRedirects: Verifies redirect blocking
- TestCreateMcpServerConnWorksWithoutRedirects: Ensures no regression
@duaraghav8
Copy link
Member

duaraghav8 commented Jul 23, 2025

@auldsyababua I tested the PR out, great work!

What do you think about making the error message better?

Currently, this is the output when I try to connect to a MCP server that redirects:

$ mcpjungle register --name redirector --url http://localhost:9000/mcp

failed to register server: request failed with status: 500, message: {"error":"failed to connect to MCP server redirector: failed to initialize connection with MCP server: transport error: request failed with status 307: "}

The error message can be better, like This MCP server is trying to redirect which might be insecure...

Will be merging & releasing this in a few days.


Reason I want to wait a few days to release this:

Although the code works as intended, it is also rejecting genuine MCP servers written with the python SDK because of this issue and this one.

Apparently when using streamable http, if the client makes a request to /mcp/, the server is happy to serve.
But when requesting /mcp (without the trailing slash), the FastMCP servers sends a 307 Redirect to /mcp/.

But this issue has been fixed in the latest mcp package release 1.12. So let's give people some time to upgrade their package to the latest.

@duaraghav8 duaraghav8 force-pushed the main branch 3 times, most recently from a6a3ae0 to cbf8ba1 Compare August 3, 2025 19:04
@duaraghav8 duaraghav8 force-pushed the main branch 3 times, most recently from 67f2579 to 6709e3a Compare September 11, 2025 13:50
@duaraghav8 duaraghav8 force-pushed the main branch 2 times, most recently from e6f9f3d to 321f552 Compare October 8, 2025 08:33
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