diff --git a/design/design.md b/design/design.md index 5a2b103..a5da382 100644 --- a/design/design.md +++ b/design/design.md @@ -742,7 +742,7 @@ To read files from the local filesystem, we recommend using `FileResourceHandler ```go // FileResourceHandler returns a ResourceHandler that reads paths using dir as a root directory. // It protects against path traversal attacks. -// It will not read any file that is not in the root set of the client session requesting the resource. +// It will not read any file that is not in the root set of the client requesting the resource. func (*Server) FileResourceHandler(dir string) ResourceHandler ``` diff --git a/mcp/mcp_test.go b/mcp/mcp_test.go index 7812350..1f697b6 100644 --- a/mcp/mcp_test.go +++ b/mcp/mcp_test.go @@ -9,6 +9,8 @@ import ( "context" "errors" "fmt" + "path/filepath" + "runtime" "slices" "strings" "sync" @@ -83,7 +85,11 @@ func TestEndToEnd(t *testing.T) { }() c := NewClient("testClient", "v1.0.0", nil) - c.AddRoots(&Root{URI: "file:///root"}) + rootAbs, err := filepath.Abs(filepath.FromSlash("testdata/files")) + if err != nil { + t.Fatal(err) + } + c.AddRoots(&Root{URI: "file://" + rootAbs}) // Connect the client. cs, err := c.Connect(ctx, ct) @@ -189,10 +195,13 @@ func TestEndToEnd(t *testing.T) { }) t.Run("resources", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TODO: fix for Windows") + } resource1 := &Resource{ Name: "public", MIMEType: "text/plain", - URI: "file:///file1.txt", + URI: "file:///info.txt", } resource2 := &Resource{ Name: "public", // names are not unique IDs @@ -200,16 +209,7 @@ func TestEndToEnd(t *testing.T) { URI: "file:///nonexistent.txt", } - readHandler := func(_ context.Context, _ *ServerSession, p *ReadResourceParams) (*ReadResourceResult, error) { - if p.URI == "file:///file1.txt" { - return &ReadResourceResult{ - Contents: &ResourceContents{ - Text: "file contents", - }, - }, nil - } - return nil, ResourceNotFoundError(p.URI) - } + readHandler := s.FileResourceHandler("testdata/files") s.AddResources( &ServerResource{resource1, readHandler}, &ServerResource{resource2, readHandler}) @@ -226,7 +226,7 @@ func TestEndToEnd(t *testing.T) { uri string mimeType string // "": not found; "text/plain": resource; "text/template": template }{ - {"file:///file1.txt", "text/plain"}, + {"file:///info.txt", "text/plain"}, {"file:///nonexistent.txt", ""}, // TODO(jba): add resource template cases when we implement them } { @@ -238,7 +238,7 @@ func TestEndToEnd(t *testing.T) { t.Errorf("%s: not found but expected it to be", tt.uri) } } else { - t.Fatalf("reading %s: %v", tt.uri, err) + t.Errorf("reading %s: %v", tt.uri, err) } } else { if got := rres.Contents.URI; got != tt.uri { diff --git a/mcp/resource.go b/mcp/resource.go new file mode 100644 index 0000000..04f1a00 --- /dev/null +++ b/mcp/resource.go @@ -0,0 +1,158 @@ +// Copyright 2025 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file. + +package mcp + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/url" + "os" + "path/filepath" + "strings" + + "github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2" +) + +// A ServerResource associates a Resource with its handler. +type ServerResource struct { + Resource *Resource + Handler ResourceHandler +} + +// A ResourceHandler is a function that reads a resource. +// If it cannot find the resource, it should return the result of calling [ResourceNotFoundError]. +type ResourceHandler func(context.Context, *ServerSession, *ReadResourceParams) (*ReadResourceResult, error) + +// ResourceNotFoundError returns an error indicating that a resource being read could +// not be found. +func ResourceNotFoundError(uri string) error { + return &jsonrpc2.WireError{ + Code: codeResourceNotFound, + Message: "Resource not found", + Data: json.RawMessage(fmt.Sprintf(`{"uri":%q}`, uri)), + } +} + +// The error code to return when a resource isn't found. +// See https://modelcontextprotocol.io/specification/2025-03-26/server/resources#error-handling +// However, the code they chose in in the wrong space +// (see https://github.com/modelcontextprotocol/modelcontextprotocol/issues/509). +// so we pick a different one, arbirarily for now (until they fix it). +// The immediate problem is that jsonprc2 defines -32002 as "server closing". +const codeResourceNotFound = -31002 + +// readFileResource reads from the filesystem at a URI relative to dirFilepath, respecting +// the roots. +// dirFilepath and rootFilepaths are absolute filesystem paths. +func readFileResource(rawURI, dirFilepath string, rootFilepaths []string) ([]byte, error) { + uriFilepath, err := computeURIFilepath(rawURI, dirFilepath, rootFilepaths) + if err != nil { + return nil, err + } + + var data []byte + err = withFile(dirFilepath, uriFilepath, func(f *os.File) error { + var err error + data, err = io.ReadAll(f) + return err + }) + if os.IsNotExist(err) { + err = ResourceNotFoundError(rawURI) + } + return data, err +} + +// computeURIFilepath returns a path relative to dirFilepath. +// The dirFilepath and rootFilepaths are absolute file paths. +func computeURIFilepath(rawURI, dirFilepath string, rootFilepaths []string) (string, error) { + // We use "file path" to mean a filesystem path. + uri, err := url.Parse(rawURI) + if err != nil { + return "", err + } + if uri.Scheme != "file" { + return "", fmt.Errorf("URI is not a file: %s", uri) + } + if uri.Path == "" { + // A more specific error than the one below, to catch the + // common mistake "file://foo". + return "", errors.New("empty path") + } + // The URI's path is interpreted relative to dirFilepath, and in the local filesystem. + // It must not try to escape its directory. + uriFilepathRel, err := filepath.Localize(strings.TrimPrefix(uri.Path, "/")) + if err != nil { + return "", fmt.Errorf("%q cannot be localized: %w", uriFilepathRel, err) + } + + // Check roots, if there are any. + if len(rootFilepaths) > 0 { + // To check against the roots, we need an absolute file path, not relative to the directory. + // uriFilepath is local, so the joined path is under dirFilepath. + uriFilepathAbs := filepath.Join(dirFilepath, uriFilepathRel) + rootOK := false + // Check that the requested file path is under some root. + // Since both paths are absolute, that's equivalent to filepath.Rel constructing + // a local path. + for _, rootFilepathAbs := range rootFilepaths { + if rel, err := filepath.Rel(rootFilepathAbs, uriFilepathAbs); err == nil && filepath.IsLocal(rel) { + rootOK = true + break + } + } + if !rootOK { + return "", fmt.Errorf("URI path %q is not under any root", uriFilepathAbs) + } + } + return uriFilepathRel, nil +} + +// fileRoots transforms the Roots obtained from the client into absolute paths on +// the local filesystem. +// TODO(jba): expose this functionality to user ResourceHandlers, +// so they don't have to repeat it. +func fileRoots(rawRoots []*Root) ([]string, error) { + var fileRoots []string + for _, r := range rawRoots { + fr, err := fileRoot(r) + if err != nil { + return nil, err + } + fileRoots = append(fileRoots, fr) + } + return fileRoots, nil +} + +// fileRoot returns the absolute path for Root. +func fileRoot(root *Root) (_ string, err error) { + defer func() { + if err != nil { + err = fmt.Errorf("root %q: %w", root.URI, err) + } + }() + + // Convert to absolute file path. + rurl, err := url.Parse(root.URI) + if err != nil { + return "", err + } + if rurl.Scheme != "file" { + return "", errors.New("not a file URI") + } + if rurl.Path == "" { + // A more specific error than the one below, to catch the + // common mistake "file://foo". + return "", errors.New("empty path") + } + // We don't want Localize here: we want an absolute path, which is not local. + fileRoot := filepath.Clean(filepath.FromSlash(rurl.Path)) + if !filepath.IsAbs(fileRoot) { + return "", errors.New("not an absolute path") + } + return fileRoot, nil +} diff --git a/mcp/resource_go124.go b/mcp/resource_go124.go new file mode 100644 index 0000000..4a35603 --- /dev/null +++ b/mcp/resource_go124.go @@ -0,0 +1,29 @@ +// Copyright 2025 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file. + +//go:build go1.24 + +package mcp + +import ( + "errors" + "os" +) + +// withFile calls f on the file at join(dir, rel), +// protecting against path traversal attacks. +func withFile(dir, rel string, f func(*os.File) error) (err error) { + r, err := os.OpenRoot(dir) + if err != nil { + return err + } + defer r.Close() + file, err := r.Open(rel) + if err != nil { + return err + } + // Record error, in case f writes. + defer func() { err = errors.Join(err, file.Close()) }() + return f(file) +} diff --git a/mcp/resource_pre_go124.go b/mcp/resource_pre_go124.go new file mode 100644 index 0000000..d1f72ee --- /dev/null +++ b/mcp/resource_pre_go124.go @@ -0,0 +1,25 @@ +// Copyright 2025 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file. + +//go:build !go1.24 + +package mcp + +import ( + "errors" + "os" + "path/filepath" +) + +// withFile calls f on the file at join(dir, rel). +// It does not protect against path traversal attacks. +func withFile(dir, rel string, f func(*os.File) error) (err error) { + file, err := os.Open(filepath.Join(dir, rel)) + if err != nil { + return err + } + // Record error, in case f writes. + defer func() { err = errors.Join(err, file.Close()) }() + return f(file) +} diff --git a/mcp/resource_test.go b/mcp/resource_test.go new file mode 100644 index 0000000..010570a --- /dev/null +++ b/mcp/resource_test.go @@ -0,0 +1,114 @@ +// Copyright 2025 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file. + +package mcp + +import ( + "path/filepath" + "runtime" + "strings" + "testing" +) + +func TestFileRoot(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TODO: fix for Windows") + } + + for _, tt := range []struct { + uri string + want string + wantErr string // error must contain this string + }{ + {uri: "file:///foo", want: "/foo"}, + {uri: "file:///foo/bar", want: "/foo/bar"}, + {uri: "file:///foo/../bar", want: "/bar"}, + {uri: "file:/foo", want: "/foo"}, + {uri: "http:///foo", wantErr: "not a file"}, + {uri: "file://foo", wantErr: "empty path"}, + {uri: ":", wantErr: "missing protocol scheme"}, + } { + got, err := fileRoot(&Root{URI: tt.uri}) + if err != nil { + if tt.wantErr == "" { + t.Errorf("%s: got %v, want success", tt.uri, err) + continue + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("%s: got %v, does not contain %q", tt.uri, err, tt.wantErr) + continue + } + } else if tt.wantErr != "" { + t.Errorf("%s: succeeded, but wanted error with %q", tt.uri, tt.wantErr) + } else if got != tt.want { + t.Errorf("%s: got %q, want %q", tt.uri, got, tt.want) + } + } +} + +func TestComputeURIFilepath(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TODO: fix for Windows") + } + // TODO(jba): test with Windows \\host paths and C: paths + dirFilepath := filepath.FromSlash("/files") + rootFilepaths := []string{ + filepath.FromSlash("/files/public"), + filepath.FromSlash("/files/shared"), + } + for _, tt := range []struct { + uri string + want string + wantErr string // error must contain this + }{ + {"file:///public", "public", ""}, + {"file:///public/file", "public/file", ""}, + {"file:///shared/file", "shared/file", ""}, + {"http:///foo", "", "not a file"}, + {"file://foo", "", "empty"}, + {"file://foo/../bar", "", "localized"}, + {"file:///secret", "", "root"}, + {"file:///secret/file", "", "root"}, + {"file:///private/file", "", "root"}, + } { + t.Run(tt.uri, func(t *testing.T) { + tt.want = filepath.FromSlash(tt.want) // handle Windows + got, gotErr := computeURIFilepath(tt.uri, dirFilepath, rootFilepaths) + if gotErr != nil { + if tt.wantErr == "" { + t.Fatalf("got %v, wanted success", gotErr) + } + if !strings.Contains(gotErr.Error(), tt.wantErr) { + t.Fatalf("got error %v, does not contain %q", gotErr, tt.wantErr) + } + return + } + if tt.wantErr != "" { + t.Fatal("succeeded unexpectedly") + } + if got != tt.want { + t.Errorf("got %q, want %q", got, tt.want) + } + }) + } +} + +func TestReadFileResource(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TODO: fix for Windows") + } + abs, err := filepath.Abs("testdata") + if err != nil { + t.Fatal(err) + } + dirFilepath := filepath.Join(abs, "files") + got, err := readFileResource("file:///info.txt", dirFilepath, nil) + if err != nil { + t.Fatal(err) + } + want := "Contents\n" + if g := string(got); g != want { + t.Errorf("got %q, want %q", g, want) + } +} diff --git a/mcp/server.go b/mcp/server.go index 07396dd..86fbe17 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -10,6 +10,7 @@ import ( "fmt" "iter" "net/url" + "path/filepath" "slices" "sync" @@ -103,34 +104,6 @@ func (s *Server) RemoveTools(names ...string) { } } -// ResourceNotFoundError returns an error indicating that a resource being read could -// not be found. -func ResourceNotFoundError(uri string) error { - return &jsonrpc2.WireError{ - Code: codeResourceNotFound, - Message: "Resource not found", - Data: json.RawMessage(fmt.Sprintf(`{"uri":%q}`, uri)), - } -} - -// The error code to return when a resource isn't found. -// See https://modelcontextprotocol.io/specification/2025-03-26/server/resources#error-handling -// However, the code they chose in in the wrong space -// (see https://github.com/modelcontextprotocol/modelcontextprotocol/issues/509). -// so we pick a different one, arbirarily for now (until they fix it). -// The immediate problem is that jsonprc2 defines -32002 as "server closing". -const codeResourceNotFound = -31002 - -// A ResourceHandler is a function that reads a resource. -// If it cannot find the resource, it should return the result of calling [ResourceNotFoundError]. -type ResourceHandler func(context.Context, *ServerSession, *ReadResourceParams) (*ReadResourceResult, error) - -// A ServerResource associates a Resource with its handler. -type ServerResource struct { - Resource *Resource - Handler ResourceHandler -} - // AddResource adds the given resource to the server and associates it with // a [ResourceHandler], which will be called when the client calls [ClientSession.ReadResource]. // If a resource with the same URI already exists, this one replaces it. @@ -247,6 +220,49 @@ func (s *Server) readResource(ctx context.Context, ss *ServerSession, params *Re return res, nil } +// FileResourceHandler returns a ReadResourceHandler that reads paths using dir as +// a base directory. +// It honors client roots and protects against path traversal attacks. +// +// The dir argument should be a filesystem path. It need not be absolute, but +// that is recommended to avoid a dependency on the current working directory (the +// check against client roots is done with an absolute path). If dir is not absolute +// and the current working directory is unavailable, FileResourceHandler panics. +// +// Lexical path traversal attacks, where the path has ".." elements that escape dir, +// are always caught. Go 1.24 and above also protects against symlink-based attacks, +// where symlinks under dir lead out of the tree. +func (s *Server) FileResourceHandler(dir string) ResourceHandler { + // Convert dir to an absolute path. + dirFilepath, err := filepath.Abs(dir) + if err != nil { + panic(err) + } + return func(ctx context.Context, ss *ServerSession, params *ReadResourceParams) (_ *ReadResourceResult, err error) { + defer func() { + if err != nil { + err = fmt.Errorf("reading resource %s: %w", params.URI, err) + } + }() + + // TODO: use a memoizing API here. + rootRes, err := ss.ListRoots(ctx, nil) + if err != nil { + return nil, fmt.Errorf("listing roots: %w", err) + } + roots, err := fileRoots(rootRes.Roots) + if err != nil { + return nil, err + } + data, err := readFileResource(params.URI, dirFilepath, roots) + if err != nil { + return nil, err + } + // TODO(jba): figure out mime type. + return &ReadResourceResult{Contents: NewBlobResourceContents(params.URI, "text/plain", data)}, nil + } +} + // Run runs the server over the given transport, which must be persistent. // // Run blocks until the client terminates the connection. diff --git a/mcp/testdata/files/info.txt b/mcp/testdata/files/info.txt new file mode 100644 index 0000000..dfe437b --- /dev/null +++ b/mcp/testdata/files/info.txt @@ -0,0 +1 @@ +Contents