fix(recovery): keep every write and delete inside the working tree #142

2026-11-10T19:03:40Z

james.maes commented

2026-11-10 19:03:40 +00:00

What

Path recovery (rename and skip choices for Windows-invalid paths) now does every file write and delete through one internal, fully tested helper, WorkingTree:

the target must resolve inside the working directory;
it must not be in a .git folder, in any spelling Windows opens as that folder (letter case, the GIT~1 short name, trailing dots and spaces, a stream suffix);
it must not be reached through a link;
an existing file is removed and created anew, never written through.

Also:

The recovery file .git\gclo-recovery.json is validated when it is loaded. A file that lists a path outside the working tree fails the row with a message that says how to rebuild it; nothing is deleted.
WindowsPathValidator treats .git and its aliases as an invalid segment, with a suggested replacement.
Cleanup of a failed clone skips links when clearing attributes.

Why

Closes #141.

The recovery materializes the tree itself instead of using a git checkout, and it was less careful than one. The stored list of previously written files decided what to delete without being checked, a segment named .git was valid, and writes followed links.

Behaviour changes worth a release note

A repository whose tree (or a rename the user chose) would put a file in a .git folder is reported as an invalid path, with a suggested name.
A gclo-recovery.json that lists a path outside the repository fails that repository until the file is deleted and Resolve is used again.
Recovery refuses to write through a folder that is a link (symbolic link or junction).

How it was tested

gclo.Engine.Tests: 680 passed (was 606). New WorkingTreeTests (resolve, create, delete; outside paths; every .git spelling; a junction inside the working tree; a file symlink where the platform allows creating one). New recovery tests: a recovery file listing an outside file, absolute and relative, is refused on pull and on apply and the file survives; a rename into .git, .GIT, git~1 stays invalid and writes nothing; a folder in the tree that is a junction to elsewhere is refused. Validator tests for the new rule and for names that only start like .git.
gclo.Cli.Tests: 142 passed. Line coverage 100% on gclo.Engine, gclo.ViewModels, gclo.
Zero-warning x64 build with locked restore; dotnet format --verify-no-changes clean.

Not run: the UI suite (no shell or view-model change). The file-symlink test returns early when the machine cannot create a symlink, so on such a machine that one case is not exercised; the junction tests always run on Windows.

Checklist

Commits are signed and signed off (git commit -S -s)
Zero build warnings; format and lint are clean
Tests added or updated; the 100% coverage gate still passes
Docs updated for user-visible changes; threat model updated if the attack surface changed (threat model T3, T4 and review log)
No secrets, credentials, or personal data
New dependencies are justified below, or none were added (none)
Substantial AI assistance, if any, is noted here: written with Claude Code (Claude Fable 5.2)

🤖 Generated with Claude Code

## What Path recovery (rename and skip choices for Windows-invalid paths) now does every file write and delete through one internal, fully tested helper, `WorkingTree`: - the target must resolve inside the working directory; - it must not be in a `.git` folder, in any spelling Windows opens as that folder (letter case, the `GIT~1` short name, trailing dots and spaces, a stream suffix); - it must not be reached through a link; - an existing file is removed and created anew, never written through. Also: - The recovery file `.git\gclo-recovery.json` is validated when it is loaded. A file that lists a path outside the working tree fails the row with a message that says how to rebuild it; nothing is deleted. - `WindowsPathValidator` treats `.git` and its aliases as an invalid segment, with a suggested replacement. - Cleanup of a failed clone skips links when clearing attributes. ## Why Closes #141. The recovery materializes the tree itself instead of using a git checkout, and it was less careful than one. The stored list of previously written files decided what to delete without being checked, a segment named `.git` was valid, and writes followed links. ## Behaviour changes worth a release note - A repository whose tree (or a rename the user chose) would put a file in a `.git` folder is reported as an invalid path, with a suggested name. - A `gclo-recovery.json` that lists a path outside the repository fails that repository until the file is deleted and Resolve is used again. - Recovery refuses to write through a folder that is a link (symbolic link or junction). ## How it was tested - `gclo.Engine.Tests`: 680 passed (was 606). New `WorkingTreeTests` (resolve, create, delete; outside paths; every `.git` spelling; a junction inside the working tree; a file symlink where the platform allows creating one). New recovery tests: a recovery file listing an outside file, absolute and relative, is refused on pull and on apply and the file survives; a rename into `.git`, `.GIT`, `git~1` stays invalid and writes nothing; a folder in the tree that is a junction to elsewhere is refused. Validator tests for the new rule and for names that only start like `.git`. - `gclo.Cli.Tests`: 142 passed. Line coverage 100% on `gclo.Engine`, `gclo.ViewModels`, `gclo`. - Zero-warning x64 build with locked restore; `dotnet format --verify-no-changes` clean. Not run: the UI suite (no shell or view-model change). The file-symlink test returns early when the machine cannot create a symlink, so on such a machine that one case is not exercised; the junction tests always run on Windows. ## Checklist - [x] Commits are signed and signed off (`git commit -S -s`) - [x] Zero build warnings; format and lint are clean - [x] Tests added or updated; the 100% coverage gate still passes - [x] Docs updated for user-visible changes; threat model updated if the attack surface changed (threat model T3, T4 and review log) - [x] No secrets, credentials, or personal data - [x] New dependencies are justified below, or none were added (none) - [x] Substantial AI assistance, if any, is noted here: written with Claude Code (Claude Fable 5.2) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 1 commit 2026-11-10 19:03:40 +00:00

fix(recovery): keep every write and delete inside the working tree c4f2e7258a

Path recovery materializes the working tree itself instead of using a git
checkout, and it was less careful than one: the list of previously written
files in .git\gclo-recovery.json decided what to delete without being
checked, a path segment named .git was valid, writes followed links, and
cleanup of a failed clone followed directory links.

- New internal WorkingTree is the only way the recovery writes or deletes
  a file. The target must resolve inside the working directory, must not
  be in a .git folder (in any spelling Windows opens as that folder: letter
  case, the GIT~1 short name, trailing dots and spaces, a stream suffix),
  and must not be reached through a link. An existing file is removed and
  created anew, never written through.
- The recovery file's stored paths are validated when it is loaded. A file
  that lists a path outside the working tree fails the row with a message
  that says how to rebuild it; nothing is deleted.
- WindowsPathValidator treats .git and its aliases as an invalid segment,
  with a suggested replacement, so a tree entry or a rename can no longer
  land in git's own folder.
- Cleanup of a failed clone skips links when clearing attributes.

Threat model: T3 and T4 updated.

Closes #141

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes added 1 commit 2026-11-10 19:32:36 +00:00

fix(recovery): names Windows opens differently, real links only, safe cleanup a900c5d9dd

Follow-up to the independent review of this branch.

- A folder whose name ends in a space was checked under one name and opened
  under another, so a link behind it could still carry a delete out of the
  working directory. WorkingTree.Resolve now refuses any part that Windows
  does not open literally (empty, trailing dot or space, a colon), and the
  recovery file's stored paths must also pass the path validator.
- A link is recognized by having a target, not by the reparse-point
  attribute: folders under a cloud-sync root carry that attribute without
  being links, and recovery refused to write into them.
- Cleanup of a failed clone moved to WorkingTree.DeleteDirectory, which
  removes a link inside the folder as a link and never enters it; it is
  tested, where the previous change to that cleanup was not.
- The destination's old content is removed whatever it is: a file, a hard
  link, a file link, a folder link. Only the name in the working tree goes.
- The git-folder rule is the one git has: .git and GIT~1 (with trailing
  dots, spaces or a stream suffix). git~2 and up are ordinary names again.
  The rule now runs before the trailing-dot rule, whose suggestion for
  ".git." was ".git" itself; every suggestion re-validates clean.
- Shared parent folders are checked for links once per materialization.
- A bare repository with a recovery file gets a clear message.
- Tests remove the junctions they create, and a hard-link test replaces
  reliance on the symlink test that needs a privilege.

Refs #141

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes commented

2026-11-10 19:33:08 +00:00

Independent review of the first commit (a second reviewer session, read-only, with its own experiments) found one blocker and several smaller defects; all addressed in a900c5d.

Blocker, fixed: a folder whose name ends in a space was checked under one name and opened under another, so a link behind it could still carry a delete out of the working directory. Resolve now refuses any part Windows does not open literally, and the recovery file's stored paths must also pass the path validator.
Fixed: a link was recognized by the reparse-point attribute, which cloud-synced folders (OneDrive) carry without being links; recovery would have refused to write there. A link is now something with a target.
Fixed: the git-folder rule was broader than git's (git~2 and up are ordinary names) and ran after the trailing-dot rule, whose suggestion for .git. was .git itself.
Fixed: cleanup of a failed clone was changed without a test; it is now WorkingTree.DeleteDirectory, tested, and removes a link as a link.
Fixed: a folder link or a read-only link at the destination; shared parent folders checked once per materialization (the first version roughly doubled the time to rewrite 4,000 files); a bare repository gets a clear message.
Tests: junctions are removed explicitly; a hard-link test (no privilege needed) pins the replace-not-overwrite behaviour.

Accepted and documented: the link check and the write are two steps (named as a residual in the threat model). A file another process holds open without delete sharing now fails where an in-place overwrite might have succeeded.

Gates after the second commit: 703 engine tests, 142 CLI tests, 100% line coverage, zero-warning locked build, format clean.

Independent review of the first commit (a second reviewer session, read-only, with its own experiments) found one blocker and several smaller defects; all addressed in a900c5d. - **Blocker, fixed:** a folder whose name ends in a space was checked under one name and opened under another, so a link behind it could still carry a delete out of the working directory. `Resolve` now refuses any part Windows does not open literally, and the recovery file's stored paths must also pass the path validator. - **Fixed:** a link was recognized by the reparse-point attribute, which cloud-synced folders (OneDrive) carry without being links; recovery would have refused to write there. A link is now something with a target. - **Fixed:** the git-folder rule was broader than git's (`git~2` and up are ordinary names) and ran after the trailing-dot rule, whose suggestion for `.git.` was `.git` itself. - **Fixed:** cleanup of a failed clone was changed without a test; it is now `WorkingTree.DeleteDirectory`, tested, and removes a link as a link. - **Fixed:** a folder link or a read-only link at the destination; shared parent folders checked once per materialization (the first version roughly doubled the time to rewrite 4,000 files); a bare repository gets a clear message. - **Tests:** junctions are removed explicitly; a hard-link test (no privilege needed) pins the replace-not-overwrite behaviour. Accepted and documented: the link check and the write are two steps (named as a residual in the threat model). A file another process holds open without delete sharing now fails where an in-place overwrite might have succeeded. Gates after the second commit: 703 engine tests, 142 CLI tests, 100% line coverage, zero-warning locked build, format clean.

james.maes merged commit 539b0867e1 into main

2026-11-10 19:33:12 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/gclo#142