From 549e9b426760ff6f3180a6533fcfa04a8160c89c Mon Sep 17 00:00:00 2001 From: Dan Wolfson Date: Mon, 5 Oct 2026 07:41:07 -0500 Subject: [PATCH] fix(mcp_adapter): stop writing the caller's password to stderr and the log (ISSUE-121) run_report and _execute_egeria_call_blocking printed user_pass to stderr, and run_report also logged it via loguru. One _describe_call() helper now builds the diagnostic for all three sites: it keeps report, params, server and user, and says which kind of credential was supplied instead of its value. Also corrects the ISSUE-121 entry (three sites in two functions; the settings-fallback path never leaked). Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Dan Wolfson --- PYEGERIA_ISSUES.md | 21 ++++++-- pyegeria/core/mcp_adapter.py | 24 +++++++-- .../test_mcp_adapter_no_secret_logging.py | 54 +++++++++++++++++++ 3 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 tests/micro-tests/test_mcp_adapter_no_secret_logging.py diff --git a/PYEGERIA_ISSUES.md b/PYEGERIA_ISSUES.md index 5cc25828..553370d0 100644 --- a/PYEGERIA_ISSUES.md +++ b/PYEGERIA_ISSUES.md @@ -1192,11 +1192,26 @@ lacks `deleteMethod`, `permittedSynchronization`, `connectionName` and ### ISSUE-121: `core/mcp_adapter.py` writes the caller's user name and plaintext password to stderr and the log on every report call -**Layer:** pyegeria (credential leak in diagnostics) · **Status:** open, -logged only, not fixed (per the gaps-tracking rule, awaiting explicit -approval) · **Found:** 2026-10-03, read-only security sweep by the Resource +**Layer:** pyegeria (credential leak in diagnostics) · **Status:** fixed on +branch `fix/issue-121-mcp-adapter-no-password-logging` (2026-10-05), pending +PR/merge · **Found:** 2026-10-03, read-only security sweep by the Resource Explorer coordinator session; no value was read or recorded. +**Correction to the original report (2026-10-05, from reading the code):** +there were **three** leak sites in **two** functions, not two in one -- +`_execute_egeria_call_blocking` printed the string to stderr, and `run_report` +both printed it to stderr and `logger.info`'d it. And the fallback path did +**not** leak the configured profile's password: the string is built before the +settings fallback, so it logged `None` there; only a password a caller passed +explicitly was written. **Fix:** one `_describe_call()` helper used by all three +sites; it keeps report/params/server/user and states the *kind* of credential +(`bearer token` / `explicit user/password` / `defaults from settings`), never the +value. Regression tests (`test_mcp_adapter_no_secret_logging.py`) assert a +sentinel password and token appear in neither stderr nor a loguru sink, and fail +on the old code. **Not done by code:** any `debug_log.*` archive written before +this fix may already hold a password, and rotating a real credential used +through this adapter is still the owner's call. + **What:** the report-execution entry point in `pyegeria/core/mcp_adapter.py` (the function whose docstring covers the `token` / `user` / `user_pass` fallback, around lines 170-180) builds a "Format set" diagnostic string that diff --git a/pyegeria/core/mcp_adapter.py b/pyegeria/core/mcp_adapter.py index f05d0fcb..6b181585 100644 --- a/pyegeria/core/mcp_adapter.py +++ b/pyegeria/core/mcp_adapter.py @@ -104,6 +104,21 @@ def describe_report(name: str, output_type: str = "DICT") -> Dict[str, Any]: return meta +def _describe_call(report: str, params: Optional[Dict[str, Any]], view_server: Optional[str], + view_url: Optional[str], user: Optional[str], user_pass: Optional[str], + token: Optional[str]) -> str: + """Diagnostic text for a report call. ISSUE-121: never includes the password or token -- + only which kind of credential the caller supplied.""" + if token is not None: + credentials = "bearer token" + elif user_pass is not None: + credentials = "explicit user/password" + else: + credentials = "defaults from settings" + return (f"Format set: {report}\nparams: {json.dumps(params)}\nview_server: {view_server}\n" + f"view_url: {view_url}\nuser: {user}\ncredentials: {credentials}") + + def _execute_egeria_call_blocking( *, report: str, @@ -124,9 +139,7 @@ def _execute_egeria_call_blocking( `user`/`user_pass` service account. """ - print( - f"Format set: {report}\nparams: {json.dumps(params)}\nview_server: {view_server}\nview_url: {view_url}\nuser: {user}\nuser_pass: {user_pass}", - file=sys.stderr) + print(_describe_call(report, params, view_server, view_url, user, user_pass, token), file=sys.stderr) # Lazy import of settings to avoid circulars when optional args are None # from pyegeria.config import settings as _settings from pyegeria.core.config import settings as _settings @@ -171,10 +184,11 @@ def run_report( account. `user`/`user_pass` remain fully backward compatible when `token` is not given. """ - print(f"Format set: {report}\nparams: {json.dumps(params)}\nview_server: {view_server}\nview_url: {view_url}\nuser: {user}\nuser_pass: {user_pass}", file=sys.stderr) + description = _describe_call(report, params, view_server, view_url, user, user_pass, token) + print(description, file=sys.stderr) # Lazy import of settings to avoid circulars when optional args are None from pyegeria.core.config import settings as _settings - logger.info(f"Format set: {report}\nparams: {json.dumps(params)}\nview_server: {view_server}\nview_url: {view_url}\nuser: {user}\nuser_pass: {user_pass}") + logger.info(description) return exec_report_spec( format_set_name=report, output_format="DICT", diff --git a/tests/micro-tests/test_mcp_adapter_no_secret_logging.py b/tests/micro-tests/test_mcp_adapter_no_secret_logging.py new file mode 100644 index 00000000..7d6669c3 --- /dev/null +++ b/tests/micro-tests/test_mcp_adapter_no_secret_logging.py @@ -0,0 +1,54 @@ +""" +ISSUE-121: mcp_adapter must not write the caller's password (or bearer token) to +stderr or the log when running a report. +""" +import pytest +from loguru import logger + +from pyegeria.core import mcp_adapter + +SENTINEL_PASSWORD = "s3ntinel-PASSWORD-do-not-log" +SENTINEL_TOKEN = "s3ntinel-TOKEN-do-not-log" + + +@pytest.fixture +def captured_log(): + messages: list[str] = [] + sink_id = logger.add(lambda m: messages.append(str(m)), level="DEBUG") + yield messages + logger.remove(sink_id) + + +@pytest.fixture(autouse=True) +def _stub_exec(monkeypatch): + monkeypatch.setattr(mcp_adapter, "exec_report_spec", lambda **kwargs: {"ok": True}) + + +@pytest.mark.parametrize("fn_name", ["run_report", "_execute_egeria_call_blocking"]) +def test_password_not_written_to_stderr_or_log(fn_name, capsys, captured_log): + getattr(mcp_adapter, fn_name)(report="Some-Report", params={"a": 1}, view_server="vs", + view_url="https://localhost:1", user="someone", + user_pass=SENTINEL_PASSWORD) + + out = capsys.readouterr() + assert SENTINEL_PASSWORD not in out.err + out.out + assert SENTINEL_PASSWORD not in "".join(captured_log) + # still useful for debugging: report, user and the kind of credential are shown + assert "Some-Report" in out.err and "someone" in out.err + assert "explicit user/password" in out.err + + +@pytest.mark.parametrize("fn_name", ["run_report", "_execute_egeria_call_blocking"]) +def test_token_not_written_to_stderr_or_log(fn_name, capsys, captured_log): + getattr(mcp_adapter, fn_name)(report="Some-Report", token=SENTINEL_TOKEN) + + out = capsys.readouterr() + assert SENTINEL_TOKEN not in out.err + out.out + assert SENTINEL_TOKEN not in "".join(captured_log) + assert "bearer token" in out.err + + +def test_fallback_to_settings_is_described(capsys): + mcp_adapter.run_report(report="Some-Report") + + assert "defaults from settings" in capsys.readouterr().err