Skip to content

Commit 6121cc7

Browse files
maysunfaisalmaysunfaisal
committed
feat(lightspeed): Address PR reviews, add changeset
Assisted-by: Claude Opus 4.6 Generated-by: Cursor Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
1 parent c245c6f commit 6121cc7

File tree

6 files changed

+129
-70
lines changed

6 files changed

+129
-70
lines changed

‎workspaces/lightspeed/.changeset/lemon-walls-fly.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-lightspeed-backend': minor
3+
'@red-hat-developer-hub/backstage-plugin-lightspeed-common': minor
4+
---
5+
6+
Added MCP Server management backend APIs with per-user preferences, on-demand validation, and new permissions (lightspeed.mcp.read, lightspeed.mcp.manage)

‎workspaces/lightspeed/app-config.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ backend:
3030
database:
3131
client: better-sqlite3
3232
connection: ':memory:'
33-
# Use the directory property to store the database in a file for local development
34-
# The database is used for the user preferences and MCP servers settings
35-
# directory: './sqlite-data'
33+
# To persist the database to a file for local development, replace the above with:
34+
# connection:
35+
# directory: './sqlite-data'
3636
# OpenShift / RHDH production — uses the PostgreSQL instance managed by the
3737
# RHDH Helm chart or Operator. These env vars are injected from the
3838
# PostgreSQL Secret into the Backstage container by the deployment config.

‎workspaces/lightspeed/plugins/lightspeed-backend/src/service/mcp-server-store.ts‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -49,29 +49,14 @@ export class McpUserSettingsStore {
4949
.first();
5050
}
5151

52-
/** Create or update user settings for a server. */
52+
/** Create or update user settings for a server (atomic). */
5353
async upsert(
5454
serverName: string,
5555
userEntityRef: string,
5656
updates: { enabled?: boolean; token?: string | null },
5757
): Promise<McpUserSettingsRow> {
58-
const existing = await this.get(serverName, userEntityRef);
5958
const now = new Date().toISOString();
6059

61-
if (existing) {
62-
const fields: Partial<McpUserSettingsRow> = { updated_at: now };
63-
if (updates.enabled !== undefined) fields.enabled = updates.enabled;
64-
if (updates.token !== undefined) {
65-
fields.token = updates.token;
66-
if (updates.token) fields.status = 'unknown';
67-
}
68-
69-
await this.db(TABLE)
70-
.where({ server_name: serverName, user_entity_ref: userEntityRef })
71-
.update(fields);
72-
return (await this.get(serverName, userEntityRef))!;
73-
}
74-
7560
const row: McpUserSettingsRow = {
7661
id: randomUUID(),
7762
server_name: serverName,
@@ -83,26 +68,41 @@ export class McpUserSettingsStore {
8368
created_at: now,
8469
updated_at: now,
8570
};
86-
await this.db(TABLE).insert(row);
87-
return row;
71+
72+
const mergeFields: Partial<McpUserSettingsRow> = { updated_at: now };
73+
if (updates.enabled !== undefined) mergeFields.enabled = updates.enabled;
74+
if (updates.token !== undefined) {
75+
mergeFields.token = updates.token;
76+
// Reset cached validation when token changes (new or cleared)
77+
mergeFields.status = 'unknown';
78+
mergeFields.tool_count = 0;
79+
}
80+
81+
await this.db(TABLE)
82+
.insert(row)
83+
.onConflict(['server_name', 'user_entity_ref'])
84+
.merge(mergeFields);
85+
86+
const result = await this.get(serverName, userEntityRef);
87+
if (!result) {
88+
throw new Error(
89+
`Failed to upsert settings for ${serverName}/${userEntityRef}`,
90+
);
91+
}
92+
return result;
8893
}
8994

90-
/** Update cached validation status for a user's server setting. */
95+
/** Update cached validation status for a user's server setting (atomic). */
9196
async updateStatus(
9297
serverName: string,
9398
userEntityRef: string,
9499
status: McpServerStatus,
95100
toolCount: number,
96101
): Promise<void> {
97-
const existing = await this.get(serverName, userEntityRef);
98102
const now = new Date().toISOString();
99103

100-
if (existing) {
101-
await this.db(TABLE)
102-
.where({ server_name: serverName, user_entity_ref: userEntityRef })
103-
.update({ status, tool_count: toolCount, updated_at: now });
104-
} else {
105-
await this.db(TABLE).insert({
104+
await this.db(TABLE)
105+
.insert({
106106
id: randomUUID(),
107107
server_name: serverName,
108108
user_entity_ref: userEntityRef,
@@ -112,7 +112,8 @@ export class McpUserSettingsStore {
112112
tool_count: toolCount,
113113
created_at: now,
114114
updated_at: now,
115-
});
116-
}
115+
})
116+
.onConflict(['server_name', 'user_entity_ref'])
117+
.merge({ status, tool_count: toolCount, updated_at: now });
117118
}
118119
}

‎workspaces/lightspeed/plugins/lightspeed-backend/src/service/mcp-server-validator.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,8 @@ export class McpServerValidator {
205205
}
206206
}
207207
} catch (error) {
208-
this.logger.warn(`Failed to parse SSE tools/list response: ${error}`);
208+
const msg = error instanceof Error ? error.message : String(error);
209+
this.logger.warn(`Failed to parse SSE tools/list response: ${msg}`);
209210
}
210211
return undefined;
211212
}

‎workspaces/lightspeed/plugins/lightspeed-backend/src/service/mcp-server.test.ts‎

Lines changed: 55 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,29 @@ jest.mock('@backstage/backend-plugin-api', () => ({
8888
})),
8989
}));
9090

91+
async function startBackendServer(
92+
config?: Record<PropertyKey, unknown>,
93+
authorizeResult?: AuthorizeResult.DENY | AuthorizeResult.ALLOW,
94+
) {
95+
const features: (BackendFeature | Promise<{ default: BackendFeature }>)[] = [
96+
lightspeedPlugin,
97+
mockServices.rootLogger.factory(),
98+
mockServices.rootConfig.factory({
99+
data: { ...BASE_CONFIG, ...config },
100+
}),
101+
mockServices.httpAuth.factory({
102+
defaultCredentials: mockCredentials.user(mockUserId),
103+
}),
104+
mockServices.permissions.mock({
105+
authorize: async () => [
106+
{ result: authorizeResult ?? AuthorizeResult.ALLOW },
107+
],
108+
}).factory,
109+
mockServices.userInfo.factory(),
110+
];
111+
return (await startTestBackend({ features })).server;
112+
}
113+
91114
describe('MCP server management endpoints', () => {
92115
const server = setupServer(...handlers, ...lcsHandlers, ...mcpHandlers);
93116

@@ -111,30 +134,6 @@ describe('MCP server management endpoints', () => {
111134
server.resetHandlers();
112135
});
113136

114-
async function startBackendServer(
115-
config?: Record<PropertyKey, unknown>,
116-
authorizeResult?: AuthorizeResult.DENY | AuthorizeResult.ALLOW,
117-
) {
118-
const features: (BackendFeature | Promise<{ default: BackendFeature }>)[] =
119-
[
120-
lightspeedPlugin,
121-
mockServices.rootLogger.factory(),
122-
mockServices.rootConfig.factory({
123-
data: { ...BASE_CONFIG, ...(config || {}) },
124-
}),
125-
mockServices.httpAuth.factory({
126-
defaultCredentials: mockCredentials.user(mockUserId),
127-
}),
128-
mockServices.permissions.mock({
129-
authorize: async () => [
130-
{ result: authorizeResult ?? AuthorizeResult.ALLOW },
131-
],
132-
}).factory,
133-
mockServices.userInfo.factory(),
134-
];
135-
return (await startTestBackend({ features })).server;
136-
}
137-
138137
// ─── GET /mcp-servers ─────────────────────────────────────────────
139138

140139
describe('GET /mcp-servers', () => {
@@ -277,6 +276,24 @@ describe('MCP server management endpoints', () => {
277276
expect(patchRes.status).toBe(404);
278277
});
279278

279+
it('resets status when token is cleared', async () => {
280+
const backendServer = await startBackendServer(MCP_CONFIG);
281+
282+
// First set a valid token (triggers validation → connected)
283+
await request(backendServer)
284+
.patch('/api/lightspeed/mcp-servers/static-mcp')
285+
.send({ token: MOCK_MCP_VALID_TOKEN });
286+
287+
// Clear the token
288+
const clearRes = await request(backendServer)
289+
.patch('/api/lightspeed/mcp-servers/static-mcp')
290+
.send({ token: null });
291+
292+
expect(clearRes.status).toBe(200);
293+
expect(clearRes.body.server.status).toBe('unknown');
294+
expect(clearRes.body.server.toolCount).toBe(0);
295+
});
296+
280297
it('returns 400 when no fields provided', async () => {
281298
const backendServer = await startBackendServer(MCP_CONFIG);
282299
const patchRes = await request(backendServer)
@@ -303,8 +320,8 @@ describe('MCP server management endpoints', () => {
303320
// ─── POST /mcp-servers/validate (generic) ─────────────────────────
304321

305322
describe('POST /mcp-servers/validate', () => {
306-
it('validates valid credentials', async () => {
307-
const backendServer = await startBackendServer();
323+
it('validates valid credentials with LCS-known URL', async () => {
324+
const backendServer = await startBackendServer(MCP_CONFIG);
308325
const response = await request(backendServer)
309326
.post('/api/lightspeed/mcp-servers/validate')
310327
.send({ url: MOCK_MCP_ADDR, token: MOCK_MCP_VALID_TOKEN });
@@ -315,8 +332,8 @@ describe('MCP server management endpoints', () => {
315332
expect(response.body.tools).toHaveLength(3);
316333
});
317334

318-
it('validates invalid credentials', async () => {
319-
const backendServer = await startBackendServer();
335+
it('validates invalid credentials with LCS-known URL', async () => {
336+
const backendServer = await startBackendServer(MCP_CONFIG);
320337
const response = await request(backendServer)
321338
.post('/api/lightspeed/mcp-servers/validate')
322339
.send({ url: MOCK_MCP_ADDR, token: 'bad-token' });
@@ -334,6 +351,16 @@ describe('MCP server management endpoints', () => {
334351
expect(response.status).toBe(400);
335352
expect(response.body.error).toContain('url and token are required');
336353
});
354+
355+
it('rejects unknown URL (SSRF protection)', async () => {
356+
const backendServer = await startBackendServer(MCP_CONFIG);
357+
const response = await request(backendServer)
358+
.post('/api/lightspeed/mcp-servers/validate')
359+
.send({ url: 'http://internal-service:1234', token: 'some-token' });
360+
361+
expect(response.status).toBe(400);
362+
expect(response.body.error).toContain('URL not recognized');
363+
});
337364
});
338365

339366
// ─── POST /mcp-servers/:name/validate (on-demand) ──────────────────

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

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,9 @@ export async function createRouter(
130130

131131
async function refreshLcsUrlCache(): Promise<void> {
132132
try {
133-
const response = await fetch(`http://0.0.0.0:${port}/v1/mcp-servers`);
133+
const response = await fetch(`http://0.0.0.0:${port}/v1/mcp-servers`, {
134+
signal: AbortSignal.timeout(5000),
135+
});
134136
if (!response.ok) {
135137
logger.warn(
136138
`Failed to fetch MCP server URLs from LCS: HTTP ${response.status}`,
@@ -145,7 +147,8 @@ export async function createRouter(
145147
}
146148
logger.info(`Cached ${lcsUrlCache.size} MCP server URL(s) from LCS`);
147149
} catch (error) {
148-
logger.warn(`Failed to fetch MCP server URLs from LCS: ${error}`);
150+
const msg = error instanceof Error ? error.message : String(error);
151+
logger.warn(`Failed to fetch MCP server URLs from LCS: ${msg}`);
149152
}
150153
}
151154

@@ -186,7 +189,7 @@ export async function createRouter(
186189
name: server.name,
187190
url: resolveServerUrl(server.name),
188191
enabled: setting ? Boolean(setting.enabled) : true,
189-
status: (setting?.status as McpServerStatus) ?? 'unknown',
192+
status: setting?.status ?? 'unknown',
190193
toolCount: setting?.tool_count ?? 0,
191194
hasToken: !!(setting?.token || server.token),
192195
};
@@ -214,6 +217,20 @@ export async function createRouter(
214217
return;
215218
}
216219

220+
// SSRF protection: only allow URLs registered in LCS
221+
let knownUrls = new Set(lcsUrlCache.values());
222+
if (!knownUrls.has(url)) {
223+
await refreshLcsUrlCache();
224+
knownUrls = new Set(lcsUrlCache.values());
225+
}
226+
if (!knownUrls.has(url)) {
227+
res.status(400).json({
228+
error:
229+
'URL not recognized — only MCP server URLs registered in LCS are allowed',
230+
});
231+
return;
232+
}
233+
217234
const result = await mcpValidator.validate(url, token);
218235
res.json(result);
219236
} catch (error) {
@@ -229,18 +246,21 @@ export async function createRouter(
229246
router.post('/mcp-servers/:name/validate', async (req, res) => {
230247
try {
231248
const credentials = await httpAuth.credentials(req);
232-
await authorizer.authorizeUser(lightspeedMcpReadPermission, credentials);
249+
await authorizer.authorizeUser(
250+
lightspeedMcpManagePermission,
251+
credentials,
252+
);
233253
const user = await userInfo.getUserInfo(credentials);
234254

235255
const { name } = req.params;
236256
const server = staticServers.find(s => s.name === name);
237257
if (!server) {
238258
res.status(404).json({
239-
error: `MCP server '${name}' not found in configuration`,
259+
error: `MCP server '${name}' is not configured — it must be defined in the Lightspeed Stack config and listed under lightspeed.mcpServers in app-config`,
240260
});
241261
return;
242262
}
243-
// Resolve URL: config override → LCS cache → fresh LCS fetch
263+
// Resolve URL: LCS cache → fresh LCS fetch
244264
let serverUrl = resolveServerUrl(server.name);
245265
if (!serverUrl) {
246266
await refreshLcsUrlCache();
@@ -301,7 +321,7 @@ export async function createRouter(
301321
const server = staticServers.find(s => s.name === name);
302322
if (!server) {
303323
res.status(404).json({
304-
error: `MCP server '${name}' not found in configuration`,
324+
error: `MCP server '${name}' is not configured — it must be defined in the Lightspeed Stack config and listed under lightspeed.mcpServers in app-config`,
305325
});
306326
return;
307327
}
@@ -320,7 +340,11 @@ export async function createRouter(
320340
});
321341

322342
let validation: McpValidationResult | undefined;
323-
const serverUrl = resolveServerUrl(server.name);
343+
let serverUrl = resolveServerUrl(server.name);
344+
if (token && !serverUrl) {
345+
await refreshLcsUrlCache();
346+
serverUrl = resolveServerUrl(server.name);
347+
}
324348
if (token && serverUrl) {
325349
validation = await mcpValidator.validate(serverUrl, token);
326350
const newStatus: McpServerStatus = validation.valid
@@ -341,10 +365,10 @@ export async function createRouter(
341365
name: server.name,
342366
url: resolveServerUrl(server.name),
343367
enabled: Boolean(setting.enabled),
344-
status: setting.status as McpServerStatus,
368+
status: setting.status,
345369
toolCount: setting.tool_count,
346370
hasToken: !!(setting.token || server.token),
347-
} as McpServerResponse,
371+
},
348372
};
349373
if (validation) result.validation = validation;
350374
res.json(result);

0 commit comments

Comments
 (0)
⚡