Skip to content

Subagent tool calls bypass the gate entirely (blocks #296) #297

Description

@CrazyWillBear

Found reviewing #296. Pre-existing, not introduced by that PR — but #296 is blocked on it, because that PR's docs promise gate.ask_when "outranks every auto-approve", which is false one layer out.

The problem

The gate is a wrapper around the registry, not a property of it. app.py:86 wraps the shared ToolRegistry in GatedTools for each session, and that wrapper is where never / approved / read_only / cards / announce / audit all live.

The subagent path gets a handle on the same registry unwrapped:

# subagents.py:120 — `registry` is the raw ToolRegistry
tools=FilteredTools(registry, definition.tools),

FilteredTools.dispatch checks its allowlist, then calls self._inner.dispatch(...) straight through.

caller path gated?
main session GatedTools → registry yes
subagent FilteredTools → registry no

So a subagent tool call raises no card, emits no announce line, and writes no audit row. This skips the whole gate, gate.never included — a tool the owner listed under never is callable from a subagent.

It defaults open, too. subagents.py:25:

tools: tuple[str, ...] | None  # None = every tool (minus spawn)

with _parse doing tuple(tools) if tools else None. An agent definition whose frontmatter simply omits tools: reaches every registered tool, ungated. That's the default for a definition written without thinking about it, not a hostile-input case.

Why it blocks #296

#296 adds gate.ask_when, which cards an approved tool when a named argument is present — motivated by hound's smart_fetch, where the same call that reads a page also clicks and submits when it carries actions. That control is real for the main session and absent for subagents, and docs/CONFIG.md as written doesn't say so.

Design (worked out, not just proposed)

Wrap the subagent's dispatcher in the same gate, bound to the parent's ToolContext.

  1. Make spawn_agent context-aware — register it with wants_context=True so the handler receives the parent's ToolContext (registry.py:81-82 already injects it).

  2. Generalize app.py's tools_factory to take the inner dispatcher instead of hardcoding registry=registry, then pass it into register_spawn_tool.

  3. In spawn_agent, build the sub-session's dispatcher as:

    tools=gated(context, FilteredTools(registry, definition.tools))

    Gate outermost, filter inside — that ordering matters. GatedTools.specs() then sees the filtered set, so its unknown-tool branch returns the plain error for a disallowed tool instead of raising a card for something the subagent could never call anyway.

Settled by inspection

  • No deadlock. ApprovalBroker's docstring is explicit: the dispatcher offers every inbound message to resolve() before starting a turn, so an answer lands even while the thread's session is mid-turn. A parent blocked inside spawn_agent, whose subagent is awaiting a card on the parent's thread, is exactly the case that already works.
  • No self-collision on the one-card-per-thread rule. ApprovalBroker.ask denies if a card is already pending on that thread_key, but loop.py:89-102 dispatches tool calls sequentially (for call in ...: await tools.dispatch(call)), so a parent can only have one subagent in flight at a time, and subagents can't spawn further subagents.
  • Which surface? The parent's thread and channel — it's the owner's live surface, and it's what ToolContext already carries.

Open — decide during implementation

  • Card text should name the calling agent. subagent 'researcher': approve tool call … — otherwise the owner is approving something they didn't initiate, with no clue where it came from. Same for the announce line.
  • What "always" means from inside a subagent. It would persist globally through gate.allow_alwaysgate_approved.json, same as anywhere. That's consistent with "no strange exceptions" and probably right, but it means a tap inside a subagent silently widens the main session's permissions — worth a deliberate call.
  • Audit attribution. Rows would carry the parent's thread_key; add the agent name so subagent calls are distinguishable after the fact.
  • Whether tools: should keep defaulting to every tool. Much less severe once the gate applies, but still a surprising default.

Acceptance criteria

  • A subagent tool call raises a card on the parent's thread when the policy says ASK.
  • gate.never denies a subagent call.
  • gate.ask_when cards a subagent call carrying a watched argument (the feat(gate): argument-conditional approval cards (gate.ask_when) #296 case).
  • read_only still auto-approves for subagents.
  • A tool absent from the definition's allowlist returns the not-allowed error and raises no card.
  • Subagent calls appear in the audit log, attributable to the agent that made them.
  • A regression test for the parent-blocked-on-subagent-card path (the deadlock that isn't).
  • docs/SECURITY.md decision-order section updated to describe the subagent path.

Key files

  • src/chief/subagents.py:95-127register_spawn_tool / spawn_agent; FilteredTools at :58-78; the default-open tools field at :25.
  • src/chief/app.py:80-91tools_factory, the only place GatedTools is constructed.
  • src/chief/tools/registry.py:28-38, 68-89Tool.wants_context and the injection point.
  • src/chief/approvals.py:45-92 — the broker; note the one-pending-card-per-thread rule.
  • src/chief/agent/loop.py:89-102 — sequential tool dispatch (why one subagent at a time).
  • docs/SECURITY.md:32-46 — the authoritative decision-order doc.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions