From ab4822101d081db1c5d0df9a1c1f5a4a4ffeec9d Mon Sep 17 00:00:00 2001 From: garethx Date: Thu, 13 Aug 2026 12:28:48 +0100 Subject: [PATCH] Treat an absent `verified` as unknown, not as unverified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The n8n session measured `/requests` listings that omit `verified`. It is present on this project — checked, every model in the listing carries it — so the finding does not reproduce here. It still exposes a real fragility: the check read the field with `.get()`, so an absent one counted as a failure. That is the wrong direction to be wrong in. "The API did not say" would have been reported as "your source is accepting forgeries", sending someone to re-paste a signing secret that was never the problem, on the one check whose whole job is telling them what is actually true. Requests are now judged only where they report themselves, counts are over what was judged rather than what was returned, and a listing that reports nothing is called unconfirmed. Co-Authored-By: Claude Opus 5 --- hookdeck/cli.py | 26 +++++++++++++++----- tests/test_cli.py | 60 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/hookdeck/cli.py b/hookdeck/cli.py index 6dc1ad5..0dab4b0 100644 --- a/hookdeck/cli.py +++ b/hookdeck/cli.py @@ -553,14 +553,28 @@ async def _check_source_verification(api: HookdeckAPI, routes: dict) -> list[Che requests = _models( await api.list_requests(source_id=source.get("id"), limit=10) ) - unverified = [r for r in requests if not r.get("verified")] - - if unverified: + # Only requests that actually carry the field can be judged. Treating + # an absent `verified` as false would turn a listing that simply does + # not report it into "your source is accepting forgeries" — a false + # alarm that sends someone to re-paste a secret that was never wrong. + judged = [r for r in requests if "verified" in r] + unverified = [r for r in judged if not r["verified"]] + + if requests and not judged: + checks.append( + Check( + True, + f"source '{name}' should verify as {scheme}, but this " + "Hookdeck response does not report whether its requests " + "were verified — unconfirmed", + ) + ) + elif unverified: checks.append( Check( False, f"source '{name}' should verify as {scheme}, but " - f"{len(unverified)} of its last {len(requests)} requests " + f"{len(unverified)} of its last {len(judged)} requests " "were not verified — its signing secret is missing or does " "not match the sender's.", note=( @@ -571,11 +585,11 @@ async def _check_source_verification(api: HookdeckAPI, routes: dict) -> list[Che ), ) ) - elif requests: + elif judged: checks.append( Check( True, - f"source '{name}' verified all of its last {len(requests)} " + f"source '{name}' verified all of its last {len(judged)} " f"requests as {scheme}", ) ) diff --git a/tests/test_cli.py b/tests/test_cli.py index e175239..099a76f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -853,3 +853,63 @@ def test_the_source_is_read_from_the_api_not_from_our_config( assert cli.hookdeck_command(_ns("doctor")) == 1 assert "should verify as GITHUB" in capsys.readouterr().out + + +def test_requests_without_a_verified_field_are_unknown_not_failures( + doctor_env, fake_api, monkeypatch, capsys +): + # A sibling integration measured `/requests` listings that omit `verified`. + # It is present on this project, but if it ever is not, reading a missing + # key as false turns "the API did not say" into "your source is accepting + # forgeries" — and sends someone to re-paste a secret that was fine. + 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": "STRIPE"}] + } + # Requests exist, but none reports the field. + fake_api.responses["list_requests"] = {"models": [{"id": "req_1"}, {"id": "req_2"}]} + _configure(doctor_env, secret="s", cli_config_path="", + routes={"payments": {"source": "payments", "source_type": "STRIPE"}}) + + assert cli.hookdeck_command(_ns("doctor")) == 0 # unknowable, not broken + out = capsys.readouterr().out + assert "does not report whether its requests were verified" in out + assert "were not verified" not in out + + +def test_a_mix_is_judged_only_on_what_reports_itself( + doctor_env, fake_api, monkeypatch, capsys +): + 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": "STRIPE"}] + } + fake_api.responses["list_requests"] = { + "models": [{"verified": False}, {"id": "no_field"}, {"verified": True}] + } + _configure(doctor_env, secret="s", cli_config_path="", + routes={"payments": {"source": "payments", "source_type": "STRIPE"}}) + + assert cli.hookdeck_command(_ns("doctor")) == 1 + # Two judged, not three: the one that said nothing is not counted either way. + assert "1 of its last 2 requests were not verified" in capsys.readouterr().out