Skip to main content

​
Security follow-up plan — beyond PR #3340

Source: CodeRabbit agent security scan, 2026-08-19, 91 open findings (high 12 / medium 54 / low 25) against main. 73 are externally reachable; 15 rated trivially exploitable. This doc records what PR #3340 closes and what it deliberately leaves for later, so the remaining items don’t get lost when the PR merges. It is a plan, not a fix.

​
1. Confirmation: the scan independently validated this PR

The scan ran against main, which does not yet contain #3340, so its findings on the files we changed are a second, independent method landing on the same targets. All nine are fixed on this branch: Two findings the 2026-08-13 audit missed, both verified live and now folded into this PR’s migration set (see §2).

​
2. Now also covered by this PR — 20260819_01

Two DB findings that need no code change (no client path touches either table) are closed in supabase/migrations/20260819_01_lockdown_internal_finicity_tables.sql:
  • public.internal — privilege escalation. authenticated INSERT + WITH CHECK (clerk_id = jwt sub) let any logged-in user add their own clerk_id to the 11-row internal allowlist that lib/is-internal.ts reads (service role) to gate quant_analyst and internal surfaces. Locked to service-role only.
  • public.finicity_users — SELECT USING (true) on a table storing user_secret. Any authenticated user could read every tenant’s aggregator credentials. Client read access removed.

​
3. Remaining DB findings — NEXT PR (need code first, then revoke)

These three are the same class as 20260813_03 but each has a live app write/exec path through the RLS-bound Clerk client, so revoking blind would break a feature. Each needs the code moved to a validated service-role path first, then the grant/policy removed. Also verify the sibling finicity tables (finicity_accounts, finicity_connections, finicity_holdings) for the same USING (true) pattern while here.

​
4. The theme this PR does not touch: the LLM as confused deputy

#3340 fixed who may call a route. The largest untouched class is what the model may do once inside a route — code, SQL, HTML, and outbound URLs derived from model output or prompt-injectable content. This is a distinct project. Recommended: a dedicated “LLM output containment” PR — deterministic guards on code execution (no ambient secrets), a row-scoped SQL execution path (not textual checks), an image/URL egress allowlist, and CSP/sandboxing for LLM-authored HTML.

​
5. Capability abuse — highest-priority single route

  • incoming-call-webhook-send-email-tool/route.ts:170 (high, trivial): unauthenticated; takes prompt, user_id, and recipient email from the body, runs a paid model, and sends from silvia@cfosilvia.com to an arbitrary address. Strictly worse than the scenario-complete hole this PR spent three review rounds on (that only mailed the user’s own address). The 2026-08-13 sweep missed it because it grepped for service-role DB clients and this route touches no DB. Add x-tool-secret (sibling voice-tool pattern) or delete if the dispatcher supersedes it. Verify whether the live [name] dispatcher already covers send_email — if so, delete.

​
6. Broader IDOR / authz set — NEXT PR

Request-supplied ids trusted against a service-role client without an ownership predicate:
  • holdings/[id]/route.ts:76 — SnapTrade account row id from the path (same branch #3340’s chip already flags; confirm the chip’s fix covers this line).
  • personal-info/route.ts:129 — every body property except one treated as writable.
  • chat/tools/document-reference-tool.ts:32 + lib/financial-data-cache.ts:529
    • documents/process-user-batch/route.ts:390 — document/loan-link ids not bound to the caller.
  • email/{welcome,pomp-welcome,mobile} — authenticated but the recipient email is caller-supplied, not the session user’s.
  • post-call-webhook:211, documents/trigger-processing:34, email/radar-complete:36 — unauthenticated or observe-only webhook writes.
  • aws/document_backup/lambdas/handler.py:15 (high, trivial) — unauthenticated Lambda with bucket-wide S3 write, request fields used as the object key.
  • twilio/events/route.ts:199 — public GET reads private Twilio diagnostics via service role (same shape as the sms/delivery-status GET this PR deleted).
The structural fix for most of these is the deferred middleware deny-by-default change (already chipped): flip middleware.ts so /api/* is 401 for signed-out callers with an explicit webhook/cron allowlist, and enforce account_deleted there (the middleware.ts:260 high finding — deletion does not revoke live Clerk sessions today).

​
7. Lower priority — batch these

  • Sensitive logging (≈9 med, mostly trivial): extract-holdings:401, vault/fetch-document-chunks:143, webhooks/clerk:462, incoming-call-webhook-tool/get-account-details:30, fine-tuning-collector:448, lib/auth.ts:11 (NextAuth debug on unconditionally). One “scrub PII from logs” sweep.
  • Denial of service (27 findings): almost all are “no rate limit on an expensive/paid route” (AI import, holdings extraction, Plaid Link token, fork, Finnhub/EVM lookups). A single edge rate-limit + per-user quota policy addresses the bulk; not one-by-one.
  • SSRF: ai-import/route.ts:95 — file_path from a caller-writable row used as a download URL. Fold into the LLM/egress-allowlist work in §4.

​
8. Excluded

  • examples/multiagent/* (view_creator.py, user_scoped_duckdb_tools.py) — sample code, not shipped. Fix or delete from the repo, but not a production risk.

​
Suggested sequencing

  1. This PR (#3340): route authz + 20260813_03 + 20260819_01 (internal, finicity_users).
  2. PR: capability + remaining DB — send-email-tool guard, tracked_tokens endpoint, crunchbase service-role writes, increment_institution_priority.
  3. PR: middleware deny-by-default + account_deleted — closes most of §6 at once.
  4. PR: LLM output containment (§4) — the largest and most distinct effort.
  5. PR: logging scrub + rate-limit policy (§7) — mechanical, batchable.