internal/mcp: add the FileResourceHandler method
Provide secure reading of files on the local filesystem.
There are subtleties in path manipulation on Windows that I don't fully
understand, so I defer that work for now.
Change-Id: Ic11f4617dde4ac0b31ccd5b0a772bb2f56db026f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/673196
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/internal/mcp/design/design.md b/internal/mcp/design/design.md
index ebf10e3..fa61960 100644
--- a/internal/mcp/design/design.md
+++ b/internal/mcp/design/design.md
@@ -742,7 +742,7 @@
```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/internal/mcp/mcp_test.go b/internal/mcp/mcp_test.go
index d8304b6..ebd988e 100644
--- a/internal/mcp/mcp_test.go
+++ b/internal/mcp/mcp_test.go
@@ -9,6 +9,8 @@
"context"
"errors"
"fmt"
+ "path/filepath"
+ "runtime"
"slices"
"strings"
"sync"
@@ -83,7 +85,11 @@
}()
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 @@
})
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 @@
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 @@
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 @@
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/internal/mcp/resource.go b/internal/mcp/resource.go
new file mode 100644
index 0000000..2d4bea7
--- /dev/null
+++ b/internal/mcp/resource.go
@@ -0,0 +1,158 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-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"
+
+ jsonrpc2 "golang.org/x/tools/internal/jsonrpc2_v2"
+)
+
+// 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/internal/mcp/resource_go124.go b/internal/mcp/resource_go124.go
new file mode 100644
index 0000000..af4c4f3
--- /dev/null
+++ b/internal/mcp/resource_go124.go
@@ -0,0 +1,29 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-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/internal/mcp/resource_pre_go124.go b/internal/mcp/resource_pre_go124.go
new file mode 100644
index 0000000..77981c5
--- /dev/null
+++ b/internal/mcp/resource_pre_go124.go
@@ -0,0 +1,25 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-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/internal/mcp/resource_test.go b/internal/mcp/resource_test.go
new file mode 100644
index 0000000..28e40eb
--- /dev/null
+++ b/internal/mcp/resource_test.go
@@ -0,0 +1,114 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-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/internal/mcp/server.go b/internal/mcp/server.go
index 24e8f1e..79f4fe0 100644
--- a/internal/mcp/server.go
+++ b/internal/mcp/server.go
@@ -10,6 +10,7 @@
"fmt"
"iter"
"net/url"
+ "path/filepath"
"slices"
"sync"
@@ -103,34 +104,6 @@
}
}
-// 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 @@
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/internal/mcp/testdata/files/info.txt b/internal/mcp/testdata/files/info.txt
new file mode 100644
index 0000000..dfe437b
--- /dev/null
+++ b/internal/mcp/testdata/files/info.txt
@@ -0,0 +1 @@
+Contents
