Skip to content

Delete a Physical folder through handles, so no link swapped in can steer it outside the root - #76

Merged
serionist merged 5 commits into
mainfrom
70-link-safe-delete
Oct 13, 2026
Merged

serionist merged 5 commits into
mainfrom
70-link-safe-delete

Conversation

@serionist

@serionist serionist commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

What and why

Step 3 of the 2027.1.1 plan. Physical deleted a folder with .NET's Directory.Delete(path, recursive: true), which goes into each subfolder by its path: a process that can create links inside the root could swap a subfolder, or a folder above it, for a link while the delete ran and steer it into a folder elsewhere, outside the root, with RefuseLinks on or off (#70). Now a folder is deleted through handles.

  • The walk (FolderDelete, shared): each entry is opened relative to its folder's handle without following a link and removed relative to that handle, or through its own on Windows. A link inside the folder, even one swapped in while the delete runs, is removed as itself and never gone into. Without RefuseLinks, the folder that holds the target is opened by its path (links followed, as every member does); with it, the walk starts at the root and opens every folder on the way without following one, so a link swapped in there after the check is refused with the same ArgumentException. Names come from listing each folder by its path; every act goes through a handle, so a listing that a rename or a swapped link made read elsewhere can only make the delete miss names and fail. Errors as .NET's recursive delete gives them: it goes on past what it can't delete, skips what's gone, and throws the first failure.
  • Linux and macOS (UnixFolders): openat with O_DIRECTORY | O_NOFOLLOW (and O_PATH on Linux, which needs no read permission), readlinkat to tell a link from a file, unlinkat. The flags and error numbers come from a table picked by system and ProcessArchitecture: Linux x64, Arm and Arm64, macOS.
  • Windows (WindowsFolders): NtCreateFile relative to the folder's handle with FILE_OPEN_REPARSE_POINT; a name-surrogate reparse point (a symbolic link, a junction, a mount point) is a link; each entry is deleted through its handle with POSIX semantics, falling back to the classic delete where the file system refuses them.
  • Elsewhere (another Linux architecture, another system): Directory.Delete, as before, and the docs say so.
  • Visible differences (the plan's D5, in the changelog): on Windows a folder holding a junction is now deleted (.NET threw UnauthorizedAccessException); a folder that still has entries after the walk throws IOException where it returned true; on macOS the folder above the deleted one (and with RefuseLinks every folder on the way) must be readable. A file is still deleted by its path.
  • Coverage (the plan's D7, the maintainer's yes): CI measures on Linux, so WindowsFolders, which holds only the Windows calls and their argument marshalling, is [ExcludeFromCodeCoverage] with the reason. Everything that decides is shared code measured on Linux: what an entry is (WindowsRules, UnixTable), what each error means, when to fall back, the \\?\ form. The Windows job runs every line of WindowsFolders, the classic delete through a test switch.

Docs: Physical's class remarks (a new paragraph on deleting a folder), RefuseLinks' remarks (the recursive delete's caveat goes), the Physical README, AGENTS.md's Physical tests line, the changelog, the plan's progress log.

Plan

docs/plans/2026-10-13-2027-1-1.md, step 3, approved by @serionist. The progress log records where the code departs from the plan's names (Folders for DeletesByHandle, ClassicDeletes).

Tested by

  • PhysicalFolderDeleteTests (new), on Linux, Windows, macOS and Linux Arm64:
    • through the Opening hook, a subfolder swapped for a link to a folder outside the root while its parent is deleted: the link goes, the outside folder keeps everything (with RefuseLinks on and off); with RefuseLinks, a folder on the way swapped for a link after the check is refused, and so is the target swapped for one; a folder on the way swapped for a file deletes nothing; a target folder replaced by a file is deleted; an entry turned from a folder into a file, or gone between the listing and its open; a target gone before it is opened;
    • links that were there: a link at the target (inside and outside the root) is removed and its target kept; with RefuseLinks it is refused; a folder holding a link to a folder (a junction on Windows), a link to a file and a link to nothing is deleted, the targets kept;
    • what the system refuses: a file open for reading inside; a read-only file (Windows) and a folder that may not be changed or searched (Linux, macOS) throw UnauthorizedAccessException, the rest deleted;
    • the delete by path where the system isn't known; the classic delete; on Windows, the classic delete with a file open in the folder throws IOException;
    • through a scripted system: a removal refused, each error that falls back to the classic delete and one that doesn't, a folder not empty, an entry that can't be opened, a folder that can't be listed, the start (by path, on the way, the target) gone or refused, the target's removal refused or gone;
    • the rules: the table each system and architecture picks, the flags it composes, what each error means on each system, what Windows' attributes and tags make an entry, the \\?\ form of a drive path, a drive's root and a share.
  • Every delete test of the conformance suite and PhysicalLinkAndSweepTests now runs through the walk.
  • Locally, every suite outside Live with Docker required (Azurite, S3Mock, MinIO, fake-gcs-server), on Windows: 2,833 tests, 0 failed. Physical on Linux x64 in a container, as a user who isn't root: 334 tests, 0 failed (the rest inconclusive for Windows-only cases).
  • CI on every head: Linux (with the coverage gate: Physical 99.10% of lines and 95.71% of branches on the first push; floors 98 and 93), Windows, Native AOT, Physical (macOS) and Physical (Linux, Arm64) green, so the walk ran through openat on x64, Arm64 and macOS and through NtCreateFile on Windows.
  • Live suites: not run. The accounts aren't configured yet; Physical's change is covered on the local disk of every system CI runs.

Review

review-code, deep tier (Opus, on the maintainer's choice: 23 agents, every lens, solo refuters and three voters), then two standard rounds on the fixes: clean on 8ce8a2d, plus one test nit applied in 372a287.

  • Round 1 (deep): no blocker, three majors, all fixed on the maintainer's yes. F1: some file systems (FUSE, some NFS servers) list . and .. as files, which .NET reports; opened relative to the folder's handle, .. would have emptied the parent, possibly outside the root, and . recursed until the stack overflowed. A listed name is now only ever an entry of its folder: . and .. skipped, a separator refused; listing went behind the seam so a test hands the walk those names. F2: the plan's R3 test (folders on the way held open without delete sharing, as a shell holds its current folder), shown to fail when the walk asks for delete access there. F3: the README said less than the remarks about where the walk runs and that a delete by path can still be steered. Minors: a subfolder gone between its open and its listing is skipped (F4, reproduced by a refuter); the walk is cancellable between entries (F5); two simplifications (F6, F7). F8 (drop ClassicDeletes) declined: the plan names the switch.
  • Round 2: F1–F7 verified, each with a test that fails without it. G1: an empty listed name would reopen the folder itself on Windows; refused. G2: the check for a name gone after a refused removal looked up a normalized path on Windows (x. read as x); it uses the extended form now, with a test that fails without it. Nits.
  • Round 3: G1–G3 verified; clean.
  • Refuted along the way: a NUL in a name (Linux ends it there; macOS's GetFullPath throws before the parent is touched); ..., .. and stream names on Windows (relative NT opens take them literally, inside the folder); skipping ./.. silently rather than recording them; IsGone hiding a real refusal; AllowUnsafeBlocks in the test project.
  • Not covered: a volume mounted in a folder inside a deleted folder (needs elevation); macOS on Intel and Linux on 32-bit Arm, which share the tested values but run nowhere in CI.

One-way doors

None. No public or protected member is added or changed; the configuration shape is untouched.

Closes #70

…cancelled

Some file systems (FUSE, some NFS servers) list '.' and '..' as files, which .NET then reports:
opened in the folder's handle, '..' would have been its parent and '.' the folder again. The walk
now skips them and refuses a name holding a separator, and lists through the system seam so a test
can hand it such names. A subfolder gone between its open and its listing is skipped, and Windows'
refusal to remove a folder already deleted counts as gone. The walk checks the cancellation token
between entries. A test holds folders on the way open without delete sharing, which fails if the
walk asks for delete access there. The README says where the walk runs and that elsewhere a delete
by path can still be steered; unused classifications go.

Refs #70
An empty name, opened relative to a folder's handle on Windows, is the folder itself: refused as
a name with a separator is. A removal refused for a name another process deleted counts as gone,
and that check now looks the name up in the extended form on Windows, which keeps a trailing dot or
space, so 'x.' is never taken for 'x'. Also: the changelog's cancellation scoped to where the walk
runs, wraps, a dead test override, and the plan's table noting ENOTEMPTY is no longer read.

Refs #70
@serionist
serionist marked this pull request as ready for review October 13, 2026 10:47
@serionist
serionist merged commit 6931d14 into main Oct 13, 2026
8 checks passed
@serionist
serionist deleted the 70-link-safe-delete branch October 13, 2026 10:47
serionist added a commit that referenced this pull request Oct 13, 2026
Every step of the 2027.1.1 plan is merged (#74, #75, #76): the changelog's 2027.1.1 section gets its date, which release.yml requires of a stable tag, and a short intro.

Refs #65, #70
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.

Physical: a recursive folder delete that a link swapped in meanwhile cannot steer outside the root

1 participant