Skip to content

Commit 9359734

Browse files
authored
fix(v10/core): Summarize SQLite upserts so Durable Object cf_ spans stay filtered (#22766)
Backport of: #22649
1 parent c366115 commit 9359734

3 files changed

Lines changed: 61 additions & 2 deletions

File tree

packages/cloudflare/test/utils/internalSqlQuery.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ describe('targetsCloudflareInternalTable', () => {
2020
['cf_ai_ prefix', 'INSERT INTO cf_ai_chat_stream_chunks (id) VALUES (?)'],
2121
['cf_mcp_ prefix', 'SELECT * FROM cf_mcp_agent_event'],
2222
['schema version', 'SELECT version FROM cf_schema_version'],
23+
// SQLite upsert forms used by the agents framework for state/schedule/MCP persistence
24+
['INSERT OR REPLACE', 'INSERT OR REPLACE INTO cf_agents_state (id, state) VALUES (?, ?)'],
25+
[
26+
'INSERT OR REPLACE with column list',
27+
`INSERT OR REPLACE INTO cf_agents_mcp_servers ( id, name, server_url, client_id, auth_url,
28+
callback_url, server_options )
29+
VALUES ( ?, ?, ?, ?, ?, ?, ? )`,
30+
],
31+
['INSERT OR IGNORE', 'INSERT OR IGNORE INTO cf_agents_sub_agents (class, name) VALUES (?, ?)'],
32+
['REPLACE INTO', 'REPLACE INTO cf_agents_queues (id, payload) VALUES (?, ?)'],
33+
['UPDATE OR REPLACE', 'UPDATE OR REPLACE cf_agents_state SET state = ? WHERE id = ?'],
2334
])('returns true for %s on internal tables', (_label, query) => {
2435
expect(targetsCloudflareInternalTable(summarize(query))).toBe(true);
2536
});
@@ -55,6 +66,9 @@ describe('targetsCloudflareInternalTable', () => {
5566
['CREATE TABLE', 'CREATE TABLE users (id TEXT PRIMARY KEY)'],
5667
['table with cf in the middle', 'SELECT * FROM my_cf_table'],
5768
['table starting with cfg', 'SELECT * FROM cfg_settings'],
69+
['INSERT OR REPLACE', 'INSERT OR REPLACE INTO users (id, name) VALUES (?, ?)'],
70+
['REPLACE INTO', 'REPLACE INTO sessions (id, token) VALUES (?, ?)'],
71+
['UPDATE OR IGNORE', 'UPDATE OR IGNORE products SET price = ? WHERE id = ?'],
5872
])('returns false for %s on user tables', (_label, query) => {
5973
expect(targetsCloudflareInternalTable(summarize(query))).toBe(false);
6074
});
@@ -69,6 +83,14 @@ describe('targetsCloudflareInternalTable', () => {
6983
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_reports_daily'), [/^cf_reports_/])).toBe(false);
7084
});
7185

86+
it('returns false for an allowlisted table targeted by an upsert', () => {
87+
expect(
88+
targetsCloudflareInternalTable(summarize('INSERT OR REPLACE INTO cf_my_table (id) VALUES (?)'), [
89+
'cf_my_table',
90+
]),
91+
).toBe(false);
92+
});
93+
7294
it('requires an exact match for string entries', () => {
7395
// Substring matches must not opt a table back in, otherwise `cf_` would allowlist everything.
7496
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_agents_state'), ['cf_agents'])).toBe(true);

packages/core/src/utils/sql.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,18 @@ const DDL_RE = new RegExp(
88
'i',
99
);
1010

11-
const INSERT_RE = new RegExp(`^\\s*(?<operation>INSERT)\\s+INTO\\s+(?<table>${TABLE_NAME})`, 'i');
12-
const UPDATE_RE = new RegExp(`^\\s*(?<operation>UPDATE)\\s+(?<table>${TABLE_NAME})`, 'i');
11+
// SQLite upserts insert an optional conflict clause between operation and INTO
12+
// (`INSERT OR REPLACE INTO`, https://sqlite.org/lang_insert.html), with `REPLACE INTO` as the
13+
// standalone shorthand. The clause is filler like INTO — stripping it keeps upserts on the same
14+
// low-cardinality summary as plain inserts.
15+
const INSERT_RE = new RegExp(
16+
`^\\s*(?<operation>INSERT|REPLACE)(?:\\s+OR\\s+(?:ROLLBACK|ABORT|FAIL|IGNORE|REPLACE))?\\s+INTO\\s+(?<table>${TABLE_NAME})`,
17+
'i',
18+
);
19+
const UPDATE_RE = new RegExp(
20+
`^\\s*(?<operation>UPDATE)(?:\\s+OR\\s+(?:ROLLBACK|ABORT|FAIL|IGNORE|REPLACE))?\\s+(?<table>${TABLE_NAME})`,
21+
'i',
22+
);
1323
const DELETE_RE = new RegExp(`^\\s*(?<operation>DELETE)\\s+FROM\\s+(?<table>${TABLE_NAME})`, 'i');
1424

1525
const SELECT_RE = /^\s*\(?\s*(?<operation>SELECT)\b/i;

packages/core/test/lib/utils/sql.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,39 @@ describe('getSqlQuerySummary', () => {
7575
'INSERT shipping_details SELECT orders',
7676
);
7777
});
78+
79+
it.each([
80+
['INSERT OR REPLACE INTO users (id) VALUES (?)', 'INSERT users'],
81+
['INSERT OR IGNORE INTO users (id) VALUES (?)', 'INSERT users'],
82+
['INSERT OR ABORT INTO users (id) VALUES (?)', 'INSERT users'],
83+
['INSERT OR FAIL INTO users (id) VALUES (?)', 'INSERT users'],
84+
['INSERT OR ROLLBACK INTO users (id) VALUES (?)', 'INSERT users'],
85+
['insert or replace into orders (id) values (?)', 'insert orders'],
86+
])('strips the SQLite conflict clause: %j => %j', (input, expected) => {
87+
expect(getSqlQuerySummary(input)).toBe(expected);
88+
});
89+
90+
it.each([
91+
['REPLACE INTO users (id) VALUES (?)', 'REPLACE users'],
92+
['replace into orders (id) values (?)', 'replace orders'],
93+
['REPLACE INTO shipping_details SELECT * FROM orders', 'REPLACE shipping_details SELECT orders'],
94+
])('handles the REPLACE INTO shorthand: %j => %j', (input, expected) => {
95+
expect(getSqlQuerySummary(input)).toBe(expected);
96+
});
97+
98+
it('captures INSERT OR REPLACE...SELECT with both targets', () => {
99+
expect(getSqlQuerySummary('INSERT OR REPLACE INTO shipping_details SELECT * FROM orders')).toBe(
100+
'INSERT shipping_details SELECT orders',
101+
);
102+
});
78103
});
79104

80105
describe('UPDATE', () => {
81106
it.each([
82107
['UPDATE users SET name = ? WHERE id = ?', 'UPDATE users'],
83108
['update orders SET status = ? WHERE created_at < ?', 'update orders'],
109+
['UPDATE OR REPLACE users SET name = ? WHERE id = ?', 'UPDATE users'],
110+
['UPDATE OR IGNORE orders SET status = ?', 'UPDATE orders'],
84111
])('%j => %j', (input, expected) => {
85112
expect(getSqlQuerySummary(input)).toBe(expected);
86113
});

0 commit comments

Comments
 (0)