Skip to content

Commit bcf32cd

Browse files
gflachsmaryliag
andauthored
fix(instrumentation-mysql2)!: Missing masking of sql queries (#2732)
Co-authored-by: maryliag <[email protected]>
1 parent 8d05684 commit bcf32cd

File tree

5 files changed

+266
-15
lines changed

5 files changed

+266
-15
lines changed

packages/instrumentation-mysql2/README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ You can set the following instrumentation options:
4848
| ------- | ---- | ----------- |
4949
| `responseHook` | `MySQL2InstrumentationExecutionResponseHook` (function) | Function for adding custom attributes from db response |
5050
| `addSqlCommenterCommentToQueries` | `boolean` | If true, adds [sqlcommenter](https://github.com/open-telemetry/opentelemetry-sqlcommenter) specification compliant comment to queries with tracing context (default false). _NOTE: A comment will not be added to queries that already contain `--` or `/* ... */` in them, even if these are not actually part of comments_ |
51+
| `maskStatement` | `boolean` | If true, masks the `db.statement` attribute in spans (default false) with the `maskStatementHook` |
52+
| `maskStatementHook` | `MySQL2InstrumentationMaskStatementHook` (function) | Function for masking the `db.statement` attribute in spans Default: `return query.replace(/\b\d+\b/g, '?').replac(/(["'])(?:(?=(\\?))\2.)*?\1/g, '?');`|
53+
5154

5255
## Semantic Conventions
5356

packages/instrumentation-mysql2/src/instrumentation.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,20 @@ export class MySQL2Instrumentation extends InstrumentationBase<MySQL2Instrumenta
137137
} else if (arguments[2]) {
138138
values = [_valuesOrCallback];
139139
}
140-
140+
const { maskStatement, maskStatementHook, responseHook } =
141+
thisPlugin.getConfig();
141142
const span = thisPlugin.tracer.startSpan(getSpanName(query), {
142143
kind: api.SpanKind.CLIENT,
143144
attributes: {
144145
...MySQL2Instrumentation.COMMON_ATTRIBUTES,
145146
...getConnectionAttributes(this.config),
146-
[SEMATTRS_DB_STATEMENT]: getDbStatement(query, format, values),
147+
[SEMATTRS_DB_STATEMENT]: getDbStatement(
148+
query,
149+
format,
150+
values,
151+
maskStatement,
152+
maskStatementHook
153+
),
147154
},
148155
});
149156

@@ -166,7 +173,6 @@ export class MySQL2Instrumentation extends InstrumentationBase<MySQL2Instrumenta
166173
message: err.message,
167174
});
168175
} else {
169-
const { responseHook } = thisPlugin.getConfig();
170176
if (typeof responseHook === 'function') {
171177
safeExecuteInTheMiddle(
172178
() => {

packages/instrumentation-mysql2/src/types.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,26 @@ export interface MySQL2InstrumentationExecutionResponseHook {
2525
(span: Span, responseHookInfo: MySQL2ResponseHookInformation): void;
2626
}
2727

28+
export interface MySQL2InstrumentationQueryMaskingHook {
29+
(query: string): string;
30+
}
31+
2832
export interface MySQL2InstrumentationConfig extends InstrumentationConfig {
33+
/**
34+
* If true, the query will be masked before setting it as a span attribute, using the {@link maskStatementHook}.
35+
*
36+
* @default false
37+
* @see maskStatementHook
38+
*/
39+
maskStatement?: boolean;
40+
41+
/**
42+
* Hook that allows masking the query string before setting it as span attribute.
43+
*
44+
* @default (query: string) => query.replace(/\b\d+\b/g, '?').replace(/(["'])(?:(?=(\\?))\2.)*?\1/g, '?')
45+
*/
46+
maskStatementHook?: MySQL2InstrumentationQueryMaskingHook;
47+
2948
/**
3049
* Hook that allows adding custom span attributes based on the data
3150
* returned MySQL2 queries.

packages/instrumentation-mysql2/src/utils.ts

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
SEMATTRS_NET_PEER_PORT,
2424
} from '@opentelemetry/semantic-conventions';
2525
import type * as mysqlTypes from 'mysql2';
26+
import { MySQL2InstrumentationQueryMaskingHook } from './types';
2627

2728
type formatType = typeof mysqlTypes.format;
2829

@@ -107,22 +108,50 @@ function getJDBCString(
107108
export function getDbStatement(
108109
query: string | Query | QueryOptions,
109110
format?: formatType,
110-
values?: any[]
111+
values?: any[],
112+
maskStatement = false,
113+
maskStatementHook: MySQL2InstrumentationQueryMaskingHook = defaultMaskingHook
111114
): string {
112-
if (!format) {
113-
return typeof query === 'string' ? query : query.sql;
114-
}
115-
if (typeof query === 'string') {
116-
return values ? format(query, values) : query;
117-
} else {
118-
// According to https://github.com/mysqljs/mysql#performing-queries
119-
// The values argument will override the values in the option object.
120-
return values || (query as QueryOptions).values
121-
? format(query.sql, values || (query as QueryOptions).values)
122-
: query.sql;
115+
const [querySql, queryValues] =
116+
typeof query === 'string'
117+
? [query, values]
118+
: [query.sql, hasValues(query) ? values || query.values : values];
119+
try {
120+
if (maskStatement) {
121+
return maskStatementHook(querySql);
122+
} else if (format && queryValues) {
123+
return format(querySql, queryValues);
124+
} else {
125+
return querySql;
126+
}
127+
} catch (e) {
128+
return 'Could not determine the query due to an error in masking or formatting';
123129
}
124130
}
125131

132+
/**
133+
* Replaces numeric values and quoted strings in the query with placeholders ('?').
134+
*
135+
* - `\b\d+\b`: Matches whole numbers (integers) and replaces them with '?'.
136+
* - `(["'])(?:(?=(\\?))\2.)*?\1`:
137+
* - Matches quoted strings (both single `'` and double `"` quotes).
138+
* - Uses a lookahead `(?=(\\?))` to detect an optional backslash without consuming it immediately.
139+
* - Captures the optional backslash `\2` and ensures escaped quotes inside the string are handled correctly.
140+
* - Ensures that only complete quoted strings are replaced with '?'.
141+
*
142+
* This prevents accidental replacement of escaped quotes within strings and ensures that the
143+
* query structure remains intact while masking sensitive data.
144+
*/
145+
function defaultMaskingHook(query: string): string {
146+
return query
147+
.replace(/\b\d+\b/g, '?')
148+
.replace(/(["'])(?:(?=(\\?))\2.)*?\1/g, '?');
149+
}
150+
151+
function hasValues(obj: Query | QueryOptions): obj is QueryOptions {
152+
return 'values' in obj;
153+
}
154+
126155
/**
127156
* The span name SHOULD be set to a low cardinality value
128157
* representing the statement executed on the database.

packages/instrumentation-mysql2/test/mysql.test.ts

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,6 +1225,200 @@ describe('mysql2', () => {
12251225
});
12261226
});
12271227
});
1228+
describe('#maskStatementHook', () => {
1229+
beforeEach(done => {
1230+
//create table user and insert data
1231+
rootConnection.query(
1232+
'CREATE TABLE user (id INT, name VARCHAR(255), age INT)',
1233+
() => {
1234+
rootConnection.query(
1235+
'INSERT INTO user (id, name, age) VALUES (1, "test", 35)',
1236+
done
1237+
);
1238+
}
1239+
);
1240+
});
1241+
1242+
afterEach(done => {
1243+
rootConnection.query('DROP TABLE user', done);
1244+
});
1245+
describe('default maskStatementHook', () => {
1246+
beforeEach(done => {
1247+
instrumentation.setConfig({
1248+
maskStatement: true,
1249+
});
1250+
memoryExporter.reset();
1251+
done();
1252+
});
1253+
1254+
it('should mask string and numbers in statements', done => {
1255+
const query =
1256+
"SELECT * FROM user WHERE name = 'test' AND age = 35 AND id = 1";
1257+
const maskedQuery =
1258+
'SELECT * FROM user WHERE name = ? AND age = ? AND id = ?';
1259+
const span = provider.getTracer('default').startSpan('test span');
1260+
context.with(trace.setSpan(context.active(), span), () => {
1261+
connection.query(query, (err, res: RowDataPacket[]) => {
1262+
assert.ifError(err);
1263+
assert.ok(res);
1264+
assert.strictEqual(res[0].name, 'test');
1265+
assert.strictEqual(res[0].age, 35);
1266+
assert.strictEqual(res[0].id, 1);
1267+
const spans = memoryExporter.getFinishedSpans();
1268+
assert.strictEqual(spans.length, 1);
1269+
assertSpan(spans[0], maskedQuery);
1270+
done();
1271+
});
1272+
});
1273+
});
1274+
});
1275+
describe('custom maskStatementHook', () => {
1276+
beforeEach(done => {
1277+
instrumentation.setConfig({
1278+
maskStatement: true,
1279+
maskStatementHook: query => {
1280+
return query
1281+
.replace(/\b\d+\b/g, '*')
1282+
.replace(/(["'])(?:(?=(\\?))\2.)*?\1/g, '*');
1283+
},
1284+
});
1285+
memoryExporter.reset();
1286+
done();
1287+
});
1288+
1289+
it('should mask string and numbers in statements', done => {
1290+
const query =
1291+
"SELECT * FROM user WHERE name = 'test' AND age = 35 AND id = 1";
1292+
const maskedQuery =
1293+
'SELECT * FROM user WHERE name = * AND age = * AND id = *';
1294+
const span = provider.getTracer('default').startSpan('test span');
1295+
context.with(trace.setSpan(context.active(), span), () => {
1296+
connection.query(query, (err, res: RowDataPacket[]) => {
1297+
assert.ifError(err);
1298+
assert.ok(res);
1299+
assert.strictEqual(res[0].name, 'test');
1300+
assert.strictEqual(res[0].age, 35);
1301+
assert.strictEqual(res[0].id, 1);
1302+
const spans = memoryExporter.getFinishedSpans();
1303+
assert.strictEqual(spans.length, 1);
1304+
assertSpan(spans[0], maskedQuery);
1305+
done();
1306+
});
1307+
});
1308+
});
1309+
});
1310+
describe('maskStatementHook with error', () => {
1311+
beforeEach(done => {
1312+
instrumentation.setConfig({
1313+
maskStatement: true,
1314+
maskStatementHook: () => {
1315+
throw new Error('random failure!');
1316+
},
1317+
});
1318+
memoryExporter.reset();
1319+
done();
1320+
});
1321+
it('should not affect the behavior of the query', done => {
1322+
const query =
1323+
"SELECT * FROM user WHERE name = 'test' AND age = 35 AND id = 1";
1324+
const errorQuery =
1325+
'Could not determine the query due to an error in masking or formatting';
1326+
const span = provider.getTracer('default').startSpan('test span');
1327+
context.with(trace.setSpan(context.active(), span), () => {
1328+
connection.query(query, (err, res: RowDataPacket[]) => {
1329+
assert.ifError(err);
1330+
assert.ok(res);
1331+
assert.strictEqual(res[0].name, 'test');
1332+
assert.strictEqual(res[0].age, 35);
1333+
assert.strictEqual(res[0].id, 1);
1334+
const spans = memoryExporter.getFinishedSpans();
1335+
assert.strictEqual(spans.length, 1);
1336+
assertSpan(spans[0], errorQuery);
1337+
done();
1338+
});
1339+
});
1340+
});
1341+
});
1342+
});
1343+
describe('#maskStatement', () => {
1344+
beforeEach(done => {
1345+
memoryExporter.reset();
1346+
done();
1347+
});
1348+
1349+
it('should mask query if maskStatement is true', done => {
1350+
instrumentation.setConfig({
1351+
maskStatement: true,
1352+
});
1353+
const query = 'SELECT 1+1 as solution';
1354+
const maskedQuery = 'SELECT ?+? as solution';
1355+
const span = provider.getTracer('default').startSpan('test span');
1356+
context.with(trace.setSpan(context.active(), span), () => {
1357+
connection.query(query, (err, res: RowDataPacket[]) => {
1358+
assert.ifError(err);
1359+
assert.ok(res);
1360+
assert.strictEqual(res[0].solution, 2);
1361+
const spans = memoryExporter.getFinishedSpans();
1362+
assert.strictEqual(spans.length, 1);
1363+
assertSpan(spans[0], maskedQuery);
1364+
done();
1365+
});
1366+
});
1367+
});
1368+
it('should return masked query, if values are present', done => {
1369+
instrumentation.setConfig({
1370+
maskStatement: true,
1371+
});
1372+
const query = 'SELECT ?+? as solution';
1373+
const maskedQuery = 'SELECT ?+? as solution';
1374+
const span = provider.getTracer('default').startSpan('test span');
1375+
context.with(trace.setSpan(context.active(), span), () => {
1376+
connection.query(query, [1, 1], (err, res: RowDataPacket[]) => {
1377+
assert.ifError(err);
1378+
assert.ok(res);
1379+
assert.strictEqual(res[0].solution, 2);
1380+
const spans = memoryExporter.getFinishedSpans();
1381+
assert.strictEqual(spans.length, 1);
1382+
assertSpan(spans[0], maskedQuery);
1383+
done();
1384+
});
1385+
});
1386+
});
1387+
it('should not mask query if maskStatement is false (default)', done => {
1388+
const query = 'SELECT 1+1 as solution';
1389+
const span = provider.getTracer('default').startSpan('test span');
1390+
context.with(trace.setSpan(context.active(), span), () => {
1391+
connection.query(query, (err, res: RowDataPacket[]) => {
1392+
assert.ifError(err);
1393+
assert.ok(res);
1394+
assert.strictEqual(res[0].solution, 2);
1395+
const spans = memoryExporter.getFinishedSpans();
1396+
assert.strictEqual(spans.length, 1);
1397+
assertSpan(spans[0], query);
1398+
done();
1399+
});
1400+
});
1401+
});
1402+
it('should return query with values, if values are present and maskStatement is false', done => {
1403+
instrumentation.setConfig({
1404+
maskStatement: false,
1405+
});
1406+
const query = 'SELECT ?+? as solution';
1407+
const queryWithValues = 'SELECT 1+1 as solution';
1408+
const span = provider.getTracer('default').startSpan('test span');
1409+
context.with(trace.setSpan(context.active(), span), () => {
1410+
connection.query(query, [1, 1], (err, res: RowDataPacket[]) => {
1411+
assert.ifError(err);
1412+
assert.ok(res);
1413+
assert.strictEqual(res[0].solution, 2);
1414+
const spans = memoryExporter.getFinishedSpans();
1415+
assert.strictEqual(spans.length, 1);
1416+
assertSpan(spans[0], queryWithValues);
1417+
done();
1418+
});
1419+
});
1420+
});
1421+
});
12281422
});
12291423
describe('promise API', () => {
12301424
let instrumentation: MySQL2Instrumentation;

0 commit comments

Comments
 (0)