Skip to content

Commit cd803ed

Browse files
authored
[RHIDP-13060] harden lightspeed proxy against arbitrary routes (#2970)
* harden lightspeed proxy against arbitrary routes Signed-off-by: Jordan Dubrick <jdubrick@redhat.com> * add changeset Signed-off-by: Jordan Dubrick <jdubrick@redhat.com> * add test displaying express normalization for path traversal Signed-off-by: Jordan Dubrick <jdubrick@redhat.com> * add comment for why feedback in both constant arrays Signed-off-by: Jordan Dubrick <jdubrick@redhat.com> --------- Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
1 parent c7ff423 commit cd803ed

6 files changed

Lines changed: 143 additions & 8 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-lightspeed-backend': patch
3+
---
4+
5+
harden proxy passthrough by adding allowlist for routes

workspaces/lightspeed/plugins/lightspeed-backend/src/service/constant.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,28 @@ export const HTTP_STATUS_NOT_FOUND = 404; // Not found
8585
export const HTTP_STATUS_CONFLICT = 409; // Conflict
8686
export const HTTP_STATUS_INTERNAL_ERROR = 500; // Internal server error
8787

88+
/**
89+
* Proxy path security - only these LCORE path prefixes may be proxied
90+
* Avoids authenticated users hitting arbitrary LCORE endpoints
91+
* /v1/feedback is here to cover the /feedback/status case as
92+
* the exact /v1/feedback has its own handler
93+
*/
94+
export const ALLOWED_PROXY_PREFIXES = [
95+
'/v1/models',
96+
'/v1/shields',
97+
'/v2/conversations',
98+
'/v1/feedback',
99+
];
100+
101+
/**
102+
* Paths that bypass the proxy middleware and are handled by dedicated route handlers
103+
*/
104+
export const PROXY_PASSTHROUGH_PATHS = [
105+
'/v1/query',
106+
'/v1/query/interrupt',
107+
'/v1/feedback',
108+
];
109+
88110
/**
89111
* SSRF Protection - Blocked hostnames for security
90112
* These hostnames are commonly used for Server-Side Request Forgery attacks

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,51 @@ describe('lightspeed router tests', () => {
711711
});
712712
});
713713

714+
describe('proxy path allowlist', () => {
715+
it('should return 404 for non-allowlisted path /v1/admin', async () => {
716+
const backendServer = await startBackendServer();
717+
const response = await request(backendServer).get(
718+
'/api/lightspeed/v1/admin',
719+
);
720+
expect(response.statusCode).toEqual(404);
721+
expect(response.body).toEqual({
722+
error: 'Requested path is not available',
723+
});
724+
});
725+
726+
it('should return 404 for non-allowlisted path /internal/config', async () => {
727+
const backendServer = await startBackendServer();
728+
const response = await request(backendServer).get(
729+
'/api/lightspeed/internal/config',
730+
);
731+
expect(response.statusCode).toEqual(404);
732+
expect(response.body).toEqual({
733+
error: 'Requested path is not available',
734+
});
735+
});
736+
737+
it('should return 404 for POST to arbitrary path', async () => {
738+
const backendServer = await startBackendServer();
739+
const response = await request(backendServer)
740+
.post('/api/lightspeed/some/arbitrary/path')
741+
.send({});
742+
expect(response.statusCode).toEqual(404);
743+
expect(response.body).toEqual({
744+
error: 'Requested path is not available',
745+
});
746+
});
747+
748+
it('should reject dot-segment path traversal attempts', async () => {
749+
const backendServer = await startBackendServer();
750+
// Express normalizes /v1/models/../admin to /v1/admin before
751+
// the request reaches our middleware, so the allowlist rejects it.
752+
const response = await request(backendServer).get(
753+
'/api/lightspeed/v1/models/../admin',
754+
);
755+
expect(response.statusCode).toEqual(404);
756+
});
757+
});
758+
714759
describe('POST /v1/query/interrupt', () => {
715760
it('returns success when interrupt succeeds', async () => {
716761
const backendServer = await startBackendServer();

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ import {
3232

3333
import { Readable } from 'node:stream';
3434

35-
import { DEFAULT_LIGHTSPEED_SERVICE_PORT } from './constant';
35+
import {
36+
DEFAULT_LIGHTSPEED_SERVICE_PORT,
37+
PROXY_PASSTHROUGH_PATHS,
38+
} from './constant';
3639
import { McpUserSettingsStore } from './mcp-server-store';
3740
import {
3841
McpServerResponse,
@@ -48,7 +51,7 @@ import {
4851
QueryRequestBody,
4952
RouterOptions,
5053
} from './types';
51-
import { validateCompletionsRequest } from './validation';
54+
import { isAllowedProxyPath, validateCompletionsRequest } from './validation';
5255

5356
const SKIP_USER_ID_ENDPOINTS = new Set(['/v1/models', '/v1/shields']);
5457

@@ -413,19 +416,19 @@ export async function createRouter(
413416
// ─── Proxy Middleware (existing) ────────────────────────────────────
414417

415418
router.use('/', async (req, res, next) => {
416-
const passthroughPaths = [
417-
'/v1/query',
418-
'/v1/query/interrupt',
419-
'/v1/feedback',
420-
];
421419
// Skip middleware for notebooks routes and specific paths
422420
if (
423421
req.path.startsWith('/notebooks') ||
424-
passthroughPaths.includes(req.path) ||
422+
PROXY_PASSTHROUGH_PATHS.includes(req.path) ||
425423
req.method === 'PUT'
426424
) {
427425
return next();
428426
}
427+
428+
if (!isAllowedProxyPath(req.path)) {
429+
return res.status(404).json({ error: 'Requested path is not available' });
430+
}
431+
429432
// TODO: parse server_id from req.body and get URL and token when multi-server is supported
430433
const credentials = await httpAuth.credentials(req);
431434
const user = await userInfo.getUserInfo(credentials);
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright Red Hat, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { isAllowedProxyPath } from './validation';
18+
19+
describe('isAllowedProxyPath', () => {
20+
it.each(['/v1/models', '/v1/shields', '/v2/conversations', '/v1/feedback'])(
21+
'should allow exact match for %s',
22+
path => {
23+
expect(isAllowedProxyPath(path)).toBe(true);
24+
},
25+
);
26+
27+
it.each([
28+
['/v2/conversations/conv-123', '/v2/conversations'],
29+
['/v1/feedback/status', '/v1/feedback'],
30+
])('should allow sub-path %s under prefix %s', path => {
31+
expect(isAllowedProxyPath(path)).toBe(true);
32+
});
33+
34+
it.each([
35+
'/v1/admin',
36+
'/internal/config',
37+
'/metrics',
38+
'/v3/something',
39+
'/debug',
40+
'',
41+
])('should reject arbitrary path %s', path => {
42+
expect(isAllowedProxyPath(path)).toBe(false);
43+
});
44+
45+
it.each([
46+
'/v1/models-secret',
47+
'/v1/shieldsadmin',
48+
'/v2/conversationsextra',
49+
'/v1/feedbackextra',
50+
])('should reject prefix false match %s', path => {
51+
expect(isAllowedProxyPath(path)).toBe(false);
52+
});
53+
});

workspaces/lightspeed/plugins/lightspeed-backend/src/service/validation.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,15 @@
1616

1717
import type { NextFunction, Request, Response } from 'express';
1818

19+
import { ALLOWED_PROXY_PREFIXES } from './constant';
1920
import { QueryRequestBody } from './types';
2021

22+
export function isAllowedProxyPath(path: string): boolean {
23+
return ALLOWED_PROXY_PREFIXES.some(
24+
prefix => path === prefix || path.startsWith(`${prefix}/`),
25+
);
26+
}
27+
2128
export const validateCompletionsRequest = (
2229
req: Request,
2330
res: Response,

0 commit comments

Comments
 (0)