fix(dashboard): refuse unknown-slot approval-mode requests before any global mutation by c020627 · Pull Request #4472 · kirodotdev/KiroCrew

c020627 · 2026-08-19T04:48:57Z

Problem / Motivation

POST /api/chat/mode (src/kiro_crew/dashboard/chat_handlers.py) carried three defects in the ordering between slot validation and global mutation:

A trust_reads request naming a slot that does not resolve silently fell through to the all-slots path, widening a slot-scoped request across every slot on the gateway. Its trust/normal siblings answer 400 unknown slot.
The unknown-slot 400 sat after safety_override().deactivate(), so a refused request ({"mode": "normal", "slot": " "}) still revoked the process-global grant -- a rejected request mutated global state.
deactivate() -- which emits a SEL event -- ran inline on the gateway loop, while the sibling activate() is offloaded with asyncio.to_thread.

Why it matters

The unknown-slot cases are exactly the malformed or stale requests an automation sends (e.g. a session created against a slot that was since deleted). Today those requests either widen a slot-scoped intent across every slot (defect 1 -- the trust_reads path that other modes explicitly refuse) or withdraw a global grant the operator relies on (defect 2), while the blocked-deactivate inline run puts SEL I/O on the gateway event loop (defect 3). None of the three has a good outcome for the operator; each is a "rejected input should be a no-op" violation.

What changed (motivation -> approach -> change)

Root cause: validation was interleaved with mutation -- trust_reads never validated, and trust/normal validated only after the global grant was already revoked. The fix is to make "slot does not resolve" a refusal that happens before any mutation, for every slot-scoped branch, and to stop re-indexing state._slots[slot_key] after an await (no check/use gap for a concurrent slot deletion).
Approach: the slot key is resolved once, inline at the top of api_chat_mode, and an unresolvable key returns 400 before touching grant or slots. Branches mutate the captured slot reference instead of re-indexing the dict. yolo is global and keeps ignoring slot (a stale key must not refuse it), preserving upstream behavior.
Non-string slots refused: body.get("slot") or None previously collapsed falsy non-strings ([], {}, 0, False) into the documented all-slots request -- so {"mode": "trust", "slot": []} trusted every slot (the same widening defect as api_chat_mode: trust_reads widens on an unknown slot, rejected requests still mutate, deactivate blocks the loop #4454 through a normalization back door). The raw value is now type-checked before that normalization: any present non-string slot answers 400 unknown slot, while yolo still ignores slot entirely.
Change: src/kiro_crew/dashboard/chat_handlers.py -- hoist validation ahead of the conditional deactivate(); refuse present non-string slots on the raw value; offload deactivate() with await asyncio.to_thread(...) matching the sibling activate(). A slot-scoped trust/trust_reads leaves the process-global grant alone (a per-slot request cannot be answered by withdrawing authority elsewhere); a grant declared in owner-only config is exempt from the narrowing, since selecting another approval mode is the one documented action that ends it.

This branch is rebased onto the current main. The branch bodies adopt main's effective_session_key() shared-session propagation (the policy is per session, the flag is per slot, so a grant/revoke clears every slot sharing the session) while keeping this PR's up-front validation; main's now-redundant late unknown-slot checks inside the trust/normal branches are dropped in favor of the single up-front refusal. Per the First Principles review: error-code-baseline.json is reverted to base (the diff removes two uncoded unknown slot responses and adds two -- net zero -- so no regeneration is needed), and the single-consumer _resolve_mode_slot() helper is inlined at its call site.

Tests

test/test_chat_mode_security.py -- 16 tests:

Rejected trust_reads/trust/normal on an unknown slot answer 400 unknown slot and never call deactivate.
No slot flags change after a refusal; a rejected request leaves an active grant live.
Falsy non-string slots ([], {}, 0, False) on trust/trust_reads answer 400 and do not widen (addresses the GPT 5.6 review back-door finding).
Named-slot requests affect only the named slot and set the approval policy for that session's key.
Absent slot keys keep the documented all-slots meaning; a non-string slot key is refused.
Named-slot trust/trust_reads do not revoke an active grant (the fix(dashboard): stop a per-slot mode change revoking the global YOLO grant #4416 x api_chat_mode: trust_reads widens on an unknown slot, rejected requests still mutate, deactivate blocks the loop #4454 interplay).
deactivate() runs off the event loop.

Regression -- test_trust_reads.py, test_channel_trust_revoke.py, test_dashboard_chat.py (785 passed; one pre-existing TestIsReadOnlyBash failure reproduces on origin/main and is unrelated to this diff). test/test_error_code_contract.py passes with the baseline at base.

Manual verification

N/A -- unit coverage sufficient: the change is confined to one handler whose every branch is exercised directly by the new contract tests, including the SEL event path.

Screenshots / video

N/A -- no user-visible UI change.

Related Issues

Closes #4454

Checklist

Single commit with a Conventional Commits title (fix(dashboard): ...)
Existing tests pass and new tests added for new functionality
Self-review completed; code follows project style guidelines
Documentation updated (if applicable) -- docs/system-specs/modules/security.md TTL table corrected
No secrets, credentials, or internal references in the diff

iamwhatever · 2026-08-19T04:51:02Z

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

## Problem / Motivation
## Why it matters
## What changed
## Tests