Give agents only their granted built-in tools and no account connectors

CHANGELOG.md

@@ -80,6 +80,20 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   and exits non-zero when the config cannot be read.
 - Configs written by `bonfire scan` on 1.0.1 are migrated on load: a
   legacy `[bonfire.persona]` table is relocated to `bonfire.profile`.
+- The SDK backend no longer passes a custom `ToolPolicy` grant verbatim
+  as the presence list (`tools=`). The presence list now carries base
+  built-in names (`Bash(git status)` becomes `Bash`, `mcp__*` entries
+  are left out, an entry with no name before `(` is left out),
+  de-duplicated in first-seen order, while the approval list
+  (`allowed_tools=`) keeps the grant verbatim. Why: a permission
+  pattern in the presence list was observed, in a manual check against
+  the bundled CLI, to remove that tool from the agent. That CLI
+  behaviour is not exercised by this repo's test suite; the unit tests
+  pin the derivation only. This is a behaviour change: duplicate names
+  in a grant no longer reach the presence list.
+  Dispatched agents also no longer see the user's claude.ai account
+  connectors: the backend sets `ENABLE_CLAUDEAI_MCP_SERVERS=false` for the
+  agent, after a manual check saw a connector reach a read-only agent.
 
 ### Changed
 

file-budget.json

@@ -145,12 +145,12 @@
     }
   },
   "packages": {
-    "src/bonfire/dispatch": 2883,
+    "src/bonfire/dispatch": 2905,
     "src/bonfire/engine": 1981,
     "src/bonfire/handlers": 3280,
     "src/bonfire/onboard": 4460,
     "tests/integration": 1702,
-    "tests/unit": 74187
+    "tests/unit": 74319
   },
   "package_raises": [
     {
@@ -248,6 +248,22 @@
       "lines": 164,
       "reason": "The 164 lines are tests/unit/test_sdk_backend_stream_closing.py, the contract for the stream-closing helper above, written and observed failing before the helper existed. It pins the same stream back, closing on early break for generators and for non-generator streams with aclose(), exception propagation, exactly one close on full consumption, pass-through for streams without aclose(), and the backend call site itself (a patched query() on the rate-limit return must be closed exactly once). Three mutants of the helper were killed by it: bare iteration at the call site, gating on the generator class instead of the capability, and a double close. Measured by cf-file-budget on the final tree: 'package tests/unit is 74187 lines (frozen 74023)'. Raised to exactly the measurement, with no headroom.",
       "approver": "Anta gates via PR merge -- raise surfaced in the PR body and in the lane report, not taken silently"
+    },
+    {
+      "package": "src/bonfire/dispatch",
+      "from": 2883,
+      "to": 2905,
+      "lines": 22,
+      "reason": "The 22 lines are, first, 18 for _builtin_tools in sdk_backend.py (a one-line mirror comment, the 15-line function and its 2 blank separators; the Sequence import joined an existing line and costs none): the derivation of the SDK presence list (tools=) from a ToolPolicy grant; and 4 for the agent environment that switches off the user's claude.ai account connectors (a one-line comment, the _AGENT_ENV constant, its blank separator and the env= kwarg), after a live check saw a Claude Docs and a Google Drive connector reach a read-only agent in 2 of 9 runs. The presence list used to be the grant verbatim, and a permission pattern such as Bash(git status) placed there was seen, in a manual check against the bundled CLI, to remove that tool from the agent, so a custom ToolPolicy written with patterns lost its tools silently. The helper has to live at the dispatch layer, beside the one ClaudeAgentOptions call that consumes it; the private lineage keeps the identical derivation at the same path and the two change together. The docstring was cut from 17 lines to 6 before this raise was measured. The loop body could fold into about two statements with dict.fromkeys and a comprehension, saving about six lines; rejected because the explicit loop states each rule of the contract on its own line. Explicitly rejected: a purpose entry for sdk_backend.py, which would drop the whole module out of the ratchet to accommodate 18 lines, the same trade the two earlier raises on this package rejected. Measured by cf-file-budget on the final tree: 'package src/bonfire/dispatch is 2905 lines (frozen 2883)'. Raised to exactly the measurement, with no headroom.",
+      "approver": "Anta gates via PR merge -- raise surfaced in the PR body and in the lane report, not taken silently"
+    },
+    {
+      "package": "tests/unit",
+      "from": 74187,
+      "to": 74319,
+      "lines": 132,
+      "reason": "The 132 lines are tests/unit/test_sdk_backend_presence_base_names.py (143 lines: 122 for the presence-list contract and 21 for the connector contract, one driving test and one pin that the backend sets no mcp_servers or strict_mcp_config), less 14 net lines removed from test_sdk_backend_tool_presence.py when its old verbatim-duplicate pin was replaced, plus a 3-line history breadcrumb in test_no_persona_names_in_public_docs.py, whose CHANGELOG line anchors moved by 14 when this rung's Fixed entry was written (anchors only, expected text unchanged, as every earlier CHANGELOG addition did). Four tests, all four of which fail on origin/main: a pattern grant reduces to its base name while the approval list keeps the pattern, an mcp__ grant stays out of the presence list but in the approval list, and a pattern-only grant keeps the tool present rather than producing an empty list. The fourth replaces a pin whose contract changed on purpose: duplicates are now removed from the presence list and still reach the approval list verbatim. The tests moved into their own module because the derivation is its own contract and the presence module would otherwise have crossed the 500-line cap. Explicitly rejected: an unbudgeted tree such as tests/dispatch, which dodges the ratchet and hides the contract from whoever maintains sdk_backend.py; and a purpose entry for the new file, an exemption in everything but name. Measured by cf-file-budget on the final tree: 'package tests/unit is 74319 lines (frozen 74187)'. Raised to exactly the measurement, with no headroom.",
+      "approver": "Anta gates via PR merge -- raise surfaced in the PR body and in the lane report, not taken silently"
     }
   ]
 }

src/bonfire/dispatch/sdk_backend.py

@@ -26,7 +26,7 @@
 from bonfire.models.envelope import Envelope, ErrorDetail
 
 if TYPE_CHECKING:
-    from collections.abc import AsyncIterator, Callable
+    from collections.abc import AsyncIterator, Callable, Sequence
 
     from bonfire.events.bus import EventBus
     from bonfire.protocols import DispatchOptions
@@ -51,6 +51,9 @@
 
 logger = logging.getLogger(__name__)
 
+# A logged-in CLI attaches the user's claude.ai account connectors; no grant declares them.
+_AGENT_ENV: dict[str, str] = {"ENABLE_CLAUDEAI_MCP_SERVERS": "false"}
+
 
 # ---------------------------------------------------------------------------
 # Traceback redaction
@@ -144,6 +147,24 @@ def _resolve_setting_sources(cwd: str | None) -> list[str]:
     return []
 
 
+# Mirror: the private v1 lineage keeps this derivation at the same path; change both together.
+def _builtin_tools(declared: Sequence[str]) -> list[str]:
+    """Derive the PRESENCE-layer ``tools=`` list: base name before the first ``(``.
+
+    Skips ``mcp__`` entries and empty bases, de-duplicates first-seen, returns a
+    fresh list; whitespace and case are untouched. Why: a permission pattern in the
+    presence list was seen, in a manual check against the bundled CLI, to drop the tool.
+    """
+    result: list[str] = []
+    for entry in declared:
+        if entry.startswith("mcp__"):
+            continue
+        base = entry.split("(", 1)[0]
+        if base and base not in result:
+            result.append(base)
+    return result
+
+
 def _map_thinking(depth: str) -> tuple[dict[str, Any], str]:
     """Map ``thinking_depth`` to SDK thinking config and effort level."""
     mapping: dict[str, tuple[dict[str, Any], str]] = {
@@ -225,7 +246,7 @@ async def _do_execute(  # noqa: C901
             max_budget_usd=options.max_budget_usd,
             cwd=options.cwd or None,
             permission_mode=options.permission_mode,
-            tools=list(options.tools),
+            tools=_builtin_tools(options.tools),
             allowed_tools=options.tools,
             hooks=_build_security_hooks_dict(
                 options.security_hooks,
@@ -235,6 +256,7 @@ async def _do_execute(  # noqa: C901
             setting_sources=_resolve_setting_sources(options.cwd),
             thinking=thinking_config,
             effort=effort_level,
+            env={**_AGENT_ENV},
             stderr=lambda line: logger.warning("[CLI stderr] %s", line),
         )
 

tests/unit/test_no_persona_names_in_public_docs.py

@@ -192,24 +192,27 @@
         # Doc-truth pass: shifted 615/617/618/635 -> 662/664/665/682 when
         # the CHANGELOG entries owed by three merged PRs were written
         # under [Unreleased]. Anchors only; expected text is unchanged.
+        # Presence-list fix entry: shifted 684/686/687/704 -> 698/700/701/718
+        # when a Fixed entry was added under [Unreleased] (11 lines, then 3 more
+        # for the connector switch). Anchors only; expected text is unchanged.
         (
             "CHANGELOG.md",
-            684,
+            698,
             "predecessor persona (Passelewe, the Chamberlain) was retired; the",
         ),
         (
             "CHANGELOG.md",
-            686,
+            700,
             "`docs/_lore/passelewe.md`. The persona builtins directory",
         ),
         (
             "CHANGELOG.md",
-            687,
+            701,
             "`src/bonfire/persona/builtins/passelewe/` was deleted; a new",
         ),
         (
             "CHANGELOG.md",
-            704,
+            718,
             'to ban `"passelewe"` in src/ (the predecessor persona is gone, so',
         ),
         # --- CLAUDE.md — constellation-pointer breadcrumbs --------------

tests/unit/test_sdk_backend_presence_base_names.py

@@ -0,0 +1,143 @@
+# SPDX-License-Identifier: Apache-2.0
+# Copyright 2026 BonfireAI
+
+"""Contract: the SDK backend's PRESENCE list (``tools=``) holds base built-in names.
+
+PRESENCE = base names (text before the first ``(``), no ``mcp__`` entries, no
+empty names, de-duplicated in first-seen order. APPROVAL (``allowed_tools=``)
+keeps the caller's grant verbatim.
+"""
+
+from __future__ import annotations
+
+from typing import Any
+from unittest.mock import patch
+
+from bonfire.dispatch.sdk_backend import ClaudeSDKBackend
+from bonfire.models.envelope import Envelope
+from bonfire.protocols import DispatchOptions
+
+
+def _make_capture() -> tuple[dict[str, Any], type]:
+    """Return ``(captured_kwargs_dict, FakeClaudeAgentOptions)`` pair."""
+    captured: dict[str, Any] = {}
+
+    class _FakeClaudeAgentOptions:
+        """Mimics SDK's ``ClaudeAgentOptions`` — captures every kwarg."""
+
+        def __init__(self, **kwargs: Any) -> None:
+            captured.update(kwargs)
+            for k, v in kwargs.items():
+                setattr(self, k, v)
+
+    return captured, _FakeClaudeAgentOptions
+
+
+async def _empty_query(*, prompt: str = "", options: Any = None):  # type: ignore[no-untyped-def]
+    """Async-gen that yields nothing — closes immediately."""
+    if False:  # pragma: no cover
+        yield None
+
+
+def _envelope(agent: str = "warrior-agent") -> Envelope:
+    return Envelope(task="do work", agent_name=agent, model="claude-opus-4-7")
+
+
+# ===========================================================================
+# PRESENCE layer holds BUILT-IN BASE names only
+# ===========================================================================
+#
+# The CLI's ``tools=`` flag accepts only built-in tool names. A permission
+# pattern such as ``Bash(git status)`` passed there silently DROPS the tool
+# (measured against the bundled CLI), and ``mcp__*`` names are not governed by
+# that flag at all. The PRESENCE layer therefore derives base names; the
+# APPROVAL layer keeps the caller's grant exactly.
+
+
+async def _capture_for(tools: list[str]) -> dict[str, Any]:
+    """Run one execute() with ``tools`` and return the captured option kwargs."""
+    captured, FakeOptions = _make_capture()
+    with (
+        patch("bonfire.dispatch.sdk_backend.ClaudeAgentOptions", FakeOptions),
+        patch("bonfire.dispatch.sdk_backend.query", _empty_query),
+    ):
+        backend = ClaudeSDKBackend()
+        await backend.execute(_envelope(), options=DispatchOptions(tools=tools))
+    return captured
+
+
+class TestPresenceLayerBaseNames:
+    """``tools=`` = de-duplicated built-in base names; ``allowed_tools=`` = verbatim grant."""
+
+    async def test_permission_patterns_reduce_to_base_names(self) -> None:
+        """``Bash(...)`` patterns become ``Bash`` once; approval keeps all patterns."""
+        grant = ["Read", "Bash(git diff:*)", "Bash(pytest:*)", "Edit"]
+        captured = await _capture_for(list(grant))
+
+        assert list(captured["tools"]) == ["Read", "Bash", "Edit"], (
+            f"from {len(grant)} granted entries expected 3 base names; got {captured['tools']!r}"
+        )
+        assert list(captured["allowed_tools"]) == grant
+
+    async def test_mcp_entries_excluded_from_presence_kept_in_approval(self) -> None:
+        """``mcp__*`` names are not built-ins: absent from ``tools=``, kept in approval."""
+        grant = ["Read", "mcp__lexicon__memory_read"]
+        captured = await _capture_for(list(grant))
+
+        assert list(captured["tools"]) == ["Read"], (
+            f"from {len(grant)} granted entries expected only 'Read'; got {captured['tools']!r}"
+        )
+        assert list(captured["allowed_tools"]) == grant
+
+    async def test_pattern_only_custom_policy_grant_keeps_the_tool_present(self) -> None:
+        """A policy granting only a pattern must not silently empty the PRESENCE layer."""
+        grant = ["Bash(git status)"]
+        captured = await _capture_for(list(grant))
+
+        assert list(captured["tools"]) == ["Bash"], (
+            f"pattern-only grant of {len(grant)} entry must expose 'Bash', "
+            f"not drop it; got {captured['tools']!r}"
+        )
+        assert list(captured["allowed_tools"]) == grant
+
+    async def test_duplicate_tool_names_dedup_in_presence_kept_in_approval(self) -> None:
+        """PRESENCE de-duplicates in first-seen order; APPROVAL keeps duplicates."""
+        # Old pin (test_duplicate_tool_names_propagate_verbatim): duplicates reached
+        # tools= verbatim. Changed because tools= now holds de-duplicated base names.
+        captured, FakeOptions = _make_capture()
+        incoming = ["Read", "Read", "Write", "Read"]
+
+        with (
+            patch("bonfire.dispatch.sdk_backend.ClaudeAgentOptions", FakeOptions),
+            patch("bonfire.dispatch.sdk_backend.query", _empty_query),
+        ):
+            backend = ClaudeSDKBackend()
+            options = DispatchOptions(tools=incoming)
+            await backend.execute(_envelope(), options=options)
+
+        assert list(captured["tools"]) == ["Read", "Write"], (
+            f"tools= must be de-duplicated in first-seen order from {len(incoming)} entries; "
+            f"got {captured['tools']!r}"
+        )
+        assert list(captured["allowed_tools"]) == incoming
+
+
+class TestAccountConnectorsExcluded:
+    """No grant declares the user's claude.ai account connectors; the CLI must not load them."""
+
+    async def test_every_grant_disables_claude_ai_connectors(self) -> None:
+        grants = [["Read", "Write", "Bash"], ["Read", "Grep"], []]
+        seen: list[Any] = []
+        for grant in grants:
+            env = (await _capture_for(list(grant))).get("env") or {}
+            seen.append(env.get("ENABLE_CLAUDEAI_MCP_SERVERS"))
+        assert len(seen) == len(grants) == 3, f"ran {len(seen)} of {len(grants)} grant cases"
+        assert seen == ["false"] * len(grants), (
+            f"ENABLE_CLAUDEAI_MCP_SERVERS per grant (of {len(grants)}) must be 'false'; "
+            f"got {seen!r}"
+        )
+
+    async def test_backend_adds_no_mcp_servers_and_no_strict_mcp_config(self) -> None:
+        captured = await _capture_for(["Read", "Grep"])
+        leaked = sorted({"mcp_servers", "strict_mcp_config"} & captured.keys())
+        assert leaked == [], f"of 2 forbidden kwargs, backend passed {leaked!r}"

tests/unit/test_sdk_backend_tool_presence.py

@@ -22,7 +22,7 @@
 
 Knight-A adversarial tests elevated to mandatory:
     fresh-list identity across repeat calls; adversarial tool content
-    (whitespace, unicode, duplicates, large lists, case preservation);
+    (whitespace, unicode, large lists, case preservation);
     existing-kwargs-preserved (model, max_turns, setting_sources, stderr);
     empty-tools kill-switch semantics.
 """
@@ -387,20 +387,6 @@ async def test_unicode_tool_names_propagate(self) -> None:
 
         assert list(captured["tools"]) == ["读取", "Write"]
 
-    async def test_duplicate_tool_names_propagate_verbatim(self) -> None:
-        """Duplicates in incoming list are NOT deduplicated by sdk_backend."""
-        captured, FakeOptions = _make_capture()
-
-        with (
-            patch("bonfire.dispatch.sdk_backend.ClaudeAgentOptions", FakeOptions),
-            patch("bonfire.dispatch.sdk_backend.query", _empty_query),
-        ):
-            backend = ClaudeSDKBackend()
-            options = DispatchOptions(tools=["Read", "Read", "Write"])
-            await backend.execute(_envelope(), options=options)
-
-        assert list(captured["tools"]) == ["Read", "Read", "Write"]
-
     async def test_very_large_tool_list_propagates(self) -> None:
         """100-tool list propagates without truncation."""
         captured, FakeOptions = _make_capture()