Skip to content

Commit 8999b78

Browse files
Add helpers to prevent SQL injection for trigger when clauses.
1 parent 680d8a2 commit 8999b78

File tree

4 files changed

+98
-15
lines changed

4 files changed

+98
-15
lines changed

packages/common/src/client/triggers/TriggerManager.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,16 @@ interface BaseCreateDiffTriggerOptions {
140140
* For example, you can use it to only trigger on certain values in the NEW row.
141141
* Note that for PowerSync the row data is stored in a JSON column named `data`.
142142
* The row id is available in the `id` column.
143+
*
144+
* NB! The WHEN clauses here added added directly to the SQLite trigger creation SQL.
145+
* Any user input strings here should be sanitized externally. The {@link when} string template function performs
146+
* some basic sanitization, extra external sanitization is recommended.
147+
*
143148
* @example
144149
* {
145-
* 'INSERT': `json_extract(NEW.data, '$.list_id') = 'abcd'`
146-
* 'UPDATE': `NEW.id = 'abcd' AND json_extract(NEW.data, '$.status') = 'active'`
147-
* 'DELETE': `json_extract(OLD.data, '$.list_id') = 'abcd'`
150+
* 'INSERT': whenClause`json_extract(NEW.data, '$.list_id') = ${sanitizeUUID(list.id)}`
151+
* 'UPDATE': whenClause`NEW.id = 'abcd' AND json_extract(NEW.data, '$.status') = 'active'`
152+
* 'DELETE': whenClause`json_extract(OLD.data, '$.list_id') = 'abcd'`
148153
* }
149154
*/
150155
when?: Partial<Record<DiffTriggerOperation, string>>;
@@ -310,7 +315,7 @@ export interface TriggerManager {
310315
* source: 'todos',
311316
* columns: ['list_id'],
312317
* when: {
313-
* [DiffTriggerOperation.INSERT]: "json_extract(NEW.data, '$.list_id') = '123'"
318+
* [DiffTriggerOperation.INSERT]: whenClause`json_extract(NEW.data, '$.list_id') = '123'`
314319
* },
315320
* operations: [DiffTriggerOperation.INSERT],
316321
* onChange: async (context) => {
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
function sanitizeString(input: string): string {
2+
return `'${input.replace(/'/g, "''")}'`;
3+
}
4+
/**
5+
* Helper function for sanitizing UUID input strings.
6+
* Typically used with {@link whenClause}.
7+
*/
8+
export function sanitizeUUID(uuid: string): string {
9+
const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
10+
const isValid = uuidRegex.test(uuid);
11+
if (!isValid) {
12+
throw new Error(`${uuid} is not a valid UUID`);
13+
}
14+
return uuid;
15+
}
16+
17+
/**
18+
* SQL string template function for {@link TrackDiffOptions#when} and {@link CreateDiffTriggerOptions#when}.
19+
*
20+
* This function performs basic string interpolation for SQLite WHEN clauses.
21+
*
22+
* **String placeholders:**
23+
* - All string values passed as placeholders are automatically wrapped in single quotes (`'`).
24+
* - Any single quotes within the string value are escaped by doubling them (`''`), as required by SQL syntax.
25+
* - Do not manually wrap placeholders in single quotes in your template string; the function will handle quoting and escaping for you.
26+
*
27+
* **Other types:**
28+
* - `null` and `undefined` are converted to SQL `NULL`.
29+
* - Objects are stringified using `JSON.stringify()` and wrapped in single quotes, with any single quotes inside the stringified value escaped.
30+
* - Numbers and other primitive types are inserted directly.
31+
*
32+
* **Usage example:**
33+
* ```typescript
34+
* const myID = "O'Reilly";
35+
* const clause = whenClause`New.id = ${myID}`;
36+
* // Result: "New.id = 'O''Reilly'"
37+
* ```
38+
*
39+
* Avoid manually quoting placeholders:
40+
* ```typescript
41+
* // Incorrect:
42+
* whenClause`New.id = '${myID}'` // Produces double quotes: New.id = ''O''Reilly''
43+
* ```
44+
*/
45+
export function whenClause(strings: TemplateStringsArray, ...values: any[]): string {
46+
let result = '';
47+
strings.forEach((str, i) => {
48+
result += str;
49+
if (i < values.length) {
50+
// For SQL, escape single quotes in string values
51+
const value = values[i];
52+
if (typeof value === 'string') {
53+
result += sanitizeString(value);
54+
} else if (value === null || value === undefined) {
55+
result += 'NULL';
56+
} else if (typeof value == 'object') {
57+
// Stringify the object and escape single quotes in the result
58+
const stringified = JSON.stringify(value);
59+
result += sanitizeString(stringified);
60+
} else {
61+
result += value;
62+
}
63+
}
64+
});
65+
return result;
66+
}

packages/common/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export * from './db/schema/Table.js';
3232
export * from './db/schema/TableV2.js';
3333

3434
export * from './client/triggers/TriggerManager.js';
35+
export * from './client/triggers/whenClause.js';
3536

3637
export * from './utils/AbortOperation.js';
3738
export * from './utils/BaseObserver.js';

packages/node/tests/trigger.test.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
1-
import { DiffTriggerOperation, ExtractedTriggerDiffRecord, TriggerDiffRecord } from '@powersync/common';
2-
// import 'source-map-support/register';
1+
import {
2+
DiffTriggerOperation,
3+
ExtractedTriggerDiffRecord,
4+
sanitizeUUID,
5+
TriggerDiffRecord,
6+
whenClause
7+
} from '@powersync/common';
38
import { describe, expect, vi } from 'vitest';
49
import { Database, databaseTest } from './utils';
510

@@ -111,7 +116,9 @@ describe('Triggers', () => {
111116
await database.triggers.trackTableDiff({
112117
source: 'todos',
113118
columns: ['list_id'],
114-
when: { [DiffTriggerOperation.INSERT]: `json_extract(NEW.data, '$.list_id') = '${firstList.id}'` },
119+
when: {
120+
[DiffTriggerOperation.INSERT]: whenClause`json_extract(NEW.data, '$.list_id') = ${sanitizeUUID(firstList.id)}`
121+
},
115122
operations: [DiffTriggerOperation.INSERT],
116123
onChange: async (context) => {
117124
// Fetches the current state of todo records that were inserted during this diff window.
@@ -189,7 +196,7 @@ describe('Triggers', () => {
189196
*/
190197
await database.triggers.trackTableDiff({
191198
source: 'lists',
192-
when: { [DiffTriggerOperation.UPDATE]: `NEW.id = '${list.id}'` },
199+
when: { [DiffTriggerOperation.UPDATE]: whenClause`NEW.id = ${sanitizeUUID(list.id)}` },
193200
operations: [DiffTriggerOperation.UPDATE, DiffTriggerOperation.DELETE],
194201
onChange: async (context) => {
195202
// Fetches the todo records that were inserted during this diff
@@ -298,7 +305,9 @@ describe('Triggers', () => {
298305
await database.triggers.trackTableDiff({
299306
source: 'todos',
300307
columns: ['list_id'],
301-
when: { [DiffTriggerOperation.INSERT]: `json_extract(NEW.data, '$.list_id') = '${firstList.id}'` },
308+
when: {
309+
[DiffTriggerOperation.INSERT]: whenClause`json_extract(NEW.data, '$.list_id') = ${sanitizeUUID(firstList.id)}`
310+
},
302311
operations: [DiffTriggerOperation.INSERT],
303312
onChange: async (context) => {
304313
// Fetches the todo records that were inserted during this diff
@@ -342,11 +351,11 @@ describe('Triggers', () => {
342351
async () => {
343352
expect(todos.length).toEqual(todoCreationCount);
344353
},
345-
{ timeout: 10000, interval: 1000 }
354+
{ timeout: 1000, interval: 100 }
346355
);
347356
});
348357

349-
databaseTest('Should extract diff values', { timeout: 10000 }, async ({ database }) => {
358+
databaseTest('Should extract diff values', async ({ database }) => {
350359
await database.execute(
351360
/* sql */ `
352361
INSERT INTO
@@ -386,7 +395,9 @@ describe('Triggers', () => {
386395
// The onChange handler is guaranteed to see any change after the state above.
387396
await database.triggers.trackTableDiff({
388397
source: 'todos',
389-
when: { [DiffTriggerOperation.INSERT]: `json_extract(NEW.data, '$.list_id') = '${firstList.id}'` },
398+
when: {
399+
[DiffTriggerOperation.INSERT]: whenClause`json_extract(NEW.data, '$.list_id') = ${sanitizeUUID(firstList.id)}`
400+
},
390401
operations: [DiffTriggerOperation.INSERT],
391402
onChange: async (context) => {
392403
// Fetches the content of the records at the time of the operation
@@ -416,11 +427,11 @@ describe('Triggers', () => {
416427
expect(changes.map((c) => c.content)).toEqual(['todo 1', 'todo 2', 'todo 3']);
417428
expect(changes.every((c) => c.operation === DiffTriggerOperation.INSERT)).toBeTruthy();
418429
},
419-
{ timeout: 10000, interval: 1000 }
430+
{ timeout: 1000, interval: 100 }
420431
);
421432
});
422433

423-
databaseTest('Should allow tracking 0 columns', { timeout: 10000 }, async ({ database }) => {
434+
databaseTest('Should allow tracking 0 columns', { timeout: 1000 }, async ({ database }) => {
424435
/**
425436
* Tracks the ids of todos reported via the trigger
426437
*/
@@ -491,7 +502,7 @@ describe('Triggers', () => {
491502
async () => {
492503
expect(changes).toEqual(ids);
493504
},
494-
{ timeout: 10000, interval: 1000 }
505+
{ timeout: 1000, interval: 100 }
495506
);
496507
});
497508
});

0 commit comments

Comments
 (0)