From 8f75a09b55f13a390b7de354bdfd594c7787f446 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 21:26:20 +0000 Subject: [PATCH 01/13] Initial plan From 558034a966957f8751c5f706e581e1ee862dc04a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 21:31:40 +0000 Subject: [PATCH 02/13] Initial plan for disc image metadata extraction Co-authored-by: gfs <98900+gfs@users.noreply.github.com> --- nuget.config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nuget.config b/nuget.config index 227ad0ce..248a5bb5 100644 --- a/nuget.config +++ b/nuget.config @@ -2,6 +2,6 @@ - + \ No newline at end of file From f71dea8a9f999da32aa315f709a0739f7ca1d18d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 21:41:09 +0000 Subject: [PATCH 03/13] Add file metadata extraction for DiscUtils-based disc image formats - Add TryGetFileMetadata() and CollectMetadata() helpers to DiscCommon for extracting Unix permissions, UID, and GID from IUnixFileSystem-compatible file systems (Ext, Xfs, Btrfs, HfsPlus, ISO 9660 with RockRidge) - Apply metadata extraction in DiscCommon (VHD/VHDX/VMDK), IsoExtractor, UdfExtractor, and WimExtractor - Add TestDataRockRidge.iso test fixture with RockRidge extensions - Add tests for ISO metadata extraction (both with and without RockRidge) - Restore nuget.config to original configuration Co-authored-by: gfs <98900+gfs@users.noreply.github.com> --- .../ExtractorTests/ExpectedNumFilesTests.cs | 2 + .../ExtractorTests/FileMetadataTests.cs | 65 ++++++++++++++++++ .../RecursiveExtractor.Tests.csproj | 3 + .../TestDataArchives/TestDataRockRidge.iso | Bin 0 -> 364544 bytes RecursiveExtractor/Extractors/DiscCommon.cs | 60 ++++++++++++++++ RecursiveExtractor/Extractors/IsoExtractor.cs | 12 ++++ RecursiveExtractor/Extractors/UdfExtractor.cs | 12 ++++ RecursiveExtractor/Extractors/WimExtractor.cs | 2 + nuget.config | 2 +- 9 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataRockRidge.iso diff --git a/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs b/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs index 92dd3c24..156fdbf1 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs @@ -38,6 +38,7 @@ public static TheoryData ArchiveData { "TestData.a",3 }, { "TestData.bsd.ar",3 }, { "TestData.iso",3 }, + { "TestDataRockRidge.iso",2 }, { "TestData.vhdx",3 }, { "EmptyFile.txt", 1 }, { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 54 : 52 }, @@ -79,6 +80,7 @@ public static TheoryData NoRecursionData { "TestData.a", 3 }, { "TestData.bsd.ar", 3 }, { "TestData.iso", 3 }, + { "TestDataRockRidge.iso", 2 }, { "TestData.vhdx", 3 }, { "EmptyFile.txt", 1 }, { "TestDataArchivesNested.zip", 14 }, diff --git a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs index 1700217f..8f5e2297 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs @@ -140,4 +140,69 @@ public void FileEntry_MetadataDefaultsToNull() var entry = new FileEntry("test.txt", stream); Assert.Null(entry.Metadata); } + + [Fact] + public async Task IsoEntries_MetadataIsNullWithoutRockRidge() + { + // TestData.iso does not have RockRidge extensions, so Unix metadata is not available + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.iso"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + // Without RockRidge extensions, metadata should be null + Assert.Null(entry.Metadata); + } + } + + [Fact] + public void IsoEntries_MetadataIsNullWithoutRockRidge_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.iso"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.Null(entry.Metadata); + } + } + + [Fact] + public async Task IsoRockRidgeEntries_HaveMetadata() + { + // TestDataRockRidge.iso has RockRidge extensions with Unix permissions + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataRockRidge.iso"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + Assert.NotNull(entry.Metadata!.Mode); + Assert.NotNull(entry.Metadata.Uid); + Assert.NotNull(entry.Metadata.Gid); + } + } + + [Fact] + public void IsoRockRidgeEntries_HaveMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataRockRidge.iso"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + Assert.NotNull(entry.Metadata!.Mode); + Assert.NotNull(entry.Metadata.Uid); + Assert.NotNull(entry.Metadata.Gid); + } + } } diff --git a/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj b/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj index 248021b2..e529dedb 100644 --- a/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj +++ b/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj @@ -165,6 +165,9 @@ PreserveNewest + + PreserveNewest + PreserveNewest diff --git a/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataRockRidge.iso b/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataRockRidge.iso new file mode 100644 index 0000000000000000000000000000000000000000..f7a9291b3d5b9c66eee576bedd1c50296a6b7414 GIT binary patch literal 364544 zcmeI)QEwDQ902g$60t$Sh#?^OFfqo+gU7X)BJt_i+qMgLx0l^5DNjbFL=q_poW>_3 ziHZ6>_ym3czYh9y8qPY2u78j#a?denR>|J)EDZU?_Y>%9b7MsFi^uxl@6CW-_3r)qQevn?M zz*;)W$~+tP*3wWH!o`)9?z#2VGOT8Uw7gbU=`bWk+N<&+e3YCGOBXMm4{7IGzA;+s z_Xg?w;DwKuyWN#=xie0CMVXJzUG9|0dN!!vm21;$n$yiS7ef6;ypmO+N_+qGc03rB z>&qh0{!jBr;6EVn=PdsJ2fCsQ1PBly zK!5-N0t5&UAV7e?0SUy@Ire%sJvuy{hmWIj<5E8>4yZY|B|v}x0RjXF5FkK+009C7 zUVuRJ&A*-ksI$XO=~NW^{@*}I1PBlyK!5-N0t5&UAVA<%68PnrYwXJL>+$b@L`Csh zd~dw@LHw}SjKBBrP|I-W!L)Gb!91^4Pshh*3-un&lJvd!-L}>&i=v(O`Ob|e_v7sD zTEBR=1Gm}^+^SarHfdqr@I2q^z}a@e;BFRux4YKx$twyE&n7>bwtgKA--x5T-+p=h z=AFNf``OJd%-1Q-hW}G^y6x)e<86L?cA|b(PrSs(s| zfYDQ;``$|az3Be$KVC#XMrYbVojKa(M<0!|Z;t9XoAw`#vTw!Fb{%E6uFqra{Wiwl z7i0a65FkK+z^f~e7CZ5g>gPKCX5-K6?OB$3)p{C=Jh>8ztlw&W zJjn-5;cC`T%TR8N$9YkO)p~=Mab9Md)16Wj8f4{qT1+dK<21=uv!qvL`6!h6MvWtC^-Z^R}{4{cJeBZRgc}8I6np0RjXFJeNQ-!yK9^=J3_+?a9{m z + /// Tries to extract file metadata from a DiscUtils file system entry. + /// For file systems implementing (Ext, Xfs, Btrfs, HfsPlus), + /// returns permissions, UID, and GID. Returns null for unsupported file systems. + /// + /// The opened disc file system + /// Path of the file within the file system + /// Populated or null when not available + internal static FileEntryMetadata? TryGetFileMetadata(DiscFileSystem fs, string filePath) + { + if (fs is not IUnixFileSystem unixFs) + { + return null; + } + + try + { + var info = unixFs.GetUnixFileInfo(filePath); + return new FileEntryMetadata + { + Mode = (long)info.Permissions, + Uid = info.UserId, + Gid = info.GroupId + }; + } + catch (Exception e) + { + Logger.Debug(e, "Could not retrieve Unix metadata for {0}", filePath); + return null; + } + } + + /// + /// Pre-collects metadata for all files while the file system is still open. + /// Used by extractors (e.g., ISO) where the file system is disposed before files are processed. + /// + /// The opened disc file system + /// The file entries to collect metadata for + /// A dictionary mapping file paths to metadata, or null if the file system does not support metadata + internal static Dictionary? CollectMetadata(DiscFileSystem fs, DiscFileInfo[] fileInfos) + { + if (fs is not IUnixFileSystem) + { + return null; + } + + var result = new Dictionary(); + foreach (var fi in fileInfos) + { + var metadata = TryGetFileMetadata(fs, fi.FullName); + if (metadata != null) + { + result[fi.FullName] = metadata; + } + } + return result; + } + /// /// Dump the FileEntries from a Logical Volume asynchronously /// @@ -59,6 +117,7 @@ public static async IAsyncEnumerable DumpLogicalVolumeAsync(LogicalVo if (fileStream != null && fi != null) { var newFileEntry = await FileEntry.FromStreamAsync($"{volume.Identity}{Path.DirectorySeparatorChar}{fi.FullName}", fileStream, parent, fi.CreationTime, fi.LastWriteTime, fi.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff).ConfigureAwait(false); + newFileEntry.Metadata = TryGetFileMetadata(fs, file); if (options.Recurse || topLevel) { await foreach (var entry in Context.ExtractAsync(newFileEntry, options, governor, false)) @@ -124,6 +183,7 @@ public static IEnumerable DumpLogicalVolume(LogicalVolumeInfo volume, if (fileStream != null) { var newFileEntry = new FileEntry($"{volume.Identity}{Path.DirectorySeparatorChar}{file}", fileStream, parent, false, creation, modification, access, memoryStreamCutoff: options.MemoryStreamCutoff); + newFileEntry.Metadata = TryGetFileMetadata(fs, file); if (options.Recurse || topLevel) { foreach (var extractedFile in Context.Extract(newFileEntry, options, governor, false)) diff --git a/RecursiveExtractor/Extractors/IsoExtractor.cs b/RecursiveExtractor/Extractors/IsoExtractor.cs index 3756b319..da94c098 100644 --- a/RecursiveExtractor/Extractors/IsoExtractor.cs +++ b/RecursiveExtractor/Extractors/IsoExtractor.cs @@ -31,11 +31,13 @@ public IsoExtractor(Extractor context) public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { DiscUtils.DiscFileInfo[]? entries = null; + Dictionary? metadataByPath = null; var failed = false; try { using var cd = new CDReader(fileEntry.Content, true); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); + metadataByPath = DiscCommon.CollectMetadata(cd, entries); } catch (Exception e) { @@ -69,6 +71,10 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra { var name = fileInfo.FullName.Replace('/', Path.DirectorySeparatorChar); var newFileEntry = await FileEntry.FromStreamAsync(name, stream, fileEntry, fileInfo.CreationTime, fileInfo.LastWriteTime, fileInfo.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff).ConfigureAwait(false); + if (metadataByPath != null && metadataByPath.TryGetValue(fileInfo.FullName, out var entryMetadata)) + { + newFileEntry.Metadata = entryMetadata; + } if (options.Recurse || topLevel) { await foreach (var entry in Context.ExtractAsync(newFileEntry, options, governor, false)) @@ -92,11 +98,13 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { DiscUtils.DiscFileInfo[]? entries = null; + Dictionary? metadataByPath = null; var failed = false; try { using var cd = new CDReader(fileEntry.Content, true); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); + metadataByPath = DiscCommon.CollectMetadata(cd, entries); } catch(Exception e) { @@ -130,6 +138,10 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti { var name = fileInfo.FullName.Replace('/', Path.DirectorySeparatorChar); var newFileEntry = new FileEntry(name, stream, fileEntry, createTime: file.CreationTime, modifyTime: file.LastWriteTime, accessTime: file.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff); + if (metadataByPath != null && metadataByPath.TryGetValue(fileInfo.FullName, out var entryMetadata)) + { + newFileEntry.Metadata = entryMetadata; + } if (options.Recurse || topLevel) { foreach (var entry in Context.Extract(newFileEntry, options, governor, false)) diff --git a/RecursiveExtractor/Extractors/UdfExtractor.cs b/RecursiveExtractor/Extractors/UdfExtractor.cs index efc016c8..27d1bb53 100644 --- a/RecursiveExtractor/Extractors/UdfExtractor.cs +++ b/RecursiveExtractor/Extractors/UdfExtractor.cs @@ -31,11 +31,13 @@ public UdfExtractor(Extractor context) public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { DiscUtils.DiscFileInfo[]? entries = null; + Dictionary? metadataByPath = null; var failed = false; try { using var cd = new UdfReader(fileEntry.Content); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); + metadataByPath = DiscCommon.CollectMetadata(cd, entries); } catch (Exception e) { @@ -69,6 +71,10 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra { var name = fileInfo.FullName.Replace('/', Path.DirectorySeparatorChar); var newFileEntry = await FileEntry.FromStreamAsync(name, stream, fileEntry, fileInfo.CreationTime, fileInfo.LastWriteTime, fileInfo.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff).ConfigureAwait(false); + if (metadataByPath != null && metadataByPath.TryGetValue(fileInfo.FullName, out var entryMetadata)) + { + newFileEntry.Metadata = entryMetadata; + } if (options.Recurse || topLevel) { await foreach (var entry in Context.ExtractAsync(newFileEntry, options, governor, false)) @@ -92,11 +98,13 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { DiscUtils.DiscFileInfo[]? entries = null; + Dictionary? metadataByPath = null; var failed = false; try { using var cd = new UdfReader(fileEntry.Content); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); + metadataByPath = DiscCommon.CollectMetadata(cd, entries); } catch(Exception e) { @@ -130,6 +138,10 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti { var name = fileInfo.FullName.Replace('/', Path.DirectorySeparatorChar); var newFileEntry = new FileEntry(name, stream, fileEntry, createTime: file.CreationTime, modifyTime: file.LastWriteTime, accessTime: file.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff); + if (metadataByPath != null && metadataByPath.TryGetValue(fileInfo.FullName, out var entryMetadata)) + { + newFileEntry.Metadata = entryMetadata; + } if (options.Recurse || topLevel) { foreach (var entry in Context.Extract(newFileEntry, options, governor, false)) diff --git a/RecursiveExtractor/Extractors/WimExtractor.cs b/RecursiveExtractor/Extractors/WimExtractor.cs index fe7fd3df..4cf9d516 100644 --- a/RecursiveExtractor/Extractors/WimExtractor.cs +++ b/RecursiveExtractor/Extractors/WimExtractor.cs @@ -58,6 +58,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra { var name = file.Replace('\\', Path.DirectorySeparatorChar); var newFileEntry = await FileEntry.FromStreamAsync($"{image.FriendlyName}{Path.DirectorySeparatorChar}{name}", stream, fileEntry, memoryStreamCutoff: options.MemoryStreamCutoff).ConfigureAwait(false); + newFileEntry.Metadata = DiscCommon.TryGetFileMetadata(image, file); if (options.Recurse || topLevel) { @@ -128,6 +129,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti var name = file.Replace('\\', Path.DirectorySeparatorChar); var newFileEntry = new FileEntry($"{image.FriendlyName}{Path.DirectorySeparatorChar}{name}", stream, fileEntry, memoryStreamCutoff: options.MemoryStreamCutoff); + newFileEntry.Metadata = DiscCommon.TryGetFileMetadata(image, file); if (options.Recurse || topLevel) { foreach (var extractedFile in Context.Extract(newFileEntry, options, governor, false)) diff --git a/nuget.config b/nuget.config index 248a5bb5..227ad0ce 100644 --- a/nuget.config +++ b/nuget.config @@ -2,6 +2,6 @@ - + \ No newline at end of file From 649794aa708e00b2888582e26f4e42882e44e905 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 05:04:54 +0000 Subject: [PATCH 04/13] Plan: Add Windows filesystem metadata support for disc image formats Co-authored-by: gfs <98900+gfs@users.noreply.github.com> --- nuget.config | 2 +- nuget.config.original | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 nuget.config.original diff --git a/nuget.config b/nuget.config index 227ad0ce..c25d0d6c 100644 --- a/nuget.config +++ b/nuget.config @@ -2,6 +2,6 @@ - + \ No newline at end of file diff --git a/nuget.config.original b/nuget.config.original new file mode 100644 index 00000000..227ad0ce --- /dev/null +++ b/nuget.config.original @@ -0,0 +1,7 @@ + + + + + + + \ No newline at end of file From 4cd536e8c691b22e8a11258eeb2b9a7db1d4b672 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 05:08:02 +0000 Subject: [PATCH 05/13] Add Windows filesystem metadata (NTFS/FAT/WIM) to disc image extraction - Add FileAttributes and SecurityDescriptorSddl properties to FileEntryMetadata - Update DiscCommon.TryGetFileMetadata() to handle IDosFileSystem (file attributes) and IWindowsFileSystem (SDDL security descriptors) in addition to IUnixFileSystem - Update DiscCommon.CollectMetadata() to recognize Windows file systems - Add VHDX/NTFS metadata tests verifying FileAttributes and SecurityDescriptorSddl - Restore nuget.config to original configuration Co-authored-by: gfs <98900+gfs@users.noreply.github.com> --- .../ExtractorTests/FileMetadataTests.cs | 43 ++++++++++++ RecursiveExtractor/Extractors/DiscCommon.cs | 66 +++++++++++++++---- RecursiveExtractor/FileEntryMetadata.cs | 16 +++++ nuget.config | 6 +- nuget.config.original | 7 -- 5 files changed, 114 insertions(+), 24 deletions(-) delete mode 100644 nuget.config.original diff --git a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs index 8f5e2297..bef2a127 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs @@ -101,6 +101,8 @@ public void MetadataDefaults_AreNull() Assert.Null(metadata.IsExecutable); Assert.Null(metadata.IsSetUid); Assert.Null(metadata.IsSetGid); + Assert.Null(metadata.FileAttributes); + Assert.Null(metadata.SecurityDescriptorSddl); } [Fact] @@ -205,4 +207,45 @@ public void IsoRockRidgeEntries_HaveMetadata_Sync() Assert.NotNull(entry.Metadata.Gid); } } + + [Fact] + public async Task VhdxNtfsEntries_HaveWindowsMetadata() + { + // TestData.vhdx contains an NTFS file system which implements IDosFileSystem and IWindowsFileSystem + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.vhdx"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + // NTFS provides Windows file attributes + Assert.NotNull(entry.Metadata!.FileAttributes); + // NTFS provides security descriptors + Assert.NotNull(entry.Metadata.SecurityDescriptorSddl); + Assert.Contains("D:", entry.Metadata.SecurityDescriptorSddl); // DACL present + // NTFS does not provide Unix metadata + Assert.Null(entry.Metadata.Mode); + Assert.Null(entry.Metadata.Uid); + Assert.Null(entry.Metadata.Gid); + } + } + + [Fact] + public void VhdxNtfsEntries_HaveWindowsMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.vhdx"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + Assert.NotNull(entry.Metadata!.FileAttributes); + Assert.NotNull(entry.Metadata.SecurityDescriptorSddl); + Assert.Null(entry.Metadata.Mode); + } + } } diff --git a/RecursiveExtractor/Extractors/DiscCommon.cs b/RecursiveExtractor/Extractors/DiscCommon.cs index 0b0269cd..7ffed751 100644 --- a/RecursiveExtractor/Extractors/DiscCommon.cs +++ b/RecursiveExtractor/Extractors/DiscCommon.cs @@ -17,33 +17,71 @@ public static class DiscCommon /// /// Tries to extract file metadata from a DiscUtils file system entry. /// For file systems implementing (Ext, Xfs, Btrfs, HfsPlus), - /// returns permissions, UID, and GID. Returns null for unsupported file systems. + /// returns permissions, UID, and GID. + /// For file systems implementing (NTFS, FAT, WIM), + /// returns Windows file attributes. + /// For file systems implementing (NTFS, WIM), + /// also returns the security descriptor in SDDL format. + /// Returns null for file systems that support none of these interfaces. /// /// The opened disc file system /// Path of the file within the file system /// Populated or null when not available internal static FileEntryMetadata? TryGetFileMetadata(DiscFileSystem fs, string filePath) { - if (fs is not IUnixFileSystem unixFs) + FileEntryMetadata? metadata = null; + + if (fs is IUnixFileSystem unixFs) { - return null; + try + { + var info = unixFs.GetUnixFileInfo(filePath); + metadata = new FileEntryMetadata + { + Mode = (long)info.Permissions, + Uid = info.UserId, + Gid = info.GroupId + }; + } + catch (Exception e) + { + Logger.Debug(e, "Could not retrieve Unix metadata for {0}", filePath); + } } - try + if (fs is IDosFileSystem dosFs) { - var info = unixFs.GetUnixFileInfo(filePath); - return new FileEntryMetadata + try { - Mode = (long)info.Permissions, - Uid = info.UserId, - Gid = info.GroupId - }; + var winInfo = dosFs.GetFileStandardInformation(filePath); + metadata ??= new FileEntryMetadata(); + metadata.FileAttributes = winInfo.FileAttributes; + } + catch (Exception e) + { + Logger.Debug(e, "Could not retrieve DOS file attributes for {0}", filePath); + } } - catch (Exception e) + + if (fs is IWindowsFileSystem windowsFs) { - Logger.Debug(e, "Could not retrieve Unix metadata for {0}", filePath); - return null; + try + { + var securityDescriptor = windowsFs.GetSecurity(filePath); + if (securityDescriptor != null) + { + metadata ??= new FileEntryMetadata(); + metadata.SecurityDescriptorSddl = securityDescriptor.GetSddlForm( + DiscUtils.Core.WindowsSecurity.AccessControl.AccessControlSections.All); + } + } + catch (Exception e) + { + Logger.Debug(e, "Could not retrieve security descriptor for {0}", filePath); + } } + + return metadata; } /// @@ -55,7 +93,7 @@ public static class DiscCommon /// A dictionary mapping file paths to metadata, or null if the file system does not support metadata internal static Dictionary? CollectMetadata(DiscFileSystem fs, DiscFileInfo[] fileInfos) { - if (fs is not IUnixFileSystem) + if (fs is not IUnixFileSystem && fs is not IDosFileSystem && fs is not IWindowsFileSystem) { return null; } diff --git a/RecursiveExtractor/FileEntryMetadata.cs b/RecursiveExtractor/FileEntryMetadata.cs index 4254c442..f7ae5606 100644 --- a/RecursiveExtractor/FileEntryMetadata.cs +++ b/RecursiveExtractor/FileEntryMetadata.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft Corporation. Licensed under the MIT License. +using System.IO; + namespace Microsoft.CST.RecursiveExtractor { /// @@ -43,5 +45,19 @@ public class FileEntryMetadata /// Null if not available from the archive format. /// public long? Gid { get; set; } + + /// + /// The Windows file attributes (e.g., ReadOnly, Hidden, System, Archive). + /// Available for NTFS, FAT, and WIM file systems. + /// Null if not available from the archive format. + /// + public FileAttributes? FileAttributes { get; set; } + + /// + /// The NTFS security descriptor in SDDL (Security Descriptor Definition Language) format. + /// Available for NTFS and WIM file systems that implement IWindowsFileSystem. + /// Null if not available from the archive format. + /// + public string? SecurityDescriptorSddl { get; set; } } } diff --git a/nuget.config b/nuget.config index c25d0d6c..ba47b6aa 100644 --- a/nuget.config +++ b/nuget.config @@ -1,7 +1,7 @@ - + - + - \ No newline at end of file + diff --git a/nuget.config.original b/nuget.config.original deleted file mode 100644 index 227ad0ce..00000000 --- a/nuget.config.original +++ /dev/null @@ -1,7 +0,0 @@ - - - - - - - \ No newline at end of file From 7913f8dbd034703e9dac6654ac945aaaa54fa8cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 05:12:35 +0000 Subject: [PATCH 06/13] Address review feedback: improve test comment clarity Co-authored-by: gfs <98900+gfs@users.noreply.github.com> --- RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs index bef2a127..5ffa462c 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs @@ -211,7 +211,7 @@ public void IsoRockRidgeEntries_HaveMetadata_Sync() [Fact] public async Task VhdxNtfsEntries_HaveWindowsMetadata() { - // TestData.vhdx contains an NTFS file system which implements IDosFileSystem and IWindowsFileSystem + // TestData.vhdx contains an NTFS file system that implements IDosFileSystem and IWindowsFileSystem var extractor = new Extractor(); var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.vhdx"); var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); From 38439c717eaab42581970c587b8791256310f2a1 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Feb 2026 08:58:23 -0800 Subject: [PATCH 07/13] Dispose backing streams and skip entries on extraction failure in TarExtractor and ZipExtractor (#187) Reliability improvements for Tar and Zip extractors to improve behavior around failure to extract individual entries. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: gfs <98900+gfs@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- RecursiveExtractor/Extractors/TarExtractor.cs | 4 ++++ RecursiveExtractor/Extractors/ZipExtractor.cs | 11 +++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/RecursiveExtractor/Extractors/TarExtractor.cs b/RecursiveExtractor/Extractors/TarExtractor.cs index 4d5a2dc5..0d7329da 100644 --- a/RecursiveExtractor/Extractors/TarExtractor.cs +++ b/RecursiveExtractor/Extractors/TarExtractor.cs @@ -62,6 +62,8 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra catch (Exception e) { Logger.Debug(Extractor.FAILED_PARSING_ERROR_MESSAGE_STRING, ArchiveFileType.TAR, fileEntry.FullPath, tarEntry.Key, e.GetType()); + fs?.Dispose(); + continue; } var name = tarEntry.Key?.Replace('/', Path.DirectorySeparatorChar); if (string.IsNullOrEmpty(name)) @@ -135,6 +137,8 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti catch (Exception e) { Logger.Debug(Extractor.FAILED_PARSING_ERROR_MESSAGE_STRING, ArchiveFileType.TAR, fileEntry.FullPath, tarEntry.Key, e.GetType()); + fs?.Dispose(); + continue; } var name = tarEntry.Key?.Replace('/', Path.DirectorySeparatorChar); if (string.IsNullOrEmpty(name)) diff --git a/RecursiveExtractor/Extractors/ZipExtractor.cs b/RecursiveExtractor/Extractors/ZipExtractor.cs index 92dfc040..49da757c 100644 --- a/RecursiveExtractor/Extractors/ZipExtractor.cs +++ b/RecursiveExtractor/Extractors/ZipExtractor.cs @@ -141,11 +141,12 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra catch (Exception e) { Logger.Debug(Extractor.FAILED_PARSING_ERROR_MESSAGE_STRING, ArchiveFileType.ZIP, fileEntry.FullPath, zipEntry.Key, e.GetType()); + target?.Dispose(); + continue; } - target ??= new MemoryStream(); var name = zipEntry.Key?.Replace('/', Path.DirectorySeparatorChar) ?? ""; - var newFileEntry = new FileEntry(name, target, fileEntry, modifyTime: zipEntry.LastModifiedTime, memoryStreamCutoff: options.MemoryStreamCutoff); + var newFileEntry = new FileEntry(name, target, fileEntry, modifyTime: zipEntry.LastModifiedTime, memoryStreamCutoff: options.MemoryStreamCutoff, passthroughStream: true); try { @@ -254,7 +255,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti governor.CheckResourceGovernor(zipEntry.Size); - using var fs = StreamFactory.GenerateAppropriateBackingStream(options, zipEntry.Size); + var fs = StreamFactory.GenerateAppropriateBackingStream(options, zipEntry.Size); try { @@ -264,10 +265,12 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti catch (Exception e) { Logger.Debug(Extractor.FAILED_PARSING_ERROR_MESSAGE_STRING, ArchiveFileType.ZIP, fileEntry.FullPath, zipEntry.Key, e.GetType()); + fs?.Dispose(); + continue; } var name = zipEntry.Key?.Replace('/', Path.DirectorySeparatorChar) ?? ""; - var newFileEntry = new FileEntry(name, fs, fileEntry, modifyTime: zipEntry.LastModifiedTime, memoryStreamCutoff: options.MemoryStreamCutoff); + var newFileEntry = new FileEntry(name, fs, fileEntry, passthroughStream: true, modifyTime: zipEntry.LastModifiedTime, memoryStreamCutoff: options.MemoryStreamCutoff); try { From b97c7840d6c133726115cb2d72c58f9875c8f108 Mon Sep 17 00:00:00 2001 From: Giulia Stocco <98900+gfs@users.noreply.github.com> Date: Mon, 23 Feb 2026 21:28:29 -0800 Subject: [PATCH 08/13] Improve ZipSlip sanitization and output-dir checks (#201) ZipSlipSanitize only stripped .. sequences but did not handle absolute paths or Windows drive roots in archive entry names. Since Path.Combine(base, absolutePath) discards the base argument when the second path is absolute, ExtractToDirectory could write files outside the intended output directory. A secondary typo ($"{sep},{sep}" instead of $"{sep}{sep}") also prevented double-separator cleanup from ever running. Changes: - Normalize path separators to OS-native, strip drive-letter roots (only when followed by a separator), strip leading separators, and remove ../. as whole path segments (not substrings, preserving names like file..txt) - Apply ZipSlipSanitize before SanitizePath in the FileEntry constructor so traversal segments are removed while path structure is intact - Add defense-in-depth resolved-path containment check in all ExtractToDirectory code paths (sync, parallel, async) - Trim trailing separator from resolved output directory before appending one, preventing double-separator mismatch when the output is a root path - Make drive-root stripping recursive to prevent nested-root bypass (e.g. C:\C:\...) - Add comprehensive SanitizePathTests covering absolute Unix/Windows paths, drive roots, .. traversal, combined attacks, colon-containing filenames, . segments, double-separator collapsing, and FileEntry integration ----------------- Co-authored-by: gfs <98900+gfs@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> --- RecursiveExtractor.Tests/SanitizePathTests.cs | 160 ++++++++++++++++++ RecursiveExtractor/Extractor.cs | 21 +++ RecursiveExtractor/FileEntry.cs | 75 ++++++-- nuget.config | 4 +- 4 files changed, 247 insertions(+), 13 deletions(-) diff --git a/RecursiveExtractor.Tests/SanitizePathTests.cs b/RecursiveExtractor.Tests/SanitizePathTests.cs index 3303238e..06c9261c 100644 --- a/RecursiveExtractor.Tests/SanitizePathTests.cs +++ b/RecursiveExtractor.Tests/SanitizePathTests.cs @@ -35,6 +35,166 @@ public void TestSanitizePathLinux(string linuxInputPath, string expectedLinuxPat } } + /// + /// ZipSlipSanitize should strip leading slashes to prevent absolute path traversal. + /// On any OS, a Unix-style absolute path like "/etc/passwd" must become relative. + /// + [Theory] + [InlineData("/etc/passwd", "etc/passwd")] + [InlineData("/tmp/evil.txt", "tmp/evil.txt")] + [InlineData("///triple/leading", "triple/leading")] + [InlineData("/", "")] + public void TestZipSlipSanitize_AbsoluteUnixPaths(string input, string expected) + { + // Normalize expected to the current OS separator + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should strip Windows drive letter roots. + /// + [Theory] + [InlineData("C:\\Windows\\System32\\evil.dll", "Windows/System32/evil.dll")] + [InlineData("D:/data/file.txt", "data/file.txt")] + [InlineData("C:\\", "")] + [InlineData("C:/", "")] + [InlineData("a:file.txt", "a:file.txt")] + public void TestZipSlipSanitize_AbsoluteWindowsPaths(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should recursively strip nested roots that are exposed after each strip. + /// A single pass is insufficient for crafted paths like "D:\C:\" (stripping D: exposes \C:\), + /// or paths starting with multiple separators like "\\C:\" (stripping both leading separators + /// exposes C:\), or "../C:\file" where removing ".." exposes C:\file. + /// + [Theory] + [InlineData("D:\\C:\\file.txt", "file.txt")] + [InlineData("\\\\C:\\file.txt", "file.txt")] + [InlineData("..\\C:\\file.txt", "file.txt")] + [InlineData("D:/C:/file.txt", "file.txt")] + [InlineData("D:\\C:\\D:\\file.txt", "file.txt")] + public void TestZipSlipSanitize_NestedRoots(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should strip ".." directory traversal components. + /// + [Theory] + [InlineData("foo/../../../etc/passwd", "foo/etc/passwd")] + [InlineData("../secret.txt", "secret.txt")] + [InlineData("a/b/../../c", "a/b/c")] + public void TestZipSlipSanitize_DotDotTraversal(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should handle combined absolute + traversal attacks. + /// After ".." is stripped, exposed leading separators are also stripped. + /// + [Theory] + [InlineData("/../../etc/passwd", "etc/passwd")] + [InlineData("C:\\..\\..\\Windows\\evil.dll", "Windows/evil.dll")] + [InlineData("/../../../tmp/evil", "tmp/evil")] + public void TestZipSlipSanitize_CombinedAbsoluteAndTraversal(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should collapse double separators that result from stripping. + /// + [Theory] + [InlineData("a/../b", "a/b")] + [InlineData("a/../../b", "a/b")] + public void TestZipSlipSanitize_CollapsesDoubleSeparators(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// Safe relative paths should pass through unmodified. + /// Filenames containing ".." as a substring (not a path segment) must be preserved. + /// + [Theory] + [InlineData("normal/path/file.txt", "normal/path/file.txt")] + [InlineData("file.txt", "file.txt")] + [InlineData("a/b/c/d.txt", "a/b/c/d.txt")] + [InlineData("file..txt", "file..txt")] + [InlineData("my..archive/data..bin", "my..archive/data..bin")] + [InlineData("a/./b", "a/b")] + [InlineData("a:file.txt", "a:file.txt")] + [InlineData("a:folder/file.txt", "a:folder/file.txt")] + public void TestZipSlipSanitize_SafePathsUnchanged(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// FileEntry.FullPath should never be absolute when constructed with a parent, + /// even if the entry name is an absolute path. + /// + [Fact] + public void TestFileEntry_AbsoluteEntryName_BecomesRelative() + { + var parent = new FileEntry("archive.zip", Stream.Null); + var child = new FileEntry("/etc/cron.d/evil", Stream.Null, parent); + Assert.False(Path.IsPathRooted(child.FullPath), + $"FullPath should be relative but was: {child.FullPath}"); + AssertNoTraversalSegments(child.FullPath); + } + + /// + /// FileEntry.FullPath should not contain ".." path segments even when the entry name has traversal. + /// + [Fact] + public void TestFileEntry_TraversalEntryName_Sanitized() + { + var parent = new FileEntry("archive.tar", Stream.Null); + var child = new FileEntry("../../../etc/passwd", Stream.Null, parent); + AssertNoTraversalSegments(child.FullPath); + } + + /// + /// Backslash-rooted paths should also be made relative. + /// + [Fact] + public void TestFileEntry_BackslashRooted_BecomesRelative() + { + var parent = new FileEntry("archive.zip", Stream.Null); + var child = new FileEntry("\\Windows\\System32\\evil.dll", Stream.Null, parent); + Assert.False(Path.IsPathRooted(child.FullPath), + $"FullPath should be relative but was: {child.FullPath}"); + } + + /// + /// Assert that no path segment is exactly ".." (ignoring ".." as a substring in filenames like "file..txt"). + /// + private static void AssertNoTraversalSegments(string path) + { + var segments = path.Split(Path.DirectorySeparatorChar); + Assert.DoesNotContain("..", segments); + } + protected static readonly NLog.Logger Logger = NLog.LogManager.GetCurrentClassLogger(); } } \ No newline at end of file diff --git a/RecursiveExtractor/Extractor.cs b/RecursiveExtractor/Extractor.cs index 85d53e8a..ec5015df 100644 --- a/RecursiveExtractor/Extractor.cs +++ b/RecursiveExtractor/Extractor.cs @@ -495,9 +495,16 @@ public ExtractionStatusCode ExtractToDirectory(string outputDirectory, FileEntry opts ??= new ExtractorOptions(); if (!opts.Parallel) { + var fullOutputDir = Path.GetFullPath(outputDirectory).TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; foreach (var entry in Extract(fileEntry, opts)) { var targetPath = Path.Combine(outputDirectory, entry.GetSanitizedPath()); + var fullTargetPath = Path.GetFullPath(targetPath); + if (!fullTargetPath.StartsWith(fullOutputDir, StringComparison.Ordinal)) + { + Logger.Warn("Skipping entry that would escape output directory: {0}", entry.FullPath); + continue; + } if (Path.GetDirectoryName(targetPath) is { } directoryPathNotNull && targetPath is { } targetPathNotNull) { try @@ -537,6 +544,7 @@ public ExtractionStatusCode ExtractToDirectory(string outputDirectory, FileEntry using var enumerator = extractedEnumeration.GetEnumerator(); // Move to the first element to prepare ConcurrentBag entryBatch = new(); + var parallelFullOutputDir = Path.GetFullPath(outputDirectory).TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; bool moreAvailable = enumerator.MoveNext(); while (moreAvailable) { @@ -559,6 +567,12 @@ public ExtractionStatusCode ExtractToDirectory(string outputDirectory, FileEntry Parallel.ForEach(entryBatch, new ParallelOptions() { CancellationToken = cts.Token }, entry => { var targetPath = Path.Combine(outputDirectory, entry.GetSanitizedPath()); + var fullTargetPath = Path.GetFullPath(targetPath); + if (!fullTargetPath.StartsWith(parallelFullOutputDir, StringComparison.Ordinal)) + { + Logger.Warn("Skipping entry that would escape output directory: {0}", entry.FullPath); + return; + } if (Path.GetDirectoryName(targetPath) is { } directoryPathNotNull && targetPath is { } targetPathNotNull) { @@ -645,11 +659,18 @@ public async Task ExtractToDirectoryAsync(string outputDir /// If we should print the filename when writing it out to disc. public async Task ExtractToDirectoryAsync(string outputDirectory, FileEntry fileEntry, ExtractorOptions? opts = null, IEnumerable? acceptFilters = null, IEnumerable? denyFilters = null, bool printNames = false) { + var asyncFullOutputDir = Path.GetFullPath(outputDirectory).TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; await foreach (var entry in ExtractAsync(fileEntry, opts)) { if (opts?.FileNamePasses(entry.FullPath) ?? true) { var targetPath = Path.Combine(outputDirectory, entry.GetSanitizedPath()); + var fullTargetPath = Path.GetFullPath(targetPath); + if (!fullTargetPath.StartsWith(asyncFullOutputDir, StringComparison.Ordinal)) + { + Logger.Warn("Skipping entry that would escape output directory: {0}", entry.FullPath); + continue; + } if (Path.GetDirectoryName(targetPath) is { } directoryPathNotNull && targetPath is { } targetPathNotNull) { diff --git a/RecursiveExtractor/FileEntry.cs b/RecursiveExtractor/FileEntry.cs index c5427fdd..ca74e5a0 100644 --- a/RecursiveExtractor/FileEntry.cs +++ b/RecursiveExtractor/FileEntry.cs @@ -40,8 +40,10 @@ public FileEntry(string name, Stream inputStream, FileEntry? parent = null, bool ModifyTime = modifyTime ?? DateTime.MinValue; AccessTime = accessTime ?? DateTime.MinValue; - // Sanitize so its safe to use with Path APIs - string sanitizedName = SanitizePath(name); + // Sanitize traversal and absolute paths first while path structure is intact + string zipSlipSafeName = ZipSlipSanitize(name); + // Then replace any remaining OS-invalid characters + string sanitizedName = SanitizePath(zipSlipSafeName); Name = Path.GetFileName(sanitizedName); // If parent is null use the provided name as the FullPath @@ -297,25 +299,76 @@ public static async Task FromStreamAsync(string name, Stream content, } /// - /// Replace .. for ZipSlip and remove any doubled up directory separators as a result - https://snyk.io/research/zip-slip-vulnerability + /// Sanitize paths to prevent ZipSlip (directory traversal) and absolute path traversal. + /// Strips leading directory separators and drive letter roots, removes ".." sequences, + /// and collapses resulting double separators. See https://snyk.io/research/zip-slip-vulnerability /// /// The path to sanitize /// The string to replace .. with - /// A path without ZipSlip + /// A relative path safe from directory traversal [Pure] - private static string ZipSlipSanitize(string fullPath, string replacement = "") + internal static string ZipSlipSanitize(string fullPath, string replacement = "") { - if (fullPath.Contains("..")) + // Normalize all separators to the OS-native separator + fullPath = fullPath.Replace('\\', Path.DirectorySeparatorChar).Replace('/', Path.DirectorySeparatorChar); + + // Strip drive letter roots and leading separators. Must run before and after ".." + // removal because ".." removal can expose a new drive root (e.g. "../C:\file" → "C:\file"). + fullPath = StripLeadingRootComponents(fullPath); + + if (fullPath.Length > 0) { - fullPath = fullPath.Replace("..", replacement); - var directorySeparator = Path.DirectorySeparatorChar.ToString(); - var doubleSeparator = $"{directorySeparator},{directorySeparator}"; - while (fullPath.Contains(doubleSeparator)) + var separator = Path.DirectorySeparatorChar; + var segments = fullPath.Split(separator); + + for (var i = 0; i < segments.Length; i++) { - fullPath = fullPath.Replace(doubleSeparator, $"{directorySeparator}"); + // Replace traversal segments (".." and ".") only when they are whole segments + if (segments[i] == ".." || segments[i] == ".") + { + segments[i] = replacement; + } } + + // Rebuild the path from non-empty segments + fullPath = string.Join(separator.ToString(), segments.Where(s => !string.IsNullOrEmpty(s))); } + fullPath = StripLeadingRootComponents(fullPath); + + return fullPath; + } + + /// + /// Repeatedly strips Windows drive letter roots (e.g. "C:\") and leading directory + /// separators until the path is stable. A single pass is insufficient for crafted paths + /// like "D:\C:\" where stripping "D:" exposes a new root "C:\". + /// + private static string StripLeadingRootComponents(string fullPath) + { + bool changed; + do + { + changed = false; + + // Strip Windows drive letter roots (e.g., "C:\", "D:/") but not relative names like "a:file.txt" + if (fullPath.Length >= 3 + && char.IsLetter(fullPath[0]) + && fullPath[1] == ':' + && fullPath[2] == Path.DirectorySeparatorChar) + { + fullPath = fullPath.Substring(2); + changed = true; + } + + // Strip leading directory separators to prevent absolute path traversal + while (fullPath.Length > 0 && fullPath[0] == Path.DirectorySeparatorChar) + { + fullPath = fullPath.Substring(1); + changed = true; + } + } + while (changed); return fullPath; } diff --git a/nuget.config b/nuget.config index 227ad0ce..ba47b6aa 100644 --- a/nuget.config +++ b/nuget.config @@ -1,7 +1,7 @@ - + - \ No newline at end of file + From 196a5575e677e5495bf50ccf6b5a2a1cb4ce5a46 Mon Sep 17 00:00:00 2001 From: Giulia Stocco <98900+gfs@users.noreply.github.com> Date: Tue, 28 Apr 2026 10:26:02 -0700 Subject: [PATCH 09/13] Minor Version Dependency Bumps (#204) * Update Microsoft.Bcl.AsyncInterfaces to version 10.0.6 * Update System.Linq.AsyncEnumerable package version --- RecursiveExtractor/RecursiveExtractor.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/RecursiveExtractor/RecursiveExtractor.csproj b/RecursiveExtractor/RecursiveExtractor.csproj index 1b6bd271..0695555f 100644 --- a/RecursiveExtractor/RecursiveExtractor.csproj +++ b/RecursiveExtractor/RecursiveExtractor.csproj @@ -45,11 +45,11 @@ - + - + From 8a14f2840bf4ad9746a6b7c75d979a8cf3920bee Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 11 May 2026 12:18:44 -0700 Subject: [PATCH 10/13] Upgrade SharpCompress to 0.47.4 and migrate extractor APIs to new OpenReader/OpenArchive surface (#205) * Initial plan * Upgrade SharpCompress to 0.47.4 and migrate breaking API calls Agent-Logs-Url: https://github.com/microsoft/RecursiveExtractor/sessions/4a1fc24b-3c85-4d07-ae1d-e352607abda7 Co-authored-by: gfs <98900+gfs@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: gfs <98900+gfs@users.noreply.github.com> --- RecursiveExtractor/Extractors/AceExtractor.cs | 8 ++++---- RecursiveExtractor/Extractors/ArcExtractor.cs | 8 ++++---- RecursiveExtractor/Extractors/ArjExtractor.cs | 8 ++++---- RecursiveExtractor/Extractors/BZip2Extractor.cs | 6 +++--- RecursiveExtractor/Extractors/RarExtractor.cs | 6 +++--- RecursiveExtractor/Extractors/SevenZipExtractor.cs | 6 +++--- RecursiveExtractor/Extractors/TarExtractor.cs | 6 +++--- RecursiveExtractor/Extractors/ZipExtractor.cs | 14 +++++++------- RecursiveExtractor/RecursiveExtractor.csproj | 2 +- 9 files changed, 32 insertions(+), 32 deletions(-) diff --git a/RecursiveExtractor/Extractors/AceExtractor.cs b/RecursiveExtractor/Extractors/AceExtractor.cs index ae0429f8..54c0b2ff 100644 --- a/RecursiveExtractor/Extractors/AceExtractor.cs +++ b/RecursiveExtractor/Extractors/AceExtractor.cs @@ -29,11 +29,11 @@ public AceExtractor(Extractor context) /// public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - AceReader? aceReader = null; + IReader? aceReader = null; try { fileEntry.Content.Position = 0; - aceReader = AceReader.Open(fileEntry.Content, new ReaderOptions() + aceReader = AceReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -100,11 +100,11 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra /// public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - AceReader? aceReader = null; + IReader? aceReader = null; try { fileEntry.Content.Position = 0; - aceReader = AceReader.Open(fileEntry.Content, new ReaderOptions() + aceReader = AceReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); diff --git a/RecursiveExtractor/Extractors/ArcExtractor.cs b/RecursiveExtractor/Extractors/ArcExtractor.cs index 5b33bd12..8247b15e 100644 --- a/RecursiveExtractor/Extractors/ArcExtractor.cs +++ b/RecursiveExtractor/Extractors/ArcExtractor.cs @@ -29,11 +29,11 @@ public ArcExtractor(Extractor context) /// public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - ArcReader? arcReader = null; + IReader? arcReader = null; try { fileEntry.Content.Position = 0; - arcReader = ArcReader.Open(fileEntry.Content, new ReaderOptions() + arcReader = ArcReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -103,11 +103,11 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra /// public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - ArcReader? arcReader = null; + IReader? arcReader = null; try { fileEntry.Content.Position = 0; - arcReader = ArcReader.Open(fileEntry.Content, new ReaderOptions() + arcReader = ArcReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); diff --git a/RecursiveExtractor/Extractors/ArjExtractor.cs b/RecursiveExtractor/Extractors/ArjExtractor.cs index 4c1f324a..bb836879 100644 --- a/RecursiveExtractor/Extractors/ArjExtractor.cs +++ b/RecursiveExtractor/Extractors/ArjExtractor.cs @@ -29,11 +29,11 @@ public ArjExtractor(Extractor context) /// public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - ArjReader? arjReader = null; + IReader? arjReader = null; try { fileEntry.Content.Position = 0; - arjReader = ArjReader.Open(fileEntry.Content, new ReaderOptions() + arjReader = ArjReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -100,11 +100,11 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra /// public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - ArjReader? arjReader = null; + IReader? arjReader = null; try { fileEntry.Content.Position = 0; - arjReader = ArjReader.Open(fileEntry.Content, new ReaderOptions() + arjReader = ArjReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); diff --git a/RecursiveExtractor/Extractors/BZip2Extractor.cs b/RecursiveExtractor/Extractors/BZip2Extractor.cs index 60ff5242..0276b403 100644 --- a/RecursiveExtractor/Extractors/BZip2Extractor.cs +++ b/RecursiveExtractor/Extractors/BZip2Extractor.cs @@ -40,7 +40,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra var failed = false; try { - using var bzipStream = new BZip2Stream(fileEntry.Content, CompressionMode.Decompress, false); + using var bzipStream = BZip2Stream.Create(fileEntry.Content, CompressionMode.Decompress, false, leaveOpen: false); await bzipStream.CopyToAsync(fs); } catch (Exception e) @@ -99,7 +99,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti try { - using var bzipStream = new BZip2Stream(fileEntry.Content, CompressionMode.Decompress, false); + using var bzipStream = BZip2Stream.Create(fileEntry.Content, CompressionMode.Decompress, false, leaveOpen: false); bzipStream.CopyTo(fs); } catch (Exception e) @@ -139,4 +139,4 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti } } } -} \ No newline at end of file +} diff --git a/RecursiveExtractor/Extractors/RarExtractor.cs b/RecursiveExtractor/Extractors/RarExtractor.cs index 829b3d30..85d7ff80 100644 --- a/RecursiveExtractor/Extractors/RarExtractor.cs +++ b/RecursiveExtractor/Extractors/RarExtractor.cs @@ -31,7 +31,7 @@ public RarExtractor(Extractor context) try { - rarArchive = RarArchive.Open(fileEntry.Content); + rarArchive = (RarArchive)RarArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { LeaveStreamOpen = true }); // Test for invalid archives. This will throw invalidformatexception var t = rarArchive.IsSolid; if (rarArchive.Entries.Any(x => x.IsEncrypted)) @@ -66,7 +66,7 @@ public RarExtractor(Extractor context) try { fileEntry.Content.Position = 0; - rarArchive = RarArchive.Open(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { Password = password, LookForHeader = true }); + rarArchive = (RarArchive)RarArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { Password = password, LookForHeader = true, LeaveStreamOpen = true }); var byt = new byte[1]; var encryptedEntry = rarArchive.Entries.FirstOrDefault(x => x is { IsEncrypted: true, Size: > 0 }); // Justification for !: Because we use FirstOrDefault encryptedEntry may be null, but we have a catch below for it @@ -197,4 +197,4 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti } } } -} \ No newline at end of file +} diff --git a/RecursiveExtractor/Extractors/SevenZipExtractor.cs b/RecursiveExtractor/Extractors/SevenZipExtractor.cs index 40f6b68e..869da19d 100644 --- a/RecursiveExtractor/Extractors/SevenZipExtractor.cs +++ b/RecursiveExtractor/Extractors/SevenZipExtractor.cs @@ -83,7 +83,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra var needsPassword = false; try { - sevenZipArchive = SevenZipArchive.Open(fileEntry.Content); + sevenZipArchive = (SevenZipArchive)SevenZipArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { LeaveStreamOpen = true }); if (sevenZipArchive.Entries.Where(x => !x.IsDirectory).Any(x => x.IsEncrypted)) { needsPassword = true; @@ -114,7 +114,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra try { fileEntry.Content.Position = 0; - sevenZipArchive = SevenZipArchive.Open(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { Password = password }); + sevenZipArchive = (SevenZipArchive)SevenZipArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { Password = password, LeaveStreamOpen = true }); // When filenames are encrypted we can't access the size of individual files // But if we can accesss the total uncompressed size we have the right password try @@ -197,4 +197,4 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti } } } -} \ No newline at end of file +} diff --git a/RecursiveExtractor/Extractors/TarExtractor.cs b/RecursiveExtractor/Extractors/TarExtractor.cs index 0d7329da..662f8597 100644 --- a/RecursiveExtractor/Extractors/TarExtractor.cs +++ b/RecursiveExtractor/Extractors/TarExtractor.cs @@ -29,7 +29,7 @@ public TarExtractor(Extractor context) /// public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - using TarArchive archive = TarArchive.Open(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() + using TarArchive archive = (TarArchive)TarArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { LeaveStreamOpen = true }); @@ -103,7 +103,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra /// public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - using TarArchive archive = TarArchive.Open(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() + using TarArchive archive = (TarArchive)TarArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { LeaveStreamOpen = true }); @@ -224,4 +224,4 @@ internal TarEntryCollectionEnumerator(ICollection entries, stri } } } -} \ No newline at end of file +} diff --git a/RecursiveExtractor/Extractors/ZipExtractor.cs b/RecursiveExtractor/Extractors/ZipExtractor.cs index 49da757c..5140a60d 100644 --- a/RecursiveExtractor/Extractors/ZipExtractor.cs +++ b/RecursiveExtractor/Extractors/ZipExtractor.cs @@ -37,7 +37,7 @@ public ZipExtractor(Extractor context) { // Create a new archive instance with the password to test it fileEntry.Content.Position = 0; - using var testArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + using var testArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { Password = password, LeaveStreamOpen = true @@ -73,7 +73,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra try { fileEntry.Content.Position = 0; - zipArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + zipArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -104,7 +104,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra // Recreate archive with password zipArchive.Dispose(); fileEntry.Content.Position = 0; - zipArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + zipArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { Password = foundPassword, LeaveStreamOpen = true @@ -199,7 +199,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti try { fileEntry.Content.Position = 0; - zipArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + zipArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -230,7 +230,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti // Recreate archive with password zipArchive.Dispose(); fileEntry.Content.Position = 0; - zipArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + zipArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { Password = foundPassword, LeaveStreamOpen = true @@ -329,7 +329,7 @@ private async IAsyncEnumerable YieldNonIndexedEntriesAsync( IReader? forwardReader = null; try { - forwardReader = ReaderFactory.Open(parentEntry.Content, new ReaderOptions { LeaveStreamOpen = true }); + forwardReader = ReaderFactory.OpenReader(parentEntry.Content, new ReaderOptions { LeaveStreamOpen = true }); } catch (Exception ex) { @@ -413,7 +413,7 @@ private IEnumerable YieldNonIndexedEntries( IReader? forwardReader = null; try { - forwardReader = ReaderFactory.Open(parentEntry.Content, new ReaderOptions { LeaveStreamOpen = true }); + forwardReader = ReaderFactory.OpenReader(parentEntry.Content, new ReaderOptions { LeaveStreamOpen = true }); } catch (Exception ex) { diff --git a/RecursiveExtractor/RecursiveExtractor.csproj b/RecursiveExtractor/RecursiveExtractor.csproj index 0695555f..23a06036 100644 --- a/RecursiveExtractor/RecursiveExtractor.csproj +++ b/RecursiveExtractor/RecursiveExtractor.csproj @@ -48,7 +48,7 @@ - + From 64b24e3dada864fc32f24183dc094c7167bbd364 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 15 Jun 2026 12:32:21 -0700 Subject: [PATCH 11/13] Bump dependencies to latest; upgrade SharpCompress to 0.49.1 (#207) * Plan: bump dependency versions to latest * Bump dependencies to latest; * Use SharpCompress 0.49.1; fix PAX-header test counts --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --- .../CliTests/CliTests.cs | 8 ++--- .../RecursiveExtractor.Cli.Tests.csproj | 6 ++-- .../RecursiveExtractor.Cli.csproj | 2 +- .../ExtractorTests/DisposeBehaviorTests.cs | 16 ++++----- .../ExtractorTests/ExpectedNumFilesTests.cs | 10 +++--- .../ExtractorTests/FilterTests.cs | 8 ++--- .../RecursiveExtractor.Tests.csproj | 10 +++--- RecursiveExtractor/RecursiveExtractor.csproj | 36 +++++++++---------- 8 files changed, 48 insertions(+), 48 deletions(-) diff --git a/RecursiveExtractor.Cli.Tests/CliTests/CliTests.cs b/RecursiveExtractor.Cli.Tests/CliTests/CliTests.cs index 7c9eb773..71a35820 100644 --- a/RecursiveExtractor.Cli.Tests/CliTests/CliTests.cs +++ b/RecursiveExtractor.Cli.Tests/CliTests/CliTests.cs @@ -27,11 +27,11 @@ public static TheoryData CliArchiveData { { "TestData.zip", 5 }, { "TestData.7z", 3 }, - { "TestData.tar", 6 }, + { "TestData.tar", 5 }, { "TestData.rar", 3 }, { "TestData.rar4", 3 }, - { "TestData.tar.bz2", 6 }, - { "TestData.tar.gz", 6 }, + { "TestData.tar.bz2", 5 }, + { "TestData.tar.gz", 5 }, { "TestData.tar.xz", 3 }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8 }, { "TestData.a", 3 }, @@ -39,7 +39,7 @@ public static TheoryData CliArchiveData { "TestData.iso", 3 }, { "TestData.vhdx", 3 }, { "EmptyFile.txt", 1 }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 54 : 52 }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 51 : 49 }, }; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/RecursiveExtractor.Cli.Tests/RecursiveExtractor.Cli.Tests.csproj b/RecursiveExtractor.Cli.Tests/RecursiveExtractor.Cli.Tests.csproj index 41d07b76..89ddffce 100644 --- a/RecursiveExtractor.Cli.Tests/RecursiveExtractor.Cli.Tests.csproj +++ b/RecursiveExtractor.Cli.Tests/RecursiveExtractor.Cli.Tests.csproj @@ -10,9 +10,9 @@ - - - + + + diff --git a/RecursiveExtractor.Cli/RecursiveExtractor.Cli.csproj b/RecursiveExtractor.Cli/RecursiveExtractor.Cli.csproj index df2aa1a5..1a24e848 100644 --- a/RecursiveExtractor.Cli/RecursiveExtractor.Cli.csproj +++ b/RecursiveExtractor.Cli/RecursiveExtractor.Cli.csproj @@ -29,7 +29,7 @@ - + diff --git a/RecursiveExtractor.Tests/ExtractorTests/DisposeBehaviorTests.cs b/RecursiveExtractor.Tests/ExtractorTests/DisposeBehaviorTests.cs index da7883bc..af9d136b 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/DisposeBehaviorTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/DisposeBehaviorTests.cs @@ -23,11 +23,11 @@ public static TheoryData DisposeData var data = new TheoryData { { "TestData.7z", 3, false }, - { "TestData.tar", 6, false }, + { "TestData.tar", 5, false }, { "TestData.rar", 3, false }, { "TestData.rar4", 3, false }, - { "TestData.tar.bz2", 6, false }, - { "TestData.tar.gz", 6, false }, + { "TestData.tar.bz2", 5, false }, + { "TestData.tar.gz", 5, false }, { "TestData.tar.xz", 3, false }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8, false }, { "TestData.a", 3, false }, @@ -37,11 +37,11 @@ public static TheoryData DisposeData { "EmptyFile.txt", 1, false }, { "TestData.zip", 5, true }, { "TestData.7z", 3, true }, - { "TestData.tar", 6, true }, + { "TestData.tar", 5, true }, { "TestData.rar", 3, true }, { "TestData.rar4", 3, true }, - { "TestData.tar.bz2", 6, true }, - { "TestData.tar.gz", 6, true }, + { "TestData.tar.bz2", 5, true }, + { "TestData.tar.gz", 5, true }, { "TestData.tar.xz", 3, true }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8, true }, { "TestData.a", 3, true }, @@ -49,8 +49,8 @@ public static TheoryData DisposeData { "TestData.iso", 3, true }, { "TestData.vhdx", 3, true }, { "EmptyFile.txt", 1, true }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 54 : 52, true }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 54 : 52, false }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 51 : 49, true }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 51 : 49, false }, }; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs b/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs index 92dd3c24..3f8eeaad 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs @@ -28,11 +28,11 @@ public static TheoryData ArchiveData { "100trees.7z", 101 }, { "TestData.zip", 5 }, { "TestData.7z",3 }, - { "TestData.tar", 6 }, + { "TestData.tar", 5 }, { "TestData.rar",3 }, { "TestData.rar4",3 }, - { "TestData.tar.bz2", 6 }, - { "TestData.tar.gz", 6 }, + { "TestData.tar.bz2", 5 }, + { "TestData.tar.gz", 5 }, { "TestData.tar.xz",3 }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8 }, { "TestData.a",3 }, @@ -40,7 +40,7 @@ public static TheoryData ArchiveData { "TestData.iso",3 }, { "TestData.vhdx",3 }, { "EmptyFile.txt", 1 }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 54 : 52 }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 51 : 49 }, { "UdfTest.iso", 3 }, { "UdfTestWithMultiSystem.iso", 3 }, { "TestData.arj", 1 }, @@ -69,7 +69,7 @@ public static TheoryData NoRecursionData { "100trees.7z", 101 }, { "TestData.zip", 5 }, { "TestData.7z", 3 }, - { "TestData.tar", 6 }, + { "TestData.tar", 5 }, { "TestData.rar", 3 }, { "TestData.rar4", 3 }, { "TestData.tar.bz2", 1 }, diff --git a/RecursiveExtractor.Tests/ExtractorTests/FilterTests.cs b/RecursiveExtractor.Tests/ExtractorTests/FilterTests.cs index 3ed7a32c..50ee5c3e 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/FilterTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/FilterTests.cs @@ -56,11 +56,11 @@ public static TheoryData DenyFilterData { { "TestData.zip", 4 }, { "TestData.7z", 2 }, - { "TestData.tar", 5 }, + { "TestData.tar", 4 }, { "TestData.rar", 2 }, { "TestData.rar4", 2 }, - { "TestData.tar.bz2", 5 }, - { "TestData.tar.gz", 5 }, + { "TestData.tar.bz2", 4 }, + { "TestData.tar.gz", 4 }, { "TestData.tar.xz", 2 }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8 }, { "TestData.a", 3 }, @@ -68,7 +68,7 @@ public static TheoryData DenyFilterData { "TestData.iso", 2 }, { "TestData.vhdx", 2 }, { "EmptyFile.txt", 1 }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 45 : 44 }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 42 : 41 }, }; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj b/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj index 248021b2..75fdbb97 100644 --- a/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj +++ b/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj @@ -10,12 +10,12 @@ - + - - - - + + + + diff --git a/RecursiveExtractor/RecursiveExtractor.csproj b/RecursiveExtractor/RecursiveExtractor.csproj index 23a06036..53fd4f35 100644 --- a/RecursiveExtractor/RecursiveExtractor.csproj +++ b/RecursiveExtractor/RecursiveExtractor.csproj @@ -31,25 +31,25 @@ - - - - - - - - - - - - - + + + + + + + + + + + + + - - - - - + + + + + From b4552e8cd82f0efb02b2a1ba7ad34df477e85182 Mon Sep 17 00:00:00 2001 From: Giulia Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 31 Jul 2026 06:49:33 -0700 Subject: [PATCH 12/13] Complete disc image metadata support: fix ISO tests, avoid per-file exceptions The two IsoEntries_MetadataIsNull* tests were written when only IUnixFileSystem was handled and were never updated when IDosFileSystem support was added, so they asserted null metadata for plain ISOs that now report FileAttributes. They failed and left the PR incomplete. - Fix the ISO tests to assert the real behavior and tighten the RockRidge and NTFS assertions to check concrete values instead of just non-null. - Add UDF and WIM metadata tests, covering the previously untested WimExtractor and UdfExtractor paths. - Skip the Unix branch for non-RockRidge ISO images. CDReader implements IUnixFileSystem unconditionally but throws for every file when the active variant is not RockRidge, which meant one thrown/caught exception per file. - Normalize an empty security descriptor to null so SecurityDescriptorSddl is either absent or meaningful (WIM images report empty SDDL). - Clarify the XML docs per review feedback and document FileEntryMetadata and the per-format support matrix in the README. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 64923784-69ff-4396-9d02-8320536ce3e2 --- README.md | 41 ++++++ .../ExtractorTests/FileMetadataTests.cs | 138 +++++++++++++++--- RecursiveExtractor/Extractors/DiscCommon.cs | 29 +++- RecursiveExtractor/FileEntryMetadata.cs | 8 +- 4 files changed, 182 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 65cccf52..d73b8aa4 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,47 @@ public string? ParentPath { get; } public DateTime CreateTime { get; } public DateTime ModifyTime { get; } public DateTime AccessTime { get; } +public FileEntryMetadata? Metadata { get; set; } +``` + + +
+File Metadata +
+When the source archive or disc image records it, `FileEntry.Metadata` holds additional attributes for the entry. Every property is nullable, and `Metadata` itself is `null` when the format carries no metadata at all, so you can distinguish "not recorded" from a real value. + +```csharp +public long? Mode { get; set; } // Unix permission bits +public bool? IsExecutable { get; } // Derived from Mode +public bool? IsSetUid { get; } // Derived from Mode +public bool? IsSetGid { get; } // Derived from Mode +public long? Uid { get; set; } // Unix owner id +public long? Gid { get; set; } // Unix group id +public FileAttributes? FileAttributes { get; set; } // Windows/DOS file attributes +public string? SecurityDescriptorSddl { get; set; } // Windows security descriptor, SDDL form +``` + +Which properties are populated depends on the format: + +| Source | Populated | +| --- | --- | +| TAR, AR/DEB | `Mode`, `Uid`, `Gid` | +| Ext, XFS, Btrfs, HFS+ (inside VHD/VHDX/VMDK/DMG) | `Mode`, `Uid`, `Gid` | +| ISO 9660 with RockRidge extensions | `Mode`, `Uid`, `Gid`, `FileAttributes` | +| ISO 9660 without RockRidge extensions | `FileAttributes` | +| FAT (inside a disc image) | `FileAttributes` | +| NTFS (inside a disc image) | `FileAttributes`, `SecurityDescriptorSddl` | +| WIM | `FileAttributes`, and `SecurityDescriptorSddl` when the image records one | +| UDF | None; `Metadata` is `null` | + +```csharp +foreach (var file in extractor.Extract("path/to/image.vhdx")) +{ + if (file.Metadata?.SecurityDescriptorSddl is { } sddl) + { + Console.WriteLine($"{file.FullPath}: {sddl}"); + } +} ```
diff --git a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs index 5ffa462c..f98133c8 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs @@ -1,8 +1,10 @@ -// Copyright (c) Microsoft Corporation. Licensed under the MIT License. +// Copyright (c) Microsoft Corporation. Licensed under the MIT License. using Microsoft.CST.RecursiveExtractor; +using System.Collections.Generic; using System.IO; using System.Linq; +using System.Runtime.InteropServices; using System.Threading.Tasks; using Xunit; @@ -144,9 +146,10 @@ public void FileEntry_MetadataDefaultsToNull() } [Fact] - public async Task IsoEntries_MetadataIsNullWithoutRockRidge() + public async Task IsoEntries_HaveDosAttributesButNoUnixMetadataWithoutRockRidge() { - // TestData.iso does not have RockRidge extensions, so Unix metadata is not available + // TestData.iso has no RockRidge extensions, so Unix metadata is unavailable. + // CDReader still implements IDosFileSystem, so file attributes are reported. var extractor = new Extractor(); var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.iso"); var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); @@ -154,13 +157,17 @@ public async Task IsoEntries_MetadataIsNullWithoutRockRidge() Assert.NotEmpty(results); foreach (var entry in results) { - // Without RockRidge extensions, metadata should be null - Assert.Null(entry.Metadata); + Assert.NotNull(entry.Metadata); + Assert.Equal(FileAttributes.ReadOnly, entry.Metadata!.FileAttributes); + Assert.Null(entry.Metadata.Mode); + Assert.Null(entry.Metadata.Uid); + Assert.Null(entry.Metadata.Gid); + Assert.Null(entry.Metadata.SecurityDescriptorSddl); } } [Fact] - public void IsoEntries_MetadataIsNullWithoutRockRidge_Sync() + public void IsoEntries_HaveDosAttributesButNoUnixMetadataWithoutRockRidge_Sync() { var extractor = new Extractor(); var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.iso"); @@ -169,7 +176,12 @@ public void IsoEntries_MetadataIsNullWithoutRockRidge_Sync() Assert.NotEmpty(results); foreach (var entry in results) { - Assert.Null(entry.Metadata); + Assert.NotNull(entry.Metadata); + Assert.Equal(FileAttributes.ReadOnly, entry.Metadata!.FileAttributes); + Assert.Null(entry.Metadata.Mode); + Assert.Null(entry.Metadata.Uid); + Assert.Null(entry.Metadata.Gid); + Assert.Null(entry.Metadata.SecurityDescriptorSddl); } } @@ -181,30 +193,67 @@ public async Task IsoRockRidgeEntries_HaveMetadata() var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataRockRidge.iso"); var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); - Assert.NotEmpty(results); + AssertRockRidgeMetadata(results); + } + + [Fact] + public void IsoRockRidgeEntries_HaveMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataRockRidge.iso"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + AssertRockRidgeMetadata(results); + } + + private static void AssertRockRidgeMetadata(IList results) + { + Assert.Equal(2, results.Count); foreach (var entry in results) { Assert.NotNull(entry.Metadata); - Assert.NotNull(entry.Metadata!.Mode); - Assert.NotNull(entry.Metadata.Uid); - Assert.NotNull(entry.Metadata.Gid); + Assert.Equal(1001, entry.Metadata!.Uid); + Assert.Equal(1001, entry.Metadata.Gid); + Assert.NotNull(entry.Metadata.FileAttributes); } + + // testfile.txt is 0755 (493 decimal), subdir/nested.txt is 0644 (420 decimal) + var topLevel = results.Single(x => x.Name == "testfile.txt"); + Assert.Equal(493, topLevel.Metadata!.Mode); + Assert.True(topLevel.Metadata.IsExecutable); + + var nested = results.Single(x => x.Name == "nested.txt"); + Assert.Equal(420, nested.Metadata!.Mode); + Assert.False(nested.Metadata.IsExecutable); } [Fact] - public void IsoRockRidgeEntries_HaveMetadata_Sync() + public async Task UdfEntries_HaveNoMetadata() { + // UdfReader implements none of the Unix/DOS/Windows file system interfaces, + // so no metadata is available for pure UDF images. var extractor = new Extractor(); - var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataRockRidge.iso"); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "UdfTest.iso"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.Null(entry.Metadata); + } + } + + [Fact] + public void UdfEntries_HaveNoMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "UdfTest.iso"); var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); Assert.NotEmpty(results); foreach (var entry in results) { - Assert.NotNull(entry.Metadata); - Assert.NotNull(entry.Metadata!.Mode); - Assert.NotNull(entry.Metadata.Uid); - Assert.NotNull(entry.Metadata.Gid); + Assert.Null(entry.Metadata); } } @@ -216,14 +265,30 @@ public async Task VhdxNtfsEntries_HaveWindowsMetadata() var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.vhdx"); var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + AssertNtfsMetadata(results); + } + + [Fact] + public void VhdxNtfsEntries_HaveWindowsMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.vhdx"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + AssertNtfsMetadata(results); + } + + private static void AssertNtfsMetadata(IList results) + { Assert.NotEmpty(results); foreach (var entry in results) { Assert.NotNull(entry.Metadata); // NTFS provides Windows file attributes - Assert.NotNull(entry.Metadata!.FileAttributes); + Assert.Equal(FileAttributes.Archive, entry.Metadata!.FileAttributes); // NTFS provides security descriptors Assert.NotNull(entry.Metadata.SecurityDescriptorSddl); + Assert.Contains("O:", entry.Metadata.SecurityDescriptorSddl); // Owner present Assert.Contains("D:", entry.Metadata.SecurityDescriptorSddl); // DACL present // NTFS does not provide Unix metadata Assert.Null(entry.Metadata.Mode); @@ -233,19 +298,48 @@ public async Task VhdxNtfsEntries_HaveWindowsMetadata() } [Fact] - public void VhdxNtfsEntries_HaveWindowsMetadata_Sync() + public async Task WimEntries_HaveWindowsFileAttributes() { + // WIM extraction is only supported on Windows + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + var extractor = new Extractor(); - var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.vhdx"); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.wim"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + AssertWimMetadata(results); + } + + [Fact] + public void WimEntries_HaveWindowsFileAttributes_Sync() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.wim"); var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + AssertWimMetadata(results); + } + + private static void AssertWimMetadata(IList results) + { Assert.NotEmpty(results); foreach (var entry in results) { Assert.NotNull(entry.Metadata); - Assert.NotNull(entry.Metadata!.FileAttributes); - Assert.NotNull(entry.Metadata.SecurityDescriptorSddl); + Assert.Equal(FileAttributes.Archive, entry.Metadata!.FileAttributes); Assert.Null(entry.Metadata.Mode); + Assert.Null(entry.Metadata.Uid); + Assert.Null(entry.Metadata.Gid); + // This WIM records no security descriptors, so an empty SDDL is normalized to null + Assert.Null(entry.Metadata.SecurityDescriptorSddl); } } } diff --git a/RecursiveExtractor/Extractors/DiscCommon.cs b/RecursiveExtractor/Extractors/DiscCommon.cs index 7ffed751..2661d94a 100644 --- a/RecursiveExtractor/Extractors/DiscCommon.cs +++ b/RecursiveExtractor/Extractors/DiscCommon.cs @@ -1,4 +1,5 @@ using DiscUtils; +using DiscUtils.Iso9660; using System; using System.Collections.Generic; using System.Collections.ObjectModel; @@ -16,12 +17,14 @@ public static class DiscCommon /// /// Tries to extract file metadata from a DiscUtils file system entry. - /// For file systems implementing (Ext, Xfs, Btrfs, HfsPlus), + /// For file systems implementing (such as Ext, Xfs, Btrfs, HfsPlus, + /// and ISO 9660 images via CDReader when RockRidge extensions are present), /// returns permissions, UID, and GID. - /// For file systems implementing (NTFS, FAT, WIM), + /// For file systems implementing (such as NTFS, FAT, WIM, and ISO 9660), /// returns Windows file attributes. - /// For file systems implementing (NTFS, WIM), - /// also returns the security descriptor in SDDL format. + /// For file systems implementing (such as NTFS and WIM), + /// also returns the security descriptor in SDDL format when the file system provides one. + /// The lists above are not exhaustive; support is determined by the interfaces the file system implements. /// Returns null for file systems that support none of these interfaces. /// /// The opened disc file system @@ -31,7 +34,7 @@ public static class DiscCommon { FileEntryMetadata? metadata = null; - if (fs is IUnixFileSystem unixFs) + if (fs is IUnixFileSystem unixFs && SupportsUnixMetadata(fs)) { try { @@ -68,11 +71,12 @@ public static class DiscCommon try { var securityDescriptor = windowsFs.GetSecurity(filePath); - if (securityDescriptor != null) + var sddl = securityDescriptor?.GetSddlForm( + DiscUtils.Core.WindowsSecurity.AccessControl.AccessControlSections.All); + if (!string.IsNullOrEmpty(sddl)) { metadata ??= new FileEntryMetadata(); - metadata.SecurityDescriptorSddl = securityDescriptor.GetSddlForm( - DiscUtils.Core.WindowsSecurity.AccessControl.AccessControlSections.All); + metadata.SecurityDescriptorSddl = sddl; } } catch (Exception e) @@ -84,6 +88,15 @@ public static class DiscCommon return metadata; } + /// + /// Determines whether a file system that implements can actually + /// return Unix metadata. CDReader implements the interface unconditionally but only exposes + /// Unix information when the active ISO 9660 variant is RockRidge, and throws for every other + /// variant. Checking up front avoids throwing and catching an exception for every file in an image. + /// + private static bool SupportsUnixMetadata(DiscFileSystem fs) + => fs is not CDReader cdReader || cdReader.ActiveVariant == Iso9660Variant.RockRidge; + /// /// Pre-collects metadata for all files while the file system is still open. /// Used by extractors (e.g., ISO) where the file system is disposed before files are processed. diff --git a/RecursiveExtractor/FileEntryMetadata.cs b/RecursiveExtractor/FileEntryMetadata.cs index f7ae5606..e78618fb 100644 --- a/RecursiveExtractor/FileEntryMetadata.cs +++ b/RecursiveExtractor/FileEntryMetadata.cs @@ -48,15 +48,15 @@ public class FileEntryMetadata /// /// The Windows file attributes (e.g., ReadOnly, Hidden, System, Archive). - /// Available for NTFS, FAT, and WIM file systems. + /// Available for disc image file systems that expose DOS attributes, such as NTFS, FAT, WIM, and ISO 9660. /// Null if not available from the archive format. /// public FileAttributes? FileAttributes { get; set; } /// - /// The NTFS security descriptor in SDDL (Security Descriptor Definition Language) format. - /// Available for NTFS and WIM file systems that implement IWindowsFileSystem. - /// Null if not available from the archive format. + /// The Windows security descriptor in SDDL (Security Descriptor Definition Language) format. + /// Available for disc image file systems that expose Windows security descriptors, such as NTFS and WIM. + /// Null if not available from the archive format, or if the file system did not record one for this file. /// public string? SecurityDescriptorSddl { get; set; } } From f61c96db0d6c2d0cf39d70a6464e2b8d69252cf0 Mon Sep 17 00:00:00 2001 From: Giulia Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:02:05 -0700 Subject: [PATCH 13/13] Address PR review: doc accuracy, metadata failure isolation, hybrid ISO RockRidge README: add ZIP, RAR and 7z to the file metadata support table. The table read as exhaustive while omitting three formats that already populate Mode, so it implied they carried no metadata. DiscCommon.CollectMetadata: read each entry under its own try/catch. It was called inside the try that guards reader construction and entry enumeration, so an unexpected failure while reading metadata would have marked the entire ISO or UDF image as a failed archive. Collection is now documented and implemented as non-throwing, and one bad entry no longer costs metadata for the rest. DiscCommon.CollectIsoMetadata: recover RockRidge metadata from images that carry both Joliet and RockRidge. CDReader is opened with Joliet given priority, and DiscUtils only parses SUSP records for the variant it activates, so on an image built with both (mkisofs -J -R) Mode, Uid and Gid were silently dropped. When the active variant is not RockRidge, a second reader that skips Joliet is opened and its Unix metadata merged by path. Plain ISOs pay one extra volume descriptor scan and nothing per file, since the fallback stops as soon as the second reader reports a non-RockRidge variant. Adds TestDataJolietRockRidge.iso (built with pycdlib, rock_ridge 1.09 + joliet 3, mirroring the layout of TestDataRockRidge.iso) with sync and async coverage, and wires it into the expected file count theories. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d67ebf0-3614-404e-ac29-0c60ee93469a --- README.md | 2 + .../ExtractorTests/ExpectedNumFilesTests.cs | 2 + .../ExtractorTests/FileMetadataTests.cs | 46 +++++++ .../RecursiveExtractor.Tests.csproj | 3 + .../TestDataJolietRockRidge.iso | Bin 0 -> 71680 bytes RecursiveExtractor/Extractors/DiscCommon.cs | 121 +++++++++++++++--- RecursiveExtractor/Extractors/IsoExtractor.cs | 4 +- 7 files changed, 158 insertions(+), 20 deletions(-) create mode 100644 RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataJolietRockRidge.iso diff --git a/README.md b/README.md index d73b8aa4..176b9979 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,8 @@ Which properties are populated depends on the format: | Source | Populated | | --- | --- | | TAR, AR/DEB | `Mode`, `Uid`, `Gid` | +| ZIP | `Mode`, when the entry records Unix permissions in its external attributes | +| RAR, 7z | `Mode`, taken from the raw attribute field the archive records: a Unix mode for archives created on Unix, DOS attributes otherwise | | Ext, XFS, Btrfs, HFS+ (inside VHD/VHDX/VMDK/DMG) | `Mode`, `Uid`, `Gid` | | ISO 9660 with RockRidge extensions | `Mode`, `Uid`, `Gid`, `FileAttributes` | | ISO 9660 without RockRidge extensions | `FileAttributes` | diff --git a/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs b/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs index 2bc44754..f3409ff8 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs @@ -39,6 +39,7 @@ public static TheoryData ArchiveData { "TestData.bsd.ar",3 }, { "TestData.iso",3 }, { "TestDataRockRidge.iso",2 }, + { "TestDataJolietRockRidge.iso",2 }, { "TestData.vhdx",3 }, { "EmptyFile.txt", 1 }, { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 51 : 49 }, @@ -81,6 +82,7 @@ public static TheoryData NoRecursionData { "TestData.bsd.ar", 3 }, { "TestData.iso", 3 }, { "TestDataRockRidge.iso", 2 }, + { "TestDataJolietRockRidge.iso", 2 }, { "TestData.vhdx", 3 }, { "EmptyFile.txt", 1 }, { "TestDataArchivesNested.zip", 14 }, diff --git a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs index f98133c8..6f1a52a9 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs @@ -227,6 +227,52 @@ private static void AssertRockRidgeMetadata(IList results) Assert.False(nested.Metadata.IsExecutable); } + [Fact] + public async Task IsoJolietRockRidgeEntries_HaveUnixMetadata() + { + // TestDataJolietRockRidge.iso carries both a Joliet supplementary volume descriptor and RockRidge + // SUSP records. DiscUtils gives Joliet priority and only parses SUSP records for the variant it + // activates, so the Unix metadata is only reachable through a second reader that skips Joliet. + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataJolietRockRidge.iso"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + AssertJolietRockRidgeMetadata(results); + } + + [Fact] + public void IsoJolietRockRidgeEntries_HaveUnixMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataJolietRockRidge.iso"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + AssertJolietRockRidgeMetadata(results); + } + + private static void AssertJolietRockRidgeMetadata(IList results) + { + Assert.Equal(2, results.Count); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + Assert.Equal(0, entry.Metadata!.Uid); + Assert.Equal(0, entry.Metadata.Gid); + // File attributes still come from the Joliet tree the entries were enumerated from + Assert.Equal(FileAttributes.ReadOnly, entry.Metadata.FileAttributes); + Assert.Null(entry.Metadata.SecurityDescriptorSddl); + } + + // testfile.txt is 0755 (493 decimal), subdir/nested.txt is 0644 (420 decimal) + var topLevel = results.Single(x => x.Name == "testfile.txt"); + Assert.Equal(493, topLevel.Metadata!.Mode); + Assert.True(topLevel.Metadata.IsExecutable); + + var nested = results.Single(x => x.Name == "nested.txt"); + Assert.Equal(420, nested.Metadata!.Mode); + Assert.False(nested.Metadata.IsExecutable); + } + [Fact] public async Task UdfEntries_HaveNoMetadata() { diff --git a/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj b/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj index 9d3a9d3b..67b13b0b 100644 --- a/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj +++ b/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj @@ -168,6 +168,9 @@ PreserveNewest + + PreserveNewest + PreserveNewest diff --git a/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataJolietRockRidge.iso b/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataJolietRockRidge.iso new file mode 100644 index 0000000000000000000000000000000000000000..28861c61721ac82c498591a15548d840d9cd8eb9 GIT binary patch literal 71680 zcmeI*Z%^As9KiA4bs9wsby|fcRPpD^HmTLrkWkU7+au$c;VmSQZ7O&)1rkOy3Q|a? z@?;v)q;2n`uVb&KPtjh&_B%TnjRJBfK(z3^2;bQ`+jpOY{9-50iI5OL009ILKmY** z5I_I{1Q0*~0R#|0009ILKmY**5I_I{1Q0*~0R#|0009ILKmY**5I_I{1Q0*~0R#|0 z009ILKmY**5I_I{1Q0*~0R#|0009ILKmY**5I_I{1Q0*~0R#|0009ILKmY**5I_I{ z1Q0*~0R#|0009ILKmY**5I_I{1Q0*~0R#|0009ILKmY**5I_I{1Q0;L_~l};WW?sA zB#4u;mv}21OHo)JuH2XWEGzV~{?g&xh#2=LqoXpJ-90&RQteY$oRg_+$&`$`!l?Xm zdvC}@I*^niPB-ABt-CpX)|1ai){-RTJ^G5ui$!g5>-Uqn3du}o({d>-e|2b{W z>l62%SD-G3;!9I@r7fHKXgtulhng=)QA)BXkJ6HxvwF{tbhO@qmMhZGbsDlKThfxQ z-dei!a&B+6*|(}R6wHSf3|WYP2)q*l|McVkJJA{%fdB#sAbKwjvz zTur@Ft8<-tN?#B_009ILKmY**5I_I{1Q58n0_o@fGw!f|+#$?riR1s9`*fh^2q1s} z0tg_000IagfB*v5RN#-?-`J1qx6NOFOB8)*j_T{5o9CXR@ScB=iS)7)o@Du+xzqch z{=v)-ri{%h_A-&*g-3c9Ke&DUSzY}|PlmkMY_>aZ z_FD%ZaH*GNGj4izzkieaS+)D~Sw7!$(K$WwV(z%X6_&$FP)OF3Z%Qe(?)?)Z-PS>O zyS>{gbYFH|8;+0M29Az(Eth&(Hshvex8bA?B?J&a009I?>j{{~rh-fB*v5S0I0k zo!lCxkX|{UMzg0MRgVvRrQqwFqv6SPFqi|KZtEooyE-sTLCY`okAwgM2q1s}0v97N zmlL~9{wRFrNzQTigbwr8()R#vkhdn${9+zBwt@fx2q1vKIRt{}*o-B=>-qPv{zK{M z(~_601U9Pq-`glGe;e4ango%*;#IX=k7`NHuT@;(Sy&EY8?V*twJ5R6waC_Maky^X z=h!yh&=c*eHV#(3Y7+W!y%KnFVEtM(@%+S=!`Qd=wWUfJuLM!La$FDma5?n7B&=0! zTw9C$pkQ?iy4U|I>MnJ2%b~6lR<#k~suyk8XSK@OYGAcFzIGzfmbljR_Z3!kZZ+L( z;T%q^iU0x#Ab`Mi5=iv3Z`*8i8+LoYV;lCn{oQt}YrnM7{?<=XyZK|w?i_A*+D#kA uwZhnS>NDBP=r{rhAb`LP68INi%q{W& literal 0 HcmV?d00001 diff --git a/RecursiveExtractor/Extractors/DiscCommon.cs b/RecursiveExtractor/Extractors/DiscCommon.cs index 2661d94a..8662566e 100644 --- a/RecursiveExtractor/Extractors/DiscCommon.cs +++ b/RecursiveExtractor/Extractors/DiscCommon.cs @@ -18,7 +18,8 @@ public static class DiscCommon /// /// Tries to extract file metadata from a DiscUtils file system entry. /// For file systems implementing (such as Ext, Xfs, Btrfs, HfsPlus, - /// and ISO 9660 images via CDReader when RockRidge extensions are present), + /// and ISO 9660 images via CDReader when RockRidge is the active variant; see + /// for images that also carry Joliet), /// returns permissions, UID, and GID. /// For file systems implementing (such as NTFS, FAT, WIM, and ISO 9660), /// returns Windows file attributes. @@ -36,20 +37,7 @@ public static class DiscCommon if (fs is IUnixFileSystem unixFs && SupportsUnixMetadata(fs)) { - try - { - var info = unixFs.GetUnixFileInfo(filePath); - metadata = new FileEntryMetadata - { - Mode = (long)info.Permissions, - Uid = info.UserId, - Gid = info.GroupId - }; - } - catch (Exception e) - { - Logger.Debug(e, "Could not retrieve Unix metadata for {0}", filePath); - } + metadata = ApplyUnixMetadata(unixFs, filePath, metadata); } if (fs is IDosFileSystem dosFs) @@ -88,6 +76,32 @@ public static class DiscCommon return metadata; } + /// + /// Reads the Unix mode, UID and GID for a file and applies them to , + /// creating it when the caller does not have one yet. + /// + /// The opened Unix file system + /// Path of the file within the file system + /// The metadata to populate, or null to create one on demand + /// The populated metadata, or the value passed in when the read failed + private static FileEntryMetadata? ApplyUnixMetadata(IUnixFileSystem fs, string filePath, FileEntryMetadata? metadata) + { + try + { + var info = fs.GetUnixFileInfo(filePath); + metadata ??= new FileEntryMetadata(); + metadata.Mode = (long)info.Permissions; + metadata.Uid = info.UserId; + metadata.Gid = info.GroupId; + } + catch (Exception e) + { + Logger.Debug(e, "Could not retrieve Unix metadata for {0}", filePath); + } + + return metadata; + } + /// /// Determines whether a file system that implements can actually /// return Unix metadata. CDReader implements the interface unconditionally but only exposes @@ -101,6 +115,10 @@ private static bool SupportsUnixMetadata(DiscFileSystem fs) /// Pre-collects metadata for all files while the file system is still open. /// Used by extractors (e.g., ISO) where the file system is disposed before files are processed. /// + /// + /// This does not throw. An entry whose metadata cannot be read is logged and skipped, so a problem + /// reading metadata never causes the archive itself to be reported as unreadable. + /// /// The opened disc file system /// The file entries to collect metadata for /// A dictionary mapping file paths to metadata, or null if the file system does not support metadata @@ -114,15 +132,82 @@ private static bool SupportsUnixMetadata(DiscFileSystem fs) var result = new Dictionary(); foreach (var fi in fileInfos) { - var metadata = TryGetFileMetadata(fs, fi.FullName); - if (metadata != null) + string? fullName = null; + try { - result[fi.FullName] = metadata; + fullName = fi.FullName; + var metadata = TryGetFileMetadata(fs, fullName); + if (metadata != null) + { + result[fullName] = metadata; + } + } + catch (Exception e) + { + Logger.Debug(e, "Could not collect metadata for {0}", fullName ?? "an unnamed entry"); } } return result; } + /// + /// Pre-collects metadata for the files of an ISO 9660 image while the reader is still open. + /// + /// + /// CDReader is opened with Joliet given priority over RockRidge, and DiscUtils only parses + /// SUSP records for the variant it activates. On an image built with both extensions (for example + /// mkisofs -J -R) the Joliet tree wins, and the RockRidge mode, UID and GID would be lost. + /// When the active variant is not RockRidge, a second reader that skips Joliet is opened to recover + /// them. Entries are matched by path; the two directory trees normally agree on names, and an entry + /// that does not match simply keeps whatever the active tree provided. + /// Like , this does not throw. + /// + /// The opened ISO 9660 reader + /// The file entries to collect metadata for + /// The stream the reader was opened over, used for the RockRidge fallback + /// A dictionary mapping file paths to metadata + internal static Dictionary? CollectIsoMetadata(CDReader cd, DiscFileInfo[] fileInfos, Stream isoStream) + { + var metadataByPath = CollectMetadata(cd, fileInfos); + + if (metadataByPath == null || cd.ActiveVariant == Iso9660Variant.RockRidge) + { + return metadataByPath; + } + + try + { + using var rockRidgeReader = new CDReader(isoStream, false); + if (rockRidgeReader.ActiveVariant != Iso9660Variant.RockRidge) + { + return metadataByPath; + } + + foreach (var fi in rockRidgeReader.Root.GetFiles("*.*", SearchOption.AllDirectories)) + { + string? fullName = null; + try + { + fullName = fi.FullName; + if (metadataByPath.TryGetValue(fullName, out var metadata)) + { + ApplyUnixMetadata(rockRidgeReader, fullName, metadata); + } + } + catch (Exception e) + { + Logger.Debug(e, "Could not collect RockRidge metadata for {0}", fullName ?? "an unnamed entry"); + } + } + } + catch (Exception e) + { + Logger.Debug(e, "Could not read RockRidge metadata from an ISO with active variant {0}", cd.ActiveVariant); + } + + return metadataByPath; + } + /// /// Dump the FileEntries from a Logical Volume asynchronously /// diff --git a/RecursiveExtractor/Extractors/IsoExtractor.cs b/RecursiveExtractor/Extractors/IsoExtractor.cs index da94c098..f3455df7 100644 --- a/RecursiveExtractor/Extractors/IsoExtractor.cs +++ b/RecursiveExtractor/Extractors/IsoExtractor.cs @@ -37,7 +37,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra { using var cd = new CDReader(fileEntry.Content, true); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); - metadataByPath = DiscCommon.CollectMetadata(cd, entries); + metadataByPath = DiscCommon.CollectIsoMetadata(cd, entries, fileEntry.Content); } catch (Exception e) { @@ -104,7 +104,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti { using var cd = new CDReader(fileEntry.Content, true); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); - metadataByPath = DiscCommon.CollectMetadata(cd, entries); + metadataByPath = DiscCommon.CollectIsoMetadata(cd, entries, fileEntry.Content); } catch(Exception e) {