🛡️ Sentinel: [CRITICAL] Fix SQL injection in database-wrapper

.jules/sentinel.md

@@ -7,3 +7,7 @@
 **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.
+## 2024-07-28 - [CRITICAL] SQL Injection Risk via sql.raw in jsonExtract function
+**Vulnerability:** A SQL injection vulnerability existed in `packages/core/src/db/database-wrapper.ts` where `jsonExtract` used `sql.raw` to construct JSON path expressions (e.g. `->>'key'`). By interpolating string values into `sql.raw`, it bypassed Drizzle's parameterization, exposing the app to SQL injection if user input were passed as a path key.
+**Learning:** Using `sql.raw()` to interpolate dynamic string variables bypasses parameterized queries. For PostgreSQL JSON operators, Drizzle requires explicit type casting of dynamic variables to resolve the overloaded operator type without relying on `sql.raw()`.
+**Prevention:** In Drizzle ORM, never use `sql.raw()` to interpolate dynamic variables. To prevent SQL injection when working with PostgreSQL JSON operators (`->`, `->>`), always explicitly cast dynamic parameters to text (e.g., `sql\`->> (${key}::text)\``) to properly resolve the overloaded operator type. Use `sql.join` to safely combine these parameterized template literals when chaining multi-level paths.

packages/core/src/db/database-wrapper.ts

@@ -105,15 +105,15 @@ 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
+    // IMPORTANT: Use explicit text casting for JSON keys to allow parameterization while properly resolving the overloaded operator type
     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)`;
     }
   }
 }

packages/core/test-drizzle.ts

@@ -0,0 +1,9 @@
+import { sql } from 'drizzle-orm';
+import { PgDialect } from 'drizzle-orm/pg-core';
+
+const dialect = new PgDialect();
+const column = sql`my_column`;
+const p = "user's key"; // testing SQL injection
+const result = sql`${column} -> (${p}::text)`;
+const query = dialect.sqlToQuery(result);
+console.log(query);

test-drizzle-2.ts

@@ -0,0 +1,11 @@
+import { sql } from 'drizzle-orm';
+import { PgDialect } from 'drizzle-orm/pg-core';
+
+const dialect = new PgDialect();
+const column = sql`my_column`;
+const parts = ["user's key", 'another key'];
+const objectParts = parts.slice(0, -1).map((p) => sql`->(${p}::text)`);
+const lastPart = parts[parts.length - 1];
+const result = sql`${column}${sql.join(objectParts, sql``)}->>(${lastPart}::text)`;
+const query = dialect.sqlToQuery(result);
+console.log(query);

test-drizzle-3.ts

@@ -0,0 +1,20 @@
+import { sql } from 'drizzle-orm';
+import { PgDialect } from 'drizzle-orm/pg-core';
+import { SQLiteSyncDialect } from 'drizzle-orm/sqlite-core';
+
+const pgDialect = new PgDialect();
+const sqliteDialect = new SQLiteSyncDialect();
+
+const column = sql`my_column`;
+const path = "path.to.my'key";
+const parts = path.split('.');
+
+console.log('Postgres 1 part:');
+const p1Result = sql`${column}->>(${parts[0]}::text)`;
+console.log(pgDialect.sqlToQuery(p1Result));
+
+console.log('Postgres multi parts:');
+const objectParts = parts.slice(0, -1).map((p) => sql`->(${p}::text)`);
+const lastPart = parts[parts.length - 1];
+const p2Result = sql`${column}${sql.join(objectParts, sql``)}->>(${lastPart}::text)`;
+console.log(pgDialect.sqlToQuery(p2Result));

test-drizzle.ts

@@ -0,0 +1,9 @@
+import { sql } from 'drizzle-orm';
+import { PgDialect } from 'drizzle-orm/pg-core';
+
+const dialect = new PgDialect();
+const column = sql`my_column`;
+const p = 'key1';
+const result = sql`${column} -> (${p}::text)`;
+const query = dialect.sqlToQuery(result);
+console.log(query);