Fix Slack reply binding and malformed-id gaps

Task 11145 · Running · Coder · created · updated

Epic: Security audit of development keys and container artifacts. This child = Architecture C2 (PRD R3 AC3.1, AC3.2, and AC3.4 for these files). ## Original requirement (verbatim, epic body) - "Other “Known gap” comments: a cross-org Slack member-ID claim (denial of service), plus notes in `alert_record.py`, `comment_service.py` and `routes/attachments.py`." - "It should not break anything in prod or dev" Operator decision D1: fix all four Known gaps. This child fixes the two Slack ones. The comment-image gaps are a sibling child. ## Approach (Architecture C2) **Reply binding** (`apps/api/app/slack_inbound.py:486-495`): - Pick the claimant before binding. For each claimant in id order: - Do one short read of `notify_targets.newest_thread_for(channel_id, thread_ts)` under `seam.bind_from(probe_session, AdminUser, claimant.id)`. - Use a fresh `SessionLocal()` per probe and close it at once (one bind per session). - Bind the first claimant whose read returns a thread with `thread.admin_user_id == claimant.id` and `thread.org_id == claimant.org_id`. Then run `_checks_in_scope` on the caller's session under that bind, as today. - If none matches, bind `claimants[0]` as now, so the refusal sentences and their order are unchanged. - Write-time claims stay unrefused, so there is no enumeration oracle. - Remove the comment. **Malformed member id** (`apps/api/app/notify_targets.py:748-751`): - Add `REASON_RECIPIENT_MALFORMED = "slack-recipient-malformed"` beside `REASON_RECIPIENT_MISSING` (:50) and in `REASON_KEYS` (:87-102). - `_slack_destination` returns it on a regex miss. - The alert path sets `recipient_misconfigured=True` for it (`apps/api/app/alert_record.py:61-65`; hand-back in `apps/api/app/task_notify.py:673-679`). - Find every exhaustive match over `REASON_KEYS`, including any console reason label map (grep `slack-recipient-missing` in `apps/web/src`), and give the new key a label. Follow `docs/standards/ui-copy-standards.md`: terse, says what to fix. - Remove the comment in `alert_record.py`. ## PRD R3 — Fix the four "Known gap" comments (A3; D1) — this child's ACs, verbatim - AC3.1 Slack (`apps/api/app/slack_inbound.py:488`): a reply is bound to the claimant the thread names, not the first claimant by id; a user in org B claiming org A's Slack member id no longer stops org A's member's Slack replies; claims still are NOT refused at write time (no enumeration oracle). - AC3.2 Alerts (`apps/api/app/alert_record.py:63`): a malformed stored Slack member id maps to a new `REASON_KEYS` member treated as a fixable misconfigured recipient (`recipient_misconfigured=True`), not `REASON_RECIPIENT_MISSING`; the card is retried once fixed, not dropped for good. - AC3.4 `grep -rn "Known gap" apps/api/app/slack_inbound.py apps/api/app/alert_record.py apps/api/app/comment_service.py apps/api/app/routes/attachments.py` prints nothing. (This child owns the first two files.) Credentials & reach: - Quick/full gate, pytest, CE tree build + `check_tree`, installer harness, tree console build | Worktree only (`./dc`, `tools/community_edition/*.py` present) | verified — files present on h2; no credential needed - PR checks / merge lane / Jaah Deploy run status | `jaah-forge` (pre-authenticated for the task) | verified — read verb `pr list` succeeded from h2 - Dev console boots, sign-in, Slack/Telegram settings, attachment upload (AC2.3, AC4.5, AC9.2) | `org/console/dev-login` (credential_get succeeded); environment: dev control plane | **deferred** to a node with reach. dev timed out from h2 (prod `app.jaah.ai` answered 200). Operator answer A5: a green Jaah Deploy is the gate, and the browser check runs on a node that reaches dev; a builder without reach records it deferred Out of scope: PRD §5, verbatim: - Do NOT change `infra/community-edition/node/install_clis.sh`'s vendor `curl https://claude.ai/install.sh | bash` (D8; deferred follow-up). - Do NOT sign or digest-pin the cloud deploy images pushed by Jaah Deploy, nor add deploy-time verification (D3 chose CE + base images only). - Do NOT add a CI/terraform guard over ECS env (D7 verify-only). - Do NOT cosign-sign `SHA256SUMS` (D4). - Do NOT remove Slack, Telegram, headscale, AWS, KMS, S3 or Bedrock from the cloud edition — the prune is CE-tree only. - Do NOT change the session-secret / dev-seed / insecure-agent refusals' semantics or the `FIXTURE_APP_ENVS` set. - Do NOT touch `infra/jaah/terraform` resources (only read them for R2). - Do NOT change port binds, setup-loopback rule, cookie flags or rate limits (A6 positives). - Architecture boundary: do not refuse Slack member-id claims at write time; do not change existing refusal wording; comment-image reaping and the CE prune are sibling children. Done means: - AC3.1 — pytest with two orgs claiming one member id (org B's user first by id): org A's member's reply in org A's thread acts; org B's claim gives B nothing in A; under org B's bind `newest_thread_for` for org A's `(channel_id, thread_ts)` returns `None`; a reply with no matching thread produces today's refusal text unchanged. Comment removed. - AC3.2 — pytest on `notify_targets._slack_destination` with a malformed id → the new reason and `recipient_misconfigured`; the console shows a human label for the new reason key (no raw key rendered). Comment removed. - AC3.4 — `grep -n "Known gap" apps/api/app/slack_inbound.py apps/api/app/alert_record.py` prints nothing. - AC9.1 `./dc check quick` green on the branch; merge lane `./dc check full` green. - AC9.2 The PR's Jaah Deploy succeeds to dev; dev console loads and a user can sign in. (browser part deferred — dev unreachable from h2; operator A5: green Jaah Deploy is the gate)

Outcome

Pull request

Activity

  • Draft
  • Ready
  • Runner started