Skip to content

mcp: accept Windows drive letter file roots #1443

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.

Sign up for GitHub

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

Open
akshita317 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

mcp: accept Windows drive letter file roots #1443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d85cdaa
mcp: accept Windows drive letter file roots
akshita317 Oct 23, 2026
c9e6d6b
Merge branch 'main' into mcp-file-roots-windows
guglielmo-san Oct 26, 2026
16a042b
mcp: accept UNC file roots on Windows and unskip the Windows tests
akshita317 Nov 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 59 additions & 60 deletions mcp/mcp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"net/http/httptest"
"net/url"
"path/filepath"
"runtime"
"slices"
"strings"
"sync"
Expand Down Expand Up @@ -168,7 +167,10 @@ func TestEndToEnd(t *testing.T) {
if err != nil {
t.Fatal(err)
}
c.AddRoots(&Root{URI: "file://" + rootAbs})
// A file URI path starts with a slash, which a Windows path such as
// C:\dir does not.
rootURI := &url.URL{Scheme: "file", Path: "/" + strings.TrimPrefix(filepath.ToSlash(rootAbs), "/")}
c.AddRoots(&Root{URI: rootURI.String()})

// Connect the client.
//
Expand Down Expand Up @@ -295,73 +297,70 @@ func TestEndToEnd(t *testing.T) {

// ===== resources =====
t.Log("Testing resources")
//TODO: fix for Windows
if runtime.GOOS != "windows" {
wantResources := []*Resource{resource2, resource1}
lrres, err := cs.ListResources(ctx, nil)
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(wantResources, lrres.Resources); diff != "" {
t.Errorf("resources/list mismatch (-want, +got):\n%s", diff)
}
wantResources := []*Resource{resource2, resource1}
lrres, err := cs.ListResources(ctx, nil)
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(wantResources, lrres.Resources); diff != "" {
t.Errorf("resources/list mismatch (-want, +got):\n%s", diff)
}

template := &ResourceTemplate{
Name: "rt",
MIMEType: "text/template",
URITemplate: "file:///{+filename}", // the '+' means that filename can contain '/'
}
s.AddResourceTemplate(template, readHandler)
tres, err := cs.ListResourceTemplates(ctx, nil)
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff([]*ResourceTemplate{template}, tres.ResourceTemplates); diff != "" {
t.Errorf("resources/list mismatch (-want, +got):\n%s", diff)
}
template := &ResourceTemplate{
Name: "rt",
MIMEType: "text/template",
URITemplate: "file:///{+filename}", // the '+' means that filename can contain '/'
}
s.AddResourceTemplate(template, readHandler)
tres, err := cs.ListResourceTemplates(ctx, nil)
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff([]*ResourceTemplate{template}, tres.ResourceTemplates); diff != "" {
t.Errorf("resources/list mismatch (-want, +got):\n%s", diff)
}

for _, tt := range []struct {
uri string
mimeType string // "": not found; "text/plain": resource; "text/template": template
fail bool // non-nil error returned
}{
{"file:///info.txt", "text/plain", false},
{"file:///fail.txt", "", false},
{"file:///template.txt", "text/template", false},
{"file:///../private.txt", "", true}, // not found: escaping disallowed
} {
rres, err := cs.ReadResource(ctx, &ReadResourceParams{URI: tt.uri})
if err != nil {
if code := errorCode(err); code == CodeResourceNotFound {
if tt.mimeType != "" {
t.Errorf("%s: not found but expected it to be", tt.uri)
}
} else if !tt.fail {
t.Errorf("%s: unexpected error %v", tt.uri, err)
for _, tt := range []struct {
uri string
mimeType string // "": not found; "text/plain": resource; "text/template": template
fail bool // non-nil error returned
}{
{"file:///info.txt", "text/plain", false},
{"file:///fail.txt", "", false},
{"file:///template.txt", "text/template", false},
{"file:///../private.txt", "", true}, // not found: escaping disallowed
} {
rres, err := cs.ReadResource(ctx, &ReadResourceParams{URI: tt.uri})
if err != nil {
if code := errorCode(err); code == CodeResourceNotFound {
if tt.mimeType != "" {
t.Errorf("%s: not found but expected it to be", tt.uri)
}
} else if !tt.fail {
t.Errorf("%s: unexpected error %v", tt.uri, err)
}
} else {
if tt.fail {
t.Errorf("%s: unexpected success", tt.uri)
} else if g, w := len(rres.Contents), 1; g != w {
t.Errorf("got %d contents, wanted %d", g, w)
} else {
if tt.fail {
t.Errorf("%s: unexpected success", tt.uri)
} else if g, w := len(rres.Contents), 1; g != w {
t.Errorf("got %d contents, wanted %d", g, w)
} else {
c := rres.Contents[0]
if got := c.URI; got != tt.uri {
t.Errorf("got uri %q, want %q", got, tt.uri)
}
if got := c.MIMEType; got != tt.mimeType {
t.Errorf("%s: got MIME type %q, want %q", tt.uri, got, tt.mimeType)
}
c := rres.Contents[0]
if got := c.URI; got != tt.uri {
t.Errorf("got uri %q, want %q", got, tt.uri)
}
if got := c.MIMEType; got != tt.mimeType {
t.Errorf("%s: got MIME type %q, want %q", tt.uri, got, tt.mimeType)
}
}
}

s.AddResource(&Resource{URI: "http://U"}, nil)
waitForNotification(t, "resources")
s.RemoveResources("http://U")
waitForNotification(t, "resources")
}

s.AddResource(&Resource{URI: "http://U"}, nil)
waitForNotification(t, "resources")
s.RemoveResources("http://U")
waitForNotification(t, "resources")

// ===== roots =====
t.Log("Testing roots")
{
Expand Down
29 changes: 27 additions & 2 deletions mcp/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"net/url"
"os"
"path/filepath"
"runtime"
"strings"

"github.com/modelcontextprotocol/go-sdk/internal/mcpgodebug"
Expand Down Expand Up @@ -185,14 +186,38 @@ func fileRoot(root *Root) (_ string, err error) {
// 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))
fileRoot := filepath.Clean(filepath.FromSlash(fileURIPath(rurl.Host, rurl.Path, runtime.GOOS)))
if !filepath.IsAbs(fileRoot) {
return "", errors.New("not an absolute path")
}
return fileRoot, nil
}

// fileURIPath returns the path of a file URI with the given host and path in
// the form used by goos. On Windows:
// - The path of a file URI for a drive letter path starts with a slash, as
// in file:///C:/dir (RFC 8089, Appendix E.2), which is dropped so that the
// result is the absolute path C:/dir.
// - A file URI with a host other than localhost names a UNC path, as in
// file://server/share/dir (RFC 8089, Appendix E.3.1), whose result is
// //server/share/dir.
func fileURIPath(host, uriPath, goos string) string {
if goos != "windows" {
return uriPath
}
if host != "" && !strings.EqualFold(host, "localhost") {
return "//" + host + uriPath
}
if len(uriPath) >= 3 && uriPath[0] == '/' && uriPath[2] == ':' && isASCIILetter(uriPath[1]) {
return uriPath[1:]
}
return uriPath

@nahapetyan-serob nahapetyan-serob Nov 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's handle UNC path as well

@akshita317 akshita317 Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 16a042b. On Windows, a host other than localhost is now a UNC path (RFC 8089, Appendix E.3.1): file://server/share/foo gives \server\share\foo, and file://localhost/C:/foo still gives C:\foo. On other systems the host is ignored as before. fileURIPath takes the host now, and TestFileURIPath and TestFileRoot have cases for both.

}

func isASCIILetter(c byte) bool {
return 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z'
}

// Matches reports whether the receiver's uri template matches the uri.
func (sr *serverResourceTemplate) Matches(uri string) bool {
tmpl, err := uritemplate.New(sr.resourceTemplate.URITemplate)
Expand Down
66 changes: 46 additions & 20 deletions mcp/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,37 @@ import (
)

func TestFileRoot(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("TODO: fix for Windows")

@nahapetyan-serob nahapetyan-serob Nov 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 3 other TODO: fix for Windows skips in this file and 1 more in mcp/mcp_test.go. Since roots now work on Windows, could we address and unskip those too?

@akshita317 akshita317 Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 16a042b. All three skips in resource_test.go are gone, and so is the TODO: fix for Windows block in TestEndToEnd. That one was failing on Windows because the test itself built the root as "file://" + rootAbs, which gives file://C:\..., not a file URI. It now builds the URI with net/url. Most of the mcp_test.go diff is re-indentation from removing the if block; it reads better with whitespace hidden.

}

for _, tt := range []struct {
type test 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"},
}
tests := []test{
{uri: "http:///foo", wantErr: "not a file"},
{uri: "file://foo", wantErr: "empty path"},
{uri: ":", wantErr: "missing protocol scheme"},
} {
}
if runtime.GOOS == "windows" {
tests = append(tests,
test{uri: "file:///C:/foo", want: `C:\foo`},
test{uri: "file:///c:/foo/bar", want: `c:\foo\bar`},
test{uri: "file:///C:/foo/../bar", want: `C:\bar`},
test{uri: "file:/C:/foo", want: `C:\foo`},
// VS Code percent-encodes the colon after the drive letter.
test{uri: "file:///c%3A/foo", want: `c:\foo`},
test{uri: "file:///foo", wantErr: "not an absolute path"},
test{uri: "file://localhost/C:/foo", want: `C:\foo`},
test{uri: "file://server/share/foo", want: `\\server\share\foo`},
)
} else {
tests = append(tests,
test{uri: "file:///foo", want: "/foo"},
test{uri: "file:///foo/bar", want: "/foo/bar"},
test{uri: "file:///foo/../bar", want: "/bar"},
test{uri: "file:/foo", want: "/foo"},
)
}
for _, tt := range tests {
got, err := fileRoot(&Root{URI: tt.uri})
if err != nil {
if tt.wantErr == "" {
Expand All @@ -47,10 +61,28 @@ func TestFileRoot(t *testing.T) {
}
}

func TestComputeURIFilepath(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("TODO: fix for Windows")
func TestFileURIPath(t *testing.T) {
for _, tt := range []struct {
host, uriPath, goos, want string
}{
{"", "/C:/foo", "windows", "C:/foo"},
{"", "/c:/foo", "windows", "c:/foo"},
{"", "/C:", "windows", "C:"},
{"", "/foo", "windows", "/foo"},
{"", "/1:/foo", "windows", "/1:/foo"},
{"localhost", "/C:/foo", "windows", "C:/foo"},
{"server", "/share/foo", "windows", "//server/share/foo"},
{"", "/C:/foo", "linux", "/C:/foo"},
{"", "/foo", "linux", "/foo"},
{"server", "/share/foo", "linux", "/share/foo"},
} {
if got := fileURIPath(tt.host, tt.uriPath, tt.goos); got != tt.want {
t.Errorf("fileURIPath(%q, %q, %q) = %q, want %q", tt.host, tt.uriPath, tt.goos, got, tt.want)
}
}
}

func TestComputeURIFilepath(t *testing.T) {
// TODO(jba): test with Windows \\host paths and C: paths
dirFilepath := filepath.FromSlash("/files")
rootFilepaths := []string{
Expand Down Expand Up @@ -95,9 +127,6 @@ func TestComputeURIFilepath(t *testing.T) {
}

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)
Expand Down Expand Up @@ -136,9 +165,6 @@ func TestTemplateMatch(t *testing.T) {
// error. Regression: the message formatted filepath.Localize's error-path
// return value, which is always "", so it never identified the offending path.
func TestComputeURIFilepathLocalizeError(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("TODO: fix for Windows")
}
dirFilepath := filepath.FromSlash("/files")
_, err := computeURIFilepath("file://foo/../bar", dirFilepath, nil)
if err == nil {
Expand Down
Loading