From 13985c38518b94c1ea79a36e67d7bfb0fa4ad4ec Mon Sep 17 00:00:00 2001 From: garethx Date: Wed, 12 Aug 2026 15:10:06 +0100 Subject: [PATCH] Report whether a typed source is actually verifying anything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 against a real project: a GITHUB source with no secret accepted an unsigned request *and* one carrying `sha256=deadbeef`, and produced events from both. With the secret set, an unsigned request came back VERIFICATION_FAILED with no event, and a genuine signed Stripe delivery passed. `setup` says the secret is a manual step, once, in its own output. The README stated the verification as a flat benefit, and nothing mentioned it again. doctor now reports it, and the interesting part is what it can and cannot know. A source with a secret configured is byte-identical to one without over the API — `config.auth` is absent and `verification` is null either way, confirmed against a source whose secret was definitely set. So there is nothing to read on the source itself. What the API does expose is `verified` on each inbound request, so the check reads observed traffic: * some requests unverified -> fail, and say the secret is missing or mismatched * all verified -> confirm, naming the count it saw * no requests at all -> say "unconfirmed", not "fine" The third case is the one worth being careful about. With no traffic there is genuinely no signal, and claiming the source is fine would be the one answer certain to be wrong. Plain WEBHOOK sources are left alone: they verify with the project's own signing secret, which the adapter already checks, so the warning would be noise. Co-Authored-By: Claude Opus 5 --- README.md | 4 +- hookdeck/api.py | 10 +++++ hookdeck/cli.py | 86 +++++++++++++++++++++++++++++++++++++++++++ tests/test_cli.py | 94 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 2 deletions(-) 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")