From f03fe7a53f92efbf7bf013df36e9e464cda3d46e Mon Sep 17 00:00:00 2001 From: Leendert de Borst Date: Mon, 11 May 2026 14:44:56 +0200 Subject: [PATCH] Fix empty attachments after import due to folder names (#1997) --- .../Utilities/ImportExportTests.cs | 90 +++++++++++++++++++ .../Importers/BaseArchiveImporter.cs | 58 ++++++------ 2 files changed, 120 insertions(+), 28 deletions(-) diff --git a/apps/server/Tests/AliasVault.UnitTests/Utilities/ImportExportTests.cs b/apps/server/Tests/AliasVault.UnitTests/Utilities/ImportExportTests.cs index 9c0541a5a..1b385609d 100644 --- a/apps/server/Tests/AliasVault.UnitTests/Utilities/ImportExportTests.cs +++ b/apps/server/Tests/AliasVault.UnitTests/Utilities/ImportExportTests.cs @@ -2176,6 +2176,96 @@ public class ImportExportTests }); } + /// + /// Regression test: a Bitwarden ZIP that includes a directory entry for the + /// item's attachment folder (e.g. "attachments/<id>/") must not produce a + /// phantom attachment with an empty filename. Many ZIP creators emit such + /// directory entries by default, and they previously matched the StartsWith + /// filter and ended up as empty-blob attachments rendered as "(unavailable)". + /// + /// Async task. + [Test] + public async Task ImportBitwardenZipWithDirectoryEntriesIgnoresPhantomAttachments() + { + // Arrange - build a Bitwarden-style ZIP in memory containing both a + // directory entry and a single file entry under the item's attachment folder. + const string ItemId = "item-with-attachment-uuid"; + const string FileName = "dataset_Backup_keys.json"; + var fileBytes = System.Text.Encoding.UTF8.GetBytes("{\"keys\":\"value\"}"); + + const string DataJson = """ + { + "encrypted": false, + "folders": [], + "items": [ + { + "id": "item-with-attachment-uuid", + "organizationId": null, + "folderId": null, + "type": 1, + "reprompt": 0, + "name": "Login with Attachment", + "notes": "This item has an attachment", + "favorite": false, + "revisionDate": "2023-08-15T16:30:00.000Z", + "fields": [], + "login": { + "uris": [{"match": null, "uri": "https://secure.example.com"}], + "username": "admin", + "password": "AdminPass456!", + "totp": null + }, + "collectionIds": null + } + ] + } + """; + + byte[] zipBytes; + using (var ms = new MemoryStream()) + { + using (var archive = new ZipArchive(ms, ZipArchiveMode.Create, leaveOpen: true)) + { + var dataEntry = archive.CreateEntry("data.json"); + using (var writer = new StreamWriter(dataEntry.Open())) + { + await writer.WriteAsync(DataJson); + } + + // Directory entries (zero-byte, trailing slash) — emitted by many ZIP tools + // and the source of the original "(unavailable)" duplicate. + archive.CreateEntry("attachments/"); + archive.CreateEntry($"attachments/{ItemId}/"); + + var fileEntry = archive.CreateEntry($"attachments/{ItemId}/{FileName}"); + using (var fileStream = fileEntry.Open()) + { + await fileStream.WriteAsync(fileBytes); + } + } + + zipBytes = ms.ToArray(); + } + + // Act + var importer = new BitwardenZipImporter(); + var importedCredentials = await importer.ImportFromArchiveAsync(zipBytes); + + // Assert - exactly one attachment with the real filename and content, + // and no phantom empty-filename entry from the directory entries. + Assert.That(importedCredentials, Has.Count.EqualTo(1)); + var credential = importedCredentials[0]; + Assert.That(credential.Attachments, Is.Not.Null); + Assert.That(credential.Attachments, Has.Count.EqualTo(1)); + Assert.Multiple(() => + { + Assert.That(credential.Attachments![0].Filename, Is.EqualTo(FileName)); + Assert.That(credential.Attachments[0].Blob, Is.EqualTo(fileBytes)); + Assert.That(credential.Attachments.Any(a => string.IsNullOrEmpty(a.Filename)), Is.False); + Assert.That(credential.Attachments.Any(a => a.Blob == null || a.Blob.Length == 0), Is.False); + }); + } + /// /// Test case for importing credentials from 1Password .1pux export format. /// diff --git a/apps/server/Utilities/AliasVault.ImportExport/Importers/BaseArchiveImporter.cs b/apps/server/Utilities/AliasVault.ImportExport/Importers/BaseArchiveImporter.cs index 6325366b4..5d226a166 100644 --- a/apps/server/Utilities/AliasVault.ImportExport/Importers/BaseArchiveImporter.cs +++ b/apps/server/Utilities/AliasVault.ImportExport/Importers/BaseArchiveImporter.cs @@ -57,26 +57,7 @@ public abstract class BaseArchiveImporter /// Dictionary mapping attachment paths to file data. protected virtual Dictionary ExtractAttachments(ZipArchive archive) { - var map = new Dictionary(); - var attachmentPathPattern = GetAttachmentPathPattern(); - - if (string.IsNullOrEmpty(attachmentPathPattern)) - { - return map; - } - - foreach (var entry in archive.Entries) - { - if (entry.FullName.StartsWith(attachmentPathPattern, StringComparison.OrdinalIgnoreCase)) - { - using var stream = entry.Open(); - using var ms = new MemoryStream(); - stream.CopyTo(ms); - map[entry.FullName] = ms.ToArray(); - } - } - - return map; + return ExtractFilesByPrefix(archive, GetAttachmentPathPattern()); } /// @@ -86,23 +67,44 @@ public abstract class BaseArchiveImporter /// Dictionary mapping logo paths to file data. protected virtual Dictionary ExtractLogos(ZipArchive archive) { - var map = new Dictionary(); - var logoPathPattern = GetLogoPathPattern(); + return ExtractFilesByPrefix(archive, GetLogoPathPattern()); + } - if (string.IsNullOrEmpty(logoPathPattern)) + /// + /// Extracts all file entries (skipping directory entries) from the archive whose + /// path starts with the given prefix. + /// + /// The ZIP archive. + /// The path prefix to match (e.g. "attachments/"). When null or empty, an empty map is returned. + /// Dictionary mapping entry paths to file data. + private static Dictionary ExtractFilesByPrefix(ZipArchive archive, string? pathPrefix) + { + var map = new Dictionary(); + + if (string.IsNullOrEmpty(pathPrefix)) { return map; } foreach (var entry in archive.Entries) { - if (entry.FullName.StartsWith(logoPathPattern, StringComparison.OrdinalIgnoreCase)) + if (!entry.FullName.StartsWith(pathPrefix, StringComparison.OrdinalIgnoreCase)) { - using var stream = entry.Open(); - using var ms = new MemoryStream(); - stream.CopyTo(ms); - map[entry.FullName] = ms.ToArray(); + continue; } + + // Skip directory entries: ZIP archives commonly include zero-byte entries + // for directories (paths ending in '/'). For those, ZipArchiveEntry.Name is empty. + // Including them here produced phantom attachments with empty filenames. + if (string.IsNullOrEmpty(entry.Name)) + { + continue; + } + + using var stream = entry.Open(); + using var ms = new MemoryStream(); + stream.CopyTo(ms); + map[entry.FullName] = ms.ToArray(); } return map;