-
Notifications
You must be signed in to change notification settings - Fork 314
feat: Add support for MCP Bundles (MCPB) in registry #295
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
Changes from 1 commit
212bccd
2ee6e00
84cdc81
aafe260
fb0283e
cec8cdd
17b3824
8287e88
ca7fc20
4ccfc41
fb39c44
00fd953
6b8dee7
c7ed33c
e242a8d
c194b3d
1aab2b4
6a3a4e1
0ec0235
ac5fedb
55f1b48
f91f45d
ed35452
b6cd4d3
53fb0e2
89971fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -389,6 +389,38 @@ The `dnx` tool ships with the .NET 10 SDK, starting with Preview 6. | |
} | ||
``` | ||
|
||
## MCP Bundle (MCPB) Package Example | ||
|
||
```json | ||
{ | ||
"name": "io.modelcontextprotocol/text-editor", | ||
"description": "MCP Bundle server for advanced text editing capabilities", | ||
"repository": { | ||
"url": "https://github.com/modelcontextprotocol/text-editor-mcpb", | ||
"source": "github", | ||
"id": "mcpb-123ab-cdef4-56789-012ghi-jklmnopqrstu" | ||
}, | ||
"version_detail": { | ||
"version": "1.0.2" | ||
}, | ||
"packages": [ | ||
{ | ||
"registry_name": "mcpb", | ||
"name": "https://github.com/modelcontextprotocol/text-editor-mcpb/releases/download/v1.0.2/text-editor.mcpb", | ||
"version": "1.0.2", | ||
"file_hashes": { | ||
"sha-256": "fe333e598595000ae021bd27117db32ec69af6987f507ba7a63c90638ff633ce" | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we consider lifting this inside the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess there is the possibility we add some registry in the future that does not do its own checks. So I'm fine keeping it here as an optional field. I assume we need to rely on the MCP Registry to generate this file hash at publish-time, right? Or is the idea it will get generated by the CLI tool en route to creating a server.json, and the registry just validates it's correct and not tampered with in the publish flow? I think we should map out and document this feature in a little more detail, maybe a dedicated doc in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not opinionated here and will defer to you all re: what makes sense for the workflow the registry committee would prefer to support; happy add docs to reflect. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joan-anthropic @domdomegg did we work out who is responsible for what in this file hash verification flow somewhere? Sorry if I missed it, still working through catching up on emails and messages There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tadasant apologies for the delay - I'm OOO through the rest of the week so might be a bit slow to respond. I ultimately wanted to leave the hash validation workflow as a question for you / @domdomegg / @toby as it'll be supported by the registry committee (iiuc). I'm happy to open a follow-up PR to reflect whatever decision ends up being made, though let me know if you're un-opinionated and I'm happy take a stab at it. Just so I understand: do we actually need to generate/verify the hash at publish time? My impression was that it was a guard against future tampering with mcpb contents - so could we trust that the publication is correct and have registry clients take responsibility for hash verification before they install the mcpb? Ie, make this an contract between mcpb author <> registry client, and not MCP registry <> registry client? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding a ticket to think through this to make sure we tackle before go-live (apologies if I'm still missing context somewhere, haven't given this field much thought yet): #351 |
||
} | ||
] | ||
} | ||
``` | ||
|
||
This example shows an MCPB (MCP Bundle) package that: | ||
- Is hosted on GitHub Releases (an allowlisted provider) | ||
- Includes a SHA-256 hash for integrity verification | ||
- Can be downloaded and executed directly by MCP clients that support MCPB | ||
|
||
## Deprecated Server Example | ||
|
||
```json | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,8 @@ | |
"npm", | ||
"pypi", | ||
"docker", | ||
"nuget" | ||
"nuget", | ||
"mcpb" | ||
] | ||
}, | ||
"runtime_hint": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,9 @@ package service | |
|
||
import ( | ||
"context" | ||
"fmt" | ||
"net/url" | ||
"strings" | ||
"time" | ||
|
||
"github.com/modelcontextprotocol/registry/internal/database" | ||
|
@@ -84,6 +87,51 @@ func (s *registryServiceImpl) GetByID(id string) (*model.ServerDetail, error) { | |
return serverDetail, nil | ||
} | ||
|
||
// validateMCPBPackage validates MCPB packages to ensure they meet requirements | ||
func validateMCPBPackage(pkg *model.Package) error { | ||
// Validate that the URL is from an allowlisted host | ||
parsedURL, err := url.Parse(pkg.Name) | ||
if err != nil { | ||
return fmt.Errorf("invalid MCPB package URL: %w", err) | ||
} | ||
|
||
// Allowlist of trusted hosts for MCPB packages | ||
allowedHosts := []string{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to verify There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lgtm |
||
"github.com", | ||
"www.github.com", | ||
"raw.githubusercontent.com", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. possibly this one (raw.githubusercontent.com) is not necessary |
||
"gitlab.com", | ||
"www.gitlab.com", | ||
} | ||
|
||
host := strings.ToLower(parsedURL.Host) | ||
isAllowed := false | ||
for _, allowed := range allowedHosts { | ||
if host == allowed { | ||
isAllowed = true | ||
break | ||
} | ||
} | ||
|
||
if !isAllowed { | ||
return fmt.Errorf("MCPB packages must be hosted on allowlisted providers (GitHub or GitLab). Host '%s' is not allowed", host) | ||
} | ||
|
||
// Validate that file_hashes is provided for MCPB packages | ||
if len(pkg.FileHashes) == 0 { | ||
return fmt.Errorf("MCPB packages must include file_hashes for integrity verification") | ||
} | ||
|
||
// Validate that at least SHA-256 is provided | ||
if _, hasSHA256 := pkg.FileHashes["sha-256"]; !hasSHA256 { | ||
if _, hasSHA256Alt := pkg.FileHashes["sha256"]; !hasSHA256Alt { | ||
return fmt.Errorf("MCPB packages must include a SHA-256 hash") | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// Publish adds a new server detail to the registry | ||
func (s *registryServiceImpl) Publish(serverDetail *model.ServerDetail) error { | ||
// Create a timeout context for the database operation | ||
|
@@ -94,6 +142,15 @@ func (s *registryServiceImpl) Publish(serverDetail *model.ServerDetail) error { | |
return database.ErrInvalidInput | ||
} | ||
|
||
// Validate MCPB packages | ||
for _, pkg := range serverDetail.Packages { | ||
if strings.ToLower(pkg.RegistryName) == "mcpb" { | ||
if err := validateMCPBPackage(&pkg); err != nil { | ||
return fmt.Errorf("validation failed: %w", err) | ||
} | ||
} | ||
} | ||
|
||
err := s.db.Publish(ctx, serverDetail) | ||
if err != nil { | ||
return err | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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 right with our current schema. But a thought for potentially renaming some details here:
name
doesn't really make sense for a URL like this, andregistry_name
doesn't formcpb
either (it's more of a packagetype
).I think I'd be in favor of something more like this here:
And that's it - drop
version
, because the only reason we haveversion
is to effectively "construct" theurl
in theregistry_name
case.Basically, my proposal is to add a nested
PackageLocation
object inside here, where we currently just store package_name, name, and version in the top level of thePackage
object. It would be an "OR" situation where either the original fields (registry_name, name, and version) or the new fields (type, url) are required.mcpb
is the only valid entry fortype
.Then the original case for other examples, like the one just before this, becomes:
It's a breaking change but I think at this point, pre-release, is a good time to make this rather than shoehorn values into
registry_name
andname
. What do you all think? cc @domdomegg @joan-anthropic @tobyThere 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.
@tadasant thanks for the quick turnaround. Agree that
registry_name
andname
feel a bit overloaded in the current proposal - I like the distinction.It'd be nice to keep version as a separate field - to better enable future auto-update workflows for DXTs. Thoughts?
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.
Not sure I follow the use case, could you explain auto-update workflows for DXTs? Is there a reason you wouldn't be able to use the
https://github.com/modelcontextprotocol/text-editor-mcpb/releases/download/v1.0.2/text-editor.mcpb
value for that (which has the version in it)?We've had some feedback from @toby that all these
version
fields throughout the various server.json levels are causing confusion, so my thinking is that this is a good opportunity to potentially eliminate one of them as it's not actually necessary (but maybe I'm missing something).Uh oh!
There was an error while loading. Please reload this page.
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.
So together I think I might propose something like:
packages
array for DXT. There will still be a version at the top level in server.json. For auto-update I think we'd use this top-level version, and maybeis_latest
.