fix(worktree): strip verbatim \?\ path prefix so git accepts worktree paths (Windows)#347
Conversation
… paths
On Windows, std::fs::canonicalize() returns an extended-length ("verbatim")
path with the \?\ prefix (e.g. \?\C:\Users\...). That prefix is passed
straight through to git.exe by worktree commands, which rejects it:
fatal: could not create leading directories of
'/?/C:/Users/.../.termul/worktrees/<name>/.git': Invalid argument
This was a regression from the path-traversal security hardening (canonicalize
was added in gnoviawan#197) and made every worktree creation fail on Windows.
- Add path_validation::strip_verbatim_prefix to rewrite \?\C:\... -> C:\... and
\?\UNC\server\share -> \server\share (pure string, cross-platform).
- Apply it in validate_project_path so all validated paths stay tool-friendly
while keeping canonicalization's symlink/traversal protections.
- Add unit tests for both prefix shapes and the no-op normal-path case.
Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds Windows path normalization to ensure canonicalized project paths are tool-friendly. A new utility function strips Windows verbatim ( ChangesWindows Verbatim Path Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src-tauri/src/path_validation.rs (1)
299-306: 💤 Low valueMisleading test name.
The test name suggests disk prefix is chosen over UNC, but the test actually verifies the opposite: that UNC paths are correctly identified and converted to
\\server\...rather than being mistakenly matched by the shorter\\?\prefix and left asUNC\....Consider renaming to something like
test_strip_verbatim_unc_prefix_takes_precedenceto match the actual assertion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/path_validation.rs` around lines 299 - 306, The test function name test_strip_verbatim_disk_prefix_chosen_over_unc_match is misleading because the assertion verifies that UNC verbatim prefix is handled (mapped to \\server\...) rather than the shorter \\?\ disk prefix; rename the test to a clear name such as test_strip_verbatim_unc_prefix_takes_precedence by updating the function identifier and any internal references/usages to that test name so it matches the asserted behavior (reference: the test function currently calling strip_verbatim_prefix(r"\\?\UNC\server\share")).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src-tauri/src/path_validation.rs`:
- Around line 299-306: The test function name
test_strip_verbatim_disk_prefix_chosen_over_unc_match is misleading because the
assertion verifies that UNC verbatim prefix is handled (mapped to \\server\...)
rather than the shorter \\?\ disk prefix; rename the test to a clear name such
as test_strip_verbatim_unc_prefix_takes_precedence by updating the function
identifier and any internal references/usages to that test name so it matches
the asserted behavior (reference: the test function currently calling
strip_verbatim_prefix(r"\\?\UNC\server\share")).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 193da85d-441b-4dfc-b5f7-49df3b848bd6
📒 Files selected for processing (2)
src-tauri/src/commands.rssrc-tauri/src/path_validation.rs
…352) PR #295 replaced the pty CWD existence check with std::fs::canonicalize, which on Windows returns a verbatim \?\-prefixed (device-namespace) path. That string was handed unmodified to cmd.exe via ConPTY; cmd.exe cannot operate in the device namespace and prints "UNC paths are not supported. Defaulting to Windows directory", making the shell land in C:\ instead of the project (regression first released in v0.4.6). Apply the existing path_validation::strip_verbatim_prefix right after canonicalize — the exact remedy already shipped for git.exe worktree paths in #347. Canonicalize still runs, so #295's symlink/traversal protections are preserved; only the string representation handed to external tools is normalized to C:\… (no-op off Windows). The cleaned path flows to the ConPTY spawn cwd, TerminalInstance.cwd, the cwd/git trackers, and the returned TerminalInfo. Verified: cargo check + cargo test path_validation (12 tests) + debug build.
Problem
Creating a worktree on Windows fails for every project with:
The malformed
/?/C:/...path is whatgit.exemakes of a verbatim (extended-length) path it was handed.Root cause
validate_project_path(src-tauri/src/commands.rs) canonicalizes the project path withstd::fs::canonicalize(). On Windows this always returns a path with the\?\prefix, e.g.:That prefix is then passed straight through to
git.exeas agit worktree addargument. Git (MSYS2) cannot parse\?\…and aborts — hence every worktree creation breaks on Windows.This is a regression from #197 (
fix(security): add path validation to worktree commands), which introduced the canonicalize for path-traversal protection.Fix
Canonicalization is kept for security (symlink resolution + existence check), but the
\?\prefix is stripped from the resulting string before it reaches external tools:path_validation::strip_verbatim_prefix— pure-string rewriter:\?\C:\…→C:\…\?\UNC\server\share→\server\sharevalidate_project_pathapplies it, so every validated path (worktree create/list/remove, PTY cwd, …) stays tool-friendly while preserving the canonicalization benefits.Verification
cargo test/cargo checklocally. The change is minimal and reviewed by hand.pr-validation.yml) which runscargo check --all-targets,cargo test,cargo clippy -- -D warnings, plus a dedicatedrust-windows-checkjob — directly relevant since this is a Windows-only bug. I'll address anything CI flags.🤖 Generated with Claude Code
Summary by CodeRabbit