From 03cf307bbe9151d6306ff5ef38875c8dbde68afc Mon Sep 17 00:00:00 2001 From: Bas Alberts Date: Fri, 10 Jul 2026 10:36:23 -0400 Subject: [PATCH 1/2] Log tool-call environment variable names instead of values Debug logging in mcp_client_params printed the full tool-call environment, which includes resolved credentials such as GH_TOKEN. Log only the variable names so secrets are never written to logs, while keeping the names for debuggability. The environment passed to the MCP server is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/seclab_taskflow_agent/mcp_utils.py | 16 +++++++++-- tests/test_mcp_utils.py | 40 +++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/seclab_taskflow_agent/mcp_utils.py b/src/seclab_taskflow_agent/mcp_utils.py index 36d1df7..a219dbb 100644 --- a/src/seclab_taskflow_agent/mcp_utils.py +++ b/src/seclab_taskflow_agent/mcp_utils.py @@ -172,6 +172,16 @@ async def call_tool(self, *args: Any, **kwargs: Any) -> Any: ClientParamsMap = dict[str, tuple[dict[str, Any], list[str], str | None, int | None]] +def _env_names(env: dict[str, Any] | None) -> list[str] | None: + """Return only the environment variable names, for safe debug logging. + + Tool-call environments routinely carry credentials (e.g. ``GH_TOKEN``), so + their values must never be written to logs. Callers log the names to keep + debug output useful without leaking secrets. + """ + return sorted(env) if env else env + + def mcp_client_params( available_tools: AvailableTools, requested_toolboxes: list[str], @@ -202,7 +212,7 @@ def mcp_client_params( case "stdio": env = dict(sp.env) if sp.env else None args = list(sp.args) if sp.args else None - logging.debug("Initializing toolbox: %s\nargs:\n%s\nenv:\n%s\n", tb, args, env) + logging.debug("Initializing toolbox: %s\nargs:\n%s\nenv names:\n%s\n", tb, args, _env_names(env)) if env: for k, v in list(env.items()): try: @@ -217,7 +227,7 @@ def mcp_client_params( "http_proxy", "https_proxy", "no_proxy"): if proxy_var not in env and proxy_var in os.environ: env[proxy_var] = os.environ[proxy_var] - logging.debug("Tool call environment: %s", env) + logging.debug("Tool call environment names: %s", _env_names(env)) if args: for i, v in enumerate(args): args[i] = swap_env(v) @@ -241,7 +251,7 @@ def mcp_client_params( if sp.command is not None: env = dict(sp.env) if sp.env else None args = list(sp.args) if sp.args else None - logging.debug("Initializing streamable toolbox: %s\nargs:\n%s\nenv:\n%s\n", tb, args, env) + logging.debug("Initializing streamable toolbox: %s\nargs:\n%s\nenv names:\n%s\n", tb, args, _env_names(env)) exe = shutil.which(sp.command) if exe is None: raise FileNotFoundError(f"Could not resolve path to {sp.command}") diff --git a/tests/test_mcp_utils.py b/tests/test_mcp_utils.py index 1480b98..bf98696 100644 --- a/tests/test_mcp_utils.py +++ b/tests/test_mcp_utils.py @@ -6,12 +6,13 @@ from __future__ import annotations import asyncio +import logging from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock import pytest -from seclab_taskflow_agent.mcp_utils import MCPNamespaceWrap, compress_name +from seclab_taskflow_agent.mcp_utils import MCPNamespaceWrap, compress_name, mcp_client_params class _FakeTool: @@ -145,3 +146,40 @@ def test_list_tools_existing_behaviour_unchanged(): obj.list_tools.assert_awaited_once_with(run_context="ctx", agent="agent") ns = compress_name("RepoContext") assert result[0].name == f"{ns}read_file" + + +# -- secret redaction in debug logs -- + + +def test_mcp_client_params_logs_env_names_not_secret_values(monkeypatch, caplog): + # A tool-call environment routinely resolves credentials like GH_TOKEN; + # debug logging must record only the variable names, never the values. + sentinel = "do-not-log-this-value" + monkeypatch.setenv("GH_TOKEN", sentinel) + server_params = SimpleNamespace( + kind="stdio", + reconnecting=False, + env={"GH_TOKEN": "{{ env('GH_TOKEN') }}", "LOG_DIR": "/tmp/logs"}, + args=None, + command="echo", + ) + toolbox = SimpleNamespace( + server_params=server_params, + confirm=[], + server_prompt=None, + client_session_timeout=None, + ) + available_tools = MagicMock() + available_tools.get_toolbox.return_value = toolbox + + with caplog.at_level(logging.DEBUG): + params = mcp_client_params(available_tools, ["pkg.tb"]) + + # The resolved secret value never reaches the logs ... + assert sentinel not in caplog.text + # ... but the variable names are still logged for debuggability. + assert "GH_TOKEN" in caplog.text + assert "LOG_DIR" in caplog.text + # Redaction is log-only: the real env (with the resolved secret) is still + # passed through to the MCP server. + assert params["pkg.tb"][0]["env"]["GH_TOKEN"] == sentinel From a77a3783384a3cb6c18408df98dd335ac63cbfa2 Mon Sep 17 00:00:00 2001 From: Bas Alberts Date: Fri, 10 Jul 2026 10:41:34 -0400 Subject: [PATCH 2/2] Return an empty list for an empty env in _env_names Check `env is None` so an empty dict yields `[]` rather than the dict, honoring the list[str] | None return contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/seclab_taskflow_agent/mcp_utils.py | 2 +- tests/test_mcp_utils.py | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/seclab_taskflow_agent/mcp_utils.py b/src/seclab_taskflow_agent/mcp_utils.py index a219dbb..2825d01 100644 --- a/src/seclab_taskflow_agent/mcp_utils.py +++ b/src/seclab_taskflow_agent/mcp_utils.py @@ -179,7 +179,7 @@ def _env_names(env: dict[str, Any] | None) -> list[str] | None: their values must never be written to logs. Callers log the names to keep debug output useful without leaking secrets. """ - return sorted(env) if env else env + return sorted(env) if env is not None else None def mcp_client_params( diff --git a/tests/test_mcp_utils.py b/tests/test_mcp_utils.py index bf98696..24c5fbb 100644 --- a/tests/test_mcp_utils.py +++ b/tests/test_mcp_utils.py @@ -12,7 +12,12 @@ import pytest -from seclab_taskflow_agent.mcp_utils import MCPNamespaceWrap, compress_name, mcp_client_params +from seclab_taskflow_agent.mcp_utils import ( + MCPNamespaceWrap, + _env_names, + compress_name, + mcp_client_params, +) class _FakeTool: @@ -183,3 +188,11 @@ def test_mcp_client_params_logs_env_names_not_secret_values(monkeypatch, caplog) # Redaction is log-only: the real env (with the resolved secret) is still # passed through to the MCP server. assert params["pkg.tb"][0]["env"]["GH_TOKEN"] == sentinel + + +def test_env_names_returns_sorted_names_or_none(): + # Non-empty env -> sorted names; empty dict -> empty list; None -> None + # (honouring the list[str] | None contract, never leaking values). + assert _env_names({"B_VAR": "x", "A_VAR": "{{ env('A') }}"}) == ["A_VAR", "B_VAR"] + assert _env_names({}) == [] + assert _env_names(None) is None