test(subscriptions): close Stripe webhook + checkout edge/failure gaps

[thomasluizon]

Summary

Closes the /prod-readiness tests-audit gaps around the Stripe webhook and checkout-session handlers with intelligent behavior/edge/failure tests, plus one minimal source fix. Every test asserts resulting state / Result, not just that a mock was invoked, so each can fail if the behavior breaks.

Source change (minimal, required to be correct)

HandleWebhookCommand — reject malformed/empty webhook bodies gracefully. Verified empirically: EventUtility.ConstructEvent (Stripe.net 52.1.0) throws System.Text.Json.JsonException (a JsonReaderException) — not a StripeException — on unparseable/empty payloads. Those previously escaped the handler unhandled → HTTP 500 → Stripe retries a body that can never parse. Now caught at the trust boundary and returned as an InvalidWebhookSignature failure, mirroring the existing signature-failure path.

Payment-failure decision (no source handler — intentional safe default)

invoice.payment_failed is intentionally not given a downgrade handler. Immediately downgrading on a transient card decline would wrongly kick a paying user off Pro; the correct Stripe pattern is to let dunning run and downgrade via the eventual customer.subscription.updated (unpaid/canceled) or customer.subscription.deleted events, which are already handled. A test now locks this safe default (a Pro user is not downgraded, no SaveChanges).

New tests

HandleWebhookCommandHandlerTests

• customer.subscription.created → safe no-op (setup is owned by checkout.session.completed; repo not queried, no save)
• invoice.payment_failed → does not downgrade a Pro user (dunning owns the lifecycle)
• Stripe API error during subscription fetch → WebhookStripeApiError failure, no save
• OperationCanceledException during fetch → propagates, not swallowed as a generic failure
• malformed body / empty body → graceful failure (exercises the source fix)
• customer.subscription.updated with a non-subscription data object → safe no-op (null-cast guard)
• checkout with a subscription that has no items → GetPeriodEnd falls back to Monthly + 1 month
• checkout with an unusual interval (week) → defaults to Monthly while keeping the item's period end
• replayed event with a stale signature timestamp → rejected by ConstructEvent tolerance

CreateCheckoutCommandHandlerTests

• Stripe customer-creation failure → graceful failure, no partial state persisted
• HTTP 500 / network timeout / malformed response during checkout (theory) → all fail gracefully with the correct Result (no unhandled throw)

Verification

• dotnet build clean; dotnet test → 2749 passed, 0 failed.
• No bare // comments (ORBIT0001).

Refs thomasluizon/orbit-ui-mobile#243

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Review Complete

Scope: PR #367 in thomasluizon/orbit-api — test(subscriptions): close Stripe webhook + checkout edge/failure gaps
Recommendation: APPROVE

Severity	Count
Critical (incl. ⚠️ old-client breaks)	0
High	0
Medium	0
Low / Info	1

---

Summary

This PR closes test gaps around the Stripe webhook and checkout-session handlers and ships one minimal, correctly-scoped source fix: HandleWebhookCommand now catches System.Text.Json.JsonException (thrown by EventUtility.ConstructEvent on malformed/empty payloads, per Stripe.net 52.1.0 behavior) alongside the existing StripeException catch, returning the same InvalidWebhookSignature failure instead of letting the exception escape as an unhandled 500. The remaining ~269 added lines are test code (HandleWebhookCommandHandlerTests.cs, CreateCheckoutCommandHandlerTests.cs) that assert on resulting state/Result, not just mock invocation. Verified: the new catch block's scoping (tightly wraps only EventUtility.ConstructEvent, cannot mask later JSON errors), the reused LogWebhookSignatureVerificationFailed log call, the BillingProviderException constructor used by the new checkout tests, ErrorMessages strings against test assertions, and the invoice.payment_failed no-op path (confirmed genuinely absent from the handler's switch, so the "safe default" test asserts real behavior, not a tautology).

Findings

Critical

None.

High

None.

Medium

None.

Low / Info

• [Info] The new catch (System.Text.Json.JsonException ex) block is scoped tightly around only the EventUtility.ConstructEvent call (src/Orbit.Application/Subscriptions/Commands/HandleWebhookCommand.cs:38-56), so it cannot mask an unrelated JSON error later in event processing. Good scoping, no fix needed.

Subagents

Agent	Verdict
security-reviewer	PASS (no findings) — reviewed HandleWebhookCommand.cs; signature verification (EventUtility.ConstructEvent) is unchanged and still runs before any state mutation, the new catch only reclassifies an exception type into the existing rejection path, no new attack surface
contract-aligner	N/A — diff touches no DTO, Controller route, or packages/shared type; no cross-repo contract surface changed

Validation

Check	Result
Build (dotnet)	N/A in this pass — CI's Build required check covers it
Tests (dotnet)	N/A in this pass — CI's Unit Tests required check covers it; PR body reports dotnet test → 2749 passed, 0 failed locally

Deferred — N/A dimensions & files not verdicted

• Dimension 8 (DESIGN.md/AI-slop) — N/A, no apps/* files in diff.
• Dimension 9 (Parity web↔mobile) — N/A, backend-only diff.
• Dimension 10 (i18n) — N/A, backend-only diff.
• Dimension 11 (Contract drift + backward-compat) — N/A, no DTO/route/shared-type change.
• Dimension 14 (FEATURES.md parity) — N/A, no user-facing feature surface changed (internal robustness fix + tests only).
• All three changed files (HandleWebhookCommand.cs, HandleWebhookCommandHandlerTests.cs, CreateCheckoutCommandHandlerTests.cs) were read in full diff context and given a verdict.
• Phase 6 adversarial pass / /second-opinion: no Critical/High finding survived to require it — that step is scoped to Critical/High only, and none exist here.

What's good

• Genuine root-cause fix (catches the exception type Stripe.net actually throws), not a defensive band-aid, and empirically verified per the PR description.
• New tests assert on resulting Result/state (user.IsPro, SaveChangesAsync not called, specific error substrings) rather than just verifying a mock was invoked.
• The invoice.payment_failed "no downgrade" test locks a deliberate, well-reasoned safe default (avoid downgrading a paying user on a transient decline; let dunning + customer.subscription.updated/deleted own the lifecycle).
• Good edge-case coverage: no-items subscriptions, unusual billing intervals, stale-signature-timestamp replay rejection, OperationCanceledException propagation (not swallowed), Stripe API errors during subscription fetch.
• No bare // narration comments introduced (verified via grep) — clean under ORBIT0001.

Recommendation

Approve as-is. No Critical/High findings; the diff is test-focused with one minimal, well-scoped, empirically-verified source fix.