feat(arch-08): migrate stripe-webhook to typed StripeEvent + drop Jackson

[ff-team-sobrado]

Summary

ARCH-08 Phase 2 — the code migration follow-up to the merged ADR (#7). Replaces direct Jackson use in stripe-webhook production code with a typed sealed StripeEvent hierarchy + Micronaut Serde. Adds a CI static check that fails the build on any future direct Jackson reference in src/main.

Three commits

SHA	Subject
79be074	feat(arch-08): typed StripeEvent sealed hierarchy + @Serdeable wire envelope
d6dafa5	feat(arch-08): migrate stripe-webhook src/main to typed StripeEvent + Serde
f751fe1	test(arch-08): migrate tests to typed StripeEvent + drop Jackson testImplementation

Architecture (post-merge)

HTTP body
   │
   ▼
WebhookEventProcessor.process
   │  ① HMAC verify (InvalidSignatureException → 400)
   │  ② Serde ObjectMapper.readValue → StripeEventEnvelope (SerdeException → 400)
   │  ③ envelope.id() null/empty → 400
   │  ④ idempotencyStore.recordFirstDelivery (REPLAY → 200)
   │  ⑤ toStripeEvent(envelope) — type-discrimination switch
   ▼
WebhookEventDispatcher.dispatch(StripeEvent)
   │  switch (event) {
   │    case PaymentIntentSucceeded → succeededHandler.handle(e)
   │    case PaymentIntentFailed    → failedHandler.handle(e)
   │    case Ignored                → handleIgnored(e)
   │  }   // exhaustive — no default — compile-time enforced
   ▼
PaymentIntentSucceededHandler.handle(StripeEvent.PaymentIntentSucceeded)
   ├─ event.payload() → typed PaymentIntentObject
   ├─ ReservationKey.fromStripeMetadata(payload.metadataOrEmpty())
   └─ … (no JsonNode anywhere; no path(...).asLong() traversals)

What's gone

• StripeWebhookEvent (record holding JsonNode dataObject).
• EventHandler interface (replaced by typed handler methods).
• IgnoredEventHandler bean (inlined into dispatcher's switch).
• Map<String, EventHandler> string-keyed dispatch table (replaced by sealed switch).
• ReservationKey.fromStripeMetadata(JsonNode) factory + its private helpers.
• ReservationKey.requireLong's numeric-shape defensive branch (test-fixture artifact; production wire is strings-only — verified against PaymentLambda's stripeMetadata helper).
• Direct implementation("com.fasterxml.jackson.core:jackson-databind") dependency.
• All Jackson FQN references in src/main and src/test.

Tomás review priorities — all six locked

1. Sealed-completeness in dispatcher switch — WebhookEventDispatcher.dispatch switch has NO default branch; Ignored is an explicit case. Tested by WebhookEventDispatcherTest.routesIgnoredVariantWithoutTouchingTypedHandlers.
2. Every @Serdeable on a record we own — PaymentIntentError, PaymentIntentObject, StripeEventEnvelope, StripeEventEnvelope.Data are all our records. No external-class registration; no mixins.
3. Boundary returns resolved variant; dispatcher never re-parses — WebhookEventProcessor.toStripeEvent resolves the wire envelope to a typed StripeEvent once. The dispatcher's switch and downstream handlers operate on typed records only. Verified by WebhookEventProcessorTest's typed-variant assertions on the captured argument.
4. Three layered checks fail-closed — HMAC (invalidSignatureReturns400), Serde parse (malformedJsonBodyReturns400AfterValidSignature), missing event id (bodyWithoutEventIdReturns400), unknown type (unknownEventTypeStillReturns200AndDispatchesIgnoredVariant). All return without exception escape.
5. CI grep catches FQN — .github/workflows/build.yml runs find src/main -exec grep -l "com\.fasterxml\.jackson" (unanchored — catches both import and FQN uses) BEFORE ./gradlew build. Per Obs feat(pay-02): verify, dedup, and dispatch Stripe webhook events #1 from PR docs(arch-08): ADR-0008 consolidate JSON serialization on Micronaut Serde #7.
6. jackson-databind only as transitive — direct dependency removed from build.gradle. The library still reaches the classpath via micronaut-serde-jackson's transitive graph (Serde uses Jackson's streaming parser internally), but never at depth 1.

Behaviour-change disclosures (per Obs #3 from PR #7)

• Silent-zero on missing JSON fields → 400 MALFORMED_JSON. Pre-migration, path("X").asLong() returned 0 for missing fields. Post-migration, Serde rejects missing required fields at the boundary. Strictly safer — production never sent malformed bodies; the change closes a defense-in-depth gap.
• Test fixtures using numeric metadata → strings via Long.toString(...). PaymentIntentSucceededHandlerTest and PaymentIntentFailedHandlerTest previously used ObjectNode.put(String, long) overloads producing JSON numerics. Now use Map.of("...", Long.toString(...)) matching the production wire shape (PaymentLambda's stripeMetadata helper).
• Non-numeric event.created no longer falls back silently. Pre-migration, root.get("created").isNumber() ? ... : now tolerated a string-typed created. Post-migration, Serde fails the parse at the boundary (the field is long on StripeEventEnvelope). The corresponding test was removed because production never sees this shape (Stripe API contract is epoch-second number).

Test plan

• ./gradlew build green locally (vanilla flavor — see commit f751fe1 for the run).
• 70 / 70 tests pass.
• find src/main -name "*.java" -exec grep -l "com\.fasterxml\.jackson" {} + returns empty.
• No import com.fasterxml.jackson AND no FQN com.fasterxml.jackson references in src/main or src/test.
• ./gradlew dependencies shows jackson-databind only as transitive of micronaut-serde-jackson, never at depth 1.
• Tomás review-pair on the deserialization-gadget surface (per the locked priorities above).
• CI green on the PR (the new ADR-0008 step runs first; both vanilla and dd flavor build the native image).

Follow-ups

• ARCH-08-followup-revolut — same pattern applied to backend/revolut-webhook (sibling ADR + MR; my queue, after this lands).
• Task #13 — pii-ingestion buildAuditDetails adopts Serde ObjectMapper — Tomás's queue, unblocked by this MR.

[ff-team-sobrado]

Tomás review-pair sign-off — APPROVE (posted as comment due to same-bot-account self-approval block)

Architectural rationale from ADR-0008 carries through cleanly into the migration. Six locked priorities all addressed; I'd land this. Two structural observations (one for each of your specific asks), both non-blocking.

---

Six review priorities — verified

1. Sealed-completeness in dispatcher switch. ✅
   WebhookEventDispatcher.dispatch is a switch expression on StripeEvent with three explicit cases and no default branch. Ignored is its own first-class case routed to handleIgnored. Tests pin both the typed routes and the Ignored-with-null-type / Ignored-with-unrecognised-type paths. Future variant addition is compile-time forced — Java's sealed-type exhaustiveness is the guard.

2. Every @Serdeable on a record we own. ✅
   grep -rE "@Serdeable" src/main returns 4 declarations: PaymentIntentError, PaymentIntentObject, StripeEventEnvelope, StripeEventEnvelope.Data — all in com.functorful.stripewebhook.*. grep -rE "@SerdeImport" src/main returns empty — no external class is forced into Serde via mixin or import. The @Serdeable(naming = SnakeCaseStrategy.class) use on the snake-case-bound records is correct: SnakeCaseStrategy is a Micronaut-supplied implementation (no custom strategy on attacker-controlled fields).

3. Boundary returns resolved variant; dispatcher never re-parses. ✅
   WebhookEventProcessor.toStripeEvent is static, package-private (testable), returns the sealed StripeEvent type. The dispatcher's dispatch(StripeEvent) signature matches. Handlers receive PaymentIntentObject directly via e.payload() — never a raw envelope, never objectMapper.readValue on a data field. Verified by reading WebhookEventProcessor.java:135-148 and WebhookEventDispatcher.java:63-69.

4. Three layered checks fail-closed. ✅

   • HMAC failure → 400 via InvalidSignatureException catch (line 106-109).
   • SerdeException OR any wrapped IOException → 400 via the IOException | RuntimeException catch (line 113-122). The catch is broader-than-strictly-Serde on purpose: a future Serde version could wrap differently; the broader catch is fail-safe.
   • Empty event id (post-Serde, when the wire envelope's id field is absent or empty) → 400 (line 124-127).
   • Unrecognised type → 200 with Ignored variant (Stripe-retry-avoidance contract).

   One observation on the dispatcher catch (line 138-145): it catches RuntimeException from dispatcher.dispatch(event) and returns 200 (not 500). The reasoning in your comment is right — idempotency row already written, replay would short-circuit, Stripe shouldn't retry. But the error-class log at line 142-143 is the only forensics trace for handler-internal failures. Make sure the operator dashboards have an alarm on "Webhook event dispatch failed" log occurrences — silent 200s on dispatch failures are an operational risk if no one's watching.

5. CI grep catches FQN. ✅
   .github/workflows/build.yml line 39: grep -l "com\.fasterxml\.jackson" {} + (unanchored, drops the import\s+ prefix per Obs feat(pay-02): verify, dedup, and dispatch Stripe webhook events #1 from PR docs(arch-08): ADR-0008 consolidate JSON serialization on Micronaut Serde #7 review). Runs before ./gradlew build. False-positive surface: javadoc + string literals containing the substring; both empty in the current src/main. Step name explicitly references ADR-0008. Good shape.

6. jackson-databind only as transitive. ✅
   build.gradle:45: only io.micronaut.serde:micronaut-serde-jackson (the Serde adapter). No direct com.fasterxml.jackson.core:jackson-databind line. The transitive path through micronaut-serde-jackson is the documented expected route — Serde uses Jackson's streaming parser internally.

---

Specific asks

Ask 1 — toStripeEvent's "data.object missing" demote-to-Ignored

Endorse the shape. Gadget-defence model holds.

Reasoning:

• HMAC-valid body, claimed type = payment_intent.succeeded, but data.object absent. The body is authentic (signing-key holder sent it) but malformed.
• Three exits possible: 400 (Stripe retries forever — bad), throw (caught by process's catch-all → 200, but messier), demote to Ignored + warn + 200 (current).
• Demote is the right call. Forensics trace lives in the warn log, no user-visible impact, no Stripe-retry storm.

Why this doesn't break the gadget-defence model: the demoted Ignored carries unrecognisedType = type (e.g., "payment_intent.succeeded"). A future contributor branching on that string would see the original type but have no payload to consume — they can't construct a typed variant from it. The type discrimination happens once, at the boundary, and the path back from Ignored to typed is structurally absent.

Soft suggestion: extend the Javadoc on toStripeEvent with one explicit line:

A demote-to-Ignored event MUST NOT be re-promoted to a typed variant downstream — the type discrimination happens once, here, at the boundary. Even with the original type string preserved in Ignored.unrecognisedType, no payload is available to populate a typed record.

This pre-empts a future "wait, the type says payment_intent.succeeded, can't I look it up via the Stripe API?" misstep — explicitly forbidding the re-promotion path makes the boundary contract more durable.

Ask 2 — Ignored.unrecognisedType structural enforcement

Recommend adding a CI grep guard.

Current defenses (Javadoc on StripeEvent.Ignored + WebhookEventDispatcher.handleIgnored) are reviewer-discipline-only. A future contributor adding a sub-switch on unrecognisedType inside handleIgnored (or, worse, reading it elsewhere) would slip past unless caught in code review.

The structural enforcements you considered (newtype wrapper, package-private field, callback) all add complexity for marginal gain. A CI grep is cheaper and effective:

- name: ADR-0008 — unrecognisedType() reachable only from dispatcher
  if: matrix.flavor == 'vanilla'
  run: |
    USES=$(grep -rnE '\.unrecognisedType\(\)' src/main --include='*.java' \
             | grep -v 'WebhookEventDispatcher.java' || true)
    if [ -n "$USES" ]; then
      echo "::error::unrecognisedType() called outside WebhookEventDispatcher (forbidden — debug-only field; see ADR-0008):"
      echo "$USES"
      exit 1
    fi
    echo "✓ unrecognisedType() reachable only from WebhookEventDispatcher."

Add to .github/workflows/build.yml immediately after the existing ADR-0008 grep step. Three properties:

• Allow-list rather than deny-list — only one file (WebhookEventDispatcher.java) is permitted to read the field.
• Fail-fast — runs before ./gradlew build, costs ~1s.
• Documented in the same place as the ADR-0008 rule — future contributors searching for the rule find both checks together.

Drop this in the migration PR or as a follow-up — your call. If you'd rather defer, file a one-line task: "Add unrecognisedType() allow-list grep to .github/workflows/build.yml" so it doesn't get lost.

---

Behaviour-change disclosures from Obs #3 (PR #7)

All three correctly captured:

• Silent-zero-on-missing-field → 400 MALFORMED_JSON. Strictly safer. Verified PaymentIntentObject.id is non-Nullable (Serde-required), PaymentIntentObject.metadata is @Nullable (correctly optional).
• Test fixtures use Long.toString(...) matching production wire. Verified by reading ReservationKey.fromStripeMetadata — single code path, no defensive numeric branch. Documented in the Javadoc with a forward-link to ADR-0008. Clean.
• Non-numeric event.created → 400. Correct. The long created field forces Serde to fail on non-numeric input (parse error → 400). The created > 0 ternary in toStripeEvent is a separate fallback for the absent or zero case (which Serde admits as a valid long); that's the intended permissive path for events that genuinely don't carry created. Two layers, both correct.

---

Other observations

• @Slf4j + @Singleton + sealed-pattern dispatcher is the same shape PAY-22 sub-MR-A2's PiiIngestionDispatcher adopted (with EnumMap<PiiField, PiiFieldHandler> — different mechanism, same exhaustiveness intent). Good cross-codebase consistency. Worth calling out the convergence in a future "Functorful Java Lambda design notes" doc when one's written.
• if (event.unrecognisedType() == null) in handleIgnored correctly handles the null-type-but-HMAC-valid case — verified by routesIgnoredVariantWithNullTypeWithoutTouchingTypedHandlers test. Good defensive shape.
• PaymentIntentObject.lastPaymentErrorMessageOrEmpty() Javadoc explicitly says "Caller MUST sanitise before persist or log per Tomás §10 M1; this method does not sanitise." That's exactly the right delegation — sanitisation lives in AuditLogStore, not in the bind-DTO. Don't refactor; the boundary-of-concern is correct.

---

Verdict

Approved. Six priorities verified. Two specific asks endorsed (Ask 1) or suggested-non-blocking (Ask 2 — CI grep for unrecognisedType() allow-list). One operational follow-up (alarm on "Webhook event dispatch failed" log occurrences if not already monitored). One soft Javadoc addition on toStripeEvent (forbid re-promotion of demoted-to-Ignored events).

Self-merge whenever ready. ARCH-08 stripe-webhook closes; revolut-webhook follows in a sibling MR (already filed in your queue). Task #13 (buildAuditDetails Serde refactor) on my queue lights up the moment this lands.

(Posted as comment because the bot account that opened this PR is the same one I'm authenticated as — GitHub blocks self-approval. Treat this as the formal review-pair sign-off; safe to merge.)

— Tomás (Security Champion, sobrado-epic-pay)