Skip to content

fix: mega audit - security, performance, error handling, type safety, dates - #84

Merged
thomasluizon merged 7 commits into
mainfrom
fix/mega-audit-2026-03-28
Mar 28, 2026
Merged

fix: mega audit - security, performance, error handling, type safety, dates#84
thomasluizon merged 7 commits into
mainfrom
fix/mega-audit-2026-03-28

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Mar 28, 2026

Copy link
Copy Markdown
Owner

Summary

Comprehensive audit fix addressing 206 findings from 30 parallel audit agents across both repos. This PR covers the orbit-api side (7 commits).

Security

  • Verification codes now use RandomNumberGenerator (crypto PRNG)
  • ConfigController gated with [Authorize]
  • Rate limiting on auth endpoints (5 req/min/IP)
  • Strict-Transport-Security + Content-Security-Policy headers
  • Chat message length capped at 4000 chars, body limit reduced to 10MB
  • ForwardedHeaders restricted with ForwardLimit = 1
  • Sanitized ex.Message exposure in bulk commands and notifications

Performance

  • Added IX_Habits_UserId index (critical - was missing after composite index drop)
  • GetHabitByIdQuery queries by ID directly (was loading ALL user habits)
  • GetHabitLogsQuery capped to 365-day lookback
  • Background schedulers batch-load instead of N+1 per-habit
  • QueryHabitsTool filters at DB level (was loading all into memory)
  • PushNotificationService uses IHttpClientFactory (was creating HttpClient per call)
  • All 9 HttpClient registrations have 30s timeout

Architecture

  • Fixed GamificationService.IsScheduledForDate diverged duplicate - now delegates to real HabitScheduleService
  • TODOs added for NotificationController MediatR migration and webhook extraction

Error Handling

  • Stripe webhook returns 500 on errors (enables retries, was returning 200)
  • Logging added to all bare catch {} blocks (gamification, referral, calendar)
  • New validators: ProcessUserChatCommandValidator, GoogleAuthCommandValidator
  • BFF timeout set to 30s

Type Safety

  • TryGetProperty pattern in Google auth (was throwing on missing keys)
  • AppConfigKeys static class (11 constants replacing 13 scattered strings)
  • .GetString() ?? string.Empty across AI tools
  • TryParseExact("yyyy-MM-dd") in 7 AI tool files
  • nameof() in EF Core join config
  • Stripe SDK constants for event types

Date/Timezone

  • Metrics calculators convert CreatedAtUtc to user timezone
  • Monthly habit anchor drift fixed (preserves original day)
  • Leap-day yearly habits fire on Feb 28 in non-leap years
  • Cache invalidation widened to +/-2 days

Subscription & Pay Gates

  • Goals CRUD now Pro-gated via IPayGateService.CanCreateGoals
  • Stripe product ID moved to config
  • DefaultFreeAiMessages comment fixed

Push Notifications & AI Chat

  • PushSubscription FK to Users with CASCADE delete
  • 5-subscription cap per user, 90-day cleanup for SentReminder/SentSlipAlert
  • Habit titles sanitized in AI system prompt (100 char truncation)
  • MoveHabitTool deep cycle detection
  • BulkLogHabitsTool filters by UserId at DB level

Env var changes

  • Add Stripe__ProProductId=prod_UBUPrTlZg8chuk to Render

Test plan

  • dotnet build passes (verified)
  • Run existing integration tests
  • Verify login flow (rate limiting active)
  • Verify habit CRUD + scheduling
  • Verify AI chat (message length validation)
  • Verify subscription webhook processing
  • Run EF migration on local DB

Companion PR: orbit-ui fix/mega-audit-2026-03-28

🤖 Generated with Claude Code

thomasluizon and others added 7 commits March 28, 2026 15:50
@thomasluizon @claude
fix: security hardening - crypto PRNG, auth rate limiting, headers, i…
eb9576b 
…nput validation

- Replace Random.Shared with RandomNumberGenerator.GetInt32 for verification codes (SendCodeCommand, RequestAccountDeletionCommand)
- Add [Authorize] to ConfigController
- Add Strict-Transport-Security and Content-Security-Policy headers to SecurityHeadersMiddleware
- Add fixed-window rate limiter (5 req/min/IP) for /api/auth/send-code and /api/auth/verify-code
- Set ForwardLimit=1 on ForwardedHeadersOptions; add note about KnownProxies per-deployment config
- Add note that IMemoryCache rate limiting is not shared across replicas
- Reduce ChatController multipart limit from 20MB to 10MB; add 4000-char message length guard
- Add MaximumLength(2000) validation for habit Description in Create/Update validators
- Add MaximumLength(500) validation for ChecklistItem.Text in Create/Update validators
- Replace ex.Message with generic error in BulkDeleteHabitsCommand, BulkLogHabitsCommand, and NotificationController.TestPush

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thomasluizon @claude
refactor: architecture cleanup - fix diverged schedule logic, add ref…
3520c66 
…actoring TODOs

- Replace diverged IsScheduledForDate in GamificationService with a direct
  delegate to HabitScheduleService.IsHabitDueOnDate (the single source of truth).
  The local copy was skipping modular arithmetic for Weekly/Monthly/Yearly habits.
- Add TODO to NotificationController noting it bypasses MediatR and injects
  OrbitDbContext directly -- needs migration to Query/Command handlers.
- Add TODOs to SubscriptionController.HandleWebhook for extracting event
  handling into dedicated MediatR commands (ActivateSubscription, Renew, Cancel).
- Add TODOs to SubscriptionController Infrastructure imports flagging that
  StripeSettings and IGeoLocationService interfaces should move to Application layer.
- Add TODO to GamificationService Portuguese translations dictionary noting
  hardcoded strings need a proper i18n/localization strategy.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thomasluizon @claude
fix: error handling - webhook retry, logging, validators, timeouts
59412ab 
- SubscriptionController: return 500 on business logic exceptions so Stripe retries
- CheckReferralCompletionCommand: log warning on push notification failure; wrap GrantCoupon in try-catch with fallback
- StripeCouponRewardService: log error before throwing InvalidOperationException for missing user
- CreateHabitCommand: log warning on gamification failure instead of bare catch
- CreateGoalCommand: log warning on gamification failure instead of bare catch
- UpdateGoalStatusCommand: log warning on gamification failure instead of bare catch
- GetCalendarEventsQuery: log warning on RRULE fetch failure instead of bare catch
- GoalsController.GetGoals: wrap in try-catch returning 500 on unexpected errors
- Add ProcessUserChatCommandValidator (message not empty, max 4000 chars)
- Add GoogleAuthCommandValidator (access token not empty)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thomasluizon @claude
fix: type safety - TryGetProperty, null guards, magic strings, parse …
fcc755f 
…formats

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thomasluizon @claude
fix: date/timezone - UTC conversion in metrics, monthly anchor drift,…
e9d8b57 
… leap-day fallback

- HabitMetricsCalculator: use user local timezone when converting CreatedAtUtc to start date
- GetHabitMetricsQuery + GamificationService: pass userTimeZone to Calculate()
- CacheInvalidationHelper: widen summary cache invalidation buffer from +/-1 to +/-2 days
- GeminiRoutineAnalysisService: add comment noting UTC cutoff for 60-day analysis window
- Habit.cs: add comment on UTC fallback in Create() EndDate validation guard
- Habit.AdvanceDueDate/CatchUpDueDate: preserve original day-of-month to prevent monthly drift
- HabitScheduleService: IsMonthlyMatch clamps to last day of month; IsYearlyMatch adds Feb 29 -> Feb 28 fallback for non-leap years

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thomasluizon @claude
fix: subscription gates - past_due handling, goals pro gate, config c…
3105e2f 
…leanup

- Add CanCreateGoals paygate to IPayGateService + PayGateService (GoalsProOnly config key, defaults to Pro-only)
- Gate CreateGoalCommand behind CanCreateGoals paygate check
- Fix IPayGateService comment: free AI messages is 20/month not 50
- Move hardcoded Stripe product ID out of StripeCouponRewardService into StripeSettings.ProProductId

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thomasluizon @claude
fix: push notifications FK/cleanup, AI chat security/safety improvements
0d1ae70 
Push Notification fixes:
- 164: Add FK from PushSubscriptions.UserId to Users.Id with CASCADE delete
- 165: Cap push subscriptions at 5 per user, removing oldest on subscribe
- 166: Daily cleanup of SentReminder and SentSlipAlert rows older than 90 days (via AccountDeletionService)
- 167: Document URL dedup hack in GoalDeadlineNotificationService with TODO for proper entity
- 168: Guard against null FirebaseMessaging.DefaultInstance before sending FCM push
- 169: Raise MinBucketCountForTimePeak from 2 to 3 in SlipPatternDetectionService
- 170: Rename MinOccurrencesPerDay to MinTotalLogs in SlipPatternDetectionService
- 171: Add SenderIdMismatch to FCM stale token removal in PushNotificationService

AI Chat fixes:
- 188: Sanitize habit titles in system prompt (strip control chars, truncate to 100 chars)
- 189: Warn in AssignTagsTool description that it replaces all existing tags
- 190: Add recursive parent-chain cycle detection in MoveHabitTool
- 191: Add UserId ownership filter to BulkLogHabitsTool habit query

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thomasluizon
thomasluizon merged commit cfd1533 into main Mar 28, 2026
2 checks passed
@thomasluizon
thomasluizon deleted the fix/mega-audit-2026-03-28 branch March 28, 2026 20:12
thomasluizon added a commit that referenced this pull request Jun 4, 2026
@thomasluizon @claude
feat(api): author feature-explanation markdown bundle (#84)
c356f2a 
Add eight grounded markdown explainer files (streaks, frequencies,
gamification, paygate, schedule-math, freezes, notifications,
ai-memory) so the in-app AI answers feature questions from
code-accurate content. Each file carries a 6-key YAML frontmatter
and a body whose constants are copied verbatim from AppConstants,
LevelDefinitions, and the streak/schedule/paygate/reminder services.

Ship the bundle inside the API binary via the first EmbeddedResource
glob in the repo; resources embed as
Orbit.Application.Chat.Content.FeatureExplanations.<key>.md, the
documented loader contract for the downstream consumer.

Refs thomasluizon/orbit-ui-mobile#84

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thomasluizon thomasluizon mentioned this pull request Jun 4, 2026
Merged
thomasluizon added a commit that referenced this pull request Jun 4, 2026
@thomasluizon @claude
feat(api): author feature-explanation markdown bundle (#84) (#172)
8b6e98f 
* feat(api): auto-activate streak freeze on inactive day (#108)

Add a dedicated StreakFreezeAutoActivationService BackgroundService that
auto-activates a streak freeze for a Pro user who held an active streak but
logged nothing on their fully-elapsed local "yesterday".

- Mirrors SlipAlertSchedulerService / HabitDueDateAdvancementService: poll
  interval, conservative UTC pre-filter, authoritative per-user TimeZoneHelper
  local-yesterday guard, single SaveChanges per tick.
- Pro-only (matches ActivateStreakFreezeCommand). Spends one freeze per missed
  day, bounded by MaxStreakFreezesAccumulated (inventory) and
  MaxStreakFreezesPerMonth (monthly).
- Presence-based: inserting a StreakFreeze row for the missed date preserves
  the streak on the next on-read RecalculateAsync; no direct streak mutation.
- Idempotent: new SentStreakFreezeAlert guard entity (unique UserId+FrozenDate)
  plus the existing StreakFreeze unique index, both re-checked before spending.
- Notifies via in-app Notification + push (IPushNotificationService); localized
  copy via LocaleHelper, mirroring GoalDeadlineNotificationService.
- EF migration AddSentStreakFreezeAlert; account reset purges the guard table;
  registered in ServiceCollectionExtensions and BackgroundServiceHealthCheck.

Unit tests cover the eligibility predicate, local-yesterday computation,
interval default, notification copy, and the guard entity.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(api): remove manual streak-freeze activation (auto-only) (#108)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(api): exclude soft-deleted habits from auto-freeze completion check (#108)

Aligns StreakFreezeAutoActivationService.LoadRecentCompletionsAsync with
UserStreakService.LoadStreakDataAsync by filtering out soft-deleted habits when
computing recent completions, so a deleted habit's log can no longer count as
activity for a date and suppress the auto-freeze. Adds DB-backed tests locking
the LoadRecentCompletionsAsync contract for soft-deleted and live habit logs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(api): author feature-explanation markdown bundle (#84)

Add eight grounded markdown explainer files (streaks, frequencies,
gamification, paygate, schedule-math, freezes, notifications,
ai-memory) so the in-app AI answers feature questions from
code-accurate content. Each file carries a 6-key YAML frontmatter
and a body whose constants are copied verbatim from AppConstants,
LevelDefinitions, and the streak/schedule/paygate/reminder services.

Ship the bundle inside the API binary via the first EmbeddedResource
glob in the repo; resources embed as
Orbit.Application.Chat.Content.FeatureExplanations.<key>.md, the
documented loader contract for the downstream consumer.

Refs thomasluizon/orbit-ui-mobile#84

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(chat): freezes.md describes auto-activation model (#108)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(chat): repoint freezes.md derived_from to auto-activation service (#108)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@thomasluizon