From 662ad75a8417ceb2f8f846408d5525cfc590aaa8 Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Thu, 27 Aug 2026 18:00:10 +0000 Subject: [PATCH 01/12] fix(tools): rewrite Windows POSIX /home paths and hint on miss Models on Windows pass /home/user//... into read_file, glob, and grep. Windows joins those onto the workspace, which surfaces GetFileAttributesEx errors. Strip synthetic /home, /Users, /tmp, and /var/tmp prefixes when they include the workspace basename, and annotate remaining POSIX-absolute misses with the workspace root and host OS. Fixes Gitlawb/zero#972 --- internal/tools/posix_windows_path.go | 177 +++++++++++++++++ internal/tools/posix_windows_path_test.go | 219 ++++++++++++++++++++++ internal/tools/workspace.go | 59 ++++-- 3 files changed, 440 insertions(+), 15 deletions(-) create mode 100644 internal/tools/posix_windows_path.go create mode 100644 internal/tools/posix_windows_path_test.go diff --git a/internal/tools/posix_windows_path.go b/internal/tools/posix_windows_path.go new file mode 100644 index 000000000..f68b3bbaa --- /dev/null +++ b/internal/tools/posix_windows_path.go @@ -0,0 +1,177 @@ +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 ("//...") or a Windows volume +// ("C:/..."). Backslashes are normalized with ToSlash first. +func looksLikePosixAbsolute(path string) bool { + normalized := strings.TrimSpace(filepath.ToSlash(path)) + if normalized == "" { + return false + } + if !strings.HasPrefix(normalized, "/") { + return false + } + if strings.HasPrefix(normalized, "//") { + return false + } + return true +} + +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 + } + return strings.Split(normalized, "/") +} + +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 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 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. +func rewritePosixWorkspacePath(goos, workspaceRoot, requested string) string { + if goos != "windows" { + return requested + } + if stripped, ok := stripSyntheticPosixWorkspacePrefix(workspaceRoot, requested); ok { + return stripped + } + return requested +} + +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. +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 + } + root := workspaceRoot + if abs, absErr := filepath.Abs(workspaceRoot); absErr == nil { + root = abs + } + return fmt.Errorf("%w; host is Windows and %q looks like a POSIX absolute path; use a workspace-relative path or a Windows path (workspace root: %s)", err, requested, root) +} diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go new file mode 100644 index 000000000..c6a1996a8 --- /dev/null +++ b/internal/tools/posix_windows_path_test.go @@ -0,0 +1,219 @@ +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}, + } + 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", + }, + } + 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, missing) { + t.Fatalf("wrapped error should unwrap to original, got %v", got) + } + msg := got.Error() + if !strings.Contains(msg, "Windows") { + t.Fatalf("hint missing Windows host: %q", msg) + } + if !strings.Contains(msg, root) { + t.Fatalf("hint missing workspace root %q: %q", root, msg) + } + if !strings.Contains(msg, "POSIX") { + t.Fatalf("hint missing POSIX path wording: %q", msg) + } + + 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, root) { + t.Fatalf("error missing workspace root %q: %q", root, msg) + } +} + +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") + } +} diff --git a/internal/tools/workspace.go b/internal/tools/workspace.go index 8b8322b32..fb444518d 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,17 +33,31 @@ 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 + // Windows-only rewrite of synthetic POSIX prefixes that include the + // workspace basename. Real Windows absolute paths are left unchanged; + // missing files are not invented. + requestedPath = rewritePosixWorkspacePath(goos, workspaceRoot, requestedPath) + + fail := func(err error) (string, string, error) { + return "", "", annotatePosixWindowsPathError(goos, workspaceRoot, original, err) + } + 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 @@ -52,19 +67,19 @@ func resolveWorkspacePath(workspaceRoot string, requestedPath string) (string, s target, err = filepath.Abs(target) if err != nil { - return "", "", err + return fail(err) } target, err = filepath.EvalSymlinks(target) if err != nil { - return "", "", err + return fail(err) } relative, err := filepath.Rel(root, target) if err != nil { - return "", "", outsideWorkspaceError(requestedPath) + return fail(outsideWorkspaceError(requestedPath)) } if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) { - return "", "", outsideWorkspaceError(requestedPath) + return fail(outsideWorkspaceError(requestedPath)) } if relative == "." { return target, ".", nil @@ -73,17 +88,31 @@ 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 + // Windows-only rewrite of synthetic POSIX prefixes that include the + // workspace basename. Real Windows absolute paths are left unchanged; + // missing files are not invented. + requestedPath = rewritePosixWorkspacePath(goos, workspaceRoot, requestedPath) + + fail := func(err error) (string, string, error) { + return "", "", annotatePosixWindowsPathError(goos, workspaceRoot, original, err) + } + 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 @@ -92,10 +121,10 @@ func resolveWorkspaceTargetPath(workspaceRoot string, requestedPath string) (str } target, err = filepath.Abs(target) if err != nil { - return "", "", err + return fail(err) } if err := recheckWorkspaceWriteTarget(root, target); err != nil { - return "", "", err + return fail(err) } existing := target @@ -106,19 +135,19 @@ 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) @@ -126,10 +155,10 @@ func resolveWorkspaceTargetPath(workspaceRoot string, requestedPath string) (str relative, err := filepath.Rel(root, resolved) if err != nil { - return "", "", outsideWorkspaceError(requestedPath) + return fail(outsideWorkspaceError(requestedPath)) } if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) { - return "", "", outsideWorkspaceError(requestedPath) + return fail(outsideWorkspaceError(requestedPath)) } if relative == "." { return resolved, ".", nil From 3f9f7d476a54bb9ec1adf344ce65412faa665bbb Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Thu, 27 Aug 2026 18:09:37 +0000 Subject: [PATCH 02/12] fix(tools): reject lexical workspace escapes before EvalSymlinks A rewritten ../../missing path failed EvalSymlinks with NotExist and got a POSIX Windows hint instead of outsideWorkspaceError. Check containment after Abs first. Cover resolveWorkspaceTargetPath rewrite and the escaped write-target path. --- internal/tools/posix_windows_path_test.go | 50 +++++++++++++++++++++++ internal/tools/workspace.go | 37 ++++++++++++----- 2 files changed, 77 insertions(+), 10 deletions(-) diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index c6a1996a8..7a81c79f0 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -217,3 +217,53 @@ func TestResolveWorkspacePathDoesNotRewriteForeignRepo(t *testing.T) { 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 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) + } +} diff --git a/internal/tools/workspace.go b/internal/tools/workspace.go index fb444518d..124fbcba0 100644 --- a/internal/tools/workspace.go +++ b/internal/tools/workspace.go @@ -69,17 +69,19 @@ func resolveWorkspacePathForGOOS(goos, workspaceRoot, requestedPath string) (str if err != nil { 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 fail(err) } - relative, err := filepath.Rel(root, target) + relative, err := workspaceRelative(root, target, requestedPath) if err != nil { - return fail(outsideWorkspaceError(requestedPath)) - } - if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) { - return fail(outsideWorkspaceError(requestedPath)) + return fail(err) } if relative == "." { return target, ".", nil @@ -123,6 +125,9 @@ func resolveWorkspaceTargetPathForGOOS(goos, workspaceRoot, requestedPath string if err != nil { return fail(err) } + if _, err := workspaceRelative(root, target, requestedPath); err != nil { + return fail(err) + } if err := recheckWorkspaceWriteTarget(root, target); err != nil { return fail(err) } @@ -153,12 +158,9 @@ func resolveWorkspaceTargetPathForGOOS(goos, workspaceRoot, requestedPath string resolved = filepath.Join(resolved, segment) } - relative, err := filepath.Rel(root, resolved) + relative, err := workspaceRelative(root, resolved, requestedPath) if err != nil { - return fail(outsideWorkspaceError(requestedPath)) - } - if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) { - return fail(outsideWorkspaceError(requestedPath)) + return fail(err) } if relative == "." { return resolved, ".", nil @@ -230,6 +232,21 @@ 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) { + 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) } From 1053e5abe31519bc2a5db36d0d14e7a1c04d10f5 Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Thu, 27 Aug 2026 18:19:03 +0000 Subject: [PATCH 03/12] fix(tools): join Windows POSIX paths with goos rules resolveWorkspacePathForGOOS("windows") still used host filepath.IsAbs and Join, so on Linux CI /tmp/does-not-exist-xyz was absolute and the missing-path hint test hit confinement instead. Join POSIX-absolute paths onto the workspace when goos is windows. --- internal/tools/posix_windows_path.go | 42 +++++++++++++++++++++++ internal/tools/posix_windows_path_test.go | 37 ++++++++++++++++++++ internal/tools/workspace.go | 10 ++---- 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/internal/tools/posix_windows_path.go b/internal/tools/posix_windows_path.go index f68b3bbaa..a86441d06 100644 --- a/internal/tools/posix_windows_path.go +++ b/internal/tools/posix_windows_path.go @@ -25,6 +25,48 @@ func looksLikePosixAbsolute(path string) bool { return true } +func isDriveLetter(b byte) bool { + return (b >= 'A' && b <= 'Z') || (b >= 'a' && b <= 'z') +} + +// isAbsForGOOS reports whether path is absolute on goos. 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 filepath.IsAbs(path) + } + if path == "" { + return false + } + n := filepath.ToSlash(path) + if strings.HasPrefix(n, "//") { + return true + } + if len(path) >= 2 && path[1] == ':' && isDriveLetter(path[0]) { + return true + } + // Volume-relative Windows abs uses a leading backslash, not POSIX "/". + return path[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 { diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index 7a81c79f0..17cf94d27 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -195,6 +195,9 @@ func TestResolveWorkspacePathAnnotatesPosixMissOnWindows(t *testing.T) { if !strings.Contains(msg, root) { t.Fatalf("error missing workspace root %q: %q", root, msg) } + if strings.Contains(msg, "must stay inside the workspace") { + t.Fatalf("POSIX miss used confinement instead of a missing-path hint: %q", msg) + } } func TestResolveWorkspacePathDoesNotRewriteForeignRepo(t *testing.T) { @@ -267,3 +270,37 @@ func TestResolveWorkspaceTargetPathRejectsLexicalEscape(t *testing.T) { 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}, + } + 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 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) + } +} diff --git a/internal/tools/workspace.go b/internal/tools/workspace.go index 124fbcba0..143643ab6 100644 --- a/internal/tools/workspace.go +++ b/internal/tools/workspace.go @@ -60,10 +60,7 @@ func resolveWorkspacePathForGOOS(goos, workspaceRoot, requestedPath string) (str 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 { @@ -117,10 +114,7 @@ func resolveWorkspaceTargetPathForGOOS(goos, workspaceRoot, requestedPath string 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 fail(err) From 78c93140fdadae98b0c5758c3025b60de3eaa497 Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Fri, 28 Aug 2026 05:27:33 +0000 Subject: [PATCH 04/12] fix(tools): compile POSIX rewrite on Unix and fold repo basename Drop the duplicate "/" switch case so workspaceBasename compiles when filepath.Separator is already "/". Compare the repo path segment with EqualFold so a Zero checkout still rewrites /home/.../zero/... on Windows. --- internal/tools/posix_windows_path.go | 6 +++--- internal/tools/posix_windows_path_test.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/internal/tools/posix_windows_path.go b/internal/tools/posix_windows_path.go index a86441d06..6e8425532 100644 --- a/internal/tools/posix_windows_path.go +++ b/internal/tools/posix_windows_path.go @@ -70,7 +70,7 @@ func joinAgainstRoot(goos, root, target string) string { func workspaceBasename(workspaceRoot string) string { repo := filepath.Base(filepath.Clean(workspaceRoot)) switch repo { - case "", ".", "..", string(filepath.Separator), "/": + case "", ".", "..", string(filepath.Separator): return "" default: return repo @@ -117,7 +117,7 @@ func matchSyntheticHomePrefix(parts []string, lead, repo string) (string, bool) if user == "" || user == "." || user == ".." { return "", false } - if parts[2] != repo { + if !strings.EqualFold(parts[2], repo) { return "", false } return restAfterPrefix(parts, 3) @@ -133,7 +133,7 @@ func matchSyntheticDirPrefix(parts []string, lead []string, repo string) (string return "", false } } - if parts[len(lead)] != repo { + if !strings.EqualFold(parts[len(lead)], repo) { return "", false } return restAfterPrefix(parts, len(lead)+1) diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index 17cf94d27..0a66b0a50 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -100,6 +100,20 @@ func TestRewritePosixWorkspacePath(t *testing.T) { 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", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 3ca0e961b7fb632953239c9198105dc3dbab74df Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Fri, 28 Aug 2026 06:36:54 +0000 Subject: [PATCH 05/12] fix(tools): reject rooted Windows paths as POSIX looksLikePosixAbsolute ran ToSlash first, so \tmp\zero\file became /tmp/zero/file and rewrote to a workspace-relative file. Reject a leading backslash before slash normalization. Keep /tmp/zero/file rewriting as the POSIX hallucination. --- internal/tools/posix_windows_path.go | 12 +++-- internal/tools/posix_windows_path_test.go | 64 +++++++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/internal/tools/posix_windows_path.go b/internal/tools/posix_windows_path.go index 6e8425532..2e89eb55b 100644 --- a/internal/tools/posix_windows_path.go +++ b/internal/tools/posix_windows_path.go @@ -9,10 +9,16 @@ import ( ) // looksLikePosixAbsolute reports whether path looks like a POSIX absolute -// path (leading "/") rather than a UNC path ("//...") or a Windows volume -// ("C:/..."). Backslashes are normalized with ToSlash first. +// 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 { - normalized := strings.TrimSpace(filepath.ToSlash(path)) + raw := strings.TrimSpace(path) + if strings.HasPrefix(raw, `\`) { + return false + } + normalized := filepath.ToSlash(raw) if normalized == "" { return false } diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index 0a66b0a50..8d14479b0 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -20,6 +20,9 @@ func TestLooksLikePosixAbsolute(t *testing.T) { {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 { @@ -114,6 +117,27 @@ func TestRewritePosixWorkspacePath(t *testing.T) { 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 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) { @@ -306,6 +330,46 @@ func TestIsAbsForGOOS(t *testing.T) { } } +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") From 15e62efa295c70ebe156287fab41776f242ccced Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Fri, 28 Aug 2026 18:21:42 +0000 Subject: [PATCH 06/12] fix(tools): stop leaking workspace root in POSIX path hints annotatePosixWindowsPathError named the workspace root, so a Windows /etc/passwd miss (joined as \etc\passwd) printed both /etc/ and the root. Keep the POSIX-vs-Windows hint without the root. isAbsForGOOS now treats a leading "/" as absolute on non-Windows goos instead of host filepath.IsAbs, so linux paths stay absolute on Windows CI. posixPathSegments drops empty interior segments so /tmp/zero//file rewrites to file. --- internal/tools/posix_windows_path.go | 29 +++++++++----- internal/tools/posix_windows_path_test.go | 49 ++++++++++++++++++++--- 2 files changed, 63 insertions(+), 15 deletions(-) diff --git a/internal/tools/posix_windows_path.go b/internal/tools/posix_windows_path.go index 2e89eb55b..c60b1c101 100644 --- a/internal/tools/posix_windows_path.go +++ b/internal/tools/posix_windows_path.go @@ -35,12 +35,14 @@ func isDriveLetter(b byte) bool { return (b >= 'A' && b <= 'Z') || (b >= 'a' && b <= 'z') } -// isAbsForGOOS reports whether path is absolute on goos. On Windows a POSIX -// leading "/" is not absolute (no volume or UNC), matching Windows -// filepath.IsAbs, so those paths join onto the workspace. +// 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 filepath.IsAbs(path) + return strings.HasPrefix(filepath.ToSlash(path), "/") } if path == "" { return false @@ -93,7 +95,15 @@ func posixPathSegments(path string) []string { if normalized == "" { return nil } - return strings.Split(normalized, "/") + 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) { @@ -207,6 +217,9 @@ func isMissingPathError(err error) bool { // (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 @@ -217,9 +230,5 @@ func annotatePosixWindowsPathError(goos, workspaceRoot, requested string, err er if !isMissingPathError(err) { return err } - root := workspaceRoot - if abs, absErr := filepath.Abs(workspaceRoot); absErr == nil { - root = abs - } - return fmt.Errorf("%w; host is Windows and %q looks like a POSIX absolute path; use a workspace-relative path or a Windows path (workspace root: %s)", err, requested, root) + return fmt.Errorf("%w; host is Windows and %q looks like a POSIX absolute path; use a workspace-relative path or a Windows path", err, requested) } diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index 8d14479b0..0ed600dbc 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -124,6 +124,13 @@ func TestRewritePosixWorkspacePath(t *testing.T) { 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", @@ -167,12 +174,28 @@ func TestAnnotatePosixWindowsPathError(t *testing.T) { if !strings.Contains(msg, "Windows") { t.Fatalf("hint missing Windows host: %q", msg) } - if !strings.Contains(msg, root) { - t.Fatalf("hint missing workspace root %q: %q", root, msg) - } if !strings.Contains(msg, "POSIX") { t.Fatalf("hint missing POSIX path wording: %q", msg) } + if strings.Contains(msg, root) { + t.Fatalf("hint must not name the workspace root %q: %q", root, msg) + } + 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) + } + if strings.Contains(msg, root) { + t.Fatalf("hint must not name the workspace root %q: %q", root, msg) + } 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) @@ -230,8 +253,8 @@ func TestResolveWorkspacePathAnnotatesPosixMissOnWindows(t *testing.T) { if !strings.Contains(msg, "Windows") { t.Fatalf("error missing Windows host hint: %q", msg) } - if !strings.Contains(msg, root) { - t.Fatalf("error missing workspace root %q: %q", root, msg) + if strings.Contains(msg, "workspace root") { + t.Fatalf("hint must not name the workspace root: %q", msg) } if strings.Contains(msg, "must stay inside the workspace") { t.Fatalf("POSIX miss used confinement instead of a missing-path hint: %q", msg) @@ -293,6 +316,22 @@ func TestResolveWorkspaceTargetPathRewritesMissingTmpFile(t *testing.T) { } } +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 { From da73e9cbec80d0cc4c5d815fa7e25bce2faab6a1 Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Tue, 1 Sep 2026 03:27:59 -0400 Subject: [PATCH 07/12] fix(tools): prefer existing POSIX joins and hide workspace root in hints The Windows POSIX rewrite was shadowing files that already existed at the literal join, and missing-path hints wrapped *os.PathError whose Path still named the workspace root. Recheck the resolved write target so a post-resolve symlink swap is not missed after a rewrite. --- internal/tools/edit_file.go | 5 +- internal/tools/posix_windows_path.go | 52 ++++++- internal/tools/posix_windows_path_test.go | 159 +++++++++++++++++++++- internal/tools/workspace.go | 16 ++- internal/tools/write_file.go | 5 +- 5 files changed, 220 insertions(+), 17 deletions(-) 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 index c60b1c101..bb58a18b8 100644 --- a/internal/tools/posix_windows_path.go +++ b/internal/tools/posix_windows_path.go @@ -187,7 +187,9 @@ func stripSyntheticPosixWorkspacePrefix(workspaceRoot, requested string) (string // 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. +// 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 @@ -198,6 +200,38 @@ func rewritePosixWorkspacePath(goos, workspaceRoot, requested string) string { return requested } +// existingLiteralPosixWorkspacePath reports whether the un-rewritten POSIX +// path already names an existing file inside the workspace. When it does, +// the rewrite must not steal the request: a model that named +// /tmp//x when that file 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 + } + _, err = os.Lstat(target) + return err == nil +} + func isMissingPathError(err error) bool { if err == nil { return false @@ -230,5 +264,19 @@ func annotatePosixWindowsPathError(goos, workspaceRoot, requested string, err er 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", err, requested) + 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 index 0ed600dbc..7a54bb9e8 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -167,8 +167,8 @@ func TestAnnotatePosixWindowsPathError(t *testing.T) { if got == nil { t.Fatal("expected wrapped missing-path error") } - if !errors.Is(got, missing) { - t.Fatalf("wrapped error should unwrap to original, got %v", got) + 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") { @@ -177,9 +177,7 @@ func TestAnnotatePosixWindowsPathError(t *testing.T) { if !strings.Contains(msg, "POSIX") { t.Fatalf("hint missing POSIX path wording: %q", msg) } - if strings.Contains(msg, root) { - t.Fatalf("hint must not name the workspace root %q: %q", root, msg) - } + errorMustNotNameWorkspaceRoot(t, msg, root) if strings.Contains(msg, "workspace root") { t.Fatalf("hint must not name the workspace root: %q", msg) } @@ -193,9 +191,21 @@ func TestAnnotatePosixWindowsPathError(t *testing.T) { if !strings.Contains(msg, "/etc/passwd") { t.Fatalf("hint should name the requested POSIX path: %q", msg) } - if strings.Contains(msg, root) { - t.Fatalf("hint must not name the workspace root %q: %q", root, 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) @@ -256,9 +266,20 @@ func TestResolveWorkspacePathAnnotatesPosixMissOnWindows(t *testing.T) { 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) { @@ -421,3 +442,127 @@ func TestJoinAgainstRootWindowsPosix(t *testing.T) { 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 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) + } +} + +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) + } + } +} diff --git a/internal/tools/workspace.go b/internal/tools/workspace.go index 143643ab6..a40b95c7d 100644 --- a/internal/tools/workspace.go +++ b/internal/tools/workspace.go @@ -39,9 +39,11 @@ func resolveWorkspacePath(workspaceRoot string, requestedPath string) (string, s func resolveWorkspacePathForGOOS(goos, workspaceRoot, requestedPath string) (string, string, error) { original := requestedPath // Windows-only rewrite of synthetic POSIX prefixes that include the - // workspace basename. Real Windows absolute paths are left unchanged; - // missing files are not invented. - requestedPath = rewritePosixWorkspacePath(goos, workspaceRoot, requestedPath) + // 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) + } fail := func(err error) (string, string, error) { return "", "", annotatePosixWindowsPathError(goos, workspaceRoot, original, err) @@ -93,9 +95,11 @@ func resolveWorkspaceTargetPath(workspaceRoot string, requestedPath string) (str func resolveWorkspaceTargetPathForGOOS(goos, workspaceRoot, requestedPath string) (string, string, error) { original := requestedPath // Windows-only rewrite of synthetic POSIX prefixes that include the - // workspace basename. Real Windows absolute paths are left unchanged; - // missing files are not invented. - requestedPath = rewritePosixWorkspacePath(goos, workspaceRoot, requestedPath) + // 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) + } fail := func(err error) (string, string, error) { return "", "", annotatePosixWindowsPathError(goos, workspaceRoot, original, err) 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 { From 51749206cb88595e850503f6e64188cab016a4ba Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Tue, 1 Sep 2026 16:05:35 -0400 Subject: [PATCH 08/12] fix(tools): normalize platform prefix in recheckWorkspaceWriteTarget --- internal/tools/posix_windows_path_test.go | 16 ++++++++++++++++ internal/tools/workspace.go | 3 +++ 2 files changed, 19 insertions(+) diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index 7a54bb9e8..86bc7ac1d 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -532,6 +532,22 @@ func TestRecheckWorkspaceWriteTargetAfterPosixRewrite(t *testing.T) { } 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 { + 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 { diff --git a/internal/tools/workspace.go b/internal/tools/workspace.go index a40b95c7d..1501a197f 100644 --- a/internal/tools/workspace.go +++ b/internal/tools/workspace.go @@ -189,6 +189,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) @@ -235,6 +237,7 @@ func outsideWorkspaceError(requestedPath string) error { // (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) From c427b76e350e6687ecab189575f9061e7671cad7 Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Tue, 1 Sep 2026 16:18:54 -0400 Subject: [PATCH 09/12] test(tools): explicitly skip when outer symlink setup fails --- internal/tools/posix_windows_path_test.go | 25 ++++++++++++----------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index 86bc7ac1d..94212c55d 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -535,18 +535,19 @@ func TestRecheckWorkspaceWriteTargetAfterPosixRewrite(t *testing.T) { // 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 { - 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) - } + 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) } } From 4373461fa00b271d1a4acbd798b5ce3bc621d71c Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Tue, 1 Sep 2026 18:13:53 -0400 Subject: [PATCH 10/12] fix(tools): reject drive-relative Windows paths without retargeting --- internal/tools/posix_windows_path.go | 30 +++++- internal/tools/posix_windows_path_test.go | 106 ++++++++++++++++++++++ internal/tools/workspace.go | 24 +++-- 3 files changed, 149 insertions(+), 11 deletions(-) diff --git a/internal/tools/posix_windows_path.go b/internal/tools/posix_windows_path.go index bb58a18b8..667071c95 100644 --- a/internal/tools/posix_windows_path.go +++ b/internal/tools/posix_windows_path.go @@ -35,6 +35,29 @@ 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 @@ -47,15 +70,16 @@ func isAbsForGOOS(goos, path string) bool { if path == "" { return false } - n := filepath.ToSlash(path) + raw := strings.TrimSpace(path) + n := filepath.ToSlash(raw) if strings.HasPrefix(n, "//") { return true } - if len(path) >= 2 && path[1] == ':' && isDriveLetter(path[0]) { + if isDriveAbsoluteWindowsPath(raw) { return true } // Volume-relative Windows abs uses a leading backslash, not POSIX "/". - return path[0] == '\\' + return raw[0] == '\\' } // joinAgainstRoot joins target onto root using goos path rules. Windows diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index 94212c55d..40a725d15 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -583,3 +583,109 @@ func errorMustNotNameWorkspaceRoot(t *testing.T, msg, root string) { } } } + +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 1501a197f..65e5369d8 100644 --- a/internal/tools/workspace.go +++ b/internal/tools/workspace.go @@ -38,6 +38,14 @@ func resolveWorkspacePath(workspaceRoot string, requestedPath string) (string, s 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. @@ -45,10 +53,6 @@ func resolveWorkspacePathForGOOS(goos, workspaceRoot, requestedPath string) (str requestedPath = rewritePosixWorkspacePath(goos, workspaceRoot, requestedPath) } - fail := func(err error) (string, string, error) { - return "", "", annotatePosixWindowsPathError(goos, workspaceRoot, original, err) - } - if requestedPath == "" { requestedPath = "." } @@ -94,6 +98,14 @@ func resolveWorkspaceTargetPath(workspaceRoot string, requestedPath string) (str 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. @@ -101,10 +113,6 @@ func resolveWorkspaceTargetPathForGOOS(goos, workspaceRoot, requestedPath string requestedPath = rewritePosixWorkspacePath(goos, workspaceRoot, requestedPath) } - fail := func(err error) (string, string, error) { - return "", "", annotatePosixWindowsPathError(goos, workspaceRoot, original, err) - } - if requestedPath == "" { requestedPath = "." } From 12f7aaa0c16544a2f817144a8ebeb3cb7eb0e82c Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Tue, 1 Sep 2026 18:20:04 -0400 Subject: [PATCH 11/12] fix(tools): handle whitespace-only paths safely in isAbsForGOOS --- internal/tools/posix_windows_path.go | 4 ++-- internal/tools/posix_windows_path_test.go | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/tools/posix_windows_path.go b/internal/tools/posix_windows_path.go index 667071c95..edf3eb953 100644 --- a/internal/tools/posix_windows_path.go +++ b/internal/tools/posix_windows_path.go @@ -67,10 +67,10 @@ func isAbsForGOOS(goos, path string) bool { if goos != "windows" { return strings.HasPrefix(filepath.ToSlash(path), "/") } - if path == "" { + raw := strings.TrimSpace(path) + if raw == "" { return false } - raw := strings.TrimSpace(path) n := filepath.ToSlash(raw) if strings.HasPrefix(n, "//") { return true diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index 40a725d15..8d95d9f05 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -382,6 +382,8 @@ func TestIsAbsForGOOS(t *testing.T) { {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 { @@ -629,6 +631,7 @@ func TestWindowsPathCategoriesResolution(t *testing.T) { input string wantRel string }{ + {"", "."}, {"file.txt", "file.txt"}, {"./file.txt", "file.txt"}, {"sub/child.txt", "sub/child.txt"}, From a875cae124caac4e620859fcd1fae81e61986319 Mon Sep 17 00:00:00 2001 From: cairn-intern Date: Wed, 2 Sep 2026 04:37:45 -0400 Subject: [PATCH 12/12] fix(tools): prefer existing literal directory over synthetic rewrite for missing write target --- internal/tools/posix_windows_path.go | 21 ++++++++++++++------ internal/tools/posix_windows_path_test.go | 24 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/internal/tools/posix_windows_path.go b/internal/tools/posix_windows_path.go index edf3eb953..385a4e04b 100644 --- a/internal/tools/posix_windows_path.go +++ b/internal/tools/posix_windows_path.go @@ -225,10 +225,11 @@ func rewritePosixWorkspacePath(goos, workspaceRoot, requested string) string { } // existingLiteralPosixWorkspacePath reports whether the un-rewritten POSIX -// path already names an existing file inside the workspace. When it does, -// the rewrite must not steal the request: a model that named -// /tmp//x when that file exists should read and write that file, -// not a different x at the workspace root. +// 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 @@ -252,8 +253,16 @@ func existingLiteralPosixWorkspacePath(goos, workspaceRoot, requested string) bo if _, err := workspaceRelative(root, target, requested); err != nil { return false } - _, err = os.Lstat(target) - return err == nil + 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 { diff --git a/internal/tools/posix_windows_path_test.go b/internal/tools/posix_windows_path_test.go index 8d95d9f05..acdbcbbc6 100644 --- a/internal/tools/posix_windows_path_test.go +++ b/internal/tools/posix_windows_path_test.go @@ -494,6 +494,30 @@ func TestResolveWorkspaceTargetPathPrefersExistingLiteralOverRewrite(t *testing. } } +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 {