fix(keepass-cred-mgr): harden REPL quoting, output parsing, and notes escaping

Research into keepassxc-cli source revealed three additional corruption vectors:

1. REPL quoting model was wrong: the CLI uses Utils::splitCommandString
   (backslash escapes any character), not QProcess::splitCommand. Updated
   comments to document the actual behavior.

2. _parse_show_output misinterpreted notes containing "Password: ..." as
   field boundaries. Since Notes is always the last standard field in
   keepassxc-cli output, only Tags: terminates notes mode now.

3. keepassxc-cli's Add.cpp/Edit.cpp replace literal \\n in --notes values
   with actual newlines before storing. Added _escape_notes_for_cli() to
   prevent silent data corruption when notes contain backslash-n text.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: chrisdpurcell

plugins/keepass-cred-mgr/CHANGELOG.md

@@ -10,6 +10,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - Newlines in REPL arguments (notes, urls) corrupt the keepassxc-cli command stream, causing garbled entries and data loss. `_repl_quote()` now sanitizes `\n`/`\r` to spaces before quoting.
 - `deactivate_entry` constructed notes with an embedded newline (`\n[DEACTIVATED: ...]`) that split the REPL command across two lines. Uses ` | ` separator instead.
 - `run_cli` stdin_lines (passwords) now reject embedded newlines with a clear error instead of silently corrupting the stream.
+- Corrected REPL quoting model: the CLI uses `Utils::splitCommandString` (backslash escapes any character), not `QProcess::splitCommand`. Updated comments and docs.
+- `_parse_show_output` no longer misinterprets notes containing `Password: ...` or `URL: ...` text as field boundaries. Notes is the last standard field; only `Tags:` terminates it.
+- Notes passed via `--notes` are now escaped to prevent keepassxc-cli from silently converting literal `\n` text into newlines (the CLI replaces `\\n` → newline before storing).
 
 ## [0.5.1] - 2026-03-15
 

plugins/keepass-cred-mgr/server/tools/read.py

@@ -23,15 +23,20 @@
 type SearchResult = dict[str, str | None]
 
 
-# Known field prefixes that terminate a multi-line notes block.
-_KNOWN_FIELDS = {"username", "password", "url", "notes", "title", "tags"}
+# Fields that appear BEFORE notes in keepassxc-cli show output.
+# Once we enter notes mode, only "tags" (which appears after notes) terminates it.
+# This prevents notes containing "Password: ..." from being misinterpreted as fields.
+_PRE_NOTES_FIELDS = {"username", "password", "url", "title"}
 
 
 def _parse_show_output(stdout: str) -> EntryFields:
     """Parse keepassxc-cli show output into a string field dict.
 
-    Notes can span multiple lines; continuation lines have no 'Key: ' prefix.
-    Continuation stops when a line starts with a known field key followed by ': '.
+    keepassxc-cli show outputs fields in order: Title, UserName, Password,
+    URL, Notes, then Tags. Notes can span multiple lines with embedded
+    newlines. Once we enter notes mode, only a Tags line terminates it;
+    lines that look like "Password: foo" inside notes are continuation lines,
+    not field boundaries.
     """
     fields: dict[str, str] = {}
     in_notes = False
@@ -41,22 +46,24 @@ def _parse_show_output(stdout: str) -> EntryFields:
         if ": " in line:
             key, _, value = line.partition(": ")
             key_lower = key.strip().lower()
-            if key_lower in _KNOWN_FIELDS:
-                if in_notes:
+
+            # Inside notes, only "tags" can terminate — everything else is content
+            if in_notes:
+                if key_lower == "tags":
                     fields["notes"] = "\n".join(notes_lines)
                     in_notes = False
-                if key_lower == "notes":
-                    notes_lines = [value.strip()]
-                    in_notes = True
-                elif key_lower == "username":
-                    fields["username"] = value.strip()
-                elif key_lower == "password":
-                    fields["password"] = value.strip()
-                elif key_lower == "url":
-                    fields["url"] = value.strip()
-                elif key_lower == "title":
-                    fields["title"] = value.strip()
+                    # Tags value is captured by _parse_tags separately; skip
+                    continue
+                notes_lines.append(line)
                 continue
+
+            if key_lower == "notes":
+                notes_lines = [value.strip()]
+                in_notes = True
+            elif key_lower in _PRE_NOTES_FIELDS:
+                fields[key_lower] = value.strip()
+            continue
+
         if in_notes:
             notes_lines.append(line)
 

plugins/keepass-cred-mgr/server/tools/write.py

@@ -38,6 +38,18 @@
 log: structlog.stdlib.BoundLogger = structlog.get_logger("keepass-cred-mgr.tools.write")
 
 
+def _escape_notes_for_cli(notes: str) -> str:
+    """Escape a notes string for --notes argument to keepassxc-cli.
+
+    The CLI's Add.cpp and Edit.cpp replace literal '\\n' with actual newlines
+    before storing (entry->setNotes(notes.replace("\\\\n", "\\n"))). If the
+    user's notes contain the two-character sequence '\\n', the CLI silently
+    converts it to a newline. Escape it to '\\\\n' so the CLI round-trips
+    to '\\n' as intended.
+    """
+    return notes.replace("\\n", "\\\\n")
+
+
 @contextmanager
 def _write_lock(vault: Vault) -> Generator[FileLock]:
     """Acquire and release a file lock around database writes."""
@@ -104,7 +116,7 @@ async def create_entry(
         if url is not None:
             cmd.extend(["--url", url])
         if notes is not None:
-            cmd.extend(["--notes", notes])
+            cmd.extend(["--notes", _escape_notes_for_cli(notes)])
         if password:
             # keepassxc-cli add has no --password flag; -p prompts stdin.
             # Write the password to stdin upfront so run_cli() doesn't deadlock.
@@ -152,7 +164,7 @@ async def deactivate_entry(
         # Update notes using new path — non-critical, log and continue on failure
         new_path = vault.entry_path(new_title, group)
         try:
-            await vault.run_cli("edit", "--notes", new_notes, db, new_path)
+            await vault.run_cli("edit", "--notes", _escape_notes_for_cli(new_notes), db, new_path)
         except KeePassCLIError:
             log.warning("deactivate_notes_update_failed", title=title, group=group)
 

plugins/keepass-cred-mgr/server/vault.py

@@ -38,18 +38,24 @@
 _REPL_PROMPT = b"\n> "
 _REPL_TIMEOUT = 30
 
-# Characters that require double-quoting in the keepassxc-cli REPL.
-# The REPL uses Qt's QProcess::splitCommand (double-quote style),
-# NOT POSIX quoting — single quotes are NOT treated as delimiters.
+# The keepassxc-cli REPL uses Utils::splitCommandString (src/cli/Utils.cpp),
+# NOT Qt's QProcess::splitCommand. Key differences from POSIX:
+#   - backslash escapes ANY next character (\x → x), not just inside quotes
+#   - double quotes toggle quoting mode (only at start or after whitespace)
+#   - single quotes have NO special meaning
+# Always double-quote arguments containing whitespace, quotes, or backslashes
+# to prevent the parser from consuming backslashes as escape characters.
 _REPL_NEEDS_QUOTING = frozenset(' \t"\\')
 
 
 def _repl_quote(arg: str) -> str:
-    """Quote one argument for keepassxc-cli REPL (double-quote style).
+    """Quote one argument for the keepassxc-cli REPL.
 
-    Qt's argument parser recognises double-quoted strings but not single-quoted
-    strings.  shlex.quote() prefers single quotes, which silently pass through
-    the REPL as literal characters — breaking any argument that contains spaces.
+    The REPL's parser (Utils::splitCommandString) treats backslash as an
+    escape character for ANY following character. An unquoted backslash in
+    a password or notes value silently drops the backslash and takes the
+    next character literally. Double-quoting prevents this: inside quotes,
+    only \\\\ and \\" are recognised as escapes.
 
     Newlines and carriage returns are replaced with spaces before quoting.
     The REPL is line-based: a literal newline inside an argument splits the

plugins/keepass-cred-mgr/tests/unit/test_tools.py

@@ -89,10 +89,31 @@ def test_multiline_notes_followed_by_field(self):
     def test_single_line_notes_unchanged(self):
         """Existing single-line notes behavior preserved."""
         from server.tools.read import _parse_show_output
-        output = "Title: Entry\nNotes: Single line\nUserName: admin\n"
+        # Real keepassxc-cli output order: Title, UserName, Password, URL, Notes
+        output = "Title: Entry\nUserName: admin\nNotes: Single line\n"
         result = _parse_show_output(output)
         assert result["notes"] == "Single line"
 
+    def test_notes_containing_field_like_text(self):
+        """Notes with 'Password: ...' inside are NOT misinterpreted as fields."""
+        from server.tools.read import _parse_show_output
+        output = (
+            "Title: Entry\n"
+            "UserName: admin\n"
+            "Password: secret\n"
+            "URL: https://example.com\n"
+            "Notes: See credentials below\n"
+            "Password: old-value-for-reference\n"
+            "URL: https://internal.example.com\n"
+        )
+        result = _parse_show_output(output)
+        assert result["password"] == "secret"
+        assert result["notes"] == (
+            "See credentials below\n"
+            "Password: old-value-for-reference\n"
+            "URL: https://internal.example.com"
+        )
+
 
 # ---------------------------------------------------------------------------
 # _parse_tags