diff --git a/internal/tools/edit_file.go b/internal/tools/edit_file.go index dc70b01da..6112f9b7c 100644 --- a/internal/tools/edit_file.go +++ b/internal/tools/edit_file.go @@ -150,7 +150,10 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any return okResult("No changes: new_string is identical to old_string.") } editedSpans := replacementByteSpans(content, oldString, newString, replaceAll) - if err := recheckScopedWriteTarget(tool.workspaceRoot, tool.scope, requestedPath); err != nil { + // Recheck the resolved target, not the original argument: a Windows POSIX + // rewrite maps /home///dir/file onto dir/file, and walking the + // original path would miss a symlink swapped into dir before WriteFile. + if err := recheckScopedWriteTarget(tool.workspaceRoot, tool.scope, absolutePath); err != nil { return errorResult("Error writing " + relativePath + ": " + err.Error()) } if err := os.WriteFile(absolutePath, []byte(updated), 0o644); err != nil { diff --git a/internal/tools/posix_windows_path.go b/internal/tools/posix_windows_path.go new file mode 100644 index 000000000..385a4e04b --- /dev/null +++ b/internal/tools/posix_windows_path.go @@ -0,0 +1,315 @@ +package tools + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" +) + +// looksLikePosixAbsolute reports whether path looks like a POSIX absolute +// path (leading "/") rather than a UNC path ("//..."), a Windows volume +// ("C:/..."), or a rooted Windows path (leading backslash). A leading +// backslash is rejected before ToSlash so a path like \tmp\zero\file is +// not treated as POSIX. +func looksLikePosixAbsolute(path string) bool { + raw := strings.TrimSpace(path) + if strings.HasPrefix(raw, `\`) { + return false + } + normalized := filepath.ToSlash(raw) + if normalized == "" { + return false + } + if !strings.HasPrefix(normalized, "/") { + return false + } + if strings.HasPrefix(normalized, "//") { + return false + } + return true +} + +func isDriveLetter(b byte) bool { + return (b >= 'A' && b <= 'Z') || (b >= 'a' && b <= 'z') +} + +// isDriveAbsoluteWindowsPath reports whether path begins with a drive letter and +// a directory separator (e.g. C:\foo or C:/foo). +func isDriveAbsoluteWindowsPath(path string) bool { + raw := strings.TrimSpace(path) + if len(raw) >= 3 && raw[1] == ':' && isDriveLetter(raw[0]) { + return raw[2] == '/' || raw[2] == '\\' + } + return false +} + +// isDriveRelativeWindowsPath reports whether path begins with a drive letter +// without a directory separator (e.g. C:foo or C:), which designates a drive-relative +// path on Windows rather than an absolute path. +func isDriveRelativeWindowsPath(path string) bool { + raw := strings.TrimSpace(path) + if len(raw) >= 2 && raw[1] == ':' && isDriveLetter(raw[0]) { + if len(raw) == 2 || (raw[2] != '/' && raw[2] != '\\') { + return true + } + } + return false +} + +// isAbsForGOOS reports whether path is absolute on goos, independently of the +// host. Non-Windows goos treats a leading "/" as absolute rather than calling +// host filepath.IsAbs. On Windows a POSIX leading "/" is not absolute (no +// volume or UNC), matching Windows filepath.IsAbs, so those paths join onto +// the workspace. +func isAbsForGOOS(goos, path string) bool { + if goos != "windows" { + return strings.HasPrefix(filepath.ToSlash(path), "/") + } + raw := strings.TrimSpace(path) + if raw == "" { + return false + } + n := filepath.ToSlash(raw) + if strings.HasPrefix(n, "//") { + return true + } + if isDriveAbsoluteWindowsPath(raw) { + return true + } + // Volume-relative Windows abs uses a leading backslash, not POSIX "/". + return raw[0] == '\\' +} + +// joinAgainstRoot joins target onto root using goos path rules. Windows +// POSIX-absolute paths are joined as relative (trim leading "/"), because +// host filepath.Join on Unix would treat them as absolute and drop root. +func joinAgainstRoot(goos, root, target string) string { + if isAbsForGOOS(goos, target) { + return target + } + if goos == "windows" && looksLikePosixAbsolute(target) { + rel := strings.Trim(filepath.ToSlash(target), "/") + if rel == "" { + return root + } + return filepath.Join(root, filepath.FromSlash(rel)) + } + return filepath.Join(root, target) +} + +func workspaceBasename(workspaceRoot string) string { + repo := filepath.Base(filepath.Clean(workspaceRoot)) + switch repo { + case "", ".", "..", string(filepath.Separator): + return "" + default: + return repo + } +} + +func posixPathSegments(path string) []string { + normalized := strings.TrimSpace(filepath.ToSlash(path)) + normalized = strings.TrimRight(normalized, "/") + if normalized == "" { + return nil + } + normalized = strings.TrimPrefix(normalized, "/") + if normalized == "" { + return nil + } + raw := strings.Split(normalized, "/") + parts := make([]string, 0, len(raw)) + for _, part := range raw { + if part == "" { + continue + } + parts = append(parts, part) + } + return parts +} + +func restAfterPrefix(parts []string, prefixLen int) (string, bool) { + if prefixLen > len(parts) { + return "", false + } + rest := parts[prefixLen:] + if len(rest) == 0 { + return ".", true + } + joined := strings.Join(rest, "/") + if strings.HasPrefix(joined, "/") { + return "", false + } + return joined, true +} + +func matchSyntheticHomePrefix(parts []string, lead, repo string) (string, bool) { + // /home///rest or /Users///rest + if len(parts) < 3 || lead == "" || repo == "" { + return "", false + } + if parts[0] != lead { + return "", false + } + user := parts[1] + if user == "" || user == "." || user == ".." { + return "", false + } + if !strings.EqualFold(parts[2], repo) { + return "", false + } + return restAfterPrefix(parts, 3) +} + +func matchSyntheticDirPrefix(parts []string, lead []string, repo string) (string, bool) { + // /tmp//rest or /var/tmp//rest + if repo == "" || len(parts) < len(lead)+1 { + return "", false + } + for i, segment := range lead { + if parts[i] != segment { + return "", false + } + } + if !strings.EqualFold(parts[len(lead)], repo) { + return "", false + } + return restAfterPrefix(parts, len(lead)+1) +} + +// stripSyntheticPosixWorkspacePrefix strips a known synthetic POSIX workspace +// prefix when it includes the workspace basename. It does not use a naive +// index of "/"+repo+"/" (a workspace named "home" would mis-strip +// /home/user/file). Callers are Windows-only; rest may contain ".." and is +// still subject to resolveWorkspacePath confinement. +func stripSyntheticPosixWorkspacePrefix(workspaceRoot, requested string) (string, bool) { + if !looksLikePosixAbsolute(requested) { + return requested, false + } + repo := workspaceBasename(workspaceRoot) + if repo == "" { + return requested, false + } + parts := posixPathSegments(requested) + if rest, ok := matchSyntheticHomePrefix(parts, "home", repo); ok { + return rest, true + } + if rest, ok := matchSyntheticHomePrefix(parts, "Users", repo); ok { + return rest, true + } + if rest, ok := matchSyntheticDirPrefix(parts, []string{"tmp"}, repo); ok { + return rest, true + } + if rest, ok := matchSyntheticDirPrefix(parts, []string{"var", "tmp"}, repo); ok { + return rest, true + } + return requested, false +} + +// rewritePosixWorkspacePath is a Windows-only rewrite of synthetic POSIX +// prefixes (/home//, /Users//, /tmp/, +// /var/tmp/) when they include the workspace basename. It does not +// rewrite real Windows absolute paths and does not invent files. Callers +// must keep the original argument when existingLiteralPosixWorkspacePath +// is true so an on-disk file at the literal join is not shadowed. +func rewritePosixWorkspacePath(goos, workspaceRoot, requested string) string { + if goos != "windows" { + return requested + } + if stripped, ok := stripSyntheticPosixWorkspacePrefix(workspaceRoot, requested); ok { + return stripped + } + return requested +} + +// existingLiteralPosixWorkspacePath reports whether the un-rewritten POSIX +// path already names an existing file or directory inside the workspace. When +// it does (or its immediate parent directory exists on disk), the rewrite must +// not steal the request: a model that named /tmp//x when that file or its +// parent directory exists should read and write that file, not a different x at +// the workspace root. +func existingLiteralPosixWorkspacePath(goos, workspaceRoot, requested string) bool { + if goos != "windows" { + return false + } + if rewritePosixWorkspacePath(goos, workspaceRoot, requested) == requested { + return false + } + root, err := filepath.Abs(workspaceRoot) + if err != nil { + return false + } + root, err = filepath.EvalSymlinks(root) + if err != nil { + return false + } + target := joinAgainstRoot(goos, root, requested) + target, err = filepath.Abs(target) + if err != nil { + return false + } + if _, err := workspaceRelative(root, target, requested); err != nil { + return false + } + if _, err := os.Lstat(target); err == nil { + return true + } + parent := filepath.Dir(target) + if parent != root && len(parent) > len(root) { + if fi, err := os.Lstat(parent); err == nil && fi.IsDir() { + return true + } + } + return false +} + +func isMissingPathError(err error) bool { + if err == nil { + return false + } + if os.IsNotExist(err) { + return true + } + var pathErr *os.PathError + if errors.As(err, &pathErr) && pathErr != nil && os.IsNotExist(pathErr) { + return true + } + return false +} + +// annotatePosixWindowsPathError appends an actionable hint when a Windows host +// fails to find a path that looks POSIX-absolute. Confinement failures +// (outsideWorkspaceError) are left unchanged — those messages are already +// actionable. The requested path is the original argument so the hint names +// what the model passed, even if a synthetic prefix was already stripped. +// The hint does not name the workspace root: a POSIX path such as /etc/passwd +// joins into the workspace as a missing file, and naming the root would leak +// it next to the requested path. +func annotatePosixWindowsPathError(goos, workspaceRoot, requested string, err error) error { + if goos != "windows" || err == nil { + return err + } + if !looksLikePosixAbsolute(requested) { + return err + } + if !isMissingPathError(err) { + return err + } + return fmt.Errorf("%w; host is Windows and %q looks like a POSIX absolute path; use a workspace-relative path or a Windows path", redactPathErrorWorkspaceRoot(err, requested), requested) +} + +// redactPathErrorWorkspaceRoot replaces PathError.Path with the original +// request so a missing POSIX path such as /etc/passwd cannot echo the +// joined workspace root next to the hint. The PathError.Err value is kept +// so errors.Is(..., os.ErrNotExist) still holds. +func redactPathErrorWorkspaceRoot(err error, requested string) error { + var pathErr *os.PathError + if !errors.As(err, &pathErr) || pathErr == nil { + return err + } + redacted := *pathErr + redacted.Path = requested + return &redacted +} diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go new file mode 100644 index 000000000..acdbcbbc6 --- /dev/null +++ b/internal/tools/posix_windows_path_test.go @@ -0,0 +1,718 @@ +package tools + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestLooksLikePosixAbsolute(t *testing.T) { + tests := []struct { + path string + want bool + }{ + {path: "/home/x", want: true}, + {path: "C:\\foo", want: false}, + {path: "C:/foo", want: false}, + {path: "//unc/share", want: false}, + {path: "relative/path", want: false}, + {path: "", want: false}, + {path: " /tmp/x ", want: true}, + {path: "/tmp/zero/file", want: true}, + {path: `\Windows\System32`, want: false}, + {path: `\tmp\zero\file`, want: false}, + } + for _, tt := range tests { + if got := looksLikePosixAbsolute(tt.path); got != tt.want { + t.Fatalf("looksLikePosixAbsolute(%q) = %v, want %v", tt.path, got, tt.want) + } + } +} + +func TestRewritePosixWorkspacePath(t *testing.T) { + workspace := filepath.Join("workspaces", "zero") + tests := []struct { + name string + goos string + workspace string + requested string + want string + }{ + { + name: "windows home repo file", + goos: "windows", + workspace: workspace, + requested: "/home/alice/zero/internal/tools/read_file.go", + want: "internal/tools/read_file.go", + }, + { + name: "windows home repo root", + goos: "windows", + workspace: workspace, + requested: "/home/alice/zero", + want: ".", + }, + { + name: "windows mac-style users prefix", + goos: "windows", + workspace: workspace, + requested: "/Users/alice/zero/pkg/x.go", + want: "pkg/x.go", + }, + { + name: "windows tmp repo file", + goos: "windows", + workspace: workspace, + requested: "/tmp/zero/foo.txt", + want: "foo.txt", + }, + { + name: "windows tmp without repo stays", + goos: "windows", + workspace: workspace, + requested: "/tmp/foo.txt", + want: "/tmp/foo.txt", + }, + { + name: "windows foreign repo stays", + goos: "windows", + workspace: workspace, + requested: "/home/alice/otherrepo/file.go", + want: "/home/alice/otherrepo/file.go", + }, + { + name: "linux does not rewrite", + goos: "linux", + workspace: workspace, + requested: "/home/alice/zero/file.go", + want: "/home/alice/zero/file.go", + }, + { + name: "windows var tmp repo file", + goos: "windows", + workspace: workspace, + requested: "/var/tmp/zero/foo.txt", + want: "foo.txt", + }, + { + name: "workspace named home does not naive-strip", + goos: "windows", + workspace: filepath.Join("workspaces", "home"), + requested: "/home/user/file", + want: "/home/user/file", + }, + { + name: "windows home repo basename case differs", + goos: "windows", + workspace: filepath.Join("workspaces", "Zero"), + requested: "/home/alice/zero/go.mod", + want: "go.mod", + }, + { + name: "windows tmp repo basename case differs", + goos: "windows", + workspace: filepath.Join("workspaces", "Zero"), + requested: "/tmp/zero/foo.txt", + want: "foo.txt", + }, + { + name: "windows posix tmp file still rewrites", + goos: "windows", + workspace: workspace, + requested: "/tmp/zero/file", + want: "file", + }, + { + name: "windows tmp double slash rewrites", + goos: "windows", + workspace: workspace, + requested: "/tmp/zero//file", + want: "file", + }, + { + name: "windows rooted backslash system32 stays", + goos: "windows", + workspace: workspace, + requested: `\Windows\System32`, + want: `\Windows\System32`, + }, + { + name: "windows rooted tmp backslash stays", + goos: "windows", + workspace: workspace, + requested: `\tmp\zero\file`, + want: `\tmp\zero\file`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := rewritePosixWorkspacePath(tt.goos, tt.workspace, tt.requested) + if got != tt.want { + t.Fatalf("rewritePosixWorkspacePath(%q, %q, %q) = %q, want %q", tt.goos, tt.workspace, tt.requested, got, tt.want) + } + }) + } +} + +func TestAnnotatePosixWindowsPathError(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + missing := &os.PathError{Op: "stat", Path: "/tmp/does-not-exist-xyz", Err: os.ErrNotExist} + + got := annotatePosixWindowsPathError("windows", root, "/tmp/does-not-exist-xyz", missing) + if got == nil { + t.Fatal("expected wrapped missing-path error") + } + if !errors.Is(got, os.ErrNotExist) { + t.Fatalf("wrapped error should unwrap to os.ErrNotExist, got %v", got) + } + msg := got.Error() + if !strings.Contains(msg, "Windows") { + t.Fatalf("hint missing Windows host: %q", msg) + } + if !strings.Contains(msg, "POSIX") { + t.Fatalf("hint missing POSIX path wording: %q", msg) + } + errorMustNotNameWorkspaceRoot(t, msg, root) + if strings.Contains(msg, "workspace root") { + t.Fatalf("hint must not name the workspace root: %q", msg) + } + + etcMiss := &os.PathError{Op: "stat", Path: "/etc/passwd", Err: os.ErrNotExist} + got = annotatePosixWindowsPathError("windows", root, "/etc/passwd", etcMiss) + if got == nil { + t.Fatal("expected wrapped missing-path error for /etc/passwd") + } + msg = got.Error() + if !strings.Contains(msg, "/etc/passwd") { + t.Fatalf("hint should name the requested POSIX path: %q", msg) + } + errorMustNotNameWorkspaceRoot(t, msg, root) + + // The real resolver's PathError.Path is the joined workspace path, not the + // POSIX argument. Wrapping that PathError would echo the root next to %q. + joinedEtc := filepath.Join(root, "etc", "passwd") + joinedMiss := &os.PathError{Op: "GetFileAttributesEx", Path: joinedEtc, Err: os.ErrNotExist} + got = annotatePosixWindowsPathError("windows", root, "/etc/passwd", joinedMiss) + if got == nil { + t.Fatal("expected wrapped missing-path error for joined /etc/passwd") + } + msg = got.Error() + if !strings.Contains(msg, "/etc/passwd") { + t.Fatalf("hint should name the requested POSIX path: %q", msg) + } + errorMustNotNameWorkspaceRoot(t, msg, root) + + if got := annotatePosixWindowsPathError("linux", root, "/tmp/does-not-exist-xyz", missing); got != missing { + t.Fatalf("linux should not wrap missing-path errors, got %v", got) + } + + confine := outsideWorkspaceError("/home/alice/zero/../secret") + if got := annotatePosixWindowsPathError("windows", root, "/home/alice/zero/../secret", confine); got != confine { + t.Fatalf("confinement errors must not be wrapped, got %v", got) + } + + if got := annotatePosixWindowsPathError("windows", root, "/tmp/x", nil); got != nil { + t.Fatalf("nil error should stay nil, got %v", got) + } + + relativeMiss := &os.PathError{Op: "stat", Path: "notes.txt", Err: os.ErrNotExist} + if got := annotatePosixWindowsPathError("windows", root, "notes.txt", relativeMiss); got != relativeMiss { + t.Fatalf("relative paths should not be annotated, got %v", got) + } +} + +func TestReadFileToolRewritesSyntheticPosixPrefixOnWindows(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + writeTestFile(t, filepath.Join(root, "notes.txt"), "hello from notes\n") + + absolute, relative, err := resolveWorkspacePathForGOOS("windows", root, "/home/alice/zero/notes.txt") + if err != nil { + t.Fatalf("resolve rewritten posix path: %v", err) + } + if relative != "notes.txt" { + t.Fatalf("relative = %q, want notes.txt", relative) + } + got, err := os.ReadFile(absolute) + if err != nil { + t.Fatalf("read resolved path: %v", err) + } + if string(got) != "hello from notes\n" { + t.Fatalf("content = %q, want %q", got, "hello from notes\n") + } +} + +func TestResolveWorkspacePathAnnotatesPosixMissOnWindows(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + + _, _, err := resolveWorkspacePathForGOOS("windows", root, "/tmp/does-not-exist-xyz") + if err == nil { + t.Fatal("expected missing-path error") + } + msg := err.Error() + if !strings.Contains(msg, "Windows") { + t.Fatalf("error missing Windows host hint: %q", msg) + } + if strings.Contains(msg, "workspace root") { + t.Fatalf("hint must not name the workspace root: %q", msg) + } + errorMustNotNameWorkspaceRoot(t, msg, root) + if strings.Contains(msg, "must stay inside the workspace") { + t.Fatalf("POSIX miss used confinement instead of a missing-path hint: %q", msg) + } + + _, _, err = resolveWorkspacePathForGOOS("windows", root, "/etc/passwd") + if err == nil { + t.Fatal("expected missing-path error for /etc/passwd") + } + msg = err.Error() + if !strings.Contains(msg, "/etc/passwd") { + t.Fatalf("hint should name the requested POSIX path: %q", msg) + } + errorMustNotNameWorkspaceRoot(t, msg, root) +} + +func TestResolveWorkspacePathDoesNotRewriteForeignRepo(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + writeTestFile(t, filepath.Join(root, "x"), "workspace x") + + requested := "/home/alice/otherrepo/x" + if got := rewritePosixWorkspacePath("windows", root, requested); got != requested { + t.Fatalf("rewrote foreign repo path %q to %q", requested, got) + } + + _, relative, err := resolveWorkspacePathForGOOS("windows", root, requested) + if err == nil { + t.Fatal("foreign repo path should not resolve to a workspace file") + } + if relative == "x" { + t.Fatalf("foreign repo path was treated as workspace file x") + } +} + +func TestResolveWorkspacePathRejectsMissingLexicalEscapeOnWindows(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + + _, _, err := resolveWorkspacePathForGOOS("windows", root, "/home/alice/zero/../../missing") + if err == nil { + t.Fatal("expected confinement error for a missing escaped target") + } + msg := err.Error() + if !strings.Contains(msg, "must stay inside the workspace") { + t.Fatalf("expected outsideWorkspaceError, got %q", msg) + } + if strings.Contains(msg, "host is Windows") { + t.Fatalf("missing lexical escape got a POSIX-path hint instead of confinement: %q", msg) + } +} + +func TestResolveWorkspaceTargetPathRewritesMissingTmpFile(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + + _, relative, err := resolveWorkspaceTargetPathForGOOS("windows", root, "/tmp/zero/new.txt") + if err != nil { + t.Fatalf("missing rewritten write target should resolve: %v", err) + } + if relative != "new.txt" { + t.Fatalf("relative = %q, want new.txt", relative) + } +} + +func TestResolveWorkspacePathRewritesDoubleSlashTmpFile(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + writeTestFile(t, filepath.Join(root, "file"), "workspace file") + + _, relative, err := resolveWorkspacePathForGOOS("windows", root, "/tmp/zero//file") + if err != nil { + t.Fatalf("double-slash POSIX tmp path should resolve: %v", err) + } + if relative != "file" { + t.Fatalf("relative = %q, want file", relative) + } +} + +func TestResolveWorkspaceTargetPathRejectsLexicalEscape(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + + _, _, err := resolveWorkspaceTargetPathForGOOS("windows", root, "/home/alice/zero/../../new.txt") + if err == nil { + t.Fatal("expected confinement error for an escaped write target") + } + msg := err.Error() + if !strings.Contains(msg, "must stay inside the workspace") { + t.Fatalf("expected outsideWorkspaceError, got %q", msg) + } +} + +func TestIsAbsForGOOS(t *testing.T) { + tests := []struct { + goos string + path string + want bool + }{ + {goos: "windows", path: "/tmp/x", want: false}, + {goos: "linux", path: "/tmp/x", want: true}, + {goos: "windows", path: "C:/foo", want: true}, + {goos: "windows", path: "C:\\foo", want: true}, + {goos: "windows", path: "//unc/share", want: true}, + {goos: "windows", path: "relative/path", want: false}, + {goos: "windows", path: "\\Windows\\System32", want: true}, + {goos: "windows", path: "", want: false}, + {goos: "windows", path: " ", want: false}, + } + for _, tt := range tests { + if got := isAbsForGOOS(tt.goos, tt.path); got != tt.want { + t.Fatalf("isAbsForGOOS(%q, %q) = %v, want %v", tt.goos, tt.path, got, tt.want) + } + } +} + +func TestResolveWorkspacePathRejectsRootedWindowsBackslash(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + writeTestFile(t, filepath.Join(root, "file"), "workspace file") + + for _, requested := range []string{`\Windows\System32`, `\tmp\zero\file`} { + if got := rewritePosixWorkspacePath("windows", root, requested); got != requested { + t.Fatalf("rooted Windows path %q rewrote to %q", requested, got) + } + + _, relative, err := resolveWorkspacePathForGOOS("windows", root, requested) + if relative == "file" { + t.Fatalf("rooted Windows path %q resolved as workspace-relative file (err=%v)", requested, err) + } + if err == nil { + t.Fatalf("rooted Windows path %q should not resolve inside the workspace (relative=%q)", requested, relative) + } + if !strings.Contains(err.Error(), "must stay inside the workspace") { + t.Fatalf("rooted Windows path %q: expected confinement, got %q", requested, err) + } + + _, relative, err = resolveWorkspaceTargetPathForGOOS("windows", root, requested) + if relative == "file" { + t.Fatalf("write resolver treated %q as workspace-relative file (err=%v)", requested, err) + } + if err == nil { + t.Fatalf("write resolver accepted rooted Windows path %q (relative=%q)", requested, relative) + } + if !strings.Contains(err.Error(), "must stay inside the workspace") { + t.Fatalf("write resolver for %q: expected confinement, got %q", requested, err) + } + } + + if got := rewritePosixWorkspacePath("windows", root, "/tmp/zero/file"); got != "file" { + t.Fatalf("POSIX /tmp/zero/file should still rewrite to file, got %q", got) + } +} + +func TestJoinAgainstRootWindowsPosix(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + got := joinAgainstRoot("windows", root, "/tmp/does-not-exist-xyz") + want := filepath.Join(root, "tmp", "does-not-exist-xyz") + if got != want { + t.Fatalf("joinAgainstRoot(windows, root, /tmp/does-not-exist-xyz) = %q, want %q", got, want) + } + got = joinAgainstRoot("linux", root, "/tmp/does-not-exist-xyz") + if got != filepath.Clean("/tmp/does-not-exist-xyz") && got != "/tmp/does-not-exist-xyz" { + t.Fatalf("joinAgainstRoot(linux, ...) should keep POSIX abs, got %q", got) + } +} + +func TestResolveWorkspacePathPrefersExistingLiteralOverRewrite(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + writeTestFile(t, filepath.Join(root, "tmp", "zero", "only.md"), "literal\n") + writeTestFile(t, filepath.Join(root, "only.md"), "rewritten\n") + + absolute, relative, err := resolveWorkspacePathForGOOS("windows", root, "/tmp/zero/only.md") + if err != nil { + t.Fatalf("existing literal POSIX path should resolve: %v", err) + } + wantRel := filepath.ToSlash(filepath.Join("tmp", "zero", "only.md")) + if relative != wantRel { + t.Fatalf("relative = %q, want %q (literal must win over rewrite)", relative, wantRel) + } + got, err := os.ReadFile(absolute) + if err != nil { + t.Fatalf("read resolved path: %v", err) + } + if string(got) != "literal\n" { + t.Fatalf("content = %q, want literal file, not rewritten workspace-root file", got) + } +} + +func TestResolveWorkspaceTargetPathPrefersExistingLiteralOverRewrite(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + writeTestFile(t, filepath.Join(root, "tmp", "zero", "only.md"), "literal\n") + + absolute, relative, err := resolveWorkspaceTargetPathForGOOS("windows", root, "/tmp/zero/only.md") + if err != nil { + t.Fatalf("existing literal write target should resolve: %v", err) + } + wantRel := filepath.ToSlash(filepath.Join("tmp", "zero", "only.md")) + if relative != wantRel { + t.Fatalf("relative = %q, want %q (literal must win over rewrite)", relative, wantRel) + } + got, err := os.ReadFile(absolute) + if err != nil { + t.Fatalf("read resolved write target: %v", err) + } + if string(got) != "literal\n" { + t.Fatalf("content = %q, want literal file", got) + } +} + +func TestResolveWorkspaceTargetPathPrefersExistingLiteralDirectoryOverRewrite(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + literalDir := filepath.Join(root, "tmp", "zero") + if err := os.MkdirAll(literalDir, 0o755); err != nil { + t.Fatalf("mkdir literal dir: %v", err) + } + + absolute, relative, err := resolveWorkspaceTargetPathForGOOS("windows", root, "/tmp/zero/new.md") + if err != nil { + t.Fatalf("write target in existing literal directory should resolve: %v", err) + } + wantRel := filepath.ToSlash(filepath.Join("tmp", "zero", "new.md")) + if relative != wantRel { + t.Fatalf("relative = %q, want %q (existing literal directory must win over rewrite)", relative, wantRel) + } + wantAbs := filepath.Join(root, "tmp", "zero", "new.md") + if absolute != wantAbs { + t.Fatalf("absolute = %q, want %q", absolute, wantAbs) + } +} + +func TestRecheckWorkspaceWriteTargetAfterPosixRewrite(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + dir := filepath.Join(root, "dir") + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatalf("mkdir dir: %v", err) + } + + original := "/home/alice/zero/dir/file" + if got := rewritePosixWorkspacePath("windows", root, original); got != filepath.ToSlash(filepath.Join("dir", "file")) && got != "dir/file" { + t.Fatalf("rewritePosixWorkspacePath(%q) = %q, want dir/file", original, got) + } + + absolute, relative, err := resolveWorkspaceTargetPathForGOOS("windows", root, original) + if err != nil { + t.Fatalf("resolve rewritten write target: %v", err) + } + if relative != "dir/file" { + t.Fatalf("relative = %q, want dir/file", relative) + } + + outside := t.TempDir() + if err := os.Remove(dir); err != nil { + t.Fatalf("remove dir: %v", err) + } + if err := os.Symlink(outside, dir); err != nil { + t.Skipf("symlink unavailable: %v", err) + } + + joinedOriginal := joinAgainstRoot("windows", root, original) + if err := recheckWorkspaceWriteTarget(root, joinedOriginal); err != nil { + t.Fatalf("recheck of the original POSIX join %q should miss (path does not exist), got %v", joinedOriginal, err) + } + if err := recheckWorkspaceWriteTarget(root, absolute); err == nil { + t.Fatal("recheck of the resolved write target must see the swapped symlink") + } else if !strings.Contains(err.Error(), "must not traverse symlink") { + t.Fatalf("expected symlink rejection, got %q", err) + } + + // Also verify with an outer symlinked parent directory (mirroring macOS /var -> /private/var). + aliasParent := filepath.Join(t.TempDir(), "parent-alias") + if err := os.Symlink(filepath.Dir(root), aliasParent); err != nil { + t.Skipf("symlink unavailable: %v", err) + } + aliasRoot := filepath.Join(aliasParent, filepath.Base(root)) + aliasJoined := joinAgainstRoot("windows", aliasRoot, original) + if err := recheckWorkspaceWriteTarget(aliasRoot, aliasJoined); err != nil { + t.Fatalf("recheck with alias root %q should miss (path does not exist), got %v", aliasJoined, err) + } + aliasAbsolute := filepath.Join(aliasRoot, "dir", "file") + if err := recheckWorkspaceWriteTarget(aliasRoot, aliasAbsolute); err == nil { + t.Fatal("recheck with alias root must see the swapped symlink") + } else if !strings.Contains(err.Error(), "must not traverse symlink") { + t.Fatalf("expected symlink rejection with alias root, got %q", err) + } +} + +func workspaceRootSpellings(t *testing.T, root string) []string { + t.Helper() + seen := make(map[string]bool) + var out []string + add := func(path string) { + if path == "" || seen[path] { + return + } + seen[path] = true + out = append(out, path) + } + add(root) + abs, err := filepath.Abs(root) + if err != nil { + return out + } + add(abs) + resolved, err := filepath.EvalSymlinks(abs) + if err == nil { + add(resolved) + } + return out +} + +func errorMustNotNameWorkspaceRoot(t *testing.T, msg, root string) { + t.Helper() + for _, spelling := range workspaceRootSpellings(t, root) { + if strings.Contains(msg, spelling) { + t.Fatalf("error names workspace root %q: %q", spelling, msg) + } + } +} + +func TestWindowsPathCategoriesResolution(t *testing.T) { + root := filepath.Join(t.TempDir(), "zero") + if err := os.MkdirAll(filepath.Join(root, "sub"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "file.txt"), []byte("ok"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "sub", "child.txt"), []byte("ok"), 0o644); err != nil { + t.Fatal(err) + } + + // 1. Drive-relative paths must be rejected without retargeting. + driveRelativeCases := []string{ + "C:file.txt", + "C:", + "C:sub/child.txt", + "C:sub\\child.txt", + "d:other.txt", + } + for _, p := range driveRelativeCases { + t.Run("drive-relative "+p, func(t *testing.T) { + _, _, err := resolveWorkspacePathForGOOS("windows", root, p) + if err == nil { + t.Fatalf("expected error for drive-relative path %q, got nil", p) + } + if !strings.Contains(err.Error(), "must stay inside the workspace") { + t.Fatalf("expected outside workspace error for %q, got %v", p, err) + } + + _, _, err = resolveWorkspaceTargetPathForGOOS("windows", root, p) + if err == nil { + t.Fatalf("expected target error for drive-relative path %q, got nil", p) + } + if !strings.Contains(err.Error(), "must stay inside the workspace") { + t.Fatalf("expected outside workspace error for target %q, got %v", p, err) + } + }) + } + + // 2. Ordinary relative paths must resolve within root. + relCases := []struct { + input string + wantRel string + }{ + {"", "."}, + {"file.txt", "file.txt"}, + {"./file.txt", "file.txt"}, + {"sub/child.txt", "sub/child.txt"}, + {filepath.Join("sub", "child.txt"), "sub/child.txt"}, + } + for _, tc := range relCases { + t.Run("relative "+tc.input, func(t *testing.T) { + target, rel, err := resolveWorkspacePathForGOOS("windows", root, tc.input) + if err != nil { + t.Fatalf("resolve error for %q: %v", tc.input, err) + } + if rel != tc.wantRel { + t.Fatalf("rel = %q, want %q", rel, tc.wantRel) + } + if target != filepath.Join(root, filepath.FromSlash(tc.wantRel)) { + t.Fatalf("target = %q, want joined path", target) + } + }) + } + + // 3. Synthetic POSIX path including workspace basename. + posixCases := []struct { + input string + wantRel string + }{ + {"/home/alice/zero/file.txt", "file.txt"}, + {"/Users/alice/zero/sub/child.txt", "sub/child.txt"}, + {"/tmp/zero/file.txt", "file.txt"}, + {"/var/tmp/zero/sub/child.txt", "sub/child.txt"}, + } + for _, tc := range posixCases { + t.Run("synthetic posix "+tc.input, func(t *testing.T) { + target, rel, err := resolveWorkspacePathForGOOS("windows", root, tc.input) + if err != nil { + t.Fatalf("resolve error for %q: %v", tc.input, err) + } + if rel != tc.wantRel { + t.Fatalf("rel = %q, want %q", rel, tc.wantRel) + } + if target != filepath.Join(root, filepath.FromSlash(tc.wantRel)) { + t.Fatalf("target = %q, want joined path", target) + } + }) + } + + // 4. UNC / rooted current-drive / outside absolute paths must be rejected. + outsideCases := []string{ + "//unc-server/share/file.txt", + `\\unc-server\share\file.txt`, + `\Windows\System32\cmd.exe`, + } + for _, p := range outsideCases { + t.Run("outside "+p, func(t *testing.T) { + _, _, err := resolveWorkspacePathForGOOS("windows", root, p) + if err == nil { + t.Fatalf("expected error for outside path %q, got nil", p) + } + }) + } +} diff --git a/internal/tools/workspace.go b/internal/tools/workspace.go index 8b8322b32..65e5369d8 100644 --- a/internal/tools/workspace.go +++ b/internal/tools/workspace.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "github.com/Gitlawb/zero/internal/sandbox" @@ -32,39 +33,58 @@ func normalizeWorkspaceRoot(workspaceRoot string) string { } func resolveWorkspacePath(workspaceRoot string, requestedPath string) (string, string, error) { + return resolveWorkspacePathForGOOS(runtime.GOOS, workspaceRoot, requestedPath) +} + +func resolveWorkspacePathForGOOS(goos, workspaceRoot, requestedPath string) (string, string, error) { + original := requestedPath + fail := func(err error) (string, string, error) { + return "", "", annotatePosixWindowsPathError(goos, workspaceRoot, original, err) + } + + if goos == "windows" && isDriveRelativeWindowsPath(requestedPath) { + return fail(outsideWorkspaceError(original)) + } + + // Windows-only rewrite of synthetic POSIX prefixes that include the + // workspace basename. Keep the literal path when that join already + // exists so the rewrite cannot shadow an on-disk file. + if !existingLiteralPosixWorkspacePath(goos, workspaceRoot, requestedPath) { + requestedPath = rewritePosixWorkspacePath(goos, workspaceRoot, requestedPath) + } + if requestedPath == "" { requestedPath = "." } root, err := filepath.Abs(workspaceRoot) if err != nil { - return "", "", err + return fail(err) } root, err = filepath.EvalSymlinks(root) if err != nil { - return "", "", err + return fail(err) } - target := requestedPath - if !filepath.IsAbs(target) { - target = filepath.Join(root, target) - } + target := joinAgainstRoot(goos, root, requestedPath) target, err = filepath.Abs(target) if err != nil { - return "", "", err + return fail(err) + } + // Reject lexical escapes before EvalSymlinks so a missing ../../target + // returns outsideWorkspaceError instead of a POSIX-path miss hint. + if _, err := workspaceRelative(root, target, requestedPath); err != nil { + return fail(err) } target, err = filepath.EvalSymlinks(target) if err != nil { - return "", "", err + return fail(err) } - relative, err := filepath.Rel(root, target) + relative, err := workspaceRelative(root, target, requestedPath) if err != nil { - return "", "", outsideWorkspaceError(requestedPath) - } - if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) { - return "", "", outsideWorkspaceError(requestedPath) + return fail(err) } if relative == "." { return target, ".", nil @@ -73,29 +93,49 @@ func resolveWorkspacePath(workspaceRoot string, requestedPath string) (string, s } func resolveWorkspaceTargetPath(workspaceRoot string, requestedPath string) (string, string, error) { + return resolveWorkspaceTargetPathForGOOS(runtime.GOOS, workspaceRoot, requestedPath) +} + +func resolveWorkspaceTargetPathForGOOS(goos, workspaceRoot, requestedPath string) (string, string, error) { + original := requestedPath + fail := func(err error) (string, string, error) { + return "", "", annotatePosixWindowsPathError(goos, workspaceRoot, original, err) + } + + if goos == "windows" && isDriveRelativeWindowsPath(requestedPath) { + return fail(outsideWorkspaceError(original)) + } + + // Windows-only rewrite of synthetic POSIX prefixes that include the + // workspace basename. Keep the literal path when that join already + // exists so a write cannot retarget an on-disk file to the workspace root. + if !existingLiteralPosixWorkspacePath(goos, workspaceRoot, requestedPath) { + requestedPath = rewritePosixWorkspacePath(goos, workspaceRoot, requestedPath) + } + if requestedPath == "" { requestedPath = "." } root, err := filepath.Abs(workspaceRoot) if err != nil { - return "", "", err + return fail(err) } root, err = filepath.EvalSymlinks(root) if err != nil { - return "", "", err + return fail(err) } - target := requestedPath - if !filepath.IsAbs(target) { - target = filepath.Join(root, target) - } + target := joinAgainstRoot(goos, root, requestedPath) target, err = filepath.Abs(target) if err != nil { - return "", "", err + return fail(err) + } + if _, err := workspaceRelative(root, target, requestedPath); err != nil { + return fail(err) } if err := recheckWorkspaceWriteTarget(root, target); err != nil { - return "", "", err + return fail(err) } existing := target @@ -106,30 +146,27 @@ func resolveWorkspaceTargetPath(workspaceRoot string, requestedPath string) (str } else if os.IsNotExist(err) { parent := filepath.Dir(existing) if parent == existing { - return "", "", err + return fail(err) } missingSegments = append([]string{filepath.Base(existing)}, missingSegments...) existing = parent continue } else { - return "", "", err + return fail(err) } } resolved, err := filepath.EvalSymlinks(existing) if err != nil { - return "", "", err + return fail(err) } for _, segment := range missingSegments { resolved = filepath.Join(resolved, segment) } - relative, err := filepath.Rel(root, resolved) + relative, err := workspaceRelative(root, resolved, requestedPath) if err != nil { - return "", "", outsideWorkspaceError(requestedPath) - } - if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) { - return "", "", outsideWorkspaceError(requestedPath) + return fail(err) } if relative == "." { return resolved, ".", nil @@ -160,6 +197,8 @@ func recheckWorkspaceWriteTarget(workspaceRoot string, requestedPath string) err return err } + target = sandbox.NormalizePrefixForRoot(target, root) + relative, err := filepath.Rel(root, target) if err != nil { return outsideWorkspaceError(requestedPath) @@ -201,6 +240,22 @@ func outsideWorkspaceError(requestedPath string) error { return fmt.Errorf("%s must stay inside the workspace", requestedPath) } +// workspaceRelative returns target relative to root, or outsideWorkspaceError +// when target is lexically outside the workspace. Call this after filepath.Abs +// (a missing ../../path must not skip confinement) and after EvalSymlinks +// (symlink escapes). +func workspaceRelative(root, target, requestedPath string) (string, error) { + target = sandbox.NormalizePrefixForRoot(target, root) + relative, err := filepath.Rel(root, target) + if err != nil { + return "", outsideWorkspaceError(requestedPath) + } + if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) { + return "", outsideWorkspaceError(requestedPath) + } + return relative, nil +} + func shouldSkipDirectory(name string) bool { return ignoredDirectories[name] || workspaceindex.ShouldSkipDir(name) } diff --git a/internal/tools/write_file.go b/internal/tools/write_file.go index 76f5f1baa..5af9db578 100644 --- a/internal/tools/write_file.go +++ b/internal/tools/write_file.go @@ -104,7 +104,10 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an if err := os.MkdirAll(filepath.Dir(absolutePath), 0o755); err != nil { return errorResult("Error writing file " + relativePath + ": " + err.Error()) } - if err := recheckScopedWriteTarget(tool.workspaceRoot, tool.scope, requestedPath); err != nil { + // Recheck the resolved target, not the original argument: a Windows POSIX + // rewrite maps /home///dir/file onto dir/file, and walking the + // original path would miss a symlink swapped into dir before WriteFile. + if err := recheckScopedWriteTarget(tool.workspaceRoot, tool.scope, absolutePath); err != nil { return errorResult("Error writing file " + relativePath + ": " + err.Error()) } if err := os.WriteFile(absolutePath, []byte(content), 0o644); err != nil {