Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes. ✅ Duplication (Python) — cleanNew Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying. ✅ Duplication (TypeScript) — cleanNew TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying. |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
PostHog Review · Standard · reviewed e5a87a4: 1 Must fix, 1 Should fix.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
HostHog preview —
|
…uests The executor sends no body for GET and HEAD, so every signer must sign the empty body. Only the HMAC-SHA256 signer had a receiver-side test for that. Standard Webhooks now verifies against the bytes received for POST, GET and HEAD, and SigV4 re-signs the received GET and HEAD request as AWS does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
A fetch that asked for Standard Webhooks and HMAC-SHA256 minted one webhook-id per signer. The HMAC signer then overwrote the webhook-id header that the Standard Webhooks signature covers, so receivers rejected it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The executor must accept hmac_sha256 everywhere before any template can ask for it, so the template choice lands in the next pr of this stack. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Note 🤖 Claude Opus 5.5 responding on behalf of Michael Decisions from the takeover session:
|
Silthus
changed the title
SigV4 covers the headers it is given. Running it before the Standard Webhooks or HMAC-SHA256 signer let those replace a header SigV4 had already signed, so AWS would reject the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Run the signed fetch calls from hog through the executor to a local receiver, in place of the queue-payload tests. Drop the comments this change added, inline the webhook id spread, and rename the header override helper now that both secret headers and the HMAC signer use it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move overrideHeaders into its own module and let the Standard Webhooks signer use it too, so its case-insensitive replace lives in one place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pass the local receiver's URL as an input, so the compiled code no longer changes with the port, and commit the bytecode fixtures CI needs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Risk: No findings This increment only adds test coverage in hog-executor.service.test.ts: refusal cases for an empty-body POST and a 24-byte multi-byte secret, plus a parameterized happy path. Each assertion matches an existing fail-closed guard (empty-sentBody refusal in hog-executor-async.service.ts:500 and the Buffer.byteLength MIN_SECRET_BYTES check in hmac-sha256-signature.ts:33-35), so the change introduces no new security risk. Sentinel reviewed |
|
Review requests went to the owners of the changed code. These owners were not requested because their part of this change looks trivial:
If you were listed here and want to review, you can still assign yourself as a reviewer. |
posthog-security-review-bot
Bot
added
the
security-review
|
✅ Security review complete — 1 finding posted as a review, in 6 min. Add the |
![parameterai[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
![posthog-security-review-bot[bot]](https://avatars.githubusercontent.com/in/4551195?s=60&v=4){kind=small}
posthog-security-review-bot
Bot
removed
the
security-review
…ecret Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-byte secret Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-adding the stamphog label gives the same result unless the changed files change.
Stamphog can't approve this pull request automatically. Two gates refused it. The deny-list gate flagged it for crypto_secrets, because it changes signing code and secret handling, such as the new nodejs/src/cdp/utils/hmac-sha256-signature.ts and the edits to standard-webhooks.ts and secret-headers.ts. The tier gate classified it as T2-never (512 lines across 12 files, a fix), which is a category that always needs a human reviewer.
The size gate passed, so splitting the PR won't clear these refusals. The author should ask a human reviewer, ideally one who owns the CDP webhook and signing code, to review it.
- posthog[bot] reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: crypto_secrets |
| size | ✓ | 190L, 7F substantive, 512L/12F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (512L, 12F, single-area, fix) |
| stamphog 2.8.0 | .stamphog/policy.yml @ unknown · reviewed head 3090b4b |
Problem
Moved from the fork PR #126752, which could not run CI.
Worth: W-20261113-webhook-signed-get-head
Note
🔵 Merge risk: low · two-way door until a function first sends
hmac_sha256, from custom Hog or the #135206 template: then its receivers depend on theX-Hub-Signature-256contract, and queued retries carry the option.hmac_sha256fetch option: no template sets it until #135206. Custom Hog that passes it now gets a signed POST or PUT, where today's worker ignores it and sends it unsigned. A request with no body, or a secret under 24 bytes, now fails with a logged error instead.nodejs/src/cdp/services/hog-executor-async.service.ts,nodejs/src/cdp/async-functions/,nodejs/src/cdp/utils/,nodejs/src/cdp/schema/)Changes
hmac_sha256fetch option. It signs each attempt withX-Hub-Signature-256: sha256=<hex>, so the queue payload holds only a secret reference.webhook-id, shared by every signer it uses and stable across retries.How did you test this code?
Test seams: the executor fetches from a local receiver that checks the bytes it got, for POST, GET, HEAD and a retry.
fetchruns through the executor to the receiver. The tests fail if two calls share awebhook-id, or if both signers of one call don't sign the same one.webhook-id, and a wrong secret gets a 401. HMAC GET, HEAD and a short secret send nothing and log why.Release status
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5)
Skills:
/writing-tests,/writing-pr-descriptions,/stacking-prs,takeover,full-stop,impact,tdd,qa-swarm,merge-risk,pr-descriptionSecurity review: not needed
More context for AI agents
sentBodychange inhog-executor-async.service.ts, since it touches every signed GET or HEAD.webhook-id, as Standard Webhooks already does.Involvement: one design pass, two review panel rounds, two upstream review rounds, three bot review rounds.
🤖 Generated with Claude Code