Skip to content

refactor: parameterize SQL queries in analytics repositories for improved security #2387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JivusAyrus merged 2 commits into from
Dec 3, 2025
Merged

refactor: parameterize SQL queries in analytics repositories for improved security #2387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b3f794
refactor: parameterize SQL queries in analytics repositories for impr…
JivusAyrus Dec 3, 2025
cac93fb
Merge branch 'main' into suvij/eng-8607-wun-q425-11-multiple-sql-inje…
JivusAyrus Dec 3, 2025
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
53 changes: 39 additions & 14 deletions controlplane/src/core/repositories/CacheWarmerRepository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,16 @@ export class CacheWarmerRepository {

const query = `
WITH
toDateTime('${start}') AS startDate,
toDateTime('${end}') AS endDate
toDateTime({startDate:UInt32}) AS startDate,
toDateTime({endDate:UInt32}) AS endDate
SELECT
max(MaxDuration) as maxDuration,
OperationHash as operationHash,
OperationName as operationName,
OperationPersistedID as operationPersistedID,
if(ClientName = 'unknown', '', ClientName) as clientName,
if(ClientVersion = 'missing', '', ClientVersion) as clientVersion,
func_rank(${quantile}, BucketCounts) as rank,
func_rank({quantile:Float64}, BucketCounts) as rank,
func_rank_bucket_lower_index(rank, BucketCounts) as b,
round(func_histogram_v2(
rank,
Expand All @@ -68,22 +68,32 @@ export class CacheWarmerRepository {
sumForEachMerge(BucketCounts) as BucketCounts
FROM ${this.client.database}.operation_planning_metrics_5_30
WHERE Timestamp >= startDate AND Timestamp <= endDate
AND FederatedGraphID = '${federatedGraphId}'
AND OrganizationID = '${organizationId}'
AND FederatedGraphID = {federatedGraphId:String}
AND OrganizationID = {organizationId:String}
AND OperationName != 'IntrospectionQuery'
GROUP BY OperationHash, OperationName, OperationPersistedID, ClientName, ClientVersion
HAVING maxDuration >= ${minPlanningTimeInMs}
ORDER BY planningTime DESC LIMIT ${maxOperationsCount}
HAVING maxDuration >= {minPlanningTimeInMs:UInt32}
ORDER BY planningTime DESC LIMIT {maxOperationsCount:UInt32}
`;

const params = {
startDate: start,
endDate: end,
quantile,
federatedGraphId,
organizationId,
minPlanningTimeInMs,
maxOperationsCount,
};

const res: {
operationHash: string;
operationName: string;
operationPersistedID: string;
clientName: string;
clientVersion: string;
planningTime: number;
}[] = await this.client.queryPromise(query);
}[] = await this.client.queryPromise(query, params);

if (Array.isArray(res)) {
return res;
Expand All @@ -108,23 +118,38 @@ export class CacheWarmerRepository {
const parsedDateRange = isoDateRangeToTimestamps(dateRange, rangeInHours);
const [start, end] = getDateRange(parsedDateRange);

// Escape single quotes in operation hashes and build the IN clause
const escapedHashes = operationHashes
.map((hash) => {
const escaped = hash.replace(/'/g, "''");
return `'${escaped}'`;
})
.join(',');

const query = `
WITH
toDateTime('${start}') AS startDate,
toDateTime('${end}') AS endDate
toDateTime({startDate:UInt32}) AS startDate,
toDateTime({endDate:UInt32}) AS endDate
SELECT
OperationContent as operationContent,
OperationHash as operationHash
FROM ${this.client.database}.gql_metrics_operations
WHERE OrganizationID = '${organizationID}'
AND FederatedGraphID = '${federatedGraphID}'
AND Timestamp >= startDate AND Timestamp <= endDate AND OperationHash IN (${operationHashes.map((hash) => `'${hash}'`).join(',')})
WHERE OrganizationID = {organizationID:String}
AND FederatedGraphID = {federatedGraphID:String}
AND Timestamp >= startDate AND Timestamp <= endDate AND OperationHash IN (${escapedHashes})
Comment thread
JivusAyrus marked this conversation as resolved.
GROUP BY
OperationContent,
OperationHash
`;

const res = await this.client.queryPromise(query);
const params = {
startDate: start,
endDate: end,
organizationID,
federatedGraphID,
};

const res = await this.client.queryPromise(query, params);

const operationContentMap = new Map<string, string>();

Expand Down
126 changes: 87 additions & 39 deletions controlplane/src/core/repositories/analytics/AnalyticsDashboardViewRepository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
SubgraphRequestRateResult,
TimeFilters,
} from '../../../types/index.js';
import { padMissingDatesForCurrentWeek } from './util.js';

export class AnalyticsDashboardViewRepository {
constructor(private client: ClickHouseClient) {}
Expand All @@ -32,8 +31,8 @@ export class AnalyticsDashboardViewRepository {
sum(TotalErrors) as erroredRequests
FROM ${this.client.database}.operation_request_metrics_5_30
WHERE Timestamp >= toDate(now()) - interval 6 day
AND FederatedGraphID = '${federatedGraphId}'
AND OrganizationID = '${organizationId}'
AND FederatedGraphID = {federatedGraphId:String}
AND OrganizationID = {organizationId:String}
GROUP BY timestamp
ORDER BY
timestamp WITH FILL
Expand All @@ -42,7 +41,12 @@ export class AnalyticsDashboardViewRepository {
)
`;

const seriesRes = await this.client.queryPromise(query);
const params = {
federatedGraphId,
organizationId,
};

const seriesRes = await this.client.queryPromise(query, params);

if (Array.isArray(seriesRes)) {
return seriesRes.map((p) => ({
Expand All @@ -68,30 +72,38 @@ export class AnalyticsDashboardViewRepository {

const query = `
WITH
toStartOfInterval(toDateTime('${filter.dateRange.start}'), INTERVAL ${filter.granule} MINUTE) AS startDate,
toDateTime('${filter.dateRange.end}') AS endDate
toStartOfInterval(toDateTime({start:UInt32}), INTERVAL {granule:UInt32} MINUTE) AS startDate,
toDateTime({end:UInt32}) AS endDate
SELECT toString(toUnixTimestamp(timestamp, 'UTC') * 1000) as timestamp, totalRequests, erroredRequests
FROM (
SELECT
toStartOfInterval(Timestamp, INTERVAL ${filter.granule} MINUTE) AS timestamp,
toStartOfInterval(Timestamp, INTERVAL {granule:UInt32} MINUTE) AS timestamp,
sum(TotalRequests) as totalRequests,
sum(TotalErrors) as erroredRequests
FROM ${this.client.database}.operation_request_metrics_5_30
WHERE timestamp >= startDate AND timestamp <= endDate
AND FederatedGraphID = '${federatedGraphId}'
AND OrganizationID = '${organizationId}'
AND FederatedGraphID = {federatedGraphId:String}
AND OrganizationID = {organizationId:String}
GROUP BY timestamp
ORDER BY
timestamp WITH FILL
FROM
toStartOfInterval(toDateTime('${filter.dateRange.start}'), INTERVAL ${filter.granule} MINUTE)
toStartOfInterval(toDateTime({start:UInt32}), INTERVAL {granule:UInt32} MINUTE)
TO
toDateTime('${filter.dateRange.end}')
STEP INTERVAL ${filter.granule} MINUTE
toDateTime({end:UInt32})
STEP INTERVAL {granule:UInt32} MINUTE
)
`;

const seriesRes = await this.client.queryPromise(query);
const params = {
start: filter.dateRange.start,
end: filter.dateRange.end,
granule: filter.granule,
federatedGraphId,
organizationId,
};

const seriesRes = await this.client.queryPromise(query, params);

if (Array.isArray(seriesRes)) {
return seriesRes.map((p) => ({
Expand All @@ -115,14 +127,21 @@ export class AnalyticsDashboardViewRepository {
OperationName as operationName,
sum(TotalRequests) as totalRequests
FROM ${this.client.database}.operation_request_metrics_5_30
WHERE Timestamp >= toDateTime('${dateRange.start}')
AND Timestamp <= toDateTime('${dateRange.end}')
AND OrganizationID = '${organizationId}'
AND FederatedGraphID = '${federatedGraphId}'
WHERE Timestamp >= toDateTime({start:UInt32})
AND Timestamp <= toDateTime({end:UInt32})
AND OrganizationID = {organizationId:String}
AND FederatedGraphID = {federatedGraphId:String}
GROUP BY OperationName, OperationHash ORDER BY totalRequests DESC LIMIT 10
`;

const res = await this.client.queryPromise(query);
const params = {
start: dateRange.start,
end: dateRange.end,
organizationId,
federatedGraphId,
};

const res = await this.client.queryPromise(query, params);

if (Array.isArray(res)) {
return res.map((r) => ({
Expand All @@ -146,18 +165,26 @@ export class AnalyticsDashboardViewRepository {
const query = `
SELECT
FederatedGraphID as federatedGraphID,
round(sum(TotalRequests) / ${multiplier}, 3) AS requestRate,
round(sum(TotalErrors) / ${multiplier}, 3) AS errorRate
round(sum(TotalRequests) / {multiplier:Float64}, 3) AS requestRate,
round(sum(TotalErrors) / {multiplier:Float64}, 3) AS errorRate
FROM ${this.client.database}.operation_request_metrics_5_30
WHERE Timestamp >= toDateTime('${dateRange.start}')
AND Timestamp <= toDateTime('${dateRange.end}')
AND FederatedGraphID = '${federatedGraphId}'
AND OrganizationID = '${organizationId}'
WHERE Timestamp >= toDateTime({start:UInt32})
AND Timestamp <= toDateTime({end:UInt32})
AND FederatedGraphID = {federatedGraphId:String}
AND OrganizationID = {organizationId:String}
GROUP BY FederatedGraphID
LIMIT 1
`;

const res = await this.client.queryPromise(query);
const params = {
start: dateRange.start,
end: dateRange.end,
multiplier,
federatedGraphId,
organizationId,
};

const res = await this.client.queryPromise(query, params);
if (Array.isArray(res)) {
return res.map((r) => ({
federatedGraphID: r.federatedGraphID,
Expand Down Expand Up @@ -197,21 +224,32 @@ export class AnalyticsDashboardViewRepository {
// to minutes
const multiplier = rangeInHours * 60;

// Properly escape subgraph IDs for SQL
const escapedSubgraphIds = subgraphs.map((s) => `'${s.id.replace(/'/g, "''")}'`).join(',');

const query = `
SELECT
SubgraphID as subgraphID,
round(sum(TotalRequests) / ${multiplier}, 3) AS requestRate,
round(sum(TotalErrors) / ${multiplier}, 3) AS errorRate
round(sum(TotalRequests) / {multiplier:Float64}, 3) AS requestRate,
round(sum(TotalErrors) / {multiplier:Float64}, 3) AS errorRate
FROM ${this.client.database}.subgraph_request_metrics_5_30
WHERE Timestamp >= toDateTime('${dateRange.start}')
AND Timestamp <= toDateTime('${dateRange.end}')
AND FederatedGraphID = '${federatedGraphId}'
AND OrganizationID = '${organizationId}'
AND SubgraphID IN (${subgraphs.map((s) => `'${s.id}'`).join(',')})
WHERE Timestamp >= toDateTime({start:UInt32})
AND Timestamp <= toDateTime({end:UInt32})
AND FederatedGraphID = {federatedGraphId:String}
AND OrganizationID = {organizationId:String}
AND SubgraphID IN (${escapedSubgraphIds})
GROUP BY SubgraphID
`;

const res = await this.client.queryPromise(query);
const params = {
start: dateRange.start,
end: dateRange.end,
multiplier,
federatedGraphId,
organizationId,
};

const res = await this.client.queryPromise(query, params);
if (Array.isArray(res)) {
return res.map((r) => ({
subgraphID: r.subgraphID,
Expand All @@ -229,6 +267,9 @@ export class AnalyticsDashboardViewRepository {
dateRange: DateRange<number>,
subgraphs: SubgraphDTO[],
): Promise<SubgraphLatencyResult[]> {
// Properly escape subgraph IDs for SQL
const escapedSubgraphIds = subgraphs.map((s) => `'${s.id.replace(/'/g, "''")}'`).join(',');

const query = `
SELECT SubgraphID as subgraphID, Latency as latency from (
SELECT SubgraphID,
Expand All @@ -245,17 +286,24 @@ export class AnalyticsDashboardViewRepository {
-- Histogram aggregations
sumForEachMerge(BucketCounts) as BucketCounts
from ${this.client.database}.subgraph_latency_metrics_5_30
WHERE Timestamp >= toDateTime('${dateRange.start}')
AND Timestamp <= toDateTime('${dateRange.end}')
AND FederatedGraphID = '${federatedGraphId}'
AND OrganizationID = '${organizationId}'
AND SubgraphID IN (${subgraphs.map((s) => `'${s.id}'`).join(',')})
WHERE Timestamp >= toDateTime({start:UInt32})
AND Timestamp <= toDateTime({end:UInt32})
AND FederatedGraphID = {federatedGraphId:String}
AND OrganizationID = {organizationId:String}
AND SubgraphID IN (${escapedSubgraphIds})
group by SubgraphID
order by SubgraphID
)
`;

const res = await this.client.queryPromise(query);
const params = {
start: dateRange.start,
end: dateRange.end,
federatedGraphId,
organizationId,
};

const res = await this.client.queryPromise(query, params);

if (Array.isArray(res)) {
return res.map((r) => ({
Expand Down
8 changes: 6 additions & 2 deletions controlplane/src/core/repositories/analytics/AnalyticsRequestViewRepository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,9 @@ export class AnalyticsRequestViewRepository {
}

if (client.length === 1) {
whereSql += `AND (${client.map((c) => `ClientName = '${c}'`).join(' OR ')})`;
// Add the client name as a parameter
queryParams.clientNameFilter = client[0];
whereSql += `AND ClientName = {clientNameFilter:String}`;
}

const query = `
Expand Down Expand Up @@ -715,7 +717,9 @@ export class AnalyticsRequestViewRepository {

// Important: This is the only place where we scope the data to a particular organization and graph.
// We can only filter for data that is part of the JWT token otherwise a user could send us whatever they want.
const scopedSql = ` AND FederatedGraphID = '${federatedGraphId}' AND OrganizationID = '${organizationId}'`;
coercedQueryParams.scopedFederatedGraphId = federatedGraphId;
coercedQueryParams.scopedOrganizationId = organizationId;
const scopedSql = ` AND FederatedGraphID = {scopedFederatedGraphId:String} AND OrganizationID = {scopedOrganizationId:String}`;

whereSql += scopedSql;

Expand Down
Loading
Loading