Fix symbolic link extraction issue by avoiding canonicalization for symlinks

Co-authored-by: slachiewicz <[email protected]>

Author: Copilot

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

@@ -290,14 +290,31 @@ 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);
+        // For symlinks, we need to get the file path without following symlinks.
+        // FileUtils.resolveFile calls getCanonicalFile() which follows symlinks,
+        // causing the symlink to resolve to its target instead of the symlink itself.
+        final File targetFileName;
+        if (!StringUtils.isEmpty(symlinkDestination)) {
+            // For symlinks, use simple path resolution without canonicalization
+            targetFileName = resolveFileWithoutFollowingSymlinks(dir, entryName);
+        } else {
+            // For regular files and directories, use the existing logic
+            targetFileName = FileUtils.resolveFile(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();
+
+        // For symlinks, we need to check the symlink path itself, not the target it points to
+        Path canonicalDestPath;
+        if (!StringUtils.isEmpty(symlinkDestination)) {
+            // For symlinks, normalize without following the link
+            canonicalDestPath = targetFileName.toPath().toAbsolutePath().normalize();
+        } else {
+            canonicalDestPath = targetFileName.getCanonicalFile().toPath();
+        }
 
         if (!canonicalDestPath.startsWith(canonicalDirPath)) {
             throw new ArchiverException("Entry is outside of the target directory (" + entryName + ")");
@@ -396,4 +413,33 @@ protected boolean shouldExtractEntry(File targetDirectory, File targetFileName,
     private String normalizedFileSeparator(String pathOrEntry) {
         return pathOrEntry.replace("/", File.separator);
     }
+
+    /**
+     * Resolves a file path relative to a base directory without following symlinks.
+     * This is similar to FileUtils.resolveFile but doesn't call getCanonicalFile(),
+     * which would follow symlinks and resolve them to their targets.
+     *
+     * @param baseDir the base directory
+     * @param filename the filename to resolve
+     * @return the resolved file
+     */
+    private File resolveFileWithoutFollowingSymlinks(File baseDir, String filename) {
+        String filenm = filename;
+        if ('/' != File.separatorChar) {
+            filenm = filename.replace('/', File.separatorChar);
+        }
+
+        if ('\\' != File.separatorChar) {
+            filenm = filenm.replace('\\', File.separatorChar);
+        }
+
+        // For absolute paths, just return a File object without canonicalization
+        if (filenm.startsWith(File.separator)) {
+            return new File(filenm);
+        }
+
+        // For relative paths, combine with base directory and get absolute path
+        // but don't call getCanonicalFile() which would follow symlinks
+        return new File(baseDir, filenm).getAbsoluteFile();
+    }
 }

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

@@ -107,4 +107,75 @@ void testSymlinkDirArchiver() throws Exception {
         symbolicLink = new File("target/output/dirarchiver-symlink/aDirWithALink/backOutsideToFileX");
         assertTrue(Files.isSymbolicLink(symbolicLink.toPath()));
     }
+
+    /**
+     * Test for the issue where symlinks were not properly resolved when extracting archives.
+     * When a symlink is extracted and then the archive is extracted again, the symlink
+     * should remain a symlink and not be resolved to its target.
+     * This test verifies:
+     * 1. Extracting a symlink creates the symlink correctly
+     * 2. Re-extracting the same archive preserves the symlink
+     * 3. Symlinks to non-existent files are handled correctly
+     */
+    @Test
+    @DisabledOnOs(OS.WINDOWS)
+    void testSymlinkExtractionTwice() throws Exception {
+        // Test with tar
+        TarArchiver tarArchiver = (TarArchiver) lookup(Archiver.class, "tar");
+        tarArchiver.setLongfile(TarLongFileMode.posix);
+
+        File srcDir = getTestFile("src/test/resources/symlinks/src");
+        tarArchiver.addDirectory(srcDir);
+        File tarFile = new File("target/output/symlink-twice-test.tar");
+        tarArchiver.setDestFile(tarFile);
+        tarArchiver.createArchive();
+
+        File outputDir = new File("target/output/symlink-twice-test-tar");
+        outputDir.mkdirs();
+
+        TarUnArchiver tarUnarchiver = (TarUnArchiver) lookup(UnArchiver.class, "tar");
+        tarUnarchiver.setSourceFile(tarFile);
+        tarUnarchiver.setDestFile(outputDir);
+
+        // First extraction
+        tarUnarchiver.extract();
+
+        // Verify symlinks exist and are actually symlinks
+        File symR = new File(outputDir, "symR");
+        assertTrue(Files.isSymbolicLink(symR.toPath()), "symR should be a symlink");
+
+        // Second extraction - this should not fail and should preserve symlinks
+        tarUnarchiver.extract();
+
+        // Verify symlinks still exist and are still symlinks
+        assertTrue(Files.isSymbolicLink(symR.toPath()), "symR should still be a symlink after re-extraction");
+
+        // Test with zip
+        ZipArchiver zipArchiver = (ZipArchiver) lookup(Archiver.class, "zip");
+        zipArchiver.addDirectory(srcDir);
+        File zipFile = new File("target/output/symlink-twice-test.zip");
+        zipFile.delete();
+        zipArchiver.setDestFile(zipFile);
+        zipArchiver.createArchive();
+
+        File zipOutputDir = new File("target/output/symlink-twice-test-zip");
+        zipOutputDir.mkdirs();
+
+        ZipUnArchiver zipUnarchiver = (ZipUnArchiver) lookup(UnArchiver.class, "zip");
+        zipUnarchiver.setSourceFile(zipFile);
+        zipUnarchiver.setDestFile(zipOutputDir);
+
+        // First extraction
+        zipUnarchiver.extract();
+
+        // Verify symlinks exist and are actually symlinks
+        File symRZip = new File(zipOutputDir, "symR");
+        assertTrue(Files.isSymbolicLink(symRZip.toPath()), "symR should be a symlink in zip");
+
+        // Second extraction - this should not fail and should preserve symlinks
+        zipUnarchiver.extract();
+
+        // Verify symlinks still exist and are still symlinks
+        assertTrue(Files.isSymbolicLink(symRZip.toPath()), "symR should still be a symlink after re-extraction in zip");
+    }
 }