Skip to content

test(webhooks): wire signer-conformance harness into npm test (#2546, #2549)#2609

Merged
bokelley merged 2 commits intomainfrom
bokelley/signer-dup-keys
Apr 21, 2026
Merged

test(webhooks): wire signer-conformance harness into npm test (#2546, #2549)#2609
bokelley merged 2 commits intomainfrom
bokelley/signer-dup-keys

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented Apr 21, 2026

Summary

The signer-side duplicate-key fixtures (#2546) and the signer-conformance harness (#2549) were both landed by PR #2548, but the harness wasn't wired into CI. A regression in the reference signer — or a fixture miscategorization — would not have failed any build.

Code review on the initial commit caught that the sibling test:hmac-vectors (verifier-side) had the same dead-wiring — present as an npm script but never invoked by any workflow.

This PR closes both gaps in one move.

What ships

  • .github/workflows/schema-validation.yml gains a new "HMAC webhook conformance (verifier + signer)" step that runs both test:hmac-vectors and test:hmac-signer-conformance on every PR.
  • package.json gains the test:hmac-signer-conformance npm script and slots it into the top-level test chain for local discoverability.
  • tests/webhook-hmac-signer-conformance.test.cjs gains a top-level assertion that the signer_side fixture block exists with non-empty rejection_vectors and positive_vectors — so a future fixture refactor can't silently make the gate vacuous.

No changes to fixtures, reference signer, or any production code.

Closes

Review findings addressed

  • code-reviewer Must Fix: CI doesn't invoke npm test; adding to the chain was insufficient. Fixed by wiring into schema-validation.yml directly.
  • security-reviewer Consider 1: harness silently no-ops on missing signer_side block. Added guard assertion.
  • security-reviewer Consider 2 + 3 (fixture-coverage gaps for overflow markers and in-name non-printables) — filed as follow-ups, out of scope for CI-wiring PR.

Test plan

  • npm run test:hmac-signer-conformance — 31 tests pass (30 + new fixture guard)
  • npm run test:hmac-vectors — passes
  • Full npm test — 631 tests pass, typecheck clean
  • schema-validation CI check passes on this PR (new step included)

🤖 Generated with Claude Code

bokelley and others added 2 commits April 20, 2026 20:25
The harness (tests/webhook-hmac-signer-conformance.test.cjs) and
fixtures (static/test-vectors/webhook-hmac-sha256.json signer_side
block) were added by #2548 but the harness had no npm script
wiring, so it wasn't on the default test path. A regression in the
reference signer or a fixture miscategorization would not have
failed CI.

Adds test:hmac-signer-conformance npm script and slots it into the
top-level test chain next to test:hmac-vectors. No code or fixture
changes.

Closes #2546 (fixture was landed by #2548).
Closes #2549 (harness wired to CI by this change).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up from code review on the initial PR. The previous commit
added test:hmac-signer-conformance to the top-level npm test chain,
but CI doesn't invoke npm test — it runs a hand-picked subset of
scripts via build-check.yml and schema-validation.yml. The script
was discoverable locally but unreachable on PRs.

Turns out the sibling test:hmac-vectors had the same dead-wiring —
landed as an npm script but never called by any workflow.

Changes:
- .github/workflows/schema-validation.yml: new step runs both
  test:hmac-vectors (verifier-side) and test:hmac-signer-conformance
  (signer-side). One gate, both sides of the duplicate-key contract.
- tests/webhook-hmac-signer-conformance.test.cjs: assert the
  signer_side fixture block exists and both vector kinds are
  non-empty, so a future fixture refactor can't silently make the
  gate vacuous (the for-of loops would otherwise exit 0 with zero
  assertions).

The npm script and test-chain slot-in from the prior commit stay —
still useful for local discovery and for any future CI path that
does invoke the top-level npm test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 62e9cf0 into main Apr 21, 2026
15 checks passed
@bokelley bokelley deleted the bokelley/signer-dup-keys branch April 21, 2026 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant