From dbbafac5e7c695f5e2fb82e8c6d85aab8c1e067e Mon Sep 17 00:00:00 2001 From: Donach <39565367+Donach@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:29:02 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20SQL=20injection=20risk=20in=20JSON=20extraction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes a potential SQL injection vulnerability in `jsonExtract` located inside `packages/core/src/db/database-wrapper.ts`. The original implementation used `sql.raw` to construct dynamic JSON path string accesses in PostgreSQL, which bypassed parameterization and made it vulnerable to SQL injection. The fix explicitly casts dynamic parameters to `text` using `::text` along with proper interpolation via Drizzle ORM's `sql` template literal string syntax (e.g., `sql\`->> (${key}::text)\``). This maintains safe query parameterization while avoiding Postgres operator overload ambiguity. Also added an entry in `.jules/sentinel.md` outlining the findings and required secure practices. --- .jules/sentinel.md | 5 +++++ packages/core/src/db/database-wrapper.ts | 7 +++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 3a5ef27423..e26064f552 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -7,3 +7,8 @@ **Vulnerability:** The daemon configuration file (`~/.agor/config.yaml`) and its parent directory (`~/.agor`) were created with default file permissions (e.g., `0o755`/`0o644`), which made them readable by other users on the system. This file stores extremely sensitive information such as API keys and master JWT secrets. **Learning:** Default Node.js filesystem operations (`fs.writeFile` and `fs.mkdir`) do not enforce strict permissions unless explicitly specified with a `mode` parameter. When handling sensitive files, relying on the system `umask` is insufficient. **Prevention:** Always specify `mode: 0o600` for sensitive files and `mode: 0o700` for their parent directories. Additionally, use `fs.chmod` to retroactively secure existing files and directories that might have been created with permissive defaults. + +## 2025-05-14 - [SQL Injection Risk via sql.raw in PostgreSQL JSON extraction] +**Vulnerability:** A potential SQL injection vulnerability in `jsonExtract` inside `packages/core/src/db/database-wrapper.ts`. The code used `sql.raw` to construct JSON path access string dynamically. +**Learning:** Using `sql.raw` with unparameterized variables makes the app susceptible to SQL injection. When generating dynamic JSON access paths for PostgreSQL `->` and `->>` operators, explicit cast to text (e.g. `sql\`->> (${key}::text)\``) is required so that Drizzle correctly handles the overloaded JSON operator without syntax errors, allowing us to safely parameterize paths. +**Prevention:** Never use `sql.raw()` to interpolate dynamic variables. Always cast dynamic parameters to text (`::text`) when using Postgres JSON operators to maintain parameterization while avoiding operator overload ambiguity. diff --git a/packages/core/src/db/database-wrapper.ts b/packages/core/src/db/database-wrapper.ts index d52d553e64..0889d0b059 100644 --- a/packages/core/src/db/database-wrapper.ts +++ b/packages/core/src/db/database-wrapper.ts @@ -105,15 +105,14 @@ export function jsonExtract(db: Database, column: SQL.Aliased | SQL | any, path: } else { // PostgreSQL: column->'path'->'to'->>'field' // Use -> for all but the last part (keeps as JSON), ->> for the last part (extracts as text) - // IMPORTANT: Use sql.raw() for JSON keys to avoid parameterization if (parts.length === 1) { // Single level: column->>'key' - return sql`${column}${sql.raw(`->>'${parts[0]}'`)}`; + return sql`${column}->>(${parts[0]}::text)`; } else { // Multiple levels: column->'key1'->'key2'->>'key3' - const objectParts = parts.slice(0, -1).map((p) => sql.raw(`->'${p}'`)); + const objectParts = parts.slice(0, -1).map((p) => sql`->(${p}::text)`); const lastPart = parts[parts.length - 1]; - return sql`${column}${sql.join(objectParts, sql``)}${sql.raw(`->>'${lastPart}'`)}`; + return sql`${column}${sql.join(objectParts, sql``)}->>(${lastPart}::text)`; } } }