From 765695269252d6fee15881546634e011e0b5350f Mon Sep 17 00:00:00 2001 From: Rohit P Date: Fri, 24 Jul 2026 22:33:13 -0700 Subject: [PATCH] security: harden Slack approval handling --- coworker/connectors/adapters.py | 2 + coworker/connectors/base.py | 10 +- coworker/connectors/gateway.py | 48 ++- coworker/connectors/relay_client.py | 3 + coworker/connectors/setup.py | 16 + coworker/server/app.py | 17 +- coworker/server/manager.py | 194 +++++++++- surfaces/gui/e2e/fixtures.ts | 29 +- surfaces/gui/e2e/inbox.spec.ts | 6 +- surfaces/gui/e2e/slack-workspaces.spec.ts | 16 + surfaces/gui/src/api.ts | 30 ++ .../gui/src/components/InboxConfigure.tsx | 49 ++- .../src/components/connectors/SlackDetail.tsx | 151 +++++++- tests/test_connectors.py | 25 ++ tests/test_fake_slack.py | 1 + tests/test_interactions.py | 10 + tests/test_slack_approval_owners.py | 343 ++++++++++++++++++ tests/test_slack_relay.py | 6 +- tests/test_slack_workspaces.py | 2 + tests/test_team_allowlist.py | 1 + tests/test_ui_refresh_e2e.py | 5 +- 21 files changed, 931 insertions(+), 33 deletions(-) create mode 100644 tests/test_slack_approval_owners.py diff --git a/coworker/connectors/adapters.py b/coworker/connectors/adapters.py index 0da9ae0e..3d64d952 100644 --- a/coworker/connectors/adapters.py +++ b/coworker/connectors/adapters.py @@ -243,7 +243,9 @@ class SlackAdapter(BasePlatformAdapter): chat_id=str(channel), message_id=ts, value=str(value), + user_id=user.get("id"), user_name=user.get("username") or user.get("name"), + response_url=body.get("response_url"), ) ) diff --git a/coworker/connectors/base.py b/coworker/connectors/base.py index b7501f55..e269c09e 100644 --- a/coworker/connectors/base.py +++ b/coworker/connectors/base.py @@ -119,14 +119,20 @@ MessageHandler = Callable[[MessageEvent], Awaitable[None]] @dataclass class InteractionEvent: - """A button click on an interactive prompt. `value` is the opaque button value (see - `interactions.decode`); `user_name` is who clicked, for the message update.""" + """A button click on an interactive prompt. + + Stable actor/workspace ids are security inputs; display names are presentation only. + `response_url` is Slack's short-lived reply capability for a private rejection notice. + """ platform: str chat_id: str message_id: Optional[str] # the clicked message's id/ts (to update it) value: str + user_id: Optional[str] = None user_name: Optional[str] = None + team_id: Optional[str] = None + response_url: Optional[str] = None InteractionHandler = Callable[[InteractionEvent], Awaitable[None]] diff --git a/coworker/connectors/gateway.py b/coworker/connectors/gateway.py index 0946feb4..226966ee 100644 --- a/coworker/connectors/gateway.py +++ b/coworker/connectors/gateway.py @@ -9,15 +9,19 @@ super-agent runner, wired in the next increment). Outbound replies go through th from __future__ import annotations import logging +from asyncio import to_thread from collections import OrderedDict from typing import Callable, Optional +from urllib.parse import urlparse from ..secrets import SecretStore from .base import ( BasePlatformAdapter, + InteractionEvent, MessageEvent, MessageHandler, SendResult, + SessionSource, parse_target, ) from .config import ConnectorSettings, is_authorized, load_settings @@ -69,10 +73,52 @@ class Gateway: adapter.set_interaction_handler(self._on_interaction) self._adapters[adapter.platform] = adapter - async def _on_interaction(self, event) -> None: + async def _on_interaction(self, event: InteractionEvent) -> None: + source = SessionSource( + platform=event.platform, + chat_id=event.chat_id, + user_id=event.user_id, + user_name=event.user_name, + chat_type="channel", + team_id=event.team_id, + ) + settings = self.settings.get(event.platform) + if settings is None or not is_authorized(settings, source): + logger.info("rejecting unauthorized interaction from %s", source.label()) + await self.reject_interaction(event) + return if self._interaction_handler is not None: await self._interaction_handler(event) + async def reject_interaction( + self, + event: InteractionEvent, + text: str = "Only a designated approval owner can respond to this request.", + ) -> None: + """Best-effort private feedback for a rejected Slack button click.""" + response_url = str(event.response_url or "") + parsed = urlparse(response_url) + if ( + event.platform != "slack" + or parsed.scheme != "https" + or parsed.hostname not in {"hooks.slack.com", "hooks.slack-gov.com"} + ): + return + + def _post() -> None: + import httpx + + try: + httpx.post( + response_url, + json={"response_type": "ephemeral", "text": text}, + timeout=10, + ) + except Exception: + logger.debug("Slack ephemeral interaction response failed", exc_info=True) + + await to_thread(_post) + async def _on_inbound(self, event: MessageEvent) -> None: self._record_recent(event) # capture identity even from unauthorized senders settings = self.settings.get(event.source.platform) diff --git a/coworker/connectors/relay_client.py b/coworker/connectors/relay_client.py index 24de2793..57fc38cc 100644 --- a/coworker/connectors/relay_client.py +++ b/coworker/connectors/relay_client.py @@ -358,7 +358,10 @@ class SlackRelayAdapter(BasePlatformAdapter): chat_id=qualify(team_id, channel), message_id=ts, value=str(value), + user_id=user.get("id"), user_name=user.get("username") or user.get("name"), + team_id=team_id, + response_url=interaction.get("response_url"), ) ) diff --git a/coworker/connectors/setup.py b/coworker/connectors/setup.py index fe716319..57d5476d 100644 --- a/coworker/connectors/setup.py +++ b/coworker/connectors/setup.py @@ -93,6 +93,9 @@ def connector_list(secrets: SecretStore) -> list[dict[str, Any]]: "enabled": bool(profile.get("enabled", True)) and connected, # The actual allow-list (the GUI manages it inline); was a bare count. "allowed_users": list(profile.get("allowed_users") or []), + # Manual Socket Mode only: explicitly selected humans who may resolve + # consequential Inbox prompts. Relay uses its OAuth installer instead. + "approval_owner_ids": list(profile.get("approval_owner_ids") or []), "tools": tool_dicts(secrets, d.name), "experimental": d.experimental, "risk_notice": d.risk_notice, @@ -107,6 +110,10 @@ def connector_list(secrets: SecretStore) -> list[dict[str, Any]]: # Managed relay is multi-workspace: each `slack:team:*` profile is one # connected workspace with its OWN allow-list (ids are workspace-scoped). entry["workspaces"] = _slack_workspaces(secrets) + if profile.get("mode") == "relay": + # Dormant Manual-mode owners may remain beside preserved Socket + # Mode credentials; they never authorize a bare Relay target. + entry["approval_owner_ids"] = [] if d.name == "gmail": # Multi-account: each `gmail:account:*` profile is one mailbox; the # :default profile is just the default pointer + privacy filters. @@ -185,6 +192,11 @@ def _slack_workspaces(secrets: SecretStore) -> list[dict[str, Any]]: "domain": profile.get("domain") or "", "allowed_users": list(profile.get("allowed_users") or []), "allow_all": bool(profile.get("allow_all")), + # Relay approvals are installer-only. Keep the list-shaped API aligned + # with Manual mode without creating a second editable relay role. + "approval_owner_ids": ( + [profile["slack_user_id"]] if profile.get("slack_user_id") else [] + ), # Who installed (authed_user) — the GUI marks their chip "you" and # keys the post-connect card's "your mentions get through" line. "installer_user_id": profile.get("slack_user_id") or "", @@ -353,6 +365,10 @@ def connect_connector( profile: dict[str, Any] = {"type": profile_type, "enabled": True, **token_creds} if any(f.key == "allowed_users" for f in d.fields): profile["allowed_users"] = allowed + if name == "slack" and existing.get("approval_owner_ids"): + # Re-pasting manual Socket Mode tokens must not erase the locally selected + # approval owners. + profile["approval_owner_ids"] = list(existing["approval_owner_ids"]) if identity: profile["account"] = identity if d.account_field: diff --git a/coworker/server/app.py b/coworker/server/app.py index baeaa2e7..3b054945 100644 --- a/coworker/server/app.py +++ b/coworker/server/app.py @@ -371,12 +371,11 @@ def create_app(manager: SessionManager) -> FastAPI: name = str(body.get("name", "")).strip() if not name: return {"ok": False, "error": "binding needs a `name`"} - manager.inbox_routing.set_binding( + return manager.set_inbox_binding( name, channel=body.get("channel") or None, target=str(body.get("target", "")), ) - return {"ok": True, "bindings": manager.inbox_routing.bindings()} @app.get("/v1/sessions/{session_id}/unattended") def get_unattended(session_id: str) -> dict[str, Any]: @@ -1245,6 +1244,20 @@ def create_app(manager: SessionManager) -> FastAPI: name, str(body.get("user_id", "")), str(body.get("team_id", "")) or None ) + @app.post("/v1/connectors/slack/approval-owners/add") + def slack_approval_owner_add(body: dict) -> dict[str, Any]: + return manager.set_slack_approval_owner( + str(body.get("user_id", "")), + add=True, + display_name=str(body.get("name", "")), + ) + + @app.post("/v1/connectors/slack/approval-owners/remove") + def slack_approval_owner_remove(body: dict) -> dict[str, Any]: + return manager.set_slack_approval_owner( + str(body.get("user_id", "")), add=False + ) + # -- audit / browser observability ------------------------------------------ @app.get("/v1/audit") def audit_list( diff --git a/coworker/server/manager.py b/coworker/server/manager.py index 2c8e8813..b2d9be0c 100644 --- a/coworker/server/manager.py +++ b/coworker/server/manager.py @@ -51,6 +51,7 @@ from ..connectors import ( load_settings, make_adapter, set_experimental_enabled, + slack_split, update_connector_tools, ) from ..connectors.browser_automation import ( @@ -1126,11 +1127,19 @@ class SessionManager: u: self._people.get(f"{c['name']}:{u}") for u in (c.get("allowed_users") or []) } + c["approval_owner_names"] = { + u: self._people.get(f"{c['name']}:{u}") + for u in (c.get("approval_owner_ids") or []) + } for w in c.get("workspaces") or []: w["allowed_user_names"] = { u: self._people.get(f"{c['name']}:{u}") for u in (w.get("allowed_users") or []) } + w["approval_owner_names"] = { + u: self._people.get(f"{c['name']}:{u}") + for u in (w.get("approval_owner_ids") or []) + } return connectors def connect_connector( @@ -1935,8 +1944,143 @@ class SessionManager: def disallow_user( self, name: str, user_id: str, team_id: Optional[str] = None ) -> dict[str, Any]: + if name == "slack" and user_id in self.slack_approval_owner_ids(team_id): + return { + "ok": False, + "error": "Remove this person as an approval owner first.", + } return self._set_allowed(name, user_id, team_id=team_id, add=False) + def slack_approval_owner_ids(self, team_id: Optional[str] = None) -> set[str]: + """Stable Slack user ids allowed to resolve consequential Inbox prompts. + + Managed relay installs are installer-owned. Manual Socket Mode has no + human OAuth identity, so its owners are selected explicitly. + """ + key = f"slack:team:{team_id}" if team_id else "slack:default" + profile = self.secrets.get(key) or {} + if team_id: + installer = str(profile.get("slack_user_id") or "").strip() + return {installer} if installer else set() + if profile.get("mode") == "relay": + return set() + return { + str(user_id).strip() + for user_id in (profile.get("approval_owner_ids") or []) + if str(user_id).strip() + } + + def set_slack_approval_owner( + self, user_id: str, *, add: bool, display_name: str = "" + ) -> dict[str, Any]: + """Edit Manual Socket Mode approval owners. + + Owner status implies inbound permission. Relay ownership is derived from + the OAuth installer and is intentionally not editable here. + """ + user_id = str(user_id).strip() + if not user_id: + return {"ok": False, "error": "user_id required"} + profile = self.secrets.get("slack:default") + if not profile: + return {"ok": False, "error": "Slack is not connected in Manual mode."} + if profile.get("mode") == "relay" or profile.get("managed"): + return { + "ok": False, + "error": "Relay approval ownership is set by the Slack installer.", + } + + owners = self.slack_approval_owner_ids() + if add: + owners.add(user_id) + else: + owners.discard(user_id) + if not owners and self._has_manual_slack_inbox_binding(): + return { + "ok": False, + "error": ( + "Choose another approval owner before removing the last one " + "while Slack Inbox routing is active." + ), + } + profile["approval_owner_ids"] = sorted(owners) + if add: + allowed = set(profile.get("allowed_users") or []) + allowed.add(user_id) + profile["allowed_users"] = sorted(allowed) + self.secrets.put("slack:default", profile) + if display_name: + self._note_person("slack", user_id, display_name) + if self.gateway is not None and "slack" in self.gateway.settings: + self.gateway.settings["slack"].allowed_users = set( + profile.get("allowed_users") or [] + ) + return { + "ok": True, + "approval_owner_ids": sorted(owners), + "allowed_users": list(profile.get("allowed_users") or []), + } + + def _has_manual_slack_inbox_binding(self) -> bool: + for raw in self.inbox_routing.bindings(): + if raw.get("channel") != "slack": + continue + team_id, _ = slack_split(str(raw.get("target") or "")) + if team_id is None: + return True + return False + + def _slack_actor_owns_item( + self, + item, + *, + actor_id: str, + chat_id: str, + team_id: Optional[str], + ) -> bool: + """Authorize a Slack resolution against both its owner and delivery binding.""" + event_team, event_channel = slack_split(chat_id) + event_team = team_id or event_team + binding = self.inbox_routing.binding_for(item.inbox) + owner_team = event_team + if binding.channel == "slack": + owner_team, bound_channel = slack_split(binding.target) + if owner_team != event_team or bound_channel != event_channel: + return False + return bool(actor_id) and actor_id in self.slack_approval_owner_ids(owner_team) + + def set_inbox_binding( + self, name: str, *, channel: Optional[str], target: str + ) -> dict[str, Any]: + """Persist an Inbox transport after validating its approval identity.""" + channel = str(channel or "").strip() or None + target = str(target or "").strip() + if channel and not target: + return {"ok": False, "error": "Choose a destination channel."} + if channel == "slack": + settings = load_settings(self.secrets).get("slack") + if settings is None or not settings.enabled: + return {"ok": False, "error": "Slack is not connected."} + team_id, destination = slack_split(target) + if not destination: + return {"ok": False, "error": "Choose a destination channel."} + key = f"slack:team:{team_id}" if team_id else "slack:default" + if not self.secrets.get(key): + return { + "ok": False, + "error": "That Slack workspace is not connected.", + } + if not self.slack_approval_owner_ids(team_id): + return { + "ok": False, + "error": ( + "Choose at least one approval owner in Slack settings before " + "routing Inbox requests there." + ), + } + self.inbox_routing.set_binding(name, channel=channel, target=target) + return {"ok": True, "bindings": self.inbox_routing.bindings()} + def _set_allowed( self, name: str, user_id: str, *, team_id: Optional[str] = None, add: bool ) -> dict[str, Any]: @@ -2458,6 +2602,12 @@ class SessionManager: binding = self.inbox_routing.binding_for(item.inbox) if not (binding.channel and self.gateway is not None): return + if binding.channel == "slack": + team_id, _ = slack_split(binding.target) + # Legacy bindings may predate approval ownership. Keep the item + # available in-app, but never mirror it to an ownerless channel. + if not self.slack_approval_owner_ids(team_id): + return target = f"{binding.channel}:{binding.target}" body = "\n".join(p for p in (item.title, item.body) if p).strip() buttons = buttons_for(item) @@ -2484,10 +2634,29 @@ class SessionManager: return item_id, resolution = decoded item = self.inbox.get(item_id) + if item is None: + return + protected_kinds = {"approval", "directory", "plan"} + if ( + getattr(event, "platform", "") == "slack" + and item.kind in protected_kinds + ): + actor_id = str(getattr(event, "user_id", "") or "") + if not self._slack_actor_owns_item( + item, + actor_id=actor_id, + chat_id=getattr(event, "chat_id", "") or "", + team_id=getattr(event, "team_id", None), + ): + if self.gateway is not None: + await self.gateway.reject_interaction(event) + return already = item is not None and item.state != "pending" - await self.resolve_inbox(item_id, resolution) + resolved = await self.resolve_inbox(item_id, resolution) + if not resolved and not already: + return who = getattr(event, "user_name", None) or "someone" - title = item.title if item is not None else "Prompt" + title = item.title outcome = "already resolved" if already else f"“{resolution}” — by {who}" if self.gateway is not None and getattr(event, "message_id", None): try: @@ -2508,7 +2677,26 @@ class SessionManager: from ..inbox_routing import resolve_from_reply text = getattr(event, "text", "") or "" - return resolve_from_reply(text, self.inbox.resolve) is not None + + def _resolve(item_id: str, resolution: str) -> bool: + item = self.inbox.get(item_id) + if item is None: + return False + if ( + getattr(event.source, "platform", "") == "slack" + and item.kind in {"approval", "directory", "plan"} + ): + actor_id = str(getattr(event.source, "user_id", "") or "") + if not self._slack_actor_owns_item( + item, + actor_id=actor_id, + chat_id=getattr(event.source, "chat_id", "") or "", + team_id=getattr(event.source, "team_id", None), + ): + return False + return self.inbox.resolve(item_id, resolution) + + return resolve_from_reply(text, _resolve) is not None # -- self-wake resumption --------------------------------------------------- async def resume_due_wakes(self) -> int: diff --git a/surfaces/gui/e2e/fixtures.ts b/surfaces/gui/e2e/fixtures.ts index b8f53743..7df802f5 100644 --- a/surfaces/gui/e2e/fixtures.ts +++ b/surfaces/gui/e2e/fixtures.ts @@ -357,11 +357,12 @@ export async function mockApi(page: import("@playwright/test").Page) { mode: "relay" as "" | "relay", account: "deeplearning.ai", allowed_users: [] as string[], // flat list (manual Socket Mode only) + approval_owner_ids: [] as string[], workspaces: [ // T1DL mirrors a managed install: the installer (authed_user) was pre-added // to the allow-list on connect (UX-027) — keys the "you" chip + setup card. - { team_id: "T1DL", account: "deeplearning.ai", domain: "dlaiteam", allowed_users: ["U_ME"] as string[], allow_all: false, allowed_user_names: {} as Record, installer_user_id: "U_ME", installer_name: "Rohit Prasad" }, - { team_id: "T2AC", account: "acme-partners", domain: "acmehq", allowed_users: [] as string[], allow_all: false, allowed_user_names: {} as Record, installer_user_id: "", installer_name: "" }, + { team_id: "T1DL", account: "deeplearning.ai", domain: "dlaiteam", allowed_users: ["U_ME"] as string[], allow_all: false, allowed_user_names: {} as Record, approval_owner_ids: ["U_ME"] as string[], approval_owner_names: { U_ME: "Rohit Prasad" } as Record, installer_user_id: "U_ME", installer_name: "Rohit Prasad" }, + { team_id: "T2AC", account: "acme-partners", domain: "acmehq", allowed_users: [] as string[], allow_all: false, allowed_user_names: {} as Record, approval_owner_ids: [] as string[], approval_owner_names: {} as Record, installer_user_id: "", installer_name: "" }, ], }; const slackConnector = () => ({ @@ -369,7 +370,9 @@ export async function mockApi(page: import("@playwright/test").Page) { auth: "bot_token", two_way: true, channels: true, available: true, brand_color: "#611f69", logo: "slack", fields: [], instructions: [], connected: slackState.connected, account: slackState.account, enabled: slackState.connected, - allowed_users: [...slackState.allowed_users], tools: [], managed: true, + allowed_users: [...slackState.allowed_users], + approval_owner_ids: [...slackState.approval_owner_ids], + tools: [], managed: true, managed_profile: slackState.mode === "relay", mode: slackState.mode, workspaces: slackState.workspaces.map((w) => ({ ...w, allowed_users: [...w.allowed_users] })), unauthorized: parked.map((x) => ({ ...x })), @@ -911,6 +914,24 @@ export async function mockApi(page: import("@playwright/test").Page) { if (add && b.name && ws) ws.allowed_user_names[b.user_id] = b.name; return json({ ok: true, allowed_users: [...pool], team_id: b.team_id ?? null }); } + if (p.endsWith("/v1/connectors/slack/approval-owners/add") && m === "POST") { + const b = req.postDataJSON(); + if (!slackState.approval_owner_ids.includes(b.user_id)) + slackState.approval_owner_ids.push(b.user_id); + if (!slackState.allowed_users.includes(b.user_id)) + slackState.allowed_users.push(b.user_id); + return json({ + ok: true, + approval_owner_ids: [...slackState.approval_owner_ids], + allowed_users: [...slackState.allowed_users], + }); + } + if (p.endsWith("/v1/connectors/slack/approval-owners/remove") && m === "POST") { + const b = req.postDataJSON(); + const i = slackState.approval_owner_ids.indexOf(b.user_id); + if (i >= 0) slackState.approval_owner_ids.splice(i, 1); + return json({ ok: true, approval_owner_ids: [...slackState.approval_owner_ids] }); + } // Workspace rosters for the pickers (users.list / conversations.list, mocked). if (/\/v1\/connectors\/slack\/workspaces\/[^/]+\/directory$/.test(p) && m === "GET") { const q = (new URL(req.url()).searchParams.get("q") || "").toLowerCase(); @@ -1137,7 +1158,7 @@ export async function mockApi(page: import("@playwright/test").Page) { // Slack managed install = add a workspace. The real flow completes in the system // browser; the mock installs instantly so the page's poll picks it up. if (p.includes("/connectors/slack/")) { - slackState.workspaces.push({ team_id: "T3NEW", account: "new-workspace", allowed_users: [], allow_all: false, allowed_user_names: {} }); + slackState.workspaces.push({ team_id: "T3NEW", account: "new-workspace", domain: "new-workspace", allowed_users: ["U_ME"], allow_all: false, allowed_user_names: { U_ME: "Rohit Prasad" }, approval_owner_ids: ["U_ME"], approval_owner_names: { U_ME: "Rohit Prasad" }, installer_user_id: "U_ME", installer_name: "Rohit Prasad" }); slackState.connected = true; slackState.mode = "relay"; } diff --git a/surfaces/gui/e2e/inbox.spec.ts b/surfaces/gui/e2e/inbox.spec.ts index 5066c655..ae7abd0b 100644 --- a/surfaces/gui/e2e/inbox.spec.ts +++ b/surfaces/gui/e2e/inbox.spec.ts @@ -59,13 +59,13 @@ test("routing: Configure tab binds the mirror channel; Pending's status line fol await page.getByTestId("inbox-route-configure").click(); const mirror = page.getByTestId("inbox-mirror-card"); await expect(mirror).toContainText("in-app Inbox only"); - await mirror.getByPlaceholder("slack:C0123 or channel link").fill("C0777"); + await mirror.getByPlaceholder("slack:C0123 or channel link").fill("slack:T1DL/C0777"); await mirror.getByRole("button", { name: "Set", exact: true }).click(); - await expect(mirror).toContainText("slack:C0777"); + await expect(mirror).toContainText("slack:T1DL/C0777"); // Back on Pending, the line reflects the new target immediately. await page.getByTestId("inbox-tab-pending").click(); - await expect(line).toContainText("slack:C0777"); + await expect(line).toContainText("slack:T1DL/C0777"); await expect(line).toContainText("replies there resolve items here"); // Clearing (also on Configure) returns Pending to local-only delivery. diff --git a/surfaces/gui/e2e/slack-workspaces.spec.ts b/surfaces/gui/e2e/slack-workspaces.spec.ts index f14284bb..e06707dc 100644 --- a/surfaces/gui/e2e/slack-workspaces.spec.ts +++ b/surfaces/gui/e2e/slack-workspaces.spec.ts @@ -61,6 +61,7 @@ test("disconnect removes one workspace and keeps the rest relaying", async ({ pa test("manual Socket Mode: one card with the flat allow-list (no regression)", async ({ page, }) => { + let owners: string[] = []; // Override the connectors payload AFTER mockApi so this test sees a manual-mode Slack // (routes registered later match first). await page.route("**/v1/connectors", (route) => @@ -74,6 +75,8 @@ test("manual Socket Mode: one card with the flat allow-list (no regression)", as auth: "bot_token", two_way: true, available: true, brand_color: "#611f69", logo: "slack", fields: [], instructions: [], connected: true, account: "acme", enabled: true, allowed_users: ["U0OK"], allowed_user_names: { U0OK: "Rohit" }, + approval_owner_ids: [...owners], + approval_owner_names: Object.fromEntries(owners.map((u) => [u, u === "U9MAYA" ? "Maya Chen" : u])), tools: [], managed: true, managed_profile: false, mode: "", workspaces: [], unauthorized: [], }, @@ -81,9 +84,22 @@ test("manual Socket Mode: one card with the flat allow-list (no regression)", as }), }), ); + await page.route("**/v1/connectors/slack/approval-owners/add", async (route) => { + const body = route.request().postDataJSON(); + owners = [...new Set([...owners, body.user_id])]; + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ ok: true, approval_owner_ids: owners }), + }); + }); await openSlackPage(page); await expect(page.getByTestId("slack-mode-badge")).toContainText("Socket Mode"); const card = page.getByTestId("slack-manual-card"); await expect(card).toContainText("acme"); await expect(card).toContainText("Rohit"); // flat allow-list chip, named + await expect(card).toContainText("Choose at least one owner"); + await page.getByTestId("add-approval-owner").click(); + await page.getByTestId("pick-person-U9MAYA").click(); + await expect(page.getByTestId("approval-owner-U9MAYA")).toContainText("Maya Chen"); }); diff --git a/surfaces/gui/src/api.ts b/surfaces/gui/src/api.ts index edca6b1c..c700cdd0 100644 --- a/surfaces/gui/src/api.ts +++ b/surfaces/gui/src/api.ts @@ -363,6 +363,8 @@ export interface SlackWorkspace { allowed_users: string[]; allow_all: boolean; allowed_user_names?: Record; + approval_owner_ids?: string[]; + approval_owner_names?: Record; // Who installed this workspace (authed_user) — pre-added to the allow-list on // connect (UX-027); the GUI marks their chip "you" and keys the setup card copy. installer_user_id?: string; @@ -443,6 +445,8 @@ export interface Connector { mcp?: boolean; // MCP-backed one-click (vendor-hosted MCP + local OAuth — no cloud sign-in) allowed_users: string[]; // the allow-list (managed inline in the Connectors tab) allowed_user_names?: Record; // id → display name (people directory) + approval_owner_ids?: string[]; // Manual Slack: humans allowed to resolve approvals + approval_owner_names?: Record; recent?: RecentSender[]; // recently-seen senders on a connected two-way connector unauthorized?: ParkedMessage[]; // parked messages from unallowed senders (§19) tools: ConnectorTool[]; @@ -1585,6 +1589,32 @@ export async function disallowUser(name: string, userId: string, teamId?: string return res.json(); } +export async function addSlackApprovalOwner( + userId: string, + displayName?: string, +): Promise<{ ok: boolean; error?: string }> { + const res = await fetch(`${httpBase()}/v1/connectors/slack/approval-owners/add`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + user_id: userId, + ...(displayName ? { name: displayName } : {}), + }), + }); + return res.json(); +} + +export async function removeSlackApprovalOwner( + userId: string, +): Promise<{ ok: boolean; error?: string }> { + const res = await fetch(`${httpBase()}/v1/connectors/slack/approval-owners/remove`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ user_id: userId }), + }); + return res.json(); +} + /** Stop relaying one managed Slack workspace (the app stays installed in Slack). */ export async function disconnectSlackWorkspace(teamId: string): Promise<{ ok: boolean; error?: string; remaining_workspaces?: number }> { const res = await fetch( diff --git a/surfaces/gui/src/components/InboxConfigure.tsx b/surfaces/gui/src/components/InboxConfigure.tsx index 79e4a54e..86f23e4b 100644 --- a/surfaces/gui/src/components/InboxConfigure.tsx +++ b/surfaces/gui/src/components/InboxConfigure.tsx @@ -1,5 +1,6 @@ import { useEffect, useState } from "react"; import { + getConnectors, getDmRoute, getInboxRouting, getRecentChannels, @@ -11,6 +12,7 @@ import { subscribeChannel, unsubscribeChannel, type RecentChannel, + type Connector, type Subscription, type UnroutedItem, } from "../api"; @@ -53,11 +55,14 @@ export function InboxConfigure() { // the "default" route (sessions fall back to it); pick a channel separate from any you subscribe to. function InboxRoutingCard() { const [recent, setRecent] = useState([]); + const [connectors, setConnectors] = useState([]); const [target, setTarget] = useState(""); // current default-binding address, e.g. "slack:C0123" const [draft, setDraft] = useState(""); + const [error, setError] = useState(null); const load = () => { getRecentChannels().then(setRecent).catch(() => setRecent([])); + getConnectors().then(setConnectors).catch(() => setConnectors([])); getInboxRouting() .then((bs) => { const def = bs.find((b) => b.name === "default"); @@ -76,15 +81,43 @@ function InboxRoutingCard() { if (!addr) return; // "slack:C0123" → channel="slack", target="C0123"; a bare id assumes slack. const [platform, id] = addr.includes(":") ? addr.split(":", 2) : ["slack", addr]; - await setInboxBinding("default", platform, id); + const result = await setInboxBinding("default", platform, id); + if (!result.ok) { + setError(result.error || "Could not update Inbox routing."); + return; + } + setError(null); setDraft(""); load(); }; const clear = async () => { - await setInboxBinding("default", null, ""); + const result = await setInboxBinding("default", null, ""); + if (!result.ok) { + setError(result.error || "Could not clear Inbox routing."); + return; + } + setError(null); load(); }; + const draftAddr = draft.trim(); + const [draftPlatform, draftTarget] = draftAddr.includes(":") + ? draftAddr.split(":", 2) + : ["slack", draftAddr]; + const slack = connectors.find((c) => c.name === "slack"); + const teamId = + draftPlatform === "slack" && draftTarget.includes("/") + ? draftTarget.split("/", 1)[0] + : null; + const owners = + draftPlatform !== "slack" + ? [] + : teamId + ? slack?.workspaces?.find((w) => w.team_id === teamId)?.approval_owner_ids ?? [] + : slack?.approval_owner_ids ?? []; + const missingSlackOwner = + draftPlatform === "slack" && draftTarget.length > 0 && owners.length === 0; + // Show the channel's NAME when the recent list knows it (raw address as the fallback/tooltip). const known = recent.find((c) => c.channel === target)?.name; @@ -103,7 +136,11 @@ function InboxRoutingCard() { - {target && ( @@ -112,6 +149,12 @@ function InboxRoutingCard() { )} + {missingSlackOwner && ( +

+ Choose an approval owner under Integrations → Slack before routing approvals here. +

+ )} + {error &&

{error}

} ); } diff --git a/surfaces/gui/src/components/connectors/SlackDetail.tsx b/surfaces/gui/src/components/connectors/SlackDetail.tsx index 61eeab42..e5871feb 100644 --- a/surfaces/gui/src/components/connectors/SlackDetail.tsx +++ b/surfaces/gui/src/components/connectors/SlackDetail.tsx @@ -1,11 +1,13 @@ import { useEffect, useRef, useState } from "react"; import { + addSlackApprovalOwner, allowUser, disallowUser, disconnectSlackWorkspace, getSlackDirectory, getSubscriptions, resolveUnauthorized, + removeSlackApprovalOwner, unsubscribeChannel, type Connector, type ParkedMessage, @@ -130,10 +132,17 @@ export function SlackDetail({ c, cloud, slack, onChanged }: DetailProps) { disallowUser("slack", u).then(changed)} onChanged={changed} /> + {(c.unauthorized ?? []) .filter((m) => !m.team_id) .map((m) => ( @@ -209,24 +218,43 @@ function WorkspaceGroup({
{empty ? ( -
- - No one allowed yet — mentions of the bot show up here for your OK. - - - -
+ <> +
+ + No one allowed yet — mentions of the bot show up here for your OK. + + + +
+ + ) : ( <> disallowUser("slack", u, w.team_id).then(onChanged)} onChanged={onChanged} /> + {parked.map((m) => ( ))} @@ -259,6 +287,7 @@ function DisconnectBtn({ teamId, busy, onClick }: { teamId: string; busy: boolea function PeopleRow({ allowed, names, + protectedIds, teamId, installerId, installerName, @@ -267,6 +296,7 @@ function PeopleRow({ }: { allowed: string[]; names?: Record; + protectedIds?: string[]; teamId: string | null; // null = manual flat list (directory queries as "default") installerId?: string; // authed_user — pre-added on managed connect (UX-027) installerName?: string; @@ -296,9 +326,18 @@ function PeopleRow({ {label(u)} {u === installerId && · you} - + {protectedIds?.includes(u) ? ( + + · owner + + ) : ( + + )} ))} @@ -314,10 +353,16 @@ function PersonPicker({ teamId, allowed, onChanged, + onPick, + buttonLabel = "＋ Add person", + testId, }: { teamId: string | null; allowed: string[]; onChanged: () => void; + onPick?: (member: SlackMember) => Promise<{ ok: boolean; error?: string }>; + buttonLabel?: string; + testId?: string; }) { const [open, setOpen] = useState(false); const [q, setQ] = useState(""); @@ -360,7 +405,13 @@ function PersonPicker({ }, [open]); const pick = async (m: SlackMember) => { - await allowUser("slack", m.id, teamId, m.name); + const result = onPick + ? await onPick(m) + : await allowUser("slack", m.id, teamId, m.name); + if (result?.ok === false) { + setErr(result.error || "could not add person"); + return; + } setOpen(false); setQ(""); onChanged(); @@ -372,11 +423,11 @@ function PersonPicker({ {open && (
; + installerId?: string; + installerName?: string; + editable: boolean; + onChanged: () => void; +}) { + const [err, setErr] = useState(null); + const label = (u: string) => + names?.[u] || (u === installerId ? installerName || "You" : u); + const remove = async (userId: string) => { + const result = await removeSlackApprovalOwner(userId); + if (!result.ok) { + setErr(result.error || "could not remove approval owner"); + return; + } + setErr(null); + onChanged(); + }; + return ( +
+ Approvals + + {owners.length === 0 && ( + + Choose at least one owner before routing Inbox approvals to Slack. + + )} + {owners.map((u) => ( + + + {initials(label(u))} + + {label(u)} + {u === installerId && · installer} + {editable && ( + + )} + + ))} + {editable && ( + addSlackApprovalOwner(m.id, m.name)} + buttonLabel="＋ Add owner" + testId="add-approval-owner" + /> + )} + {!editable && owners.length > 0 && ( + Set by the workspace installer. + )} + {err && {err}} + +
+ ); +} + function WaitingRow({ m, onChanged }: { m: ParkedMessage; onChanged: () => void }) { const act = async (action: "dismiss" | "allow" | "allow_deliver") => { await resolveUnauthorized("slack", m.id, action); diff --git a/tests/test_connectors.py b/tests/test_connectors.py index a6b7a557..cb155fa6 100644 --- a/tests/test_connectors.py +++ b/tests/test_connectors.py @@ -475,6 +475,31 @@ def test_connect_missing_required_field(tmp_path): assert res["ok"] is False and "missing" in res["error"] +def test_manual_slack_reconnect_preserves_approval_owners(tmp_path): + from coworker.connectors import connect_connector + + secrets = SecretStore(tmp_path / "secrets.json") + secrets.put( + "slack:default", + { + "bot_token": "xoxb-old", + "app_token": "xapp-old", + "allowed_users": ["U_OWNER"], + "approval_owner_ids": ["U_OWNER"], + }, + ) + result = connect_connector( + secrets, + "slack", + {"bot_token": "xoxb-new", "app_token": "xapp-new"}, + validate=False, + ) + assert result["ok"] is True + profile = secrets.get("slack:default") + assert profile["approval_owner_ids"] == ["U_OWNER"] + assert profile["allowed_users"] == ["U_OWNER"] + + def test_connect_validation_runs(tmp_path): from coworker.connectors import connect_connector from coworker.connectors.descriptors import ValidationResult, get_descriptor diff --git a/tests/test_fake_slack.py b/tests/test_fake_slack.py index e3d7dfe0..05df88a8 100644 --- a/tests/test_fake_slack.py +++ b/tests/test_fake_slack.py @@ -174,6 +174,7 @@ async def test_interaction_reaches_gateway_handler(fake_slack): ie = seen[0] assert ie.platform == "slack" assert ie.value == "item-id|allow" + assert ie.user_id == "U1" assert ie.user_name == "alice" assert ie.chat_id == "C1" assert ie.message_id == "1700000001.000001" diff --git a/tests/test_interactions.py b/tests/test_interactions.py index 9dc633db..8b5617d8 100644 --- a/tests/test_interactions.py +++ b/tests/test_interactions.py @@ -60,6 +60,15 @@ def test_slack_blocks_shape(): def test_interaction_click_resolves_item(tmp_path): mgr = SessionManager(workspace=tmp_path, provider=ScriptedProvider([])) + mgr.secrets.put( + "slack:default", + { + "bot_token": "xoxb-test", + "app_token": "xapp-test", + "allowed_users": ["U_BOB"], + "approval_owner_ids": ["U_BOB"], + }, + ) item = mgr.inbox.add_approval("sX", "Run `write_file`?") resolved: list = [] @@ -79,6 +88,7 @@ def test_interaction_click_resolves_item(tmp_path): chat_id="C1", message_id="111.2", value=encode(item.id, "allow"), + user_id="U_BOB", user_name="bob", ) ) diff --git a/tests/test_slack_approval_owners.py b/tests/test_slack_approval_owners.py new file mode 100644 index 00000000..6461d242 --- /dev/null +++ b/tests/test_slack_approval_owners.py @@ -0,0 +1,343 @@ +"""Slack approval ownership is distinct from ordinary inbound access.""" + +from __future__ import annotations + +import asyncio + +import httpx +from fastapi.testclient import TestClient + +from coworker.connectors import ConnectorSettings, Gateway, TeamAuth +from coworker.connectors.base import InteractionEvent, MessageEvent, SessionSource +from coworker.interactions import encode +from coworker.providers import ModelCapabilities, ProviderClient +from coworker.server import create_app +from coworker.server.manager import SessionManager + + +class NoTurnsProvider(ProviderClient): + def complete(self, *, model, messages, tools=None, **settings): + raise AssertionError("no model turns expected") + + def capabilities(self, model): + return ModelCapabilities() + + +def _manager(tmp_path) -> SessionManager: + return SessionManager(data_dir=tmp_path / "data", provider=NoTurnsProvider()) + + +def _manual_profile(manager: SessionManager, **extra) -> None: + manager.secrets.put( + "slack:default", + { + "bot_token": "xoxb-test", + "app_token": "xapp-test", + "enabled": True, + **extra, + }, + ) + + +def test_manual_owner_is_required_for_binding_and_implies_allowed(tmp_path): + manager = _manager(tmp_path) + _manual_profile(manager) + + denied = manager.set_inbox_binding( + "default", channel="slack", target="C_APPROVALS" + ) + assert denied["ok"] is False + + added = manager.set_slack_approval_owner( + "U_OWNER", add=True, display_name="Owner" + ) + assert added["ok"] is True + profile = manager.secrets.get("slack:default") + assert profile["approval_owner_ids"] == ["U_OWNER"] + assert profile["allowed_users"] == ["U_OWNER"] + + bound = manager.set_inbox_binding( + "default", channel="slack", target="C_APPROVALS" + ) + assert bound["ok"] is True + assert manager.disallow_user("slack", "U_OWNER")["ok"] is False + assert manager.set_slack_approval_owner("U_OWNER", add=False)["ok"] is False + + manager.set_slack_approval_owner("U_SECOND", add=True) + assert manager.set_slack_approval_owner("U_OWNER", add=False)["ok"] is True + assert manager.slack_approval_owner_ids() == {"U_SECOND"} + + +def test_manual_owner_rest_flow_surfaces_identity_and_binding(tmp_path): + manager = _manager(tmp_path) + _manual_profile(manager) + client = TestClient(create_app(manager)) + + added = client.post( + "/v1/connectors/slack/approval-owners/add", + json={"user_id": "U_OWNER", "name": "Ada"}, + ).json() + assert added["ok"] is True + + slack = next( + row + for row in client.get("/v1/connectors").json()["connectors"] + if row["name"] == "slack" + ) + assert slack["approval_owner_ids"] == ["U_OWNER"] + assert slack["approval_owner_names"] == {"U_OWNER": "Ada"} + + bound = client.post( + "/v1/inbox/routing/binding", + json={"name": "default", "channel": "slack", "target": "C_APPROVALS"}, + ).json() + assert bound["ok"] is True + + +def test_relay_binding_uses_installer_identity(tmp_path): + manager = _manager(tmp_path) + manager.secrets.put( + "slack:default", {"mode": "relay", "enabled": True, "managed": True} + ) + manager.secrets.put( + "slack:team:T1", + { + "bot_token": "xoxb-team", + "managed": True, + "slack_user_id": "U_INSTALLER", + "allowed_users": ["U_INSTALLER"], + }, + ) + assert manager.set_inbox_binding( + "default", channel="slack", target="T1/C_APPROVALS" + )["ok"] + + manager.secrets.put( + "slack:team:T2", + {"bot_token": "xoxb-team-2", "managed": True}, + ) + assert not manager.set_inbox_binding( + "default", channel="slack", target="T2/C_APPROVALS" + )["ok"] + + +def test_relay_does_not_reuse_dormant_manual_owners_for_bare_target(tmp_path): + manager = _manager(tmp_path) + manager.secrets.put( + "slack:default", + { + "mode": "relay", + "enabled": True, + "managed": True, + "bot_token": "xoxb-dormant", + "app_token": "xapp-dormant", + "approval_owner_ids": ["U_OLD_MANUAL_OWNER"], + }, + ) + assert manager.slack_approval_owner_ids() == set() + assert not manager.set_inbox_binding( + "default", channel="slack", target="C_AMBIGUOUS" + )["ok"] + + +def test_nonowner_cannot_resolve_approval_but_can_answer_question(tmp_path): + manager = _manager(tmp_path) + _manual_profile( + manager, + allowed_users=["U_OWNER", "U_MEMBER"], + approval_owner_ids=["U_OWNER"], + ) + + class GatewayStub: + def __init__(self): + self.rejections = [] + self.updates = [] + + async def reject_interaction(self, event, text=""): + self.rejections.append(event.user_id) + + async def update_message(self, *args): + self.updates.append(args) + + gateway = GatewayStub() + manager.gateway = gateway + approval = manager.inbox.add_approval("s1", "Run it?") + question = manager.inbox.add_question("s1", "Which one?", options=["A", "B"]) + + async def scenario(): + await manager._on_interaction( + InteractionEvent( + platform="slack", + chat_id="C1", + message_id="1", + value=encode(approval.id, "allow"), + user_id="U_MEMBER", + user_name="Member", + ) + ) + await manager._on_interaction( + InteractionEvent( + platform="slack", + chat_id="C1", + message_id="2", + value=encode(question.id, "A"), + user_id="U_MEMBER", + user_name="Member", + ) + ) + + asyncio.run(scenario()) + assert manager.inbox.get(approval.id).state == "pending" + assert manager.inbox.get(question.id).resolution == "A" + assert gateway.rejections == ["U_MEMBER"] + assert len(gateway.updates) == 1 + + +def test_owner_click_from_a_different_bound_channel_is_rejected(tmp_path): + manager = _manager(tmp_path) + _manual_profile( + manager, + allowed_users=["U_OWNER"], + approval_owner_ids=["U_OWNER"], + ) + assert manager.set_inbox_binding( + "default", channel="slack", target="C_EXPECTED" + )["ok"] + item = manager.inbox.add_approval("s1", "Run it?") + + class GatewayStub: + def __init__(self): + self.rejections = [] + + async def reject_interaction(self, event, text=""): + self.rejections.append(event.chat_id) + + gateway = GatewayStub() + manager.gateway = gateway + asyncio.run( + manager._on_interaction( + InteractionEvent( + platform="slack", + chat_id="C_OTHER", + message_id="1", + value=encode(item.id, "allow"), + user_id="U_OWNER", + ) + ) + ) + assert manager.inbox.get(item.id).state == "pending" + assert gateway.rejections == ["C_OTHER"] + + +def test_gateway_rejects_interaction_from_unallowed_actor(): + handled = [] + + async def handler(event): + handled.append(event) + + gateway = Gateway( + settings={ + "slack": ConnectorSettings( + platform="slack", + enabled=True, + teams={"T1": TeamAuth(allowed_users={"U_ALLOWED"})}, + ) + }, + interaction_handler=handler, + ) + + async def scenario(): + await gateway._on_interaction( + InteractionEvent( + platform="slack", + chat_id="T1/C1", + message_id="1", + value="ignored", + user_id="U_OTHER", + team_id="T1", + ) + ) + + asyncio.run(scenario()) + assert handled == [] + + +def test_rejected_click_uses_only_slacks_ephemeral_response_url(monkeypatch): + calls = [] + + def fake_post(url, **kwargs): + calls.append((url, kwargs)) + + monkeypatch.setattr(httpx, "post", fake_post) + gateway = Gateway(settings={}) + + async def scenario(): + await gateway.reject_interaction( + InteractionEvent( + platform="slack", + chat_id="C1", + message_id="1", + value="ignored", + response_url="https://hooks.slack.com/actions/abc", + ) + ) + await gateway.reject_interaction( + InteractionEvent( + platform="slack", + chat_id="C1", + message_id="1", + value="ignored", + response_url="https://hooks.slack.com.evil.example/actions/abc", + ) + ) + + asyncio.run(scenario()) + assert len(calls) == 1 + assert calls[0][0] == "https://hooks.slack.com/actions/abc" + assert calls[0][1]["json"]["response_type"] == "ephemeral" + assert "approval owner" in calls[0][1]["json"]["text"] + + +def test_slack_reply_token_is_owner_only_for_approvals(tmp_path): + manager = _manager(tmp_path) + _manual_profile( + manager, + allowed_users=["U_OWNER", "U_MEMBER"], + approval_owner_ids=["U_OWNER"], + ) + item = manager.inbox.add_approval("s1", "Run it?") + + unauthorized = MessageEvent( + text=f"approve [ow:{item.id}]", + source=SessionSource("slack", "C1", user_id="U_MEMBER"), + ) + assert manager._resolve_inbox_reply(unauthorized) is True + assert manager.inbox.get(item.id).state == "pending" + + owner = MessageEvent( + text=f"approve [ow:{item.id}]", + source=SessionSource("slack", "C1", user_id="U_OWNER"), + ) + assert manager._resolve_inbox_reply(owner) is True + assert manager.inbox.get(item.id).resolution == "allow" + + +def test_ownerless_legacy_binding_does_not_mirror(tmp_path): + manager = _manager(tmp_path) + _manual_profile(manager, allowed_users=["U_MEMBER"]) + manager.inbox_routing.set_binding( + "default", channel="slack", target="C_APPROVALS" + ) + item = manager.inbox.add_approval("s1", "Run it?") + + class GatewayStub: + def __init__(self): + self.deliveries = [] + + async def deliver_interactive(self, *args): + self.deliveries.append(args) + + gateway = GatewayStub() + manager.gateway = gateway + asyncio.run(manager.mirror_inbox_item(item)) + assert gateway.deliveries == [] diff --git a/tests/test_slack_relay.py b/tests/test_slack_relay.py index e031c2d7..b05ce807 100644 --- a/tests/test_slack_relay.py +++ b/tests/test_slack_relay.py @@ -261,9 +261,10 @@ async def test_relay_interactivity_maps_to_interaction(): "kind": "interactivity", "team_id": "T2", "interaction": { - "user": {"username": "bob"}, + "user": {"id": "U2", "username": "bob"}, "channel": {"id": "C7"}, "message": {"ts": "9.9"}, + "response_url": "https://hooks.slack.com/actions/abc", "actions": [{"value": "approve:42"}], }, } @@ -282,6 +283,9 @@ async def test_relay_interactivity_maps_to_interaction(): assert len(seen) == 1 assert seen[0].chat_id == "T2/C7" and seen[0].value == "approve:42" + assert seen[0].user_id == "U2" + assert seen[0].team_id == "T2" + assert seen[0].response_url == "https://hooks.slack.com/actions/abc" async def test_relay_revoked_drops_team(): diff --git a/tests/test_slack_workspaces.py b/tests/test_slack_workspaces.py index 4b1cd9ef..89aaf66e 100644 --- a/tests/test_slack_workspaces.py +++ b/tests/test_slack_workspaces.py @@ -33,6 +33,7 @@ def _install_form(team_id: str) -> dict: "team_id": team_id, "access_token": f"xoxb-{team_id}", "bot_user_id": "B1", + "slack_user_id": f"U_{team_id}", "account": f"Workspace {team_id}", "team_domain": f"dom-{team_id.lower()}", "connection_id": f"conn_{team_id}", @@ -71,6 +72,7 @@ def test_managed_callback_installs_and_hot_reloads(client, monkeypatch): if c["name"] == "slack" ][0] assert [w["domain"] for w in slack["workspaces"]] == ["dom-t1"] + assert slack["workspaces"][0]["approval_owner_ids"] == ["U_T1"] assert client.manager.secrets.get("slack:default")["mode"] == "relay" assert refreshes # new workspace's token loads without an app restart diff --git a/tests/test_team_allowlist.py b/tests/test_team_allowlist.py index 60e37ee5..37549145 100644 --- a/tests/test_team_allowlist.py +++ b/tests/test_team_allowlist.py @@ -260,4 +260,5 @@ def test_workspace_listing_carries_installer_identity(tmp_path): ) (w,) = _slack_workspaces(s) assert w["installer_user_id"] == "U_ME" + assert w["approval_owner_ids"] == ["U_ME"] assert w["installer_name"] == "" diff --git a/tests/test_ui_refresh_e2e.py b/tests/test_ui_refresh_e2e.py index 4281ecfc..edf00c27 100644 --- a/tests/test_ui_refresh_e2e.py +++ b/tests/test_ui_refresh_e2e.py @@ -154,6 +154,7 @@ async def test_ui_refresh_cross_cutting_e2e(fake_slack, tmp_path, monkeypatch): fake_slack.add_channel(CHANNEL, CHANNEL_NAME) # allow-list: the inbound gate drops unknown senders unless allowed assert mgr.allow_user("slack", USER)["ok"] is True + assert mgr.set_slack_approval_owner(USER, add=True)["ok"] is True # an Ops "incident" session subscribed to the channel, Unattended, approvals -> the channel mgr.session_store.save( @@ -166,7 +167,9 @@ async def test_ui_refresh_cross_cutting_e2e(fake_slack, tmp_path, monkeypatch): ) ) mgr.subscriptions.subscribe(SID, f"slack:{CHANNEL}") - mgr.inbox_routing.set_binding("ops-incidents", channel="slack", target=CHANNEL) + assert mgr.set_inbox_binding( + "ops-incidents", channel="slack", target=CHANNEL + )["ok"] mgr.inbox_routing.set_session_override(SID, "ops-incidents") mgr.unattended.set(SID, True)