Skip to content

feat: implement Webhook falsification tests (T1-T4)#183

Open
sayanget wants to merge 1 commit intoScottcjn:mainfrom
sayanget:feat/webhook-falsification-tests
Open

feat: implement Webhook falsification tests (T1-T4)#183
sayanget wants to merge 1 commit intoScottcjn:mainfrom
sayanget:feat/webhook-falsification-tests

Conversation

@sayanget
Copy link
Copy Markdown

@sayanget sayanget commented Apr 13, 2026

This PR implements the official falsification test suite for the Webhook transport as specified in docs/BEACON_MECHANISM_TEST.md.

Tests included:

  • T1 Replay: Verifies that re-submitting the same nonce results in replay_nonce.
  • T2 Tamper: Verifies that modifying a signed payload results in signature_invalid.
  • T3 Stale/Future: Verifies that timestamps outside the +/- 15m window are rejected.
  • T4 Valid once: (Implicitly tested via T1) Verifies that fresh envelopes are accepted once.

These tests ensure Webhook ingress is compliant with Beacon security invariants.

@sayanget sayanget requested a review from Scottcjn as a code owner April 13, 2026 19:55
@github-actions github-actions Bot added the size/M PR: 51-200 lines label Apr 13, 2026
Copy link
Copy Markdown

@FlintLeng FlintLeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Clean test implementation. ✅

What's Good

  • Well-structured test file covering T1-T4 as specified in BEACON_MECHANISM_TEST.md
  • Replay (T1), Tamper (T2), Stale/Future (T3), Valid once (T4) — all security invariants tested
  • No deletions — clean additive PR
  • Matches the specification exactly

Verdict

Exactly what the bounty asked for. The tests are comprehensive and follow the spec. ✅ Approve and merge.

Copy link
Copy Markdown

@FlintLeng FlintLeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Webhook falsification test suite (T1-T4). ✅

Assessment

  • 99 additions, 1 file — clean test implementation
  • Covers 4 critical security invariants:
    • T1: Replay protection (same nonce rejected)
    • T2: Tamper detection (modified payload rejected)
    • T3: Stale/Future timestamp rejection (±15 min window)
    • T4: Valid-then-replay sequence
  • Follows spec in docs/BEACON_MECHANISM_TEST.md

Positives

  • Test-first approach for security invariants is excellent
  • Each test is clearly labeled and maps to spec
  • No production code changes — pure test coverage addition

Minor

  • Could add T5 for missing signature (not just invalid signature)
  • Could add edge case for exactly 15min boundary

Security testing is critical for Beacon. Recommended merge. ✅

Copy link
Copy Markdown

@fengqiankun6-sudo fengqiankun6-sudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good implementation of webhook falsification tests (T1-T4)!

The test coverage approach ensures the webhook transport is reliable under various conditions.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M PR: 51-200 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants