Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function instrumentSqlStorage(sql: SqlStorage): SqlStorage {
const allowlist = (getClient()?.getOptions() as CloudflareClientOptions | undefined)
?.durableObjectSqlSpanAllowlist;

if (targetsCloudflareInternalTable(querySummary, allowlist)) {
if (targetsCloudflareInternalTable(querySummary, allowlist, sanitizedQuery)) {
return (original as (...a: unknown[]) => ReturnType<SqlStorage['exec']>).apply(target, args);
}

Expand Down
33 changes: 24 additions & 9 deletions packages/cloudflare/src/utils/internalSqlQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,40 @@ import { stringMatchesSomePattern } from '@sentry/core';
*
* The check operates on the query summary produced by `getSqlQuerySummary` (`{operation} {table} ...`,
* the same value used as the span name), so table targets are already isolated from the rest of the
* query.
* query. The one exception is `CREATE INDEX`: its summary carries the index name, not the indexed
* table (upstream OTel convention), so the `cf_` target only appears in the ON clause of the full
* statement, which `queryText` is needed for.
*/
export function targetsCloudflareInternalTable(
querySummary: string | undefined,
allowlist?: Array<string | RegExp>,
queryText?: string,
): boolean {
if (!querySummary) {
return false;
}

const indexedTable = queryText ? CREATE_INDEX_TABLE_RE.exec(queryText)?.groups?.['table'] : undefined;
if (indexedTable) {
return isCloudflareInternalTable(indexedTable, allowlist);
}

const [, ...tables] = querySummary.split(' ');

return tables.some(table => {
if (!table.toLowerCase().startsWith('cf_')) {
return false;
}
return tables.some(table => isCloudflareInternalTable(table, allowlist));
}

// `CREATE [UNIQUE] INDEX [IF NOT EXISTS] <name> ON <table>` — the IF EXISTS shape mirrors DDL_RE
// in @sentry/core.
const CREATE_INDEX_TABLE_RE =
/^\s*CREATE\s+(?:UNIQUE\s+)?INDEX(?:\s+IF\s+(?:NOT\s+)?EXISTS)?\s+[^\s(,;)]+\s+ON\s+(?<table>[^\s(,;)]+)/i;

function isCloudflareInternalTable(table: string, allowlist?: Array<string | RegExp>): boolean {
if (!table.toLowerCase().startsWith('cf_')) {
return false;
}

// A table on the allowlist is treated as a user table and stays instrumented, even though it
// matches the reserved prefix.
return !allowlist?.length || !stringMatchesSomePattern(table, allowlist, true);
});
// A table on the allowlist is treated as a user table and stays instrumented, even though it
// matches the reserved prefix.
return !allowlist?.length || !stringMatchesSomePattern(table, allowlist, true);
}
134 changes: 116 additions & 18 deletions packages/cloudflare/test/instrumentSqlStorage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,31 +159,129 @@ describe('instrumentSqlStorage', () => {
expect(result).toBe(mockCursor);
});

it('still creates a span for user queries', () => {
const startSpanSpy = vi.spyOn(sentryCore, 'startSpan');
const mockSql = createMockSqlStorage();
const instrumented = instrumentSqlStorage(mockSql);
describe('internal tables (cf_ prefix) are skipped', () => {
it.each([
['SELECT', 'SELECT * FROM cf_agents_state WHERE id = ?'],
['INSERT', 'INSERT INTO cf_agents_fibers (id, callback) VALUES (?, ?)'],
['DELETE', 'DELETE FROM cf_agents_schedules WHERE id = ?'],
['UPDATE', 'UPDATE cf_agent_tool_runs SET output_json = ? WHERE id = ?'],
['CREATE TABLE', 'CREATE TABLE IF NOT EXISTS cf_agents_workflows (id TEXT PRIMARY KEY NOT NULL)'],
['ALTER TABLE', 'ALTER TABLE cf_agents_queues ADD COLUMN retry_options TEXT'],
['DROP TABLE', 'DROP TABLE cf_agents_state'],
['cf_agent_ prefix', 'SELECT * FROM cf_agent_identity'],
['cf_ai_ prefix', 'INSERT INTO cf_ai_chat_stream_chunks (id) VALUES (?)'],
['cf_mcp_ prefix', 'SELECT * FROM cf_mcp_agent_event'],
['schema version', 'SELECT version FROM cf_schema_version'],
// SQLite upsert forms used by the agents framework for state/schedule/MCP persistence
['INSERT OR REPLACE', 'INSERT OR REPLACE INTO cf_agents_state (id, state) VALUES (?, ?)'],
[
'INSERT OR REPLACE with column list',
`INSERT OR REPLACE INTO cf_agents_mcp_servers ( id, name, server_url, client_id, auth_url,
callback_url, server_options )
VALUES ( ?, ?, ?, ?, ?, ?, ? )`,
],
['INSERT OR IGNORE', 'INSERT OR IGNORE INTO cf_agents_sub_agents (class, name) VALUES (?, ?)'],
['REPLACE INTO', 'REPLACE INTO cf_agents_queues (id, payload) VALUES (?, ?)'],
['UPDATE OR REPLACE', 'UPDATE OR REPLACE cf_agents_state SET state = ? WHERE id = ?'],
// The summary of a CREATE INDEX carries the index name, not the indexed table — the cf_
// target only exists in the ON clause of the full statement.
[
'CREATE INDEX (framework statement)',
`create index if not exists idx_ai_chat_agent_tool_request_id
on cf_ai_chat_agent_tool_runs(request_id)`,
],
['CREATE INDEX (uppercase)', 'CREATE INDEX idx_agents_state_id ON cf_agents_state (id)'],
['CREATE UNIQUE INDEX', 'CREATE UNIQUE INDEX idx_agents_state_id ON cf_agents_state (id)'],
[
'CREATE INDEX (without IF NOT EXISTS)',
'CREATE INDEX idx_chunks_stream ON cf_ai_chat_stream_chunks (stream_id)',
],
[
'JOIN between internal tables',
`SELECT f.fiber_id, f.status
FROM cf_agents_fibers f
LEFT JOIN cf_agents_runs r ON r.id = f.fiber_id
WHERE f.status IN ('pending', 'running')`,
],
// `.some()` — any internal table present means the query is framework-driven noise.
['JOIN with a user table', 'SELECT * FROM cf_agents_state s JOIN users u ON u.id = s.id'],
['lowercase keywords and prefix', 'select * from CF_AGENTS_STATE'],
])('skips %s', (_label, query) => {
expect(execCreatesSpan(query)).toBe(false);
});
});

instrumented.exec('SELECT * FROM users WHERE id = ?', 1);
describe('user queries stay instrumented', () => {
it.each([
['SELECT', 'SELECT * FROM users WHERE id = ?'],
['INSERT', 'INSERT INTO orders (id, total) VALUES (?, ?)'],
['UPDATE', 'UPDATE products SET price = ? WHERE id = ?'],
['DELETE', 'DELETE FROM sessions WHERE expired = 1'],
['CREATE TABLE', 'CREATE TABLE users (id TEXT PRIMARY KEY)'],
['CREATE INDEX', 'CREATE INDEX idx_name ON users (name)'],
['table with cf in the middle', 'SELECT * FROM my_cf_table'],
['table starting with cfg', 'SELECT * FROM cfg_settings'],
['INSERT OR REPLACE', 'INSERT OR REPLACE INTO users (id, name) VALUES (?, ?)'],
['REPLACE INTO', 'REPLACE INTO sessions (id, token) VALUES (?, ?)'],
['UPDATE OR IGNORE', 'UPDATE OR IGNORE products SET price = ? WHERE id = ?'],
// No resolvable table target — safe default is to instrument.
['no-table SELECT', 'SELECT 1'],
['PRAGMA', 'PRAGMA foreign_keys = ON'],
['bare operation', 'BEGIN'],
['empty query', ''],
])('instruments %s', (_label, query) => {
expect(execCreatesSpan(query)).toBe(true);
});
});

expect(startSpanSpy).toHaveBeenCalledTimes(1);
describe('durableObjectSqlSpanAllowlist (opt a cf_ table back into instrumentation)', () => {
it.each([
['exact string', 'SELECT * FROM cf_my_table', ['cf_my_table']],
['regex', 'SELECT * FROM cf_reports_daily', [/^cf_reports_/]],
['upsert target', 'INSERT OR REPLACE INTO cf_my_table (id) VALUES (?)', ['cf_my_table']],
['CREATE INDEX target', 'CREATE INDEX idx_mine ON cf_my_table (id)', ['cf_my_table']],
])('instruments an allowlisted table matched by %s', (_label, query, allowlist) => {
expect(execCreatesSpan(query, allowlist)).toBe(true);
});

it.each([
// Substring matches must not opt a table back in, otherwise `cf_` would allowlist everything.
['a string entry only matches exactly', 'SELECT * FROM cf_agents_state', ['cf_agents']],
['a non-matching entry leaves internal tables skipped', 'SELECT * FROM cf_agents_state', ['cf_my_table']],
[
'an internal table joined with an allowlisted table is still skipped',
'SELECT * FROM cf_my_table t JOIN cf_agents_state s ON s.id = t.id',
['cf_my_table'],
],
['an empty allowlist is ignored', 'SELECT * FROM cf_agents_state', []],
])('%s', (_label, query, allowlist) => {
expect(execCreatesSpan(query, allowlist)).toBe(false);
});
});
});
});

it('creates a span for a cf_ table on the durableObjectSqlSpanAllowlist', () => {
const startSpanSpy = vi.spyOn(sentryCore, 'startSpan');
vi.spyOn(sentryCore, 'getClient').mockReturnValue({
getOptions: () => ({ durableObjectSqlSpanAllowlist: ['cf_my_table'] }),
} as unknown as ReturnType<typeof sentryCore.getClient>);
/**
* Runs a query through the real `instrumentSqlStorage` proxy and reports whether it produced a
* `db.query` span, so the filtering matrix exercises the actual code path rather than a
* reimplementation of it.
*/
function execCreatesSpan(query: string, allowlist?: Array<string | RegExp>): boolean {
const startSpanSpy = vi.spyOn(sentryCore, 'startSpan');

const mockSql = createMockSqlStorage();
const instrumented = instrumentSqlStorage(mockSql);
if (allowlist) {
vi.spyOn(sentryCore, 'getClient').mockReturnValue({
getOptions: () => ({ durableObjectSqlSpanAllowlist: allowlist }),
} as unknown as ReturnType<typeof sentryCore.getClient>);
}

instrumented.exec('SELECT * FROM cf_my_table WHERE id = ?', 1);
const mockSql = createMockSqlStorage();
instrumentSqlStorage(mockSql).exec(query);

expect(startSpanSpy).toHaveBeenCalledTimes(1);
});
});
});
expect(mockSql.exec).toHaveBeenCalledWith(query);

return startSpanSpy.mock.calls.length > 0;
}

function createMockCursor() {
return {
Expand Down
133 changes: 15 additions & 118 deletions packages/cloudflare/test/utils/internalSqlQuery.test.ts
Original file line number Diff line number Diff line change
@@ -1,128 +1,25 @@
import { _INTERNAL_getSqlQuerySummary } from '@sentry/core';
import { describe, expect, it } from 'vitest';
import { targetsCloudflareInternalTable } from '../../src/utils/internalSqlQuery';

// Builds the summary the same way `instrumentSqlStorage` does, so the test exercises the real
// operation -> summary -> detection path rather than hand-written summaries.
const summarize = (query: string): string | undefined => _INTERNAL_getSqlQuerySummary(query);

// Behavioural coverage of the filter lives in `instrumentSqlStorage.test.ts`, which drives real
// queries through the instrumented `exec`. What remains here are the signature-level contracts that
// call path cannot reach: an absent summary, and an absent `queryText`.
describe('targetsCloudflareInternalTable', () => {
describe('internal queries (cf_ tables)', () => {
it.each([
['SELECT', 'SELECT * FROM cf_agents_state WHERE id = ?'],
['INSERT', 'INSERT INTO cf_agents_fibers (id, callback) VALUES (?, ?)'],
['DELETE', 'DELETE FROM cf_agents_schedules WHERE id = ?'],
['UPDATE', 'UPDATE cf_agent_tool_runs SET output_json = ? WHERE id = ?'],
['CREATE TABLE', 'CREATE TABLE IF NOT EXISTS cf_agents_workflows (id TEXT PRIMARY KEY NOT NULL)'],
['ALTER TABLE', 'ALTER TABLE cf_agents_queues ADD COLUMN retry_options TEXT'],
['DROP TABLE', 'DROP TABLE cf_agents_state'],
['cf_agent_ prefix', 'SELECT * FROM cf_agent_identity'],
['cf_ai_ prefix', 'INSERT INTO cf_ai_chat_stream_chunks (id) VALUES (?)'],
['cf_mcp_ prefix', 'SELECT * FROM cf_mcp_agent_event'],
['schema version', 'SELECT version FROM cf_schema_version'],
// SQLite upsert forms used by the agents framework for state/schedule/MCP persistence
['INSERT OR REPLACE', 'INSERT OR REPLACE INTO cf_agents_state (id, state) VALUES (?, ?)'],
[
'INSERT OR REPLACE with column list',
`INSERT OR REPLACE INTO cf_agents_mcp_servers ( id, name, server_url, client_id, auth_url,
callback_url, server_options )
VALUES ( ?, ?, ?, ?, ?, ?, ? )`,
],
['INSERT OR IGNORE', 'INSERT OR IGNORE INTO cf_agents_sub_agents (class, name) VALUES (?, ?)'],
['REPLACE INTO', 'REPLACE INTO cf_agents_queues (id, payload) VALUES (?, ?)'],
['UPDATE OR REPLACE', 'UPDATE OR REPLACE cf_agents_state SET state = ? WHERE id = ?'],
])('returns true for %s on internal tables', (_label, query) => {
expect(targetsCloudflareInternalTable(summarize(query))).toBe(true);
});

it('returns true for an internal JOIN', () => {
const query = `
SELECT f.fiber_id, f.status
FROM cf_agents_fibers f
LEFT JOIN cf_agents_runs r ON r.id = f.fiber_id
WHERE f.status IN ('pending', 'running')
`;
expect(targetsCloudflareInternalTable(summarize(query))).toBe(true);
});

it('returns true when an internal table is joined with a user table', () => {
// `.some()` — any internal table present means the query is framework-driven noise.
expect(
targetsCloudflareInternalTable(summarize('SELECT * FROM cf_agents_state s JOIN users u ON u.id = s.id')),
).toBe(true);
});

it('handles case-insensitive keywords and prefixes', () => {
expect(targetsCloudflareInternalTable(summarize('select * from CF_AGENTS_STATE'))).toBe(true);
});
});

describe('user queries (must be instrumented)', () => {
it.each([
['SELECT', 'SELECT * FROM users WHERE id = ?'],
['INSERT', 'INSERT INTO orders (id, total) VALUES (?, ?)'],
['UPDATE', 'UPDATE products SET price = ? WHERE id = ?'],
['DELETE', 'DELETE FROM sessions WHERE expired = 1'],
['CREATE TABLE', 'CREATE TABLE users (id TEXT PRIMARY KEY)'],
['table with cf in the middle', 'SELECT * FROM my_cf_table'],
['table starting with cfg', 'SELECT * FROM cfg_settings'],
['INSERT OR REPLACE', 'INSERT OR REPLACE INTO users (id, name) VALUES (?, ?)'],
['REPLACE INTO', 'REPLACE INTO sessions (id, token) VALUES (?, ?)'],
['UPDATE OR IGNORE', 'UPDATE OR IGNORE products SET price = ? WHERE id = ?'],
])('returns false for %s on user tables', (_label, query) => {
expect(targetsCloudflareInternalTable(summarize(query))).toBe(false);
});
it.each([
['undefined', undefined],
['empty', ''],
])('returns false for a %s summary', (_label, summary) => {
expect(targetsCloudflareInternalTable(summary)).toBe(false);
});

describe('allowlist (opt a cf_ table back into instrumentation)', () => {
it('returns false for an allowlisted table matched by exact string', () => {
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_my_table'), ['cf_my_table'])).toBe(false);
});

it('returns false for an allowlisted table matched by regex', () => {
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_reports_daily'), [/^cf_reports_/])).toBe(false);
});

it('returns false for an allowlisted table targeted by an upsert', () => {
expect(
targetsCloudflareInternalTable(summarize('INSERT OR REPLACE INTO cf_my_table (id) VALUES (?)'), [
'cf_my_table',
]),
).toBe(false);
});

it('requires an exact match for string entries', () => {
// Substring matches must not opt a table back in, otherwise `cf_` would allowlist everything.
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_agents_state'), ['cf_agents'])).toBe(true);
});

it('still skips genuine internal tables that are not allowlisted', () => {
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_agents_state'), ['cf_my_table'])).toBe(true);
});

it('still skips when an internal table is joined with an allowlisted table', () => {
expect(
targetsCloudflareInternalTable(summarize('SELECT * FROM cf_my_table t JOIN cf_agents_state s ON s.id = t.id'), [
'cf_my_table',
]),
).toBe(true);
});

it('ignores an empty allowlist', () => {
expect(targetsCloudflareInternalTable(summarize('SELECT * FROM cf_agents_state'), [])).toBe(true);
});
it('falls back to the summary when no queryText is passed', () => {
expect(targetsCloudflareInternalTable('SELECT cf_agents_state')).toBe(true);
expect(targetsCloudflareInternalTable('SELECT users')).toBe(false);
});

describe('summaries without a resolvable table target (safe default: instrument)', () => {
it.each([
['undefined', undefined],
['empty', ''],
['no-table SELECT', 'SELECT 1'],
['PRAGMA', 'PRAGMA foreign_keys = ON'],
['bare operation', 'BEGIN'],
])('returns false for %s', (_label, value) => {
const summary = typeof value === 'string' ? summarize(value) : value;
expect(targetsCloudflareInternalTable(summary)).toBe(false);
});
// Without queryText a CREATE INDEX summary carries the index name, so the cf_ table in the ON
// clause is invisible and the query is instrumented — the caller must pass queryText to filter it.
it('cannot resolve a CREATE INDEX target from the summary alone', () => {
expect(targetsCloudflareInternalTable('CREATE INDEX idx_agents_state_id')).toBe(false);
});
});
Loading