Add Slack interactivity webhook support

docs/hackbot/api.md

@@ -27,10 +27,54 @@ the download to that run's prefix and prevents probing unrelated objects.
 
 ### Inbound webhooks — HMAC signature
 
-| POST | `/webhooks/phabricator` | `@hackbot` mention on a revision triggers a bug-fix run |
-
-Authenticated by Phabricator's own HMAC signature over the raw body, so it sits on its own
-router without the API-key dependency. See [triggers.md](triggers.md).
+| Method | Path                           | Does                                                    |
+| ------ | ------------------------------ | ------------------------------------------------------- |
+| POST   | `/webhooks/phabricator`        | `@hackbot` mention on a revision triggers a bug-fix run |
+| POST   | `/webhooks/slack/interactions` | A click on an interactive element of a hackbot message  |
+
+Each is authenticated by the sender's own HMAC signature over the raw body, so each sits on
+its own router without the API-key dependency: neither sender can send an `X-API-Key`. The
+Phabricator receiver is covered in [triggers.md](triggers.md).
+
+**Slack interactions** all arrive on this one route: Slack posts every click on every
+interactive element to the single Request URL configured for Interactivity, so the receiver
+demultiplexes on the element's `action_id`. The path names the feature because Slack
+configures one Request URL **per feature** — Event Subscriptions and Slash Commands are
+separate URLs with their own payload shapes, and they would get their own routes beside this
+one rather than sharing it. Three things about the delivery shape the route:
+
+- The body is **not JSON**. It is `application/x-www-form-urlencoded` with the JSON in a
+  single `payload` field, parsed from the same raw bytes the signature covers.
+- **Slack expects a response within 3 seconds** and shows the person who clicked an error if
+  it does not arrive, so real work belongs off the request: publish an event and answer the
+  message afterwards through the delivery's `response_url` or `chat.update`.
+- **A delivery this cannot act on is answered `200` and logged**, not `4xx`/`5xx`. Slack
+  retries a non-2xx, and a payload that cannot be parsed will not parse on retry either,
+  so refusing it only shows a failure nobody can fix.
+
+Nothing posts an interactive element yet, so nothing reaches this route in practice: it
+authenticates a delivery, parses it, and records that it happened. Acting on a click, and
+the authorization that has to come first, lands with the first button. A click that starts
+a run will then appear in [triggers.md](triggers.md).
+
+What the endpoint answers:
+
+| Delivery                                           | Status |
+| -------------------------------------------------- | ------ |
+| Signature verifies                                 | `200`  |
+| A signature header is absent (a malformed request) | `422`  |
+| Both headers present, signature or freshness fails | `401`  |
+| Signed, but the payload cannot be understood       | `200`  |
+
+The two signature headers are declared required, so an absent one is a validation failure
+rather than an authentication one. Slack always sends both, so a delivery missing them is
+not a Slack delivery.
+
+Turning it on is Slack-app config, not a deploy: **Interactivity & Shortcuts → Request URL**
+= `https://<hackbot-api-host>/webhooks/slack/interactions`, and `SLACK_SIGNING_SECRET` from
+**Basic Information → App Credentials**. Interactivity needs no new OAuth scopes, so no
+workspace reinstall. The secret is not optional: without a usable one the service does not
+start at all (see below).
 
 ## Creating a run
 
@@ -122,8 +166,13 @@ idempotent — see [actions.md](actions.md).
 Commands and the full config reference are in [deployment.md](deployment.md). Two things
 specific to this service:
 
-- **`WEBHOOK_SECRET` has no default**, so a missing one fails at startup rather than
-  silently accepting or rejecting deliveries.
+- **`WEBHOOK_SECRET` and `SLACK_SIGNING_SECRET` have no defaults**, so a missing one fails
+  at startup rather than silently accepting or rejecting deliveries. Both are HMAC keys for
+  an inbound receiver: an empty one would mean either accepting every delivery
+  unauthenticated or rejecting every real one, and neither is a state worth booting into.
+  `SLACK_SIGNING_SECRET` is validated **non-blank** rather than merely present, so `=""`
+  fails at startup too, and every consumer downstream can take a usable key for granted
+  instead of carrying an unconfigured case.
 - **Signing GCS URLs needs an impersonating credential** —
   `gcloud auth application-default login --impersonate-service-account=<sa>`. See
   [security.md](security.md) for why, and what the deployed service needs instead.

docs/hackbot/deployment.md

@@ -64,7 +64,14 @@ fastest:
 Two things those files do not tell you:
 
 - **Nested models bind from prefixed vars**, splitting on the first underscore only:
-  `PHABRICATOR_API_KEY` → `phabricator.api_key`, `WEBHOOK_SECRET` → `webhook.secret`.
+  `PHABRICATOR_API_KEY` → `phabricator.api_key`, `WEBHOOK_SECRET` → `webhook.secret`,
+  `SLACK_SIGNING_SECRET` → `slack.signing_secret`.
+- **A prefix does not make a var a setting.** `SLACK_BOT_TOKEN` looks like it belongs to the
+  same nested model and does not: it is read straight from the environment by the apply-side
+  Slack handler in `hackbot-runtime`, which has no access to these settings. The two live
+  side by side on hackbot-api and serve opposite directions — the token posts messages, the
+  signing secret verifies clicks coming back — so a deployment that posts fine can still
+  reject every interaction, and vice versa.
 - **An agent container's env arrives from three places.** Per-execution overrides from the
   API (`RUN_ID`, the results bucket/prefix/policy, one var per input-schema field); static
   Job env fixed at deploy time (`BROKER_URL`, `SOURCE_REPO`, the Anthropic federation ids,

docs/hackbot/security.md

@@ -70,14 +70,24 @@ delegating `sign_bytes` to the IAM `signBlob` API. This is why its service accou
 
 ## Authenticating callers of hackbot-api
 
-Three distinct schemes, one per class of caller:
+Four distinct schemes, one per class of caller:
 
 | Caller                      | Scheme                                                                                 |
 | --------------------------- | -------------------------------------------------------------------------------------- |
 | UI, pulse listener, scripts | `X-API-Key`, compared in constant time                                                 |
 | Phabricator                 | HMAC-SHA256 over the raw body, constant-time compared                                  |
+| Slack                       | HMAC-SHA256 over `v0:{timestamp}:{raw body}`, plus a 5-minute timestamp window         |
 | Eventarc / Pub/Sub push     | Google-signed OIDC bearer token, verified for audience **and** issuing service account |
 
+The two HMAC schemes are not interchangeable. Slack signs a base string that **includes the
+delivery's timestamp**, and that timestamp is checked against the clock, so a captured
+delivery cannot be replayed later: refreshing the timestamp invalidates the signature, and
+keeping it puts the delivery outside the window. The Phabricator signature covers the body
+alone and has no such window, which is why the Slack receiver has its own verifier rather
+than reusing the existing one. Neither can run without its key: the Phabricator secret is
+required, and the Slack one is required **and** validated non-blank, so a deployment without
+a usable key fails to start rather than quietly rejecting every delivery it receives.
+
 The push-token check is not redundant with platform IAM. The service allows unauthenticated
 invocations — that is how API-key callers reach it at all — so IAM on the subscription does
 not protect these routes on its own. The token is verified in the route.
@@ -95,6 +105,19 @@ A webhook signature proves the delivery came from Phabricator; it says nothing a
 commented. So the comment author must additionally be a member of the `bmo-editbugs-team`
 project. [triggers.md](triggers.md) covers that check and the other guards on the path.
 
+## Authorizing Slack clicks
+
+The same split applies, and the second half is not built yet. A valid signature proves the
+delivery came from the Slack app; it says nothing about _who_ clicked, and a Slack user id
+is not an identity this platform trusts. Resolving one to a `@mozilla.com` address (through
+`users.info`, which needs the `users:read` and `users:read.email` scopes) and checking the
+click's workspace is what a click would need before it could cause anything.
+
+Until then the receiver is deliberately inert: it verifies, parses and logs, and no click
+path reaches an effect. That ordering is the point — the authorization lands in the same
+change as the first button, so no interactive element ever exists ahead of the check that
+guards it. See [api.md](api.md).
+
 ## Recorded actions as a review gate
 
 The record-then-apply split ([actions.md](actions.md)) is a security property as much as a

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,25 @@ async def require_phabricator_signature(
         )
 
 
+async def require_slack_signature(
+    request: Request,
+    x_slack_request_timestamp: str = Header(),
+    x_slack_signature: str = Header(),
+) -> None:
+    verifier = SignatureVerifier(settings.slack.signing_secret)
+    raw = await request.body()
+    try:
+        valid = verifier.is_valid(raw, x_slack_request_timestamp, x_slack_signature)
+    except ValueError:
+        valid = False
+
+    if not valid:
+        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

@@ -1,7 +1,14 @@
+from typing import Annotated
+
 from phabricator_client import PhabricatorSettings
-from pydantic import BaseModel
+from pydantic import BaseModel, StringConstraints
 from pydantic_settings import BaseSettings
 
+# An HMAC key that must actually be a key. Required alone only rejects a *missing*
+# value, and an empty or whitespace one would start a service that rejects every
+# delivery it receives -- worse than not starting, because nothing says so.
+HmacSecret = Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)]
+
 
 class WebhookSettings(BaseModel):
     """Inbound webhook receiver config (Phabricator "@hackbot" mentions).
@@ -22,6 +29,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, verifying the HMAC on every interaction
+    # delivery.
+    signing_secret: HmacSecret
+
+
 class Settings(BaseSettings):
     # GCP
     gcp_project: str = ""
@@ -50,6 +68,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,61 @@
+"""Inbound Slack interactivity receiver: clicks on the messages hackbot posts.
+
+``docs/hackbot/api.md`` covers the endpoint, the delivery shape and the Slack-app
+config; ``docs/hackbot/security.md`` covers the signature and what it does not
+prove.
+"""
+
+import logging
+
+from fastapi import APIRouter, Depends, Request, Response, status
+
+from app.auth import require_slack_signature
+from app.slack_webhook import parse_interaction
+
+log = logging.getLogger(__name__)
+
+# Under `/webhooks` with the Phabricator receiver, since both are signature-verified
+# inbound deliveries, and one level deeper because Slack configures a Request URL per
+# feature: Event Subscriptions and Slash Commands are separate URLs with their own
+# payload shapes, and they belong beside this one rather than sharing it.
+router = APIRouter(prefix="/webhooks/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)
+
+    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; nothing acts on it. What is still
+    # missing, in the order it has to happen (the reasoning is in
+    # `docs/hackbot/api.md` and `docs/hackbot/security.md`):
+    #
+    # 1. Dispatch on `click.kind` against the kinds that have a receiver, and
+    #    answer 200 for one that does not.
+    # 2. Authorize the clicker from `click.user_id` and `click.team_id`.
+    # 3. Make the effect at-most-once, keyed on something stable such as
+    #    (`click.message_ts`, `click.kind`).
+    # 4. Publish the click and act on it off this request (see `app/pubsub.py`).
+    # 5. Answer through `click.response_url`, then `chat.update` the message.
+
+    return Response(status_code=status.HTTP_200_OK)