fix(mcp): redact database_url credentials in diagnostics output

Extends the existing _SECRET_FIELDS redaction to also strip the
user:password userinfo component from any URL-bearing config fields
(database_url) before surfacing them in diagnostics output. Adds
_redact_url() helper and a _URL_FIELDS set so future credential-bearing
URL fields are easy to add. Postgres passwords no longer leak to MCP
clients or support transcripts; host/port/db-name remain visible.

Fixes the P1 security finding raised in review of PR #963.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Drew Cain <groksrc@gmail.com>

src/basic_memory/mcp/tools/basic_memory_diagnostics.py

@@ -3,6 +3,7 @@
 import json
 import platform
 import sys
+from urllib.parse import urlparse, urlunparse
 
 import basic_memory
 from basic_memory.config import CONFIG_FILE_NAME, ConfigManager
@@ -11,15 +12,59 @@ from basic_memory.mcp.server import mcp
 # Fields in BasicMemoryConfig that contain secrets and must never be surfaced.
 _SECRET_FIELDS = frozenset({"cloud_api_key"})
 
+# Fields whose values are URLs that may embed user:password credentials.
+# The userinfo component is stripped before surfacing.
+_URL_FIELDS = frozenset({"database_url"})
+
+
+def _redact_url(url: str) -> str:
+    """Strip the userinfo (user:password) from a URL string.
+
+    Replaces any credentials with *** so the host/path remain visible for
+    diagnostics (e.g. ``postgresql://***@localhost/mydb``).  If the value
+    cannot be parsed as a URL it is returned unchanged.
+    """
+    try:
+        parsed = urlparse(url)
+    except Exception:  # pragma: no cover — urlparse is very permissive
+        return url
+
+    if not parsed.hostname:
+        # Not a meaningful URL (e.g. a bare file path); leave it alone.
+        return url
+
+    if parsed.username or parsed.password:
+        # Rebuild with credentials replaced by a placeholder.
+        netloc = f"***@{parsed.hostname}"
+        if parsed.port:
+            netloc = f"{netloc}:{parsed.port}"
+        redacted = parsed._replace(netloc=netloc)
+        return urlunparse(redacted)
+
+    return url
+
 
 def _redact_config(raw: dict) -> dict:
     """Return a copy of the raw config dict with secret fields removed.
 
-    Only top-level keys are redacted. Nested secret-looking keys within
-    project entries are not currently present, but the pattern is explicit
-    so it is easy to extend.
+    - Keys in ``_SECRET_FIELDS`` are dropped entirely.
+    - Keys in ``_URL_FIELDS`` have their userinfo component stripped so that
+      host and database name remain visible for diagnostics.
+
+    Only top-level keys are processed. Nested keys within project entries are
+    not currently credential-bearing, but the two sets make the pattern easy
+    to extend.
     """
-    return {k: v for k, v in raw.items() if k not in _SECRET_FIELDS}
+    result: dict = {}
+    for k, v in raw.items():
+        if k in _SECRET_FIELDS:
+            # Drop entirely — value has no diagnostic value.
+            continue
+        if k in _URL_FIELDS and isinstance(v, str):
+            result[k] = _redact_url(v)
+        else:
+            result[k] = v
+    return result
 
 
 @mcp.tool(

tests/mcp/test_tool_basic_memory_diagnostics.py

@@ -8,6 +8,7 @@ from unittest.mock import MagicMock, patch
 import basic_memory
 from basic_memory.mcp.tools.basic_memory_diagnostics import (
     _redact_config,
+    _redact_url,
     basic_memory_diagnostics,
 )
 
@@ -144,3 +145,112 @@ def test_diagnostics_output_sections():
     assert "## Version" in result
     assert "## System" in result
     assert "## Configuration" in result
+
+
+# ---------------------------------------------------------------------------
+# Unit tests for _redact_url helper
+# ---------------------------------------------------------------------------
+
+
+def test_redact_url_strips_password():
+    url = "postgresql://user:secret@localhost/mydb"
+    result = _redact_url(url)
+    assert "secret" not in result
+    assert "user" not in result
+    assert "localhost" in result
+    assert "mydb" in result
+    assert "***" in result
+
+
+def test_redact_url_strips_only_password_when_no_username():
+    # password-only userinfo (unusual but valid per RFC)
+    url = "postgresql://:secret@db.example.com/app"
+    result = _redact_url(url)
+    assert "secret" not in result
+    assert "db.example.com" in result
+
+
+def test_redact_url_preserves_port():
+    url = "postgresql://admin:pw@db.internal:5432/prod"
+    result = _redact_url(url)
+    assert "pw" not in result
+    assert "5432" in result
+    assert "db.internal" in result
+
+
+def test_redact_url_no_credentials_unchanged():
+    url = "postgresql://db.internal:5432/prod"
+    assert _redact_url(url) == url
+
+
+def test_redact_url_non_url_string_unchanged():
+    # Bare file paths / non-URL values must not be mangled.
+    path = "/home/user/.local/share/basic-memory/main.db"
+    assert _redact_url(path) == path
+
+
+# ---------------------------------------------------------------------------
+# _redact_config tests for database_url
+# ---------------------------------------------------------------------------
+
+
+def test_redact_config_scrubs_database_url_credentials():
+    raw = {
+        "default_project": "main",
+        "database_url": "postgresql://dbuser:dbpass@host.example.com:5432/bm",
+        "projects": {},
+    }
+    result = _redact_config(raw)
+    assert "dbpass" not in result["database_url"]
+    assert "dbuser" not in result["database_url"]
+    # Host and db should still be present for diagnostic value.
+    assert "host.example.com" in result["database_url"]
+    assert "bm" in result["database_url"]
+
+
+def test_redact_config_leaves_database_url_without_credentials():
+    raw = {"database_url": "sqlite:////tmp/basic-memory/main.db"}
+    result = _redact_config(raw)
+    assert result["database_url"] == "sqlite:////tmp/basic-memory/main.db"
+
+
+def test_redact_config_drops_secret_fields_independently():
+    raw = {
+        "cloud_api_key": "bmc_top_secret",
+        "database_url": "postgresql://dbuser:dbpassword@host/db",
+        "default_project": "main",
+    }
+    result = _redact_config(raw)
+    assert "cloud_api_key" not in result
+    assert "dbpassword" not in result["database_url"]
+    assert "dbuser" not in result["database_url"]
+    assert "main" == result["default_project"]
+
+
+# ---------------------------------------------------------------------------
+# Integration: database_url redaction surfaces in diagnostic output
+# ---------------------------------------------------------------------------
+
+
+def test_diagnostics_redacts_database_url_password(tmp_path):
+    """Postgres password in database_url must not appear in diagnostic output."""
+    config_data = {
+        "default_project": "main",
+        "database_url": "postgresql://pguser:supersecret@db.internal:5432/basicmemory",
+        "projects": {},
+    }
+    with patch("basic_memory.mcp.tools.basic_memory_diagnostics.ConfigManager") as MockMgr:
+        mock_mgr = MagicMock()
+        mock_mgr.config_dir = tmp_path
+        MockMgr.return_value = mock_mgr
+
+        config_file = tmp_path / "config.json"
+        config_file.write_text(json.dumps(config_data))
+
+        result = basic_memory_diagnostics()
+
+    assert "supersecret" not in result
+    assert "pguser" not in result
+    # Host and port remain visible for diagnostics.
+    assert "db.internal" in result
+    assert "5432" in result