[ 
https://issues.apache.org/jira/browse/GROOVY-12359?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111839#comment-18111839
 ] 

ASF GitHub Bot commented on GROOVY-12359:
-----------------------------------------

paulk-asert opened a new pull request, #2880:
URL: https://github.com/apache/groovy/pull/2880

   …eaves
   
   Since GROOVY-12125 a symbolic link inside a tree being deleted is removed as 
the link itself and never entered. The guard was Files.isSymbolicLink, which 
does not report a Windows directory junction, so a junction was still traversed 
and the contents of its target deleted. A junction is the link form that 
matters most there: mklink /J needs no privilege, where a Windows symbolic link 
needs one most users do not hold, and it is planted in advance rather than 
raced.
   
   Both implementations, File.deleteDir and the groovy-nio Path.deleteDir, now 
read each node's attributes without following links and treat anything that is 
neither a regular file nor a directory as a leaf: a symbolic link is removed as 
before, and a junction or other reparse point, a pipe, a socket or a device is 
removed as the node it is, never entered or opened. Handed such a node 
directly, deleteDir returns false as it does for any non-directory, and does 
not touch what the node points at.
   
   On POSIX the visible behaviour is unchanged — a pipe inside a tree was 
deleted before and still is, now through the leaf branch, which is covered by 
new mkfifo tests that run on the platforms the build runs on. The junction 
behaviour cannot be exercised on those platforms, so the Windows-gated tests 
are the verification rather than a formality: they create a junction with 
mklink /J, which succeeds unprivileged on the CI runners, delete the enclosing 
tree, and assert the target's contents survive. Until they have run on Windows 
CI, the junction half of this change is implemented to the documented attribute 
behaviour, not demonstrated.




> deleteDir: treat Windows directory junctions as leaves, as symbolic links 
> already are
> -------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12359
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12359
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Paul King
>            Assignee: Paul King
>            Priority: Major
>
> Since GROOVY-12125, {{File.deleteDir()}} and the {{groovy-nio}} 
> {{Path.deleteDir()}} treat a symbolic link as a leaf: the link is removed, 
> and the directory it points at is not entered. The guard is 
> {{Files.isSymbolicLink}}, and its own comment records the gap this ticket 
> closes:
> {code:java}
> // never follow a symbolic link into its target; remove the link itself.
> // Note: Files.isSymbolicLink does not detect Windows directory junctions
> // (reparse points), which are therefore still traversed.
> {code}
> A junction inside a tree being deleted is followed, and the *target's* 
> contents are deleted. Two things make this the half of the link problem worth 
> code rather than documentation:
> * creating a junction needs no privilege — {{mklink /J}} works for any user, 
> where Windows symbolic links require a privilege most users do not hold, so 
> the unprivileged local actor's tool is exactly the one the guard misses;
> * it needs no timing — a junction is planted in advance and waits, unlike the 
> check-then-delete race, which requires writing into the tree concurrently 
> during deletion and is out of scope here (the threat model's local adversary 
> is not granted that; addressed separately).
> h3. Proposed change
> Replace the symlink-only test at both decision points (the root check and the 
> per-child check) in both implementations with an attribute read that refuses 
> to treat any reparse point as an ordinary directory:
> {code:java}
> BasicFileAttributes attrs = Files.readAttributes(path, 
> BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS);
> boolean leaf = attrs.isSymbolicLink() || attrs.isOther();   // symlink, 
> junction, or other reparse point
> {code}
> On POSIX, {{isOther()}} is false for ordinary files and directories, so 
> behaviour there does not change. A junction is then deleted as the link node 
> it is, matching the existing symlink behaviour.
> h3. Verification
> Junctions do not exist on POSIX, so this cannot be exercised on the 
> development machine; the expected {{isOther()}} attribute behaviour for a 
> junction is confirmed by the test rather than assumed in advance. The 
> regression test is Windows-gated: it creates a junction with {{cmd /c mklink 
> /J}} — which succeeds unprivileged on the CI runners — points it at a 
> directory holding a marker file, deletes the enclosing tree, and asserts the 
> marker survives and the junction node is gone. On other platforms the test is 
> skipped; the existing symlink tests from GROOVY-12125 continue to cover the 
> POSIX side.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to