diff --git a/libs/importer/src/importers/keepass2-xml-importer.spec.ts b/libs/importer/src/importers/keepass2-xml-importer.spec.ts index 8fbb021883c..c1c0947936b 100644 --- a/libs/importer/src/importers/keepass2-xml-importer.spec.ts +++ b/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"); + }); + }); }); diff --git a/libs/importer/src/importers/keepass2-xml-importer.ts b/libs/importer/src/importers/keepass2-xml-importer.ts index 0af7a6f829c..429ab2aa1b7 100644 --- a/libs/importer/src/importers/keepass2-xml-importer.ts +++ b/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); } }); diff --git a/libs/importer/src/importers/spec-data/keepass2-xml/keepass2-xml-importer-testdata.ts b/libs/importer/src/importers/spec-data/keepass2-xml/keepass2-xml-importer-testdata.ts index e06ca2cf655..9e1599b7078 100644 --- a/libs/importer/src/importers/spec-data/keepass2-xml/keepass2-xml-importer-testdata.ts +++ b/libs/importer/src/importers/spec-data/keepass2-xml/keepass2-xml-importer-testdata.ts @@ -354,6 +354,57 @@ line2 `; +export const TestDataWithProtectedFields = ` + + + + KvS57lVwl13AfGFLwkvq4Q== + Root + + fAa543oYlgnJKkhKag5HLw== + + Title + Test Entry + + + UserName + testuser + + + Password + testpass + + + URL + https://example.com + + + Notes + Regular notes + + + SAFE UN-LOCKING instructions + Secret instructions here + + + CustomField + Custom value + + + LongProtectedField + 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. + + + MultilineProtectedField + Line 1 +Line 2 +Line 3 + + + + +`; + export const TestData2 = ` KeePass