fix(security): key the spawn auto-approve rung on event identity (#6506)

[CrysisDeu]

Summary

Fixes #6506.

The auto_approve_subagent_spawn rung keyed solely on the model-authored event title, so any tool whose title read spawn_run — including a SHELL command the model re-titled — rode the rung and was auto-approved without a human prompt.

The rung now keys on canonical, non-model-authored identity through one shared predicate, hooks.event_is_spawn_run, used by every surface (messaging driver/dispatch, slack native + transport, discord, telegram). All four conjuncts must hold:

• event.tool_name == "spawn_run" — the canonical name from _meta.kiro.toolName, cached per toolCallId, never model-authored;
• event.mcp_identity_trusted — the provenance flag set only when both canonical-identity cache reads hit, so a future inline population path fails closed;
• event.mcp_server_name == CORE_MCP_SERVER (session_directive.CORE_MCP_SERVER) — a foreign MCP server or a built-in that merely names a tool spawn_run cannot ride the rung;
• event.title == "spawn_run" — not as identity (the title is forgeable and never sufficient), but because the channel PreToolUse gate keys deny rules on the title: a rephrased-title genuine spawn falls to the normal approval ladder instead of approving past a title-keyed deny. This keeps the rung's approval surface exactly its pre-fix shape, minus the forgeries.

There is deliberately no title fallback: an event without canonical identity (backend that doesn't emit _meta.kiro, or the correlated provenance-cache miss) simply does not ride the rung and falls to the channel's normal approval ladder (session trust / YOLO / interactive) — a downgrade, never a hard block. Genuine spawn_run MCP calls stay auto-approved, so unattended fan-out remains unblocked.

The three inline title-only replicas (discord, telegram, and the lambda previously duplicated in slack transport) are deleted in favor of the shared build_auto_approve / event_is_spawn_run, and a structural test pins that no module re-inlines a title == "spawn_run" predicate.

Review adoptions

• Pre-push blind review (Opus + GPT, both adversarial): provenance flag required instead of name non-emptiness; CORE_MCP_SERVER pin; stale predicate signature in docs/system-specs/modules/messaging.md; over-broad structural assertion narrowed.
• CI round 1 (GPT blocking, FP advisory): removed the title-only tier-2 fallback entirely (a no-meta non-shell tool could otherwise still ride the rung by title); reuse session_directive.CORE_MCP_SERVER instead of a duplicated constant.
• CI round 2 (GPT blocking): restored the title conjunct alongside the canonical checks — the channel tool gate keys deny rules on the title, so a canonical-only rung could approve a re-titled spawn past a title-keyed deny. The underlying channel-gate parity gap (deny plane never sees canonical identity on channel surfaces, unlike the dashboard) is filed as Channel tool gates omit canonical mcp identity from on_tool_call, so title-keyed deny/governance rules can be bypassed by re-titling #6938.
• CI round 3 (First Principles blocking): dropped the undeclared rider — the whole-file black reformat of hooks.py/test_hooks.py and their baseline prune are reverted (the gate only demanded test_slack_handler_coverage.py, which stays formatted); deleted a stale title-floor clause from build_auto_approve's docstring.

Testing

• test/test_hooks.py — TestEventIsSpawnRun (all four conjuncts individually load-bearing, fail-closed floors, the GPT-lane send_file-forgery and rephrased-title cases) + TestShouldAutoApproveSpawn on event doubles.
• test/test_messaging_driver.py::TestAutoApproveTool — driver passes the EVENT (not the title) to the predicate; forged shell titled spawn_run prompts; genuine spawn auto-approves.
• test/test_name_grant_surfaces.py::TestSpawnRungEventIdentity — per-surface pins (slack/discord/telegram/messaging) + structural pin that no module re-inlines a title-only predicate.
• Full suite: 75k+ passed locally; the only failures are pre-existing host-environment issues (xdist budget, AF_UNIX path length) with zero in touched areas. isort/flake8/mypy/black gates green. 5 targeted mutants all killed.

🤖 Kiro Crew Auto-Pipeline [operator: CrysisDeu#0c98c3a2]

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of c98c6fa3fa0c7e85aa158a5f0ffaf0b0533c527e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The event attributes (mcp_identity_trusted, tool_name, mcp_server_name) are real fields on AcpEvent populated from the per-toolCallId cache in acp/_dispatch.py, and CORE_MCP_SERVER is the existing constant — the predicate's foundation checks out. The spec was updated in the same commit, the three inline title-only replicas are deleted in favor of one shared predicate, and the residual deny-plane parity gap is filed as #6938.

Design-Verdict: PASS

Root-cause fix: the rung now keys on non-forgeable canonical identity at one shared seam, fail-closed, with the residual deny-parity gap explicitly filed (#6938).

Suggestions

• The updated messaging.md line says the predicate keys on "canonical identity only" and omits the title == "spawn_run" conjunct — but that conjunct is load-bearing (deny-plane parity until Channel tool gates omit canonical mcp identity from on_tool_call, so title-keyed deny/governance rules can be bypassed by re-titling #6938); document it in the spec so a future editor doesn't "simplify" it away.

[DESIGN-REVIEWED] c98c6fa

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of c98c6fa3fa0c7e85aa158a5f0ffaf0b0533c527e and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] c98c6fa

False positive or not applicable? A repository writer can comment:
/ai-review override gpt c98c6fa3fa0c7e85aa158a5f0ffaf0b0533c527e: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed c98c6fa3fa0c7e85aa158a5f0ffaf0b0533c527e — this comment is updated in place on each push.

Review details

The change is a security hardening that keys the spawn auto-approve rung on canonical event identity (tool_name + mcp_server_name == CORE_MCP_SERVER + mcp_identity_trusted) instead of the model-authored title. I verified: the AcpEvent type genuinely carries these fields (acp/types.py:542-561), the same canonical-identity pattern is already established in child_mcp_identity_trusted (acp/types.py:654-693), CORE_MCP_SERVER == "kirocrew-core" matches the test doubles, and the predicate fails closed on every attribute miss. No regression to genuine spawn auto-approval, and no grounded defect I can add at the required bar.

No findings.

[OPUS-REVIEWED] c98c6fa

Verdict parsed from the review's SHA-scoped output markers for commit c98c6fa3fa0c7e85aa158a5f0ffaf0b0533c527e.

False positive or not applicable? A repository writer can comment:
/ai-review override fable c98c6fa3fa0c7e85aa158a5f0ffaf0b0533c527e: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of c98c6fa3fa0c7e85aa158a5f0ffaf0b0533c527e — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

The fix earns its place at cause level, but slack keeps a private copy of the predicate the description says was deleted into the shared builder.

What this change ships

Intent: stop a model-forged spawn_run title from riding the spawn auto-approve rung on messaging channels — a FIX.

1. Forged-title shell/tool no longer auto-approves on any channel — justified (the fix, untrusted model content).
2. Genuine spawn with a rephrased title now prompts instead of auto-approving — justified, declared (title-keyed deny plane, gap filed Channel tool gates omit canonical mcp identity from on_tool_call, so title-keyed deny/governance rules can be bypassed by re-titling #6938).
3. Spawn on a backend without _meta.kiro falls to the ladder — justified fail-closed floor, declared.
4. Foreign MCP server's spawn_run refused — justified (CORE_MCP_SERVER pin).
5. Discord/telegram inline replicas replaced by shared build_auto_approve — justified dedup (3 consumers: dispatch.py:634, discord:608, telegram:789).
6. New hooks.event_is_spawn_run — justified, 2 consumers (dispatch.py:317, slack/handler.py:178).
7. Slack keeps its own _should_auto_approve_spawn wrapper — duplicate of messaging/dispatch.build_auto_approve.
8. Structural test pins against re-inlined title predicates — justified (guards the fix's invariant).
9. messaging.md predicate signature updated — mandated (spec-same-commit rule).

Watch

• Description says the slack-transport lambda was "deleted in favor of the shared build_auto_approve", but slack/transport_dispatch.py:596 still routes through slack-local _should_auto_approve_spawn (slack/handler.py:167), whose body is now identical to build_auto_approve's inner function (grep _should_auto_approve_spawn: 2 non-test consumers, both slack). Two spellings of one rung will diverge; the identity core is shared, so risk is confined to the hook-flag wrapper.
• Root cause one level down — the channel deny plane keys on the forgeable title — is left, declared, and filed as Channel tool gates omit canonical mcp identity from on_tool_call, so title-keyed deny/governance rules can be bypassed by re-titling #6938; the title conjunct here is the interim shim for it.

Subtractions

• Delete _should_auto_approve_spawn (slack/handler.py:167); use build_auto_approve(context_builder) at transport_dispatch.py:596 and build_auto_approve(context_builder)(event) at handler.py:3539 — import direction slack→messaging is the sanctioned one per messaging/dispatch.py's module docstring.

[FIRST-PRINCIPLES-REVIEWED] c98c6fa

[bolichen97]

Readiness passed on the current head, all checks green, mergeable against main, and no outstanding change requests from any reviewer. Approving as part of a maintainer sweep of fix-type PRs.