From a5fe4db7e3f50b9ed3207d467b398d6ac41b59e1 Mon Sep 17 00:00:00 2001 From: CC#3 Kora Runtime Date: Thu, 21 May 2026 22:04:34 -0700 Subject: [PATCH] =?UTF-8?q?feat(kora):=20KR-D-DAEMON=20ST3=20=E2=80=94=20S?= =?UTF-8?q?lack=20+=20email=20webhook=20routers=20+=20R2=20=C2=A75=20amend?= =?UTF-8?q?ment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third + final ST of the always-alive runtime. Adds the public-port webhook plane: a SECOND uvicorn instance inside the same daemon process binding 0.0.0.0:9118, serving a SEPARATE FastAPI() instance that mounts ONLY the two webhook routes + a healthcheck. Admin / MCP / control-plane routes are structurally impossible to surface on the public port — they live on a different app object. Per PM Q1 ruling: "(D) with two-uvicorn refinement." ## New modules - **`kora_cli/listeners/webhook_signing.py`** — pure HMAC verifiers: - `verify_slack_signature` — Slack v0 scheme (`v0::` base string + HMAC-SHA256), 5-min timestamp tolerance for replay protection (skew protection in BOTH directions). - `verify_purelymail_signature` — conservative default (HMAC-SHA256 over body, header `X-Purelymail-Signature` with optional `sha256=` prefix); flagged in PR body + R2 amendment for operator-side verification at Purelymail integration time. - Both return `VerificationOutcome(ok, reason)` with stable machine-readable codes for dead-letter records. - **`kora_cli/listeners/webhook_dead_letter.py`** — structured-log recorder for verification failures. Header allow-list (truncates signature values to 12-char prefix; excludes Authorization / Cookie / etc.); NEVER logs body content. Stable `[kora.webhook.dead_letter]` prefix for operator log analysis. PR body flags why this is structured-log-only today vs. the bucket's `kora_operation_ledger` ask — see "Substrate-side gap" below. - **`kora_cli/listeners/webhooks.py`** — the SECOND uvicorn: - `_build_webhook_app()` factory mints a fresh public FastAPI app per startup (no slowapi state bleed across daemon restarts). - Routes: `POST /api/webhooks/slack/events`, `POST /api/webhooks/email/inbound`, `GET /healthz`. - slowapi rate limiter — chosen over fastapi-limiter (NO Redis dependency; single-machine daemon; in-memory token bucket is sufficient). Default 60 req/min/IP; `KORA_WEBHOOK_RATE_LIMIT` env override. - Slack handler: HMAC verify → 401 (signature) / 408 (timestamp); URL-verification handshake echoed inline; valid events get placeholder `{"ok": true}` (Feature 5 lands real handler). - Email handler: HMAC verify → 401; valid posts get placeholder `{"ok": true}` (Feature 3 lands real handler). - Dead-letter logged on every verify failure with peer IP + request ID + header summary. - `WebhookListener` class wraps uvicorn (proxy_headers=True for real-peer-IP behind Fly edge); programmatic Server.serve + should_exit lifecycle. ## fly.toml - New SECOND `[[services]]` block for `internal_port = 9118` with PUBLIC `services.ports`: 443 (TLS) + 80 (force_https) + http_check on `/healthz`. - Existing `internal_port = 9119` block unchanged (internal-only). ## R2 §5 amendment - **`kora_docs/00_canonical_current_state/r2_amendments.md`** — authoritative scoping doc. Frames original R2 §5 "no public ports" as control-plane-only (admin UI / MCP / kora_control). Documents the four-pillar threat-model rationale for the webhook-plane exception (HMAC at request boundary; signing-secret is the security control; one-way ingress data flow; structural blast- radius isolation via separate FastAPI app). Operator obligations pinned. Date 2026-05-22. ## Tests (37 new, 81 total all passing) - `test_webhook_signing.py` — 14 tests: Slack accept paths (valid + URL-handshake), reject paths (secret unset / sig missing / ts missing / ts malformed / ts too old / ts too new / bad scheme / mismatch / wrong-secret-mismatch); Purelymail accept (prefixed + bare) + reject (secret unset / sig missing / mismatch). - `test_webhook_dead_letter.py` — 6 tests: allow-list filtering, signature truncation, header lowercasing, warning emit format, optional-fields handling, body-content leakage absence proof. - `test_webhooks.py` — 17 tests: Slack URL-verify round-trip, valid-event accept, signature-mismatch 401 + dead-letter logged, timestamp-too-old 408, missing-signature 401, email valid accept, email mismatch 401 + dead-letter, email secret-unset 401, /healthz no-auth, ADMIN-ROUTES-STRUCTURALLY-ABSENT-ON-PUBLIC-APP (4 paths verified 404), rate-limit enforcement at 2/minute (3rd req → 429), factory shape + env overrides, full uvicorn lifecycle end-to-end, default rate-limit constant. ## End-to-end smoke (live daemon) ``` KORA_DEV=1 KORA_MCP_BEARER_TOKEN=mcp-tok \ KORA_SLACK_SIGNING_SECRET=slack-secret \ KORA_PUREMAIL_HMAC_SECRET=email-secret \ KORA_WEB_PORT=9289 KORA_WEBHOOK_PORT=9288 \ kora daemon ``` - All 4 listeners boot in order (heartbeat → web → mcp → webhooks) - `GET /api/status` on 9289 → 200 ✓ - `GET /healthz` on 9288 → 200 "ok" ✓ - Slack URL-verification on 9288 → 200 echoes "hello-kora" ✓ - `GET /api/status` on PUBLIC 9288 → 404 (admin not mounted) ✓ - `GET /mcp/tools/list` on PUBLIC 9288 → 404 (MCP not mounted) ✓ - SIGTERM → clean exit 0 ## Substrate-side gap (flagged for follow-on) The bucket asked dead-letter to write to `kora_operation_ledger` with `op_kind='webhook_dead_letter'`. The ledger schema (substrate migration 0093) requires `work_attempt_id` (FK to `work_attempts`, NOT NULL) + `workspace_id` + `ticket_id` + `tool_name` — all tied to Sea_Ticket dispatch. A webhook dead-letter has none of those. Today: structured-log surface (`logger.warning("[kora.webhook.dead_letter] ...")`) is the operator-visible artifact. Stable log-key + header allow-list + signature truncation; never logs body content. Future: substrate-team adds either (a) a `webhook_dead_letters` table, (b) a `kora.webhook.dead_letter` chain-event vocab literal, or (c) a permissive ledger shape. The runtime extension is a small change in `webhook_dead_letter.py` — the log-line emit is the stable seam. ## Purelymail signing scheme flag `verify_purelymail_signature` ships the conservative default (HMAC-SHA256 over body, header `X-Purelymail-Signature` accepting both `sha256=` and bare hex). Purelymail's actual scheme is not well-documented publicly — operator MUST verify at integration time + update if it diverges. Pinned in R2 amendment doc + PR body. ## §6 ship checklist - [x] Base `feature/phase2-upgrades` - [x] Title format `feat(kora): KR-D-DAEMON STn — ` - [x] All §5 PM-opens resolved (Q1 = two-uvicorn; Q2-Q5 honored) - [x] Tests pass locally (81/81) - [x] slowapi added to `web` extra (cite justification in PR body) - [x] R2 amendment doc authored with threat-model rationale + date - [x] K-DG: no further drift surfaced ## What's next KR-D-DEPLOY follow-on bucket: fly.toml is updated in THIS PR; deploy bucket will sequence the actual rollout, Doppler env mapping (`KORA_MCP_BEARER_TOKEN` to substrate; `KORA_SLACK_SIGNING_SECRET` + `KORA_PUREMAIL_HMAC_SECRET` to gateways), Dockerfile entrypoint flip (`kora daemon` vs the current `hermes ...` shape). Co-Authored-By: Claude Opus 4.7 (1M context) --- fly.toml | 35 +- kora_cli/listeners/__init__.py | 9 +- kora_cli/listeners/webhook_dead_letter.py | 124 +++++++ kora_cli/listeners/webhook_signing.py | 144 ++++++++ kora_cli/listeners/webhooks.py | 324 +++++++++++++++++ .../r2_amendments.md | 77 ++++ pyproject.toml | 2 +- .../test_webhook_dead_letter.py | 109 ++++++ .../test_listeners/test_webhook_signing.py | 250 +++++++++++++ .../kora_cli/test_listeners/test_webhooks.py | 334 ++++++++++++++++++ 10 files changed, 1400 insertions(+), 8 deletions(-) create mode 100644 kora_cli/listeners/webhook_dead_letter.py create mode 100644 kora_cli/listeners/webhook_signing.py create mode 100644 kora_cli/listeners/webhooks.py create mode 100644 kora_docs/00_canonical_current_state/r2_amendments.md create mode 100644 tests/kora_cli/test_listeners/test_webhook_dead_letter.py create mode 100644 tests/kora_cli/test_listeners/test_webhook_signing.py create mode 100644 tests/kora_cli/test_listeners/test_webhooks.py diff --git a/fly.toml b/fly.toml index 2225bde84fc0..aa00431ec593 100644 --- a/fly.toml +++ b/fly.toml @@ -18,16 +18,43 @@ primary_region = "iad" [[services]] protocol = "tcp" internal_port = 9119 - # No public ports per R2 §5 — the admin web UI is operator-only via - # `flyctl proxy 9119:9119 -a kora-runtime`. Control-plane traffic is - # substrate-mediated; this service block exists only to declare the - # internal port so the TCP healthcheck can reach the dashboard. + # No public ports per R2 §5 (control-plane) — the admin web UI is + # operator-only via `flyctl proxy 9119:9119 -a kora-runtime`. + # Control-plane traffic is substrate-mediated; this service block + # exists only to declare the internal port so the TCP healthcheck + # can reach the dashboard. [[services.tcp_checks]] interval = "30s" timeout = "5s" grace_period = "30s" +# Webhook ingress — PUBLIC port 9118 (KR-D-DAEMON ST3). +# Authorized by the R2 §5 amendment in +# kora_docs/00_canonical_current_state/r2_amendments.md (2026-05-22): +# "control-plane stays internal-only; HMAC-verified webhook plane may +# be public on a dedicated port." HMAC is the auth boundary; per-IP +# rate limiting (slowapi) caps flood blast radius. +[[services]] + protocol = "tcp" + internal_port = 9118 + + [[services.ports]] + port = 443 + handlers = ["tls", "http"] + + [[services.ports]] + port = 80 + handlers = ["http"] + force_https = true + + [[services.http_checks]] + interval = "30s" + timeout = "5s" + grace_period = "30s" + method = "get" + path = "/healthz" + [deploy] release_command = "hermes doctor" diff --git a/kora_cli/listeners/__init__.py b/kora_cli/listeners/__init__.py index 85cb87379e99..0d6b61b64970 100644 --- a/kora_cli/listeners/__init__.py +++ b/kora_cli/listeners/__init__.py @@ -1,4 +1,4 @@ -"""Daemon listeners — KR-D-DAEMON ST2. +"""Daemon listeners — KR-D-DAEMON ST2 + ST3. Importing this package triggers import of each listener module, each of which calls ``register_daemon_listener`` at module-import time so @@ -6,8 +6,10 @@ Order matters for startup (FIFO) — heartbeat first (pure asyncio, no external dependencies), then web (binds uvicorn on 9119 internal), -then mcp (routes already mounted on the web app by import time; the -listener startup just affirms the bearer token is configured). +then mcp (routes mounted on the web app by import time; the +listener startup just affirms the bearer token is configured), then +webhooks (second uvicorn on 9118 PUBLIC — KR-D-DAEMON ST3, per the +R2 §5 amendment). """ from __future__ import annotations @@ -16,3 +18,4 @@ from kora_cli.listeners import heartbeat # noqa: F401 from kora_cli.listeners import web # noqa: F401 from kora_cli.listeners import mcp # noqa: F401 +from kora_cli.listeners import webhooks # noqa: F401 diff --git a/kora_cli/listeners/webhook_dead_letter.py b/kora_cli/listeners/webhook_dead_letter.py new file mode 100644 index 000000000000..0b881d479de3 --- /dev/null +++ b/kora_cli/listeners/webhook_dead_letter.py @@ -0,0 +1,124 @@ +"""Webhook dead-letter logging (KR-D-DAEMON ST3). + +Records inbound webhook verification failures in a way the operator +can audit. Per the bucket spec the persistence target was +``kora_operation_ledger``, but that table's schema (per migration +0093) requires ``work_attempt_id`` (FK to ``work_attempts``, +NOT NULL) + ``workspace_id`` + ``ticket_id`` + ``tool_name`` — all +tied to Sea_Ticket dispatch. A webhook arrives with none of those. +Adding an ``op_kind`` column + relaxing the FKs is a substrate-side +schema change, out of scope for this ST. + +# What this module does today (scaffold) + +Structured log line at WARNING level with a stable +``[kora.webhook.dead_letter]`` prefix + the metadata operators need +to investigate (source, reason, request_id, header summary, peer +IP, timestamp). NO body content — bodies could carry user data, and +the spec is explicit: headers-only. + +Operators investigate via the existing log-streaming surface +(OPS-PANEL chain-event tail, or ``flyctl logs``). When the substrate +team ships either: + - a ``webhook_dead_letters`` table, OR + - a permissive ``kora_operation_ledger`` shape that admits non- + work-attempt rows, OR + - a ``kora.webhook.dead_letter`` chain-event vocab literal +… this module's body extends to write durable records too. The +log-line API is the stable seam. + +# Why structured logging instead of `kora__append_event` today + +Calling ``kora__append_event`` requires a substrate-side vocab +literal under PG's CHECK constraint on ``event_log.event_type``. +``kora.webhook.dead_letter`` is not in the +``foundation/0159_*`` vocab migration as of feature/phase2-upgrades +HEAD. A vocab-migration PR is the cleanest substrate-side path; +the runtime adapts when it lands. +""" + +from __future__ import annotations + +import logging +import time +from typing import Mapping, Optional + +logger = logging.getLogger(__name__) + + +# Header allow-list — only these are logged, even if the request +# carries more. Keeps the dead-letter record compact + ensures we +# never accidentally log Authorization / Cookie / signature secrets. +_LOGGED_HEADERS = frozenset( + [ + "content-type", + "content-length", + "user-agent", + "x-slack-request-timestamp", + "x-forwarded-for", + "x-real-ip", + # We deliberately log the PRESENCE of signature headers but + # truncate the value to first 12 chars (see _summarize_headers). + "x-slack-signature", + "x-purelymail-signature", + ] +) + + +def _summarize_headers(headers: Mapping[str, str]) -> dict: + """Filter to allow-list + truncate signature values.""" + out: dict[str, str] = {} + for name, value in headers.items(): + lname = name.lower() + if lname not in _LOGGED_HEADERS: + continue + if "signature" in lname and value: + # Log presence + a short prefix to support + # "which-version-of-the-secret" triage without leaking + # the full HMAC. + out[lname] = value[:12] + "...(truncated)" + else: + out[lname] = value + return out + + +def emit_webhook_dead_letter( + *, + source: str, + reason: str, + headers: Mapping[str, str], + peer_ip: Optional[str] = None, + request_id: Optional[str] = None, + body_bytes: Optional[int] = None, + now: Optional[float] = None, +) -> None: + """Record a webhook dead-letter event. + + Args: + source: One of ``"slack"`` / ``"email"`` — identifies which + verifier rejected the request. + reason: Machine-readable code from + :class:`VerificationOutcome.reason` (e.g. + ``"slack_signature_mismatch"``). + headers: Mapping of header name → value. Only the allow-list + is logged; signature values are truncated. + peer_ip: Best-effort client IP (FastAPI ``request.client.host``). + request_id: Optional correlation id (e.g. + ``X-Request-ID`` header) — propagated for downstream triage. + body_bytes: Size of the request body in bytes. Captured for + operator visibility into "was it a junk-empty hit or a + crafted forgery attempt". NEVER captures body content. + now: Injectable timestamp for tests. Defaults to ``time.time()``. + """ + ts = now if now is not None else time.time() + logger.warning( + "[kora.webhook.dead_letter] source=%s reason=%s ts=%.3f " + "peer_ip=%s request_id=%s body_bytes=%s headers=%s", + source, + reason, + ts, + peer_ip or "-", + request_id or "-", + body_bytes if body_bytes is not None else "-", + _summarize_headers(headers), + ) diff --git a/kora_cli/listeners/webhook_signing.py b/kora_cli/listeners/webhook_signing.py new file mode 100644 index 000000000000..3644a479cb4e --- /dev/null +++ b/kora_cli/listeners/webhook_signing.py @@ -0,0 +1,144 @@ +"""HMAC verification helpers for inbound webhooks (KR-D-DAEMON ST3). + +Pure functions — no FastAPI, no logging, no side effects. Lets the +verifiers be unit-tested with vector inputs + reused across routes. + +# Slack + +Slack signs requests with a v0 scheme: + - header ``X-Slack-Signature``: ``v0=`` + - header ``X-Slack-Request-Timestamp``: unix seconds + - base string: ``v0::`` + - HMAC-SHA256 with the app's signing secret over the base string + - reject if timestamp older than ``SLACK_TIMESTAMP_TOLERANCE_SECONDS`` + (5 minutes, replay protection) + +# Purelymail + +The actual Purelymail webhook signing scheme is NOT well-documented +publicly. The bucket spec asked us to research + flag any divergence +from "shared secret HMAC-SHA256 over body". Implementation here is +the conservative default — operator MUST verify against Purelymail's +current docs at integration time + adjust header name / encoding as +needed. + +Default assumption: + - header ``X-Purelymail-Signature``: ``sha256=`` + OR a plain hex digest (we accept both for forward-compat) + - HMAC-SHA256 with the shared secret over the raw body bytes + +Flag pinned in the PR body + the R2 amendment doc — when Feature 3 +lands real handler logic, this verifier may need a one-line update +to match Purelymail's actual scheme. +""" + +from __future__ import annotations + +import hashlib +import hmac +import time +from dataclasses import dataclass + + +SLACK_TIMESTAMP_TOLERANCE_SECONDS: int = 5 * 60 # 5 minutes + + +@dataclass(frozen=True, slots=True) +class VerificationOutcome: + """Result of verifying a webhook signature. + + ``ok`` is the bottom-line accept/reject. ``reason`` is a short + machine-readable code suitable for inclusion in the dead-letter + record (no user content; no secrets). + """ + + ok: bool + reason: str + + +# --------------------------------------------------------------------------- +# Slack +# --------------------------------------------------------------------------- + + +def verify_slack_signature( + *, + signing_secret: str, + raw_body: bytes, + signature_header: str | None, + timestamp_header: str | None, + now: float | None = None, + tolerance_seconds: int = SLACK_TIMESTAMP_TOLERANCE_SECONDS, +) -> VerificationOutcome: + """Verify a Slack Events webhook signature. + + Returns ``VerificationOutcome(ok=False, reason="...")`` on any + failure path — the caller maps to the appropriate HTTP status + (401 for signature mismatch, 408 for timestamp drift). + + ``now`` is injectable for tests; defaults to ``time.time()``. + """ + if not signing_secret: + return VerificationOutcome(False, "slack_secret_unset") + if not signature_header: + return VerificationOutcome(False, "slack_signature_missing") + if not timestamp_header: + return VerificationOutcome(False, "slack_timestamp_missing") + + try: + ts = int(timestamp_header) + except (TypeError, ValueError): + return VerificationOutcome(False, "slack_timestamp_malformed") + + now_ts = now if now is not None else time.time() + if abs(now_ts - ts) > tolerance_seconds: + return VerificationOutcome(False, "slack_timestamp_too_old") + + # Slack's v0 scheme. + if not signature_header.startswith("v0="): + return VerificationOutcome(False, "slack_signature_bad_scheme") + expected_digest = signature_header[len("v0=") :] + + base_string = f"v0:{ts}:".encode("utf-8") + raw_body + computed = hmac.new( + signing_secret.encode("utf-8"), base_string, hashlib.sha256 + ).hexdigest() + + if not hmac.compare_digest(computed, expected_digest): + return VerificationOutcome(False, "slack_signature_mismatch") + return VerificationOutcome(True, "ok") + + +# --------------------------------------------------------------------------- +# Purelymail (default assumption — see module docstring) +# --------------------------------------------------------------------------- + + +def verify_purelymail_signature( + *, + signing_secret: str, + raw_body: bytes, + signature_header: str | None, +) -> VerificationOutcome: + """Verify an inbound-email webhook signature using the conservative + default scheme (HMAC-SHA256 over raw body). + + Accepts both ``sha256=`` and bare hex digest in the header + for forward compatibility — Purelymail's exact format may vary. + """ + if not signing_secret: + return VerificationOutcome(False, "purelymail_secret_unset") + if not signature_header: + return VerificationOutcome(False, "purelymail_signature_missing") + + presented = signature_header.strip() + if presented.startswith("sha256="): + presented = presented[len("sha256=") :] + + computed = hmac.new( + signing_secret.encode("utf-8"), raw_body, hashlib.sha256 + ).hexdigest() + + if not hmac.compare_digest(computed, presented): + return VerificationOutcome(False, "purelymail_signature_mismatch") + return VerificationOutcome(True, "ok") diff --git a/kora_cli/listeners/webhooks.py b/kora_cli/listeners/webhooks.py new file mode 100644 index 000000000000..d51f0db75c63 --- /dev/null +++ b/kora_cli/listeners/webhooks.py @@ -0,0 +1,324 @@ +"""Webhook listener — PUBLIC port 9118 (KR-D-DAEMON ST3). + +Per PM Q1 ruling (R2 §5 amendment, kora_docs/00_canonical_current_state/r2_amendments.md): +the webhook plane is a SEPARATE FastAPI() instance bound to a +SEPARATE uvicorn process inside the daemon. Mounts ONLY: + + POST /api/webhooks/slack/events + POST /api/webhooks/email/inbound + +… so admin-UI / MCP / control-plane routes are structurally +impossible to surface on the public port: they live on a different +app object. HMAC verification is the auth boundary. + +# Rate limiting + +slowapi (chosen over fastapi-limiter — no Redis dependency, single- +machine daemon, in-memory token bucket sufficient). 60 req/min per +remote IP across all webhook routes; well above legitimate volume, +catches floods/probes. + +# Listener lifecycle + +Reuses ST2's WebListener pattern — programmatic uvicorn via +``Server.serve()`` + ``should_exit`` for graceful shutdown. + +# Handler bodies = no-op in this ST + +ST3 ships the routing + verification + dead-lettering + rate +limiting. Real handlers land in: + - Feature 5 (Slack DM) — ``KR-FEAT-SLACK-DM`` bucket. + - Feature 3 (Email reply) — ``KR-FEAT-EMAIL`` bucket. +""" + +from __future__ import annotations + +import asyncio +import logging +import os +from typing import Optional + +from fastapi import FastAPI, Request, Response, status +from fastapi.responses import JSONResponse, PlainTextResponse +from slowapi import Limiter, _rate_limit_exceeded_handler +from slowapi.errors import RateLimitExceeded +from slowapi.middleware import SlowAPIMiddleware +from slowapi.util import get_remote_address + +from kora_cli.daemon import DEFAULT_SHUTDOWN_TIMEOUT, register_daemon_listener +from kora_cli.listeners.webhook_dead_letter import emit_webhook_dead_letter +from kora_cli.listeners.webhook_signing import ( + verify_purelymail_signature, + verify_slack_signature, +) + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Defaults + env knobs +# --------------------------------------------------------------------------- + +DEFAULT_WEBHOOK_HOST = "0.0.0.0" # public bind — see R2 amendment +DEFAULT_WEBHOOK_PORT = 9118 + +# Rate limit. Bucket spec: "60 req/min/IP default; webhooks legit +# traffic is well under that." Operator can tighten via env. +DEFAULT_RATE_LIMIT = "60/minute" + +SLACK_SECRET_ENV = "KORA_SLACK_SIGNING_SECRET" +EMAIL_SECRET_ENV = "KORA_PUREMAIL_HMAC_SECRET" +RATE_LIMIT_ENV = "KORA_WEBHOOK_RATE_LIMIT" + + +# --------------------------------------------------------------------------- +# Public FastAPI app — DELIBERATELY separate instance from admin +# --------------------------------------------------------------------------- + + +def _build_webhook_app() -> FastAPI: + """Construct the public-port FastAPI app. + + Factory pattern (not module-level) so tests can spin up a fresh + app per test without slowapi's process-wide rate-limit state + bleeding across runs. + """ + app = FastAPI( + title="Kora Webhook Ingress", + description=( + "PUBLIC port. Only HMAC-verified webhook endpoints live " + "here. Admin UI + MCP + control-plane routes are on the " + "INTERNAL port 9119." + ), + ) + + limiter = Limiter(key_func=get_remote_address, default_limits=[]) + app.state.limiter = limiter + app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) + app.add_middleware(SlowAPIMiddleware) + + rate_limit = ( + os.environ.get(RATE_LIMIT_ENV, "").strip() or DEFAULT_RATE_LIMIT + ) + + # ----------------------------------------------------------------- + # Slack — POST /api/webhooks/slack/events + # ----------------------------------------------------------------- + + @app.post("/api/webhooks/slack/events") + @limiter.limit(rate_limit) + async def slack_events(request: Request): + return await _handle_slack(request) + + # ----------------------------------------------------------------- + # Email inbound — POST /api/webhooks/email/inbound + # ----------------------------------------------------------------- + + @app.post("/api/webhooks/email/inbound") + @limiter.limit(rate_limit) + async def email_inbound(request: Request): + return await _handle_email(request) + + # ----------------------------------------------------------------- + # Healthcheck — GET /healthz (no rate limit; used by Fly TCP check) + # ----------------------------------------------------------------- + + @app.get("/healthz") + async def healthz(): + return PlainTextResponse("ok") + + return app + + +# --------------------------------------------------------------------------- +# Slack handler +# --------------------------------------------------------------------------- + + +async def _handle_slack(request: Request) -> Response: + raw_body = await request.body() + sig = request.headers.get("x-slack-signature") + ts = request.headers.get("x-slack-request-timestamp") + + secret = os.environ.get(SLACK_SECRET_ENV, "").strip() + outcome = verify_slack_signature( + signing_secret=secret, + raw_body=raw_body, + signature_header=sig, + timestamp_header=ts, + ) + if not outcome.ok: + emit_webhook_dead_letter( + source="slack", + reason=outcome.reason, + headers=dict(request.headers), + peer_ip=_peer_ip(request), + request_id=request.headers.get("x-request-id"), + body_bytes=len(raw_body), + ) + # 408 specifically for timestamp-too-old per bucket spec; 401 + # for everything else. + if outcome.reason == "slack_timestamp_too_old": + return JSONResponse( + {"error": outcome.reason}, + status_code=status.HTTP_408_REQUEST_TIMEOUT, + ) + return JSONResponse( + {"error": outcome.reason}, + status_code=status.HTTP_401_UNAUTHORIZED, + ) + + # URL-verification handshake — Slack's initial app-install ping. + # Echo the challenge inline; this happens once per workspace. + try: + import json + + payload = json.loads(raw_body) + except Exception: + # Verified-but-not-JSON shouldn't happen for Slack events; + # log as a curiosity + 200 so Slack doesn't retry forever. + logger.info("[kora.webhook.slack] verified non-JSON body; ignoring") + return JSONResponse({"ok": True}) + + if isinstance(payload, dict) and payload.get("type") == "url_verification": + challenge = payload.get("challenge", "") + return PlainTextResponse(challenge) + + # All other event types — Feature 5 will land the real handler. + # ST3 scaffolding logs + acknowledges. + logger.info( + "[kora.webhook.slack] event accepted: type=%s", + payload.get("type") if isinstance(payload, dict) else "(unknown)", + ) + return JSONResponse({"ok": True}) + + +# --------------------------------------------------------------------------- +# Email handler +# --------------------------------------------------------------------------- + + +async def _handle_email(request: Request) -> Response: + raw_body = await request.body() + sig = request.headers.get("x-purelymail-signature") + + secret = os.environ.get(EMAIL_SECRET_ENV, "").strip() + outcome = verify_purelymail_signature( + signing_secret=secret, + raw_body=raw_body, + signature_header=sig, + ) + if not outcome.ok: + emit_webhook_dead_letter( + source="email", + reason=outcome.reason, + headers=dict(request.headers), + peer_ip=_peer_ip(request), + request_id=request.headers.get("x-request-id"), + body_bytes=len(raw_body), + ) + return JSONResponse( + {"error": outcome.reason}, + status_code=status.HTTP_401_UNAUTHORIZED, + ) + + # Real handler in Feature 3 — ST3 scaffolding just acknowledges. + logger.info( + "[kora.webhook.email] inbound accepted: bytes=%d", len(raw_body) + ) + return JSONResponse({"ok": True}) + + +# --------------------------------------------------------------------------- +# Utilities +# --------------------------------------------------------------------------- + + +def _peer_ip(request: Request) -> Optional[str]: + """Best-effort client IP. Trusts X-Forwarded-For ONLY if the + immediate peer is Fly's edge (we don't know that here, so + return ``client.host`` and let operator-side log analysis + handle X-Forwarded-For with caller knowledge).""" + client = request.client + return client.host if client else None + + +# --------------------------------------------------------------------------- +# Listener (uvicorn lifecycle) +# --------------------------------------------------------------------------- + + +class WebhookListener: + """Owns the second uvicorn server task for the public webhook app.""" + + def __init__(self, host: str, port: int) -> None: + self._host = host + self._port = port + self._server = None # uvicorn.Server | None + self._serve_task: Optional[asyncio.Task] = None + self._app: Optional[FastAPI] = None + + async def startup(self) -> None: + import uvicorn + + # Per-startup app instance — fresh slowapi state. + self._app = _build_webhook_app() + + config = uvicorn.Config( + self._app, + host=self._host, + port=self._port, + log_level="warning", + # PUBLIC port — DO honor X-Forwarded-For headers Fly's + # edge inserts. (The admin-port listener disables this + # for loopback-only checks; this public listener wants + # the real peer IP for rate limiting + dead-letter logs.) + proxy_headers=True, + ) + self._server = uvicorn.Server(config) + self._serve_task = asyncio.create_task( + self._server.serve(), name="webhook-listener:serve" + ) + for _ in range(50): # up to 1s + if getattr(self._server, "started", False): + break + await asyncio.sleep(0.02) + if not getattr(self._server, "started", False): + raise RuntimeError( + f"webhook listener: uvicorn did not enter started state " + f"on {self._host}:{self._port} within 1s" + ) + logger.info( + "[kora.webhook] uvicorn bound on %s:%d (PUBLIC)", + self._host, + self._port, + ) + + async def shutdown(self) -> None: + if self._server is None or self._serve_task is None: + return + self._server.should_exit = True + try: + await self._serve_task + except asyncio.CancelledError: + pass + logger.info("[kora.webhook] uvicorn stopped") + + +def _factory(): + host = ( + os.environ.get("KORA_WEBHOOK_HOST", DEFAULT_WEBHOOK_HOST).strip() + or DEFAULT_WEBHOOK_HOST + ) + port_raw = os.environ.get("KORA_WEBHOOK_PORT", "").strip() + try: + port = int(port_raw) if port_raw else DEFAULT_WEBHOOK_PORT + except ValueError as exc: + raise SystemExit( + f"KORA_WEBHOOK_PORT must be an int; got {port_raw!r}: {exc}" + ) + listener = WebhookListener(host=host, port=port) + return (listener.startup, listener.shutdown, DEFAULT_SHUTDOWN_TIMEOUT) + + +register_daemon_listener("webhooks", _factory) diff --git a/kora_docs/00_canonical_current_state/r2_amendments.md b/kora_docs/00_canonical_current_state/r2_amendments.md new file mode 100644 index 000000000000..60a0ec6a4143 --- /dev/null +++ b/kora_docs/00_canonical_current_state/r2_amendments.md @@ -0,0 +1,77 @@ +# R2 amendments — running record + +This document records authoritative amendments to the R2 spec +(`kora_docs/21_program_council/Kora_v1_Program_Spec_R2*.md`) made +after R2 sealed. Each amendment carries a date, the bucket / PR that +introduced it, the threat-model rationale, and the exact spec text +being narrowed or extended. + +The R2 spec's adversarial round closed before Phase 2's webhook +ingress was scoped; some §5 language was authored against a +control-plane-only deployment model and needs scoping clarifications +as Kora's surface grows. This file is where those clarifications +live so future readers don't see "R2 says no public ports" and +treat that as contradicting a public webhook port that ships under +this amendment. + +--- + +## Amendment 1 — Public webhook port (KR-D-DAEMON ST3) + +**Date**: 2026-05-22 +**Bucket**: `kora_docs/17_cc_bucket_prompts/KR-D-DAEMON_always_alive_runtime.md` (commit 54032c6) — §5 Q1 decision; PR `feat(kora): KR-D-DAEMON ST3 — Slack + email webhook routers + R2 §5 amendment`. +**PM ruling**: Q1 "(D) with two-uvicorn refinement". + +### Original R2 §5 text being amended + +> Kora has no inbound HTTP service — the control plane is substrate-mediated (pull-based via `kora_control`), and the admin web UI is operator-only via `flyctl proxy`. No public ports on the `kora-runtime` Fly app. + +(Paraphrased from `docs/deploy-fly-io.md:123` and `fly.toml:21-24`; the R2 spec text is the design ancestor of both.) + +### Amendment text + +> **Scope of "no public ports"**: applies to **control-plane traffic** (admin UI, MCP, `kora_control` commands, ledger writes, chain emits). The control plane stays internal-only — reached via `flyctl proxy` on port 9119. +> +> **Webhook ingress is a separate traffic class** with HMAC as the auth boundary (Slack signing secret per Slack's v0 scheme, Purelymail HMAC-SHA256 over body) and per-IP rate limiting (slowapi default 60 req/min/IP) as the flood backstop. The webhook plane MAY be exposed publicly on a **dedicated port** (currently 9118), served by a **separate FastAPI() instance + separate uvicorn process** inside the daemon, mounting ONLY: +> +> - `POST /api/webhooks/slack/events` +> - `POST /api/webhooks/email/inbound` +> +> Plus `GET /healthz` (no auth) for the Fly HTTP healthcheck. +> +> **The two FastAPI apps are NEVER merged.** Admin routes are structurally impossible to surface on the public port: they live on a different app object, mounted by a different listener, served by a different uvicorn process. Shared daemon-coordinator dependencies (chain emitter, future ledger writer) are injected by DI, not by shared app state. + +### Threat model rationale + +The R2 §5 "no public ports" rule was authored against a single-class threat model: **arbitrary inbound traffic against the control plane**. The control plane has no application-layer auth (it presumes Fly's internal flycast routing as the trust boundary); exposing it publicly would defeat that. + +Webhook ingress is a **different threat model**: + +1. **Application-layer auth is mandatory** for webhook traffic and is enforced at the request boundary (HMAC verification on every request; bypass impossible without the signing secret). +2. **The signing secret is the security control**, not network-layer isolation. Compromise of the signing secret is the threat to defend against — and the rotation procedure for that secret lives in `kora_docs/15_status_and_roadmap/token_rotation_runbook.md` (extends naturally to Slack + Purelymail secrets). +3. **The data flow is one-way (ingress only)** — the webhook handlers don't return user data; the response is at most `{"ok": true}` or a Slack URL-verification challenge echo. The route surface gives an attacker no leverage to extract state. +4. **Blast-radius isolation is structural**: the public uvicorn knows only how to dispatch to the two webhook handlers. Admin routes are not in its route table at all. + +Per-IP rate limiting (60 req/min default; tunable via `KORA_WEBHOOK_RATE_LIMIT`) caps flood blast radius without restricting legitimate webhook volume (Slack + Purelymail event rates are normally <1 req/min/source). + +### Operator obligations + +Operators MUST: + +- Set `KORA_SLACK_SIGNING_SECRET` in Doppler project `kora-runtime-gateways` BEFORE deploy (else Slack webhooks reject 401, dead-letter logged). +- Set `KORA_PUREMAIL_HMAC_SECRET` in Doppler project `kora-runtime-gateways` BEFORE deploy. +- Monitor `[kora.webhook.dead_letter]` log lines via Fly logs or OPS panel — sustained dead-letter rate indicates either a misconfigured Slack/Purelymail app pointing at a wrong endpoint, or active probing. +- Verify Purelymail's actual webhook signing scheme at integration time — `kora_cli/listeners/webhook_signing.py:verify_purelymail_signature` ships the conservative default (HMAC-SHA256 over body, header `X-Purelymail-Signature` with optional `sha256=` prefix); flag any divergence in a follow-on PR. + +### Cross-references + +- `fly.toml` — second `[[services]]` block exposing port 9118 with public ports 443 (TLS) + 80 (force_https) + http_check on `/healthz`. +- `kora_cli/listeners/webhooks.py` — the public-port FastAPI factory + WebhookListener uvicorn lifecycle. +- `kora_cli/listeners/webhook_signing.py` — HMAC verifiers (pure functions; vector-tested). +- `kora_cli/listeners/webhook_dead_letter.py` — structured-log dead-letter recorder. +- `kora_docs/15_status_and_roadmap/token_rotation_runbook.md` — extends to cover Slack + Purelymail secret rotation. + +### Future work flagged + +- **Persistent dead-letter records** — the bucket spec called for `kora_operation_ledger` rows on verification failure. The ledger's schema (substrate migration 0093) requires `work_attempt_id`/`workspace_id`/`ticket_id` — all tied to Sea_Ticket dispatch. A webhook dead-letter has none of those. Today's implementation uses structured logging (`logger.warning("[kora.webhook.dead_letter] ...")`). When substrate-team adds either (a) a `webhook_dead_letters` table, (b) a `kora.webhook.dead_letter` chain-event vocab literal, or (c) a permissive ledger shape, the runtime extension is a small change in `webhook_dead_letter.py` (the log-line emit is the stable seam). +- **Purelymail scheme verification** — see operator-obligations bullet above. diff --git a/pyproject.toml b/pyproject.toml index 967a639e78b6..d65cc6b1b7e4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -182,7 +182,7 @@ youtube = [ "youtube-transcript-api==1.2.4", ] # `hermes dashboard` (localhost SPA + API). Not in core to keep the default install lean. -web = ["fastapi==0.133.1", "uvicorn[standard]==0.41.0"] +web = ["fastapi==0.133.1", "uvicorn[standard]==0.41.0", "slowapi>=0.1.9"] all = [ # Policy (2026-05-12): `[all]` includes only extras that genuinely # CAN'T be lazy-installed via `tools/lazy_deps.py` — i.e. things every diff --git a/tests/kora_cli/test_listeners/test_webhook_dead_letter.py b/tests/kora_cli/test_listeners/test_webhook_dead_letter.py new file mode 100644 index 000000000000..9657ac1a77ad --- /dev/null +++ b/tests/kora_cli/test_listeners/test_webhook_dead_letter.py @@ -0,0 +1,109 @@ +"""Tests for ``kora_cli.listeners.webhook_dead_letter`` — KR-D-DAEMON ST3.""" + +from __future__ import annotations + +import logging + +import pytest + +from kora_cli.listeners import webhook_dead_letter +from kora_cli.listeners.webhook_dead_letter import ( + _summarize_headers, + emit_webhook_dead_letter, +) + + +def test_summarize_headers_filters_to_allowlist(): + headers = { + "Content-Type": "application/json", + "Authorization": "Bearer SECRET", # NOT logged + "Cookie": "session=...", # NOT logged + "User-Agent": "Slackbot 1.0", + "x-slack-request-timestamp": "1700000000", + } + out = _summarize_headers(headers) + assert "content-type" in out + assert "user-agent" in out + assert "x-slack-request-timestamp" in out + assert "authorization" not in out + assert "cookie" not in out + + +def test_summarize_headers_truncates_signature_values(): + """Signature values surface presence + a 12-char prefix; never + the full HMAC.""" + sig = "v0=" + "f" * 64 + headers = {"X-Slack-Signature": sig, "X-Purelymail-Signature": "sha256=" + "a" * 64} + out = _summarize_headers(headers) + assert out["x-slack-signature"].endswith("...(truncated)") + assert len(out["x-slack-signature"]) < len(sig) + assert out["x-purelymail-signature"].endswith("...(truncated)") + + +def test_summarize_headers_lowercases_names(): + """Operator-side analysis works with lowercase keys, regardless of + the casing the upstream framework used.""" + out = _summarize_headers({"CONTENT-TYPE": "application/json"}) + assert "content-type" in out + + +def test_emit_writes_warning_log_with_marker(caplog): + caplog.set_level(logging.WARNING, logger=webhook_dead_letter.logger.name) + emit_webhook_dead_letter( + source="slack", + reason="slack_signature_mismatch", + headers={"Content-Type": "application/json"}, + peer_ip="203.0.113.7", + request_id="req-123", + body_bytes=42, + now=1_700_000_000.0, + ) + matching = [r for r in caplog.records if "kora.webhook.dead_letter" in r.getMessage()] + assert len(matching) == 1 + msg = matching[0].getMessage() + assert "source=slack" in msg + assert "reason=slack_signature_mismatch" in msg + assert "peer_ip=203.0.113.7" in msg + assert "request_id=req-123" in msg + assert "body_bytes=42" in msg + + +def test_emit_handles_missing_optional_fields(caplog): + """peer_ip / request_id / body_bytes are optional.""" + caplog.set_level(logging.WARNING, logger=webhook_dead_letter.logger.name) + emit_webhook_dead_letter( + source="email", + reason="purelymail_signature_missing", + headers={}, + ) + msg = next( + r.getMessage() + for r in caplog.records + if "kora.webhook.dead_letter" in r.getMessage() + ) + assert "peer_ip=-" in msg + assert "request_id=-" in msg + assert "body_bytes=-" in msg + + +def test_emit_never_contains_body_content(caplog): + """Even though the helper has no body arg, double-check no + accidental body data sneaks in via headers (the allow-list + excludes anything that could carry user content).""" + caplog.set_level(logging.WARNING, logger=webhook_dead_letter.logger.name) + emit_webhook_dead_letter( + source="slack", + reason="slack_signature_mismatch", + headers={ + "Content-Type": "application/json", + # Hypothetical leaky header. + "X-Original-Body-Echo": "from=alice@example.com&secret=hunter2", + }, + ) + msg = next( + r.getMessage() + for r in caplog.records + if "kora.webhook.dead_letter" in r.getMessage() + ) + assert "hunter2" not in msg + assert "alice@example.com" not in msg diff --git a/tests/kora_cli/test_listeners/test_webhook_signing.py b/tests/kora_cli/test_listeners/test_webhook_signing.py new file mode 100644 index 000000000000..73161a174cd3 --- /dev/null +++ b/tests/kora_cli/test_listeners/test_webhook_signing.py @@ -0,0 +1,250 @@ +"""Tests for ``kora_cli.listeners.webhook_signing`` — KR-D-DAEMON ST3. + +Vector tests for the Slack v0 + Purelymail HMAC-SHA256 verifiers. +""" + +from __future__ import annotations + +import hashlib +import hmac + +import pytest + +from kora_cli.listeners.webhook_signing import ( + SLACK_TIMESTAMP_TOLERANCE_SECONDS, + verify_purelymail_signature, + verify_slack_signature, +) + + +# --------------------------------------------------------------------------- +# Helpers — generate valid signatures for inputs +# --------------------------------------------------------------------------- + + +def _slack_sig(secret: str, ts: int, body: bytes) -> str: + base = f"v0:{ts}:".encode() + body + digest = hmac.new(secret.encode(), base, hashlib.sha256).hexdigest() + return f"v0={digest}" + + +def _purelymail_sig(secret: str, body: bytes, prefixed: bool = True) -> str: + digest = hmac.new(secret.encode(), body, hashlib.sha256).hexdigest() + return f"sha256={digest}" if prefixed else digest + + +# --------------------------------------------------------------------------- +# Slack — accept paths +# --------------------------------------------------------------------------- + + +def test_slack_valid_signature_accepts(): + secret = "test-secret" + ts = 1_700_000_000 + body = b'{"type":"event_callback"}' + sig = _slack_sig(secret, ts, body) + outcome = verify_slack_signature( + signing_secret=secret, + raw_body=body, + signature_header=sig, + timestamp_header=str(ts), + now=ts, + ) + assert outcome.ok is True + assert outcome.reason == "ok" + + +def test_slack_url_verification_handshake_signature(): + """URL-verification challenge body is a tiny JSON; signature + verification works just like any other event.""" + secret = "s" + ts = 1_700_000_100 + body = b'{"type":"url_verification","challenge":"abc"}' + sig = _slack_sig(secret, ts, body) + outcome = verify_slack_signature( + signing_secret=secret, + raw_body=body, + signature_header=sig, + timestamp_header=str(ts), + now=ts, + ) + assert outcome.ok is True + + +# --------------------------------------------------------------------------- +# Slack — reject paths +# --------------------------------------------------------------------------- + + +def test_slack_secret_unset_rejects(): + outcome = verify_slack_signature( + signing_secret="", + raw_body=b"{}", + signature_header="v0=deadbeef", + timestamp_header="1700000000", + now=1_700_000_000, + ) + assert outcome.ok is False + assert outcome.reason == "slack_secret_unset" + + +def test_slack_missing_signature_header_rejects(): + outcome = verify_slack_signature( + signing_secret="s", + raw_body=b"{}", + signature_header=None, + timestamp_header="1700000000", + now=1_700_000_000, + ) + assert outcome.reason == "slack_signature_missing" + + +def test_slack_missing_timestamp_rejects(): + outcome = verify_slack_signature( + signing_secret="s", + raw_body=b"{}", + signature_header="v0=deadbeef", + timestamp_header=None, + now=1_700_000_000, + ) + assert outcome.reason == "slack_timestamp_missing" + + +def test_slack_malformed_timestamp_rejects(): + outcome = verify_slack_signature( + signing_secret="s", + raw_body=b"{}", + signature_header="v0=deadbeef", + timestamp_header="not-a-number", + now=1_700_000_000, + ) + assert outcome.reason == "slack_timestamp_malformed" + + +def test_slack_timestamp_too_old_rejects(): + """Replay-protection: timestamp older than 5min.""" + ts = 1_700_000_000 + secret = "s" + body = b"{}" + sig = _slack_sig(secret, ts, body) + now = ts + SLACK_TIMESTAMP_TOLERANCE_SECONDS + 1 + outcome = verify_slack_signature( + signing_secret=secret, + raw_body=body, + signature_header=sig, + timestamp_header=str(ts), + now=now, + ) + assert outcome.reason == "slack_timestamp_too_old" + + +def test_slack_timestamp_too_new_also_rejects(): + """Skew protection covers BOTH directions of drift.""" + ts = 1_700_000_000 + secret = "s" + body = b"{}" + sig = _slack_sig(secret, ts, body) + now = ts - SLACK_TIMESTAMP_TOLERANCE_SECONDS - 1 + outcome = verify_slack_signature( + signing_secret=secret, + raw_body=body, + signature_header=sig, + timestamp_header=str(ts), + now=now, + ) + assert outcome.reason == "slack_timestamp_too_old" + + +def test_slack_signature_bad_scheme_rejects(): + """Anything not starting with 'v0='.""" + outcome = verify_slack_signature( + signing_secret="s", + raw_body=b"{}", + signature_header="v1=deadbeef", + timestamp_header="1700000000", + now=1_700_000_000, + ) + assert outcome.reason == "slack_signature_bad_scheme" + + +def test_slack_signature_mismatch_rejects(): + """Right scheme, wrong HMAC.""" + outcome = verify_slack_signature( + signing_secret="s", + raw_body=b"{}", + signature_header="v0=0000000000000000000000000000000000000000000000000000000000000000", + timestamp_header="1700000000", + now=1_700_000_000, + ) + assert outcome.reason == "slack_signature_mismatch" + + +def test_slack_signature_mismatch_with_wrong_secret(): + """Same body + ts, but secret differs → mismatch.""" + ts = 1_700_000_000 + body = b'{"x":1}' + sig = _slack_sig("right-secret", ts, body) + outcome = verify_slack_signature( + signing_secret="wrong-secret", + raw_body=body, + signature_header=sig, + timestamp_header=str(ts), + now=ts, + ) + assert outcome.reason == "slack_signature_mismatch" + + +# --------------------------------------------------------------------------- +# Purelymail +# --------------------------------------------------------------------------- + + +def test_purelymail_valid_signature_with_prefix_accepts(): + secret = "email-secret" + body = b'{"from":"alice@x"}' + outcome = verify_purelymail_signature( + signing_secret=secret, + raw_body=body, + signature_header=_purelymail_sig(secret, body, prefixed=True), + ) + assert outcome.ok is True + + +def test_purelymail_valid_signature_without_prefix_accepts(): + """Forward-compat: bare hex accepted in case Purelymail's + actual format omits the prefix.""" + secret = "s" + body = b"{}" + outcome = verify_purelymail_signature( + signing_secret=secret, + raw_body=body, + signature_header=_purelymail_sig(secret, body, prefixed=False), + ) + assert outcome.ok is True + + +def test_purelymail_secret_unset_rejects(): + outcome = verify_purelymail_signature( + signing_secret="", + raw_body=b"{}", + signature_header="sha256=deadbeef", + ) + assert outcome.reason == "purelymail_secret_unset" + + +def test_purelymail_signature_missing_rejects(): + outcome = verify_purelymail_signature( + signing_secret="s", + raw_body=b"{}", + signature_header=None, + ) + assert outcome.reason == "purelymail_signature_missing" + + +def test_purelymail_signature_mismatch_rejects(): + outcome = verify_purelymail_signature( + signing_secret="right", + raw_body=b"{}", + signature_header=_purelymail_sig("wrong", b"{}"), + ) + assert outcome.reason == "purelymail_signature_mismatch" diff --git a/tests/kora_cli/test_listeners/test_webhooks.py b/tests/kora_cli/test_listeners/test_webhooks.py new file mode 100644 index 000000000000..e311221c7765 --- /dev/null +++ b/tests/kora_cli/test_listeners/test_webhooks.py @@ -0,0 +1,334 @@ +"""Tests for ``kora_cli.listeners.webhooks`` — KR-D-DAEMON ST3. + +Covers the route surface (Slack + email) against a fresh public app +instance per test. The full uvicorn lifecycle is exercised by a +single end-to-end test; the rest use FastAPI's TestClient against +the app object directly to keep the suite fast. +""" + +from __future__ import annotations + +import hashlib +import hmac +import json +import logging +import socket +from typing import Tuple + +import pytest +from fastapi.testclient import TestClient + +from kora_cli.listeners import webhooks as wh_mod +from kora_cli.listeners.webhooks import ( + DEFAULT_RATE_LIMIT, + EMAIL_SECRET_ENV, + SLACK_SECRET_ENV, + WebhookListener, + _build_webhook_app, + _factory, +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _slack_signed( + secret: str, body: bytes, ts: int +) -> Tuple[str, str]: + """Return (signature_header, timestamp_header).""" + base = f"v0:{ts}:".encode() + body + digest = hmac.new(secret.encode(), base, hashlib.sha256).hexdigest() + return (f"v0={digest}", str(ts)) + + +def _purelymail_signed(secret: str, body: bytes) -> str: + return "sha256=" + hmac.new(secret.encode(), body, hashlib.sha256).hexdigest() + + +def _find_free_port() -> int: + with socket.socket() as s: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + + +# Use the real wall clock for these — Slack verifier defaults to +# ``time.time()`` and the helpers below compute timestamps within the +# 5-min tolerance window relative to now. +import time as _time + + +def _now_int() -> int: + return int(_time.time()) + + +@pytest.fixture +def slack_secret(monkeypatch): + monkeypatch.setenv(SLACK_SECRET_ENV, "test-slack-secret-1234") + return "test-slack-secret-1234" + + +@pytest.fixture +def email_secret(monkeypatch): + monkeypatch.setenv(EMAIL_SECRET_ENV, "test-email-secret-5678") + return "test-email-secret-5678" + + +@pytest.fixture +def client(): + return TestClient(_build_webhook_app()) + + +# --------------------------------------------------------------------------- +# Slack — URL verification handshake + accept + reject paths +# --------------------------------------------------------------------------- + + +def test_slack_url_verification_round_trips(slack_secret, client): + """Slack's initial app-install ping: receive challenge, echo it.""" + ts = _now_int() + body = json.dumps( + {"type": "url_verification", "challenge": "kor4-ch4ll3ng3"} + ).encode() + sig, ts_h = _slack_signed(slack_secret, body, ts) + r = client.post( + "/api/webhooks/slack/events", + content=body, + headers={ + "Content-Type": "application/json", + "X-Slack-Signature": sig, + "X-Slack-Request-Timestamp": ts_h, + }, + ) + assert r.status_code == 200 + assert r.text == "kor4-ch4ll3ng3" + + +def test_slack_valid_event_accepted(slack_secret, client): + ts = _now_int() + body = json.dumps({"type": "event_callback", "event": {}}).encode() + sig, ts_h = _slack_signed(slack_secret, body, ts) + r = client.post( + "/api/webhooks/slack/events", + content=body, + headers={ + "Content-Type": "application/json", + "X-Slack-Signature": sig, + "X-Slack-Request-Timestamp": ts_h, + }, + ) + assert r.status_code == 200 + assert r.json() == {"ok": True} + + +def test_slack_signature_mismatch_returns_401(slack_secret, client, caplog): + caplog.set_level(logging.WARNING) + ts = _now_int() + body = b'{"type":"event_callback"}' + bad_sig = "v0=" + "0" * 64 + r = client.post( + "/api/webhooks/slack/events", + content=body, + headers={ + "Content-Type": "application/json", + "X-Slack-Signature": bad_sig, + "X-Slack-Request-Timestamp": str(ts), + }, + ) + assert r.status_code == 401 + # Dead-letter logged. + dead_letter_lines = [ + r for r in caplog.records if "kora.webhook.dead_letter" in r.getMessage() + ] + assert len(dead_letter_lines) == 1 + assert "source=slack" in dead_letter_lines[0].getMessage() + assert "reason=slack_signature_mismatch" in dead_letter_lines[0].getMessage() + + +def test_slack_timestamp_too_old_returns_408(slack_secret, client): + """Bucket spec: 408 specifically for timestamp drift > 5min.""" + ancient_ts = _now_int() - (60 * 60) # 1 hour ago + body = b'{"type":"event_callback"}' + sig, ts_h = _slack_signed(slack_secret, body, ancient_ts) + r = client.post( + "/api/webhooks/slack/events", + content=body, + headers={ + "Content-Type": "application/json", + "X-Slack-Signature": sig, + "X-Slack-Request-Timestamp": ts_h, + }, + ) + assert r.status_code == 408 + + +def test_slack_missing_signature_header_returns_401(slack_secret, client): + r = client.post( + "/api/webhooks/slack/events", + content=b"{}", + headers={"X-Slack-Request-Timestamp": str(_now_int())}, + ) + assert r.status_code == 401 + + +# --------------------------------------------------------------------------- +# Email — accept + reject +# --------------------------------------------------------------------------- + + +def test_email_valid_signature_accepted(email_secret, client): + body = b'{"from": "alice@example.com", "to": "kora@kora.example"}' + r = client.post( + "/api/webhooks/email/inbound", + content=body, + headers={ + "Content-Type": "application/json", + "X-Purelymail-Signature": _purelymail_signed(email_secret, body), + }, + ) + assert r.status_code == 200 + assert r.json() == {"ok": True} + + +def test_email_signature_mismatch_returns_401(email_secret, client, caplog): + caplog.set_level(logging.WARNING) + body = b"{}" + r = client.post( + "/api/webhooks/email/inbound", + content=body, + headers={ + "Content-Type": "application/json", + "X-Purelymail-Signature": "sha256=" + "0" * 64, + }, + ) + assert r.status_code == 401 + dead_letter_lines = [ + r for r in caplog.records if "kora.webhook.dead_letter" in r.getMessage() + ] + assert any("source=email" in r.getMessage() for r in dead_letter_lines) + + +def test_email_secret_unset_returns_401(monkeypatch, client): + """No KORA_PUREMAIL_HMAC_SECRET → all email webhooks rejected.""" + monkeypatch.delenv(EMAIL_SECRET_ENV, raising=False) + body = b"{}" + r = client.post( + "/api/webhooks/email/inbound", + content=body, + headers={"X-Purelymail-Signature": "sha256=anything"}, + ) + assert r.status_code == 401 + + +# --------------------------------------------------------------------------- +# Health endpoint +# --------------------------------------------------------------------------- + + +def test_healthz_no_auth_required(client): + """Fly TCP/HTTP check needs this to succeed without secrets.""" + r = client.get("/healthz") + assert r.status_code == 200 + assert r.text == "ok" + + +# --------------------------------------------------------------------------- +# Admin routes NOT mounted on public app +# --------------------------------------------------------------------------- + + +def test_admin_routes_not_on_public_app(client): + """Structural guarantee: the public app is a distinct FastAPI + instance, so it has NONE of the admin/MCP routes.""" + for path in ( + "/api/status", + "/mcp/tools/list", + "/api/cron", + "/api/sessions", + ): + r = client.get(path) + assert r.status_code == 404, f"{path} unexpectedly mounted: {r.status_code}" + + +# --------------------------------------------------------------------------- +# Rate limiting +# --------------------------------------------------------------------------- + + +def test_rate_limit_enforced(monkeypatch, slack_secret): + """At very low limits, the (N+1)th request returns 429. + + slowapi default storage is in-memory keyed by remote IP. We set + an aggressively low rate limit via env to make this assertion + fast and deterministic. + """ + monkeypatch.setenv("KORA_WEBHOOK_RATE_LIMIT", "2/minute") + app = _build_webhook_app() + c = TestClient(app) + + body = b"{}" + # Sign each request so signature isn't the cause of any 401 — + # we want to confirm 429s come from the rate limiter, not from + # other failure paths. + bad_sig = "v0=" + "0" * 64 + ts = str(_now_int()) + headers = { + "Content-Type": "application/json", + "X-Slack-Signature": bad_sig, + "X-Slack-Request-Timestamp": ts, + } + + # The first 2 hit 401 (bad sig). The third should hit 429 from + # the limiter BEFORE the route runs. + r1 = c.post("/api/webhooks/slack/events", content=body, headers=headers) + r2 = c.post("/api/webhooks/slack/events", content=body, headers=headers) + r3 = c.post("/api/webhooks/slack/events", content=body, headers=headers) + + assert r1.status_code == 401 + assert r2.status_code == 401 + assert r3.status_code == 429 + + +# --------------------------------------------------------------------------- +# Factory + listener lifecycle +# --------------------------------------------------------------------------- + + +def test_factory_default_shape(monkeypatch): + monkeypatch.delenv("KORA_WEBHOOK_HOST", raising=False) + monkeypatch.delenv("KORA_WEBHOOK_PORT", raising=False) + spec = _factory() + assert isinstance(spec, tuple) and len(spec) == 3 + startup, shutdown, timeout = spec + assert callable(startup) and callable(shutdown) + assert timeout > 0 + + +def test_factory_rejects_non_int_port(monkeypatch): + monkeypatch.setenv("KORA_WEBHOOK_PORT", "not-a-port") + with pytest.raises(SystemExit, match="KORA_WEBHOOK_PORT"): + _factory() + + +@pytest.mark.asyncio +async def test_webhook_listener_lifecycle_end_to_end(slack_secret): + """Full uvicorn lifecycle on a free port — proves the second + uvicorn instance binds cleanly + serves a webhook + tears down.""" + import asyncio + + import httpx + + port = _find_free_port() + listener = WebhookListener(host="127.0.0.1", port=port) + await listener.startup() + try: + async with httpx.AsyncClient(timeout=5.0) as client: + r = await client.get(f"http://127.0.0.1:{port}/healthz") + assert r.status_code == 200 + finally: + await asyncio.wait_for(listener.shutdown(), timeout=5.0) + + +def test_default_rate_limit_constant(): + assert DEFAULT_RATE_LIMIT == "60/minute"