fix: verify name-grant program identity on all auto-approve surfaces (#6361)

[CrysisDeu]

Summary

The name-grant check (src/kiro_crew/name_grant.py) refuses to honour a name-based shell auto-approve when a program name no longer resolves to the program it appears to name — a PATH-shadowing shim (~/.local/bin/head over /usr/bin/head), a resolution inside an agent-writable tree, or a file no approval ever identified. It was wired into the dashboard chat loop only. Every other surface honoured the same hooks.on_tool_call TOOL_AUTO_APPROVE grant verbatim — and those are the unattended surfaces, where a shadowed name is the cheaper attack path, not the rarer one.

This PR makes the check surface-agnostic and wires every honour point:

• name_grant.py gains the public async entry points refusal_for_command_off_loop (the ONE off-loop dispatch — worker thread for the filesystem work, the constant Windows decline answered on-loop) and refusal_for_event (duck-typed on is_shell/shell_command; returns None for non-shell/commandless events). refusal_for_event never raises (cancellation excepted): an unexpected check failure answers as an UNINSPECTABLE refusal, so a provider event loop is never wedged with an unanswered ACP permission.
• Dashboard (dashboard/chat_runner.py) — behavior identical: _name_grant_refusal_off_loop becomes a module alias of the promoted helper (the seam its tests stub), _name_grant_refusal_for stays a thin wrapper (now carrying the same decline-not-raise guard), _audit_name_grant_refusal delegates to the shared writer with a byte-identical SEL row.
• Task runner (task_executor.py) — the hook auto-approve branch verifies before honouring; a refusal downgrades to the interactive prompt, or the pre-existing headless deny-by-default.
• Subagents (subagent.py) — same, downgrading to the remaining rungs (parent policy, interactive factory/fallback, headless reject).
• Channel turns (messaging/driver.py) — verification happens at TurnDriver's gate "auto_approve" honour point, the one place shared by every channel's _tool_gate (messaging pipeline + the Slack/Discord/Telegram transports). The gates stay synchronous and check-free (they run on the loop); the honour point is already async, so no gate contract change was needed. The driver gains caller-injected audit_session_key/audit_agent (audit-only), wired at all four construction sites.
• Native Slack handler (slack/handler.py) — same shape as the dashboard rung.
• llm_helpers._resolve_permission (cron / autonudge / heartbeat turns) — verified only when the caller passed an interactive approver (that is the downgrade target); without one the helper's fall-through approves by default, so a decline would change nothing while writing a misleading audit row.

Every decline is audited through ONE shared writer, name_grant.log_decline (outcome=auto_approve_declined, reason=name_grant + refusal code + tier, error=Refusal.log_text — a table constant — and a credential/exfiltration-redacted title; never Refusal.detail, which names resolved paths). Callers pass their module-level sel binding (sel_factory, required) so each module's audit test seam observes the row.

Closes #6361

Deliberate decisions

• A refusal DOWNGRADES, never hard-blocks. Each surface falls to exactly what happens today when no grant matches. On headless surfaces (standalone kirocrew run, subagents with no approver) that path is the pre-existing deny-by-default — so a shadowed/project-local (.venv/bin) program that used to be auto-approved is now rejected there. This is the issue's intent: unattended is precisely where a planted shim would be honoured with nobody watching. Called out in the shipped doc (src/kiro_crew/docs/task-runner.md) including the two audit rows (name_grant decline, then headless_no_authorization reject).
• Windows declines every name-based shell grant (the check models neither cmd.exe search order nor POSIX tokenization — the fail-closed answer the original dashboard wiring already established). On the new unattended surfaces this means shell auto-approve is effectively off on Windows; documented in task-runner.md, subagent.md, messaging.md, heartbeat.md, messaging-transport.md.
• messaging/dispatch.py's _tool_gate is unchanged (no sync-gate decline, no async conversion): the honour point in TurnDriver is already async and covers all four channel gates at once. Loop-safety pins keep the gates check-free.
• Full-trust rungs stay unchanged (YOLO, per-session Trust, parent_policy=auto, run_auto_approve, APPROVAL_TRUST_READS kind-rung, the spawn_run title predicate): none is a name-based shell grant. The trust-reads/kind-rung boundary is documented in messaging.md and pinned by test.
• security_posture.py: name_grant.py registered in NON_EGRESS_REDACTION_MODULES (audit-side log hygiene, not an egress boundary) per that registry's drift guard.

Pre-push adversarial review

4 rounds, two blind model-pinned lanes per round (GPT + Opus mirrors), ~20 findings fixed with pinning tests — among them: redacted decline audits with disclosure assertions on all surfaces, the shared audit writer replacing five hand-copied rows, the never-raise guard (a check exception previously orphaned the ACP permission), the llm_helpers cron/autonudge honour point the first exemption rationale missed, driver audit attribution, a test order-dependence on the process-global trusted-session set, and doc corrections (subagent cascade YOLO-snapshot, sel outcome enum, heartbeat interaction).

Dispositioned (documented, not changed here):

• auto_approve_subagent_spawn title predicate: an event titled spawn_run is approved by that rung today regardless of hook verdicts or name grants — pre-existing, not widened by this PR; hardening it means changing the title-only predicate signature across four channels + the native Slack helper (follow-up issue to be filed).
• Channel users see no decline reason (the dashboard shows a 🛡️ notice; channels just get the ordinary prompt). Refusal.detail is human-facing and could ride the approval prompt — follow-up UX.
• Slack /stats counters don't count declines (no other outcome-specific counter exists for this shape).
• task_planner's decomposition phase approves every non-denied permission by design (like _resolve_permission without an approver) — not a name-based grant; the shipped task-runner doc's verification claim is scoped to step execution.

Testing

• test/test_name_grant_surfaces.py (new): per-surface downgrade/clean/audit regression tests for the task runner, subagents, TurnDriver (incl. trust-reads boundary + audit identity), native Slack, and stream_and_collect; promoted-entry-point unit tests; decline-not-raise guard tests (shared helper + dashboard wrapper); platform-independent loop-safety pins (every surface routes through the one off-loop entry point; all four channel gates stay check-free) living outside the POSIX-skipped module.
• test/test_name_grant.py: loop-safety pins updated to the promoted entry point; all existing fix(security): stop a name-based auto-approve from honouring a shadowed program name #4920 verdict/wiring tests pass unchanged.
• Local gates green: isort, flake8, mypy (1131 files), diff-scoped black gate, brand gate, harness-parity gate, subprocess-encoding gate.
• Full suite: 71,686 passed; the 94 failures + 2 errors are host-environment and byte-identical on a pristine base-commit worktree (proven by diffing the failure sets).

no linked issue screenshots: backend security change, no UI surface touched.

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound consolidation, but the _resolve_permission surface contradicts the PR's own stated design and adds a reject next to an approve-by-default open gate.

Watch

• Description/tests/code disagree on the headless _resolve_permission decision. The PR says the check runs "only when the caller passed an interactive approver… a decline would change nothing while writing a misleading audit row," and the test-class docstring repeats it — but the shipped code verifies unconditionally and rejects headless (name_grant_headless_reject). Whichever is intended, the recorded rationale is now the opposite of the behavior; humans reviewing unattended-cron regressions will reason from the wrong invariant.
• That reject also breaks the PR's own downgrade rule and buys little. On this surface the "normal path when no grant matches" is the caller-less default auto-approve, so a hook-granted shadowed name is rejected while any command the hook never granted sails through the fall-through unverified — the fence has an open gate beside it, and legitimate venv-resolving cron/heartbeat commands newly fail where an ungranted equivalent would run.
• The fix re-instantiates the failure shape it fixes. The original bug was one-surface wiring drift; the remedy is six hand-wired honour points pinned only by per-surface tests. Nothing structural stops honour point fix(sandbox): bypass toolbox shim to fix nested sandbox failure on macOS 26 #7 from honouring TOOL_AUTO_APPROVE unverified.

Suggestions

• Add a scrub-lint-style CI guard (like check_harness_parity.py) failing a newly added TOOL_AUTO_APPROVE honour site with no adjacent refusal_for_event — that closes the drift class instead of the current instances.
• Reconcile the _resolve_permission headless decision with the surface's real fall-through: either verify the default-approve path too, or restore the documented "no approver → no check" and fix the description/docstring.

[DESIGN-REVIEWED] 23cca72

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 23cca72e3e3314437a0b9f08478517fcebc4ebbf — 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 counts verified: every TOOL_AUTO_APPROVE honour point in src/ is now either verified or explicitly dispositioned (cli_chat's is a deny-ceiling that still asks), the new entry points have 6+ real consumers each, and the slack/gateway.py reads-rung change appears nowhere in the description. Final review follows.

First-Principles-Verdict: CONCERNS

One verified surface — the gateway --approval reads rung — ships undeclared; everything else is the declared fix at the right level.

What this change ships

Intent: stop a PATH-shadowed or agent-writable program from riding a name-based shell auto-approve on unattended surfaces (#6361) — a FIX.

1. Task-runner shell auto-approve now verified; refusal falls to prompt or headless reject — justified
2. Subagent shell auto-approve verified; headless refusal rejects — justified
3. All four channel turns verified once, at the driver's honour point — justified
4. Native Slack handler verified, same downgrade — justified
5. Cron/autonudge/heartbeat turns verified; approver-less refusal now rejects instead of approving — justified
6. Gateway --approval reads rung verified (tier=cli_approval_reads) — undeclared
7. Declines audited via one shared writer, new SEL outcome auto_approve_declined — justified
8. Windows: name-based shell auto-approve effectively off on unattended surfaces — justified, declared
9. TurnDriver gains caller-injected audit identity, wired at 4 sites — justified
10. subagent.md approval cascade corrected (YOLO snapshot, hook rung) — rides along, declared

Watch

• The slack/gateway.py reads-mode verification is absent from the description's surface list and its testing list, yet it changes what a cron/autonudge turn auto-approves. I grepped every TOOL_AUTO_APPROVE honour point (10 in src/): all others are declared or dispositioned; this one is real behavior a human should see declared, not discover.
• The check sits at mechanism level by necessity — it cannot live in the sync, loop-bound hooks.on_tool_call — and I count zero unverified name-based honour points left, so the per-surface wiring is the general fix, not a point patch.

Subtractions

• Fold the six near-identical logger.warning("declining a ...auto-approve: ...") blocks (driver.py, subagent.py, task_executor.py, slack/handler.py, slack/gateway.py, llm_helpers.py) into name_grant.log_decline, which already receives the refusal and tier; the per-surface tail phrase repeats what source/tier record.

[FIRST-PRINCIPLES-REVIEWED] 23cca72

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 23cca72

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

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 23cca72

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

[CrysisDeu]

ai-review-disposition — First Principles lane

Finding: the decline-not-raise guard shipped as two hand-maintained copies, leaving the dashboard's trusted-pattern and trust-reads tiers unguarded.

Disposition: accepted — the Subtraction is adopted as proposed, in head 8edc7c58f:

• The try/except is hoisted into name_grant.refusal_for_command_off_loop, the chokepoint every tier on every surface reaches. One copy replaces two.
• The duplicated guard body in chat_runner._name_grant_refusal_for is deleted; the wrapper is a pure pre-filter again.
• The two previously-unguarded dashboard tiers (trusted_pattern, trust_reads) call the module alias directly and therefore inherit the guard with zero new code — exactly the mechanism the finding named.
• Tests updated to exercise the real chokepoint (the inner sync check is made to raise; both refusal_for_event and the dashboard seam are pinned to answer UNINSPECTABLE instead of propagating).

One consequence worth stating: a raising STUB installed at a rung seam no longer gets a second-chance catch in the wrapper — the guard protects failures inside the real check, which is the failure the invariant is about.

[CrysisDeu]

CI status — blocked on a main-side breakage, not this PR

Rebased onto current main (16239989d, head now 3d0777df3). Two reds remain, both inherited from main and reproduced on a pristine 16239989d worktree (this PR's diff touches neither file):

1. Backend Tests (3.10, 3) / (Windows, 3) — test/test_session_storage.py::TestCotenantRefusalTextIsForgeSafe::test_move_to_trash_refusal_error_escapes_the_name raises TypeError: <lambda>() got an unexpected keyword argument 'cached'. Commit #6453 gave cotenant_sids a cached= kwarg and routed move_to_trash → _scan_units → _scan_raw through the cotenant_sids(cached=cached) call site (session_storage.py:446), but the test's monkeypatched cotenant_sids lambda still takes no args. Fails identically on a clean 16239989d checkout.
2. Frontend Lint & Type Check — eslint … --max-warnings 664 exceeds the cap on main's own website/src warnings. This PR is backend-only; no website/src file is touched.

All of this PR's own owned checks are green (GPT/Opus/Design/First-Principles AI lanes settled, the four earlier CI fix rounds resolved). Per the pipeline's main-inheritance policy I am NOT patching main's test on this branch. Holding on a low-frequency watch of main's own CI; will rebase and re-verify once main is green again.

[bolichen97]

Re-approving after conflict-resolution rebase: code rebased clean; three doc conflicts resolved editorially (main's newer structure kept, PR's name-grant verification prose woven in; single commit, 22 files unchanged in scope).