fix(webhooks): bound signature verification work - #2475
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 1 existing customization changed. Compared
31 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 32903923844 --repo openai/openai-node \
--name castiron-custom-code-32903923844-1 --dir /tmp/castiron-custom-code-32903923844-1
git apply --stat /tmp/castiron-custom-code-32903923844-1/custom-code.patch
cat /tmp/castiron-custom-code-32903923844-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 9dd86484d9d5a2212ea8ec472fad6f76c747ba14 05ff3a5300b372032de7d97645b4accac10b6a1b
python3 scripts/castiron/custom_code_report.py report \
--base 9dd86484d9d5a2212ea8ec472fad6f76c747ba14 \
--head 05ff3a5300b372032de7d97645b4accac10b6a1b --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-05ff3a5300b3
cat /tmp/castiron-custom-code-05ff3a5300b3/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83d421a70f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Requesting changes for the existing P2 discussion at src/lib/webhook-signature.ts:40. The new undocumented 32-candidate limit rejects an entire otherwise valid webhook header before checking any signature, including a valid rotated signature in slot 33 or even slot 1 when additional candidates are present. The prior public verifier accepted a valid signature at any position, and no owner-approved protocol maximum is documented. Bound expensive HMAC work without rejecting previously valid rotation headers—for example, derive the expected HMAC once and safely compare candidates—or establish an explicit approved/documented compatibility limit and migration. Add a valid >32-candidate regression. Existing inline discussion already captures the issue.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 429c28088d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2 participants
Summary
Validation