fix(importer): preserve protected KeePass custom fields as hidden fields (#18136)

Protected fields (ProtectInMemory="True") were being appended to notes
when they exceeded 200 characters or contained newlines, instead of
being imported as hidden custom fields.

Now protected fields are always imported as hidden fields regardless
of their length or content, preserving their protected status.

Fixes #16897

Signed-off-by: majiayu000 <1835304752@qq.com>
Co-authored-by: John Harrington <84741727+harr1424@users.noreply.github.com>

libs/importer/src/importers/keepass2-xml-importer.spec.ts

@@ -1,3 +1,4 @@
+import { FieldType } from "@bitwarden/common/vault/enums";
 import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
 
 import { KeePass2XmlImporter } from "./keepass2-xml-importer";
@@ -5,6 +6,7 @@ import {
   TestData,
   TestData1,
   TestData2,
+  TestDataWithProtectedFields,
 } from "./spec-data/keepass2-xml/keepass2-xml-importer-testdata";
 
 describe("KeePass2 Xml Importer", () => {
@@ -43,4 +45,73 @@ describe("KeePass2 Xml Importer", () => {
     const result = await importer.parse(TestData2);
     expect(result.success).toBe(false);
   });
+
+  describe("protected fields handling", () => {
+    it("should import protected custom fields as hidden fields", async () => {
+      const importer = new KeePass2XmlImporter();
+      const result = await importer.parse(TestDataWithProtectedFields);
+
+      expect(result.success).toBe(true);
+      expect(result.ciphers.length).toBe(1);
+
+      const cipher = result.ciphers[0];
+      expect(cipher.name).toBe("Test Entry");
+      expect(cipher.login.username).toBe("testuser");
+      expect(cipher.login.password).toBe("testpass");
+      expect(cipher.notes).toContain("Regular notes");
+
+      // Check that protected custom field is imported as hidden field
+      const protectedField = cipher.fields.find((f) => f.name === "SAFE UN-LOCKING instructions");
+      expect(protectedField).toBeDefined();
+      expect(protectedField?.value).toBe("Secret instructions here");
+      expect(protectedField?.type).toBe(FieldType.Hidden);
+
+      // Check that regular custom field is imported as text field
+      const regularField = cipher.fields.find((f) => f.name === "CustomField");
+      expect(regularField).toBeDefined();
+      expect(regularField?.value).toBe("Custom value");
+      expect(regularField?.type).toBe(FieldType.Text);
+    });
+
+    it("should import long protected fields as hidden fields (not appended to notes)", async () => {
+      const importer = new KeePass2XmlImporter();
+      const result = await importer.parse(TestDataWithProtectedFields);
+
+      const cipher = result.ciphers[0];
+
+      // Long protected field should be imported as hidden field
+      const longField = cipher.fields.find((f) => f.name === "LongProtectedField");
+      expect(longField).toBeDefined();
+      expect(longField?.type).toBe(FieldType.Hidden);
+      expect(longField?.value).toContain("This is a very long protected field");
+
+      // Should not be appended to notes
+      expect(cipher.notes).not.toContain("LongProtectedField");
+    });
+
+    it("should import multiline protected fields as hidden fields (not appended to notes)", async () => {
+      const importer = new KeePass2XmlImporter();
+      const result = await importer.parse(TestDataWithProtectedFields);
+
+      const cipher = result.ciphers[0];
+
+      // Multiline protected field should be imported as hidden field
+      const multilineField = cipher.fields.find((f) => f.name === "MultilineProtectedField");
+      expect(multilineField).toBeDefined();
+      expect(multilineField?.type).toBe(FieldType.Hidden);
+      expect(multilineField?.value).toContain("Line 1");
+
+      // Should not be appended to notes
+      expect(cipher.notes).not.toContain("MultilineProtectedField");
+    });
+
+    it("should not append protected custom fields to notes", async () => {
+      const importer = new KeePass2XmlImporter();
+      const result = await importer.parse(TestDataWithProtectedFields);
+
+      const cipher = result.ciphers[0];
+      expect(cipher.notes).not.toContain("SAFE UN-LOCKING instructions");
+      expect(cipher.notes).not.toContain("Secret instructions here");
+    });
+  });
 });

libs/importer/src/importers/keepass2-xml-importer.ts

@@ -1,6 +1,7 @@
 // FIXME: Update this file to be type safe and remove this and next line
 // @ts-strict-ignore
 import { FieldType } from "@bitwarden/common/vault/enums";
+import { FieldView } from "@bitwarden/common/vault/models/view/field.view";
 import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
 
 import { ImportResult } from "../models/import-result";
@@ -92,16 +93,26 @@ export class KeePass2XmlImporter extends BaseImporter implements Importer {
         } else if (key === "Notes") {
           cipher.notes += value + "\n";
         } else {
-          let type = FieldType.Text;
           const attrs = valueEl.attributes as any;
-          if (
+          const isProtected =
             attrs.length > 0 &&
             attrs.ProtectInMemory != null &&
-            attrs.ProtectInMemory.value === "True"
-          ) {
-            type = FieldType.Hidden;
+            attrs.ProtectInMemory.value === "True";
+
+          if (isProtected) {
+            // Protected fields should always be imported as hidden fields,
+            // regardless of length or newlines (fixes #16897)
+            if (cipher.fields == null) {
+              cipher.fields = [];
+            }
+            const field = new FieldView();
+            field.type = FieldType.Hidden;
+            field.name = key;
+            field.value = value;
+            cipher.fields.push(field);
+          } else {
+            this.processKvp(cipher, key, value, FieldType.Text);
           }
-          this.processKvp(cipher, key, value, type);
         }
       });
 

libs/importer/src/importers/spec-data/keepass2-xml/keepass2-xml-importer-testdata.ts

@@ -354,6 +354,57 @@ line2</Value>
 		</Group>
 		<DeletedObjects />
 </KeePassFile>`;
+export const TestDataWithProtectedFields = `<?xml version="1.0" encoding="utf-8" standalone="yes"?>
+<KeePassFile>
+	<Root>
+		<Group>
+			<UUID>KvS57lVwl13AfGFLwkvq4Q==</UUID>
+			<Name>Root</Name>
+			<Entry>
+				<UUID>fAa543oYlgnJKkhKag5HLw==</UUID>
+				<String>
+					<Key>Title</Key>
+					<Value>Test Entry</Value>
+				</String>
+				<String>
+					<Key>UserName</Key>
+					<Value>testuser</Value>
+				</String>
+				<String>
+					<Key>Password</Key>
+					<Value ProtectInMemory="True">testpass</Value>
+				</String>
+				<String>
+					<Key>URL</Key>
+					<Value>https://example.com</Value>
+				</String>
+				<String>
+					<Key>Notes</Key>
+					<Value>Regular notes</Value>
+				</String>
+				<String>
+					<Key>SAFE UN-LOCKING instructions</Key>
+					<Value ProtectInMemory="True">Secret instructions here</Value>
+				</String>
+				<String>
+					<Key>CustomField</Key>
+					<Value>Custom value</Value>
+				</String>
+				<String>
+					<Key>LongProtectedField</Key>
+					<Value ProtectInMemory="True">This is a very long protected field value that exceeds 200 characters. It contains sensitive information that should be imported as a hidden field and not appended to the notes section. This text is long enough to trigger the old behavior.</Value>
+				</String>
+				<String>
+					<Key>MultilineProtectedField</Key>
+					<Value ProtectInMemory="True">Line 1
+Line 2
+Line 3</Value>
+				</String>
+			</Entry>
+		</Group>
+	</Root>
+</KeePassFile>`;
+
 export const TestData2 = `<?xml version="1.0" encoding="utf-8" standalone="yes"?>
 	<Meta>
 		<Generator>KeePass</Generator>