Fix Clerk-Supabase Auth Integration and Next.js 15 DB Error - #708
Fix Clerk-Supabase Auth Integration and Next.js 15 DB Error#708ngoiyaeric wants to merge 1 commit into
Conversation
- Refactored lib/db/index.ts to manually parse DATABASE_URL using pg-connection-string, bypassing problematic internal URL parsing in the pg driver that triggered searchParams errors in Next.js 15. - Added public.clerk_id() and public.is_clerk_user() helper functions to the Supabase database to support Clerk JWT authentication in RLS policies. - Updated RLS policies for users, chats, messages, and other tables to authorize Clerk users. - Improved error handling and logging in lib/auth/get-current-user.ts. - Created Drizzle migration 0003_add_clerk_auth_helpers.sql to track database changes. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
WalkthroughThis PR adds a SQL migration introducing Clerk authentication helper functions ( ChangesClerk Authorization Migration
Database connection and error handling updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@drizzle/migrations/0003_add_clerk_auth_helpers.sql`:
- Around line 63-78: Split the current messages_owner_participant_access policy
in the messages policy definition so participant-based access is only used for
reading, not writing. The existing FOR ALL policy in the messages policy block
allows the chat_participants EXISTS branch to satisfy INSERT/UPDATE/DELETE
checks even when messages.user_id does not match the current user; keep that
participant logic for SELECT access, but create separate write rules that
require messages.user_id to equal auth.uid() or the Clerk-backed user check
before allowing INSERT/UPDATE/DELETE.
- Around line 4-5: The mixed Clerk/Supabase policy checks are using raw
auth.uid() comparisons, which can fail for Clerk text-based subject IDs before
the public.is_clerk_user(...) branch is evaluated. Update the helper in this
migration to use a safe UUID-returning auth function and replace every
auth.uid() usage in the affected policies/functions with that helper so Clerk
requests are handled consistently.
- Around line 134-143: The INSERT policy for chat participants currently depends
on an existing owner row in public.chat_participants, which blocks creating the
first owner. Update the policy in the chat_participants migration to
special-case the bootstrap insert by allowing the creator identified by
public.chats.user_id to insert the initial owner participant for that chat. Keep
the existing owner-check logic for subsequent inserts, and reference the policy
definition around the "Only owners can insert participants" policy when making
the change.
In `@lib/db/index.ts`:
- Line 3: Add pg-connection-string to the root production dependencies because
lib/db/index.ts and lib/db/migrate.ts import it directly at runtime. Update
package.json so the dependency is declared explicitly, and keep the existing
imports using parse from pg-connection-string unchanged.
- Line 31: The SSL handling in the database connection setup is treating an
explicit false as if SSL was unset. In lib/db/index.ts within the connection
logic that checks Supabase URLs, update the condition around poolConfig.ssl so
it only applies the Supabase SSL override when ssl is undefined, not when it was
explicitly set to false. Keep the existing behavior in the
connectionString-based branch and preserve the caller’s sslmode/ssl=false
intent.
- Around line 8-12: In lib/db/index.ts, the DATABASE_URL handling currently
falls back to a localhost connection even when NODE_ENV is production, which can
let server actions and API routes run real queries against the wrong database.
Update the module-level connection setup around connectionString so production
fails closed: when process.env.NODE_ENV is production and DATABASE_URL is
missing, stop initialization by throwing an error (or otherwise preventing pool
creation) instead of defaulting to postgres://localhost:5432/postgres. Keep the
existing warning behavior only for non-production environments, and ensure the
fix is applied in the db initialization path that creates the pool.
In `@lib/db/migrate.ts`:
- Around line 15-24: The migration pool setup in migrate.ts is ignoring
parsedConfig.ssl, which can make migration connections behave differently from
the main database path. Update the Pool configuration in the migration logic to
prefer parsedConfig.ssl from parse(connectionString), and only apply the
Supabase ssl fallback when parsedConfig.ssl is not present; use the existing
migrate.ts Pool construction and the same connectionString parsing flow to keep
SSL behavior consistent with lib/db/index.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4b90b2bb-b274-4962-b923-936dfa3bdea0
📒 Files selected for processing (5)
drizzle/migrations/0003_add_clerk_auth_helpers.sqldrizzle/migrations/meta/_journal.jsonlib/auth/get-current-user.tslib/db/index.tslib/db/migrate.ts
📜 Review details
🧰 Additional context used
🪛 SQLFluff (4.2.2)
drizzle/migrations/0003_add_clerk_auth_helpers.sql
[error] 19-19: Do not use special characters in identifiers.
(RF05)
[error] 20-20: Do not use special characters in identifiers.
(RF05)
[error] 29-29: Do not use special characters in identifiers.
(RF05)
[error] 30-30: Do not use special characters in identifiers.
(RF05)
[error] 38-38: Do not use special characters in identifiers.
(RF05)
[error] 39-39: Do not use special characters in identifiers.
(RF05)
[error] 47-47: Do not use special characters in identifiers.
(RF05)
[error] 48-48: Do not use special characters in identifiers.
(RF05)
[error] 82-82: Do not use special characters in identifiers.
(RF05)
[error] 83-83: Do not use special characters in identifiers.
(RF05)
[error] 92-92: Do not use special characters in identifiers.
(RF05)
[error] 93-93: Do not use special characters in identifiers.
(RF05)
[error] 102-102: Do not use special characters in identifiers.
(RF05)
[error] 103-103: Do not use special characters in identifiers.
(RF05)
[error] 112-112: Do not use special characters in identifiers.
(RF05)
[error] 113-113: Do not use special characters in identifiers.
(RF05)
[error] 122-122: Do not use special characters in identifiers.
(RF05)
[error] 123-123: Do not use special characters in identifiers.
(RF05)
[error] 134-134: Do not use special characters in identifiers.
(RF05)
[error] 135-135: Do not use special characters in identifiers.
(RF05)
[error] 146-146: Do not use special characters in identifiers.
(RF05)
[error] 147-147: Do not use special characters in identifiers.
(RF05)
[error] 158-158: Do not use special characters in identifiers.
(RF05)
[error] 159-159: Do not use special characters in identifiers.
(RF05)
🔇 Additional comments (4)
lib/auth/get-current-user.ts (1)
20-26: LGTM!Also applies to: 55-58
drizzle/migrations/meta/_journal.json (1)
25-34: LGTM!drizzle/migrations/0003_add_clerk_auth_helpers.sql (1)
122-166: 🩺 Stability & AvailabilityNo RLS recursion here.
public.chat_participantsdoesn’t have RLS enabled, so these self-queries won’t trigger recursive policy evaluation.> Likely an incorrect or invalid review comment.lib/db/migrate.ts (1)
4-4: Dependency declaration concern already covered.The direct
pg-connection-stringimport needs the same manifest fix noted inlib/db/index.tsLine 3.
| SELECT (auth.jwt() ->> 'sub'); | ||
| $$ LANGUAGE sql STABLE; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files | rg 'drizzle/migrations/0003_add_clerk_auth_helpers\.sql|drizzle/migrations/.*\.sql$|supabase|auth|rls|policy' || true
printf '\n== Search auth.uid / clerk_id / auth.jwt ==\n'
rg -n "auth\.uid\(|clerk_id\(|auth\.jwt\(" drizzle supabase . --glob '*.sql' --glob '*.ts' --glob '*.js' --glob '*.mjs' --glob '*.cjs' || true
printf '\n== Read target migration ==\n'
if [ -f drizzle/migrations/0003_add_clerk_auth_helpers.sql ]; then
nl -ba drizzle/migrations/0003_add_clerk_auth_helpers.sql | sed -n '1,220p'
fiRepository: QueueLab/QCX
Length of output: 5840
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' drizzle/migrations/0003_add_clerk_auth_helpers.sql | cat -nRepository: QueueLab/QCX
Length of output: 6712
Avoid raw auth.uid() in the mixed Clerk/Supabase policies (drizzle/migrations/0003_add_clerk_auth_helpers.sql:20-166). Clerk sub values are text IDs, so these checks can fail for Clerk requests before the public.is_clerk_user(...) branch runs. Replace them with a safe UUID helper and use it throughout this migration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drizzle/migrations/0003_add_clerk_auth_helpers.sql` around lines 4 - 5, The
mixed Clerk/Supabase policy checks are using raw auth.uid() comparisons, which
can fail for Clerk text-based subject IDs before the public.is_clerk_user(...)
branch is evaluated. Update the helper in this migration to use a safe
UUID-returning auth function and replace every auth.uid() usage in the affected
policies/functions with that helper so Clerk requests are handled consistently.
| CREATE POLICY "messages_owner_participant_access" ON public.messages | ||
| FOR ALL USING ( | ||
| user_id = auth.uid() | ||
| OR | ||
| public.is_clerk_user(user_id) | ||
| OR | ||
| EXISTS ( | ||
| SELECT 1 FROM public.chats | ||
| WHERE chats.id = messages.chat_id AND (chats.user_id = auth.uid() OR public.is_clerk_user(chats.user_id)) | ||
| ) | ||
| OR | ||
| EXISTS ( | ||
| SELECT 1 FROM public.chat_participants | ||
| WHERE chat_participants.chat_id = messages.chat_id AND (chat_participants.user_id = auth.uid() OR public.is_clerk_user(chat_participants.user_id)) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Split message read and write policies.
This FOR ALL policy lets any chat participant satisfy INSERT/UPDATE/DELETE checks through the participant EXISTS, regardless of messages.user_id. A participant can create or mutate messages attributed to another user. Keep participant access for SELECT, but require messages.user_id to match the current Supabase/Clerk user for INSERT/UPDATE/DELETE.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drizzle/migrations/0003_add_clerk_auth_helpers.sql` around lines 63 - 78,
Split the current messages_owner_participant_access policy in the messages
policy definition so participant-based access is only used for reading, not
writing. The existing FOR ALL policy in the messages policy block allows the
chat_participants EXISTS branch to satisfy INSERT/UPDATE/DELETE checks even when
messages.user_id does not match the current user; keep that participant logic
for SELECT access, but create separate write rules that require messages.user_id
to equal auth.uid() or the Clerk-backed user check before allowing
INSERT/UPDATE/DELETE.
| DROP POLICY IF EXISTS "Only owners can insert participants" ON public.chat_participants; | ||
| CREATE POLICY "Only owners can insert participants" ON public.chat_participants | ||
| FOR INSERT WITH CHECK ( | ||
| EXISTS ( | ||
| SELECT 1 FROM public.chat_participants cp | ||
| WHERE cp.chat_id = chat_participants.chat_id | ||
| AND (cp.user_id = auth.uid() OR public.is_clerk_user(cp.user_id)) | ||
| AND cp.role = 'owner' | ||
| ) | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== migration excerpt ==\n'
nl -ba drizzle/migrations/0003_add_clerk_auth_helpers.sql | sed -n '110,165p'
printf '\n== search related schema/policies ==\n'
rg -n "chat_participants|CREATE POLICY|public\.chats|is_clerk_user|owner" drizzle -g '!**/node_modules/**'Repository: QueueLab/QCX
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== migration excerpt around chat tables/helpers ==\n'
nl -ba drizzle/migrations/0003_add_clerk_auth_helpers.sql | sed -n '1,220p'
printf '\n== file list for migration/schema refs ==\n'
git ls-files drizzle | sed -n '1,120p'Repository: QueueLab/QCX
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== migration excerpt ==\n'
awk 'NR>=110 && NR<=165 {printf "%d:%s\n", NR, $0}' drizzle/migrations/0003_add_clerk_auth_helpers.sql
printf '\n== surrounding file search ==\n'
rg -n "chat_participants|CREATE POLICY|public\.chats|is_clerk_user|owner" drizzle
printf '\n== file list under drizzle ==\n'
git ls-files drizzle | head -200Repository: QueueLab/QCX
Length of output: 12098
Allow bootstrapping the first owner participant. This INSERT policy requires an existing owner row in public.chat_participants, so a new chat cannot add its initial owner. Add a bootstrap branch that allows the creator from public.chats.user_id.
🧰 Tools
🪛 SQLFluff (4.2.2)
[error] 134-134: Do not use special characters in identifiers.
(RF05)
[error] 135-135: Do not use special characters in identifiers.
(RF05)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drizzle/migrations/0003_add_clerk_auth_helpers.sql` around lines 134 - 143,
The INSERT policy for chat participants currently depends on an existing owner
row in public.chat_participants, which blocks creating the first owner. Update
the policy in the chat_participants migration to special-case the bootstrap
insert by allowing the creator identified by public.chats.user_id to insert the
initial owner participant for that chat. Keep the existing owner-check logic for
subsequent inserts, and reference the policy definition around the "Only owners
can insert participants" policy when making the change.
| import { Pool, type PoolConfig } from 'pg'; // Uses Pool from pg, import PoolConfig | ||
| import * as dotenv from 'dotenv'; | ||
| import { Pool, type PoolConfig } from 'pg'; | ||
| import { parse } from 'pg-connection-string'; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify pg-connection-string is declared in package manifests.
fd -a '^package\.json$' -x sh -c '
for f do
echo "== $f"
jq -r '"'"'{
dependencies: .dependencies["pg-connection-string"],
devDependencies: .devDependencies["pg-connection-string"],
peerDependencies: .peerDependencies["pg-connection-string"]
}'"'"' "$f"
done
' sh {}Repository: QueueLab/QCX
Length of output: 267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## manifests"
fd -a '^package\.json$' . | sort
echo
echo "## declaration scan"
for f in $(fd -a '^package\.json$' . | sort); do
if jq -e '.dependencies["pg-connection-string"] or .devDependencies["pg-connection-string"] or .peerDependencies["pg-connection-string"]' "$f" >/dev/null; then
echo "FOUND in $f"
jq -r '{file:"'"$f"'", dep:.dependencies["pg-connection-string"], dev:.devDependencies["pg-connection-string"], peer:.peerDependencies["pg-connection-string"]}' "$f"
fi
done
echo
echo "## import scan"
rg -n "pg-connection-string|from 'pg-connection-string'|from \"pg-connection-string\"" lib package.json . -g '!node_modules' -g '!dist' -g '!build' || true
echo
echo "## relevant files"
for f in lib/db/index.ts lib/db/migrate.ts; do
if [ -f "$f" ]; then
echo "== $f =="
cat -n "$f" | sed -n '1,120p'
fi
doneRepository: QueueLab/QCX
Length of output: 5190
Declare pg-connection-string directly. lib/db/index.ts and lib/db/migrate.ts import it at runtime, but it is not listed in the root package.json; relying on pg’s transitive copy can break under pnpm, PnP, or other non-hoisted installs. Add it to production dependencies.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/db/index.ts` at line 3, Add pg-connection-string to the root production
dependencies because lib/db/index.ts and lib/db/migrate.ts import it directly at
runtime. Update package.json so the dependency is declared explicitly, and keep
the existing imports using parse from pg-connection-string unchanged.
| if (!process.env.DATABASE_URL && process.env.NODE_ENV === 'production') { | ||
| console.warn('DATABASE_URL environment variable is not set. Database features will be unavailable.'); | ||
| } | ||
|
|
||
| const connectionString = process.env.DATABASE_URL || 'postgres://localhost:5432/postgres'; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'lib/db/index.ts' 'lib/db/**' 'package.json' 'next.config.*' 'app/**' 'pages/**' 'src/**' | sed -n '1,200p'
echo '--- lib/db/index.ts ---'
cat -n lib/db/index.ts | sed -n '1,120p'
echo '--- search DATABASE_URL usage ---'
rg -n "DATABASE_URL|postgres://localhost:5432/postgres|connectionString|db/index" lib src app pages . --glob '!node_modules' --glob '!dist' --glob '!build'
echo '--- package.json ---'
cat -n package.json | sed -n '1,220p'Repository: QueueLab/QCX
Length of output: 5459
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- next.config.mjs ---'
cat -n next.config.mjs | sed -n '1,220p'
echo '--- package.json ---'
cat -n package.json | sed -n '1,240p'
echo '--- db import sites ---'
rg -n "from '.*lib/db|from \" .*lib/db|from './db|from '../db|from '`@/lib/db`|from 'lib/db|from \"lib/db" app lib --glob '!node_modules' --glob '!dist' --glob '!build'
echo '--- app/api/chat/route.ts ---'
cat -n app/api/chat/route.ts | sed -n '1,220p'
echo '--- app/actions.tsx ---'
cat -n app/actions.tsx | sed -n '1,260p'Repository: QueueLab/QCX
Length of output: 22015
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- lib/actions/chat.ts ---'
cat -n lib/actions/chat.ts | sed -n '1,260p'
echo '--- lib/actions/chat-db.ts ---'
cat -n lib/actions/chat-db.ts | sed -n '1,260p'
echo '--- lib/auth/get-current-user.ts ---'
cat -n lib/auth/get-current-user.ts | sed -n '1,220p'
echo '--- lib/actions/users.ts ---'
cat -n lib/actions/users.ts | sed -n '1,220p'
echo '--- lib/actions/system-prompt-db.ts ---'
cat -n lib/actions/system-prompt-db.ts | sed -n '1,220p'
echo '--- lib/actions/calendar.ts ---'
cat -n lib/actions/calendar.ts | sed -n '1,240p'Repository: QueueLab/QCX
Length of output: 37522
Do not let production runtime fall back to localhost
lib/db/index.ts is imported by server actions and API routes, so if DATABASE_URL is missing in production this code will still create a pool pointed at postgres://localhost:5432/postgres and run real queries there. Make production fail closed instead of silently targeting a local/stale database.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/db/index.ts` around lines 8 - 12, In lib/db/index.ts, the DATABASE_URL
handling currently falls back to a localhost connection even when NODE_ENV is
production, which can let server actions and API routes run real queries against
the wrong database. Update the module-level connection setup around
connectionString so production fails closed: when process.env.NODE_ENV is
production and DATABASE_URL is missing, stop initialization by throwing an error
(or otherwise preventing pool creation) instead of defaulting to
postgres://localhost:5432/postgres. Keep the existing warning behavior only for
non-production environments, and ensure the fix is applied in the db
initialization path that creates the pool.
| // Conditionally apply SSL for Supabase URLs | ||
| if (process.env.DATABASE_URL && process.env.DATABASE_URL.includes('supabase.co')) { | ||
| // Conditionally apply SSL for Supabase URLs if not already specified in the connection string | ||
| if (connectionString.includes('supabase.co') && !poolConfig.ssl) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File outline ==\n'
ast-grep outline lib/db/index.ts --view expanded || true
printf '\n== Relevant lines in lib/db/index.ts ==\n'
cat -n lib/db/index.ts | sed -n '1,120p'
printf '\n== Search for ssl parsing / poolConfig.ssl usage ==\n'
rg -n "poolConfig\.ssl|ssl\s*===" lib/db/index.ts lib -g '!**/node_modules/**' || trueRepository: QueueLab/QCX
Length of output: 2836
🌐 Web query:
pg-connection-string parse ssl false undefined sslmode disable source
💡 Result:
In the pg-connection-string library, the use of ssl=false in a connection string can lead to unexpected behavior because the library parses the "false" value as a string rather than a boolean [1]. Since many database clients (like knex) expect the ssl property to be a boolean, passing ssl=false can result in the library interpreting it as truthy, sometimes causing the client to incorrectly attempt an SSL connection [1]. The authoritative and recommended way to disable SSL in a connection string when using pg-connection-string is to use the sslmode parameter [2]. By setting sslmode=disable in your connection string (e.g., postgres://user:password@host/db?sslmode=disable), the library will correctly interpret this and set the ssl configuration to false [3][4]. This approach aligns with standard PostgreSQL libpq connection URI behavior, whereas ssl=false is not a standard libpq parameter and can cause errors or inconsistencies [2]. Summary of recommendations: 1. Avoid using ssl=false in connection strings [2]. 2. Use sslmode=disable to explicitly disable SSL [3][2]. 3. Ensure your client library supports the parsed object output from pg-connection-string, as some drivers may require explicit configuration if they do not automatically map sslmode to the appropriate connection settings [1][5].
Citations:
- 1: Unable to connect to PostgreSQL database using pg-connection-string and not using SSL knex/knex#5438
- 2: Fix parsing of ssl=false connection string parameter brianc/node-postgres#2423
- 3: https://github.com/brianc/node-postgres/blob/master/packages/pg-connection-string/README.md
- 4: https://registry.npmjs.org/pg-connection-string
- 5: Postgres
sslmodeparameter support brianc/node-postgres#1949
🌐 Web query:
pg-connection-string README sslmode disable parse output ssl boolean false
💡 Result:
In the pg-connection-string library, setting sslmode=disable in a connection string correctly parses the ssl property to the boolean false [1][2][3]. When parsing a PostgreSQL connection string, the library interprets various query parameters to configure the SSL settings [2][4]. Specifically, for the sslmode parameter: - When uselibpqcompat is not set: - sslmode=disable sets the ssl property to false [3][5]. - sslmode=no-verify sets the ssl property to { rejectUnauthorized: false } [3][4]. - Other modes (prefer, require, verify-ca, verify-full) set the ssl property to true [3][6]. - When uselibpqcompat=true is used: - sslmode=disable also sets the ssl property to false [2][7]. Additionally, you can explicitly set the ssl status using the ssl query parameter directly (e.g., ssl=0 or ssl=false), which also forces the ssl property to be false [2][8].
Citations:
- 1: https://github.com/brianc/node-postgres/blob/master/packages/pg-connection-string/README.md
- 2: https://www.npmjs.com/package/pg-connection-string
- 3: https://github.com/brianc/node-postgres/tree/master/packages/pg-connection-string
- 4: https://npmx.dev/package/pg-connection-string/v/%5E2.13.0
- 5: https://registry.npmjs.org/pg-connection-string
- 6: https://npmx.dev/package/pg-connection-string/v/2.12.0
- 7: https://jspm-packages.deno.dev/package/pg-connection-string@2.13.0
- 8: https://github.com/brianc/node-postgres/blob/95d7e620ef8b51743b4cbca05dd3c3ce858ecea7/packages/pg-connection-string/README.md
Respect explicit SSL settings from the connection string (lib/db/index.ts:31).
poolConfig.ssl can be false for sslmode=disable or ssl=false, so !poolConfig.ssl treats an explicit opt-out as unset and overwrites it for Supabase URLs. Check for undefined instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/db/index.ts` at line 31, The SSL handling in the database connection
setup is treating an explicit false as if SSL was unset. In lib/db/index.ts
within the connection logic that checks Supabase URLs, update the condition
around poolConfig.ssl so it only applies the Supabase SSL override when ssl is
undefined, not when it was explicitly set to false. Keep the existing behavior
in the connectionString-based branch and preserve the caller’s sslmode/ssl=false
intent.
| const parsedConfig = parse(connectionString); | ||
|
|
||
| const pool = new Pool({ | ||
| connectionString: process.env.DATABASE_URL, | ||
| ssl: { | ||
| rejectUnauthorized: false, // Ensure this is appropriate for your Supabase connection | ||
| }, | ||
| // max: 1, // Optional: restrict to 1 connection for migration | ||
| host: parsedConfig.host || undefined, | ||
| port: parsedConfig.port ? parseInt(parsedConfig.port, 10) : undefined, | ||
| user: parsedConfig.user || undefined, | ||
| password: parsedConfig.password || undefined, | ||
| database: parsedConfig.database || undefined, | ||
| ssl: connectionString.includes('supabase.co') ? { rejectUnauthorized: false } : undefined, | ||
| max: 1, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'lib/db/*' 'lib/db/migrate.ts' 'lib/db/index.ts'
printf '\n--- migrate.ts ---\n'
cat -n lib/db/migrate.ts
printf '\n--- index.ts ---\n'
cat -n lib/db/index.ts
printf '\n--- search for parse(...) and ssl handling ---\n'
rg -n "parse\\(|sslmode|rejectUnauthorized|ssl:" lib/dbRepository: QueueLab/QCX
Length of output: 4659
Preserve parsed SSL settings in migrations. lib/db/migrate.ts drops parsedConfig.ssl, so URLs that rely on sslmode=require, certificates, or sslmode=no-verify can behave differently from lib/db/index.ts and fail during migrations. Use parsedConfig.ssl first, then apply the Supabase fallback only when SSL isn’t set.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/db/migrate.ts` around lines 15 - 24, The migration pool setup in
migrate.ts is ignoring parsedConfig.ssl, which can make migration connections
behave differently from the main database path. Update the Pool configuration in
the migration logic to prefer parsedConfig.ssl from parse(connectionString), and
only apply the Supabase ssl fallback when parsedConfig.ssl is not present; use
the existing migrate.ts Pool construction and the same connectionString parsing
flow to keep SSL behavior consistent with lib/db/index.ts.
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
Blocking feedback
0003_add_clerk_auth_helpers.sqlreferencesusers.clerk_user_id, but there is no earlier migration in this repo that adds that column; this can break the migration chain on fresh/reset databases — drizzle/.../0003_add_clerk_auth_helpers.sql#L13- The
chat_participantspolicies self-querypublic.chat_participants(same pattern at lines 126/138/150/162), which causes recursive RLS evaluation and can fail withinfinite recursion detected in policy for relation "chat_participants"— drizzle/.../0003_add_clerk_auth_helpers.sql#L126
If you want me to push fixes, reply with the item numbers you want addressed (for example: please fix 1-2).
| RETURNS boolean AS $$ | ||
| SELECT EXISTS ( | ||
| SELECT 1 FROM public.users | ||
| WHERE id = user_id AND clerk_user_id = public.clerk_id() |
There was a problem hiding this comment.
is_clerk_user depends on users.clerk_user_id, but this migration set does not include a prior ALTER TABLE users ADD COLUMN clerk_user_id .... On fresh/reset databases, this reference can fail during migration and block deploys.
Suggested fix: add a migration step before this function to create clerk_user_id (and the intended uniqueness constraint/index), or ensure a guaranteed earlier migration provides it.
| CREATE POLICY "Only owners can delete participants" ON public.chat_participants | ||
| FOR DELETE USING ( | ||
| EXISTS ( | ||
| SELECT 1 FROM public.chat_participants cp |
There was a problem hiding this comment.
This policy queries public.chat_participants from inside a policy on public.chat_participants. PostgreSQL applies RLS to that subquery too, which can recurse and fail with infinite recursion detected in policy for relation "chat_participants" (same self-reference pattern appears again in the INSERT/UPDATE/SELECT policies below).
Suggested fix: move the ownership/membership lookup into a hardened SECURITY DEFINER helper owned by a role that bypasses RLS (or otherwise avoid self-referencing this table inside its own policies).
1 participant
This PR addresses the "Error loading chat history unauthorized" issue by resolving the authentication mismatch between Clerk and Supabase and fixing a database connection error specific to Next.js 15.
Key Changes
lib/db/index.tsandlib/db/migrate.tsto manually parseDATABASE_URLusingpg-connection-string. This avoids theTypeError: Cannot read properties of undefined (reading 'searchParams')caused by thepgdriver's internal URL parsing in Next.js 15.public.clerk_id()andpublic.is_clerk_user()helper functions to the Supabase database.users,chats,messages, etc.) to authorize users based on their Clerk identity extracted from the JWT.lib/auth/get-current-user.tswith better error handling and logging.0003_add_clerk_auth_helpers.sqlto track the database schema changes.These changes ensure that authenticated Clerk users can correctly store and retrieve their chat history while maintaining security via Supabase RLS.
PR created automatically by Jules for task 5012417053378019399 started by @ngoiyaeric
Summary by CodeRabbit
New Features
Bug Fixes
Chores