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

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>

docs/security/threat-model.md

@@ -85,8 +85,8 @@ flowchart LR
 | --- | --- | --- | --- | --- | --- | --- | --- |
 | T1 | Update feed | Tampering | Attacker serves a malicious update | Low | Critical | HTTPS to this repository's Releases only; Velopack hash verification; immutable releases; provenance and SBOM attestations; Scorecard; Authenticode pending (EX-0002) | Mitigated (signing pending) |
 | T2 | Package managers / downloads | Tampering | Modified installer or CLI zip on a mirror | Low | High | `SHA256SUMS`; Scoop and Chocolatey manifests pin the hash; "Verifying a release" instructions | Mitigated |
-| T3 | Git remotes | Tampering / EoP | Repository with Windows-invalid or traversal paths (`..`, reserved names, case collisions) writes outside or corrupts the target | Medium | High | Pre-checkout validation of every incoming path, which also refuses a `.git` segment in any spelling Windows opens as that folder; `RepositoryPathResolver` containment; recovery by rename/skip or clone in WSL; `core.longpaths`. The recovery materializes the tree itself, so every write and delete it makes goes through `WorkingTree`: inside the working directory, never in `.git`, never through a link, and an existing file is replaced rather than written through (#141) | Mitigated |
-| T4 | Git remotes | Tampering | Hostile `.git` config or hooks in a pre-existing target repository | Low | High | gclo runs no hooks (libgit2, no shell); only fetch and fast-forward; `gclo.checkoutpending` marks incomplete trees; a folder is fetched only when its `origin` names the listed repository, and the remote contacted is the listed URL, not origin's spelling of it; a `url.<base>.insteadOf` rewrite in the folder's own configuration is refused, and so is its `core.sshCommand` when the fetch is not HTTPS (`RemoteUrl`, #139). The user's global git configuration still applies, as it does to git itself The recovery file `.git\gclo-recovery.json` is validated when it is loaded: a listed path that is not in the working tree fails the row instead of being deleted (#141) | Mitigated for the remote, the transport program, and the recovery file; other `.git` content of an existing folder is still trusted |
+| T3 | Git remotes | Tampering / EoP | Repository with Windows-invalid or traversal paths (`..`, reserved names, case collisions) writes outside or corrupts the target | Medium | High | Pre-checkout validation of every incoming path, which also refuses a `.git` segment in any spelling Windows opens as that folder; `RepositoryPathResolver` containment; recovery by rename/skip or clone in WSL; `core.longpaths`. The recovery materializes the tree itself, so every write and delete it makes goes through `WorkingTree`: inside the working directory, never in `.git`, never through a link, and an existing file is replaced rather than written through (#141). Residual: the link check and the write are two steps, so a link swapped in between them by something already running as the user is not caught | Mitigated |
+| T4 | Git remotes | Tampering | Hostile `.git` config or hooks in a pre-existing target repository | Low | High | gclo runs no hooks (libgit2, no shell); only fetch and fast-forward; `gclo.checkoutpending` marks incomplete trees; a folder is fetched only when its `origin` names the listed repository, and the remote contacted is the listed URL, not origin's spelling of it; a `url.<base>.insteadOf` rewrite in the folder's own configuration is refused, and so is its `core.sshCommand` when the fetch is not HTTPS (`RemoteUrl`, #139). The user's global git configuration still applies, as it does to git itself. The recovery file `.git\gclo-recovery.json` is validated when it is loaded: a listed path that is not in the working tree fails the row instead of being deleted (#141) | Mitigated for the remote, the transport program, and the recovery file; other `.git` content of an existing folder is still trusted |
 | T5 | PAT handling | Information disclosure | Token leaks to disk, logs, process listings, or UI | Medium | Critical | Memory-only in Quick Sync; Credential Manager otherwise; accounts may reference the default token (`TokenSource`), resolved at use time through one resolver and never copied; logs record lengths; no `--token`; WSL via env only; secret scanning + gitleaks + CodeQL in CI | Mitigated |
 | T6 | Credential Manager | Information disclosure | Another process as the same user reads the stored token | Medium | High | Documented boundary (SECURITY.md); vault scoped per data directory so test or portable instances never see the default profile's tokens | Accepted (OS boundary) |
 | T7 | GitHub API | Denial of service | Rate limits or abuse detection stop a sync | Medium | Low | Secondary-rate-limit and abuse handling with backoff; per-repo failure isolation; cancellation | Mitigated |

gclo.Engine.Tests/PathRecoveryTests.cs

@@ -238,21 +238,32 @@ public sealed class PathRecoveryTests : IDisposable
    // ---------------------------------------------------------------- inside the working tree only (#141)
 
    [Theory]
-   [InlineData(false)]
-   [InlineData(true)]
-   public async Task RecoveryFileListingAFileOutsideTheRepository_IsRefused_AndThatFileSurvives(bool relative)
+   [InlineData("absolute")]
+   [InlineData("relative")]
+   [InlineData("trailing-space-folder")]
+   [InlineData("empty")]
+   public async Task RecoveryFileListingAFileOutsideTheRepository_IsRefused_AndThatFileSurvives(string kind)
    {
       string source = CreateForgedRepo(_root, ("bad:file.txt", "bad"), ("keep.txt", "keep"));
-      string target = await CloneExpectingInvalidPathsAsync(source, "outside-" + relative);
+      string target = await CloneExpectingInvalidPathsAsync(source, "outside-" + kind);
       await _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None);
-      string victim = NewPath("victim-" + relative + ".txt");
+      string victim = NewPath("victim-" + kind + ".txt");
       File.WriteAllText(victim, "untouched");
+      string listed = kind switch
+      {
+         "absolute" => victim,
+         "relative" => "../" + Path.GetFileName(victim),
+         // Inside the tree as written, but "docs " is a name a link can hide behind:
+         // whoever checks "docs" and whoever opens "docs " would not mean the same folder.
+         "trailing-space-folder" => "docs /victim.txt",
+         _ => "",
+      };
       // .git\gclo-recovery.json sits on disk between syncs; this is it after someone
       // (or something) added a path the recovery never wrote. The list of materialized
       // paths is what the next apply deletes from.
       string recoveryFile = RecoveryFilePath(target);
       JsonNode document = JsonNode.Parse(File.ReadAllText(recoveryFile))!;
-      document["MaterializedPaths"]!.AsArray().Add(relative ? "../" + Path.GetFileName(victim) : victim);
+      document["MaterializedPaths"]!.AsArray().Add(listed);
       File.WriteAllText(recoveryFile, document.ToJsonString());
       ReplaceForgedEntry(source, remove: "keep.txt", add: ("other.txt", "o"));
 
@@ -286,7 +297,7 @@ public sealed class PathRecoveryTests : IDisposable
 
       InvalidPathInfo info = Assert.Single(ex.Paths);
       Assert.Equal(gitFolderName, info.RepoPath);
-      Assert.Contains("reserved for the repository's own data", info.Reason);
+      Assert.Contains("git's own folder", info.Reason);
       Assert.False(File.Exists(Path.Combine(target, ".git", "poc.txt")));
       Assert.False(File.Exists(Path.Combine(target, "keep.txt"))); // validated before any write
    }
@@ -301,13 +312,49 @@ public sealed class PathRecoveryTests : IDisposable
       string elsewhere = NewPath("elsewhere");
       Directory.CreateDirectory(elsewhere);
       // The folder the tree is about to be written into already exists, as a link.
-      WorkingTreeTests.LinkDirectory(Path.Combine(target, "vendor"), elsewhere);
+      string link = Path.Combine(target, "vendor");
+      WorkingTreeTests.CreateDirectoryLink(link, elsewhere);
+      try
+      {
+         InvalidOperationException ex = await Assert.ThrowsAsync<InvalidOperationException>(
+             () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None));
 
-      InvalidOperationException ex = await Assert.ThrowsAsync<InvalidOperationException>(
-          () => _client.ApplyRecoveryAsync(target, Recovery(renames: [("bad:file.txt", "bad_file.txt")]), CancellationToken.None));
+         Assert.Contains("is a link to another location", ex.Message);
+         Assert.Empty(Directory.EnumerateFileSystemEntries(elsewhere));
+      }
+      finally
+      {
+         WorkingTreeTests.RemoveLink(link);
+      }
+   }
 
-      Assert.Contains("is a link to another location", ex.Message);
-      Assert.Empty(Directory.EnumerateFileSystemEntries(elsewhere));
+
+
+   [Fact]
+   public async Task FailedClone_CleanupNeverLeavesTheFolder_AndRemovesIt()
+   {
+      // A clone that fails after its fetch is removed again. What it leaves must be
+      // deleted without following a link out of the folder.
+      string source = CreateForgedRepo(_root, ("readme.txt", "hello"));
+      string target = NewPath("cleanup");
+      string elsewhere = NewPath("cleanup-elsewhere");
+      Directory.CreateDirectory(elsewhere);
+      string victim = Path.Combine(elsewhere, "victim.txt");
+      File.WriteAllText(victim, "untouched");
+      File.SetAttributes(victim, FileAttributes.ReadOnly);
+      _client.AfterFetchForTesting = () =>
+      {
+         WorkingTreeTests.CreateDirectoryLink(Path.Combine(target, "linked"), elsewhere);
+         throw new InvalidOperationException("fails between fetch and checkout");
+      };
+
+      await Assert.ThrowsAsync<InvalidOperationException>(
+          () => _client.CloneAsync(source, target, Token, null, CancellationToken.None));
+
+      Assert.False(Directory.Exists(target));
+      Assert.Equal("untouched", File.ReadAllText(victim));
+      Assert.True(File.GetAttributes(victim).HasFlag(FileAttributes.ReadOnly));
+      File.SetAttributes(victim, FileAttributes.Normal);
    }
 
 

gclo.Engine.Tests/WindowsPathValidationTests.cs

@@ -65,7 +65,11 @@ public sealed class WindowsPathValidationTests : IDisposable
    [InlineData(".GIT", "_GIT")]
    [InlineData(".Git", "_Git")]
    [InlineData("git~1", "git_1")]
-   [InlineData("GIT~12", "GIT_12")]
+   [InlineData("GIT~1", "GIT_1")]
+   [InlineData(".git.", "_git")]
+   [InlineData(".git ", "_git")]
+   [InlineData("git~1.", "git_1")]
+   [InlineData(".git:stream", "_git_stream")]
    public void ValidatePaths_GitsOwnFolderName_IsInvalid_AsAnySegment(string segment, string expectedSuggestion)
    {
       // git refuses such a tree entry at checkout. The recovery materializes the tree
@@ -75,18 +79,24 @@ public sealed class WindowsPathValidationTests : IDisposable
          InvalidPathInfo info = Assert.Single(WindowsPathValidator.ValidatePaths([path]));
 
          Assert.EndsWith(segment, info.RepoPath, StringComparison.Ordinal);
-         Assert.Contains("reserved for the repository's own data", info.Reason);
+         Assert.Contains("git's own folder", info.Reason);
          Assert.Equal(expectedSuggestion, info.SuggestedName);
+         // The suggestion is itself a valid name: accepting it settles the matter.
+         Assert.Empty(WindowsPathValidator.ValidatePaths([info.SuggestedName!]));
       }
    }
 
 
 
    [Fact]
-   public void ValidatePaths_NamesThatOnlyStartLikeGitsFolder_AreFine()
+   public void ValidatePaths_NamesThatOnlyResembleGitsFolder_AreFine()
    {
+      // git~2 and up are ordinary names: git accepts them and GitHub can host them.
       Assert.Empty(WindowsPathValidator.ValidatePaths(
-          [".github/workflows/ci.yml", ".gitignore", ".gitattributes", ".gitmodules", "git/readme.md", "git~x/readme.md", "docs/.git-notes.md"]));
+          [
+              ".github/workflows/ci.yml", ".gitignore", ".gitattributes", ".gitmodules", "git/readme.md",
+              "git~2/readme.md", "GIT~10/readme.md", "git~x/readme.md", "docs/.git-notes.md",
+          ]));
    }
 
 

gclo.Engine.Tests/WorkingTreeTests.cs

@@ -23,6 +23,8 @@ public sealed class WorkingTreeTests : IDisposable
 
    private readonly string _outside;
 
+   private readonly List<string> _links = [];
+
 
 
    public WorkingTreeTests()
@@ -35,7 +37,16 @@ public sealed class WorkingTreeTests : IDisposable
 
 
 
-   public void Dispose() => TryDeleteDirectory(_root);
+   public void Dispose()
+   {
+      // Links first, as links: a recursive delete that meets a junction stops there
+      // and leaves the folders above it behind.
+      foreach(string link in _links)
+      {
+         RemoveLink(link);
+      }
+      TryDeleteDirectory(_root);
+   }
 
 
 
@@ -48,8 +59,10 @@ public sealed class WorkingTreeTests : IDisposable
    [InlineData(".gitignore")]
    [InlineData(".gitattributes")]
    [InlineData("docs/git~notes.md")]
+   [InlineData("git~2/file.txt")]
+   [InlineData("GIT~10/file.txt")]
    [InlineData("git~x/file.txt")]
-   [InlineData("git~/file.txt")]
+   [InlineData("name with spaces/and.dots.txt")]
    public void Resolve_PathInsideTheWorkingTree_IsItsFullPath(string repoPath)
    {
       string expected = Path.Combine(_workdir, repoPath.Replace('/', Path.DirectorySeparatorChar));
@@ -99,8 +112,13 @@ public sealed class WorkingTreeTests : IDisposable
    [InlineData(".git")]
    [InlineData("git~1/config")]
    [InlineData("GIT~1/config")]
-   [InlineData("sub/git~12/config")]
+   [InlineData("sub/Git~1/config")]
    [InlineData("src/../.git/config")]
+   [InlineData(".git./config")]
+   [InlineData(".git /config")]
+   [InlineData(".git.. ./config")]
+   [InlineData("GIT~1./config")]
+   [InlineData("sub/.GIT. /hooks/pre-commit")]
    public void Resolve_PathInsideAGitFolder_AtAnyDepthOrSpelling_IsRefused(string repoPath)
    {
       InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() => WorkingTree.Resolve(_workdir, repoPath));
@@ -111,18 +129,18 @@ public sealed class WorkingTreeTests : IDisposable
 
 
    [Theory]
-   [InlineData(".git./config")]
-   [InlineData(".git /config")]
-   [InlineData(".git.. ./config")]
-   [InlineData("GIT~1./config")]
-   [InlineData("sub/.GIT. /hooks/pre-commit")]
-   public void Resolve_TrailingDotsAndSpaces_CannotDisguiseTheGitFolder(string repoPath)
+   [InlineData("docs /victim.txt")]
+   [InlineData("a/b. ./c.txt")]
+   [InlineData("docs/")]
+   [InlineData("notes.txt:stream")]
+   [InlineData("folder:stream/file.txt")]
+   public void Resolve_PartThatWindowsWouldOpenUnderAnotherName_IsRefused(string repoPath)
    {
-      // Windows drops trailing dots and spaces when it opens a path, so these all open
-      // the real .git folder there.
+      // "docs " and "docs" are two different folders to whoever checks the first and
+      // one folder to Windows when it opens the second: such a name is never trusted.
       InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() => WorkingTree.Resolve(_workdir, repoPath));
 
-      Assert.Contains("'.git' folder", ex.Message);
+      Assert.Contains("does not open under the name it is written with", ex.Message);
    }
 
 
@@ -151,7 +169,7 @@ public sealed class WorkingTreeTests : IDisposable
    [InlineData(".git", true)]
    [InlineData(".GIT", true)]
    [InlineData("git~1", true)]
-   [InlineData("GIT~27", true)]
+   [InlineData("GIT~1", true)]
    [InlineData(".git.", true)]
    [InlineData(".git ", true)]
    [InlineData(".GIT. .", true)]
@@ -159,16 +177,18 @@ public sealed class WorkingTreeTests : IDisposable
    [InlineData(".git::$INDEX_ALLOCATION", true)]
    [InlineData(".git:stream", true)]
    [InlineData("git~1::$INDEX_ALLOCATION", true)]
-   [InlineData(".gitx:stream", false)]
-   [InlineData(".", false)]
-   [InlineData(" ", false)]
+   [InlineData("git~2", false)] // a name git itself accepts, and GitHub can host
+   [InlineData("GIT~27", false)]
    [InlineData("git~", false)]
    [InlineData("git~1a", false)]
+   [InlineData(".gitx:stream", false)]
    [InlineData(".github", false)]
    [InlineData(".gitignore", false)]
    [InlineData("git", false)]
+   [InlineData(".", false)]
+   [InlineData(" ", false)]
    [InlineData("", false)]
-   public void IsGitDirectoryName_TheNameAndItsShortNameAlias_Only(string segment, bool expected)
+   public void IsGitDirectoryName_TheNamesGitItselfRefuses_Only(string segment, bool expected)
    {
       Assert.Equal(expected, WorkingTree.IsGitDirectoryName(segment));
    }
@@ -221,6 +241,49 @@ public sealed class WorkingTreeTests : IDisposable
 
 
 
+   [Fact]
+   public void Create_ALinkDeeperOnTheWay_IsRefusedToo_EvenWithFoldersAlreadyVerified()
+   {
+      var verified = new HashSet<string>(StringComparer.Ordinal);
+      using(WorkingTree.Create(_workdir, "src/app/main.cs", verified))
+      {
+      }
+      Assert.Contains(Path.Combine(_workdir, "src"), verified);
+      Assert.Contains(Path.Combine(_workdir, "src", "app"), verified);
+      LinkDirectory(Path.Combine(_workdir, "src", "app", "vendor"), _outside);
+
+      // The shared parents are remembered; the new folder is still checked.
+      using(WorkingTree.Create(_workdir, "src/app/util.cs", verified))
+      {
+      }
+      Assert.Throws<InvalidOperationException>(() => WorkingTree.Create(_workdir, "src/app/vendor/planted.txt", verified));
+
+      Assert.Empty(Directory.EnumerateFileSystemEntries(_outside));
+   }
+
+
+
+   [Fact]
+   public void Create_WhereAHardLinkSits_ReplacesIt_AndLeavesTheOtherNameOfThatFileAlone()
+   {
+      // A hard link needs no privilege, and writing "in place" would change the file
+      // under its other name too.
+      string victim = Path.Combine(_outside, "victim.txt");
+      File.WriteAllText(victim, "untouched");
+      string link = Path.Combine(_workdir, "report.txt");
+      HardLink(link, victim);
+
+      using(FileStream file = WorkingTree.Create(_workdir, "report.txt"))
+      {
+         file.Write("payload"u8);
+      }
+
+      Assert.Equal("untouched", File.ReadAllText(victim));
+      Assert.Equal("payload", File.ReadAllText(link));
+   }
+
+
+
    [Fact]
    public void Create_WhereAFileLinkSits_ReplacesTheLink_AndLeavesItsTargetAlone()
    {
@@ -233,7 +296,7 @@ public sealed class WorkingTreeTests : IDisposable
       }
       catch(Exception ex) when(ex is IOException or UnauthorizedAccessException)
       {
-         return; // creating a file symlink needs a privilege (or Developer Mode) on Windows
+         return; // a file symlink needs a privilege (or Developer Mode) on Windows; the hard-link test above always runs
       }
 
       using(FileStream file = WorkingTree.Create(_workdir, "report.txt"))
@@ -248,6 +311,24 @@ public sealed class WorkingTreeTests : IDisposable
 
 
 
+   [Fact]
+   public void Create_WhereAFolderLinkSits_ReplacesTheLinkWithTheFile_AndLeavesItsTargetAlone()
+   {
+      File.WriteAllText(Path.Combine(_outside, "kept.txt"), "kept");
+      string link = Path.Combine(_workdir, "report.txt");
+      LinkDirectory(link, _outside);
+
+      using(FileStream file = WorkingTree.Create(_workdir, "report.txt"))
+      {
+         file.Write("payload"u8);
+      }
+
+      Assert.Equal("payload", File.ReadAllText(link));
+      Assert.Equal("kept", File.ReadAllText(Path.Combine(_outside, "kept.txt")));
+   }
+
+
+
    [Fact]
    public void Create_OutsideOrInGit_IsRefused_BeforeAnythingIsTouched()
    {
@@ -255,9 +336,11 @@ public sealed class WorkingTreeTests : IDisposable
 
       Assert.Throws<InvalidOperationException>(() => WorkingTree.Create(_workdir, "../outside/planted.txt"));
       Assert.Throws<InvalidOperationException>(() => WorkingTree.Create(_workdir, ".git/config"));
+      Assert.Throws<InvalidOperationException>(() => WorkingTree.Create(_workdir, "docs /planted.txt"));
 
       Assert.Empty(Directory.EnumerateFileSystemEntries(_outside));
       Assert.Empty(Directory.EnumerateFileSystemEntries(Path.Combine(_workdir, ".git")));
+      Assert.Single(Directory.EnumerateFileSystemEntries(_workdir)); // only .git
    }
 
 
@@ -334,36 +417,145 @@ public sealed class WorkingTreeTests : IDisposable
 
 
 
+   // ---------------------------------------------------------------- delete a directory
+
+   [Fact]
+   public void DeleteDirectory_RemovesEverything_ReadOnlyFilesAndFoldersIncluded()
+   {
+      string folder = Path.Combine(_workdir, "half-clone");
+      string pack = Path.Combine(folder, ".git", "objects", "pack", "pack-1.idx");
+      Directory.CreateDirectory(Path.GetDirectoryName(pack)!);
+      File.WriteAllText(pack, "x");
+      File.SetAttributes(pack, FileAttributes.ReadOnly | FileAttributes.Hidden);
+      File.WriteAllText(Path.Combine(folder, "readme.txt"), "x");
+      new DirectoryInfo(Path.Combine(folder, ".git")).Attributes |= FileAttributes.Hidden | FileAttributes.ReadOnly;
+
+      WorkingTree.DeleteDirectory(folder);
+
+      Assert.False(Directory.Exists(folder));
+   }
+
+
+
+   [Fact]
+   public void DeleteDirectory_ALinkInside_IsRemovedAsALink_AndWhatItPointsAtIsNeitherEnteredNorChanged()
+   {
+      string victim = Path.Combine(_outside, "victim.txt");
+      File.WriteAllText(victim, "untouched");
+      File.SetAttributes(victim, FileAttributes.ReadOnly);
+      string folder = Path.Combine(_workdir, "half-clone");
+      Directory.CreateDirectory(Path.Combine(folder, "src"));
+      LinkDirectory(Path.Combine(folder, "src", "vendor"), _outside);
+
+      WorkingTree.DeleteDirectory(folder);
+
+      Assert.False(Directory.Exists(folder));
+      Assert.Equal("untouched", File.ReadAllText(victim));
+      Assert.True(File.GetAttributes(victim).HasFlag(FileAttributes.ReadOnly), "the linked folder's files keep their attributes");
+      File.SetAttributes(victim, FileAttributes.Normal);
+   }
+
+
+
+   [Fact]
+   public void DeleteDirectory_OfALink_RemovesOnlyTheLink()
+   {
+      File.WriteAllText(Path.Combine(_outside, "kept.txt"), "kept");
+      string link = Path.Combine(_workdir, "linked");
+      LinkDirectory(link, _outside);
+
+      WorkingTree.DeleteDirectory(link);
+
+      Assert.False(Directory.Exists(link));
+      Assert.Equal("kept", File.ReadAllText(Path.Combine(_outside, "kept.txt")));
+   }
+
+
+
+   [Fact]
+   public void DeleteDirectory_MissingFolder_IsFine_AndABlankPathIsACallerError()
+   {
+      WorkingTree.DeleteDirectory(Path.Combine(_workdir, "never-existed"));
+
+      Assert.Throws<ArgumentException>(() => WorkingTree.DeleteDirectory(" "));
+   }
+
+
+
    // ---------------------------------------------------------------- helpers
 
    /// <summary>
    /// Makes <paramref name="link"/> a directory link to <paramref name="target"/>: a
    /// junction on Windows (which, unlike a symbolic link, needs no privilege), a
-   /// symbolic link elsewhere.
+   /// symbolic link elsewhere. The link is removed again when the test ends.
    /// </summary>
-   internal static void LinkDirectory(string link, string target)
+   private void LinkDirectory(string link, string target)
    {
-      if(!OperatingSystem.IsWindows())
+      CreateDirectoryLink(link, target);
+      _links.Add(link);
+   }
+
+
+
+   /// <summary>Creates a directory link without tracking it; the caller removes it with <see cref="RemoveLink"/>.</summary>
+   internal static void CreateDirectoryLink(string link, string target)
+   {
+      if(OperatingSystem.IsWindows())
+      {
+         Run("cmd.exe", "/c", "mklink", "/J", link, target);
+      }
+      else
       {
          Directory.CreateSymbolicLink(link, target);
-         return;
       }
+      Assert.NotNull(new DirectoryInfo(link).LinkTarget);
+   }
 
-      var startInfo = new ProcessStartInfo("cmd.exe")
+
+
+   /// <summary>Removes a directory link itself (never its target); fine when it is already gone.</summary>
+   internal static void RemoveLink(string link)
+   {
+      var info = new DirectoryInfo(link);
+      if(info.LinkTarget is not null)
+      {
+         info.Delete();
+      }
+   }
+
+
+
+   private static void HardLink(string link, string target)
+   {
+      if(OperatingSystem.IsWindows())
+      {
+         Run("cmd.exe", "/c", "mklink", "/H", link, target);
+      }
+      else
+      {
+         Run("ln", target, link);
+      }
+   }
+
+
+
+   private static void Run(string fileName, params string[] arguments)
+   {
+      var startInfo = new ProcessStartInfo(fileName)
       {
          UseShellExecute = false,
          CreateNoWindow = true,
          RedirectStandardOutput = true,
          RedirectStandardError = true,
       };
-      startInfo.ArgumentList.Add("/c");
-      startInfo.ArgumentList.Add("mklink");
-      startInfo.ArgumentList.Add("/J");
-      startInfo.ArgumentList.Add(link);
-      startInfo.ArgumentList.Add(target);
+      foreach(string argument in arguments)
+      {
+         startInfo.ArgumentList.Add(argument);
+      }
       using Process process = Process.Start(startInfo)!;
+      string error = process.StandardError.ReadToEnd();
+      process.StandardOutput.ReadToEnd();
       process.WaitForExit();
-      Assert.True(process.ExitCode == 0, "mklink /J failed: " + process.StandardError.ReadToEnd());
-      Assert.True(new DirectoryInfo(link).Attributes.HasFlag(FileAttributes.ReparsePoint));
+      Assert.True(process.ExitCode == 0, $"{fileName} {string.Join(' ', arguments)} failed: {error}");
    }
 }

gclo.Engine/LibGit2GitClient.cs

@@ -269,7 +269,7 @@ public sealed class LibGit2GitClient : IGitClient
          // tip through the stored mapping. Freshly-invalid paths the mapping does
          // not cover surface as a typed failure listing the effective paths.
          AdvanceHeadToTrackedTip(repo);
-         RecoveryDocument stored = LoadRecoveryDocument(recoveryFile, repo.Info.WorkingDirectory);
+         RecoveryDocument stored = LoadRecoveryDocument(recoveryFile, WorkingDirectoryOf(repo));
          if(stored.MaterializedTip is not null && string.Equals(stored.MaterializedTip, repo.Head.Tip?.Sha, StringComparison.Ordinal))
          {
             // The tree on disk already reflects this tip: nothing to rewrite. A
@@ -676,8 +676,9 @@ public sealed class LibGit2GitClient : IGitClient
          repo.Config.Set("core.longpaths", true, ConfigurationLevel.Local);
       }
 
+      string workdir = WorkingDirectoryOf(repo);
       string recoveryFile = GetRecoveryFilePath(repo);
-      RecoveryDocument? previous = File.Exists(recoveryFile) ? LoadRecoveryDocument(recoveryFile, repo.Info.WorkingDirectory) : null;
+      RecoveryDocument? previous = File.Exists(recoveryFile) ? LoadRecoveryDocument(recoveryFile, workdir) : null;
       if(previous is not null)
       {
          recovery = MergeRecoveries(stored: ToRecovery(previous), incoming: recovery);
@@ -697,7 +698,6 @@ public sealed class LibGit2GitClient : IGitClient
          throw new InvalidRepositoryPathsException(stillInvalid);
       }
 
-      string workdir = repo.Info.WorkingDirectory;
 
       // A real checkout removes what left the tree; this materialization must too,
       // or a recovery-managed working tree accumulates every file upstream ever
@@ -711,6 +711,8 @@ public sealed class LibGit2GitClient : IGitClient
          }
       }
 
+      // Parent folders are shared by many files; each is checked for a link once.
+      var verifiedFolders = new HashSet<string>(StringComparer.Ordinal);
       foreach((string? originalPath, string? effectivePath, Blob? blob) in entries)
       {
          ct.ThrowIfCancellationRequested();
@@ -719,7 +721,7 @@ public sealed class LibGit2GitClient : IGitClient
          // exactly as a checkout of that entry would. Force semantics: whatever is
          // at the destination is replaced, read-only or not (see WorkingTree).
          using Stream content = blob.GetContentStream(new FilteringOptions(originalPath));
-         using FileStream file = WorkingTree.Create(workdir, effectivePath);
+         using FileStream file = WorkingTree.Create(workdir, effectivePath, verifiedFolders);
          content.CopyTo(file);
       }
 
@@ -938,12 +940,18 @@ public sealed class LibGit2GitClient : IGitClient
       {
          try
          {
+            // Every path gclo ever recorded here had passed the validator and names a
+            // file in the tree; one that does not was not written by gclo.
+            if(string.IsNullOrEmpty(path) || WindowsPathValidator.ValidatePaths([path]).Count > 0)
+            {
+               throw new InvalidOperationException("not a path gclo could have written");
+            }
             WorkingTree.Resolve(workdir, path);
          }
-         catch(Exception ex) when(ex is InvalidOperationException or ArgumentException)
+         catch(Exception ex) when(ex is InvalidOperationException or ArgumentException or IOException or NotSupportedException)
          {
             throw new InvalidOperationException(
-                $"The path-recovery file '{filePath}' lists a file that is not in this repository's working tree ('{path}'). "
+                $"The path-recovery file '{filePath}' lists a file that is not in this repository's working tree ('{DisplayPath(path)}'). "
                 + "Delete it and use Resolve on this repository again to rebuild it.", ex);
          }
       }
@@ -953,6 +961,23 @@ public sealed class LibGit2GitClient : IGitClient
 
 
 
+   /// <summary>A path from an untrusted file, cut to a length that fits a message.</summary>
+   private static string DisplayPath(string? path)
+       => path is null ? "" : path.Length <= 200 ? path : path[..200] + "...";
+
+
+
+   /// <summary>
+   /// The working directory path recovery writes into. A bare repository has none,
+   /// and nothing gclo clones is bare: one that carries a recovery file was put
+   /// together by hand.
+   /// </summary>
+   private static string WorkingDirectoryOf(Repository repo)
+       => repo.Info.WorkingDirectory
+           ?? throw new InvalidOperationException("This is a bare repository; it has no working tree to materialize.");
+
+
+
    private static PathRecovery ToRecovery(RecoveryDocument document)
        => new(
            new Dictionary<string, string>(document.SegmentRenames ?? new(StringComparer.Ordinal), StringComparer.Ordinal),
@@ -1008,20 +1033,10 @@ public sealed class LibGit2GitClient : IGitClient
          {
             return;
          }
-         // Pack files under .git are written read-only; clear attributes so delete
-         // succeeds. Links are skipped, so the walk never leaves the folder (a checked
-         // out symlink to a directory would otherwise lead it elsewhere); the delete
-         // itself removes a link, not what it points at (#141).
-         var insideOnly = new EnumerationOptions
-         {
-            RecurseSubdirectories = true,
-            AttributesToSkip = FileAttributes.ReparsePoint,
-         };
-         foreach(string file in Directory.EnumerateFiles(path, "*", insideOnly))
-         {
-            File.SetAttributes(file, FileAttributes.Normal);
-         }
-         Directory.Delete(path, recursive: true);
+         // Read-only pack files and all; never through a link inside the folder (a
+         // checked-out symlink to a directory would otherwise lead the delete
+         // elsewhere), see WorkingTree (#141).
+         WorkingTree.DeleteDirectory(path);
       }
       catch
       {

gclo.Engine/WindowsPathValidator.cs

@@ -254,6 +254,16 @@ public static class WindowsPathValidator
       {
          return ($"segment longer than {MaxSegmentLength} characters", segment[..MaxSegmentLength]);
       }
+      if(WorkingTree.IsGitDirectoryName(segment))
+      {
+         // git refuses such an entry at checkout; the recovery's own materialization
+         // must too, or a tree entry (or a rename) could write into .git (#141).
+         // Checked before the trailing-dot rule, whose suggestion for ".git." would
+         // be ".git" itself.
+         string plain = new string(segment.TrimEnd(' ', '.').Select(c => s_invalidChars.Contains(c) ? '_' : c).ToArray());
+         return ("this name is git's own folder ('.git', or its short name 'GIT~1')", plain.Replace('.', '_').Replace('~', '_'));
+      }
+
       if(segment.EndsWith(' ') || segment.EndsWith('.'))
       {
          string trimmed = segment.TrimEnd(' ', '.');
@@ -265,13 +275,6 @@ public static class WindowsPathValidator
          return ("contains a character that is invalid on Windows", sanitized);
       }
 
-      if(WorkingTree.IsGitDirectoryName(segment))
-      {
-         // git refuses such an entry at checkout; the recovery's own materialization
-         // must too, or a tree entry (or a rename) could write into .git (#141).
-         return ("'.git' is reserved for the repository's own data", segment.Replace('.', '_').Replace('~', '_'));
-      }
-
       string baseName = segment.Split('.')[0];
       if(s_reservedNames.Contains(baseName))
       {

gclo.Engine/WorkingTree.cs

@@ -14,21 +14,28 @@ namespace gclo.Engine;
 /// tree, a rename the user typed, or a file on disk that anything could have edited
 /// (#141).
 /// </summary>
+/// <remarks>
+/// The link check and the write that follows it are two steps. Something that swaps
+/// a folder for a link between them, on the user's own machine and with the user's
+/// own rights, is outside what this guards against.
+/// </remarks>
 internal static class WorkingTree
 {
    /// <summary>
    /// The full path of <paramref name="repoPath"/> (forward-slash separated, relative)
-   /// under <paramref name="workdir"/>. Throws when the path would leave the working
-   /// directory, or names the repository's own <c>.git</c> folder at any depth, in any
-   /// of the spellings Windows opens as that folder (see <see cref="IsGitDirectoryName"/>).
-   /// The check is made on the resolved path, so <c>..</c> cannot hide either.
+   /// under <paramref name="workdir"/>. Throws unless the path, once resolved, is
+   /// strictly inside the working directory and every segment of it is a name that
+   /// Windows opens literally: not empty, no trailing dot or space (Windows drops
+   /// those, so <c>docs </c> and <c>docs</c> would be two spellings of one check and
+   /// two different folders on disk), no colon (a stream of another file), and not
+   /// git's own folder (see <see cref="IsGitDirectoryName"/>).
    /// </summary>
    public static string Resolve(string workdir, string repoPath)
    {
       ArgumentException.ThrowIfNullOrWhiteSpace(workdir);
       ArgumentException.ThrowIfNullOrWhiteSpace(repoPath);
 
-      string root = Path.TrimEndingDirectorySeparator(Path.GetFullPath(workdir));
+      string root = Root(workdir);
       string full = Path.GetFullPath(Path.Combine(root, repoPath.Replace('/', Path.DirectorySeparatorChar)));
 
       if(!full.StartsWith(root + Path.DirectorySeparatorChar, PathComparison))
@@ -42,6 +49,13 @@ internal static class WorkingTree
          {
             throw new InvalidOperationException($"'{repoPath}' is inside a '.git' folder, which belongs to git, not to the working tree.");
          }
+
+         if(segment.Length == 0 || segment[^1] is '.' or ' ' || segment.Contains(':', StringComparison.Ordinal))
+         {
+            throw new InvalidOperationException(
+                $"'{repoPath}' has a part that Windows does not open under the name it is written with "
+                + "(empty, ending in a dot or a space, or containing a colon).");
+         }
       }
 
       return full;
@@ -50,12 +64,14 @@ internal static class WorkingTree
 
 
    /// <summary>
-   /// True for every name Windows opens as git's own folder: <c>.git</c> in any letter
-   /// case; the 8.3 short name NTFS may give it (<c>GIT~1</c>, <c>GIT~2</c>, ...);
-   /// either with trailing dots or spaces, which Windows drops when it opens a path;
-   /// and either followed by a stream name (<c>.git::$INDEX_ALLOCATION</c>), which
+   /// True for the names git itself refuses in a tree because Windows opens them as
+   /// the repository's own folder: <c>.git</c> in any letter case, and <c>GIT~1</c>,
+   /// the short name NTFS gives it; either with trailing dots or spaces, which Windows
+   /// drops; either followed by a stream name (<c>.git::$INDEX_ALLOCATION</c>), which
    /// names the same folder. The rule is the same on every platform, so a path's
-   /// answer does not depend on where it is asked.
+   /// answer does not depend on where it is asked. (Should NTFS have given
+   /// <c>.git</c> another short name, <see cref="Resolve"/> still refuses it:
+   /// resolving a path expands short names.)
    /// </summary>
    public static bool IsGitDirectoryName(string segment)
    {
@@ -63,27 +79,27 @@ internal static class WorkingTree
 
       int stream = segment.IndexOf(':', StringComparison.Ordinal);
       ReadOnlySpan<char> name = (stream < 0 ? segment : segment[..stream]).AsSpan().TrimEnd(". ");
-      if(name.Equals(".git", StringComparison.OrdinalIgnoreCase))
-      {
-         return true;
-      }
-
-      return name.Length > 4
-          && name.StartsWith("git~", StringComparison.OrdinalIgnoreCase)
-          && name[4..].IndexOfAnyExceptInRange('0', '9') < 0;
+      return name.Equals(".git", StringComparison.OrdinalIgnoreCase)
+          || name.Equals("git~1", StringComparison.OrdinalIgnoreCase);
    }
 
 
 
    /// <summary>
    /// Opens <paramref name="repoPath"/> for writing as a new, empty file, creating its
-   /// folders. A file that is already there is removed first, read-only or not, so the
-   /// content never goes through a link someone left in its place.
+   /// folders. Whatever file is already there is removed first, read-only or not, so
+   /// the content never goes through a link (symbolic or hard) left in its place.
    /// </summary>
-   public static FileStream Create(string workdir, string repoPath)
+   /// <param name="workdir">The repository's working directory.</param>
+   /// <param name="repoPath">Forward-slash separated path relative to it.</param>
+   /// <param name="verifiedFolders">
+   /// Folders already found not to be links during the same operation; pass one set
+   /// to every call of one materialization so shared parent folders are checked once.
+   /// </param>
+   public static FileStream Create(string workdir, string repoPath, ISet<string>? verifiedFolders = null)
    {
       string full = Resolve(workdir, repoPath);
-      RefuseLinkOnTheWay(workdir, full);
+      RefuseLinkOnTheWay(workdir, full, verifiedFolders);
 
       Directory.CreateDirectory(Path.GetDirectoryName(full)!);
       RemoveFile(full);
@@ -99,10 +115,10 @@ internal static class WorkingTree
    public static void Delete(string workdir, string repoPath)
    {
       string full = Resolve(workdir, repoPath);
-      RefuseLinkOnTheWay(workdir, full);
+      RefuseLinkOnTheWay(workdir, full, verifiedFolders: null);
       RemoveFile(full);
 
-      string root = Path.TrimEndingDirectorySeparator(Path.GetFullPath(workdir));
+      string root = Root(workdir);
       string? directory = Path.GetDirectoryName(full);
       while(directory is not null
           && !string.Equals(Path.TrimEndingDirectorySeparator(directory), root, PathComparison)
@@ -116,43 +132,100 @@ internal static class WorkingTree
 
 
 
+   /// <summary>
+   /// Removes <paramref name="directory"/> and everything in it, read-only or not,
+   /// without ever leaving it: a link inside (symbolic link or junction) is removed
+   /// as a link, and what it points at is neither entered nor changed. A missing
+   /// directory is fine.
+   /// </summary>
+   public static void DeleteDirectory(string directory)
+   {
+      ArgumentException.ThrowIfNullOrWhiteSpace(directory);
+
+      var folder = new DirectoryInfo(directory);
+      if(folder.LinkTarget is not null)
+      {
+         folder.Delete(); // the link itself
+         return;
+      }
+
+      if(!folder.Exists)
+      {
+         return;
+      }
+
+      foreach(DirectoryInfo child in folder.EnumerateDirectories())
+      {
+         DeleteDirectory(child.FullName);
+      }
+
+      foreach(FileInfo file in folder.EnumerateFiles())
+      {
+         RemoveFile(file.FullName);
+      }
+
+      folder.Attributes = FileAttributes.Directory;
+      folder.Delete();
+   }
+
+
+
    private static StringComparison PathComparison
        => OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
 
 
 
+   private static string Root(string workdir) => Path.TrimEndingDirectorySeparator(Path.GetFullPath(workdir));
+
+
+
    /// <summary>
    /// A folder between the working directory and <paramref name="full"/> that is a
    /// link (a symbolic link or a junction) would carry the write or delete somewhere
    /// else. Nothing in a tree gclo materializes is ever a link, so one on the way is
-   /// refused.
+   /// refused. A link is recognized by having a target, not by the reparse-point
+   /// attribute, which cloud-synced folders carry without leading anywhere.
    /// </summary>
-   private static void RefuseLinkOnTheWay(string workdir, string full)
+   private static void RefuseLinkOnTheWay(string workdir, string full, ISet<string>? verifiedFolders)
    {
-      string root = Path.TrimEndingDirectorySeparator(Path.GetFullPath(workdir));
+      string root = Root(workdir);
       string current = root;
       string[] segments = full[(root.Length + 1)..].Split(Path.DirectorySeparatorChar);
       for(int i = 0; i < segments.Length - 1; i++)
       {
          current = Path.Combine(current, segments[i]);
-         var folder = new DirectoryInfo(current);
-         if(folder.Exists && folder.Attributes.HasFlag(FileAttributes.ReparsePoint))
+         if(verifiedFolders?.Contains(current) == true)
+         {
+            continue;
+         }
+
+         if(new DirectoryInfo(current).LinkTarget is not null)
          {
             throw new InvalidOperationException(
                 $"'{current}' is a link to another location, so gclo will not write or delete through it. "
                 + "Remove the link, then try again.");
          }
+
+         verifiedFolders?.Add(current);
       }
    }
 
 
 
    /// <summary>
-   /// Removes one file, read-only or not, when it is there; removes a link itself (even
-   /// one whose target is gone), never what it points at.
+   /// Removes what is at <paramref name="full"/> when it is a file or a link, read-only
+   /// or not: a link is removed itself (even one whose target is gone, even one to a
+   /// directory), never what it points at.
    /// </summary>
    private static void RemoveFile(string full)
    {
+      var directory = new DirectoryInfo(full);
+      if(directory.LinkTarget is not null && directory.Attributes.HasFlag(FileAttributes.Directory))
+      {
+         directory.Delete(); // a junction or a symbolic link to a directory
+         return;
+      }
+
       var file = new FileInfo(full);
       bool isLink = file.LinkTarget is not null;
       if(!file.Exists && !isLink)
@@ -160,7 +233,9 @@ internal static class WorkingTree
          return;
       }
 
-      if(!isLink)
+      // Clearing read-only on a link acts on the link itself on Windows, but on what
+      // it points at elsewhere; there a link is deleted whatever its mode says.
+      if(!isLink || OperatingSystem.IsWindows())
       {
          file.Attributes = FileAttributes.Normal;
       }