Skip to content

Commit bd90a81

Browse files
n1ru4ln1ru4l
authored
fix: validate operation body (#5537)
1 parent ee0ec17 commit bd90a81

File tree

3 files changed

+145
-29
lines changed

3 files changed

+145
-29
lines changed

‎integration-tests/tests/usage-reporting/usage-reporting-format-2.spec.ts‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import { initSeed } from 'testkit/seed';
2+
import { TargetAccessScope } from '../../testkit/gql/graphql';
3+
import { getServiceHost } from '../../testkit/utils';
4+
5+
test('valid operation is accepted', async () => {
6+
const { createOrg } = await initSeed().createOwner();
7+
const { createProject } = await createOrg();
8+
const { createToken } = await createProject();
9+
const { secret: accessToken } = await createToken({
10+
targetScopes: [TargetAccessScope.RegistryWrite, TargetAccessScope.RegistryRead],
11+
});
12+
13+
const usageAddress = await getServiceHost('usage', 8081);
14+
15+
const response = await fetch(`http://${usageAddress}`, {
16+
method: 'POST',
17+
headers: {
18+
'X-Usage-API-Version': '2',
19+
'Content-Type': 'application/json',
20+
Authorization: `Bearer ${accessToken}`,
21+
},
22+
body: JSON.stringify({
23+
size: 3,
24+
map: {
25+
c3b6d9b0: {
26+
operationName: 'me',
27+
operation: 'query me { me { id name } }',
28+
fields: ['Query', 'Query.me', 'User', 'User.id', 'User.name'],
29+
},
30+
},
31+
operations: [
32+
{
33+
operationMapKey: 'c3b6d9b0',
34+
timestamp: 1663158676535,
35+
execution: {
36+
ok: true,
37+
duration: 150000000,
38+
errorsTotal: 0,
39+
},
40+
metadata: {
41+
client: {
42+
name: 'demo',
43+
version: '0.0.1',
44+
},
45+
},
46+
},
47+
],
48+
}),
49+
});
50+
expect(response.status).toBe(200);
51+
expect(await response.json()).toMatchObject({
52+
id: expect.any(String),
53+
operations: {
54+
accepted: 1,
55+
rejected: 0,
56+
},
57+
});
58+
});
59+
60+
test('invalid operation is rejected', async () => {
61+
const { createOrg } = await initSeed().createOwner();
62+
const { createProject } = await createOrg();
63+
const { createToken } = await createProject();
64+
const { secret: accessToken } = await createToken({
65+
targetScopes: [TargetAccessScope.RegistryWrite, TargetAccessScope.RegistryRead],
66+
});
67+
68+
const usageAddress = await getServiceHost('usage', 8081);
69+
// GraphQL operation is invalid at Query.me(id:)
70+
const faultyOperation = 'query me { me(id: ) { id name } }';
71+
72+
const response = await fetch(`http://${usageAddress}`, {
73+
method: 'POST',
74+
headers: {
75+
'X-Usage-API-Version': '2',
76+
'Content-Type': 'application/json',
77+
Authorization: `Bearer ${accessToken}`,
78+
},
79+
body: JSON.stringify({
80+
size: 3,
81+
map: {
82+
c3b6d9b0: {
83+
operationName: 'me',
84+
operation: faultyOperation,
85+
fields: ['Query', 'Query.me', 'User', 'User.id', 'User.name'],
86+
},
87+
},
88+
operations: [
89+
{
90+
operationMapKey: 'c3b6d9b0',
91+
timestamp: 1663158676535,
92+
execution: {
93+
ok: true,
94+
duration: 150000000,
95+
errorsTotal: 0,
96+
},
97+
metadata: {
98+
client: {
99+
name: 'demo',
100+
version: '0.0.1',
101+
},
102+
},
103+
},
104+
],
105+
}),
106+
});
107+
expect(response.status).toBe(200);
108+
expect(await response.json()).toMatchObject({
109+
id: expect.any(String),
110+
operations: {
111+
accepted: 0,
112+
rejected: 1,
113+
},
114+
});
115+
});

‎packages/services/usage/src/usage-processor-1.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -269,21 +269,21 @@ export function validateOperationMapRecord(record: OperationMapRecord) {
269269
};
270270
}
271271

272-
function isValidOperationBody(op: OperationMapRecord) {
273-
const cached = validOperationBodyCache.get(op.operation);
272+
export function isValidOperationBody(operation: string) {
273+
const cached = validOperationBodyCache.get(operation);
274274

275275
if (typeof cached === 'boolean') {
276276
return cached;
277277
}
278278

279279
try {
280-
parse(op.operation, {
280+
parse(operation, {
281281
noLocation: true,
282282
});
283-
validOperationBodyCache.set(op.operation, true);
283+
validOperationBodyCache.set(operation, true);
284284
return true;
285285
} catch (error) {
286-
validOperationBodyCache.set(op.operation, false);
286+
validOperationBodyCache.set(operation, false);
287287
return false;
288288
}
289289
}
@@ -304,7 +304,7 @@ export function validateOperation(operation: IncomingOperation, operationMap: Op
304304
}
305305

306306
if (validate(operation)) {
307-
if (!isValidOperationBody(operationMap[operation.operationMapKey])) {
307+
if (!isValidOperationBody(operationMap[operation.operationMapKey].operation)) {
308308
return {
309309
valid: false,
310310
errors: [

‎packages/services/usage/src/usage-processor-2.ts‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import * as tb from '@sinclair/typebox';
1010
import * as tc from '@sinclair/typebox/compiler';
1111
import { invalidRawOperations, rawOperationsSize, totalOperations, totalReports } from './metrics';
1212
import { TokensResponse } from './tokens';
13+
import { isValidOperationBody } from './usage-processor-1';
1314

1415
export function usageProcessorV2(
1516
logger: Logger,
@@ -74,15 +75,35 @@ export function usageProcessorV2(
7475

7576
const newKeyMappings = new Map<OperationMapRecord, string>();
7677

77-
function getOperationMapRecord(operationMapKey: string): string | null {
78+
function getOperationMapRecordKey(operationMapKey: string): string | null {
7879
const operationMapRecord = incoming.map[operationMapKey] as OperationMapRecord | undefined;
7980

8081
if (!operationMapRecord) {
82+
logger.warn(
83+
`Detected invalid operation. Operation map key could not be found. (target=%s): %s`,
84+
token.target,
85+
operationMapKey,
86+
);
87+
invalidRawOperations
88+
.labels({
89+
reason: 'operation_map_key_not_found',
90+
})
91+
.inc(1);
8192
return null;
8293
}
8394

8495
let newOperationMapKey = newKeyMappings.get(operationMapRecord);
8596

97+
if (!isValidOperationBody(operationMapRecord.operation)) {
98+
logger.warn(`Detected invalid operation (target=%s): %s`, operationMapKey);
99+
invalidRawOperations
100+
.labels({
101+
reason: 'invalid_operation_body',
102+
})
103+
.inc(1);
104+
return null;
105+
}
106+
86107
if (newOperationMapKey === undefined) {
87108
const sortedFields = operationMapRecord.fields.sort();
88109
newOperationMapKey = createHash('md5')
@@ -106,20 +127,10 @@ export function usageProcessorV2(
106127
}
107128

108129
for (const operation of incomingOperations) {
109-
const operationMapKey = getOperationMapRecord(operation.operationMapKey);
130+
const operationMapKey = getOperationMapRecordKey(operation.operationMapKey);
110131

111132
// if the record does not exist -> skip the operation
112133
if (operationMapKey === null) {
113-
logger.warn(
114-
`Detected invalid operation. Operation map key could not be found. (target=%s): %s`,
115-
token.target,
116-
operation.operationMapKey,
117-
);
118-
invalidRawOperations
119-
.labels({
120-
reason: 'operation_map_key_not_found',
121-
})
122-
.inc(1);
123134
continue;
124135
}
125136

@@ -154,20 +165,10 @@ export function usageProcessorV2(
154165
}
155166

156167
for (const operation of incomingSubscriptionOperations) {
157-
const operationMapKey = getOperationMapRecord(operation.operationMapKey);
168+
const operationMapKey = getOperationMapRecordKey(operation.operationMapKey);
158169

159170
// if the record does not exist -> skip the operation
160171
if (operationMapKey === null) {
161-
logger.warn(
162-
`Detected invalid operation. Operation map key could not be found. (target=%s): %s`,
163-
token.target,
164-
operation.operationMapKey,
165-
);
166-
invalidRawOperations
167-
.labels({
168-
reason: 'operation_map_key_not_found',
169-
})
170-
.inc(1);
171172
continue;
172173
}
173174

0 commit comments

Comments
 (0)