Audit/onboarding supabase security#27
Conversation
The live goals table predated migration tracking and enforced
('fat_loss','maintenance','muscle_gain') while the app has always sent
'lean_bulk' — so every Lean Bulk onboarding save and goal edit failed
with SQLSTATE 23514, deterministically, on every retry. 10 of 54
production users were stuck with a profile but no goal; none recovered.
The corrected constraint existed in 20260401000014 but its CREATE TABLE
IF NOT EXISTS silently no-opped against the pre-existing live table.
This migration drops and re-adds the constraint explicitly (applied to
production 2026-06-11; file version matches the recorded remote
migration) and adds a pgTAP shape test so CI fails if the constraint
ever drifts from the app's GoalType raw values again.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extracts the onboarding error classification into a shared, unit-tested SaveErrorClassification used by the results step, EditGoalView, and EditProfileView. Deterministic server rejects (23502/23503/23514/42501/ PGRST204) now show a "server problem — update the app or contact support" message instead of an infinite "Please try again" loop, which is what Lean Bulk users saw for two months during the goal_type constraint incident. Adds SentryMonitoring.captureNonFatal and reports the critical catch sites (onboarding save, goal/profile edits, delete-account, user-data fetch) with code-only tags — Sentry showed zero events while 10 users were failing onboarding repeatedly. Tags carry enum/code values only, never PII; offline URLErrors are excluded from fetch capture to avoid noise. Includes a GoalType/Pace raw-value contract test mirroring the DB constraint as an app-side drift tripwire. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
handle(.initialSession/.signedIn) cleared guest mode before the unbounded fetchUserData await and never re-checked it, while the 10s cold-start timeout could surface an interactive AuthView mid- resolution. A user tapping "Continue as Guest" in that window landed authenticated with guestStore still active — every subsequent log silently wrote to UserDefaults instead of Supabase. Re-check guestStore.isActive after the await and cancel the timeout as soon as an auth event is being processed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both quick-log catch blocks were intentionally empty, so a failed insert (flaky network, expired session) dropped the food log with zero feedback — users believed the entry was logged and daily totals were silently wrong. Failures now show a "Couldn't log food" alert (mirroring DashboardView's showDeleteError pattern) and report to Sentry with code-only tags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- supabase/README.md: new live-vs-migration drift verification section (migration list --linked, db diff, CHECK/FK constraint dumps) — the missing procedure that let the goal_type drift ship; adds the three untracked migrations rows, water_entries schema, and a warning on hand-applying SQL in the dashboard - docs/onboarding-save-flow.md: two-step save sequence, partial-failure matrix, error classification, observability, and the 2026-06 incident - docs/release-checklist.md: pre-flight drift check, version bump, smoke-test list (incl. lean-bulk onboarding), post-release monitoring - docs/debugging-supabase-errors.md: error-code → cause → next-step map for 23514/42501/PGRST*/401/403 - README.md: status section updated from "planning phase" to the shipped reality; setup steps now document the real Secrets.xcconfig template flow and the intentional fatalError on missing config Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dd899fb20
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| update public.goals | ||
| set goal_type = 'lean_bulk' | ||
| where goal_type = 'muscle_gain'; |
There was a problem hiding this comment.
In environments that actually contain legacy goal_type = 'muscle_gain' rows, this UPDATE runs while the old goals_goal_type_check is still in force, so changing those rows to lean_bulk violates the existing check and aborts the migration before the constraint is dropped. The comment says this is meant to be defensive/idempotent for environments with such rows; drop the old constraint first, then normalize the data before adding the new check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR hardens and documents the onboarding “final save” path and related Supabase interactions by (1) fixing/guarding the goals.goal_type contract that caused deterministic onboarding failures, and (2) centralizing Supabase error classification + adding non-fatal telemetry (Sentry) across key save surfaces.
Changes:
- Adds a Supabase migration + pgTAP regression test to enforce the app↔DB
goal_typeallowed-value contract (lean_bulkfix). - Introduces shared
SaveErrorClassification(with unit tests) and wires Sentry non-fatal capture into onboarding, edit flows, quick-log, and fetch/delete error paths. - Expands operational docs: schema drift checks, onboarding save behavior, release checklist, and Supabase error-code triage.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/tests/database/goals_constraint_shape.test.sql | New pgTAP test to assert goals_goal_type_check shape/values (incident regression guard). |
| supabase/README.md | Adds warnings about dashboard-applied SQL, drift-check procedure, and documents water_entries + new tests. |
| supabase/migrations/20260611054748_fix_goals_goal_type_check.sql | Migration to repair production goals_goal_type_check to accept lean_bulk. |
| README.md | Updates local setup instructions (secrets first) and refreshes project status/operational doc pointers. |
| docs/release-checklist.md | Adds operational release checklist including Supabase drift checks and smoke tests. |
| docs/onboarding-save-flow.md | Documents onboarding save sequence, partial-failure behavior, classification, and observability. |
| docs/debugging-supabase-errors.md | Adds a code→cause→next-step map for Supabase/PostgREST/Auth error codes. |
| AkFit/Services/Supabase/SaveErrorClassification.swift | New shared save-error classification + user messaging + log-field helpers. |
| AkFit/Services/SentryMonitoring.swift | Adds captureNonFatal helper for handled errors with strict non-PII tagging. |
| AkFit/AkFitTests/SaveErrorClassificationTests.swift | New unit tests for classification + app↔DB GoalType contract tripwire. |
| AkFit/AkFit/Views/Settings/EditProfileView.swift | Routes save failures through classification + Sentry non-fatal capture. |
| AkFit/AkFit/Views/Settings/EditGoalView.swift | Routes save failures through classification + Sentry non-fatal capture. |
| AkFit/AkFit/Views/Search/SearchView.swift | Adds user-visible alert for quick-log failures + Sentry capture. |
| AkFit/AkFit/Views/Onboarding/OnboardingView.swift | Centralizes error classification and adds Sentry non-fatal telemetry for onboarding save failures. |
| AkFit/AkFit/Auth/AuthManager.swift | Prevents loading-timeout flash during auth resolution; adds Sentry capture for fetch/delete failures and tightens guest/auth transition safety. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| update public.goals | ||
| set goal_type = 'lean_bulk' | ||
| where goal_type = 'muscle_gain'; | ||
|
|
||
| alter table public.goals | ||
| drop constraint if exists goals_goal_type_check; | ||
|
|
||
| alter table public.goals | ||
| add constraint goals_goal_type_check | ||
| check (goal_type in ('fat_loss', 'maintenance', 'lean_bulk')); |
| nonisolated enum SaveErrorClassification { | ||
|
|
||
| // MARK: - Outcome buckets | ||
|
|
||
| nonisolated enum Kind { |
| "postgrest_code": SaveErrorClassification.postgrestCode(of: error), | ||
| ] | ||
| ) | ||
| showQuickLogError = true |
| "postgrest_code": SaveErrorClassification.postgrestCode(of: error), | ||
| ] | ||
| ) | ||
| showQuickLogError = true |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Updates to Preview Branch (audit/onboarding-supabase-security) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfbff9163d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tags: [String: String] = [:] | ||
| ) { | ||
| guard !AppConfig.sentryDSN.isEmpty else { return } | ||
| SentrySDK.capture(error: error) { scope in |
There was a problem hiding this comment.
When this helper is called from the new onboarding/edit save catch paths with a PostgrestError from a constraint or validation failure, capture(error:) sends the raw error object rather than only the sanitized tags. Postgres/PostgREST error details can include failing row values such as profile fields, user IDs, or macro targets, so this new production telemetry can leak user data despite the privacy rule above; capture a scrubbed error/message built from the classification and error code instead.
Useful? React with 👍 / 👎.
This PR audits and hardens the AkFit onboarding save flow, focusing on Supabase session validation, profile/goal writes, RLS compatibility, and clearer user-facing error handling.
Includes:
Greptile Summary
This PR audits and hardens the AkFit onboarding/profile save flow following a production incident where users choosing "Lean Bulk" were blocked from completing onboarding (SQLSTATE 23514,
goals_goal_type_checkrejectinglean_bulk). The fix ships a corrected migration, centralizes error classification, improves user-facing messages, and adds Sentry observability across all save surfaces.20260611054748_fix_goals_goal_type_check.sqldrops and re-adds thegoals_goal_type_checkconstraint with the correct value set (fat_loss,maintenance,lean_bulk), and a pgTAP test + Swift contract test guard against future drift.SaveErrorClassificationis extracted fromOnboardingViewand shared withEditGoalView,EditProfileView, andSearchView— replacing divergent inline logic with a single tested rule set that correctly distinguishesnonRetryable(check/RLS/FK violations) fromretryableerrors, preventing "try again" messages for errors that can never succeed on retry.AuthViewflash mid-fetch), and adds a secondguestStore.clearAll()guard afterfetchUserDatato handle the race where "Continue as Guest" is tapped while a session fetch is in progress.Confidence Score: 5/5
This PR is safe to merge — the database migration is idempotent and transaction-scoped, the Swift changes are well-tested refactors, and the auth race fix is correct.
All changed paths are either pure refactors (centralising existing logic) or hardening additions (Sentry capture, better user messages). The migration follows the correct DROP-then-ADD pattern inside a single transaction, and the pgTAP test plus Swift contract test guard against recurrence of the root-cause bug. No new state machines, no schema changes beyond the targeted constraint fix, and no auth logic is loosened.
No files require special attention.
Important Files Changed
Comments Outside Diff (1)
AkFit/AkFitTests/SaveErrorClassificationTests.swift, line 413-420 (link)SaveErrorClassification.kind(of:)has an explicit branch that returns.sessionExpiredwhen anAuthError.apicarrieserrorCode == .invalidJWT || errorCode == .badJWT. This path is reachable in production but has no test coverage — a future refactor could silently reclassify these as.retryable.Reviews (3): Last reviewed commit: "Fix save error review issues" | Re-trigger Greptile