fix(api): bound unbounded queries + add Stripe transient-retry (prod-readiness)

[thomasluizon]

Bounds the last unbounded query-expansion path and adds transient-error resilience to the Stripe billing calls, closing the still-open prod-readiness Medium findings. All changes are internal upper bounds / retry wrappers — no DTO shape changes, openapi.json unchanged, fully backward-compatible with old mobile clients.

Findings addressed

2.152 — HabitSchedule Instances array materialized a full 366-day range

HabitScheduleService.GetInstances now caps its forward horizon to a new named constant AppConstants.MaxInstanceHorizonDays = 90 (clamps dateTo to dateFrom + 90 before expanding). This is the single point both callers flow through (HabitScheduleFilters for the schedule query and GetCalendarMonthQuery), so both are bounded by one change.

Verified no real caller depends on a larger expansion:

• calendar-month endpoint is validator-capped to MaxCalendarRangeDays = 62 (≤ 90) — unaffected.
• schedule/interval endpoint's frontend clamps to MAX_RANGE_DAYS = 14 (≤ 90) — unaffected.
• the list_habits MCP tool can request up to MaxRangeDays = 366 but does not read Instances.
• only an abusive request spanning > 90 days is now bounded (worst case ~366 → ~90 instances/habit).

The Instances list field on HabitScheduleItem/HabitScheduleChildItem keeps the same shape and type; only the worst-case element count shrinks.

2.150 — StripeBillingService had no retry on transient errors

Added Orbit.Infrastructure.Common.StripeRetryPolicy (mirrors the existing HttpRetryPolicy pattern): a bounded exponential-backoff retry (2 retries, 200ms base) that retries only transient failures — connection/network errors and timeouts (which the Stripe SDK surfaces as raw HttpRequestException / OperationCanceledException) and 429 / 5xx responses (surfaced as StripeException carrying the status). Business errors (card declined 402, invalid request 400, auth 401, etc.) and user cancellation are never retried and surface on the first attempt. Every Stripe SDK call in StripeBillingService (create customer, create checkout/portal session, get subscription, list invoices, get price, get coupon) is now wrapped; the existing catch (StripeException) → BillingProviderException wrapping is preserved.

Already remediated (verified against current main)

The three query-cap findings from the report were already closed by prior PRs and re-verified here (server-side .Take(<named constant>) + unit tests already present) — no further change needed:

• 2.155 GetHabitLogsQuery — IHabitLogReader.ReadRecentLogsAsync caps at MaxHabitLogsReturned = 1000, newest-first (PR perf(api): bound habit-logs read with server-side ordering + row cap #347, HabitLogReaderTests).
• 2.156 GetFriendsQuery — ISocialGraphReader.ReadVisibleFriendshipsAsync caps at MaxFriends = 500 (PR perf(api): push social query filtering + pagination into the DB #344, SocialGraphReaderTests.VisibleFriendships_CapAtLimit).
• 2.157 GetCheersQuery — ISocialGraphReader.ReadCheersPageAsync caps at MaxCheersReturned = 200, newest-first (PR perf(api): push social query filtering + pagination into the DB #344, SocialGraphReaderTests.VisibleCheers_OrderNewestFirst_AndCapAtLimit).

Tests

• HabitScheduleServiceTests.GetInstances_RangeExceedingHorizon_CapsForwardWindow — a 200-day request returns exactly MaxInstanceHorizonDays + 1 instances, none past the horizon.
• HabitScheduleServiceTests.GetInstances_RangeWithinHorizon_ReturnsEveryScheduledDate — a 30-day request is unaffected (all 31 dates returned).
• StripeRetryPolicyTests — transient StripeException (503) and network HttpRequestException retry then succeed; card-declined (402) does not retry (1 attempt); persistent transient exhausts retries then throws; IsTransient status classification.

Verification

• dotnet build Orbit.slnx — 0 errors.
• dotnet test Orbit.slnx — all green.
• dotnet ef migrations has-pending-model-changes — false ("No changes have been made to the model since the last migration").
• src/Orbit.Api/openapi.json — unchanged (no contract/DTO change).

Refs thomasluizon/orbit-ui-mobile#243 (prod-readiness remediation)

🤖 Generated with Claude Code

[claude]

Code Review: PR #403 (fix(api): bound unbounded queries + add Stripe transient-retry)

Recommendation: APPROVE

Summary
Small, well-scoped prod-readiness PR: caps HabitScheduleService.GetInstances forward horizon at a new AppConstants.MaxInstanceHorizonDays = 90 constant, and adds a bounded exponential-backoff StripeRetryPolicy wrapping every Stripe SDK call in StripeBillingService for transient (429/5xx/network/timeout) failures only. No DTO, endpoint, or contract-surface changes -- confirmed by diff inspection (only AppConstants.cs, HabitScheduleService.cs, the new StripeRetryPolicy.cs, StripeBillingService.cs, and two test files changed). Both changes are covered by new unit tests exercising the boundary conditions. No Critical or High findings.

Findings

Critical: None.

High: None.

Medium

[MEDIUM] Stripe Checkout/Portal session creation has no idempotency key

• dimension: Security (#12) -- defense-in-depth
• location: src/Orbit.Infrastructure/Services/StripeBillingService.cs:78, :93
• issue: CreateCheckoutSessionAsync and CreatePortalSessionAsync pass no RequestOptions.IdempotencyKey to Stripe, unlike CreateCustomerAsync (line 38) which does.
• risk: A client-side timeout followed by a caller-level retry (now more likely given the new retry wrapper) can create a duplicate Session object server-side. Low real-world impact -- Sessions are single-use, pre-charge redirect containers with no route exposing the orphaned duplicate to the user -- but it is inconsistent with the idempotency pattern already established for customer creation.
• fix: Pass a deterministic RequestOptions.IdempotencyKey (e.g. a key derived from userId + priceId) to CheckoutSessions.CreateAsync and PortalSessions.CreateAsync, consistent with CreateCustomerAsync.
• reference: orbit-api Stripe idempotency pattern, OWASP defense-in-depth

[MEDIUM] StripeRetryPolicy retries 429 without honoring Retry-After

• dimension: Security / Reliability (#12)
• location: src/Orbit.Infrastructure/Common/StripeRetryPolicy.cs:44-48
• issue: On a 429 (TooManyRequests) StripeException, the policy retries on a fixed exponential backoff (200ms/400ms) rather than reading the Retry-After header Stripe may return.
• risk: During a real rate-limit window this can retry sooner than Stripe wants, extending the throttle rather than backing off correctly. Bounded impact given only 2 retries total.
• fix: Read Retry-After from the StripeException/response headers when present and use it as the delay, falling back to the current exponential backoff otherwise.
• reference: Stripe API rate-limit guidance

Low / Info

• StripeBillingService.cs:187 -- a Stripe price ID is interpolated into a BillingProviderException message; confirmed this only surfaces server-side (never returned to the client -- CreateCheckoutCommand.cs maps it to a generic PaymentServiceUnavailable message first), so no data-exposure issue today.
• The horizon cap in HabitScheduleService.GetInstances is placed after the IsCompleted/IsFlexible early return and before any list allocation, so it never does unnecessary work. Clean placement.

Subagents

• security-reviewer: PASS (2 Medium, no Critical/High)
• contract-aligner: N/A -- no DTO, Controller route, or packages/shared type changed in this diff

Validation

• Build (dotnet): PASS (separate required CI check)
• Tests (dotnet): PASS (separate required CI check)
• SonarCloud Quality Gate: FAILURE -- 78.4% coverage on new code vs required >= 80% (informational; not a rubric Critical/High, does not change this recommendation, but this required check is currently red and will independently gate merge)

Deferred -- N/A dimensions and files not verdicted

• Dimensions 8 (DESIGN.md/AI-slop), 9 (Parity), 10 (i18n): N/A, diff touches only orbit-api src/tests, no apps/* UI files.
• Dimension 11 (Contract drift + backward-compat guard): N/A, no DTO/schema/endpoint changed; PR body confirms openapi.json unchanged, verified against the file list.
• Dimension 14 (FEATURES.md parity): N/A, purely internal hardening (query bound + retry policy), no user-facing feature/gating/platform change.
• Every changed file (AppConstants.cs, HabitScheduleService.cs, StripeRetryPolicy.cs, StripeBillingService.cs, HabitScheduleServiceTests.cs, StripeRetryPolicyTests.cs) received a verdict above -- nothing else deferred.

What is good

• The horizon cap is a single choke point (GetInstances) that both real callers (HabitScheduleFilters, GetCalendarMonthQuery) flow through, verified by grep -- no duplicated bound logic.
• StripeRetryPolicy cleanly separates transient (retry) from business (never retry) failures, mirrors the existing HttpRetryPolicy convention, and is tested for both the retry path and the non-retry path (card decline) plus exhaustion.
• Tests target the actual boundary conditions (exactly-90-days cap, within-horizon no-op, retry-then-succeed, exhaust-then-throw, non-transient no-retry) rather than just happy path.
• XML-doc comments added are compliant triple-slash summaries, not narration.

Recommendation
Approve as-is. The two Medium findings (idempotency key on Checkout/Portal sessions, Retry-After honoring) are defense-in-depth improvements worth a fast-follow, not blockers. Separately, note the SonarCloud coverage gate is currently failing (78.4% < 80%) -- a required check outside this rubric scope but will need attention before merge regardless of this review outcome.

[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]

Code Review: PR #403 (fix(api): bound unbounded queries + add Stripe transient-retry)

Recommendation: REQUEST CHANGES

Summary

This PR bounds HabitScheduleService.GetInstances' forward horizon to AppConstants.MaxInstanceHorizonDays = 90 and adds a bounded, transient-only StripeRetryPolicy around every Stripe SDK call in StripeBillingService. Both changes are individually well-tested. However, the GetInstances cap introduces a real data-consistency gap on the same response DTO that the diff does not close, and a Medium finding on Stripe idempotency remains open.

Findings

[HIGH] Instances is now capped at 90 days but the sibling ScheduledDates field on the same DTO -- and the validator gating both -- is not, so a legal request silently returns inconsistent data

• dimension: Correctness / Validation
• location: src/Orbit.Application/Habits/Services/HabitScheduleService.cs:418-419
• issue: GetInstances clamps dateTo to dateFrom + 90 days before building the Instances list. But HabitScheduleItem/HabitScheduleChildItem (src/Orbit.Application/Habits/Queries/GetHabitScheduleQuery.cs:34,48 and :67,77) carry a sibling ScheduledDates field computed via the uncapped HabitScheduleService.GetScheduledDates (called with the same ctx.DateFrom/ctx.DateTo at HabitScheduleFilters.cs:77,211). GetHabitScheduleQueryValidator.cs:26-29 still permits dateTo - dateFrom up to AppConstants.MaxRangeDays = 366 for this endpoint -- the validator was not touched by this PR.
• risk: For any validator-legal request with a range between 91 and 366 days, the response silently contains a full-range ScheduledDates array but an Instances array truncated at day 90, with no error or truncation indicator. The PR body's own safety argument for this cap rests entirely on frontend clamping ("frontend clamps to MAX_RANGE_DAYS = 14"), which the project's own hard rule explicitly disallows as a substitute for backend validation (Orbit.Application/CLAUDE.md: "Frontend validation does NOT count"). This is the same failure mode the PR is nominally fixing (backend-unbounded query), just moved one field over.
• fix: Either (a) tighten GetHabitScheduleQueryValidator to cap the range at MaxInstanceHorizonDays for this endpoint, (b) apply the same 90-day clamp inside GetScheduledDates/HabitScheduleFilters so both fields are always consistent, or (c) return a clamped effective range so the two fields can't diverge. Add a regression test at the HabitScheduleFilters/handler level (the new tests only cover GetInstances in isolation) asserting ScheduledDates and Instances coverage stay consistent for a >90-day request.
• reference: Orbit.Application/CLAUDE.md Validation section; orbit-api hard rule "Validation... The backend is the source of truth."

[MEDIUM] Stripe Checkout/Portal session creation still has no idempotency key (open from prior review)

• dimension: Security -- defense-in-depth
• location: src/Orbit.Infrastructure/Services/StripeBillingService.cs:78-80 (CreateCheckoutSessionAsync), :93-99 (CreatePortalSessionAsync)
• issue: Both calls are now wrapped in the new retry policy, which makes duplicate-object creation on transient-failure-then-retry more likely, but neither passes RequestOptions.IdempotencyKey -- unlike CreateCustomerAsync:38 in the same file.
• risk: Bounded (Sessions are single-use, no route exposes the duplicate), but avoidable and inconsistent with the pattern already established for customer creation. Flagged in this PR's prior review pass and still unaddressed.
• fix: Pass a deterministic IdempotencyKey (e.g. derived from userId + priceId) to CheckoutSessions.CreateAsync and PortalSessions.CreateAsync, mirroring CreateCustomerAsync.

What's good

StripeRetryPolicy correctly isolates transient failures (network/timeout/429/5xx) from business errors (card decline, invalid request, user cancellation) with solid boundary-condition test coverage. The GetInstances horizon cap is placed efficiently (before any list allocation) and is well-tested in isolation. No DTO/route/contract shape changed -- confirmed backward-compatible with existing clients.

Deferred / N/A

• Dimensions 8-10 (DESIGN.md/AI-slop, parity, i18n): N/A, backend-only diff, no apps/* files touched.
• Contract-aligner: N/A, no DTO/endpoint/shared-type change.
• dotnet build/test: skipped -- CI runs Build/Unit Tests/SonarCloud as separate required checks.

Not verifiable in CI

• Cross-repo consumer impact (orbit-ui-mobile's actual request ranges for the schedule endpoint) -- the sibling repo is not checked out in this job; the High finding above is a backend contract gap independent of what the current mobile client happens to send today.

[claude]

[HIGH] Correctness / Validation — this clamp bounds Instances to 90 days, but the sibling ScheduledDates field on the same HabitScheduleItem/HabitScheduleChildItem DTO is computed via the uncapped GetScheduledDates, and GetHabitScheduleQueryValidator still allows ranges up to AppConstants.MaxRangeDays = 366 for this endpoint (untouched by this PR). A validator-legal request spanning 91–366 days will silently return a full-range ScheduledDates array alongside an Instances array truncated at day 90, with no error or truncation flag.

The PR's stated safety justification for this cap relies on frontend clamping (MAX_RANGE_DAYS = 14), which Orbit.Application/CLAUDE.md explicitly rules out as a substitute for backend validation.

Fix: either cap the validator's range for this endpoint to MaxInstanceHorizonDays, apply the same clamp inside GetScheduledDates/HabitScheduleFilters so both fields stay consistent, or surface a clamped effective range. See full review for details.