fix: reject mangled publisher metadata (#1310)
## Summary - add a publisher preflight that rejects non-UTF-8 `server.json` bytes before parsing - reject unpaired UTF-16 surrogate escapes inside JSON string fields before publish/validate requests are sent - keep valid non-ASCII descriptions, including em dashes, round-tripping through the publish path - make the publisher test token helper set `USERPROFILE` as well as `HOME` so Windows tests use the temp home directory Closes #1306. ## To verify - `go test ./cmd/publisher/commands -run "TestPublishCommand_(Success|PreservesNonASCIIDescription|RejectsInvalidUTF8ServerJSON|RejectsUnpairedSurrogateEscape)$" -count=1` - `go test ./cmd/publisher/commands -run TestPublishCommand -count=1` - `go test ./cmd/publisher/commands -run TestValidateCommand -count=1` - `git diff --check` Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com>
This commit is contained in:
parent
2d3262c8aa
commit
c36b521bf1
5 changed files with 190 additions and 1 deletions
|
|
@ -28,6 +28,9 @@ func PublishCommand(args []string) error {
|
|||
}
|
||||
return fmt.Errorf("failed to read server.json: %w", err)
|
||||
}
|
||||
if err := validateJSONUnicode(serverFile, serverData); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Validate JSON
|
||||
var serverJSON apiv0.ServerJSON
|
||||
|
|
@ -99,6 +102,10 @@ func PublishCommand(args []string) error {
|
|||
}
|
||||
|
||||
func publishToRegistry(registryURL string, serverData []byte, token string) (*apiv0.ServerResponse, int, error) {
|
||||
if err := validateJSONUnicode("server.json", serverData); err != nil {
|
||||
return nil, 0, err
|
||||
}
|
||||
|
||||
// Parse the server JSON data
|
||||
var serverJSON apiv0.ServerJSON
|
||||
err := json.Unmarshal(serverData, &serverJSON)
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ import (
|
|||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/modelcontextprotocol/registry/cmd/publisher/commands"
|
||||
|
|
@ -37,6 +39,57 @@ func TestPublishCommand_Success(t *testing.T) {
|
|||
assert.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestPublishCommand_PreservesNonASCIIDescription(t *testing.T) {
|
||||
server := SetupMockRegistryServer(t,
|
||||
func(w http.ResponseWriter, r *http.Request) {
|
||||
var req apiv0.ServerJSON
|
||||
require.NoError(t, json.NewDecoder(r.Body).Decode(&req))
|
||||
assert.Equal(t, "Tools for agents — safely", req.Description)
|
||||
|
||||
w.WriteHeader(http.StatusCreated)
|
||||
_ = json.NewEncoder(w).Encode(apiv0.ServerResponse{
|
||||
Server: apiv0.ServerJSON{
|
||||
Name: req.Name,
|
||||
Version: req.Version,
|
||||
},
|
||||
})
|
||||
},
|
||||
nil,
|
||||
)
|
||||
SetupTestToken(t, server.URL, "test-token")
|
||||
|
||||
CreateTestServerJSON(t, apiv0.ServerJSON{
|
||||
Schema: model.CurrentSchemaURL,
|
||||
Name: "com.example/test-server",
|
||||
Description: "Tools for agents — safely",
|
||||
Version: "1.0.0",
|
||||
})
|
||||
|
||||
require.NoError(t, commands.PublishCommand([]string{}))
|
||||
}
|
||||
|
||||
func TestPublishCommand_RejectsInvalidUTF8ServerJSON(t *testing.T) {
|
||||
createRawServerJSON(t, []byte{
|
||||
'{', '"', '$', 's', 'c', 'h', 'e', 'm', 'a', '"', ':', '"', '"', ',',
|
||||
'"', 'n', 'a', 'm', 'e', '"', ':', '"', 'c', 'o', 'm', '.', 'e', 'x', 'a', 'm', 'p', 'l', 'e', '/', 't', 'e', 's', 't', '"', ',',
|
||||
'"', 'd', 'e', 's', 'c', 'r', 'i', 'p', 't', 'i', 'o', 'n', '"', ':', '"', 'b', 'a', 'd', ' ', 0xe2, '"', ',',
|
||||
'"', 'v', 'e', 'r', 's', 'i', 'o', 'n', '"', ':', '"', '1', '.', '0', '.', '0', '"', '}',
|
||||
})
|
||||
|
||||
err := commands.PublishCommand([]string{})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "UTF-8")
|
||||
}
|
||||
|
||||
func TestPublishCommand_RejectsUnpairedSurrogateEscape(t *testing.T) {
|
||||
createRawServerJSON(t, []byte(`{"$schema":"","name":"com.example/test","description":"bad \udc94","version":"1.0.0"}`))
|
||||
|
||||
err := commands.PublishCommand([]string{})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "unpaired UTF-16 surrogate")
|
||||
assert.Contains(t, err.Error(), "$.description")
|
||||
}
|
||||
|
||||
func TestPublishCommand_422ValidationFlow(t *testing.T) {
|
||||
validateCallCount := 0
|
||||
publishCallCount := 0
|
||||
|
|
@ -102,6 +155,20 @@ func TestPublishCommand_422ValidationFlow(t *testing.T) {
|
|||
assert.Equal(t, 1, validateCallCount, "validate endpoint should be called once after 422")
|
||||
}
|
||||
|
||||
func createRawServerJSON(t *testing.T, data []byte) {
|
||||
t.Helper()
|
||||
|
||||
tempDir := t.TempDir()
|
||||
serverFile := filepath.Join(tempDir, "server.json")
|
||||
require.NoError(t, os.WriteFile(serverFile, data, 0600))
|
||||
|
||||
originalDir, err := os.Getwd()
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = os.Chdir(originalDir) })
|
||||
|
||||
require.NoError(t, os.Chdir(tempDir))
|
||||
}
|
||||
|
||||
func TestPublishCommand_422WithMultipleIssues(t *testing.T) {
|
||||
validateCallCount := 0
|
||||
|
||||
|
|
|
|||
|
|
@ -68,9 +68,10 @@ func SetupMockRegistryServer(t *testing.T, publishHandler func(w http.ResponseWr
|
|||
func SetupTestToken(t *testing.T, registryURL, token string) string {
|
||||
t.Helper()
|
||||
|
||||
// Override $HOME so tokenFilePath() resolves to a temp directory
|
||||
// Override the home directory so tokenFilePath() resolves to a temp directory.
|
||||
tempHome := t.TempDir()
|
||||
t.Setenv("HOME", tempHome)
|
||||
t.Setenv("USERPROFILE", tempHome)
|
||||
|
||||
dir := filepath.Join(tempHome, ".config", "mcp-publisher")
|
||||
require.NoError(t, os.MkdirAll(dir, 0700))
|
||||
|
|
|
|||
107
cmd/publisher/commands/unicode.go
Normal file
107
cmd/publisher/commands/unicode.go
Normal file
|
|
@ -0,0 +1,107 @@
|
|||
package commands
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"strconv"
|
||||
"unicode/utf8"
|
||||
)
|
||||
|
||||
func validateJSONUnicode(filename string, data []byte) error {
|
||||
if !utf8.Valid(data) {
|
||||
return fmt.Errorf("%s must be encoded as UTF-8", filename)
|
||||
}
|
||||
|
||||
if path, ok := findUnpairedSurrogate(data, "$"); ok {
|
||||
return fmt.Errorf("%s contains an unpaired UTF-16 surrogate escape at %s", filename, path)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func findUnpairedSurrogate(raw json.RawMessage, path string) (string, bool) {
|
||||
raw = bytes.TrimSpace(raw)
|
||||
if len(raw) == 0 {
|
||||
return "", false
|
||||
}
|
||||
|
||||
switch raw[0] {
|
||||
case '"':
|
||||
if hasUnpairedSurrogateEscape(raw) {
|
||||
return path, true
|
||||
}
|
||||
case '{':
|
||||
var obj map[string]json.RawMessage
|
||||
if err := json.Unmarshal(raw, &obj); err != nil {
|
||||
return "", false
|
||||
}
|
||||
for key, value := range obj {
|
||||
if badPath, ok := findUnpairedSurrogate(value, path+"."+key); ok {
|
||||
return badPath, true
|
||||
}
|
||||
}
|
||||
case '[':
|
||||
var items []json.RawMessage
|
||||
if err := json.Unmarshal(raw, &items); err != nil {
|
||||
return "", false
|
||||
}
|
||||
for i, item := range items {
|
||||
if badPath, ok := findUnpairedSurrogate(item, fmt.Sprintf("%s[%d]", path, i)); ok {
|
||||
return badPath, true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return "", false
|
||||
}
|
||||
|
||||
func hasUnpairedSurrogateEscape(s []byte) bool {
|
||||
for i := 1; i < len(s)-1; i++ {
|
||||
if s[i] != '\\' || i+5 >= len(s) || s[i+1] != 'u' {
|
||||
continue
|
||||
}
|
||||
|
||||
code, ok := parseHex4(s[i+2 : i+6])
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
|
||||
if isLowSurrogate(code) {
|
||||
return true
|
||||
}
|
||||
|
||||
if !isHighSurrogate(code) {
|
||||
i += 5
|
||||
continue
|
||||
}
|
||||
|
||||
if i+11 >= len(s) || s[i+6] != '\\' || s[i+7] != 'u' {
|
||||
return true
|
||||
}
|
||||
|
||||
next, ok := parseHex4(s[i+8 : i+12])
|
||||
if !ok || !isLowSurrogate(next) {
|
||||
return true
|
||||
}
|
||||
i += 11
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
func parseHex4(b []byte) (rune, bool) {
|
||||
v, err := strconv.ParseInt(string(b), 16, 32)
|
||||
if err != nil {
|
||||
return 0, false
|
||||
}
|
||||
return rune(v), true
|
||||
}
|
||||
|
||||
func isHighSurrogate(r rune) bool {
|
||||
return r >= 0xD800 && r <= 0xDBFF
|
||||
}
|
||||
|
||||
func isLowSurrogate(r rune) bool {
|
||||
return r >= 0xDC00 && r <= 0xDFFF
|
||||
}
|
||||
|
|
@ -142,6 +142,9 @@ func ValidateCommand(args []string) error {
|
|||
}
|
||||
return fmt.Errorf("failed to read %s: %w", serverFile, err)
|
||||
}
|
||||
if err := validateJSONUnicode(serverFile, serverData); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Validate JSON
|
||||
var serverJSON apiv0.ServerJSON
|
||||
|
|
@ -188,6 +191,10 @@ func ValidateCommand(args []string) error {
|
|||
|
||||
// validateViaAPI calls the /validate endpoint on the registry
|
||||
func validateViaAPI(registryURL string, serverData []byte) (*validators.ValidationResult, error) {
|
||||
if err := validateJSONUnicode("server.json", serverData); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Parse the server JSON data to ensure it's valid JSON
|
||||
var serverJSON apiv0.ServerJSON
|
||||
err := json.Unmarshal(serverData, &serverJSON)
|
||||
|
|
|
|||
Loading…
Reference in a new issue