fix(agents): agent-created files get lowercased names on Windows#109823
Conversation
toRelativePathUnderRoot passed root and candidate through normalizeWindowsPathForComparison, which lowercases, and then returned the resulting relative path. Callers build files out of that path, so an agent asking for src/Components/MyComponent.tsx got src\components\mycomponent.tsx on disk. NTFS is case-preserving, so nothing fails locally, but git records the lowercased name and the imports the agent wrote break on Linux and in CI. Lowercasing is not even case-safe for every name: "İstanbul.md" lowercases to "i̇stanbul.md" (U+0130 becomes U+0069 U+0307), one code point longer and not reversible, so the filename is corrupted rather than merely recased. The lowercasing was never needed for the boundary math: path.win32.relative already matches the root case-insensitively and returns the tail in its original case. Extended-length prefix stripping is still needed, or a \?\ candidate relativizes to ..\..\..\?\C:\... and reads as an escape, so this adds normalizeWindowsPathPreservingCase next to the comparison variant. It mirrors that helper step for step, including the trim, minus the lowercasing; a test pins the equivalence so the two cannot drift. The containment decision is unchanged: relative(lower(a), lower(b)) and relative(a, b) return the same structure, and that structure is all validateRelativePathWithinBoundary inspects. Sibling surfaces checked: the other two callers of normalizeWindowsPathForComparison use it as a comparison key and are correct as-is (installed-plugin-index-record-reader.ts:215 compares with ===, fs-safe's isPathInside discards the relative and returns a boolean). path-policy.ts was the only site returning the normalized value.
|
Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 5:44 PM ET / 21:44 UTC. Summary PR surface: Source +24, Tests +96. Total +120 across 5 files. Reproducibility: yes. from source and provided live Windows output: the current path helper returns a relative path derived from a lowercasing comparison key, and the branch demonstrates the exact write path retaining mixed-case and Unicode names after the change. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Choose one owned case-preserving Windows normalization contract—preferably expose it from Do we have a high-confidence way to reproduce the issue? Yes, from source and provided live Windows output: the current path helper returns a relative path derived from a lowercasing comparison key, and the branch demonstrates the exact write path retaining mixed-case and Unicode names after the change. Is this the best way to solve the issue? Unclear. Preserving the returned filename case is the right behavioral repair, but duplicating fs-safe normalization internals in core is not clearly the best durable ownership model for this security-sensitive path contract. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against fa9ae68c3423. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +24, Tests +96. Total +120 across 5 files. View PR surface stats
Security concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (9 earlier review cycles; latest 8 shown)
|
Co-authored-by: Yigtwxx <[email protected]>
|
On the P1 drift risk — the ownership question is genuinely yours to decide, but one input for it: the
expect(normalizeWindowsPathPreservingCase(input).toLowerCase()).toBe(
normalizeWindowsPathForComparison(input),
);Since the right-hand side is fs-safe's implementation, an fs-safe change to extended-prefix or So the failure mode is a red CI on the next dependency bump, not a silent security-boundary drift. The honest limit: that guard is only as wide as its six inputs (extended-length, UNC upper/lower, mixed That does not settle the ownership question — if you'd rather |
|
Merged via squash.
|
…nclaw#109823) * fix(agents): preserve filename case for agent file writes on Windows toRelativePathUnderRoot passed root and candidate through normalizeWindowsPathForComparison, which lowercases, and then returned the resulting relative path. Callers build files out of that path, so an agent asking for src/Components/MyComponent.tsx got src\components\mycomponent.tsx on disk. NTFS is case-preserving, so nothing fails locally, but git records the lowercased name and the imports the agent wrote break on Linux and in CI. Lowercasing is not even case-safe for every name: "İstanbul.md" lowercases to "i̇stanbul.md" (U+0130 becomes U+0069 U+0307), one code point longer and not reversible, so the filename is corrupted rather than merely recased. The lowercasing was never needed for the boundary math: path.win32.relative already matches the root case-insensitively and returns the tail in its original case. Extended-length prefix stripping is still needed, or a \?\ candidate relativizes to ..\..\..\?\C:\... and reads as an escape, so this adds normalizeWindowsPathPreservingCase next to the comparison variant. It mirrors that helper step for step, including the trim, minus the lowercasing; a test pins the equivalence so the two cannot drift. The containment decision is unchanged: relative(lower(a), lower(b)) and relative(a, b) return the same structure, and that structure is all validateRelativePathWithinBoundary inspects. Sibling surfaces checked: the other two callers of normalizeWindowsPathForComparison use it as a comparison key and are correct as-is (installed-plugin-index-record-reader.ts:215 compares with ===, fs-safe's isPathInside discards the relative and returns a boolean). path-policy.ts was the only site returning the normalized value. * test(agents): verify Windows filename case end to end Co-authored-by: Yigtwxx <[email protected]> * style(agents): apply oxfmt to the workspace path case test --------- Co-authored-by: Peter Steinberger <[email protected]>
What Problem This Solves
On Windows, files and directories an agent creates get lowercased names. Ask for
src/Components/MyComponent.tsxand what lands on disk issrc\components\mycomponent.tsx.NTFS is case-insensitive but case-preserving, so nothing fails locally — the agent reads its own file
back fine. The damage shows up downstream: git records the lowercased name, and the TypeScript/React
import the agent just wrote (
./Components/MyComponent) resolves on the author's Windows box but breaksfor every teammate on Linux/macOS and in CI. Any agent-authored name that is not already lowercase is
affected:
README.md,Dockerfile,MyClass.ts.For some names it is worse than a case change.
"İstanbul.md".toLowerCase()is"i̇stanbul.md"—U+0130 becomes the two code points U+0069 U+0307, one longer than the original and not reversible. So a
Turkish filename does not come back lowercased, it comes back corrupted.
This is Windows-only. The POSIX branch of the same function preserves case correctly today.
Why This Change Was Made
toRelativePathUnderRoot(src/agents/path-policy.ts:76-90) ran both root and candidate throughnormalizeWindowsPathForComparisonand then returned the resulting relative path:That helper lowercases (
@openclaw/[email protected],dist/path.js:17→normalizeLowercaseStringOrEmpty(...)). Lowercasing is right for a comparison key — which is exactlyhow the other two call sites use it — but this function hands the relative path back to callers, and they
build files out of it:
src/agents/agent-tools.read.ts:1089-1092—mkdir:path.resolve(root, relative)→fs.mkdirsrc/agents/agent-tools.read.ts:1059-1060—writeWorkspaceFile→.write(relative, { mkdir: true })src/agents/apply-patch.ts:310,318,326—apply_patchwriteFile / remove / mkdirptoCanonicalRelativeWorkspacePath(agent-tools.read.ts:1167-1179) does not rescue it:fs.realpathrestores the true case only of the already-existing ancestor, the new basename stays lowercased, and
line 1179 re-lowercases the whole thing on the way out.
assertSandboxPathdoes not either — it ishanded the already-derived
resolvedpath, not the original candidate.The lowercasing was also never needed for the boundary math.
path.win32.relativealready matches theroot case-insensitively and returns the tail in its original case, so the pre-lowercasing bought
nothing and cost the filename. The one part of that normalization the boundary check does rely on is
extended-length prefix stripping — without it a
\\?\C:\...candidate relativizes to..\..\..\?\C:\...and gets rejected as an escape. So this addsnormalizeWindowsPathPreservingCasenext to the comparison variant in
src/infra/path-guards.ts. It mirrors that helper step for step —including the trim — minus the lowercasing, and a test pins that equivalence directly so the two cannot
drift:
Sibling surfaces
All three callers of
normalizeWindowsPathForComparisonwere checked. Only this one was misusing it:src/plugins/installed-plugin-index-record-reader.ts:215===)isPathInside(fs-safe)src/agents/path-policy.tsEvidence
Live before/after on real Windows (Windows 11, NTFS, Turkish locale), driving the real
toRelativeWorkspacePaththrough the exact call pattern ofagent-tools.read.ts, then reading the namesback with
fs.readdirSync:Before (
origin/main):After (this branch):
Why the existing test did not catch it:
path-policy.test.ts:23used an all-lowercase candidate(
c:/users/user/openclaw/memory/log.txt), which passes either way. The new cases use mixed case.Tests — every added mixed-case assertion fails on
origin/mainand passes here:Merge Risk
Low, but this touches a sandbox boundary, so here is the containment argument explicitly.
The boundary decision is unchanged.
path.win32.relativecompares case-insensitively, sorelative(lower(a), lower(b))andrelative(a, b)return the same structure — same number of..segments, same segment count, same absoluteness. Only the case of the returned characters differs.
validateRelativePathWithinBoundarylooks at exactly that structure (""/".","..","../","..\\",isAbsolute), so every accept/reject verdict is identical. Verified empirically:Two of the added tests pin this directly — an escape that differs from the root only by case is still
rejected, and a differently-cased root still resolves to
""underallowRoot. Both pass before andafter the change, which is the point.
Scope is one file's Windows branch plus one new helper with a single caller. POSIX behavior is untouched.
AI-assisted.