Skip to content

restore: refuse a Linux name's backslash on a Windows restore - #420

Merged
allens merged 3 commits into
mainfrom
fix/restore-backslash-escape
Oct 16, 2026
Merged

allens merged 3 commits into
mainfrom
fix/restore-backslash-escape

Conversation

@allens

@allens allens commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

On Windows, restore --output of a Linux or macOS snapshot could write outside the output folder.

A backslash is an ordinary character in a Linux name, and Windows reads it as a separator. reroot splits a snapshot path on / alone and hands each segment to join, which on Windows splits on \ too and resolves any .. it then finds. The reader's ./.. check (pathFault, ADR-0101) also splits on /, so nothing caught it. Measured on Windows, reroot + planRestore for --output C:\restore-out:

Snapshot path main this PR
/home/u/edge/back\slash.txt write C:\restore-out\edge\back\slash.txt (a folder and a file) refuse
/home/u/edge/a\..\..\..\Windows\x.txt write C:\Windows\x.txt refuse
/home/u/edge/plain.txt write write

Under --overwrite the escaped write replaced an existing file, and a\b/c.txt and a/b/c.txt restored to one file, one of them lost.

Two guards close it:

  • reroot returns no destination for a name holding \ under a Windows output, checking only the names it joins, so a \ in a folder above the backed-up root doesn't matter. planRestore refuses such a name and lists it under its snapshot path.
  • Without --output, Windows' isAbsolute took /home/… too, on the current drive, so the escape was open there as well. On Windows only a Windows-shaped path now counts as absolute (restore.mjs): a plain restore of a Linux or macOS snapshot is refused up front and pointed at --output, as the reverse already was.

Refused names are listed and the run exits non-zero, as for names Windows refuses. guide/restore.md says so, and guide/format.md's restorer pitfalls say why \ differs from characters that simply fail. The spec date is unchanged, so the clean-room stamp still matches.

Found by the Windows restore clean-room experiment (2026-10-16), whose restorer refused these names. Each new test fails with its guard disabled. Not run: test:integration, since this is the planner and the path guard, not the S3 read path.

🤖 Generated with Claude Code

A backslash is an ordinary character in a Linux or macOS name, and
Windows reads it as a separator. reroot splits a snapshot path on `/`
alone and hands each segment to join, which on Windows splits on `\`
too and resolves any `..` it then finds. So restore --output of a Linux
snapshot on Windows wrote back\slash.txt as a folder and a file, sent
a\b/c.txt and a/b/c.txt to one file, and placed
a\..\..\..\Windows\x.txt at C:\Windows\x.txt, over an existing file
under --overwrite. The reader's ./.. check (ADR-0101) splits on `/`
too, so nothing caught it.

planRestore now refuses such a name, beside putsColonInName's refusal,
but asked of the source path: by then dest has already resolved the
`..`. Found by the Windows restore clean-room experiment (2026-10-16),
whose restorer refused the names.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@allens-claude-reviewer allens-claude-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review (Opus, high): 8 findings. The first three compare.py findings were confirmed by running it on Windows (Python 3.14.5): each prints IDENTICAL and exits 0.

src/lib/restore.mjs:15 — "Only through --output" is false, here and in the new doc that copies the premise: a POSIX path is win32-absolute, so a plain restore on Windows reaches NTFS too. A Linux row /home/u/a:b.txt restored without --output writes into stream b.txt of a file a and is reported restored. Same root cause as the backslash escape, and the comment tells the next reader both refusals are complete.

src/commands/restore.mjs:375 — isRefusedName's doc still says only a : is refused up front. The PR updated the header comment but not this sentence, so the fact now lives in two places and this one is stale. Point at planRestore instead of restating it.

Comment thread src/lib/restore.mjs Outdated
Comment thread src/lib/restore.mjs
Comment thread src/lib/restore.mjs Outdated
Comment thread guide/format.md Outdated
Comment thread guide/restore.md Outdated
Comment thread src/lib/restore.test.mjs Outdated
…in reroot

Review of #420 found the first fix incomplete. Without --output,
restore's absolute-path guard used isAbsolute, which on Windows takes a
POSIX /home/... too, on the current drive, where a \ in its names is
read as a separator: the escape was still open. On Windows only a
Windows-shaped path now counts as absolute, so a Linux or macOS
snapshot is refused up front and pointed at --output, as the reverse
already was.

The backslash check moves from planRestore into reroot, which sees only
the names it joins: a \ in a folder above the backed-up root no longer
refuses the whole set. reroot returns undefined for a name it can't
place, and planRestore refuses it under the snapshot path rather than
the resolved one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@allens

allens commented Oct 16, 2026

Copy link
Copy Markdown
Owner Author

On the two findings in the review body, both fixed in 42e2fcc:

  • restore.mjs:15 ("only through --output" is false): it was false then, and is true now, since a plain restore of a POSIX snapshot on Windows is refused up front (restore.mjs). That closes the a:b.txt stream case without --output too, so the comment stands as written.
  • commands/restore.mjs:375 (stale isRefusedName doc): it now points at planRestore instead of restating which names it refuses, and so does the header comment.

@allens
allens enabled auto-merge (squash) October 16, 2026 18:43
@allens
allens merged commit 32d6f6b into main Oct 16, 2026
12 checks passed
@allens
allens deleted the fix/restore-backslash-escape branch October 16, 2026 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant