[parked] feat(channels): govern which chat connections may attach, and control it

[chenmingwei23]

1. What is the problem?

A credential was the entire admission decision for a chat connection. Putting a
bot token into config.json and enabling the transport was enough: the gateway
connected whatever it found configured, and the only thing that could refuse was
an enterprise channels policy that most installs do not have. The effective
default was any bot holding a token may attach.

Three consequences followed:

• No enrolment. Nothing recorded that an operator ever approved a given bot,
  so there was no answer to "who decided this principal may talk to my agent?".
• Sender admission lived in config.json. allowed_user_ids (and weixin's
  dm_policy) sit in a file the agent can rewrite, are snapshotted by the
  transport at construction, and produce no per-decision audit record.
• The dashboard said nothing, and offered nothing. A session driven by a bot
  looked like any other tab, and there was no product surface for deciding which
  bot may attach.

The governance model already had the right slot: channels is a ScopedMap with
members and a policy-only posture. It was addressed per TRANSPORT, though, so
a rule could only speak about a protocol, never about one of the credentials
speaking it.

2. Why this issue matters to the user

An instance is increasingly shared — a team attaches several bots to one
KiroCrew. Under a transport-grained ceiling those bots are indistinguishable, so
"the ops bot may not write files" cannot be said without also saying it about the
owner's personal bot. And because attachment was default-open, a stale credential
left in config keeps a live inbound path into the operator's agent, with nothing
in the product acknowledging it exists or letting anyone close it.

3. How our fix solves it

Symptom — any bot with a token attaches; nothing says what bounds it and
nothing lets you change it.

Root cause — the unit of governance was the transport, and there was no
operator-owned record of which connections may attach at all.

Chain:

1. Give the credential an identity. messaging/connections.py makes a
   connection the governed principal, always fully qualified
   (telegram/default), so a per-bot rule is reachable and a transport that
   later grows a second connection must come through this gate.
2. Keep every existing policy meaning what it meant. A channel matcher
   makes a bare-transport pattern cover every connection on that transport —
   same shape as the existing @server / @server/tool rule for MCP.
3. Add the missing admission decision. messaging/trust.py holds an
   operator-owned roster; a connection not on it does not connect. The roster is
   a trust root (_SENSITIVE_HOME_DIRS), so the agent can neither read the list
   of principals allowed to talk to it nor add one of its own.
4. Make enrolment enforceable without breaking upgrades. Absent or malformed
   reads as tampering and admits NOTHING — that is what stops default-open from
   being restored by deleting one file. A permissive roster is seeded ONCE from
   the already-configured channels, so an install whose channels worked before
   keeps working.
5. Move sender admission under the ceiling. The inbound gate asks three
   fail-closed questions — enrolment, member, sender. The sender leaf reads the
   policy-only channels.posture.<connection>.senders and intersects with
   the config allowlist, so it can only narrow. Enrolment is re-checked per
   message, so a revoke bites on the next message rather than the next restart.
6. Give the operator the control, not just the verdict. POST /api/connections/enrol / /revoke edit the keystone roster exactly the way
   Settings > Security edits denied_commands.json — shared config lock, atomic
   0600 write, blocking work offloaded, one SEL record per attempt, refreshed
   snapshot returned. The control sits on the Channels card because that is where
   an operator already goes to make a bot work.

Two switches, because they have different owners

Switch	Owner	Liftable by the running app?
messaging.connection_governance (config)	operator	yes — config / overlay / CLI
capabilities.channel_connections (trust-root policy)	fleet	no

A fleet whose per-surface scoping will arrive as crew members pins the capability
off and gets the pre-roster behaviour back, rather than half-adopting a surface it
plans to replace. Either switch being off makes the feature inert: no roster is
consulted, none is seeded (the file's presence is the intended-open signal),
and every surface renders nothing.

The switch itself is deliberately not fail-closed, unlike every gate behind
it. A gate refusing on doubt costs one blocked message; this refusing on doubt
would stop every channel on a host that never opted out — an unrelated governance
hiccup becoming a total outage.

Other deliberate choices

• No new profile concept and no new bind type. Per-connection control rides
  the existing channels ScopedMap, so nothing has to be unpicked when
  per-surface scoping moves to crew members.
• Enrolment is not part of the ceiling. The roster answers "may this attach
  at all" (the operator's question); the ceiling answers "what may it then do"
  (the enterprise's). Refusals are audited distinguishably —
  rule=trust-roster / layer=operator versus a policy layer — so a reader can
  tell "nobody approved this bot" from "the ceiling refused it".
• Every surface self-hides when nothing is bounded, so an install with no
  policy sees the pages unchanged.
• A transient evaluation failure is never rendered as an admin deny, and an
  unreadable roster gets its own wording everywhere — and offers no write, since
  the API refuses to overwrite a corrupt roster rather than discard it.

Two changes worth a reviewer's attention

• The posture query separator moves from / to #. A member id now contains
  /, so splitting on it would read one connection's posture rule as the whole
  transport's. One production call site (slack/enterprise.py) and one
  conformance vector are updated; two new vectors cover container resolution.
• Blast radius of the enrolment gate: if the seed fails, every channel goes
  offline. The failure is loud and the log names the marker to delete so the next
  start re-seeds. That is the intended direction for a tampered trust root, but it
  is a real operational edge and should be reviewed as one.

4. What tests we did

Backend — 61 new tests, plus the existing suites for every touched module:

• test_channel_trust.py (16): roster load/seed semantics, fail-closed on
  absent / unreadable / wrong-version, one malformed entry not disabling the
  rest, a named connection not admitted by its sibling, the start gate, and the
  trust-root fencing.
• test_channel_trust_api.py (13): enrol takes effect with no restart, idempotent
  enrol/revoke, revoke matching the terse spelling too, a corrupt roster left
  byte-for-byte, and the written file being 0600 and still agent-fenced.
• test_channel_connection_switch.py (13): the config switch, the policy pin, a
  profile-layer denial NOT counting as a pin, disabled being fully inert
  (no gate, no seed, no surface), and the switch itself not fail-closing.
• test_channel_inbound_governance.py (10): the three questions, revoking without
  a restart, a caller that knows no sender not being failed on a leaf it cannot
  answer, and the audit attribution of each refusal shape.
• test_connections_endpoint.py (9): the read model, three-state verdict,
  roster-unreadable reporting, per-connection pinning.
• test_governance_policy.py: the # separator vector updated, plus two new
  vectors proving a transport-authored posture still reaches a qualified
  connection and that a specific posture key beats a container one.
• conftest.py seeds a roster for every test so the default test posture matches
  the default runtime posture (a fresh tmp home has none, which turned 16
  pre-existing chokepoint tests red on the tamper path).

Gates, all green locally: isort, flake8, mypy (865 files), tsc,
npm run i18n:check 13/13, and vitest.

Verified end-to-end on an isolated pod, not just in unit tests. The enrol
button was clicked against a real gateway: the card flipped from "not allowed to
attach yet" to "is allowed to attach", the button became Revoke, and the
connection dropped out of the Security page's bounded list — proving the write
reached the roster and both surfaces read the same truth.

Known failures, characterised against a clean base

Two things fail on this host, and both fail on a clean main checkout too (same
machine, PYTHONPATH forcing base sources so the comparison is base code + base
tests, not branch code + base tests):

• Full backend suite: 59 failures are shared with base (git-backed ledger
  sync, gh, systemd). 15 were branch-only and 8 base-only, but 14 of those 15
  come from the same test classes as the base-only 8 — one flake family sampling
  differently per run. Exactly one was genuinely mine
  (test_messaging_identity.py::test_inbound_governed_deny_is_sel_audited, the
  audited item becoming qualified) and it is fixed.
• Full vitest: ContextBreakdownPanel.test.tsx hits the 15s per-test timeout.
  It does the same on base, and passes in isolation every time (15/15).

5. Any other suggestions on the work

• Capability narrowing per connection is deliberately NOT here. "This bot may
  not write files / may not spawn" needs a Level-2 profile, and that scoping
  belongs to a crew member. Building it now would build the thing that has to be
  migrated.
• Three outbound chokepoints still query the bare transport
  (mcp_core send_message, chat_runner mirroring,
  chat_compaction_notice). Nothing is broken — a transport-level rule still
  covers every connection through the channel matcher — but a rule naming one
  connection is enforced at attach and inbound and not on those paths. Worth
  closing before multi-connection transports land; it is a four-site change.
• Multi-connection transports are the natural follow-up. The model, the
  ceiling grammar, the API and all four surfaces already handle N connections;
  what remains is per-transport credential config plus teaching
  link.channel_namespace_of and sel._infer_source the telegram.<name>
  surface segment. Landing this first means such a transport cannot return
  without an admission decision.
• sel._infer_source mis-attributes a dotted surface segment. A key like
  telegram.ops-bot:… matches no namespace and falls to the trailing Slack
  default. Not fixed here on purpose: nothing on main produces that shape, so
  the fix belongs with whatever reintroduces named connections rather than as
  dead code.
• config-baseline.json is 19 fields stale on main. This PR adds only its own
  entry rather than regenerating, to keep unrelated churn out of the diff.

[chenmingwei23]

Do not merge yet — held deliberately.

Kept as a draft (a draft cannot be merged, which is the only self-lock GitHub allows an author). Outstanding before this is ready:

• full backend suite still running
• pod-e2e screenshots not yet captured or reviewed
• the enrolment gate fails closed on a tampered/absent trust roster, so its blast radius is "every channel goes offline" — that needs a deliberate human read, not a green-checks merge

I will mark it ready and say so here when those clear.

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

Advisory UX-level review of d1eabccf4b2dcf0e1ab5968a866fa11ca97749db — updated in place on each push; does not block merge.

UX-Verdict: CONCERNS

The governance card makes promises the form doesn't keep: "the sender field below is not editable" while the field stays fully editable.

Watch

• senders_pinned copy contradicts the rendered form. "Allowed senders are pinned by the security policy, so the sender field below is not editable" — but no diff hunk disables allowed_user_ids in BotChannelPanel.tsx (grep: senders_pinned is read only by the three new components). Every pinned-policy operator hits it, every visit; they either think the form is broken or edit a field the card just declared dead. Fix: gate the ChipsInput's disabled on senders_pinned, or soften the string to "policy narrows whatever you enter here."
• Enrol button promises attachment it can't deliver when policy denies. ChannelGovernanceCard headline checks !connection.enrolled before permitted === false, so a policy-denied, unenrolled bot shows "Enrol it to let it connect" — user clicks, succeeds, and only then reads "enrolling it will not let it attach." Rare state, but a broken label promise on a security control. Fix: check permitted === false first.
• The card vanishes on fetch error (if (isError … ) return null). The surface exists to explain "credentialed but silently not attaching" — and disappears silently in exactly that scenario, leaving the ordinary form with no hint an admission gate exists. Fix: render a one-line "connection status unavailable" instead of null.
• ConnectionChip popover (role="dialog") has no Escape, outside-click dismiss, or focus move — only re-clicking the chip closes it; a keyboard user opens a panel over the chat header and Escape does nothing. Fix: Escape + outside-click close, matching Radix popover behavior used elsewhere.

Suggestions

• connectionChip.read_only_note: "the ceiling in the security policy" is internal governance jargon — say "and its limits in the security policy."
• ConnectionGovernanceList "Decided by" renders the raw layer enum (policy/both/default) in mono — map to localized labels like the sibling verdict strings.

[UX-REVIEWED] d1eabcc

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

I've now traced all the security-sensitive paths: the matcher change (identifier→channel), the _query_level posture separator move (/→#), the fail-closed roster, the seed ordering, the inbound gate, the feature kill-switch, the keystone fencing, and the frontend consumers.

A few things I verified and cleared:

• The channels posture-separator move to # is consistent: channels is the only SCOPEDMAP scope, and its only two posture-query call sites (enterprise.py, identity.py) were both updated; policy-document keys are separate JSON keys, unaffected.
• _match_channel correctly makes a bare-transport pattern cover <transport>/default in both allow and deny mode, and _resolve_posture container resolution finds a transport-authored posture for a qualified member.
• The fail-closed roster + one-shot marker-guarded seed is deliberate (flagged in the PR), loud, and correct; seeding runs off-loop before the first admission decision.
• The keystone fencing (channel_trust.json on _SENSITIVE_HOME_DIRS) is present and tested.
• The feature_enabled switch is deliberately fail-open (with each downstream gate fail-closed), which is the correct choice to avoid a governance hiccup taking every channel offline.

One observation I considered and dropped: the new sender/connection params on channel_inbound_permitted are wired through identity.py but no inbound dispatch caller actually passes a sender, so the channels.posture.<connection>.senders leaf is never exercised on a real message. However, making it functional requires editing caller files not touched by this PR (telegram/, discord/, slack/ dispatch, messaging/dispatch.py), so the fix is out of this bot's scope per the FIX BAR — and the PR's own "deliberately NOT here" framing suggests per-connection wiring is a later step. Dropped.

No findings.

[OPUS-REVIEWED] d1eabcc

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

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

[github-actions]

GPT 5.6 Review — 🔴 changes requested (blocking)

GPT 5.6 found at least one blocking issue that must be resolved before merging d1eabccf4b2dcf0e1ab5968a866fa11ca97749db.

This comment is updated in place on each push.

BLOCKING -- src/kiro_crew/messaging/identity.py:124 -- sender policy is skipped by every inbound caller
if permitted and sender and governance_on:
Policy-denied sender -> existing caller omits sender -> posture check is skipped -> message reaches the agent.
Fix: Revert the sender-posture hunk until inbound callers provide the authenticated sender.

BLOCKING -- src/kiro_crew/dashboard/handlers/channel_trust.py:84 -- mutation overwrites malformed roster data
if not isinstance(data, dict):
Malformed connections value -> enrol/revoke request -> value is replaced with a new list -> original roster data is lost.
Fix: Reject unsupported versions and non-list connections before returning data.

BLOCKING -- src/kiro_crew/messaging/trust.py:249 -- first-run roster write is non-atomic
path.write_text(json.dumps(body, indent=2) + "\n", encoding="utf-8")
Process interruption during seeding -> partial roster remains -> next startup records the seed marker -> all channels remain offline.
Fix: Write the roster through the existing atomic JSON writer before creating the marker.

FINDING -- src/kiro_crew/dashboard/handlers/channel_trust.py:43 -- "from kiro_crew import sel as _pkg" begins added function-local imports across this file and other changed backend modules, violating top-level-imports -> Fix: move them to module scope, retaining only documented circular or optional imports.

[BLOCK-MERGE] d1eabcc
[GPT-REVIEWED] d1eabcc
False positive or not applicable? A repository writer can comment:
/ai-review override gpt d1eabccf4b2dcf0e1ab5968a866fa11ca97749db: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Advisory design-level review of d1eabccf4b2dcf0e1ab5968a866fa11ca97749db — updated in place on each push; does not block merge.

Design-Verdict: CONCERNS

Enrolment design is sound, but the claimed sender-admission gate ships unwired — no production caller ever passes a sender — while the UI advertises it.

Watch

• The sender leg is dormant. The description claims "the inbound gate asks three fail-closed questions — enrolment, member, sender" and that sender admission moved "under the ceiling," but every production call site (slack/handler.py:2626, slack/interactions.py, telegram/transport_dispatch.py:208, discord/transport_dispatch.py:206, messaging/dispatch.py:107) calls channel_inbound_permitted("<transport>") with no sender, so a channels.posture.<connection>.senders rule never evaluates against a real message — yet _collect_connections reports senders_pinned to the dashboard, showing operators a restriction that does not bite. Wire the sender id through at least the transports that already know it, or cut the claim and the senders_pinned surface to the follow-up that does.
• The roster's anti-agent guarantee routes around itself. security.py says an agent that could write the roster "would enrol a bot of its own choosing," but the agent can reach the same end by writing messaging.connection_governance: false plus a bot token into agent-writable config.json — the switch disables both the roster and the sender posture. The switch's liftability is a deliberate availability choice, but the trust-root framing overstates what it buys against that adversary; consider letting policy pin the capability on (un-liftably), not only off, or reframe the roster as tamper-evidence rather than agent-proofing.

Suggestions

• The # posture-separator migration and container-resolving _resolve_posture are load-bearing contract changes buried in a feature PR; they'd be safer reviewed and landed as their own commit ahead of the roster.

[DESIGN-REVIEWED] d1eabcc

[chenmingwei23]

Parked — do not merge. Per-surface scoping is going to arrive as crew members, so this waits for that rather than shipping a surface it would duplicate.

Staying a draft on purpose (a draft cannot be merged, which is the only self-lock GitHub gives an author). The branch is pushed and the work is complete and verified, so resuming is a rebase and a re-run, not a rebuild.

What is in the branch (d1eabccf4, single commit)

• Connection identity — a connection, not a transport, is the governed principal (telegram/default). A channel matcher keeps every pre-existing channels policy meaning exactly what it meant.
• Trust roster (channel_trust.json, keystone-fenced) — a connection not enrolled does not attach. Absent/malformed admits nothing; a permissive roster is seeded once from the already-configured channels so an upgrade is transparent.
• Three fail-closed inbound questions — enrolment (re-checked per message), member, sender. The sender leaf is policy-only and intersects with the config allowlist, so it can only narrow.
• Enrol / revoke control — POST /api/connections/enrol|revoke, editing the keystone roster the same way Settings > Security edits denied_commands.json.
• Two switches with different owners — messaging.connection_governance (operator, liftable) and capabilities.channel_connections (fleet, not liftable by the running app or its agent). Either off makes the feature fully inert: no gate, no seed, no surface. This is the switch that turns the whole thing off when crew members land.
• Four read surfaces, each self-hiding when nothing is bounded.

61 new backend tests; isort / flake8 / mypy / tsc / vitest / i18n:check 13/13 all green locally. Verified end-to-end on an isolated pod: clicking Enrol flipped the card, produced the Revoke control, and dropped the connection out of the Security page's bounded list.

If this is picked up again

1. Rebase onto current main and re-run the gates — main moved three times during the original work.
2. Decide whether the roster survives as its own concept or folds into a crew member's admission. If it folds, messaging/trust.py and the two write endpoints are the pieces to re-home; the connection identity and the channel matcher are useful either way.
3. Three outbound chokepoints still query the bare transport (mcp_core send_message, chat_runner mirroring, chat_compaction_notice). Nothing is broken today — a transport-level rule covers every connection — but a rule naming one connection is not enforced on those paths. Four-site change.
4. sel._infer_source mis-attributes a dotted surface segment (telegram.ops-bot:… falls to the Slack default). Deliberately not fixed here: nothing on main produces that shape, so it belongs with whatever reintroduces named connections.

[bolichen97]

@chenmingwei23 The 2026-09-08 open-PR audit read this at d1eabcc and found nothing here duplicated on main, so we are not closing it. Keeping it open with a rebase path.

None of this layer exists on main today: no channel_trust.json roster, no src/kiro_crew/messaging/trust.py or messaging/connections.py, no channel matcher or # posture separator in src/kiro_crew/platform/governance.py, no messaging.connection_governance key, and channel_inbound_permitted in src/kiro_crew/messaging/identity.py still takes only a transport. That gap is real and worth closing.

What moved under you: main grew an unrelated connections domain meaning provider authorization (#9149, #9148) that now owns the /api/connections/* prefix, the module name and the UI vocabulary this branch picked; security.py became the src/kiro_crew/security/ package (#9089, #9183), so the _CREW_SECRET_LEAVES hunk belongs in security/paths.py; and route registration left dashboard/server.py for dashboard/routes/. The branch is 3623 commits behind and currently conflicted.

Ask: rebase onto main, re-home those hunks, and rename the new surface away from /api/connections so it does not collide with provider auth. Please also narrow scope to what actually evaluates. The diff adds sender and connection parameters to channel_inbound_permitted without updating any dispatch caller, so the senders posture never runs on a real message, and BotChannelPanel.tsx still leaves allowed_user_ids editable while the card says it is pinned. Expect textual conflicts in governance.py and test/test_governance_policy.py with #7362 and #6307.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

[chenmingwei23]

Closing this. The feature is not being pursued.

Recording the salvage assessment so a future attempt does not have to re-derive it.
The branch (feat/connection-governance, head d1eabcc) is NOT deleted, so the
work remains reachable.

State at close

3787 commits behind main; mergeable=CONFLICTING with 12 conflicting files, one
of them a modify/delete (src/kiro_crew/security.py, which main split into a
package in #9183).

The feature was never superseded

connection_governance and channel_connections have zero hits on main, and
allowed_user_ids still sits in config/loader.py and in each transport. The
problem this PR described is still main's behaviour. Closing is a decision not to
build it, not a claim that it landed some other way.

Every hook point still exists, in the same shape

Measured against main @ 67716d7:

• _governance_posture_permits_workspace in slack/enterprise.py still contains
  the two patched lines (governance_permits("channels", f"slack/{leaf}:..."))
  byte-identically, now at 274 and 278.
• _channel_transport_permitted(member) is at slack/gateway.py:1349, with two
  more call sites than at branch time.
• _channel_inbound_permitted_sync still sits in messaging/identity.py, which
  moved only +10 -14.
• channels: ScopeSpec(SCOPEDMAP) with members plus policy-only posture
  survives, as do ScopedMap.from_dict, _MATCHERS, _query_level and
  _match_mcp.
• _CREW_SECRET_LEAVES moved to security/paths.py:199; the 8-line
  security.py change is one list entry beside denied_commands.json at 362.
• The locale set is unchanged at 14 files.

The conflict count overstates the cost

Conflict size measures how far main moved, not how much this branch changed:

File	conflict lines	this branch	main since base
config/loader.py	2057	+19 -0	+3275 -4290
dashboard/server.py	665	+3 -0	+1924 -955
ChatSidebar.tsx	451	+3 -0	+5165 -1282
slack/gateway.py	100	+56 -4	+6047 -613
types/index.ts	67	+41 -0	+609 -33
platform/governance.py	23	+113 -10	+1388 -88
messaging/identity.py	35	+82 -9	+10 -14

The three largest conflicts carry 25 branch lines between them: take main's file
and re-insert. The genuinely new work is confirming the # posture separator and
_match_channel against the 1388 lines main added to governance.py, and
re-finding the frontend insertion point since ChannelsPanel.tsx was rewritten
(+76 -129).

The parking premise has expired

The capability pin existed so a fleet awaiting per-surface scoping "as crew
members" could opt out. Crew members has since shipped (members.py,
dashboard/handlers/members.py, member-<slug> slots). Anyone reviving this
should first settle whether the operator-owned roster or the members roster owns
the "which connection may attach" decision, rather than reviving the roster as
designed here.