Riderx/fix email otp rpc reopen

[riderx]

Summary

Test plan

Screenshots

Checklist

• My code follows the code style of this project and passes
  bun run lint:backend && bun run lint.
• My change requires a change to the documentation.
• I have updated the documentation
  accordingly.
• My change has adequate E2E test coverage.
• I have tested my code manually, and I have provided steps how to reproduce
  my tests

Summary by CodeRabbit

• Bug Fixes

  • Improved email OTP verification security by enforcing server-side only function execution.

• Chores

  • Refactored OTP verification process to use explicit user identification for enhanced reliability.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR modifies the email OTP verification flow to use an admin-scoped Supabase client for recording verification instead of a token-scoped client. The new approach explicitly passes the user_id to a redesigned PostgreSQL function, which now validates the input and enforces access via service_role only.

Changes

Cohort / File(s)	Summary
OTP Verification Logic supabase/functions/_backend/private/verify_email_otp.ts	Switched from supabaseClient to supabaseAdmin(c) for calling the RPC. Now constructs otpVerifiedAt locally and passes explicit user_id to the function. Simplified error condition by removing dependency on undefined verifiedAt.
Database Function & Permissions supabase/migrations/20260224100000_revoke_record_email_otp_verified_auth_role.sql	Introduces new RPC record_email_otp_verified(user_id uuid) with NULL validation and SECURITY DEFINER. Upserts verification record into public.user_security. Grants EXECUTE only to service_role and postgres, revoking access from public and authenticated roles. Removes legacy zero-argument overload.

Sequence Diagram

sequenceDiagram
    actor Client
    participant VerifyFunc as Verify Function<br/>(TypeScript)
    participant AdminClient as Supabase Admin<br/>Client
    participant RPC as RPC Function<br/>(PostgreSQL)
    participant DB as user_security<br/>Table

    Client->>VerifyFunc: OTP Verification Request
    VerifyFunc->>VerifyFunc: Validate OTP
    Note over VerifyFunc: Create otpVerifiedAt<br/>timestamp locally
    VerifyFunc->>AdminClient: Call with user_id
    AdminClient->>RPC: record_email_otp_verified(user_id)
    RPC->>RPC: Validate user_id NOT NULL
    RPC->>DB: Upsert email_otp_verified_at
    DB-->>RPC: Return current timestamp
    RPC-->>AdminClient: Return timestamptz
    AdminClient-->>VerifyFunc: Return result
    VerifyFunc-->>Client: Return verified_at in response

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Harden email OTP verification #1582: Directly related—modifies the same verify_email_otp function and OTP recording flow, with this PR switching to the new admin client approach and updated record_email_otp_verified(user_id) RPC signature.

Poem

🐰 A rabbit hops through token scopes,
But now with admin powers, renewed hopes!
User IDs explicit, permissions tight,
Service-role gatekeeper keeps access right. ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description consists entirely of an empty template with no content filled in—the Summary and Test plan sections are blank, and all checklist items remain unchecked.	Complete the description template: add a summary explaining the RPC refactoring changes, document test steps for email OTP verification, and indicate which checklist items apply.
Title check	❓ Inconclusive	The title is vague and uses non-descriptive terms ('fix', 'reopen') that don't clearly convey the actual changes involving email OTP RPC refactoring and permission restrictions.	Revise the title to be more specific about the core change, such as 'Refactor email OTP verification to use admin RPC with explicit user_id and restrict permissions' or similar.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch riderx/fix-email-otp-rpc-reopen

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 SQLFluff (4.0.4)

supabase/migrations/20260224100000_revoke_record_email_otp_verified_auth_role.sql

User Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects:
ansi, athena, bigquery, clickhouse, databricks, db2, doris, duckdb, exasol, flink, greenplum, hive, impala, mariadb, materialize, mysql, oracle, postgres, redshift, snowflake, soql, sparksql, sqlite, starrocks, teradata, trino, tsql, vertica

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 344bfffb69

ℹ️ 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".

[chatgpt-codex-connector]

Preserve legacy RPC during staged deployment

Dropping record_email_otp_verified() immediately creates a rollout compatibility gap: in .github/workflows/build_and_deploy.yml, supabase db push runs before supabase functions deploy, so the currently deployed edge function (still calling the zero-arg RPC until the next step) will fail OTP verification during that window with missing/unauthorized function errors. Keep the legacy signature (e.g., wrapper overload) for at least one deploy cycle, then remove it after all function runtimes have switched to the new user_id form.

Useful? React with 👍 / 👎.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

supabase/functions/_backend/private/verify_email_otp.ts (1)

81-88: ⚠️ Potential issue | 🔴 Critical

Guard verifyData.user before dereferencing id.

Line 87 dereferences verifyData.user.id without a hard null check. This matches the TS18047 failure and can throw at runtime when user is null.

🔧 Proposed fix

-  if (verifyData.user?.id && verifyData.user.id !== auth.userId) {
+  const verifiedUserId = verifyData.user?.id
+  if (!verifiedUserId) {
+    return quickError(401, 'invalid_otp', 'Invalid or expired OTP')
+  }
+  if (verifiedUserId !== auth.userId) {
     return quickError(403, 'otp_user_mismatch', 'OTP does not match current user')
   }

   const otpVerifiedAt = new Date().toISOString()
   const { error: recordError } = await supabaseAdmin(c).rpc('record_email_otp_verified', {
-    user_id: verifyData.user.id,
+    user_id: verifiedUserId,
   })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/_backend/private/verify_email_otp.ts` around lines 81 -
88, The code dereferences verifyData.user.id without ensuring verifyData.user
exists, which can cause runtime/TS18047 errors; update the flow in
verify_email_otp.ts to explicitly guard verifyData.user before accessing .id
(e.g., if (!verifyData.user || !verifyData.user.id) return quickError(400,
'otp_no_user', 'No user found for OTP')), and only call
supabaseAdmin(c).rpc('record_email_otp_verified', { user_id: verifyData.user.id
}) after that guard so record_email_otp_verified and the otpVerifiedAt
assignment never run with a null user.

🧹 Nitpick comments (1)

supabase/functions/_backend/private/verify_email_otp.ts (1)

85-95: Prefer returning the DB-persisted verification timestamp.

Line 85 currently uses worker time, while the migration writes DB time. Returning the RPC value avoids drift between API response and stored email_otp_verified_at.

♻️ Suggested adjustment

-  const otpVerifiedAt = new Date().toISOString()
-  const { error: recordError } = await supabaseAdmin(c).rpc('record_email_otp_verified', {
+  const { data: otpVerifiedAt, error: recordError } = await supabaseAdmin(c).rpc('record_email_otp_verified', {
     user_id: verifyData.user.id,
   })

-  if (recordError) {
+  if (recordError || !otpVerifiedAt) {
     cloudlog({ requestId: c.get('requestId'), context: 'verify_email_otp - record failed', error: recordError?.message })
     return quickError(500, 'record_failed', 'Failed to record OTP verification')
   }

   return c.json({ verified_at: otpVerifiedAt })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/_backend/private/verify_email_otp.ts` around lines 85 -
95, The code returns a local otpVerifiedAt (new Date().toISOString()) but the
RPC record_email_otp_verified writes the canonical DB timestamp; change the flow
to capture the RPC result payload from
supabaseAdmin(c).rpc('record_email_otp_verified', { user_id: verifyData.user.id
}) (e.g., inspect the returned data field) and return the persisted timestamp
from that payload (email_otp_verified_at) in the JSON response instead of
otpVerifiedAt; keep the existing error handling for recordError unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@supabase/functions/_backend/private/verify_email_otp.ts`:
- Around line 86-88: The RPC type for record_email_otp_verified is out of sync
causing a TS error at the call in verify_email_otp.ts
(supabaseAdmin(c).rpc('record_email_otp_verified', { user_id: verifyData.user.id
})). Regenerate your Supabase client types so the function signature in
supabase/functions/_backend/utils/supabase.types.ts (record_email_otp_verified:
{ Args: ... }) includes the user_id: uuid argument; then re-run type generation
(e.g., via the Supabase CLI or your project’s gen script) and commit the updated
types so the RPC call typechecks.

---

Outside diff comments:
In `@supabase/functions/_backend/private/verify_email_otp.ts`:
- Around line 81-88: The code dereferences verifyData.user.id without ensuring
verifyData.user exists, which can cause runtime/TS18047 errors; update the flow
in verify_email_otp.ts to explicitly guard verifyData.user before accessing .id
(e.g., if (!verifyData.user || !verifyData.user.id) return quickError(400,
'otp_no_user', 'No user found for OTP')), and only call
supabaseAdmin(c).rpc('record_email_otp_verified', { user_id: verifyData.user.id
}) after that guard so record_email_otp_verified and the otpVerifiedAt
assignment never run with a null user.

---

Nitpick comments:
In `@supabase/functions/_backend/private/verify_email_otp.ts`:
- Around line 85-95: The code returns a local otpVerifiedAt (new
Date().toISOString()) but the RPC record_email_otp_verified writes the canonical
DB timestamp; change the flow to capture the RPC result payload from
supabaseAdmin(c).rpc('record_email_otp_verified', { user_id: verifyData.user.id
}) (e.g., inspect the returned data field) and return the persisted timestamp
from that payload (email_otp_verified_at) in the JSON response instead of
otpVerifiedAt; keep the existing error handling for recordError unchanged.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 149b293 and 344bfff.

📒 Files selected for processing (2)

• supabase/functions/_backend/private/verify_email_otp.ts
• supabase/migrations/20260224100000_revoke_record_email_otp_verified_auth_role.sql

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

TYPE_FILE="$(fd 'supabase.types.ts' | head -n1)"
echo "Using type file: ${TYPE_FILE}"

echo "== Migration signature =="
rg -n -C2 "record_email_otp_verified" supabase/migrations/20260224100000_revoke_record_email_otp_verified_auth_role.sql

echo "== Generated TS signature =="
rg -n -C4 "record_email_otp_verified" "${TYPE_FILE}"

echo "== Current callsite =="
rg -n -C3 "rpc\\('record_email_otp_verified'" supabase/functions/_backend/private/verify_email_otp.ts

Repository: Cap-go/capgo

Length of output: 1046

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for supabase type files
echo "=== Finding Supabase type files ==="
find . -type f -name "*supabase*.ts" -o -name "database.types.ts" | grep -v node_modules | head -20

echo ""
echo "=== Searching for record_email_otp_verified in all TypeScript files ==="
rg -t ts "record_email_otp_verified" --max-count=5

echo ""
echo "=== Current callsite code ==="
cat -n supabase/functions/_backend/private/verify_email_otp.ts | sed -n '80,95p'

Repository: Cap-go/capgo

Length of output: 1281

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Full supabase.types.ts file ==="
wc -l supabase/functions/_backend/utils/supabase.types.ts

echo ""
echo "=== Search for record_email_otp_verified in types file ==="
rg -n -C10 "record_email_otp_verified" supabase/functions/_backend/utils/supabase.types.ts

echo ""
echo "=== Check if file mentions Database interface ==="
head -100 supabase/functions/_backend/utils/supabase.types.ts | tail -50

Repository: Cap-go/capgo

Length of output: 2372

---

Regenerate Supabase RPC types for record_email_otp_verified(uuid).

The generated type definition at supabase/functions/_backend/utils/supabase.types.ts:3882 shows record_email_otp_verified: { Args: never; Returns: string }, which means no arguments are accepted. However, the migration defines CREATE OR REPLACE FUNCTION "public"."record_email_otp_verified"("user_id" uuid), and the callsite passes { user_id: verifyData.user.id }. This causes a TypeScript error because the generated types are out of sync with the actual RPC signature. Regenerate the Supabase types to reflect the RPC parameter.

🧰 Tools

🪛 GitHub Actions: Run tests

[error] 86-86: TS2345: Argument of type '{ user_id: string; }' is not assignable to parameter of type 'undefined'.

---

[error] 87-87: TS18047: 'verifyData.user' is possibly 'null'.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/_backend/private/verify_email_otp.ts` around lines 86 -
88, The RPC type for record_email_otp_verified is out of sync causing a TS error
at the call in verify_email_otp.ts
(supabaseAdmin(c).rpc('record_email_otp_verified', { user_id: verifyData.user.id
})). Regenerate your Supabase client types so the function signature in
supabase/functions/_backend/utils/supabase.types.ts (record_email_otp_verified:
{ Args: ... }) includes the user_id: uuid argument; then re-run type generation
(e.g., via the Supabase CLI or your project’s gen script) and commit the updated
types so the RPC call typechecks.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud