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>

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
 ```
 

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 {

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
+}

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)
+}

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)
+}

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)
+	}
+}

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.

mcp/testdata/files/info.txt

@@ -0,0 +1 @@
+Contents