Unwrap the raw-body envelope, and read auth_type where the API states it - #17
Merged
Conversation
Both from the OpenClaw session's live-testing notes, verified here against the
same project before acting on either.
**`hookdeck_get_event_body` returned an envelope, not a payload.**
`GET /events/{id}/raw_body` answers `{"body": "<text>"}`, and we handed that
back whole — so the model asked what a provider sent and received an escaped
string inside a wrapper. Confirmed live against a real event.
Worth recording how it hid. This tool was exercised live earlier in the week
and looked right, because the agent's summary showed a clean payload: the model
unwraps it while writing prose. What was checked was the transcript, not the
tool's return value. A test asserting the tool's own output would have caught it
on day one, and now does.
**doctor can sometimes just ask.** The earlier check inferred verification from
observed traffic, on the grounds that a source's config never reveals whether a
secret is set. That holds for a source whose `type` names the provider, and I
had generalised it. It is not true of a generic source carrying
`auth_type: STRIPE`, which reports that `auth_type` back — measured: an unsigned
request to one came back VERIFICATION_FAILED, and the source's config showed
`auth_type: STRIPE` with the secret itself hidden.
So doctor now reads `auth_type` when it is there and falls back to traffic when
it is not, and the README no longer says the secret can only come from the
dashboard — that is true of one shape, not both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of my own change, which had introduced a worse bug than the one it
fixed.
Reading `auth_type` told doctor the source was *configured*, and I then
returned early — so a source with `auth_type` set and the wrong secret reported
green while refusing every request. The traffic check I was replacing would
have caught exactly that, and Stripe issues a different signing secret per
endpoint, so a mismatch is the realistic way to get there rather than an exotic
one. Configured now refines the message and never suppresses the check.
Two further corrections while re-reading it:
* The check keyed off `source_type` in our config, which says what we would
provision rather than what exists. It now reads the source from the API, so a
source created by hand or changed in the dashboard is seen for what it is —
and the generic `auth_type` shape, which lives on a WEBHOOK-typed source, is
no longer excluded by the very filter meant to find it.
* `_payload_text` unwrapped any dict containing `body`. Measured across several
events, the envelope is always exactly `{"body": "<string>"}`, so it now
matches that shape and nothing else. A payload is third-party JSON and may
carry its own `body` field; unwrapping that would hand back a fragment of the
event as though it were the whole thing.
Four mutations checked. The first attempt at one of them was a no-op — it
assigned a variable the next line overwrote — and reported as survived; re-run
properly, the short-circuit is caught by two tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From the OpenClaw session's live-testing notes. I verified both against the same project before acting — three of their other findings didn't apply to us (we don't call
/bulk/requests/replay, we already readissues.type, and our boot recovery uses the ledger rather than a request-replay cursor).hookdeck_get_event_bodyreturned an envelope, not a payloadGET /events/{id}/raw_bodyanswers{"body": "<text>"}. We handed that back whole:So an agent asking what a provider sent got an escaped string inside a wrapper.
How it hid is the interesting part. I exercised this tool live earlier in the week and it looked correct — the agent's summary showed a clean
{"kind":"burst-test","n":1}. The model unwraps it while writing prose. I verified the transcript rather than the tool's return value. A test asserting the tool's own output catches it immediately, and now does.doctor can sometimes just ask
The check added in #16 infers verification from observed traffic, on the grounds that a source never reveals whether a secret is configured. That holds for a source whose
typenames the provider, and I generalised it too far. A generic source carryingauth_type: STRIPEreports thatauth_typeback — measured:So doctor now reads
auth_typewhen present and falls back to traffic when not. The README no longer claims the secret can only come from the dashboard — true of one shape, not both.Worth a follow-up, not in this PR
Since the generic shape is API-settable,
hermes hookdeck setupcould configure provider verification directly instead of leaving it a manual step. That is a provisioning design change — typed sources may carry behaviour we would lose — so it deserves its own decision rather than riding along here.317 tests, ruff clean. Ships in the wheel.