fix(cron): confine channel agents from scheduling a privileged agent (#8371)

[chenmingwei23]

What is the problem?

cron_add and cron_update let a contained caller schedule a durable job that runs as a MORE privileged agent than the caller itself. Both take an agent (cron_update maps it onto agent_id on an existing job) and pass it straight to add_job / update_job with no check comparing the requested agent against the caller's confinement:

job = svc.add_job(..., agent_id=agent or "", approval_mode=approval_mode or "", ...)

Why this matters to the user

A channel agent's reach is deliberately bounded to channel posts -- CHANNEL_AGENT_BLOCKED_TOOLS already holds back send_message, send_notification, and every session_* verb for exactly that reason. Neither cron verb was covered anywhere. So a channel agent could:

cron_add(name="...", agent="kirocrew", approval_mode="auto", every=...)

and have a full-tool agent (shell, filesystem, everything the named agent carries) run on the gateway host on a timer, with its own restrictions nowhere in the picture. It is durable (outlives the turn and the channel conversation), and cron_update is a second door -- it re-targets an existing job's agent_id without creating anything.

How the fix solves it -- symptom to root cause

The root cause is that these verbs run agent-chosen work outside the channel, which is the exact shape CHANNEL_AGENT_BLOCKED_TOOLS exists to hold back. The fix denies both verbs to a channel agent in mcp_cron (_deny_channel_agent_cron), keyed on the STRICT, gateway-vouched caller session (_authz_session_key, which refuses forgeable sources), applied as the first check in both the cron_add and cron_update branches. It mirrors the existing mcp_core._deny_channel_agent_messaging.

The placement is the whole point, and it corrects the first revision of this PR. The first revision added cron_add / cron_update to CHANNEL_AGENT_BLOCKED_TOOLS. That list is consulted only in channel.py's permission-request gate -- but an @kirocrew-cron/cron_add entry in a channel agent's allowedTools is translated to a KAS auto-approve permission (acp/kas_permissions.py; MCP tools are not in WITHHELD_FROM_AUTO_APPROVE), and auto-approval is the absence of a permission request, so the list guard never runs. GPT 5.6's blocking review caught this and it is correct -- a named entry in that list is not a boundary for an auto-approved call. Moving the denial to MCP dispatch, on the verified identity, closes the auto-approve path. (The list approach also broke the neighbouring non-shell-tool contract in test_channel_trusted_patterns.py, which uses cron_add as its canonical non-shell example -- another reason the list was the wrong home.)

Only the channel: orchestrator-agent namespace is confined. A slack: / discord: session is an allow-listed HUMAN participant scheduling their own recurring work, and a dashboard session scheduling a job for a different crew is the legitimate flow the issue is careful not to break -- neither consults this gate, so both keep working.

Scope note (from review): the in-code / earlier claim that cron_update can re-point "a job an operator already created and reviewed" is narrowed. _check_cron_job_ownership restricts updates to jobs keyed to the caller's own session, so the real reach is the channel agent's OWN-session jobs -- still a named channel-boundary harm (it re-targets the agent those jobs run as), so both verbs are confined, but the threat is own-session, not operator-owned.

Out of scope (noted per the issue): the audit-trail half -- created_by is unwritten by mcp_cron and later runs are attributed to cron:<job_id> rather than the authoring principal.

Known limitation -- descendant propagation (filed separately). _deny_channel_agent_cron keys on the IMMEDIATE caller's session (_authz_session_key), so it closes a channel agent calling cron_add / cron_update directly. It does NOT propagate the channel agent's confinement to a session it spawns: spawn_run is not in CHANNEL_AGENT_BLOCKED_TOOLS, and a spawned subagent's key is subagent:<id> (not channel:<...>), so a channel agent that spawns a subagent which then calls cron_add evades this guard. Closing that requires threading channel-origin identity through the MCP caller context (mcp_cron is a separate stdio process and cannot see the spawn chain -- it has no reference to the subagent manager or parent_session_key), and the identical exposure exists for the mirrored mcp_core._deny_channel_agent_messaging, so it is a systemic channel-confinement-propagation gap rather than a cron-specific one. Tracked as a separate follow-up issue; deliberately not attempted here.

What tests we did

Added to test/test_mcp_cron_caller_identity.py (the existing home for caller-identity-gated cron behavior, which already drives _call_tool_inner with a strict caller block):

• A channel agent's cron_add is refused and mints no job.
• A channel agent's cron_update is refused before the ownership check.
• The denial holds at _call_tool_inner -- the dispatch path a call reaches AFTER approval -- proving it does not depend on a permission event (the auto-approve bypass), and it denies even a plain self-scoped schedule with no agent override (all-or-nothing, matching send_* / session_*).
• A slack: human session may still schedule cron (legitimate flow preserved).
• A dashboard: session may schedule a job for a different crew (the flow the issue protects).

Mutation-verified: neutering _deny_channel_agent_cron to a no-op turns 3 of these red while the "still allowed" tests stay green. All 37 tests in the file pass (32 pre-existing caller-identity tests unaffected). The two test_channel_trusted_patterns.py failures from the first revision are resolved (that file is untouched now). Ran targeted only, -n0.

Lint run independently, all clean on both touched files: black --target-version py310, isort, flake8, mypy (mcp_cron.py). Black baseline gate passes with no baseline change (the first revision's baseline-shrink was an artifact of touching a baselined file, now reverted).

Pattern harvest

Rule candidate: When confining a tool that an existing block-list already governs, verify WHERE that list is enforced before adding to it. CHANNEL_AGENT_BLOCKED_TOOLS is checked only at channel.py's permission-request event; an MCP tool named in an agent's allowedTools is KAS auto-approved and fires no such event, so the list never sees it. A capability confinement for an MCP tool must live at MCP dispatch on the verified caller identity (the _deny_channel_agent_* pattern), not on a permission-event list -- a named entry in a list is not a boundary until something refuses the call on the path the call actually takes.

Closes #8371

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 20ebc0e

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

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

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 20ebc0eab32a9030ab0352ccdbc2f5fbf5d52a0c — 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.

All checks complete. The fix is well-derived and correctly placed; the one real finding is that the new deny helper is a near-verbatim second copy of an existing one it explicitly "mirrors". Final review:

First-Principles-Verdict: CONCERNS

_deny_channel_agent_cron re-spells mcp_core._deny_channel_agent_messaging line-for-line instead of reusing it — two copies of one security boundary will diverge.

What this change ships

Intent: stop a confined channel agent from scheduling durable cron work that runs as a more privileged agent (issue #8371). This is a FIX.

1. A channel agent's cron_add is refused and creates no job — justified
2. A channel agent's cron_update is refused before the ownership check — justified
3. Even a self-scoped, no-agent schedule is refused (all-or-nothing) — declared, derived: any cron job runs outside channel confinement
4. Each refusal writes a rejected_blocked_tool SEL audit record — justified, part of the mirrored pattern
5. slack:/discord: humans and dashboard: cross-crew scheduling unchanged — justified, preserved flows

The placement claim checks out: CHANNEL_AGENT_BLOCKED_TOOLS is consulted only at channel.py:853's permission-request handler, and kas_permissions.py:26 documents that auto-approval fires no such event — so the existing list genuinely cannot do this job, and dispatch-level denial is not a duplicate of it.

Watch

• The helper is a second spelling: same prefix check, same best-effort SEL write, same error shape as mcp_core._deny_channel_agent_messaging (mcp_core.py:906; grepped _deny_channel_agent — 2 helpers, 2 consumers each), and mcp_cron.py:52 already imports from mcp_core. The description says "mirrors", meaning copied, not reused.
• Grepped if name == "cron_" — 9 verbs, 2 confined. cron_trigger/cron_resume can still run/re-enable an own-session job a channel agent created before this fix; ownership-gated and no agent arg, so a shrinking legacy window, but it is the same "durable work outside the channel" root cause.

Subtractions

• Delete the ~50-line _deny_channel_agent_cron body; parameterize mcp_core._deny_channel_agent_messaging with tool_kind and refusal text and call it from both cron branches — one boundary, one implementation.

[FIRST-PRINCIPLES-REVIEWED] 20ebc0e

[github-actions]

GPT 5.6 Review — 🔴 changes requested (blocking)

GPT 5.6 found at least one blocking issue that must be resolved before merging 20ebc0eab32a9030ab0352ccdbc2f5fbf5d52a0c. 1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

This comment is updated in place on each push.

BLOCKING -- src/kiro_crew/mcp_cron.py:1633 -- Descendants bypass channel-agent cron confinement
if not caller_session.startswith("channel:"): return None
Channel agent → spawn_run → subagent:<id> → cron_add passes this guard → durable privileged job.
Anchor: residual/security
Fix: Propagate verified channel origin to descendants and reject them here, or deny channel-agent spawn_run at dispatch.
[BLOCK-MERGE] 20ebc0e
[GPT-REVIEWED] 20ebc0e

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

I've verified the chain. Findings summary:

The subagent session key is subagent:<id> (session.py:438, dashboard/ws.py:237), which does not start with channel:, so the guard at mcp_cron.py:1633 returns None for a subagent. spawn_run is not in CHANNEL_AGENT_BLOCKED_TOOLS (channel.py:61-69), so a channel agent can spawn, and the spawn body (spawn.py:645) carries no channel-origin marker — the confinement is not propagated to the child. A subagent runs with full tool access, so it can call cron_add, which flows agent/approval_mode straight to add_job. The condition combination is direct and ordinary (spawn → subagent cron_add), not extreme, contradictory, or legacy-only; I cannot complete a rarity record that would let a human accept the residual risk. Unbounded harm (privilege escalation past channel confinement) → UPHOLD-FENCED.

[ADJUDICATION] 20ebc0eab32a9030ab0352ccdbc2f5fbf5d52a0c total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] 20ebc0eab32a9030ab0352ccdbc2f5fbf5d52a0c
[ADJUDICATION-FENCED] 20ebc0eab32a9030ab0352ccdbc2f5fbf5d52a0c fenced=1 flagged=0
UPHOLD-FENCED F1 src/kiro_crew/mcp_cron.py:1633 -- A channel agent can spawn a subagent (spawn_run is not blocked and carries no channel-origin marker), and the child's `subagent:` session key evades the `channel:`-prefix guard, letting it schedule a durable privileged cron job — a direct, non-extreme bypass of the confinement.
[GPT-ADJUDICATED-FENCED] 20ebc0eab32a9030ab0352ccdbc2f5fbf5d52a0c

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

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Right boundary, right layer: the deny sits at MCP dispatch on the gateway-vouched identity, exactly where the auto-approve path can't skip it.

Suggestions

• The guard is a hand-inserted branch prefix in each verb (chan_err = _deny_channel_agent_cron("cron_add") repeated per verb); checking once at _call_tool_inner entry against a named frozenset of confined verbs would make the next escalation-shaped cron verb a one-line set addition instead of a remembered edit — the same class of miss this PR is fixing.

[DESIGN-REVIEWED] 20ebc0e