IFC-2257: add unit test for webhook signature validation#8606
Conversation
Adds a regression test that validates HMAC signature computation against a hardcoded expected value using a non-trivial payload. Catches two historical bugs: signing str(dict) instead of JSON, and using spaced instead of compact JSON encoding. Relates to: IFC-2257 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (4)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
Merging this PR will improve performance by 38.68%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_get_schema |
324 ms | 259.4 ms | +24.94% |
| ⚡ | test_relationshipmanager_getpeer |
156.7 µs | 134.5 µs | +16.52% |
| ⚡ | test_get_menu |
245 ms | 193.6 ms | +26.54% |
| ⚡ | test_schemabranch_duplicate |
7.3 ms | 5.8 ms | +25.96% |
| ⚡ | test_query_one_model |
463.7 ms | 353.4 ms | +31.24% |
| ⚡ | test_query_rel_one |
662.6 ms | 501 ms | +32.26% |
| ⚡ | test_query_rel_many |
692.2 ms | 524 ms | +32.11% |
| ⚡ | test_base_schema_duplicate_CoreProposedChange |
2.1 ms | 1.7 ms | +28.15% |
| ⚡ | test_graphql_generate_schema |
516.2 ms | 372.3 ms | +38.68% |
| ⚡ | test_nodemanager_querypeers |
1.5 ms | 1.3 ms | +21.66% |
| ⚡ | test_load_node_to_db_node_schema |
66.3 ms | 52.1 ms | +27.29% |
| ⚡ | test_schemabranch_process |
1,027 ms | 823.4 ms | +24.72% |
Comparing pmi-20260314-serialization-unit-test-ifc2257 (f8d3e9d) with stable (43d16b8)1
Footnotes
ogenstad
left a comment
There was a problem hiding this comment.
LGTM, it would be nice if we could also capture any changes within the httpx package though.
I added a new unit test package named A new unit test has been added testing the |
9bd3f98
into
pmi-20260227-webhooks-ifc-2272
Why
Webhook HMAC signature calculation has had two regressions in the past:
str(dict)instead ofjson.dumps(dict)json.dumpsused default separators (,and:) instead of compact (,,:) after HTTPX 0.28.x switchThe existing
test_standard_webhook_headertest only validates signing with an empty payload, which serializes identically regardless of format — so it catches neither regression.Closes https://opsmill.atlassian.net/browse/IFC-2257
What changed
test_webhook_signature_with_payloadtobackend/tests/unit/webhook/test_models.pyHow to review
Single test addition in
backend/tests/unit/webhook/test_models.py. The hardcoded signature was verified by mutating the source to reintroduce both historical bugs and confirming the test fails in each case.How to test
Checklist