From 5a5da821d68ea7d84dfa27a1d3bb9493637276e4 Mon Sep 17 00:00:00 2001 From: gharden Date: Thu, 26 Mar 2026 13:09:53 -0400 Subject: [PATCH 1/4] fix(cost-management): add authorization, validation, and confirmation for Apply Recommendation - New ros.apply permission required to execute Apply Recommendation workflow - New POST /apply-recommendation backend endpoint validates resourceType against server-side allowlist and checks ros.apply permission before forwarding to Orchestrator - Workflow execution now routes through the cost-management backend instead of directly to the Orchestrator plugin, enabling server-side authorization and audit logging - Confirmation dialog prevents accidental workflow execution - Register costPluginPermissions in permission integration router (was previously missing) Fixes: FLPATH-3488, FLPATH-3492, FLPATH-3491 Made-with: Cursor --- .../.changeset/apply-recommendation-auth.md | 12 + workspaces/cost-management/docs/rbac.md | 31 +++ .../src/models/RouterOptions.ts | 4 + .../cost-management-backend/src/plugin.ts | 10 + .../src/routes/applyRecommendation.test.ts | 209 ++++++++++++++++++ .../src/routes/applyRecommendation.ts | 182 +++++++++++++++ .../src/service/router.test.ts | 2 + .../src/service/router.ts | 15 +- .../report-clients.api.md | 3 +- .../report-permissions.api.md | 6 + .../OrchestratorSlimClient.ts | 40 ++-- .../cost-management-common/src/permissions.ts | 9 + .../OptimizationEngineTab.tsx | 104 ++++++--- 13 files changed, 581 insertions(+), 46 deletions(-) create mode 100644 workspaces/cost-management/.changeset/apply-recommendation-auth.md create mode 100644 workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts create mode 100644 workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts diff --git a/workspaces/cost-management/.changeset/apply-recommendation-auth.md b/workspaces/cost-management/.changeset/apply-recommendation-auth.md new file mode 100644 index 00000000000..4419e9f6158 --- /dev/null +++ b/workspaces/cost-management/.changeset/apply-recommendation-auth.md @@ -0,0 +1,12 @@ +--- +'@red-hat-developer-hub/plugin-cost-management-backend': minor +'@red-hat-developer-hub/plugin-cost-management-common': minor +'@red-hat-developer-hub/plugin-cost-management': minor +--- + +Add authorization, input validation, and confirmation dialog for Apply Recommendation workflow. + +- New `ros.apply` permission required to execute the Apply Recommendation workflow +- New backend `POST /apply-recommendation` endpoint validates `resourceType` against server-side allowlist and checks `ros.apply` permission before forwarding to Orchestrator +- Workflow execution now routes through the cost-management backend instead of directly to the Orchestrator plugin, enabling server-side authorization and audit logging +- Confirmation dialog added before workflow execution to prevent accidental clicks diff --git a/workspaces/cost-management/docs/rbac.md b/workspaces/cost-management/docs/rbac.md index 20e4403346e..b203d0603a8 100644 --- a/workspaces/cost-management/docs/rbac.md +++ b/workspaces/cost-management/docs/rbac.md @@ -18,6 +18,17 @@ When a frontend request arrives at `/api/cost-management/proxy/*`, the backend: This means granting `ros.demolab` only allows seeing data for the `demolab` cluster — the user cannot modify query parameters to access other clusters. +### Apply Recommendation authorization + +When a user clicks "Apply recommendation", the frontend sends the request to the backend's `/api/cost-management/apply-recommendation` endpoint. The backend: + +1. Validates the `resourceType` against a server-side allowlist (`deployment`, `replicaset`, `daemonset`, `statefulset`, `deploymentconfig`, `replicationcontroller`) +2. Checks the `ros.apply` permission — the user must be explicitly granted this permission to execute workflows +3. Forwards the validated request to the Orchestrator plugin using service-to-service authentication +4. Audit logs the action (user, cluster, namespace, workload, workflow ID, outcome) + +A confirmation dialog on the frontend also prevents accidental clicks. + ## 1. Optimizations Section The Optimizations section allows users to view resource usage trends and optimization recommendations for workloads running on OpenShift clusters. @@ -29,6 +40,7 @@ The Optimizations section allows users to view resource usage trends and optimiz | ros.plugin | - | read | Allows the user to access all optimization data in the Cost Management plugin | | ros.[CLUSTER_NAME] | - | read | Allows the user to access optimization data for a specific Cluster in the Cost Management plugin | | ros.[CLUSTER_NAME].[PROJECT_NAME] | - | read | Allows the user to access optimization data for a specific Project within a specific Cluster in the Cost Management plugin | +| ros.apply | - | update | Allows the user to apply optimization recommendations via workflow execution | The user is permitted to do an action if either the generic permission or the specific one allows it. In other words, it is not possible to grant generic ros.plugin and then selectively disable it for a specific cluster via ros.[CLUSTER_NAME] with deny. @@ -70,6 +82,11 @@ p, role:default/rosUser, ros.demolab, read, allow p, role:default/rosUser, ros.demolab.thanos, read, allow p, role:default/rosUser, ros.OpenShift on Azure.mobile, read, allow +#### +# Optimizations Section (ros.) - Apply Recommendation permission +#### +p, role:default/rosUser, ros.apply, update, allow + #### # OpenShift Section (cost.) - Generic permissions #### @@ -131,6 +148,20 @@ p, role:default/rosClusterProjectUser, ros.demolab.thanos, read, allow g, user:default/test_user_3, role:default/rosClusterProjectUser ``` +#### ros.apply Permission + +Since the `test_user_7` user has the `default/rosApplyUser` role, which has `ros.apply` permission, it can: + +- Execute the Apply Recommendation workflow to modify workload resource configurations + +```csv +p, role:default/rosApplyUser, ros.apply, update, allow + +g, user:default/test_user_7, role:default/rosApplyUser +``` + +> **Note:** `ros.apply` is separate from the read permissions. A user can have read access to optimization data without being able to apply recommendations, and vice versa. Typically both `ros.plugin` (or cluster-specific read) and `ros.apply` are granted together. + ### OpenShift Cost Section #### cost.plugin Permission diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/models/RouterOptions.ts b/workspaces/cost-management/plugins/cost-management-backend/src/models/RouterOptions.ts index 565b59c4b91..bade0338192 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/models/RouterOptions.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/models/RouterOptions.ts @@ -24,6 +24,8 @@ import type { HttpAuthService, PermissionsService, CacheService, + DiscoveryService, + AuthService, } from '@backstage/backend-plugin-api'; /** @public */ @@ -33,6 +35,8 @@ export interface RouterOptions { httpAuth: HttpAuthService; permissions: PermissionsService; cache: CacheService; + discovery: DiscoveryService; + auth: AuthService; optimizationApi: OptimizationsApi; costManagementApi: CostManagementSlimApi; } diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/plugin.ts b/workspaces/cost-management/plugins/cost-management-backend/src/plugin.ts index a59f9cf7b92..a2c90dca714 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/plugin.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/plugin.ts @@ -38,6 +38,8 @@ export const costManagementPlugin = createBackendPlugin({ httpAuth: coreServices.httpAuth, permissions: coreServices.permissions, cache: coreServices.cache, + discovery: coreServices.discovery, + auth: coreServices.auth, optimizationApi: optimizationServiceRef, costManagementApi: costManagementServiceRef, }, @@ -48,6 +50,8 @@ export const costManagementPlugin = createBackendPlugin({ httpAuth, permissions, cache, + discovery, + auth, optimizationApi, costManagementApi, }) { @@ -57,6 +61,8 @@ export const costManagementPlugin = createBackendPlugin({ httpAuth, permissions, cache, + discovery, + auth, optimizationApi, costManagementApi, }); @@ -78,6 +84,10 @@ export const costManagementPlugin = createBackendPlugin({ path: '/proxy', allow: 'user-cookie', }); + httpRouter.addAuthPolicy({ + path: '/apply-recommendation', + allow: 'user-cookie', + }); }, }); }, diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts new file mode 100644 index 00000000000..2bb93617e5c --- /dev/null +++ b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts @@ -0,0 +1,209 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import express from 'express'; +import request from 'supertest'; +import { mockServices } from '@backstage/backend-test-utils'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { applyRecommendation } from './applyRecommendation'; +import type { RouterOptions } from '../models/RouterOptions'; + +const validBody = { + workflowId: 'patch-k8s-resource', + inputData: { + clusterName: 'test-cluster', + resourceType: 'deployment', + resourceNamespace: 'default', + resourceName: 'my-app', + containerName: 'main', + containerResources: { + limits: { cpu: 0.5, memory: 134217728 }, + requests: { cpu: 0.25, memory: 67108864 }, + }, + }, +}; + +describe('applyRecommendation', () => { + let app: express.Express; + let mockPermissions: ReturnType; + let mockDiscovery: ReturnType; + + beforeEach(() => { + jest.resetAllMocks(); + mockPermissions = mockServices.permissions.mock(); + mockDiscovery = mockServices.discovery(); + + const options: RouterOptions = { + logger: mockServices.rootLogger(), + httpAuth: mockServices.httpAuth(), + permissions: mockPermissions, + cache: mockServices.cache.mock(), + discovery: mockDiscovery, + auth: mockServices.auth(), + optimizationApi: { + getRecommendationList: jest.fn(), + getRecommendationById: jest.fn(), + }, + costManagementApi: { + getCostManagementReport: jest.fn(), + downloadCostManagementReport: jest.fn(), + searchOpenShiftProjects: jest.fn(), + searchOpenShiftClusters: jest.fn(), + searchOpenShiftNodes: jest.fn(), + getOpenShiftTags: jest.fn(), + getOpenShiftTagValues: jest.fn(), + }, + }; + + app = express(); + app.use(express.json()); + app.post('/apply-recommendation', applyRecommendation(options)); + }); + + it('returns 400 when workflowId is missing', async () => { + const response = await request(app) + .post('/apply-recommendation') + .send({ inputData: validBody.inputData }); + + expect(response.status).toBe(400); + expect(response.body.error).toContain('workflowId'); + }); + + it('returns 400 for invalid resourceType', async () => { + mockPermissions.authorize.mockResolvedValueOnce([ + { result: AuthorizeResult.ALLOW }, + ]); + + const response = await request(app) + .post('/apply-recommendation') + .send({ + ...validBody, + inputData: { ...validBody.inputData, resourceType: 'cronjob' }, + }); + + expect(response.status).toBe(400); + expect(response.body.error).toContain('Invalid resourceType'); + expect(response.body.error).toContain('cronjob'); + }); + + it('returns 400 when required fields are missing', async () => { + const response = await request(app) + .post('/apply-recommendation') + .send({ + workflowId: 'patch-k8s-resource', + inputData: { + resourceType: 'deployment', + containerResources: {}, + }, + }); + + expect(response.status).toBe(400); + expect(response.body.error).toContain('Missing or invalid'); + }); + + it('returns 403 when ros.apply permission is denied', async () => { + mockPermissions.authorize.mockResolvedValueOnce([ + { result: AuthorizeResult.DENY }, + ]); + + const response = await request(app) + .post('/apply-recommendation') + .send(validBody); + + expect(response.status).toBe(403); + expect(response.body.error).toContain('ros.apply'); + }); + + it('validates all allowed resourceType values', async () => { + const allowedTypes = [ + 'deployment', + 'replicaset', + 'daemonset', + 'statefulset', + 'deploymentconfig', + 'replicationcontroller', + ]; + + for (const resourceType of allowedTypes) { + mockPermissions.authorize.mockResolvedValueOnce([ + { result: AuthorizeResult.ALLOW }, + ]); + + // eslint-disable-next-line no-restricted-syntax + const fetchSpy = jest + .spyOn(globalThis, 'fetch') + .mockResolvedValueOnce( + new Response(JSON.stringify({ id: 'instance-1' }), { status: 200 }), + ); + + const response = await request(app) + .post('/apply-recommendation') + .send({ + ...validBody, + inputData: { ...validBody.inputData, resourceType }, + }); + + expect(response.status).toBe(200); + fetchSpy.mockRestore(); + } + }); + + it('forwards to orchestrator and returns instance id on success', async () => { + mockPermissions.authorize.mockResolvedValueOnce([ + { result: AuthorizeResult.ALLOW }, + ]); + + const fetchSpy = jest.spyOn(globalThis, 'fetch').mockResolvedValueOnce( + new Response(JSON.stringify({ id: 'workflow-instance-123' }), { + status: 200, + }), + ); + + const response = await request(app) + .post('/apply-recommendation') + .send(validBody); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ id: 'workflow-instance-123' }); + expect(fetchSpy).toHaveBeenCalledTimes(1); + + const fetchUrl = fetchSpy.mock.calls[0][0] as string; + expect(fetchUrl).toContain('/v2/workflows/patch-k8s-resource/execute'); + + fetchSpy.mockRestore(); + }); + + it('returns upstream error status on orchestrator failure', async () => { + mockPermissions.authorize.mockResolvedValueOnce([ + { result: AuthorizeResult.ALLOW }, + ]); + + const fetchSpy = jest.spyOn(globalThis, 'fetch').mockResolvedValueOnce( + new Response(JSON.stringify({ error: 'Workflow not found' }), { + status: 404, + }), + ); + + const response = await request(app) + .post('/apply-recommendation') + .send(validBody); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Workflow not found' }); + + fetchSpy.mockRestore(); + }); +}); diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts new file mode 100644 index 00000000000..e4b66ec553c --- /dev/null +++ b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts @@ -0,0 +1,182 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { RequestHandler } from 'express'; +import type { RouterOptions } from '../models/RouterOptions'; +import { authorize } from '../util/checkPermissions'; +import { rosApplyPermissions } from '@red-hat-developer-hub/plugin-cost-management-common/permissions'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; + +const ALLOWED_RESOURCE_TYPES = new Set([ + 'deployment', + 'replicaset', + 'daemonset', + 'statefulset', + 'deploymentconfig', + 'replicationcontroller', +]); + +interface ApplyRecommendationBody { + workflowId: string; + inputData: { + clusterName: string; + resourceType: string; + resourceNamespace: string; + resourceName: string; + containerName: string; + containerResources: { + limits?: { cpu?: number; memory?: number }; + requests?: { cpu?: number; memory?: number }; + }; + }; +} + +function validateBody( + body: unknown, +): + | { valid: true; data: ApplyRecommendationBody } + | { valid: false; error: string } { + const b = body as ApplyRecommendationBody; + + if (!b?.workflowId || typeof b.workflowId !== 'string') { + return { valid: false, error: 'Missing or invalid workflowId' }; + } + + const input = b.inputData; + if (!input || typeof input !== 'object') { + return { valid: false, error: 'Missing inputData' }; + } + + if (!input.resourceType || !ALLOWED_RESOURCE_TYPES.has(input.resourceType)) { + return { + valid: false, + error: `Invalid resourceType: ${ + input.resourceType ?? 'undefined' + }. Allowed: ${[...ALLOWED_RESOURCE_TYPES].join(', ')}`, + }; + } + + for (const field of [ + 'clusterName', + 'resourceNamespace', + 'resourceName', + 'containerName', + ] as const) { + if (!input[field] || typeof input[field] !== 'string') { + return { valid: false, error: `Missing or invalid ${field}` }; + } + } + + if ( + !input.containerResources || + typeof input.containerResources !== 'object' + ) { + return { valid: false, error: 'Missing containerResources' }; + } + + return { valid: true, data: b }; +} + +/** + * Backend endpoint that validates inputs, checks ros.apply permission, + * and forwards the workflow execution to the Orchestrator plugin. + */ +export const applyRecommendation: (options: RouterOptions) => RequestHandler = + options => async (req, res) => { + const { logger, httpAuth, permissions, discovery, auth } = options; + + const validation = validateBody(req.body); + if (!validation.valid) { + return res.status(400).json({ error: validation.error }); + } + const { workflowId, inputData } = validation.data; + + const decision = await authorize( + req, + rosApplyPermissions, + permissions, + httpAuth, + ); + if (decision.result !== AuthorizeResult.ALLOW) { + logger.info('audit:apply-recommendation:denied', { + action: 'apply_recommendation', + decision: 'DENY', + workflowId, + cluster: inputData.clusterName, + namespace: inputData.resourceNamespace, + workload: inputData.resourceName, + resourceType: inputData.resourceType, + }); + return res + .status(403) + .json({ error: 'Access denied: ros.apply permission required' }); + } + + try { + const orchestratorBase = await discovery.getBaseUrl('orchestrator'); + const { token } = await auth.getPluginRequestToken({ + onBehalfOf: await httpAuth.credentials(req), + targetPluginId: 'orchestrator', + }); + + const executeUrl = `${orchestratorBase}/v2/workflows/${encodeURIComponent( + workflowId, + )}/execute`; + + const upstreamResponse = await fetch(executeUrl, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${token}`, + }, + body: JSON.stringify({ inputData }), + }); + + const payload = await upstreamResponse.json(); + + if (!upstreamResponse.ok) { + logger.warn('audit:apply-recommendation:upstream-error', { + action: 'apply_recommendation', + decision: 'ALLOW', + workflowId, + cluster: inputData.clusterName, + namespace: inputData.resourceNamespace, + workload: inputData.resourceName, + resourceType: inputData.resourceType, + upstreamStatus: upstreamResponse.status, + }); + return res.status(upstreamResponse.status).json(payload); + } + + logger.info('audit:apply-recommendation:success', { + action: 'apply_recommendation', + decision: 'ALLOW', + workflowId, + instanceId: (payload as { id?: string }).id, + cluster: inputData.clusterName, + namespace: inputData.resourceNamespace, + workload: inputData.resourceName, + resourceType: inputData.resourceType, + }); + + return res.status(200).json(payload); + } catch (error) { + logger.error('Apply recommendation proxy error', error); + return res + .status(500) + .json({ error: 'Internal error executing workflow' }); + } + }; diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts index 41d3708e6d8..4159fe3a957 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.test.ts @@ -29,6 +29,8 @@ describe('createRouter', () => { httpAuth: mockServices.httpAuth(), permissions: mockServices.permissions.mock(), cache: mockServices.cache.mock(), + discovery: mockServices.discovery(), + auth: mockServices.auth(), optimizationApi: { getRecommendationList: jest.fn(), getRecommendationById: jest.fn(), diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts index cfbcd01e9af..c8e6ec2d8e8 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts @@ -18,10 +18,15 @@ import express from 'express'; import Router from 'express-promise-router'; import type { RouterOptions } from '../models/RouterOptions'; import { createPermissionIntegrationRouter } from '@backstage/plugin-permission-node'; -import { rosPluginPermissions } from '@red-hat-developer-hub/plugin-cost-management-common/permissions'; +import { + rosPluginPermissions, + rosApplyPermissions, + costPluginPermissions, +} from '@red-hat-developer-hub/plugin-cost-management-common/permissions'; import { getAccess } from '../routes/access'; import { getCostManagementAccess } from '../routes/costManagementAccess'; import { secureProxy } from '../routes/secureProxy'; +import { applyRecommendation } from '../routes/applyRecommendation'; /** @public */ export async function createRouter( @@ -29,7 +34,11 @@ export async function createRouter( ): Promise { const router = Router(); const permissionsIntegrationRouter = createPermissionIntegrationRouter({ - permissions: rosPluginPermissions, + permissions: [ + ...rosPluginPermissions, + ...rosApplyPermissions, + ...costPluginPermissions, + ], }); router.use(express.json()); @@ -43,6 +52,8 @@ export async function createRouter( router.get('/access/cost-management', getCostManagementAccess(options)); + router.post('/apply-recommendation', applyRecommendation(options)); + router.all('/proxy/*', secureProxy(options)); return router; diff --git a/workspaces/cost-management/plugins/cost-management-common/report-clients.api.md b/workspaces/cost-management/plugins/cost-management-common/report-clients.api.md index ea797a8be38..4fa8f31e33b 100644 --- a/workspaces/cost-management/plugins/cost-management-common/report-clients.api.md +++ b/workspaces/cost-management/plugins/cost-management-common/report-clients.api.md @@ -427,10 +427,11 @@ export interface OrchestratorSlimApi { // @public (undocumented) export class OrchestratorSlimClient implements OrchestratorSlimApi { + /** @deprecated identityApi is retained for backward compatibility but no longer used. */ constructor(options: { discoveryApi: DiscoveryApi; fetchApi: FetchApi; - identityApi: IdentityApi; + identityApi?: IdentityApi; }); // (undocumented) checkWorkflowAvailability( diff --git a/workspaces/cost-management/plugins/cost-management-common/report-permissions.api.md b/workspaces/cost-management/plugins/cost-management-common/report-permissions.api.md index 708f472419e..c7af585f915 100644 --- a/workspaces/cost-management/plugins/cost-management-common/report-permissions.api.md +++ b/workspaces/cost-management/plugins/cost-management-common/report-permissions.api.md @@ -22,6 +22,12 @@ export const costPluginPermissions: BasicPermission[]; // @public (undocumented) export const costPluginReadPermission: BasicPermission; +// @public (undocumented) +export const rosApplyPermission: BasicPermission; + +// @public (undocumented) +export const rosApplyPermissions: BasicPermission[]; + // @public (undocumented) export const rosClusterProjectPermission: ( clusterName: string, diff --git a/workspaces/cost-management/plugins/cost-management-common/src/clients/orchestrator-slim/OrchestratorSlimClient.ts b/workspaces/cost-management/plugins/cost-management-common/src/clients/orchestrator-slim/OrchestratorSlimClient.ts index 7431fa82f71..f21bc83ac5b 100644 --- a/workspaces/cost-management/plugins/cost-management-common/src/clients/orchestrator-slim/OrchestratorSlimClient.ts +++ b/workspaces/cost-management/plugins/cost-management-common/src/clients/orchestrator-slim/OrchestratorSlimClient.ts @@ -29,17 +29,16 @@ import type { JsonObject } from '@backstage/types'; export class OrchestratorSlimClient implements OrchestratorSlimApi { private readonly discoveryApi: DiscoveryApi; private readonly fetchApi: FetchApi; - private readonly identityApi: IdentityApi; private baseUrl?: string; + /** @deprecated identityApi is retained for backward compatibility but no longer used. */ constructor(options: { discoveryApi: DiscoveryApi; fetchApi: FetchApi; - identityApi: IdentityApi; + identityApi?: IdentityApi; }) { this.discoveryApi = options.discoveryApi; this.fetchApi = options.fetchApi; - this.identityApi = options.identityApi; } async isWorkflowAvailable(workflowId: string): Promise { @@ -122,29 +121,38 @@ export class OrchestratorSlimClient implements OrchestratorSlimApi { } } - /** @public */ + /** + * Executes a workflow through the cost-management backend which validates + * inputs and checks ros.apply permission before forwarding to orchestrator. + * @public + */ async executeWorkflow( workflowId: string, workflowInputData: D, ): Promise<{ id: string }> { - if (!this.baseUrl) { - this.baseUrl = await this.discoveryApi.getBaseUrl('orchestrator'); - } - - const { token } = await this.identityApi.getCredentials(); - const url = `${this.baseUrl}/v2/workflows/${encodeURIComponent( - workflowId, - )}/execute`; + const costManagementBase = await this.discoveryApi.getBaseUrl( + 'cost-management', + ); + const url = `${costManagementBase}/apply-recommendation`; const response = await this.fetchApi.fetch(url, { method: 'POST', - body: JSON.stringify(workflowInputData), + body: JSON.stringify({ + workflowId, + ...(workflowInputData as object), + }), headers: { 'Content-Type': 'application/json', - Authorization: `Bearer ${token}`, }, }); - const payload = (await response.json()) as { id: string }; - return payload; + if (!response.ok) { + const errorPayload = await response.json().catch(() => ({})); + const message = + (errorPayload as { error?: string }).error ?? + `Workflow execution failed (${response.status})`; + throw new Error(message); + } + + return (await response.json()) as { id: string }; } } diff --git a/workspaces/cost-management/plugins/cost-management-common/src/permissions.ts b/workspaces/cost-management/plugins/cost-management-common/src/permissions.ts index aef26815f70..86900e4e6a2 100644 --- a/workspaces/cost-management/plugins/cost-management-common/src/permissions.ts +++ b/workspaces/cost-management/plugins/cost-management-common/src/permissions.ts @@ -22,6 +22,12 @@ export const rosPluginReadPermission = createPermission({ attributes: { action: 'read' }, }); +/** @public */ +export const rosApplyPermission = createPermission({ + name: 'ros.apply', + attributes: { action: 'update' }, +}); + /** @public */ export const rosClusterSpecificPermission = (clusterName: string) => createPermission({ @@ -67,3 +73,6 @@ export const costPluginPermissions = [costPluginReadPermission]; /** @public */ export const rosPluginPermissions = [rosPluginReadPermission]; + +/** @public */ +export const rosApplyPermissions = [rosApplyPermission]; diff --git a/workspaces/cost-management/plugins/cost-management/src/pages/optimizations-breakdown/components/optimization-engine-tab/OptimizationEngineTab.tsx b/workspaces/cost-management/plugins/cost-management/src/pages/optimizations-breakdown/components/optimization-engine-tab/OptimizationEngineTab.tsx index 4b3aeb1deea..9eebc26aa1a 100644 --- a/workspaces/cost-management/plugins/cost-management/src/pages/optimizations-breakdown/components/optimization-engine-tab/OptimizationEngineTab.tsx +++ b/workspaces/cost-management/plugins/cost-management/src/pages/optimizations-breakdown/components/optimization-engine-tab/OptimizationEngineTab.tsx @@ -14,8 +14,18 @@ * limitations under the License. */ -import React, { useMemo } from 'react'; -import { Box, Button, Grid, Tooltip } from '@material-ui/core'; +import React, { useCallback, useMemo, useState } from 'react'; +import { + Box, + Button, + Dialog, + DialogActions, + DialogContent, + DialogContentText, + DialogTitle, + Grid, + Tooltip, +} from '@material-ui/core'; import type { WorkflowUnavailableReason } from '@red-hat-developer-hub/plugin-cost-management-common/clients'; import { RecommendationType } from '../../models/ChartEnums'; import { ChartInfoCard } from './components/chart-info-card/ChartInfoCard'; @@ -44,49 +54,66 @@ interface OptimizationEngineTabProps extends ContainerInfoCardProps { workflowErrorMessage?: string; } -export const OptimizationEngineTab = (props: OptimizationEngineTabProps) => { - const isWorkflowAvailable = !!props.workflowId; +export const OptimizationEngineTab = ({ + workflowId, + workflowErrorMessage, + workflowUnavailableReason, + onApplyRecommendation, + ...restProps +}: OptimizationEngineTabProps) => { + const isWorkflowAvailable = !!workflowId; + const [confirmOpen, setConfirmOpen] = useState(false); const tooltipMessage = useMemo(() => { if (isWorkflowAvailable) { return ''; } - // Prefer the actual error message from the API - if (props.workflowErrorMessage) { - return props.workflowErrorMessage; + if (workflowErrorMessage) { + return workflowErrorMessage; } - // Fall back to default messages based on reason - if (props.workflowUnavailableReason) { - return DEFAULT_WORKFLOW_MESSAGES[props.workflowUnavailableReason]; + if (workflowUnavailableReason) { + return DEFAULT_WORKFLOW_MESSAGES[workflowUnavailableReason]; } return DEFAULT_WORKFLOW_MESSAGES.not_configured; - }, [ - isWorkflowAvailable, - props.workflowErrorMessage, - props.workflowUnavailableReason, - ]); + }, [isWorkflowAvailable, workflowErrorMessage, workflowUnavailableReason]); + + const handleApplyClick = useCallback(() => { + setConfirmOpen(true); + }, []); + + const handleConfirmCancel = useCallback(() => { + setConfirmOpen(false); + }, []); + + const handleConfirmApply = useCallback( + (event: React.MouseEvent) => { + setConfirmOpen(false); + onApplyRecommendation?.(event); + }, + [onApplyRecommendation], + ); return ( { + + + ); }; From a008bd8b179bca5f327e0af0255ca9d6ce6934c4 Mon Sep 17 00:00:00 2001 From: gharden Date: Thu, 26 Mar 2026 14:27:25 -0400 Subject: [PATCH 2/4] fix(cost-management): regenerate API reports to match CI expectations Run yarn build:api-reports:only to update report-clients.api.md and report.api.md with the correct auto-generated format. Made-with: Cursor --- .../plugins/cost-management-common/report-clients.api.md | 3 +-- .../plugins/cost-management-common/report.api.md | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/workspaces/cost-management/plugins/cost-management-common/report-clients.api.md b/workspaces/cost-management/plugins/cost-management-common/report-clients.api.md index 4fa8f31e33b..10e04f26b70 100644 --- a/workspaces/cost-management/plugins/cost-management-common/report-clients.api.md +++ b/workspaces/cost-management/plugins/cost-management-common/report-clients.api.md @@ -427,7 +427,7 @@ export interface OrchestratorSlimApi { // @public (undocumented) export class OrchestratorSlimClient implements OrchestratorSlimApi { - /** @deprecated identityApi is retained for backward compatibility but no longer used. */ + // @deprecated constructor(options: { discoveryApi: DiscoveryApi; fetchApi: FetchApi; @@ -437,7 +437,6 @@ export class OrchestratorSlimClient implements OrchestratorSlimApi { checkWorkflowAvailability( workflowId: string, ): Promise; - // (undocumented) executeWorkflow( workflowId: string, workflowInputData: D, diff --git a/workspaces/cost-management/plugins/cost-management-common/report.api.md b/workspaces/cost-management/plugins/cost-management-common/report.api.md index 1e70114aed0..5685496ff87 100644 --- a/workspaces/cost-management/plugins/cost-management-common/report.api.md +++ b/workspaces/cost-management/plugins/cost-management-common/report.api.md @@ -750,16 +750,16 @@ export interface OrchestratorSlimApi { // @public (undocumented) export class OrchestratorSlimClient implements OrchestratorSlimApi { + // @deprecated constructor(options: { discoveryApi: DiscoveryApi; fetchApi: FetchApi; - identityApi: IdentityApi; + identityApi?: IdentityApi; }); // (undocumented) checkWorkflowAvailability( workflowId: string, ): Promise; - // (undocumented) executeWorkflow( workflowId: string, workflowInputData: D, From b8840838b6d9299e63152b74d7de37e9428de200 Mon Sep 17 00:00:00 2001 From: gharden Date: Thu, 26 Mar 2026 14:56:15 -0400 Subject: [PATCH 3/4] fix(cost-management): address Qodo review findings - Wrap decodeURIComponent in try/catch, return 400 on malformed encoding - Safe JSON parse in applyRecommendation (check content-type first) - Change router.all('/proxy/*') to router.get (proxy is read-only) - Delete unused routes/token.ts Made-with: Cursor --- .../src/routes/applyRecommendation.test.ts | 2 + .../src/routes/applyRecommendation.ts | 10 ++- .../src/routes/token.ts | 68 ------------------- .../src/service/router.ts | 2 +- 4 files changed, 12 insertions(+), 70 deletions(-) delete mode 100644 workspaces/cost-management/plugins/cost-management-backend/src/routes/token.ts diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts index 2bb93617e5c..49f659ea8e0 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.test.ts @@ -169,6 +169,7 @@ describe('applyRecommendation', () => { const fetchSpy = jest.spyOn(globalThis, 'fetch').mockResolvedValueOnce( new Response(JSON.stringify({ id: 'workflow-instance-123' }), { status: 200, + headers: { 'Content-Type': 'application/json' }, }), ); @@ -194,6 +195,7 @@ describe('applyRecommendation', () => { const fetchSpy = jest.spyOn(globalThis, 'fetch').mockResolvedValueOnce( new Response(JSON.stringify({ error: 'Workflow not found' }), { status: 404, + headers: { 'Content-Type': 'application/json' }, }), ); diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts index e4b66ec553c..ef9f2ef2755 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts @@ -145,7 +145,15 @@ export const applyRecommendation: (options: RouterOptions) => RequestHandler = body: JSON.stringify({ inputData }), }); - const payload = await upstreamResponse.json(); + const contentType = upstreamResponse.headers.get('content-type') || ''; + let payload: unknown; + try { + payload = contentType.includes('application/json') + ? await upstreamResponse.json() + : { message: await upstreamResponse.text() }; + } catch { + payload = { error: 'Upstream returned unparseable response' }; + } if (!upstreamResponse.ok) { logger.warn('audit:apply-recommendation:upstream-error', { diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/routes/token.ts b/workspaces/cost-management/plugins/cost-management-backend/src/routes/token.ts deleted file mode 100644 index 6a0a4125cbb..00000000000 --- a/workspaces/cost-management/plugins/cost-management-backend/src/routes/token.ts +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright Red Hat, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import assert from 'assert'; -import type { RequestHandler } from 'express'; -import type { GetTokenResponse } from '../models/GetTokenResponse'; -import { RouterOptions } from '../models/RouterOptions'; - -const DEFAULT_SSO_BASE_URL = 'https://sso.redhat.com'; - -export const getToken: (options: RouterOptions) => RequestHandler = - options => async (_, response) => { - const { logger, config } = options; - - assert(typeof config !== 'undefined', 'Config is undefined'); - - logger.info('Requesting new access token'); - - const ssoBaseUrl = - config.getOptionalString('costManagement.ssoBaseUrl') ?? - DEFAULT_SSO_BASE_URL; - const params = { - tokenUrl: `${ssoBaseUrl}/auth/realms/redhat-external/protocol/openid-connect/token`, - clientId: config.getString('costManagement.clientId'), - clientSecret: config.getString('costManagement.clientSecret'), - scope: 'api.console', - grantType: 'client_credentials', - } as const; - - const rhSsoResponse = await fetch(params.tokenUrl, { - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, - body: new URLSearchParams( - Object.entries({ - client_id: params.clientId, - client_secret: params.clientSecret, - scope: params.scope, - grant_type: params.grantType, - }).map(([k, v]) => [encodeURIComponent(k), encodeURIComponent(v)]), - ), - method: 'POST', - }); - - if (rhSsoResponse.ok) { - const { access_token, expires_in } = await rhSsoResponse.json(); - const body: GetTokenResponse = { - accessToken: access_token, - expiresAt: Date.now() + expires_in * 1000, - }; - response.json(body); - } else { - throw new Error(rhSsoResponse.statusText); - } - }; diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts index c8e6ec2d8e8..e30e631d513 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/service/router.ts @@ -54,7 +54,7 @@ export async function createRouter( router.post('/apply-recommendation', applyRecommendation(options)); - router.all('/proxy/*', secureProxy(options)); + router.get('/proxy/*', secureProxy(options)); return router; } From f9a5eb70f718380b23aa9374817d6dc449c10e96 Mon Sep 17 00:00:00 2001 From: gharden Date: Thu, 26 Mar 2026 20:09:47 -0400 Subject: [PATCH 4/4] fix(cost-management): sanitize inputData before Orchestrator forwarding Construct a clean inputData object with only known fields before forwarding to the Orchestrator, preventing extra injected fields from passing through to the workflow execution. Co-Authored-By: Claude Opus 4.6 --- .../src/routes/applyRecommendation.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts index ef9f2ef2755..150e3d5c6d0 100644 --- a/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts +++ b/workspaces/cost-management/plugins/cost-management-backend/src/routes/applyRecommendation.ts @@ -142,7 +142,16 @@ export const applyRecommendation: (options: RouterOptions) => RequestHandler = 'Content-Type': 'application/json', Authorization: `Bearer ${token}`, }, - body: JSON.stringify({ inputData }), + body: JSON.stringify({ + inputData: { + clusterName: inputData.clusterName, + resourceType: inputData.resourceType, + resourceNamespace: inputData.resourceNamespace, + resourceName: inputData.resourceName, + containerName: inputData.containerName, + containerResources: inputData.containerResources, + }, + }), }); const contentType = upstreamResponse.headers.get('content-type') || '';