Unwrap the raw-body envelope, and read auth_type where the API states it

README.md

@@ -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 — 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.
+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. A source whose `type` names the provider can only take that secret through the dashboard; a generic source carrying `auth_type` can be configured over the API. Either way `hermes hookdeck setup` does not set it today, and until it is set the 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.
 

hookdeck/cli.py

@@ -507,77 +507,99 @@ async def _go() -> str:
 
 
 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.
+    """Whether a source that should be verifying actually is.
+
+    Naming a provider does not switch verification on. The provider's own
+    signing secret has to be set, 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.
+
+    Two things are asked, because they answer different questions:
+
+    * **Is it configured?** A generic source carrying `auth_type: STRIPE`
+      reports that back, so this is a direct answer. A source whose `type`
+      names the provider hides its config entirely — one with a secret set is
+      byte-identical to one without — so for those there is nothing to read.
+    * **Is it working?** Only traffic can say. Each inbound request carries
+      `verified`, and a configured-but-wrong secret looks exactly like a
+      missing one from the outside. Stripe issues a different secret per
+      endpoint, so a mismatch is a realistic way to get here.
+
+    Configured is therefore never treated as sufficient: it refines the message
+    and never suppresses the traffic check.
+
+    The source is read from the API rather than from our config, because the
+    config says what we would provision, not what is there. A source created by
+    hand, or changed in the dashboard afterwards, is the case worth catching.
     """
-    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()
+    for route_name, route in routes.items():
         source_name = route.get("source") or route_name
         found = _models(await api.list_sources(name=source_name))
         if not found:
             continue
         source = found[0]
+        name = source.get("name") or source_name
+        declared = ((source.get("config") or {}).get("auth_type") or "").upper()
+        source_type = str(source.get("type") or "WEBHOOK").upper()
+
+        # A plain WEBHOOK source with no provider auth verifies with the
+        # project's own signing secret, which the adapter checks on every
+        # delivery. Nothing to say.
+        if source_type == "WEBHOOK" and not declared:
+            continue
+
+        scheme = declared or source_type
         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"source '{name}' should verify as {scheme}, 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."
+                        "Hookdeck dashboard, and check it is the one for the "
+                        "endpoint actually sending. Until it matches, the "
+                        "source accepts anything, including forged payloads."
                     ),
                 )
             )
+        elif requests:
+            checks.append(
+                Check(
+                    True,
+                    f"source '{name}' verified all of its last {len(requests)} "
+                    f"requests as {scheme}",
+                )
+            )
+        elif declared:
+            checks.append(
+                Check(
+                    True,
+                    f"source '{name}' is configured to verify as {scheme} — no "
+                    "traffic yet to confirm the secret is the right one",
+                )
+            )
         else:
             checks.append(
                 Check(
                     True,
-                    f"source '{source.get('name')}' verified all of its last "
-                    f"{len(requests)} requests as {source_type}",
+                    f"source '{name}' is type {scheme} — no traffic yet, so "
+                    "verification is unconfirmed",
+                    note=(
+                        f"A {scheme} source verifies nothing until the "
+                        "provider's signing secret is set on it in the Hookdeck "
+                        "dashboard, and for this source shape the API does not "
+                        "report whether it is. Until then it accepts forged "
+                        "payloads."
+                    ),
                 )
             )
     return checks

hookdeck/tools.py

@@ -110,6 +110,26 @@ def _with_ledger(action) -> None:
         ledger.close()
 
 
+def _payload_text(body: Any) -> str:
+    """The payload out of ``GET /events/{id}/raw_body``.
+
+    The endpoint answers ``{"body": "<text>"}`` — a wrapper, not the payload.
+    Returning it whole hands the model an escaped string inside an envelope
+    when what it asked for was what the provider sent.
+
+    Missed for a long time because a model tidies it up when it summarises, so
+    a transcript looks right while the tool's own return value is wrong.
+
+    Matched exactly — one key, holding a string — rather than on the presence
+    of ``body``. A payload is third-party JSON and may well have a ``body``
+    field of its own; unwrapping that would quietly return a fragment of the
+    event as though it were the whole thing.
+    """
+    if isinstance(body, dict) and set(body) == {"body"} and isinstance(body["body"], str):
+        return body["body"]
+    return body if isinstance(body, str) else json.dumps(body)
+
+
 def _models(result: Any) -> list[dict]:
     if not isinstance(result, dict):
         return []
@@ -203,8 +223,7 @@ def hookdeck_get_event_body(args: dict) -> str:
     async def _go() -> str:
         async with HookdeckAPI() as api:
             body = await api.get_event_raw_body(event_id)
-        text = json.dumps(body) if not isinstance(body, str) else body
-        return text[:8000]
+        return _payload_text(body)[:8000]
 
     return _run(_go())
 

tests/test_cli.py

@@ -686,7 +686,9 @@ def _typed_route_doctor(doctor_env, fake_api, monkeypatch, *, requests):
                     "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_sources"] = {
+        "models": [{"id": "src_1", "name": "payments", "type": "STRIPE"}]
+    }
     fake_api.responses["list_requests"] = {"models": requests}
     _configure(doctor_env, secret="s", cli_config_path="",
                routes={"payments": {"source": "payments", "source_type": "STRIPE"}})
@@ -756,3 +758,98 @@ def test_a_plain_webhook_source_is_not_nagged_about_provider_secrets(
     out = capsys.readouterr().out
     assert "verification is unconfirmed" not in out
     assert not calls_named(fake_api, "list_requests")
+
+
+def test_doctor_reads_auth_type_directly_when_the_source_states_it(
+    doctor_env, fake_api, monkeypatch, capsys
+):
+    # A generic source carrying `auth_type: STRIPE` reports it back, so there
+    # is no need to infer verification from traffic. A typed source hides its
+    # config entirely, which is why the traffic fallback exists at all.
+    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", "type": "WEBHOOK",
+                    "config": {"auth_type": "STRIPE", "auth": {}}}]
+    }
+    fake_api.responses["list_requests"] = {"models": []}
+    _configure(doctor_env, secret="s", cli_config_path="",
+               routes={"payments": {"source": "payments", "source_type": "STRIPE"}})
+
+    assert cli.hookdeck_command(_ns("doctor")) == 0
+    out = capsys.readouterr().out
+    assert "configured to verify as STRIPE" in out
+    assert "no traffic yet to confirm the secret is the right one" in out
+    # Configured is not working: the traffic check still runs, because a wrong
+    # secret looks identical to a missing one from the outside.
+    assert calls_named(fake_api, "list_requests")
+
+
+def test_a_declared_auth_type_does_not_excuse_failing_traffic(
+    doctor_env, fake_api, monkeypatch, capsys
+):
+    # The regression this exists for: reporting "configured to verify as
+    # STRIPE" and stopping there. A wrong secret leaves auth_type set and every
+    # request failing — and Stripe issues a different secret per endpoint, so
+    # pasting the wrong one is a realistic way to arrive here.
+    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", "type": "WEBHOOK",
+                    "config": {"auth_type": "STRIPE", "auth": {}}}]
+    }
+    fake_api.responses["list_requests"] = {"models": [{"verified": False}] * 3}
+    _configure(doctor_env, secret="s", cli_config_path="",
+               routes={"payments": {"source": "payments", "source_type": "STRIPE"}})
+
+    assert cli.hookdeck_command(_ns("doctor")) == 1
+    out = capsys.readouterr().out
+    assert "3 of its last 3 requests were not verified" in out
+    assert "the one for the endpoint actually sending" in out
+
+
+def test_the_source_is_read_from_the_api_not_from_our_config(
+    doctor_env, fake_api, monkeypatch, capsys
+):
+    # Config says what we would provision; the source says what is there. A
+    # source changed in the dashboard afterwards is the case worth catching, so
+    # a route claiming WEBHOOK must not hide a provider-typed source.
+    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", "type": "GITHUB"}]
+    }
+    fake_api.responses["list_requests"] = {"models": [{"verified": False}]}
+    # Route says nothing about a source type at all.
+    _configure(doctor_env, secret="s", cli_config_path="",
+               routes={"payments": {"source": "payments"}})
+
+    assert cli.hookdeck_command(_ns("doctor")) == 1
+    assert "should verify as GITHUB" in capsys.readouterr().out

tests/test_tools.py

@@ -401,6 +401,44 @@ def test_an_event_body_is_truncated_before_it_reaches_the_context(api):
     assert len(call("hookdeck_get_event_body", {"event_id": "evt_1"})) == 8000
 
 
+def test_the_raw_body_envelope_is_unwrapped(api):
+    # The endpoint answers {"body": "<text>"}. Returning that whole hands the
+    # model an escaped string inside an envelope instead of the payload —
+    # invisible in a transcript, because the model tidies it up when it
+    # summarises.
+    api.responses["get_event_raw_body"] = {
+        "body": '{"kind":"charge.succeeded","amount":2000}'
+    }
+    assert call("hookdeck_get_event_body", {"event_id": "evt_1"}) == (
+        '{"kind":"charge.succeeded","amount":2000}'
+    )
+
+
+def test_an_unwrapped_string_body_still_works(api):
+    api.responses["get_event_raw_body"] = '{"kind":"already-plain"}'
+    assert call("hookdeck_get_event_body", {"event_id": "evt_1"}) == (
+        '{"kind":"already-plain"}'
+    )
+
+
+def test_only_the_exact_envelope_shape_is_unwrapped(api):
+    # Measured: the endpoint always answers exactly {"body": "<string>"}. So a
+    # dict shaped any other way is the payload, not the envelope — including
+    # third-party JSON that happens to carry its own `body` field. Unwrapping
+    # that would return a fragment of the event as though it were the whole
+    # thing, which is worse than the envelope it replaced.
+    api.responses["get_event_raw_body"] = {"body": "inner", "from": "acme"}
+    assert json.loads(call("hookdeck_get_event_body", {"event_id": "evt_1"})) == {
+        "body": "inner",
+        "from": "acme",
+    }
+
+    api.responses["get_event_raw_body"] = {"body": {"kind": "structured"}}
+    assert json.loads(call("hookdeck_get_event_body", {"event_id": "evt_1"})) == {
+        "body": {"kind": "structured"}
+    }
+
+
 def test_a_structured_body_is_serialised(api):
     api.responses["get_event_raw_body"] = {"type": "charge.succeeded"}
     assert json.loads(call("hookdeck_get_event_body", {"event_id": "evt_1"})) == {