Fix symbolic links not being overwritten during archive extraction

src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java

@@ -33,7 +33,6 @@
 import org.codehaus.plexus.components.io.filemappers.FileMapper;
 import org.codehaus.plexus.components.io.fileselectors.FileSelector;
 import org.codehaus.plexus.components.io.resources.PlexusIoResource;
-import org.codehaus.plexus.util.FileUtils;
 import org.codehaus.plexus.util.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -290,21 +289,32 @@ protected void extractFile(
             }
         }
 
-        // Hmm. Symlinks re-evaluate back to the original file here. Unsure if this is a good thing...
-        final File targetFileName = FileUtils.resolveFile(dir, entryName);
+        // Don't use FileUtils.resolveFile as it follows symlinks, which would cause issues
+        // when trying to overwrite an existing symlink
+        final File targetFileName = new File(dir, entryName);
 
         // Make sure that the resolved path of the extracted file doesn't escape the destination directory
         // getCanonicalFile().toPath() is used instead of getCanonicalPath() (returns String),
         // because "/opt/directory".startsWith("/opt/dir") would return false negative.
         Path canonicalDirPath = dir.getCanonicalFile().toPath();
-        Path canonicalDestPath = targetFileName.getCanonicalFile().toPath();
+        // Don't follow symlinks for the target file, to avoid issues with symlink handling
+        Path targetPath = targetFileName.toPath();
+        // For security check, we need the canonical path but without following the last symlink
+        Path canonicalDestPath;
+        if (Files.isSymbolicLink(targetPath)) {
+            // If it's a symlink, get the canonical path of the parent and append the file name
+            canonicalDestPath =
+                    targetFileName.getParentFile().getCanonicalFile().toPath().resolve(targetFileName.getName());
+        } else {
+            canonicalDestPath = targetFileName.getCanonicalFile().toPath();
+        }
 
         if (!canonicalDestPath.startsWith(canonicalDirPath)) {
             throw new ArchiverException("Entry is outside of the target directory (" + entryName + ")");
         }
 
         // don't allow override target symlink by standard file
-        if (StringUtils.isEmpty(symlinkDestination) && Files.isSymbolicLink(canonicalDestPath)) {
+        if (StringUtils.isEmpty(symlinkDestination) && Files.isSymbolicLink(targetPath)) {
             throw new ArchiverException("Entry is outside of the target directory (" + entryName + ")");
         }
 
@@ -320,6 +330,11 @@ protected void extractFile(
             }
 
             if (!StringUtils.isEmpty(symlinkDestination)) {
+                // Delete existing symlink if it exists
+                Path symlinkPath = targetFileName.toPath();
+                if (Files.isSymbolicLink(symlinkPath)) {
+                    Files.delete(symlinkPath);
+                }
                 SymlinkUtils.createSymbolicLink(targetFileName, new File(symlinkDestination));
             } else if (isDirectory) {
                 targetFileName.mkdirs();
@@ -362,6 +377,11 @@ protected boolean shouldExtractEntry(File targetDirectory, File targetFileName,
         // scenario (4) and (5).
         // No matter the case sensitivity of the file system, file.exists() returns false when there is no file with the
         // same name (1).
+        // Check if it's a symlink first, as exists() follows symlinks and may give incorrect results
+        if (Files.isSymbolicLink(targetFileName.toPath())) {
+            // For symlinks, always extract to overwrite the existing symlink
+            return true;
+        }
         if (!targetFileName.exists()) {
             return true;
         }

src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java

@@ -14,6 +14,7 @@
 import org.junit.jupiter.api.condition.DisabledOnOs;
 import org.junit.jupiter.api.condition.OS;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -107,4 +108,156 @@ void testSymlinkDirArchiver() throws Exception {
         symbolicLink = new File("target/output/dirarchiver-symlink/aDirWithALink/backOutsideToFileX");
         assertTrue(Files.isSymbolicLink(symbolicLink.toPath()));
     }
+
+    @Test
+    @DisabledOnOs(OS.WINDOWS)
+    void testSymlinkOverwriteZip() throws Exception {
+        // Create temporary directory structure for testing
+        File tempDir = new File("target/test-symlink-overwrite");
+        // Clean up from any previous test runs
+        if (tempDir.exists()) {
+            org.codehaus.plexus.util.FileUtils.deleteDirectory(tempDir);
+        }
+        tempDir.mkdirs();
+
+        // Create two target files
+        File target1 = new File(tempDir, "target1.txt");
+        File target2 = new File(tempDir, "target2.txt");
+        Files.write(target1.toPath(), "content1".getBytes());
+        Files.write(target2.toPath(), "content2".getBytes());
+
+        // Create first archive with symlink pointing to target1
+        File archive1Dir = new File(tempDir, "archive1");
+        archive1Dir.mkdirs();
+        File archive1Target1 = new File(archive1Dir, "target1.txt");
+        Files.write(archive1Target1.toPath(), "content1".getBytes());
+        Files.createSymbolicLink(
+                new File(archive1Dir, "link.txt").toPath(),
+                archive1Target1.toPath().getFileName());
+
+        ZipArchiver archiver1 = (ZipArchiver) lookup(Archiver.class, "zip");
+        archiver1.addDirectory(archive1Dir);
+        File zipFile1 = new File(tempDir, "archive1.zip");
+        archiver1.setDestFile(zipFile1);
+        archiver1.createArchive();
+
+        // Extract first archive
+        File outputDir = new File(tempDir, "output");
+        outputDir.mkdirs();
+        ZipUnArchiver unarchiver1 = (ZipUnArchiver) lookup(UnArchiver.class, "zip");
+        unarchiver1.setSourceFile(zipFile1);
+        unarchiver1.setDestFile(outputDir);
+        unarchiver1.extract();
+
+        // Verify symlink points to target1.txt
+        File extractedLink = new File(outputDir, "link.txt");
+        assertTrue(Files.isSymbolicLink(extractedLink.toPath()));
+        assertEquals(
+                "target1.txt", Files.readSymbolicLink(extractedLink.toPath()).toString());
+
+        // Create second archive with symlink pointing to target2
+        File archive2Dir = new File(tempDir, "archive2");
+        archive2Dir.mkdirs();
+        File archive2Target2 = new File(archive2Dir, "target2.txt");
+        Files.write(archive2Target2.toPath(), "content2".getBytes());
+        Files.createSymbolicLink(
+                new File(archive2Dir, "link.txt").toPath(),
+                archive2Target2.toPath().getFileName());
+
+        ZipArchiver archiver2 = (ZipArchiver) lookup(Archiver.class, "zip");
+        archiver2.addDirectory(archive2Dir);
+        File zipFile2 = new File(tempDir, "archive2.zip");
+        archiver2.setDestFile(zipFile2);
+        archiver2.createArchive();
+
+        // Extract second archive (should overwrite the symlink)
+        ZipUnArchiver unarchiver2 = (ZipUnArchiver) lookup(UnArchiver.class, "zip");
+        unarchiver2.setSourceFile(zipFile2);
+        unarchiver2.setDestFile(outputDir);
+        unarchiver2.extract();
+
+        // Verify symlink now points to target2.txt (THIS IS THE KEY TEST)
+        assertTrue(Files.isSymbolicLink(extractedLink.toPath()), "link.txt should still be a symlink");
+        assertEquals(
+                "target2.txt",
+                Files.readSymbolicLink(extractedLink.toPath()).toString(),
+                "Symlink should be updated to point to target2.txt");
+    }
+
+    @Test
+    @DisabledOnOs(OS.WINDOWS)
+    void testSymlinkOverwriteTar() throws Exception {
+        // Create temporary directory structure for testing
+        File tempDir = new File("target/test-symlink-overwrite-tar");
+        // Clean up from any previous test runs
+        if (tempDir.exists()) {
+            org.codehaus.plexus.util.FileUtils.deleteDirectory(tempDir);
+        }
+        tempDir.mkdirs();
+
+        // Create two target files
+        File target1 = new File(tempDir, "target1.txt");
+        File target2 = new File(tempDir, "target2.txt");
+        Files.write(target1.toPath(), "content1".getBytes());
+        Files.write(target2.toPath(), "content2".getBytes());
+
+        // Create first archive with symlink pointing to target1
+        File archive1Dir = new File(tempDir, "archive1");
+        archive1Dir.mkdirs();
+        File archive1Target1 = new File(archive1Dir, "target1.txt");
+        Files.write(archive1Target1.toPath(), "content1".getBytes());
+        Files.createSymbolicLink(
+                new File(archive1Dir, "link.txt").toPath(),
+                archive1Target1.toPath().getFileName());
+
+        TarArchiver archiver1 = (TarArchiver) lookup(Archiver.class, "tar");
+        archiver1.setLongfile(TarLongFileMode.posix);
+        archiver1.addDirectory(archive1Dir);
+        File tarFile1 = new File(tempDir, "archive1.tar");
+        archiver1.setDestFile(tarFile1);
+        archiver1.createArchive();
+
+        // Extract first archive
+        File outputDir = new File(tempDir, "output");
+        outputDir.mkdirs();
+        TarUnArchiver unarchiver1 = (TarUnArchiver) lookup(UnArchiver.class, "tar");
+        unarchiver1.setSourceFile(tarFile1);
+        unarchiver1.setDestFile(outputDir);
+        unarchiver1.extract();
+
+        // Verify symlink points to target1.txt
+        File extractedLink = new File(outputDir, "link.txt");
+        assertTrue(Files.isSymbolicLink(extractedLink.toPath()));
+        assertEquals(
+                "target1.txt", Files.readSymbolicLink(extractedLink.toPath()).toString());
+
+        // Create second archive with symlink pointing to target2
+        File archive2Dir = new File(tempDir, "archive2");
+        archive2Dir.mkdirs();
+        File archive2Target2 = new File(archive2Dir, "target2.txt");
+        Files.write(archive2Target2.toPath(), "content2".getBytes());
+        Files.createSymbolicLink(
+                new File(archive2Dir, "link.txt").toPath(),
+                archive2Target2.toPath().getFileName());
+
+        TarArchiver archiver2 = (TarArchiver) lookup(Archiver.class, "tar");
+        archiver2.setLongfile(TarLongFileMode.posix);
+        archiver2.addDirectory(archive2Dir);
+        File tarFile2 = new File(tempDir, "archive2.tar");
+        archiver2.setDestFile(tarFile2);
+        archiver2.createArchive();
+
+        // Extract second archive (should overwrite the symlink)
+        TarUnArchiver unarchiver2 = (TarUnArchiver) lookup(UnArchiver.class, "tar");
+        unarchiver2.setSourceFile(tarFile2);
+        unarchiver2.setDestFile(outputDir);
+        unarchiver2.extract();
+
+        // Verify symlink now points to target2.txt (THIS IS THE KEY TEST)
+        assertTrue(Files.isSymbolicLink(extractedLink.toPath()), "link.txt should still be a symlink");
+        assertEquals(
+                "target2.txt",
+                Files.readSymbolicLink(extractedLink.toPath()).toString(),
+                "Symlink should be updated to point to target2.txt");
+    }
 }