🛡️ Sentinel: [CRITICAL] Fix SQL Injection Risk in JSON Path Extraction - #155
🛡️ Sentinel: [CRITICAL] Fix SQL Injection Risk in JSON Path Extraction#155Donach wants to merge 1 commit into
Conversation
The `jsonExtract` helper used `sql.raw()` to interpolate dynamic JSON path arguments into queries. This completely bypassed Drizzle ORM's parameterization and introduced a severe SQL Injection risk when executing complex searches against JSON fields. This patch replaces `sql.raw` with standard `sql` string parameterization, explicitly casting the variables as PostgreSQL text fragments (`::text`) prior to building the traversal path, satisfying both Drizzle's static checking and preventing query manipulation.
|
👋 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. |
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: The
jsonExtractdatabase helper in@agor/core/db/database-wrapper.tsdynamically constructed JSON path strings for queries. Instead of properly parameterizing the variables using Drizzle'ssqlstring literal tag, it concatenated them manually usingsql.raw(). This completely evaded SQL injection protections.🎯 Impact: Any endpoint allowing a client to supply dynamic JSON dot paths (like nested filtering configurations or sorting orders) could be manipulated to terminate the JSON selection string early and inject arbitrary SQL payload logic.
🔧 Fix: Removed
sql.raw()fromjsonExtractcompletely. Reimplemented the PostgreSQL path construction using explicit parameterized array joins and text casting (::text), matching standard Drizzle best practices without sacrificing functionality.✅ Verification:
pnpm testandpnpm lintpassed successfully without regressions. Tested explicitly with thetest-drizzle.tsquery rendering script to confirm the output is parameterized (e.g.-> ($1::text) ->> ($2::text)).PR created automatically by Jules for task 15461631222607097162 started by @Donach