Wave 7 — full security & correctness remediation + AI multi-provider layer#3
Open
kimhons wants to merge 194 commits into
Open
Wave 7 — full security & correctness remediation + AI multi-provider layer#3kimhons wants to merge 194 commits into
kimhons wants to merge 194 commits into
Conversation
Approved design for sequencing Wave 7 remediation into a bounded MVP via workflow vertical slices, with two milestones (closed beta → public launch). Defines MVP scope (Auth, Payments, Investments, Financial, Credit, Mobile, Ancillary), deferral discipline for Trading/Commerce/white-label (preserved, flag-gated, Wave 8), the foundation block, per-vertical CRITICAL/HIGH mapping, and gate criteria. Confidence: high Scope-risk: narrow
Fixes 4 blocking + 8 should-fix issues from spec review: - C1: Appendix B uses the master plan's explicit 32-item CRITICAL list; documents the SSOT 32-vs-33 off-by-one for TASK-PRE-01 - C2: identifies orphaned HIGHs (FND-035/039/040/045) with no Wave 7 task; mandates task creation - C3: removes invented TASK-TRD-W7-00; PCTT failures are an M1 blocker, not Wave 8 - C4: deferred-code compile-safety becomes an owned obligation - adds Money Correctness track (Phase 3) — FND-024-027 are mandatory CRITICALs even though Commerce workflow is deferred Confidence: high Scope-risk: narrow
Bite-sized TDD plan for the Wave 7 prerequisites and auth/RBAC rebuild — the unblocking foundation for all MVP verticals. Covers PRE-01..07 and AUTH-01..12 (incl. AUTH-03 sub-batches a-f and AUTH-04-staging), grounded in a map of the current auth code. Closes 13 CRITICAL + 7 HIGH findings. Confidence: high Scope-risk: narrow
The PCTT pre-market checklist's market_calendar check reads the real system date (pre-market-checklist.ts: `ctx.timestamp ?? new Date()`). The two PCTT trading test files never mocked the clock, so the weekend gate short-circuited executeTrade on Sat/Sun and 35 tests failed — deterministically green Mon-Fri, red Sat-Sun. This is the root cause of the "35 failing PCTT tests" recorded in health_metrics.md (that re-baseline ran on a weekend). Fix: jest.useFakeTimers with only Date faked (timer fns left real to avoid async breakage), frozen to Wed 2026-05-13. Suite: 496/496. Resolves the tracked Wave 7 PCTT test-determinism concern. Confidence: high Scope-risk: narrow Not-tested: none — full suite 14967/14967 green
Consolidate the Role type that was defined three times with conflicting members (rbac.ts had super_admin/no enterprise; auth-middleware.ts and api-guard.ts had enterprise/no super_admin) into one canonical module src/lib/auth/roles.ts. The enterprise role was a premium alias and is removed everywhere. admin-rbac-audit.ts RequiredRole is now an alias of the canonical Role. Review fixes: - auth-middleware ROLE_PERMISSIONS gains a super_admin entry (wildcard, mirrors rbac.ts) so super_admin no longer falls through to no perms. - roles.test.ts covers isRole (the security-critical JWT-claim guard). - rbac.ts hasRole delegates to the canonical isAtLeast helper instead of rebuilding a local hierarchy array. Constraint: behavior unchanged beyond type consolidation Scope-risk: moderate Confidence: high
…e JWT claim (FND-005) The live API authorization path (withAuth/withRole/withPermission/rbac) trusted the JWT role claim / user_metadata / app_metadata, which a forged or stale token controls. AUTH-01 makes profiles.role the single trusted source for every authorization decision; the JWT establishes identity only. - New src/lib/auth/resolve-role.ts: resolveRoleFromDb(userId) reads profiles.role via a module-level service-role client + 15s per-userId cache; unknown role values default to "user". - api-guard.ts: new exported AuthedUser type; withAuth/withRole/ withPermission/withOptionalAuth discard the JWT role claim and resolve the role from the DB; withPermission feeds the DB-role AuthedUser to rbac. - session.ts: getUser() no longer sets role from user_metadata; getUserRole() delegates to resolveRoleFromDb. - rbac.ts: internal getUserRole drops the user.role/app_metadata/ user_metadata fallbacks. - Tests: resolve-role.test.ts; api-guard.test.ts asserts the FND-005 attack on both withRole and withPermission (forged-admin JWT -> 403). DEFER-COMPILE: clean (project-wide tsc --noEmit passes). Confidence: high Scope-risk: moderate
Consolidate four rate-limiter implementations to one. - Keep src/lib/security/redis-rate-limiting.ts as the canonical limiter. - Delete src/lib/rate-limit.ts, src/lib/security/rate-limiting.ts, and src/lib/security/rate-limiter.ts (plus the two doomed test files). - Move RateLimitConfig/RateLimitResult type defs into the survivor so it is self-contained. - Add a behavior-preserving rateLimit() compat factory to the survivor matching the removed src/lib/rate-limit.ts API (check throws on exceed, reset, getRemaining); repoint 12 investments API routes to it. - Repoint rate-limit-headers.ts and security/index.ts to the survivor. - Extend the survivor test: explicit Nth+1 block test plus compat-factory coverage. Constraint: behavior-preserving — every route stays rate-limited with identical effective limits. Scope-risk: narrow
Add an explicit regression guard asserting the rateLimit() compat factory blocks the (N+1)th call in the in-memory fallback path with Redis env unset. The factory instantiates a fresh RedisRateLimiter per check() call; this is only correct because fallbackStore is module-scoped and shared across instances. The new test fails if the store is ever moved to instance scope (which would make rate limiting a silent no-op in every non-Redis environment) and passes against the current code. Verified fail-before / pass-after by temporarily moving the store to instance scope. Scope-risk: narrow
…FND-050, FND-051)
Wrap all 11 src/app/api/admin/** routes (sub_batch=a) in the rebuilt
withRole("admin") guard. Removes the deprecated requireRole/createAuthResponse
inline auth and the inline rbac.hasPermission("admin:analytics") check on the
affiliate revenue route, promoting both to the wrapper. Closes the
unauthenticated admin POST audit / PATCH disputes / DELETE subscriptions holes.
Adds co-located negative-auth tests (401 + 403 per exported handler) and
rewires the existing admin test suites from the removed requireRole middleware
to the jwtValidation + resolveRoleFromDb guard path.
Constraint: behavior-preserving except the intended auth hardening
Confidence: high
Scope-risk: narrow
…/bill_* + complete audit-log anonymisation
…int rule (FND-061) Migrates chat-engine, financial-chat-engine, entity-extractor, intent-recognizer, ai-stock-analyst to ModelRouter.complete. Adds the no-direct-aiml-service ESLint rule (error severity) enforcing that aiml-service is importable only by model-router.ts + the documented multi-model exempt engines. Fixed a jest.mock hoisting TDZ in the ai-stock-analyst test (mock response moved inside the factory).
…telist (FND-059/060) - ai/chat route: remove client-supplied model field; route all completions through ModelRouter.complete(TaskType.GENERAL_CHAT, messages, options) via getModelRouter() — eliminates arbitrary model selection and cost burn - voice/synthesize route: add VALID_TTS_MODELS = ['tts-1', 'tts-1-hd'] server-side allowlist; reject non-whitelisted model with 400 - eslint-rules/no-direct-aiml-service.js: add voice/synthesize as documented exempt (audio.speech TTS call; no chat-completion path in ModelRouter) - Tests: 12 new tests (6 per route) covering red→green for FND-059/060; lambda-wrapper pattern survives resetMocks:true in jest config - Full suite: 16,088 passing, 0 failures; type-check: 0 errors; lint: 0 errors, 0 no-direct-aiml-service violations
MNY-6 review LOW finding: the non-whitelisted-model 400 test now also asserts mockGenerateSpeech was not called, so a future regression that moves the guard after the service call is caught.
…calls (FND-062/063)
- Add src/lib/aiml/sanitizer.ts: sanitizeUserInput() and sanitizeContextValue()
compose anonymizePII() from pii-protection.ts + 18 injection-pattern regexes.
No import of aiml-service (respects no-direct-aiml-service lint rule).
No PII logged (respects warn-pii-logging hookify guard).
- Fix financial-chat-engine.ts detectIntent(): replace .replace('{{MESSAGE}}'...)
and .replace('{{CONTEXT}}'...) template interpolation with a two-message array
[system: INTENT_DETECTION_SYSTEM_PROMPT, user: sanitized message + context].
User-derived text never appears in a system-role string.
- Fix financial-chat-engine.ts generateResponse(): replace .replace('{{INTENT}}')
and .replace('{{CONTEXT}}') with [system: RESPONSE_GENERATION_SYSTEM_PROMPT,
user: sanitized intent JSON + context JSON]. Both intent entity values and
context.sessionHistory (prior user text) are sanitized before the AI call.
- Fix chat-engine.ts buildPrompt(): apply sanitizeUserInput() to the current
user message and to all user-role history messages before they enter the
messages array passed to ModelRouter.complete().
- Add INTENT_DETECTION_SYSTEM_PROMPT and RESPONSE_GENERATION_SYSTEM_PROMPT
exports to financial-chat-prompts.ts. Retain old INTENT_DETECTION_PROMPT
and RESPONSE_GENERATION_PROMPT as @deprecated exports (backward compat).
- Add 11 sanitizer unit tests (sanitizer.test.ts) and 7 engine integration
tests (engine-sanitization.test.ts). All 18 tests: RED before fix, GREEN after.
Closes FND-062 (prompt injection via .replace()) and FND-063 (PII to AIML API).
7 tasks closing the notifications + admin finding set (FND-041..055). AUTH-03 already closed the 'zero auth' half of FND-041..044/049..051 — plan's findings table corrects the stale register. Real remaining work: mark-read/delete IDOR (FND-046), in-memory persistence (FND-047/048), email XSS (FND-045), dispute mass-assignment (FND-051/054), fabricated admin analytics (FND-052/053), unbounded limit (FND-055).
Critic caught 4 BLOCKING premise errors, all verified at HEAD: - FND-046 IDOR is in notification-service-db.ts (zero callers); the LIVE path is the in-memory notification-service.ts — NTF-2/NTF-3 ordering corrected (NTF-2 hardens service, NTF-3 wires + reconciles) - 3-way NotificationType enum mismatch — NTF-3 must reconcile or POST 500s - notification_preferences table does not exist — NTF-4 creates it - system_logs table does not exist — ADM-2 makes logs honestly-empty Plan grew to 8 tasks; auth claim confirmed accurate.
Critic iteration 2: all 4 BLOCKING + 4 SHOULD-FIX resolved, APPROVED. Folded in: NTF-3 must update NotificationItem/NotificationCenter switch/filterOptions when narrowing NotificationType; NTF-4 FK target fixed to auth.users(id) + migration-timestamp-after-20260518000002 note.
…on routes (FND-041..044)
…mark-read/delete (FND-046)
…concile NotificationType (FND-047)
… persistence (FND-048)
- new notification_preferences table (user_id PK FK auth.users CASCADE,
RLS owner-only); makes the GDPR erasure-RPC reference resolve
- preferences/route.ts GET/PUT persist through the table; module-level
preferencesStore Record deleted
- deleted the fake POST stub (no-op subscribe/unsubscribe that lied
{success:true}, redundant with PUT); removed its now-orphaned tests
…+ PUT validation NTF-4 review (2 MEDIUM + 2 LOW, all on new code): - migration: REVOKE direct PostgREST access + GRANT service_role (matches the Wave 7 table-migration pattern); drop the redundant user_id index (the PK already provides it) - PUT: reject non-object channels/quietHours with 400 (was a 500) - GET/PUT: log caught errors instead of discarding them
…ssignment (FND-051/054)
…date fields ADM-1 review (2 MEDIUM + 1 LOW, all on new code): - DISPUTE_STATUS_VALUES gains 'escalated', DISPUTE_OUTCOME_VALUES gains 'pending' — the route enum now matches the authoritative disputes-table CHECK constraint (20250204000000_credit_repair_schema.sql); a valid status no longer gets a wrongful 400 - resolved_at/sent_at validated as parseable dates → clean 400 instead of a DB-layer 500 on a non-date string
…move fabricated data (FND-052/053)
- analytics/route.ts: replace all Math.random() with real Supabase queries
(disputes by status, subscriptions by plan, user growth buckets, revenue by month)
6-tier priceMap (free/standard/pro/family_duo/family/family_plus)
- stats/route.ts: remove env-var-missing mock fallback and outer catch mock;
fix 3-tier priceMap (price_basic/premium/enterprise) -> 6-tier; use plan
column not stripe_price_id; missing env vars -> 503; DB errors -> 500
- audit/route.ts: remove 42P01 branch that returned generateMockAuditLogs();
remove outer catch mock fallback; both error paths -> 500 with error
- logs/route.ts: system_logs table does not exist — return honest
{ logs: [], total: 0, dataAvailable: false } immediately; no Supabase call
Tests (Test Integrity Rule: old mock-encoding assertions migrated, not weakened):
- adm2-real-queries.test.ts: 9 new TDD tests covering all 4 de-mocked routes
- stats-and-disputes.test.ts: 3 tests updated (503 on no env, 6-tier revenue,
500 on exception)
- audit-and-logs.test.ts: 42P01 test -> 500+error; exception test -> 500+error;
logs section replaced with 4 honest-unavailable-response tests
- settings-and-analytics.test.ts: add @supabase/supabase-js mock + env vars;
add analytics beforeEach with full query chain mock; update subscriptionsByTier
assertions from old Math.random tier names to real 6-tier plan names
Verified: 16,159 tests pass, 0 failures; tsc --noEmit 0 errors;
grep Math.random across all 4 routes: zero hits
…tion role gates (FND-055/049/050)
… force-dynamic pages The production build was silently broken across Wave 7 — task agents verified with type-check + jest, which do NOT catch App-Router server/client boundary or build-time module-eval failures; only `next build` does, and it was never run. Root causes fixed: - auth-service pulled supabase/server (next/headers) into a client graph → split the service-role client into next/headers-free src/lib/supabase/admin.ts - model-router's lazy aiml-service import dragged openai/shims/node (node:fs) into the ai-tools client page - ~13 modules constructed Supabase/Resend clients at module scope → 'supabaseUrl is required' during build page-data collection; made each lazy (Proxy defers construction to first use) - getSupabase() returns a lazy Proxy so module-scope callers are safe - client.ts createClient() untyped + placeholder fallback for SSR prerender; data-dependent pages routed through it / force-dynamic - 7 test suites' mocks re-pointed to the new module structure Build: exit 0, 500 routes. type-check: 0 errors. suite: 16,165 pass / 0 fail.
The root _layout.tsx declared 9 <Stack.Screen name="dispute/*"> entries. After MOB-8 (defd635) made dispute a nested navigator with its own app/dispute/_layout.tsx, those children are owned by the nested layout, so expo-router emitted a "No route named ..." warning for each on launch. Collapse to a single <Stack.Screen name="dispute">. Verified on iOS simulator: route warnings 9 -> 0.
…n submit
authStore.initialize() surfaced Supabase's AuthSessionMissingError ("Auth session missing!") as a red error banner to users who simply had not signed in yet. Treat a missing session as the normal unauthenticated state; genuine init failures (e.g. network) still surface. login.tsx silently no-op'd when Sign In was tapped with empty fields; now shows "Please enter your email and password." which clears as the user types. Verified on iOS simulator.
Adds direct provider adapters (OpenAI, Anthropic, Google Gemini, DeepSeek, Mistral) behind a unified AIProvider interface plus a prefix-routing registry; AIML API remains the catch-all fallback. ModelRouter.complete() now walks the full model chain for a TaskType, advancing past retryable errors (429/5xx/timeout) to the next provider and surfacing the last error; non-retryable errors (auth/bad-request) fail fast. All provider keys are optional env vars (env-validation.ts); AIML_API_KEY stays the only required AI key. OpenAI SDK shim imported before the SDK and AIMLService imported lazily to keep next build's page-data collection clean. Verified: type-check 0 errors, build exit 0 (453 pages), 16,179 tests pass / 0 failures, lint clean. NOTE: cross-provider failover is covered by mock-based unit tests only; live multi-provider failover is not yet exercised.
kimhons
changed the title
Terms of Service / Privacy Policy on the register screen were styled to look like links but had no handler (inert text). Wire them to the web /terms and /privacy-policy pages via the FND-070 openExternalUrl allowlist (https-only, configurable EXPO_PUBLIC_WEB_URL with fynvita.com default), with accessibilityRole="link". Also change the subtitle "Start your credit repair journey" -> "Start your financial wellness journey" — on-brand for the financial-wellness positioning and avoids CROA "credit repair" framing on the signup tagline. Verified on iOS simulator: subtitle updated; ToS/Privacy now render as tappable AXLink elements. Type-check clean.
The Forgot Password link was a <Link> wrapped in a <TouchableOpacity> with no onPress. In React Native the outer touchable becomes the press responder and swallows the tap, so the inner <Link> never fired — users could not reach password reset from the login screen (a broken critical auth flow). Move navigation onto the TouchableOpacity via router.push and drop the swallowed inner <Link> (Link import retained for the Sign up link). Verified on iOS simulator: tapping Forgot password? now navigates to the reset screen (3 prior taps did nothing). Type-check clean.
…rollToIndex handleNext() calls FlatList.scrollToIndex with no getItemLayout and no onScrollToIndexFailed handler, so a jump to an unmeasured index can throw. Slides are fixed full-window width, so provide getItemLayout (length: width, offset: width*index) to make scrollToIndex deterministic. Type-check clean; carousel paging verified on iOS simulator (Next advances all 5 slides, Get Started -> login).
…protected screens app/index.tsx self-redirects on entry, but deep-linking straight to a protected route (e.g. exp://.../--/dashboard) bypassed it and rendered the full authenticated UI — credit scores, gamification, all tabs — with no session. Add a root-layout segment guard (useSegments) that router.replace()s unauthenticated users to /(auth)/login for every route except the public ones: the (auth) group, onboarding, the handoff deep-link handler, and the index entry (which self-redirects). Verified on iOS simulator: deep-linking /dashboard with no session now lands on the login screen (previously rendered the full dashboard). Type-check clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wave 7 — Foundation Block (Phase 0 + Phase 1, partial)
Rebuilds the auth/RBAC layer and stands up the Wave 7 prerequisites. Executed task-by-task; every code task went through implement → spec-compliance review → code-quality review → fix loop → re-review.
Base:
feat/asset-system-regen(this branch forked from it — 21 commits).Closed findings
CRITICAL (4): FND-002 (AIML key reused as inbound auth), FND-003 + FND-004 (admin email whitelist + enterprise-tier=admin grant), FND-005 (privilege self-grant — authorization trusted the JWT role claim /
user_metadata).HIGH (8): FND-007 (in-memory sessions), FND-008 (CSRF secret silent default), FND-009 (non-atomic signup → orphaned auth users), FND-010 (backup-code TOCTOU), FND-011 (non-timing-safe secret comparisons), FND-012 (3+ conflicting
Roledefinitions), FND-013 (4 rate-limiter implementations).What changed
resolve-role.ts: authorization role now resolved fromprofiles.role(DB) on every request, cached 15s;withAuth/withRole/withPermission/withOptionalAuthdiscard the JWT role claim. FND-005 attack tests (forged-admin JWT → 403) on bothwithRoleandwithPermission.profiles.role === "admin"; hardcoded email whitelist and enterprise-tier grant deleted.validateAPIKeyno longer authenticates callers presenting the AIML provider key.redis-rate-limiting.ts); 14 importers repointed.crypto.timingSafeEqualfor all secret comparisons, CSRF secret hard-fails in production, atomic backup-code redemption RPC (FOR UPDATE), atomic signup with rollback.Roletype (roles.ts);enterpriseremoved.no-math-random-in-prodlint guard + Wave 7 npm scripts.Test plan
Suite green throughout — 14,978 passing, 0 failures;
tsc --noEmitclean project-wide; type-check clean. Each auth change carries negative-path tests (forged-token, wrong-role, service-failure).NOT in this PR — gated work
docs/superpowers/auth-route-inventory.csv(294 routes classified) — included for SEC sign-off. AUTH-03 (wrap all 294 routes inwithAuth/withRole/withPermission, ≥568 negative-auth tests) and AUTH-04 (middleware deny-by-default + 24h staging soak) run after sign-off.Tracked residual findings (for follow-up tasks)
src/app/api/financial/plaid/hosted-link/route.ts— inlinevalidation.user.role(JWT-claim) authz check; closed by AUTH-03 wrapping.src/lib/auth/mfa-service.ts— a separateuser_backup_codesTOCTOU (distinct from AUTH-10'sbackup_codespath; currently unreachable — no wired route).🤖 Generated with Claude Code
🔄 Scope update — 2026-05-24
This PR has grown well beyond its original "Phase 0 + Phase 1 partial" title. The branch now carries the full Wave 7 remediation plus follow-on work: 100 commits, 896 files, +53,460 / −21,677. The sections above describe only the initial auth/RBAC foundation; everything below has since landed on the same branch.
Now included (beyond the original auth foundation)
withAuth/withRole/withPermission(209/295 route files guarded; remainder are public or tracked gaps), and middleware deny-by-default is implemented (gated behind theauth.deny_by_defaultfeature flag).processed_webhook_eventsidempotency table, 6-tiergetTierFromPriceIdmapping, handlers rethrow, server-authoritative checkout, signature-verification audit, tier backfill.Money/Centsbranded type + lint rule..eq("user_id", userId), Plaid token-out-of-URL, credit/disputes/documents de-mocked onto DB), analytics div-by-zero + honest-volatility fixes, month-rollover fix, N+1 Plaid elimination, real debt persistence.delete_user_data_cascadeRPC, ModelRouter execution path (no longer dead code), PII sanitizer + prompt-injection guard, AI route lockdown.__DEV__auth bypass (FND-064, regression-tested), SecureStore migration,Linking.openURLallowlist, mobile CVE fixes, deleted deprecatedfinancialStore, authenticated mobile API client.Latest 3 commits on this branch
e2ab13afeat(ai)— multi-provider fallback layer (OpenAI/Anthropic/Google/DeepSeek/Mistral) behind ModelRouter; AIML remains the required fallback.f24f868fix(mobile)— suppress benign "Auth session missing!" on the login screen; validate empty login submit.67371d3fix(mobile)— collapse 9 staledispute/*route declarations (route warnings 9 → 0).Verification (worktree, 2026-05-24)
next buildexit 0 — compiled successfully, 453 static pages, 560 kB first-load JS.feat/asset-system-regenhas 35 genuineopenPosition()failures here, so this branch fixes them (they are not "weekend/time-dependent").Honest open items (NOT fully closed — do not read this as ship-ready)
marketplace/tradelinesmissing auth wrapper.fetch()s still bypass the authenticated client; mobile suite health poor (51 failures + coverage threshold).npm audit6 high / 14 moderate; mobile 39 (1 critical / 17 high).