From a3a08fc7ca22b56c928c353a084787c6fbfa041c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 01:43:58 +0000 Subject: [PATCH 1/2] Initial plan From 39217a625f59d3af2da21615c3eb4039201ed24a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 01:52:14 +0000 Subject: [PATCH 2/2] Fix symbolic link extraction issue by avoiding canonicalization for symlinks Co-authored-by: slachiewicz <6705942+slachiewicz@users.noreply.github.com> --- .../plexus/archiver/AbstractUnArchiver.java | 52 +++++++++++++- .../codehaus/plexus/archiver/SymlinkTest.java | 71 +++++++++++++++++++ 2 files changed, 120 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java index c61327c5..e245a003 100644 --- a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java +++ b/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(); + } } diff --git a/src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java b/src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java index b118e81b..1389788c 100644 --- a/src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java +++ b/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"); + } }