Skip to content

Conversation

@Avish34
Copy link
Contributor

@Avish34 Avish34 commented Sep 4, 2025

Add check for localhost in remote url.
#274 (comment)

Motivation and Context

We don't want authors to publish server with remote url set as localhost.

How Has This Been Tested?

It has been tested using UTs.

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@Avish34
Copy link
Contributor Author

Avish34 commented Sep 4, 2025

@tadasant / @domdomegg I see we have one testcase which says "allow localhost urls" in validation_test.go. Can you please help clarify what's the expectation here?

{ name: "localhost URLs allowed with any namespace", serverDetail: apiv0.ServerJSON{ Name: "com.example/test-server", Remotes: []model.Remote{ {URL: "http://localhost:3000/sse"}, }, }, expectError: false, },

domdomegg
domdomegg previously approved these changes Sep 4, 2025
Copy link
Member

@domdomegg domdomegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy to merge as-is and add the test as a follow-up, will hold open for a few hours so you can add the test to this one if you want :)

@domdomegg
Copy link
Member

@Avish34 I think we probably don't want to enable localhost urls, that test is wrong and can be deleted. I think we'll figure out the latest with how we handle local servers in #345 (comment), for now we can disallow them here.

domdomegg
domdomegg previously approved these changes Sep 4, 2025
@domdomegg
Copy link
Member

will fix the merge conflicts for this in a sec

Merge main into fix-remote-url branch, keeping localhost validation tests.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@domdomegg domdomegg merged commit 8d28eed into modelcontextprotocol:main Sep 5, 2025
3 checks passed
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