Skip to content

Conversation

ossamalafhel
Copy link
Contributor

Summary

Added validation to reject server names with multiple slashes, ensuring consistency between JSON schema pattern and API validation.

  • Count slashes in parseServerName() and reject if > 1
  • Add ErrMultipleSlashesInServerName error constant
  • Add unit tests in validators_test.go
  • Add integration test in publish_test.go

Motivation and Context

Fixes #471. The JSON schema regex pattern ^[^/]+/[^/]+$ only allows a single slash in server names, but the API validation was not enforcing this constraint. This inconsistency
allowed invalid server names like com.example/server/path to be published.

How Has This Been Tested?

  • Unit tests added to verify slash validation logic
  • Integration tests added to verify the publish endpoint rejects multi-slash names
  • Tested various edge cases: trailing slashes, consecutive slashes, URL-like paths
  • All existing tests pass

Breaking Changes

None. This change only adds validation to reject previously invalid server names that shouldn't have been accepted.

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

  • The fix ensures server names follow the intended format: namespace/name (e.g., com.example/my-server)
  • This validation aligns the API behavior with the existing JSON schema regex pattern
  • Test coverage includes edge cases like com.example//server, com.example/server/, and a/b/c/d/e/f
  • The error message clearly indicates the issue: "server name cannot contain multiple slashes"

@ossamalafhel ossamalafhel force-pushed the fix/server-name-multiple-slashes-validation branch 3 times, most recently from 7964d83 to c9ae434 Compare September 14, 2025 12:35
@ossamalafhel ossamalafhel force-pushed the fix/server-name-multiple-slashes-validation branch from 75aafc5 to 10350ea Compare September 14, 2025 12:49
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.

Thanks! Maybe we could actually add it as regex validation on the API types, which would show up in the docs and align the specs more?

Happy to merge this for now though

@domdomegg domdomegg merged commit 63cf08e into modelcontextprotocol:main Sep 15, 2025
3 checks passed
@ossamalafhel
Copy link
Contributor Author

ossamalafhel commented Sep 15, 2025

Thanks! Maybe we could actually add it as regex validation on the API types, which would show up in the docs and align the specs more?

Happy to merge this for now though

Hi @domdomegg,

Thanks for the review and the great suggestion! I've added the regex validation directly to the API types as you recommended.

The pattern ^[^/]+/[^/]+$ is now on the Name field in pkg/api/v0/types.go, which should make the validation visible in the API documentation and align the specs with the backend validation.

This complements the existing validation logic nicely - clients can now see the expected format in the API docs, and we still have the detailed error handling in the backend.

Let me know if this looks good to you!

new PR here: #479

jalateras added a commit to jalateras/registry that referenced this pull request Sep 17, 2025
Add proper boundary validation to prevent domains like micro.com from
claiming permissions for com.microsoft/* through prefix overlap attacks.

Changes:
- Add delimiter checking after domain prefix (must be . or /)
- Enforce single-slash rule for server names per PR modelcontextprotocol#476
- Add comprehensive test cases for prefix attack scenarios
- Update existing tests to align with new validation rules

The validation now ensures name patterns have proper delimiters and
follow the established server naming conventions.
jalateras added a commit to jalateras/registry that referenced this pull request Sep 17, 2025
…tprotocol#481)

Implements optional n=<pattern> parameter in DNS TXT records to allow
fine-grained permission scoping for server names. This enables DNS
controllers to delegate limited namespaces to different teams while
maintaining security isolation.

Key changes:
- Added DNSAuthRecord struct to support name patterns alongside public keys
- Collect all valid DNS records instead of stopping at first match
- Validate name patterns must start with reverse domain to prevent
  cross-domain privilege escalation
- Add delimiter checking after domain prefix (must be . or /)
- Enforce single-slash rule for server names per PR modelcontextprotocol#476
- Added comprehensive tests for all scoping scenarios

Example usage:
v=MCPv1; k=ed25519; p=<key>; n=com.example/team-foo-*

This change addresses Microsoft's requirement for granular permission
control within path portions while preserving existing behavior for
records without the n= parameter.

Fixes modelcontextprotocol#481
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.

Does server name after slash allow more slashes? (inconsistent schema and endpoint)

2 participants