diff --git a/internal/tools/workspace.go b/internal/tools/workspace.go index 8b8322b32..a6fe7f8b3 100644 --- a/internal/tools/workspace.go +++ b/internal/tools/workspace.go @@ -1,9 +1,12 @@ package tools import ( + "errors" "fmt" + "io/fs" "os" "path/filepath" + "runtime" "strings" "github.com/Gitlawb/zero/internal/sandbox" @@ -56,6 +59,9 @@ func resolveWorkspacePath(workspaceRoot string, requestedPath string) (string, s } target, err = filepath.EvalSymlinks(target) if err != nil { + if hint := posixRootedPathHint(runtime.GOOS, requestedPath, root, err); hint != "" { + return "", "", errors.New(hint) + } return "", "", err } @@ -72,6 +78,26 @@ func resolveWorkspacePath(workspaceRoot string, requestedPath string) (string, s return target, filepath.ToSlash(relative), nil } +// posixRootedPathHint explains read misses whose requested path is +// POSIX-rooted. On Windows a leading "/" names no drive, so filepath.IsAbs is +// false and "/home/user/repo/file.go" joins onto the workspace root; the raw +// miss that follows ("GetFileAttributesEx \home: ...") names a directory +// the model never meant and invites retrying the same wrong shape. Naming the +// host OS and the workspace root lets one retry correct the shape. A double +// leading slash is excluded: Go treats //server/share/... as an absolute UNC +// path on Windows, so a miss there never joined the workspace root and the +// hint would misstate how the path was resolved. The hint deliberately does +// not echo the requested path: echoing a POSIX fragment next to the workspace +// root trips the no-leak checks that confine out-of-workspace escapes, and +// tool wrappers that name the request add it themselves. Only the message +// changes; the path itself is never rewritten. +func posixRootedPathHint(goos, requestedPath, root string, err error) string { + if goos != "windows" || !strings.HasPrefix(requestedPath, "/") || strings.HasPrefix(requestedPath, "//") || !errors.Is(err, fs.ErrNotExist) { + return "" + } + return fmt.Sprintf("path does not exist on this Windows host: a leading / carries no drive here, so it resolved against the workspace root %s. Use a workspace-relative path or a Windows absolute path", root) +} + func resolveWorkspaceTargetPath(workspaceRoot string, requestedPath string) (string, string, error) { if requestedPath == "" { requestedPath = "." diff --git a/internal/tools/workspace_posix_hint_test.go b/internal/tools/workspace_posix_hint_test.go new file mode 100644 index 000000000..3f77f96c2 --- /dev/null +++ b/internal/tools/workspace_posix_hint_test.go @@ -0,0 +1,127 @@ +package tools + +import ( + "context" + "io/fs" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func TestPosixRootedPathHint(t *testing.T) { + cases := []struct { + name string + goos string + path string + err error + wantHint bool + }{ + { + name: "windows posix-rooted miss is explained", + goos: "windows", + path: "/home/user/repo/main.go", + err: fs.ErrNotExist, + wantHint: true, + }, + { + name: "windows unc-style double slash keeps its raw error", + goos: "windows", + path: "//server/share/missing.go", + err: fs.ErrNotExist, + wantHint: false, + }, + { + name: "linux posix-rooted miss keeps its raw error", + goos: "linux", + path: "/home/user/repo/main.go", + err: fs.ErrNotExist, + wantHint: false, + }, + { + name: "windows relative miss keeps its raw error", + goos: "windows", + path: "src/main.go", + err: fs.ErrNotExist, + wantHint: false, + }, + { + name: "windows posix-rooted permission error keeps its raw error", + goos: "windows", + path: "/home/user/repo/main.go", + err: fs.ErrPermission, + wantHint: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + hint := posixRootedPathHint(tc.goos, tc.path, `C:\ws`, tc.err) + if !tc.wantHint { + if hint != "" { + t.Fatalf("expected no hint, got %q", hint) + } + return + } + if hint == "" { + t.Fatal("expected a hint, got none") + } + if strings.Contains(hint, tc.path) { + t.Errorf("hint must not echo the requested path (no-leak contract for out-of-workspace paths): %q", hint) + } + if !strings.Contains(hint, `C:\ws`) { + t.Errorf("hint does not name the workspace root: %q", hint) + } + if !strings.Contains(hint, "Windows") { + t.Errorf("hint does not name the host OS: %q", hint) + } + }) + } +} + +func TestReadFileToolPosixRootedPathHintOnWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("hint fires only in Windows path resolution") + } + root := t.TempDir() + + result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "/home/user/project/main.go", + }) + + if result.Status != StatusError { + t.Fatalf("expected error status, got %s with output %q", result.Status, result.Output) + } + for _, want := range []string{ + "/home/user/project/main.go", + "does not exist on this Windows host", + root, + "workspace-relative", + } { + if !strings.Contains(result.Output, want) { + t.Errorf("expected output to mention %q, got %q", want, result.Output) + } + } +} + +// A workspace that really does keep sources under home/... must keep reading +// them through a POSIX-rooted request: the hint only replaces the message of a +// miss, it must not turn an existing target into an error. +func TestReadFileToolPosixRootedTwinInsideWorkspaceReadsNormally(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("POSIX-rooted requests join onto the root only on Windows") + } + root := t.TempDir() + writeTestFile(t, filepath.Join(root, "home", "user", "x.go"), "package x\n") + + result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "/home/user/x.go", + }) + + if result.Status != StatusOK { + t.Fatalf("expected ok status for an existing workspace twin, got %s with output %q", result.Status, result.Output) + } + if !strings.Contains(result.Output, "home/user/x.go") { + t.Errorf("expected the workspace-relative path in output, got %q", result.Output) + } +}