fix(mcp,redact,gateway_tools): mask credentials in tool results, names kept and values gone

src/beaver_gateway/core/gateway_tools.py

@@ -8,12 +8,14 @@ session gets comes from ``ClaudeAgent.gateway_tools``.
 from __future__ import annotations
 
 import logging
+from dataclasses import replace
 from typing import TYPE_CHECKING, Any, cast
 
 from claude_agent_sdk import create_sdk_mcp_server, tool
 
 from beaver_gateway.core.injects import URGENCY
 from beaver_gateway.core.kinds import as_kind
+from beaver_gateway.core.redact import redact_data
 
 URGENCY_HELP = (
     "normal waits for the hourly batch or rides with the next turn, wake "
@@ -52,13 +54,34 @@ def build_tool_server(
     if unknown:
         msg = f"unknown gateway tools: {sorted(unknown)}"
         raise ValueError(msg)
-    tools = [t for t in _tools(conversations, conversation_key) if t.name in wanted]
+    tools = [
+        _redacting(t)
+        for t in _tools(conversations, conversation_key)
+        if t.name in wanted
+    ]
     if not tools:
         return None
     server = create_sdk_mcp_server(SERVER_NAME, tools=tools)
     return cast("McpSdkServerConfig", {**server, "alwaysLoad": True})
 
 
+def _redacting(spec: SdkMcpTool[Any]) -> SdkMcpTool[Any]:
+    """Put a tool's result through the same mask as every other MCP.
+
+    These tools are mounted in-process by the SDK, so they bypass the
+    ``FastMCP`` middleware in :mod:`beaver_gateway.mcp.redacting` and
+    need the filter attached here instead. ``read_conversation`` is the
+    one that earns it: it replays a transcript, and a transcript written
+    before any of this existed can still hold a credential.
+    """
+    inner = spec.handler
+
+    async def handler(args: Any) -> dict[str, Any]:
+        return cast("dict[str, Any]", redact_data(await inner(args)))
+
+    return replace(spec, handler=handler)
+
+
 def _tools(conversations: Conversations, key: str) -> list[SdkMcpTool[Any]]:
     async def current() -> Any:
         conv = await conversations.get(key)

src/beaver_gateway/core/redact.py

@@ -1,4 +1,4 @@
-"""Keep credentials out of the process log.
+"""Recognise credentials in free text and mask them.
 
 ``docker logs`` is not a vault: the stack's stdout lands in an unrotated
 json file on the host, and the model itself can page through it (the
@@ -18,9 +18,16 @@ feed, both handed to the calendar MCP as query parameters.
 
 So: silence the HTTP clients' per-request chatter (nothing here reads
 it), and run one redaction pass over every formatted record as a second
-line of defence. Redaction knows two things — patterns for credentials
-that live in URLs and headers, and the literal values of the
-secret-looking environment variables this process was started with.
+line of defence.
+
+The same pass guards the wider channel — what an MCP tool hands back to
+the model (see :mod:`beaver_gateway.mcp.redacting`). :func:`redact`
+recognises three things: literal values of the secret-looking
+environment variables this process was started with, credentials that
+live in URLs and headers, and ``NAME: value`` assignments whose *name*
+says the value is a credential. The last one is what covers a config
+dump of a stack whose secrets this process never held — and it keeps
+the name, masking only the value, so the dump still says what is set.
 """
 
 from __future__ import annotations
@@ -32,7 +39,7 @@ from typing import TYPE_CHECKING
 from urllib.parse import quote, unquote
 
 if TYPE_CHECKING:
-    from collections.abc import Mapping
+    from collections.abc import Callable, Mapping
 
 MASK = "<…>"
 
@@ -44,10 +51,16 @@ CHATTY: tuple[str, ...] = ("httpx", "httpcore", "aiohttp.client", "urllib3")
 # ``dictConfig`` drops a logger's handlers but keeps its filters.
 OWN_HANDLERS: tuple[str, ...] = ("uvicorn", "uvicorn.access", "uvicorn.error")
 
-# Env vars whose *value* is a credential — matched as a substring of the
-# name, so ``FIREFLY_PAT`` and ``T3_MAC_TOKEN`` both qualify.
+# Names whose *value* is a credential — matched as a substring, so
+# ``FIREFLY_PAT`` and ``T3_MAC_TOKEN`` both qualify. Used twice: to pick
+# which environment variables contribute literal values, and to decide
+# whether a ``NAME: value`` line in a config dump should keep its value.
+# ``MCP`` is in here because every MCP URL in this stack carries its
+# credential in the query string.
 _SECRET_NAME = re.compile(
-    r"TOKEN|SECRET|KEY|PASSWORD|PASS\b|PAT\b|BEARER|CREDENTIAL|MCPS", re.IGNORECASE
+    r"TOKEN|SECRET|KEY|PASSWORD|PASS\b|PAT\b|BEARER|CREDENTIAL"
+    r"|MCPS?\b|AUTH|PRIVATE|SESSION|DSN",
+    re.IGNORECASE,
 )
 _MIN_SECRET = 8
 
@@ -57,7 +70,38 @@ _LEFT = r"(?:(?<![A-Za-z0-9])|(?<=%26)|(?<=%3F))"
 # ...and runs until the next separator, encoded ``&`` included.
 _VALUE = r"(?:(?!%26)[^&\s\"'<>,;)\]}])+"
 
-_RULES: tuple[tuple[re.Pattern[str], str], ...] = (
+
+# Names worth masking where they sit in a dump next to their password,
+# but not worth hunting for as literals across every log line: a
+# username is short and ordinary, and ``CLAUDE_RUNNER_USER=beaver-runner``
+# turned into a global literal would mask half the entrypoint script.
+_SECRET_IN_DUMP = re.compile(r"USER\b|LOGIN\b", re.IGNORECASE)
+
+
+def is_secret_name(name: str) -> bool:
+    """Whether a variable/field of this name holds a credential."""
+    return bool(_SECRET_NAME.search(name) or _SECRET_IN_DUMP.search(name))
+
+
+def _mask_named_value(match: re.Match[str]) -> str:
+    """Mask ``value`` in a ``NAME: value`` match, keeping the name.
+
+    A dump is still worth reading when it says *which* variables are
+    set; it is the values that must not travel. An already-masked or
+    empty value is left alone so the pass stays idempotent, and ``lead``
+    / ``tail`` carry back whatever quoting the surrounding format used.
+    """
+    groups = match.groupdict()
+    name, value = groups["name"], groups["value"]
+    if not is_secret_name(name) or value.strip(" \t\"'") in ("", MASK):
+        return match[0]
+    lead, tail = groups.get("lead") or "", groups.get("tail") or ""
+    return f"{lead}{name}{groups['sep']}{MASK}{tail}"
+
+
+type _Repl = str | Callable[[re.Match[str]], str]
+
+_RULES: tuple[tuple[re.Pattern[str], _Repl], ...] = (
     # Google Calendar's secret path segment; the ``/basic.ics`` after it
     # survives, encoded or not, so the line still says what it fetched.
     (
@@ -85,6 +129,50 @@ _RULES: tuple[tuple[re.Pattern[str], str], ...] = (
     ),
     # A bare ``Bearer <token>`` anywhere else.
     (re.compile(r"\b(bearer)\s+[A-Za-z0-9._~+/=-]{8,}", re.IGNORECASE), r"\1 " + MASK),
+    # ``NAME: value`` / ``NAME=value`` at the start of a line — the shape
+    # of a compose ``environment:`` block, an ``.env`` file, a printed
+    # settings object. Anchored to the line so that an ``actor=token:foo``
+    # in the middle of a log line doesn't swallow the rest of it, and
+    # ``=`` must be tight (``NAME=value``, the way env files write it) so
+    # that ``SESSION_SECRET = os.environ[…]`` in source the dispatcher is
+    # reading through t3code stays readable.
+    (
+        re.compile(
+            r"""(?m)^(?P<lead>[ \t]*(?:-[ \t]+)?["']?)
+                 (?P<name>[A-Z][A-Z0-9_]{2,})
+                 (?P<sep>["']?(?::[ \t]*|=))
+                 (?P<value>"[^"\n]*"|'[^'\n]*'|[^\n]*)""",
+            re.VERBOSE,
+        ),
+        _mask_named_value,
+    ),
+    # The same at the start of a line in lower case, but only for names
+    # that are unambiguously a credential — ``_SECRET_NAME`` is too broad
+    # here, it would read the ``mcp:`` prefix of a log line as one.
+    (
+        re.compile(
+            r"""(?m)^(?P<lead>[ \t]*(?:-[ \t]+)?["']?)
+                 (?P<name>password|passwd|secret|token|api[-_]?key|apikey
+                   |access[-_]?token|auth[-_]?token|private[-_]?key|credentials?)
+                 (?P<sep>["']?(?::[ \t]*|=))
+                 (?P<value>"[^"\n]*"|'[^'\n]*'|[^\n]*)""",
+            re.VERBOSE | re.IGNORECASE,
+        ),
+        _mask_named_value,
+    ),
+    # ``scheme://user:password@host`` — the credential a connection
+    # string carries. Host and database stay, so the line still locates
+    # the service it failed to reach.
+    (re.compile(r"(://[^/\s:@]+:)[^@\s/]+(@)"), r"\1" + MASK + r"\2"),
+    # ``"name": "value"`` anywhere — the same idea for one-line JSON,
+    # where the quotes make the end of the value unambiguous.
+    (
+        re.compile(
+            r'(?P<lead>")(?P<name>[^"\n]{1,64})(?P<sep>"\s*:\s*")'
+            r'(?P<value>[^"\n]*)(?P<tail>")'
+        ),
+        _mask_named_value,
+    ),
 )
 
 _ENV_SECRETS: list[str] = []
@@ -120,6 +208,26 @@ def redact(text: str) -> str:
     return text
 
 
+def redact_data(value: object) -> object:
+    """Mask every string inside a decoded JSON tree.
+
+    A field name is the only context a bare value has: ``"0d1e…"`` on
+    its own looks like nothing, ``{"ADMIN_PASS": "0d1e…"}`` is a
+    password. So a secret-looking key masks its whole subtree — the
+    structured twin of the ``NAME: value`` rule above.
+    """
+    if isinstance(value, str):
+        return redact(value)
+    if isinstance(value, dict):
+        return {
+            key: MASK if is_secret_name(str(key)) and item else redact_data(item)
+            for key, item in value.items()
+        }
+    if isinstance(value, list):
+        return [redact_data(item) for item in value]
+    return value
+
+
 class RedactingFormatter(logging.Formatter):
     """Wrap another formatter and redact whatever it produced.
 

src/beaver_gateway/mcp/internal_app.py

@@ -33,6 +33,7 @@ from starlette.applications import Starlette
 from starlette.routing import Mount
 
 from beaver_gateway.mcp.client_pool import build_http_proxy, build_stdio_proxy
+from beaver_gateway.mcp.redacting import RedactingMiddleware
 from beaver_gateway.mcp.types import HttpMcp, PythonToolMcp, StdioMcp
 from beaver_gateway.mcp.wrap import build_python_tool_server
 
@@ -69,6 +70,12 @@ def build_internal_app(
         MCP tools into the Raycast wire).
     """
     servers: dict[str, FastMCP] = {spec.name: _build_server(spec) for spec in mcps}
+    # The one place every tool result passes through on its way to the
+    # model. Attached per namespace rather than once at the top so that
+    # a direct ``call_tool`` on a child (the Raycast backend does that)
+    # is filtered too.
+    for server in servers.values():
+        server.add_middleware(RedactingMiddleware())
 
     child_apps = {
         name: s.http_app(transport="http", path="/") for name, s in servers.items()
@@ -80,6 +87,7 @@ def build_internal_app(
     all_app = None
     if servers:
         all_server = _build_all_server(servers)
+        all_server.add_middleware(RedactingMiddleware())
         all_app = all_server.http_app(transport="http", path="/")
         routes.append(Mount(f"/mcp/{ALL_NAMESPACE}", app=all_app))
 

src/beaver_gateway/mcp/redacting.py

@@ -0,0 +1,85 @@
+"""Redact credentials on the way out of an MCP tool.
+
+A tool result is a wider channel than the log. It goes straight into the
+model's context, from there into the turn record in Postgres, and from
+there into the markdown transcript in ``💬 чаты`` — which Obsidian Sync
+carries off the machine. One ``komodo`` deploy returns the resolved
+compose file, ``environment:`` block and all, so a single call can put
+every secret of a stack into all four places at once.
+
+One filter, not a list of exceptions: every MCP the model can reach is
+built into a ``FastMCP`` by :mod:`beaver_gateway.mcp.internal_app` —
+``python_tool`` bundles like komodo, stdio subprocesses, remote HTTP
+servers — and every route into one of them runs its middleware chain.
+That covers the per-namespace ``/mcp/<name>/`` mounts claude-code talks
+to, the ``/mcp/all`` bundle, the external MCP frontend reverse-proxying
+into both, and the Raycast backend's direct ``call_tool``. A new MCP in
+``config.py`` is covered the day it is added, without anyone
+remembering to list it here.
+
+What this does not reach: tools that never touch a FastMCP server — the
+gateway's own ``gateway`` tools, and everything claude-code runs inside
+its own process (``Bash``, ``Read``). Those are guarded by
+:mod:`beaver_gateway.core.policy` and the vault mounts instead.
+"""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+import mcp.types as mt
+from fastmcp.server.middleware import Middleware
+from fastmcp.tools.base import ToolResult
+
+from beaver_gateway.core.redact import redact, redact_data
+
+if TYPE_CHECKING:
+    from fastmcp.server.middleware import CallNext, MiddlewareContext
+
+__all__ = ["RedactingMiddleware"]
+
+
+def _redact_block(block: mt.ContentBlock) -> mt.ContentBlock:
+    """Mask a content block's text; binary blocks pass through."""
+    if isinstance(block, mt.TextContent):
+        masked = redact(block.text)
+        return (
+            block if masked == block.text else block.model_copy(update={"text": masked})
+        )
+    if isinstance(block, mt.EmbeddedResource) and isinstance(
+        block.resource, mt.TextResourceContents
+    ):
+        masked = redact(block.resource.text)
+        if masked == block.resource.text:
+            return block
+        resource = block.resource.model_copy(update={"text": masked})
+        return block.model_copy(update={"resource": resource})
+    return block
+
+
+class RedactingMiddleware(Middleware):
+    """Mask credentials in whatever a tool call returns.
+
+    Idempotent, so mounting a server both on its own namespace and in
+    the ``/mcp/all`` bundle costs a second pass, not a double mask.
+    """
+
+    async def on_call_tool(
+        self,
+        context: MiddlewareContext[mt.CallToolRequestParams],
+        call_next: CallNext[mt.CallToolRequestParams, ToolResult],
+    ) -> ToolResult:
+        result = await call_next(context)
+        content = [_redact_block(block) for block in result.content]
+        structured = redact_data(result.structured_content)
+        if content == result.content and structured == result.structured_content:
+            return result
+        return ToolResult(
+            content=content,
+            structured_content=structured,
+            # Masked structured content no longer has to satisfy the
+            # tool's output schema; a non-None meta takes the
+            # ``CallToolResult`` path that skips that validation, the
+            # same trick ``ResponseLimitingMiddleware`` uses.
+            meta=result.meta if result.meta is not None else {},
+        )

tests/test_mcp_redacting.py

@@ -0,0 +1,208 @@
+from __future__ import annotations
+
+import mcp.types as mt
+import pytest
+from fastmcp import FastMCP
+from fastmcp.tools.base import ToolResult
+
+from beaver_gateway.core import redact as redact_mod
+from beaver_gateway.core.redact import redact
+from beaver_gateway.mcp.internal_app import build_internal_app
+from beaver_gateway.mcp.redacting import RedactingMiddleware
+from beaver_gateway.mcp.types import McpServer
+
+# A trimmed but otherwise verbatim ``komodo deploy`` result: this is the
+# shape Komodo returns for the ``Compose Config`` stage of a
+# DeployStackService, which the komodo tool passes on to the model as
+# ``-- <stage> ok`` plus the stage's stdout. Values are substituted.
+KOMODO_DEPLOY = """DeployStackService Complete ok
+-- Service/s ok
+gateway
+-- Compose Config ok
+name: beaver-agent
+services:
+  gateway:
+    container_name: beaver-gateway
+    environment:
+      ADMIN_PASS: 0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f50
+      ADMIN_USER: admin
+      BEAVERGRAM_MCP: https://tg.example.dev/mcp?token=bg-0123456789abcdef
+      BOOTSTRAP_TOKENS: admin:bs-0123456789abcdef,komodo:hook-0123456789
+      CALENDAR_MCPS: personal=https://cal.example.com/api/mcp?icsUrl=https%3A%2F%2Fcalendar.google.com%2Fcalendar%2Fical%2Fx%2Fprivate-0123456789abcdef%2Fbasic.ics
+      CLAUDE_CODE_OAUTH_TOKEN: sk-ant-oat01-0123456789abcdef
+      CLAUDE_HOME: /home/beaver-runner
+      COMPOSE_PROFILES: db,gateway,obsidian,t3
+      DATABASE_URL: postgresql+psycopg://beaver:pg-0123456789abcdef@postgres:5432/beaver
+      KOMODO_URL: https://komod.example.dev
+      POSTGRES_DB: beaver
+      POSTGRES_PASSWORD: pg-0123456789abcdef
+      POSTGRES_USER: beaver
+    ports:
+      - 62990:62990
+-- Compose Up ok
+ Container beaver-gateway  Started
+"""
+
+SECRETS = (
+    "0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f50",
+    "bg-0123456789abcdef",
+    "bs-0123456789abcdef",
+    "hook-0123456789",
+    "private-0123456789abcdef",
+    "sk-ant-oat01-0123456789abcdef",
+    "pg-0123456789abcdef",
+)
+
+
+@pytest.fixture(autouse=True)
+def _no_env_secrets() -> None:
+    """Prove the patterns alone carry it — no literal env values loaded."""
+    redact_mod._ENV_SECRETS[:] = []  # noqa: SLF001
+
+
+def test_compose_config_dump_loses_every_value_but_keeps_every_name() -> None:
+    out = redact(KOMODO_DEPLOY)
+    for secret in SECRETS:
+        assert secret not in out, secret
+    # Names stay: the dump still says what is configured.
+    for name in (
+        "ADMIN_PASS",
+        "BOOTSTRAP_TOKENS",
+        "CALENDAR_MCPS",
+        "POSTGRES_PASSWORD",
+        # a username is half of a login pair — in prod ADMIN_USER is a
+        # 64-hex string, not "admin"
+        "ADMIN_USER",
+        "POSTGRES_USER",
+    ):
+        assert f"{name}: <…>" in out
+    # Non-secret values are untouched, so the result stays worth reading.
+    assert "POSTGRES_DB: beaver" in out
+    assert "CLAUDE_HOME: /home/beaver-runner" in out
+    assert "COMPOSE_PROFILES: db,gateway,obsidian,t3" in out
+    assert "KOMODO_URL: https://komod.example.dev" in out
+    assert "container_name: beaver-gateway" in out
+    assert "-- Compose Up ok" in out
+    assert " Container beaver-gateway  Started" in out
+
+
+def test_database_url_keeps_its_shape_and_loses_its_password() -> None:
+    # DATABASE_URL is not a secret-looking name, so the whole line is not
+    # masked; the password is caught as the userinfo of the connection
+    # string, which is what keeps the host and database readable.
+    out = redact(KOMODO_DEPLOY)
+    line = next(ln for ln in out.splitlines() if "DATABASE_URL" in ln)
+    assert "pg-0123456789abcdef" not in line
+    assert "postgres:5432/beaver" in line
+
+
+def test_redaction_is_idempotent() -> None:
+    once = redact(KOMODO_DEPLOY)
+    assert redact(once) == once
+
+
+def test_a_log_line_is_not_swallowed_by_the_assignment_rule() -> None:
+    # ``token:`` in mid-line must not eat the rest of an audit line.
+    line = "mcp: actor=token:komodo namespace=calendar POST /mcp/ -> 200"
+    assert redact(line) == line
+
+
+@pytest.mark.anyio
+async def test_middleware_masks_a_tool_result() -> None:
+    def deploy(stack: str) -> str:
+        """Deploy a stack."""
+        return f"{stack}\n{KOMODO_DEPLOY}"
+
+    server = FastMCP(name="komodo", tools=[deploy])
+    server.add_middleware(RedactingMiddleware())
+    result = await server.call_tool("deploy", {"stack": "beaver-agent"})
+    text = "\n".join(b.text for b in result.content if isinstance(b, mt.TextContent))
+    for secret in SECRETS:
+        assert secret not in text, secret
+    assert "ADMIN_PASS: <…>" in text
+
+
+@pytest.mark.anyio
+async def test_middleware_masks_structured_content() -> None:
+    def config() -> dict[str, object]:
+        """Return a config blob."""
+        return {
+            "env": {"ADMIN_PASS": "0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f50"},
+            "port": 62990,
+        }
+
+    server = FastMCP(name="cfg", tools=[config])
+    server.add_middleware(RedactingMiddleware())
+    result = await server.call_tool("config", {})
+    dumped = str(result.structured_content) + str(result.content)
+    assert "0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f50" not in dumped
+    assert "62990" in dumped
+
+
+@pytest.mark.anyio
+async def test_clean_result_passes_through_untouched() -> None:
+    def ping() -> str:
+        """Say hello."""
+        return "pong"
+
+    server = FastMCP(name="p", tools=[ping])
+    server.add_middleware(RedactingMiddleware())
+    result = await server.call_tool("ping", {})
+    assert isinstance(result, ToolResult)
+    assert result.content[0].text == "pong"  # ty: ignore[unresolved-attribute]
+
+
+@pytest.mark.anyio
+async def test_every_namespace_built_by_the_aggregator_is_filtered() -> None:
+    def deploy() -> str:
+        """Deploy."""
+        return KOMODO_DEPLOY
+
+    _app, _urls, servers = build_internal_app(
+        [McpServer.python_tool(name="komodo", tools=[deploy])],
+        host="127.0.0.1",
+        port=8765,
+    )
+    for name, server in servers.items():
+        result = await server.call_tool(
+            "deploy" if name == "komodo" else "komodo_deploy", {}
+        )
+        text = "\n".join(
+            b.text for b in result.content if isinstance(b, mt.TextContent)
+        )
+        assert "0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f50" not in text, name
+
+
+@pytest.mark.anyio
+async def test_gateway_own_tools_are_filtered_too() -> None:
+    # These bypass the FastMCP middleware — the SDK mounts them
+    # in-process — so they carry their own wrapper.
+    from claude_agent_sdk import SdkMcpTool
+
+    from beaver_gateway.core.gateway_tools import _redacting
+
+    async def handler(_args: dict[str, object]) -> dict[str, object]:
+        return {"content": [{"type": "text", "text": KOMODO_DEPLOY}]}
+
+    spec = _redacting(
+        SdkMcpTool(
+            name="read_conversation", description="", input_schema={}, handler=handler
+        )
+    )
+    out = str(await spec.handler({}))
+    for secret in SECRETS:
+        assert secret not in out, secret
+
+
+def test_source_code_the_dispatcher_reads_is_not_mangled() -> None:
+    # t3code hands back files and diffs. A SCREAMING_SNAKE constant is
+    # an assignment too, but ``NAME = expr`` is code, not a config dump.
+    source = (
+        'SESSION_SECRET = os.environ["SESSION_SECRET"]\nAPI_KEY_HEADER = "X-Api-Key"'
+    )
+    assert redact(source) == source
+
+
+def test_env_file_lines_are_still_masked() -> None:
+    out = redact("ADMIN_PASS=0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f50\nPOSTGRES_DB=beaver")
+    assert out == "ADMIN_PASS=<…>\nPOSTGRES_DB=beaver"