test(keepass-cred-mgr): add security, integration, and unit tests (204 → 241)

Security tests: audit redaction, CLI notes injection, file shredding
verification, XML special char escaping. Integration tests: get_entry,
search, get/add attachment, import with password-auth merge adapter.
Fixed missing pytestmark=pytest.mark.unit on 4 test files that caused
pytest -m unit to silently drop 74 tests. Coverage: 80% → 99.8%.

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

Author: chrisdpurcell

plugins/keepass-cred-mgr/CHANGELOG.md

@@ -4,6 +4,18 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
+## [0.5.3] - 2026-03-18
+
+### Added
+- Security tests: audit log redaction (`_sanitize_extra`), CLI notes injection prevention (`_escape_notes_for_cli`), temp file shredding content verification, XML special character escaping in bulk import
+- Integration tests: `get_entry`, `search_entries`, `get_attachment`, `add_attachment`, and `import_entries` against real keepassxc-cli
+- Unit tests: REPL newline sanitization, stdin_lines embedded newline guard, diagnostics edge cases, `_configure_logging`, `list_entries` error handler
+- Testing section in README with run commands
+- `docs/testing/TEST_STATUS.json` for persistent test posture tracking
+
+### Fixed
+- Four test files (`test_audit.py`, `test_config.py`, `test_main.py`, `test_yubikey.py`) were missing `pytestmark = pytest.mark.unit`, causing `pytest -m unit` to silently drop 74 tests and report 80% coverage instead of 99%
+
 ## [0.5.2] - 2026-03-18
 
 ### Fixed

plugins/keepass-cred-mgr/README.md

@@ -188,6 +188,24 @@ audit_log_path: ~/.local/share/keepass-cred-mgr/audit.jsonl
 
 - **Two-database architecture**: A primary database (YubiKey-only, used by the MCP server) and a backup database (password-only, never used by the server) kept in sync via KeePassXC's merge function. If the YubiKey is lost, the backup provides recovery without compromising the primary's auth model.
 
+## Testing
+
+241 tests across unit, integration, and security categories at 99% line coverage.
+
+```bash
+# Unit tests (no external dependencies)
+cd plugins/keepass-cred-mgr
+.venv/bin/python -m pytest tests/unit/ -m unit
+
+# Integration tests (requires keepassxc-cli + test.kdbx fixture)
+.venv/bin/python -m pytest tests/integration/ -m integration
+
+# Full suite with coverage
+.venv/bin/python -m pytest tests/ --cov=server --cov-report=term-missing
+```
+
+Security-specific tests cover audit log redaction of sensitive keys, CLI notes injection prevention, temp file shredding verification, XML special character escaping, and tag-based access control enforcement (AI RESTRICTED and READ ONLY).
+
 ## Planned Features
 
 - **Credential provisioning agent**: A purpose-built subagent for setting up new project environments; audits existing vault entries, generates missing credentials, and stores them with consistent naming.

plugins/keepass-cred-mgr/docs/testing/TEST_STATUS.json

@@ -0,0 +1,80 @@
+{
+  "project": "keepass-cred-mgr",
+  "last_analysis": "2026-03-18T20:10:00Z",
+  "profile": "generic-python",
+  "coverage": {
+    "total_pct": 99,
+    "tool": "pytest-cov",
+    "by_file": {
+      "server/audit.py": 100,
+      "server/config.py": 100,
+      "server/diagnostics.py": 100,
+      "server/main.py": 99,
+      "server/tools/read.py": 100,
+      "server/tools/write.py": 100,
+      "server/vault.py": 100,
+      "server/yubikey.py": 100
+    }
+  },
+  "categories": {
+    "unit": {
+      "applicable": true,
+      "test_count": 230,
+      "status": "pass",
+      "files": [
+        "tests/unit/test_audit.py",
+        "tests/unit/test_config.py",
+        "tests/unit/test_diagnostics.py",
+        "tests/unit/test_main.py",
+        "tests/unit/test_tools.py",
+        "tests/unit/test_vault.py",
+        "tests/unit/test_yubikey.py"
+      ]
+    },
+    "integration": {
+      "applicable": true,
+      "test_count": 11,
+      "status": "pass",
+      "notes": "Requires keepassxc-cli + test.kdbx fixture; skips cleanly when unavailable",
+      "files": [
+        "tests/integration/test_integration.py"
+      ]
+    },
+    "e2e": { "applicable": false },
+    "contract": { "applicable": false },
+    "security": {
+      "applicable": true,
+      "test_count": 27,
+      "status": "pass",
+      "notes": "Tag enforcement (AI RESTRICTED, READ ONLY), audit redaction (_sanitize_extra), notes CLI injection prevention, file shredding verification, XML special character escaping",
+      "files": [
+        "tests/unit/test_audit.py (TestSanitizeExtra)",
+        "tests/unit/test_tools.py (TestAiRestrictedEnforcement, TestReadOnlyEnforcement, TestEscapeNotesForCli, TestShredFileContentOverwrite, TestBuildImportXmlEscaping)"
+      ]
+    },
+    "ui": { "applicable": false }
+  },
+  "known_gaps": [
+    {
+      "file": "server/main.py",
+      "lines": "312, 316",
+      "category": "unit",
+      "priority": "low",
+      "description": "main() and __main__ guard — entry point boilerplate",
+      "reason_deferred": "Not unit-testable; exercised by running the server"
+    }
+  ],
+  "source_bugs_fixed": [],
+  "history": [
+    {
+      "date": "2026-03-18",
+      "action": "convergence_loop",
+      "summary": "Gap analysis + convergence: added 7 tests (3 vault newline guards, 2 diagnostics edge cases, 2 stdin_lines guards). Fixed missing pytestmark=pytest.mark.unit on 4 test files. Coverage: 80% → 99% (with -m unit filter). 206 tests, all passing. 3 low-priority gaps deferred (main.py entry-point boilerplate)."
+    },
+    {
+      "date": "2026-03-18",
+      "action": "convergence_loop",
+      "summary": "Full multi-category gap analysis. Added 35 tests: 9 security (audit redaction, notes escaping, shred verification, XML escaping), 4 unit (configure_logging, list_entries error, main callable), 6 integration (get_entry, search, get/add attachment, import with password-auth merge adapter), 2 diagnostics. Coverage: 99% → 99.8%. Total: 241 tests (230 unit + 11 integration), all passing. 1 gap deferred (main.py entry point)."
+    }
+  ]
+}

plugins/keepass-cred-mgr/tests/integration/test_integration.py

@@ -142,3 +142,136 @@ async def test_any_group_accessible(self, integration_setup):
         result = await list_entries(vault, group="Servers")
         titles = [e["title"] for e in result]
         assert any("Server" in t or "DB" in t for t in titles)
+
+
+class TestIntegrationGetEntry:
+    async def test_get_entry_returns_password(self, integration_setup):
+        """get_entry returns full record including password via real keepassxc-cli."""
+        from server.tools.read import get_entry
+
+        vault, audit, config, db = integration_setup
+        # test.kdbx has pre-seeded entries in Servers group
+        groups = await vault.run_cli("ls", str(db))
+        group_names = [line.rstrip("/") for line in groups.strip().splitlines() if line.endswith("/")]
+        assert len(group_names) > 0
+
+        # List entries in first group and get one
+        from server.tools.read import list_entries
+        entries = await list_entries(vault, group=group_names[0])
+        if entries:
+            entry = await get_entry(vault, audit, title=entries[0]["title"], group=group_names[0])
+            assert "title" in entry
+            assert "password" in entry
+            assert "username" in entry
+
+
+class TestIntegrationSearch:
+    async def test_search_entries_finds_existing(self, integration_setup):
+        """search_entries finds entries by keyword via real keepassxc-cli."""
+        from server.tools.read import search_entries
+
+        vault, audit, config, db = integration_setup
+        # "Server" should match entries in the Servers group
+        results = await search_entries(vault, query="Server")
+        assert len(results) > 0
+        assert any("Server" in r.get("title", "") or "server" in r.get("title", "").lower()
+                    for r in results)
+
+
+class TestIntegrationGetAttachment:
+    async def test_get_attachment_binary(self, integration_setup):
+        """get_attachment returns binary data for an existing attachment."""
+        from server.tools.read import get_attachment, list_entries
+        from server.vault import KeePassCLIError
+
+        vault, audit, config, db = integration_setup
+        # Create an entry with an attachment first, then retrieve it
+        from server.tools.write import create_entry, add_attachment
+
+        await create_entry(vault, audit, title="Attach Test", group="SSH Keys", username="u")
+        test_content = b"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5 test@host"
+        await add_attachment(
+            vault, audit, title="Attach Test", attachment_name="test_key.pub",
+            content=test_content, group="SSH Keys",
+        )
+        result = await get_attachment(
+            vault, audit, title="Attach Test",
+            attachment_name="test_key.pub", group="SSH Keys",
+        )
+        assert result == test_content
+
+
+class TestIntegrationAddAttachment:
+    async def test_add_attachment_roundtrip(self, integration_setup):
+        """add_attachment stores a file that can be retrieved."""
+        from server.tools.write import add_attachment, create_entry
+        from server.tools.read import get_attachment
+
+        vault, audit, config, db = integration_setup
+        await create_entry(vault, audit, title="File Store", group="API Keys", username="u")
+        content = b"api_key=abc123\nsecret=xyz789"
+        await add_attachment(
+            vault, audit, title="File Store", attachment_name="creds.env",
+            content=content, group="API Keys",
+        )
+        result = await get_attachment(
+            vault, audit, title="File Store",
+            attachment_name="creds.env", group="API Keys",
+        )
+        assert result == content
+
+
+class TestIntegrationImport:
+    async def test_import_creates_entries(self, integration_setup):
+        """import_entries creates entries that appear in list_entries after re-unlock.
+
+        import_entries runs keepassxc-cli merge as a direct subprocess with
+        --yubikey auth. This test patches subprocess.run to adapt the merge
+        command for password auth (matching the PasswordVault test database).
+        """
+        import subprocess as _subprocess
+        from unittest.mock import patch as _patch
+
+        from server.tools.read import list_entries
+        from server.tools.write import import_entries
+
+        vault, audit, config, db = integration_setup
+        db_password = "testpassword"
+        real_run = _subprocess.run
+
+        def merge_with_password(cmd, *args, **kwargs):
+            """Adapt keepassxc-cli merge from YubiKey to password auth."""
+            if isinstance(cmd, list) and len(cmd) > 1 and cmd[1] == "merge":
+                new_cmd = []
+                skip_next = False
+                for c in cmd:
+                    if skip_next:
+                        skip_next = False
+                        continue
+                    if c == "--yubikey":
+                        skip_next = True
+                        continue
+                    if c == "--no-password":
+                        continue
+                    new_cmd.append(c)
+                existing_input = kwargs.get("input", "")
+                kwargs["input"] = db_password + "\n" + existing_input
+                return real_run(new_cmd, *args, **kwargs)
+            return real_run(cmd, *args, **kwargs)
+
+        with _patch("subprocess.run", side_effect=merge_with_password):
+            result = await import_entries(vault, audit, entries=[
+                {"group": "Servers", "title": "Imported-1", "username": "import_user"},
+                {"group": "Servers", "title": "Imported-2", "username": "import_user2"},
+            ])
+        assert "Imported 2" in result
+
+        # Vault is locked after import — re-unlock and verify entries exist.
+        # keepassxc-cli merge creates entries under a parallel group (UUID
+        # mismatch), so search vault-wide instead of listing a specific group.
+        await vault.unlock()
+        from server.tools.read import search_entries
+        found = await search_entries(vault, query="Imported")
+        found_titles = [e["title"] for e in found]
+        assert "Imported-1" in found_titles
+        assert "Imported-2" in found_titles

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

@@ -3,6 +3,8 @@
 
 import pytest
 
+pytestmark = pytest.mark.unit
+
 
 class TestAuditLogger:
     def test_log_creates_file(self, test_config):
@@ -116,3 +118,69 @@ def test_permission_error_logs_warning(self, tmp_path, caplog):
             logger.log(tool="test", title="Test")
         finally:
             os.chmod(audit_path, 0o644)
+
+
+class TestSanitizeExtra:
+    """_sanitize_extra redacts values for keys containing sensitive fragments.
+
+    This is a security-critical function: a regression silently leaks
+    passwords, tokens, or API keys to the audit log file on disk.
+    """
+
+    def test_password_key_redacted(self):
+        from server.audit import _sanitize_extra
+        result = _sanitize_extra({"database_password": "s3cret"})
+        assert result["database_password"] == "**REDACTED**"
+
+    def test_token_key_redacted(self):
+        from server.audit import _sanitize_extra
+        result = _sanitize_extra({"refresh_token": "abc123"})
+        assert result["refresh_token"] == "**REDACTED**"
+
+    def test_api_key_redacted(self):
+        from server.audit import _sanitize_extra
+        result = _sanitize_extra({"api_key": "key-xyz"})
+        assert result["api_key"] == "**REDACTED**"
+
+    def test_auth_key_redacted(self):
+        from server.audit import _sanitize_extra
+        result = _sanitize_extra({"auth_header": "Bearer xyz"})
+        assert result["auth_header"] == "**REDACTED**"
+
+    def test_case_insensitive_matching(self):
+        from server.audit import _sanitize_extra
+        result = _sanitize_extra({"API_KEY": "val", "Secret_Value": "val2"})
+        assert result["API_KEY"] == "**REDACTED**"
+        assert result["Secret_Value"] == "**REDACTED**"
+
+    def test_non_sensitive_key_passes_through(self):
+        from server.audit import _sanitize_extra
+        result = _sanitize_extra({"group": "Servers", "title": "Test"})
+        assert result["group"] == "Servers"
+        assert result["title"] == "Test"
+
+    def test_mixed_sensitive_and_safe(self):
+        from server.audit import _sanitize_extra
+        result = _sanitize_extra({
+            "title": "My Entry",
+            "password_hash": "abc",
+            "count": 5,
+        })
+        assert result["title"] == "My Entry"
+        assert result["password_hash"] == "**REDACTED**"
+        assert result["count"] == 5
+
+    def test_empty_dict_returns_empty(self):
+        from server.audit import _sanitize_extra
+        assert _sanitize_extra({}) == {}
+
+    def test_redaction_used_in_audit_log(self, tmp_path):
+        """End-to-end: extra kwargs with sensitive keys are redacted in the log file."""
+        import json
+        from server.audit import AuditLogger
+
+        audit_path = tmp_path / "audit.jsonl"
+        logger = AuditLogger(str(audit_path))
+        logger.log(tool="test", title="Test", credential="should-be-redacted")
+        record = json.loads(audit_path.read_text().strip())
+        assert record["credential"] == "**REDACTED**"

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

@@ -4,6 +4,8 @@
 import pytest
 import yaml
 
+pytestmark = pytest.mark.unit
+
 
 @pytest.fixture
 def valid_config(tmp_path):

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

@@ -128,3 +128,29 @@ def test_hidraw_not_readable_treated_as_missing(self, mock_run, mock_glob, mock_
         ]
         result = diagnose_unlock_failure(_make_config())
         assert "hidraw" in result.lower() or "unbind" in result.lower()
+
+    @patch("os.access", return_value=True)
+    @patch("glob.glob", return_value=["/dev/hidraw0"])
+    @patch("subprocess.run")
+    def test_udevadm_nonzero_returncode_skips_device(self, mock_run, mock_glob, mock_access):
+        """When udevadm returns non-zero for a hidraw device, the loop continues past it."""
+        from server.diagnostics import diagnose_unlock_failure
+
+        mock_run.side_effect = [
+            MagicMock(returncode=3),  # pcscd inactive
+            MagicMock(returncode=1, stdout=""),  # udevadm fails for hidraw0
+        ]
+        # No YubiKey OTP interface found → falls through to USB reset message
+        result = diagnose_unlock_failure(_make_config())
+        assert "hidraw" in result.lower() or "unbind" in result.lower()
+
+    def test_check_raising_generic_exception_falls_through(self):
+        """A diagnostic check raising a non-subprocess Exception is caught and skipped."""
+        from server.diagnostics import diagnose_unlock_failure
+
+        with patch("server.diagnostics._check_pcscd", side_effect=RuntimeError("unexpected")), \
+             patch("server.diagnostics._check_hidraw", return_value=None), \
+             patch("server.diagnostics._check_serial", return_value=None):
+            result = diagnose_unlock_failure(_make_config())
+        # All checks either raised or returned None → empty string
+        assert result == ""