Add Slack interactivity webhook support

services/hackbot-api/app/auth.py

@@ -5,6 +5,7 @@
 from fastapi import Header, HTTPException, Request, status
 from google.auth.transport import requests as google_requests
 from google.oauth2 import id_token
+from slack_sdk.signature import SignatureVerifier
 
 from app.config import settings
 
@@ -46,6 +47,50 @@ async def require_phabricator_signature(
         )
 
 
+def verify_slack_signature(
+    raw_body: bytes, timestamp: str | None, signature: str | None
+) -> bool:
+    """Constant-time-check Slack's `X-Slack-Signature` over the raw request body.
+
+    Slack signs `v0:{timestamp}:{body}` with the app's signing secret and sends the
+    digest as `v0=<hex>` in the header. `slack_sdk`'s verifier does that comparison
+    and additionally rejects a timestamp more than five minutes from now, which is
+    what stops a captured delivery from being replayed later. The Phabricator
+    signature has no such window, so this cannot simply reuse it.
+
+    Returns False if the secret is unconfigured or either header is missing or
+    garbled, so a service without `SLACK_SIGNING_SECRET` rejects every delivery
+    instead of accepting them all.
+    """
+    secret = settings.slack.signing_secret
+    if not secret or not timestamp or not signature:
+        return False
+    try:
+        return SignatureVerifier(secret).is_valid(raw_body, timestamp, signature)
+    except ValueError:
+        # A non-numeric timestamp header reaches an `int()` inside the verifier.
+        return False
+
+
+async def require_slack_signature(
+    request: Request,
+    x_slack_request_timestamp: str | None = Header(default=None),
+    x_slack_signature: str | None = Header(default=None),
+) -> None:
+    """Reject the request unless Slack's delivery signature is valid.
+
+    Same shape as `require_phabricator_signature`: the raw body is read here (and
+    cached by Starlette, so the route can read it again) because the signature
+    covers the bytes as sent, which a parse-and-reserialise would not reproduce.
+    """
+    raw = await request.body()
+    if not verify_slack_signature(raw, x_slack_request_timestamp, x_slack_signature):
+        raise HTTPException(
+            status_code=status.HTTP_401_UNAUTHORIZED,
+            detail="Invalid or missing Slack signature",
+        )
+
+
 async def require_api_key(x_api_key: str | None = Header(default=None)) -> None:
     if not settings.external_api_key:
         raise HTTPException(

services/hackbot-api/app/config.py

@@ -22,6 +22,17 @@ class WebhookSettings(BaseModel):
     dedupe_ttl_seconds: int = 6 * 60 * 60
 
 
+class SlackSettings(BaseModel):
+    """Inbound Slack interactivity config (clicks on the app's own messages).
+
+    Populated from SLACK_* env vars as part of the single settings parse.
+    """
+
+    # Slack's app-level signing secret (Basic Information -> App Credentials),
+    # used to verify the HMAC on every interaction delivery.
+    signing_secret: str
+
+
 class Settings(BaseSettings):
     # GCP
     gcp_project: str = ""
@@ -50,6 +61,8 @@ class Settings(BaseSettings):
     # Required via its `secret` field, so WEBHOOK_SECRET must be set at startup.
     webhook: WebhookSettings
 
+    slack: SlackSettings
+
     # The webhook receiver triggers runs over the public API (rather than calling
     # the DB/jobs internals directly), so splitting it into its own service later
     # is just a matter of repointing this at the remote API. While co-located,

services/hackbot-api/app/main.py

@@ -7,7 +7,12 @@
 from app import __version__
 from app.config import settings
 from app.database.connection import close_db, init_db
-from app.routers import events_router, runs_router, webhooks_router
+from app.routers import (
+    events_router,
+    runs_router,
+    slack_router,
+    webhooks_router,
+)
 
 if settings.sentry_dsn:
     sentry_sdk.init(
@@ -42,6 +47,7 @@ async def lifespan(app: FastAPI):
 app.include_router(runs_router)
 app.include_router(events_router)
 app.include_router(webhooks_router)
+app.include_router(slack_router)
 
 
 @app.get("/health")

services/hackbot-api/app/routers/__init__.py

@@ -1,5 +1,6 @@
 from app.routers.events import router as events_router
 from app.routers.runs import router as runs_router
+from app.routers.slack import router as slack_router
 from app.routers.webhooks import router as webhooks_router
 
-__all__ = ["events_router", "runs_router", "webhooks_router"]
+__all__ = ["events_router", "runs_router", "slack_router", "webhooks_router"]

services/hackbot-api/app/routers/slack.py

@@ -0,0 +1,98 @@
+"""Inbound Slack interactivity receiver: clicks on the messages hackbot posts.
+
+A message recorded with buttons (``hackbot_runtime.actions.slack.button``) is
+posted as Block Kit by the apply step; when someone clicks one, Slack POSTs the
+click here. Authenticated by Slack's HMAC signature rather than the ``X-API-Key``
+the other routes use, so this lives on its own router without ``require_api_key``,
+the same way the Phabricator receiver does.
+
+Slack app setup (one URL for the whole app, no new OAuth scopes, no reinstall):
+
+- Interactivity & Shortcuts -> Interactivity: on
+- Request URL: ``https://<hackbot-api-host>/slack/interactions``
+- ``SLACK_SIGNING_SECRET`` in this service's env, from Basic Information ->
+  App Credentials. Until it is set every delivery is rejected with a 401.
+
+Two constraints shape what may go in this route. Slack expects a response within
+**3 seconds** and shows the clicker an error if it does not arrive, so real work
+belongs off this request (publish an event, as ``run.completed`` does, and answer
+the message afterwards through ``response_url`` or ``chat.update``). And Slack
+retries a non-2xx delivery, so a payload this cannot act on is answered 200 and
+logged, not 4xx/5xx: a retry of it would fail identically while the person who
+clicked watches it fail.
+"""
+
+import logging
+
+from fastapi import APIRouter, Depends, Request, Response, status
+from hackbot_runtime.actions.slack import BUTTON_KINDS
+
+from app.auth import require_slack_signature
+from app.slack_webhook import parse_interaction
+
+log = logging.getLogger(__name__)
+
+router = APIRouter(prefix="/slack")
+
+
+@router.post(
+    "/interactions",
+    status_code=status.HTTP_200_OK,
+    dependencies=[Depends(require_slack_signature)],
+)
+async def slack_interactions(request: Request) -> Response:
+    # Already read (and cached) by the signature dependency: the signature covers
+    # the bytes as sent, and the form body is parsed from those same bytes.
+    click = parse_interaction(await request.body())
+    if click is None:
+        # Not a click this can act on. Already logged with the reason.
+        return Response(status_code=status.HTTP_200_OK)
+
+    if click.kind not in BUTTON_KINDS:
+        # A button whose kind no longer has a receiver: an older message still in
+        # a channel's history, or a kind retired without retiring its buttons.
+        log.warning(
+            "Slack: no receiver for button kind %r (from user %s in channel %s)",
+            click.kind,
+            click.user_id,
+            click.channel_id,
+        )
+        return Response(status_code=status.HTTP_200_OK)
+
+    log.info(
+        "Slack: %s clicked by %s (%s) in channel %s on message %s, args=%s",
+        click.kind,
+        click.user_name or "unknown",
+        click.user_id,
+        click.channel_id,
+        click.message_ts,
+        click.args,
+    )
+
+    # ACTION HANDLING GOES HERE
+    #
+    # The click is authenticated and parsed at this point; nothing acts on it yet.
+    # What belongs here, and what it needs from `click`:
+    #
+    # 1. Authorize the clicker. `click.user_id` is a Slack id, not an identity this
+    #    service trusts: resolve it to an email with `users.info` (needs the
+    #    `users:read` / `users:read.email` scopes, so a reinstall) and require the
+    #    same @mozilla.com bar the UI applies. Check `click.team_id` is the expected
+    #    workspace too. Fail closed.
+    # 2. Make it at-most-once. Slack retries deliveries and people double-click, so
+    #    the effect has to be keyed on something stable, e.g. (message_ts, kind),
+    #    in a row that only one caller can transition out of pending.
+    # 3. Hand the work off rather than doing it here, to stay inside the 3-second
+    #    budget: publish the click and let a push subscription act on it (see
+    #    `app/pubsub.py`), which is how run completions already reach their handler.
+    # 4. Answer the person who clicked, twice: strip the buttons immediately via
+    #    `click.response_url` so a second click has nothing to hit, then report the
+    #    outcome from the worker with `chat.update` on the message the posted action
+    #    recorded (`{"channel", "ts"}` in its result).
+    #
+    # For `trigger_bug_fix` specifically, `click.args` carries `bug_id` and the
+    # `run_id` of the triage run that proposed the fix, which is everything a
+    # `POST /agents/bug-fix/runs` needs (attributed to the clicker through
+    # `X-On-Behalf-Of`).
+
+    return Response(status_code=status.HTTP_200_OK)

services/hackbot-api/app/slack_webhook.py

@@ -0,0 +1,150 @@
+"""Slack interaction payload handling: parsing a verified delivery into a click.
+
+Slack posts every interaction with the app's messages to one URL, so this turns a
+delivery into either a :class:`ButtonClick` or None, and the route in
+``app/routers/slack.py`` decides what to do with it. Kept separate from the route
+for the same reason as ``app/phabricator_webhook.py``: payload shapes are worth
+testing without a client.
+
+Two things about the delivery are easy to get wrong. It is not JSON: the body is
+``application/x-www-form-urlencoded`` with the JSON in a single ``payload`` field,
+which is why this takes raw bytes (already needed for signature verification) and
+parses them itself rather than reading a model off the request. And a body that
+cannot be understood returns None rather than raising: it will not parse on a
+retry either, and a non-2xx makes Slack both retry it and show the person who
+clicked an error for something they cannot fix.
+"""
+
+from __future__ import annotations
+
+import json
+import logging
+from dataclasses import dataclass
+from typing import Any
+from urllib.parse import parse_qs
+
+log = logging.getLogger(__name__)
+
+# Only clicks on message elements are handled here. Slack sends other interaction
+# types to the same URL (`view_submission` when a modal is submitted,
+# `block_suggestion` for a select's options), which are ignored until something
+# records a button that needs them.
+BLOCK_ACTIONS = "block_actions"
+
+# The `v` an encoded button `value` must carry, matching
+# `hackbot_runtime.actions.slack.VALUE_VERSION` at the time this was written. A
+# click on a button posted before a shape change reports a version this does not
+# know, and is dropped rather than read with the wrong meaning.
+SUPPORTED_VALUE_VERSION = 1
+
+
+@dataclass(frozen=True)
+class ButtonClick:
+    """A click on one button of a message this app posted.
+
+    ``kind`` is the button's Slack ``action_id``, which is the kind the recording
+    side gave it (see ``hackbot_runtime.actions.slack.BUTTON_KINDS``), and ``args``
+    is what that side put on the button. Everything else identifies the click:
+    who, where, on which message, and the two single-use handles Slack provides
+    for replying (``response_url``, valid ~30 minutes) and for opening a modal
+    (``trigger_id``, valid ~3 seconds).
+    """
+
+    kind: str
+    args: dict[str, Any]
+    user_id: str
+    user_name: str | None
+    team_id: str | None
+    channel_id: str | None
+    message_ts: str | None
+    response_url: str | None
+    trigger_id: str | None
+
+
+def _decode_value(raw: str | None) -> dict[str, Any] | None:
+    """The args off a button's ``value``, or None if it is not one of ours."""
+    if not raw:
+        return None
+    try:
+        decoded = json.loads(raw)
+    except ValueError:
+        log.warning("Slack interaction: button value is not JSON")
+        return None
+    if not isinstance(decoded, dict) or decoded.get("v") != SUPPORTED_VALUE_VERSION:
+        log.warning(
+            "Slack interaction: unsupported button value version %r",
+            (decoded or {}).get("v") if isinstance(decoded, dict) else None,
+        )
+        return None
+    args = decoded.get("args")
+    return args if isinstance(args, dict) else {}
+
+
+def parse_payload(payload: dict[str, Any]) -> ButtonClick | None:
+    """Turn an interaction payload into a :class:`ButtonClick`, or None.
+
+    None covers every payload this cannot act on: another interaction type, a
+    click carrying no action, or a button whose value did not come from a version
+    of the recording side this understands. Each is logged, since a button that
+    silently does nothing is indistinguishable from a broken receiver.
+    """
+    kind_of_payload = payload.get("type")
+    if kind_of_payload != BLOCK_ACTIONS:
+        log.info("Ignoring Slack interaction of type %r", kind_of_payload)
+        return None
+
+    actions = payload.get("actions") or []
+    # A click reports exactly one action even in a block of several buttons, so
+    # anything past the first would be a payload shape this does not know.
+    action = actions[0] if actions else None
+    if not isinstance(action, dict) or not action.get("action_id"):
+        log.warning("Ignoring Slack %s delivery with no action", BLOCK_ACTIONS)
+        return None
+
+    args = _decode_value(action.get("value"))
+    if args is None:
+        return None
+
+    user = payload.get("user") or {}
+    if not user.get("id"):
+        # Every real click names its user; without one there is nobody to
+        # authorize, so this is a payload to drop rather than guess at.
+        log.warning("Ignoring Slack %s delivery with no user", BLOCK_ACTIONS)
+        return None
+
+    return ButtonClick(
+        kind=action["action_id"],
+        args=args,
+        user_id=user["id"],
+        user_name=user.get("username") or user.get("name"),
+        team_id=(payload.get("team") or {}).get("id"),
+        channel_id=(payload.get("channel") or {}).get("id"),
+        message_ts=(payload.get("message") or {}).get("ts"),
+        response_url=payload.get("response_url"),
+        trigger_id=payload.get("trigger_id"),
+    )
+
+
+def parse_interaction(raw_body: bytes) -> ButtonClick | None:
+    """Parse a raw interaction delivery: form body, then ``payload`` JSON."""
+    try:
+        form = parse_qs(raw_body.decode("utf-8"))
+    except UnicodeDecodeError:
+        log.warning("Slack interaction: body is not UTF-8")
+        return None
+
+    encoded = form.get("payload")
+    if not encoded:
+        log.warning("Slack interaction: body has no payload field")
+        return None
+
+    try:
+        payload = json.loads(encoded[0])
+    except ValueError:
+        log.warning("Slack interaction: payload is not JSON")
+        return None
+    if not isinstance(payload, dict):
+        log.warning("Slack interaction: payload is not an object")
+        return None
+
+    return parse_payload(payload)

services/hackbot-api/pyproject.toml

@@ -18,6 +18,7 @@ dependencies = [
     "google-auth>=2.29.0",
     "sentry-sdk>=2.51.0",
     "cachetools>=5.3.0",
+    "slack-sdk>=3.27.0",
     "httpx>=0.26.0",
     "hackbot-runtime",
     "phabricator-client",

services/hackbot-api/tests/conftest.py

@@ -1,9 +1,10 @@
 import os
 
 # The global Settings embeds required nested models, validated when Settings() is
-# built at import: PhabricatorSettings needs a 32-char api_key, and
-# WebhookSettings needs a secret. Provide dummies here (before app.config is
-# imported) so the suite imports even in tests that don't exercise these.
-# `setdefault` leaves any real env value intact.
+# built at import: PhabricatorSettings needs a 32-char api_key, WebhookSettings
+# needs a secret, and SlackSettings needs a signing secret. Provide dummies here
+# (before app.config is imported) so the suite imports even in tests that don't
+# exercise these. `setdefault` leaves any real env value intact.
 os.environ.setdefault("PHABRICATOR_API_KEY", "api-" + "a" * 28)
 os.environ.setdefault("WEBHOOK_SECRET", "test-webhook-secret")
+os.environ.setdefault("SLACK_SIGNING_SECRET", "test-signing-secret")