diff --git a/README.md b/README.md index bd33c9b..ecdeb81 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Agent runs are not ordinary webhook handlers. They take seconds to minutes, cost | | Built-in webhooks | With this plugin | |---|---|---| -| Signature verification | Limited providers | ~140 provider schemes verified by Hookdeck | +| Signature verification | Limited providers | ~140 provider schemes verified by Hookdeck, once the provider's secret is set on the source | | Gateway offline | Events lost | Paused events held server-side, drained on resume | | Traffic bursts | 30/min fixed window, excess dropped | Queued; overflow answered with 503 + `Retry-After` | | Duplicates | In-memory 1h cache | Hookdeck dedup + restart-safe SQLite ledger | @@ -63,7 +63,7 @@ A [free Hookdeck account](https://dashboard.hookdeck.com/signup) is enough for d Events flow: **provider -> Hookdeck -> (CLI or HTTP push) -> plugin listener -> Hermes agent run**, with three reliability layers on top: -1. **Signature verification.** Every delivery carries an `x-hookdeck-signature` header, verified with HMAC-SHA256 in constant time. Provider-side verification (Stripe, Shopify, GitHub, and ~140 others) happens at Hookdeck's edge before the event ever reaches you. +1. **Signature verification.** Every delivery carries an `x-hookdeck-signature` header, verified with HMAC-SHA256 in constant time. Provider-side verification (Stripe, Shopify, GitHub, and ~140 others) happens at Hookdeck's edge before the event ever reaches you — but only once you paste that provider's signing secret onto the source in the Hookdeck dashboard. `hermes hookdeck setup` creates the source with the right type and cannot set the secret; until it is set, a typed source accepts unsigned and forged payloads. `hermes hookdeck doctor` reports what the source actually verified. 2. **Run ledger.** A local SQLite database records each delivery attempt and its agent-run outcome. If the process crashes mid-run, boot-time recovery finds the orphaned events and re-runs them. 3. **Backpressure.** `max_concurrent` caps simultaneous agent runs. Requests over the cap get a 503 with `Retry-After`, and Hookdeck redelivers on schedule instead of piling runs onto your box. diff --git a/hookdeck/api.py b/hookdeck/api.py index c68cec4..66c75a5 100644 --- a/hookdeck/api.py +++ b/hookdeck/api.py @@ -171,6 +171,16 @@ async def upsert_connection(self, payload: Mapping[str, Any]) -> Any: async def list_connections(self, **params: Any) -> Any: return await self.request("GET", "/connections", params=params) + async def list_requests(self, **params: Any) -> Any: + """GET /requests — inbound requests, before they fan out into events. + + The only place the API says whether a source actually verified what it + received: each request carries ``verified``. A source's own record does + not expose whether a provider secret is configured, so observed traffic + is the only signal there is. + """ + return await self.request("GET", "/requests", params=params) + async def list_sources(self, **params: Any) -> Any: return await self.request("GET", "/sources", params=params) diff --git a/hookdeck/cli.py b/hookdeck/cli.py index f0d670c..4df6e5d 100644 --- a/hookdeck/cli.py +++ b/hookdeck/cli.py @@ -506,6 +506,91 @@ async def _go() -> str: return "" +async def _check_source_verification(api: HookdeckAPI, routes: dict) -> list[Check]: + """Whether a provider-typed source is actually verifying anything. + + Setting a source's type to STRIPE or GITHUB does *not* switch verification + on. The provider's own signing secret has to be set on the source, and + until it is the source accepts anything — measured: an unsigned request and + one carrying `sha256=deadbeef` were both accepted by a GITHUB source with + no secret, and both produced events. + + The source's own record does not say whether a secret is configured; a + source with one set is byte-identical to one without over the API. So the + only signal is observed traffic, where each request carries `verified`. + That means this can confirm a problem but never confirm its absence, and it + says which of the two it is doing rather than implying the stronger one. + """ + typed = { + name: route + for name, route in routes.items() + if str(route.get("source_type") or "WEBHOOK").upper() != "WEBHOOK" + } + if not typed: + return [] + + checks: list[Check] = [] + for route_name, route in typed.items(): + source_type = str(route["source_type"]).upper() + source_name = route.get("source") or route_name + found = _models(await api.list_sources(name=source_name)) + if not found: + continue + source = found[0] + requests = _models( + await api.list_requests(source_id=source.get("id"), limit=10) + ) + if not requests: + checks.append( + Check( + True, + f"source '{source.get('name')}' is type {source_type} — no " + "traffic yet, so verification is unconfirmed", + note=( + f"A {source_type} source verifies nothing until the " + "provider's signing secret is set on it in the Hookdeck " + "dashboard, and the API does not report whether it is. " + "Until then the source accepts forged payloads." + ), + ) + ) + continue + + unverified = [r for r in requests if not r.get("verified")] + if unverified: + checks.append( + Check( + False, + f"source '{source.get('name')}' is type {source_type} but " + f"{len(unverified)} of its last {len(requests)} requests " + "were not verified — its signing secret is missing or does " + "not match the sender's.", + note=( + "Set the provider's signing secret on the source in the " + "Hookdeck dashboard. Without it the source accepts " + "anything, including forged payloads." + ), + ) + ) + else: + checks.append( + Check( + True, + f"source '{source.get('name')}' verified all of its last " + f"{len(requests)} requests as {source_type}", + ) + ) + return checks + + +def _models(result: Any) -> list[dict]: + """The list out of a paginated response, whichever key it used.""" + if not isinstance(result, dict): + return [] + models = result.get("models") or result.get("data") or [] + return models if isinstance(models, list) else [] + + def _burst_headroom(connection: dict, retry_rule: dict, extra: dict) -> Check: """How large a burst this connection absorbs before events start dying. @@ -710,6 +795,7 @@ async def _check_live_connections(routes: dict, extra: dict) -> list[Check]: ) ) checks.append(_burst_headroom(connection, rule, extra)) + checks += await _check_source_verification(api, routes) checks.append(Check(True, "Hookdeck API reachable and the key is accepted")) return checks diff --git a/tests/test_cli.py b/tests/test_cli.py index 413d71c..24524b9 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -269,6 +269,12 @@ async def list_issues(self, **kw): async def list_connections(self, **kw): return self._answer("list_connections", **kw) + async def list_sources(self, **kw): + return self._answer("list_sources", **kw) + + async def list_requests(self, **kw): + return self._answer("list_requests", **kw) + async def pause_connection(self, cid): return self._answer("pause_connection", cid) @@ -662,3 +668,91 @@ def test_unlimited_concurrency_defers_nothing(doctor_env, fake_api, monkeypatch, cli.hookdeck_command(_ns("doctor")) assert "nothing is deferred for capacity" in capsys.readouterr().out + + +# ── doctor: named source types verify nothing without a secret ────── + + +def _typed_route_doctor(doctor_env, fake_api, monkeypatch, *, requests): + """A doctor run with one STRIPE-typed route and the given request history.""" + from hookdeck.provision import retryable_status_codes + + monkeypatch.setenv("HOOKDECK_API_KEY", "key") + doctor_env.setattr(cli.shutil, "which", lambda _b: "/usr/local/bin/hookdeck") + doctor_env.setattr(cli, "_cli_version", lambda _b: "2.4.0") + doctor_env.setattr(cli, "_other_hookdeck_binaries", lambda _r: []) + fake_api.responses["list_connections"] = { + "models": [{"name": "payments", "team_id": "tm_1", + "rules": [{"type": "retry", "count": 10, + "response_status_codes": retryable_status_codes()}]}] + } + fake_api.responses["list_sources"] = {"models": [{"id": "src_1", "name": "payments"}]} + fake_api.responses["list_requests"] = {"models": requests} + _configure(doctor_env, secret="s", cli_config_path="", + routes={"payments": {"source": "payments", "source_type": "STRIPE"}}) + return cli.hookdeck_command(_ns("doctor")) + + +def test_doctor_fails_when_a_typed_source_is_not_verifying( + doctor_env, fake_api, monkeypatch, capsys +): + # Measured, not theorised: a GITHUB source with no secret accepted an + # unsigned request and one signed `sha256=deadbeef`, and made events from + # both. The type alone verifies nothing. + code = _typed_route_doctor( + doctor_env, fake_api, monkeypatch, + requests=[{"verified": False}, {"verified": False}, {"verified": True}], + ) + out = capsys.readouterr().out + assert code == 1 + assert "2 of its last 3 requests were not verified" in out + assert "signing secret is missing or does not match" in out + assert "accepts anything" in out + + +def test_doctor_confirms_verification_when_the_traffic_shows_it( + doctor_env, fake_api, monkeypatch, capsys +): + code = _typed_route_doctor( + doctor_env, fake_api, monkeypatch, + requests=[{"verified": True}, {"verified": True}], + ) + out = capsys.readouterr().out + assert code == 0 + assert "verified all of its last 2 requests as STRIPE" in out + + +def test_doctor_says_unconfirmed_rather_than_ok_with_no_traffic( + doctor_env, fake_api, monkeypatch, capsys +): + # The source's own record is identical whether or not a secret is set, so + # with no requests there is nothing to read. Claiming it is fine would be + # the one answer that is definitely wrong. + code = _typed_route_doctor(doctor_env, fake_api, monkeypatch, requests=[]) + out = capsys.readouterr().out + assert code == 0 # not a failure — unknowable, not broken + assert "no traffic yet, so verification is unconfirmed" in out + assert "accepts forged payloads" in out + + +def test_a_plain_webhook_source_is_not_nagged_about_provider_secrets( + doctor_env, fake_api, monkeypatch, capsys +): + # WEBHOOK sources verify with the project's own signing secret, which the + # adapter already checks. This warning would be noise. + from hookdeck.provision import retryable_status_codes + + monkeypatch.setenv("HOOKDECK_API_KEY", "key") + doctor_env.setattr(cli.shutil, "which", lambda _b: "/usr/local/bin/hookdeck") + doctor_env.setattr(cli, "_cli_version", lambda _b: "2.4.0") + doctor_env.setattr(cli, "_other_hookdeck_binaries", lambda _r: []) + fake_api.responses["list_connections"] = { + "models": [{"name": "generic", "team_id": "tm_1", + "rules": [{"type": "retry", "count": 10, + "response_status_codes": retryable_status_codes()}]}] + } + _configure(doctor_env, secret="s", cli_config_path="", routes={"generic": {}}) + cli.hookdeck_command(_ns("doctor")) + out = capsys.readouterr().out + assert "verification is unconfirmed" not in out + assert not calls_named(fake_api, "list_requests")