Commit graph

2 commits

Author SHA1 Message Date
Adam Moussa
a6275b4000
fix(agent-team): harden listener respawn/close, broaden handle_event guard, channel_ref partial-unique (#29)
Three robustness/hardening fixes surfaced by /sh-security-review on the
agent-team listener/coordinator surface. None alter AUTHZ-01 allowlist
behavior or the first-answer-wins compare-and-set semantics.

1. Respawn close() leak (CWE-772). The watchdog _supervise_slack_listener
   respawned the inbound Slack listener without tearing down the dead one,
   leaking a Socket Mode WebSocket / SDK thread set per flap. Now the dead
   listener is closed before respawn (new _close_dead_listener, idempotent),
   AND _run_listener has a finally that always closes the listener so a
   crashed serve() releases its socket. SlackListener.close() is idempotent,
   so the belt-and-braces close stays a safe no-op.

2. Broadened exception guard in handle_event (CWE-248). The submit block
   only caught ValueError; the accept path (submit_answer ->
   _question_turn/_question_thread) can raise KeyError on a concurrently
   mutated row, and the CAS can raise sqlite3.Error. An uncaught exception
   would escape into the Bolt dispatch. Added a separate `except Exception`
   that logs at WARNING (not silent, not debug) and returns None. The
   existing ValueError-as-debug behavior is unchanged; authorization still
   runs first, so the trust boundary is not widened.

3. channel_ref partial-unique index (defense-in-depth). Added
   uq_pending_questions_open_channel_ref — a PARTIAL UNIQUE index on
   (channel_ref) WHERE channel_ref IS NOT NULL AND status='open' — so two
   OPEN rows can never share a non-null channel_ref (a thread_ts can never
   map to two open questions). Installed in init_db AND unconditionally in
   migrate (idempotent IF NOT EXISTS) so existing v1 DBs gain it. NULLs and
   closed rows are excluded; mirrored verbatim into schema.sql.

Tests: +8 (was 960, now 968). New: schema partial-unique reject/null/closed/
migrate cases; handle_event KeyError + sqlite3.Error swallow cases;
coordinator close-before-respawn + run_listener-closes-on-crash. Fixed the
operator-cli test fixture to use a per-question channel_ref (it previously
inserted multiple open rows sharing one ref, which the new index correctly
rejects).
2026-06-22 16:14:22 -04:00
15a416d31a Add Plane-2 leaf scaffold (pipeline graph, nodes, HITL, transports, CI)
Consolidates the 18 leaf modules from the r720-plane2-scaffold workflow onto
the foundation commit. Full suite: 535 passed, 1 skipped; ruff + format clean.

Built (pre-deployment scaffold only — nothing provisioned/enabled):
- LangGraph pipeline graph.py (INTAKE->CLARIFY->PLAN, interrupt()/resume, checkpointer-injectable)
- nodes: clarifier (98% gate), planner, review_loop (GPT-4.1), builders->candidate diff, verifier
- §3.3.1 HITL: ledger ops, resume_worker, deadline_timer, recovery sweep, responder
- transports: slack / github / claude_code adapters
- ci_gate (pure-code pass/fail), operator_cli, run-team.py entry, P1 sim harness
- ci/agent-team-apply-verify.yml (split untrusted/privileged jobs) — authored, disabled

KNOWN OPEN FINDINGS (verifier/cross-review, not yet fixed — see follow-up):
- builders denylist: 4 execution-proven bypasses (delete, mode-change, copy-to, out-of-scope delete)
- §3.3.1 CAS: BEGIN IMMEDIATE outside try/except; shared-connection txn nesting unsafe under concurrency
- operator_cli: missing re-deliver/force-resume; audit-after-mutate ordering gap
- ci yaml: GPT-4.1 cross-review PASS w/ 4 FIX items (symlink path escape, etc.)
- P1 sim harness models the ledger layer, not real LangGraph interrupt/resume; P1 exit criteria not yet truly proven

Deploy-gated (NOT done): IAM/step-ca/Roles Anywhere/confluence-bot provisioning,
/sh-security-review sign-off, live Slack/CI, rsync, live dry-runs, Adam approval.
2026-06-17 15:16:12 -04:00