Skip to content

Commit 058dcfe

Browse files
committed
refactor(cost-management): reduce secureProxy cognitive complexity
Extract three helper functions from the secureProxy handler to bring cognitive complexity from 24 down below the SonarQube threshold of 15: - isPathTraversal: path validation guard - parseClientQueryParams: query string parsing with RBAC key stripping - injectRbacFilters: server-side RBAC filter injection Made-with: Cursor
1 parent 90ad7a3 commit 058dcfe

17 files changed

Lines changed: 1007 additions & 55 deletions

File tree

workspaces/cost-management/plugins/cost-management-backend/src/routes/secureProxy.ts

Lines changed: 69 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,59 @@ async function resolveCostManagementAccess(
228228
);
229229
}
230230

231+
function isPathTraversal(proxyPath: string): boolean {
232+
return /(?:^|\/)\.\.(\/|$)/.test(proxyPath) || proxyPath.startsWith('/');
233+
}
234+
235+
/**
236+
* Parses the raw query string, stripping RBAC-controlled keys and
237+
* decoding percent-encoded parameters. Returns null if encoding is malformed.
238+
*/
239+
function parseClientQueryParams(
240+
originalUrl: string,
241+
rbacControlledKeys: Set<string>,
242+
): { key: string; value: string }[] | null {
243+
const rawQuery = originalUrl.split('?')[1] || '';
244+
const rawParams = rawQuery.split('&').filter(p => p.length > 0);
245+
const result: { key: string; value: string }[] = [];
246+
247+
for (const param of rawParams) {
248+
const eqIdx = param.indexOf('=');
249+
const rawKey = eqIdx >= 0 ? param.substring(0, eqIdx) : param;
250+
const rawVal = eqIdx >= 0 ? param.substring(eqIdx + 1) : '';
251+
252+
try {
253+
const decodedKey = decodeURIComponent(rawKey);
254+
const decodedVal = decodeURIComponent(rawVal);
255+
if (!rbacControlledKeys.has(decodedKey)) {
256+
result.push({ key: decodedKey, value: decodedVal });
257+
}
258+
} catch {
259+
return null;
260+
}
261+
}
262+
263+
return result;
264+
}
265+
266+
/**
267+
* Appends server-side RBAC cluster/project filters to the target URL
268+
* using the appropriate key format for ROS vs Cost Management APIs.
269+
*/
270+
function injectRbacFilters(targetUrl: URL, access: AccessResult): void {
271+
const clusterKey =
272+
access.filterStyle === 'ros' ? 'cluster' : 'filter[exact:cluster]';
273+
const projectKey =
274+
access.filterStyle === 'ros' ? 'project' : 'filter[exact:project]';
275+
276+
access.clusterFilters.forEach(c =>
277+
targetUrl.searchParams.append(clusterKey, c),
278+
);
279+
access.projectFilters.forEach(p =>
280+
targetUrl.searchParams.append(projectKey, p),
281+
);
282+
}
283+
231284
/**
232285
* Server-side proxy that keeps the SSO token on the backend and enforces
233286
* RBAC before forwarding requests to the Cost Management API.
@@ -244,7 +297,7 @@ export const secureProxy: (options: RouterOptions) => RequestHandler =
244297
return res.status(400).json({ error: 'Missing proxy path' });
245298
}
246299

247-
if (/(?:^|\/)\.\.(\/|$)/.test(proxyPath) || proxyPath.startsWith('/')) {
300+
if (isPathTraversal(proxyPath)) {
248301
return res
249302
.status(400)
250303
.json({ error: 'Invalid proxy path: traversal not allowed' });
@@ -273,89 +326,50 @@ export const secureProxy: (options: RouterOptions) => RequestHandler =
273326

274327
// Express qs parser converts bracket keys like filter[time_scope_value]
275328
// into nested objects, losing the flat key format the RHCC API expects.
276-
// Decode each param before matching so percent-encoded variants of
277-
// RBAC-controlled keys (e.g. filter%5Bexact%3Acluster%5D) are also caught.
278329
const rbacControlledKeys = new Set(
279330
access.filterStyle === 'ros'
280331
? ['cluster', 'project']
281332
: ['filter[exact:cluster]', 'filter[exact:project]'],
282333
);
283334

284-
const rawQuery = req.originalUrl.split('?')[1] || '';
285-
const rawParams = rawQuery.split('&').filter(p => p.length > 0);
286-
287-
for (const param of rawParams) {
288-
const eqIdx = param.indexOf('=');
289-
const rawKey = eqIdx >= 0 ? param.substring(0, eqIdx) : param;
290-
const rawVal = eqIdx >= 0 ? param.substring(eqIdx + 1) : '';
291-
292-
let decodedKey: string;
293-
let decodedVal: string;
294-
try {
295-
decodedKey = decodeURIComponent(rawKey);
296-
decodedVal = decodeURIComponent(rawVal);
297-
} catch {
298-
return res
299-
.status(400)
300-
.json({ error: 'Malformed percent-encoding in query string' });
301-
}
302-
303-
if (!rbacControlledKeys.has(decodedKey)) {
304-
targetUrl.searchParams.append(decodedKey, decodedVal);
305-
}
335+
const clientParams = parseClientQueryParams(
336+
req.originalUrl,
337+
rbacControlledKeys,
338+
);
339+
if (!clientParams) {
340+
return res
341+
.status(400)
342+
.json({ error: 'Malformed percent-encoding in query string' });
306343
}
307344

308-
// Inject server-side RBAC filters (empty arrays = full access, no filter needed)
309-
if (access.clusterFilters.length > 0) {
310-
if (access.filterStyle === 'ros') {
311-
access.clusterFilters.forEach(c =>
312-
targetUrl.searchParams.append('cluster', c),
313-
);
314-
} else {
315-
access.clusterFilters.forEach(c =>
316-
targetUrl.searchParams.append('filter[exact:cluster]', c),
317-
);
318-
}
319-
}
320-
if (access.projectFilters.length > 0) {
321-
if (access.filterStyle === 'ros') {
322-
access.projectFilters.forEach(p =>
323-
targetUrl.searchParams.append('project', p),
324-
);
325-
} else {
326-
access.projectFilters.forEach(p =>
327-
targetUrl.searchParams.append('filter[exact:project]', p),
328-
);
329-
}
345+
for (const { key, value } of clientParams) {
346+
targetUrl.searchParams.append(key, value);
330347
}
331348

349+
injectRbacFilters(targetUrl, access);
350+
332351
logger.info(
333352
`Proxying ${req.method} to ${targetUrl.pathname}${targetUrl.search}`,
334353
);
335354

336-
const acceptHeader = req.headers.accept || 'application/json';
337-
338355
const upstreamResponse = await fetch(targetUrl.toString(), {
339356
headers: {
340357
'Content-Type': 'application/json',
341-
Accept: acceptHeader,
358+
Accept: req.headers.accept || 'application/json',
342359
Authorization: `Bearer ${token}`,
343360
},
344361
method: 'GET',
345362
});
346363

347364
const contentType = upstreamResponse.headers.get('content-type') || '';
348-
349365
res.status(upstreamResponse.status);
350366

351367
if (contentType.includes('application/json')) {
352-
const data = await upstreamResponse.json();
353-
return res.json(data);
368+
return res.json(await upstreamResponse.json());
354369
}
355370

356-
const text = await upstreamResponse.text();
357371
res.set('Content-Type', contentType);
358-
return res.send(text);
372+
return res.send(await upstreamResponse.text());
359373
} catch (error) {
360374
logger.error('Secure proxy error', error);
361375
return res.status(500).json({ error: 'Internal proxy error' });
4.3 MB
Binary file not shown.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"status": "failed",
3+
"failedTests": []
4+
}

0 commit comments

Comments
 (0)