From c16a72a0b02b6b99f2f0b7250b464d68233e496a Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Mon, 4 May 2026 22:43:47 -0700 Subject: [PATCH] fix: validate build uuid overrides --- src/app/api/v2/builds/[uuid]/route.test.ts | 217 ++++++++++++++++++ src/app/api/v2/builds/[uuid]/route.ts | 105 ++++++++- src/pages/api/v1/builds/[uuid]/index.ts | 9 +- .../services/__tests__/activityStream.test.ts | 189 +++++++++++++++ src/server/services/activityStream.ts | 15 +- src/server/services/override.ts | 44 +++- src/shared/openApiSpec.ts | 29 +++ 7 files changed, 593 insertions(+), 15 deletions(-) create mode 100644 src/app/api/v2/builds/[uuid]/route.test.ts create mode 100644 src/server/services/__tests__/activityStream.test.ts diff --git a/src/app/api/v2/builds/[uuid]/route.test.ts b/src/app/api/v2/builds/[uuid]/route.test.ts new file mode 100644 index 00000000..ec6a0c25 --- /dev/null +++ b/src/app/api/v2/builds/[uuid]/route.test.ts @@ -0,0 +1,217 @@ +/** + * Copyright 2026 Lifecycle contributors + * + * 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 { NextRequest } from 'next/server'; + +const mockGetBuildByUUID = jest.fn(); +const mockQueueAdd = jest.fn(); +const mockFindOne = jest.fn(); +const mockWithGraphFetched = jest.fn(); +const mockValidateUuid = jest.fn(); +const mockUpdateBuildUuid = jest.fn(); + +jest.mock('nanoid', () => ({ + nanoid: jest.fn(() => 'run-uuid'), +})); + +jest.mock('server/lib/logger', () => ({ + getLogger: jest.fn(() => ({ + error: jest.fn(), + info: jest.fn(), + })), + LogStage: { + BUILD_QUEUED: 'build_queued', + }, +})); + +jest.mock('server/services/build', () => ({ + __esModule: true, + default: jest.fn().mockImplementation(() => ({ + getBuildByUUID: (...args: unknown[]) => mockGetBuildByUUID(...args), + resolveAndDeployBuildQueue: { + add: (...args: unknown[]) => mockQueueAdd(...args), + }, + })), +})); + +jest.mock('server/services/override', () => { + class BuildUuidValidationError extends Error { + constructor(message: string) { + super(message); + this.name = 'BuildUuidValidationError'; + } + } + + return { + __esModule: true, + BuildUuidValidationError, + default: jest.fn().mockImplementation(() => ({ + db: { + models: { + Build: { + query: jest.fn(() => ({ + findOne: (...args: unknown[]) => mockFindOne(...args), + })), + }, + }, + }, + validateUuid: (...args: unknown[]) => mockValidateUuid(...args), + updateBuildUuid: (...args: unknown[]) => mockUpdateBuildUuid(...args), + })), + }; +}); + +import { GET, PATCH } from './route'; + +function makeRequest(body?: Record): NextRequest { + return { + json: jest.fn().mockResolvedValue(body || {}), + headers: new Headers([['x-request-id', 'req-test']]), + nextUrl: new URL('http://localhost/api/v2/builds/current-build'), + } as unknown as NextRequest; +} + +describe('/api/v2/builds/[uuid]', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockFindOne.mockReturnValue({ + withGraphFetched: mockWithGraphFetched, + }); + mockValidateUuid.mockResolvedValue({ valid: true }); + mockUpdateBuildUuid.mockResolvedValue({ + build: { + id: 42, + uuid: 'new-build', + }, + deploysUpdated: 3, + }); + }); + + it('GET returns a build by UUID', async () => { + mockGetBuildByUUID.mockResolvedValueOnce({ + id: 42, + uuid: 'current-build', + }); + + const response = await GET(makeRequest(), { + params: { + uuid: 'current-build', + }, + }); + const body = await response.json(); + + expect(response.status).toBe(200); + expect(mockGetBuildByUUID).toHaveBeenCalledWith('current-build'); + expect(body.data).toEqual({ + id: 42, + uuid: 'current-build', + }); + }); + + it('PATCH validates and updates the build UUID', async () => { + const build = { + id: 42, + uuid: 'current-build', + pullRequest: { + deployOnUpdate: true, + }, + }; + mockWithGraphFetched.mockResolvedValueOnce(build); + + const response = await PATCH(makeRequest({ uuid: 'new-build' }), { + params: { + uuid: 'current-build', + }, + }); + const body = await response.json(); + + expect(response.status).toBe(200); + expect(mockFindOne).toHaveBeenCalledWith({ uuid: 'current-build' }); + expect(mockWithGraphFetched).toHaveBeenCalledWith('pullRequest'); + expect(mockValidateUuid).toHaveBeenCalledWith('new-build', 42); + expect(mockUpdateBuildUuid).toHaveBeenCalledWith(build, 'new-build'); + expect(mockQueueAdd).toHaveBeenCalledWith('resolve-deploy', { + buildId: 42, + runUUID: 'run-uuid', + correlationId: 'req-test', + }); + expect(body.data).toEqual({ + id: 42, + uuid: 'new-build', + }); + }); + + it('PATCH rejects unavailable UUIDs before updating', async () => { + mockWithGraphFetched.mockResolvedValueOnce({ + id: 42, + uuid: 'current-build', + }); + mockValidateUuid.mockResolvedValueOnce({ + valid: false, + error: 'UUID is not available', + }); + + const response = await PATCH(makeRequest({ uuid: 'existing-build' }), { + params: { + uuid: 'current-build', + }, + }); + const body = await response.json(); + + expect(response.status).toBe(400); + expect(body.error.message).toBe('UUID is not available'); + expect(mockUpdateBuildUuid).not.toHaveBeenCalled(); + expect(mockQueueAdd).not.toHaveBeenCalled(); + }); + + it('PATCH returns 404 when the build does not exist', async () => { + mockWithGraphFetched.mockResolvedValueOnce(null); + + const response = await PATCH(makeRequest({ uuid: 'new-build' }), { + params: { + uuid: 'missing-build', + }, + }); + + expect(response.status).toBe(404); + expect(mockValidateUuid).not.toHaveBeenCalled(); + expect(mockUpdateBuildUuid).not.toHaveBeenCalled(); + }); + + it('PATCH rejects missing UUID bodies and no-op UUID changes', async () => { + const missingUuidResponse = await PATCH(makeRequest({}), { + params: { + uuid: 'current-build', + }, + }); + expect(missingUuidResponse.status).toBe(400); + + mockWithGraphFetched.mockResolvedValueOnce({ + id: 42, + uuid: 'current-build', + }); + + const sameUuidResponse = await PATCH(makeRequest({ uuid: 'current-build' }), { + params: { + uuid: 'current-build', + }, + }); + + expect(sameUuidResponse.status).toBe(400); + expect(mockValidateUuid).not.toHaveBeenCalled(); + expect(mockUpdateBuildUuid).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/v2/builds/[uuid]/route.ts b/src/app/api/v2/builds/[uuid]/route.ts index 87f0a8d6..6c34387d 100644 --- a/src/app/api/v2/builds/[uuid]/route.ts +++ b/src/app/api/v2/builds/[uuid]/route.ts @@ -1,7 +1,15 @@ +import { nanoid } from 'nanoid'; import { NextRequest } from 'next/server'; import { createApiHandler } from 'server/lib/createApiHandler'; +import { getLogger, LogStage } from 'server/lib/logger'; import { errorResponse, successResponse } from 'server/lib/response'; import BuildService from 'server/services/build'; +import OverrideService, { BuildUuidValidationError } from 'server/services/override'; + +interface UpdateBuildUuidRequest { + uuid?: unknown; +} + /** * @openapi * /api/v2/builds/{uuid}: @@ -44,7 +52,7 @@ const getHandler = async (req: NextRequest, { params }: { params: { uuid: string const build = await buildService.getBuildByUUID(params.uuid); if (!build) { - return errorResponse(`Build with UUID ${params.uuid} not found`, { status: 404 }, req); + return errorResponse(new Error(`Build with UUID ${params.uuid} not found`), { status: 404 }, req); } return successResponse( @@ -56,4 +64,99 @@ const getHandler = async (req: NextRequest, { params }: { params: { uuid: string ); }; +/** + * @openapi + * /api/v2/builds/{uuid}: + * patch: + * summary: Update a build UUID + * description: Updates a build UUID and the related deployable and deploy UUID fields. + * tags: + * - Builds + * operationId: updateBuildUUID + * parameters: + * - in: path + * name: uuid + * required: true + * schema: + * type: string + * description: The current UUID of the build to update. + * requestBody: + * required: true + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/UpdateBuildUUIDRequest' + * responses: + * '200': + * description: Updated build object. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/UpdateBuildUUIDSuccessResponse' + * '400': + * description: Invalid or unavailable UUID. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/ApiErrorResponse' + * '404': + * description: Build not found. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/ApiErrorResponse' + * '500': + * description: Server error. + * content: + * application/json: + * schema: + * $ref: '#/components/schemas/ApiErrorResponse' + */ +const patchHandler = async (req: NextRequest, { params }: { params: { uuid: string } }) => { + const body = (await req.json().catch(() => null)) as UpdateBuildUuidRequest | null; + const newUuid = body?.uuid; + + if (!newUuid || typeof newUuid !== 'string') { + return errorResponse(new Error('uuid is required'), { status: 400 }, req); + } + + const override = new OverrideService(); + const build = await override.db.models.Build.query().findOne({ uuid: params.uuid }).withGraphFetched('pullRequest'); + + if (!build) { + return errorResponse(new Error(`Build with UUID ${params.uuid} not found`), { status: 404 }, req); + } + + if (newUuid === build.uuid) { + return errorResponse(new Error('UUID must be different'), { status: 400 }, req); + } + + const validation = await override.validateUuid(newUuid, build.id); + if (!validation.valid) { + return errorResponse(new Error(validation.error || 'Invalid UUID'), { status: 400 }, req); + } + + try { + const result = await override.updateBuildUuid(build, newUuid); + + if (build.pullRequest?.deployOnUpdate) { + getLogger({ stage: LogStage.BUILD_QUEUED }).info('Triggering redeploy after UUID update'); + await new BuildService().resolveAndDeployBuildQueue.add('resolve-deploy', { + buildId: build.id, + runUUID: nanoid(), + correlationId: req.headers.get('x-request-id') || `api-build-update-${Date.now()}`, + }); + } + + return successResponse(result.build, { status: 200 }, req); + } catch (error) { + if (error instanceof BuildUuidValidationError) { + return errorResponse(error, { status: 400 }, req); + } + + throw error; + } +}; + export const GET = createApiHandler(getHandler); +export const PATCH = createApiHandler(patchHandler); diff --git a/src/pages/api/v1/builds/[uuid]/index.ts b/src/pages/api/v1/builds/[uuid]/index.ts index eebd38aa..04169623 100644 --- a/src/pages/api/v1/builds/[uuid]/index.ts +++ b/src/pages/api/v1/builds/[uuid]/index.ts @@ -18,7 +18,7 @@ import { nanoid } from 'nanoid'; import { NextApiRequest, NextApiResponse } from 'next/types'; import { withLogContext, getLogger, LogStage } from 'server/lib/logger'; import BuildService from 'server/services/build'; -import OverrideService from 'server/services/override'; +import OverrideService, { BuildUuidValidationError } from 'server/services/override'; async function retrieveBuild(req: NextApiRequest, res: NextApiResponse) { try { @@ -80,7 +80,7 @@ async function updateBuild(req: NextApiRequest, res: NextApiResponse, correlatio return res.status(400).json({ error: 'UUID must be different' }); } - const validation = await override.validateUuid(newUuid); + const validation = await override.validateUuid(newUuid, build.id); if (!validation.valid) { getLogger().debug(`UUID validation failed: error=${validation.error}`); return res.status(400).json({ error: validation.error }); @@ -103,6 +103,11 @@ async function updateBuild(req: NextApiRequest, res: NextApiResponse, correlatio }, }); } catch (error) { + if (error instanceof BuildUuidValidationError) { + getLogger().debug(`UUID validation failed: error=${error.message}`); + return res.status(400).json({ error: error.message }); + } + getLogger({ error }).error(`API: UUID update failed newUuid=${newUuid}`); return res.status(500).json({ error: 'An unexpected error occurred' }); } diff --git a/src/server/services/__tests__/activityStream.test.ts b/src/server/services/__tests__/activityStream.test.ts new file mode 100644 index 00000000..23eec66e --- /dev/null +++ b/src/server/services/__tests__/activityStream.test.ts @@ -0,0 +1,189 @@ +/** + * Copyright 2026 Lifecycle contributors + * + * 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. + */ + +const mockLogger = { + debug: jest.fn(), + error: jest.fn(), + info: jest.fn(), + warn: jest.fn(), +}; +const mockBuildFindOne = jest.fn(); +const mockBuildQuery = jest.fn(); +const mockBuildPatch = jest.fn(); +const mockEnqueueResolveAndDeployBuild = jest.fn(); +const mockRegisterQueue = jest.fn(); + +jest.mock('server/lib/dependencies', () => ({ + defaultDb: {}, + defaultRedis: {}, + defaultRedlock: {}, + defaultQueueManager: {}, + redisClient: { + getConnection: jest.fn(), + }, +})); + +jest.mock('server/lib/logger', () => ({ + getLogger: jest.fn(() => mockLogger), + withLogContext: jest.fn((_context, fn) => fn()), + extractContextForQueue: jest.fn(() => ({})), + LogStage: {}, +})); + +jest.mock('shared/config', () => ({ + LIFECYCLE_UI_URL: 'https://lifecycle.example.com', + QUEUE_NAMES: { + COMMENT_QUEUE: 'comment', + }, +})); + +jest.mock('server/lib/fastly', () => + jest.fn().mockImplementation(() => ({ + getServiceDashboardUrl: jest.fn(), + purgeService: jest.fn(), + })) +); + +jest.mock('server/lib/metrics', () => ({ + Metrics: jest.fn().mockImplementation(() => ({ + increment: jest.fn().mockReturnThis(), + event: jest.fn().mockReturnThis(), + })), +})); + +jest.mock('server/lib/nativeHelm', () => ({ + ChartType: {}, + determineChartType: jest.fn(), +})); + +jest.mock('server/lib/utils', () => ({ + enableKillSwitch: jest.fn(), + flattenObject: jest.fn((value) => value), + getDeployLabel: jest.fn().mockResolvedValue('lifecycle-deploy!'), + getDisabledLabel: jest.fn().mockResolvedValue('lifecycle-disabled!'), + getStatusCommentLabel: jest.fn().mockResolvedValue('lifecycle-status-comments!'), + hasDeployLabel: jest.fn().mockResolvedValue(false), + hasStatusCommentLabel: jest.fn().mockResolvedValue(false), + isControlCommentsEnabled: jest.fn().mockResolvedValue(true), + isDefaultStatusCommentsEnabled: jest.fn().mockResolvedValue(false), + isStaging: jest.fn(() => false), +})); + +jest.mock('server/lib/github', () => ({ + checkIfCommentExists: jest.fn(), + createOrUpdatePullRequestComment: jest.fn(), +})); + +jest.mock('server/lib/kubernetes', () => ({ + deleteNamespace: jest.fn().mockResolvedValue(undefined), +})); + +jest.mock('server/services/deploy', () => ({ + __esModule: true, + default: jest.fn().mockImplementation(() => ({ + hostForDeployableDeploy: jest.fn(), + hostForServiceDeploy: jest.fn(), + })), +})); + +jest.mock('server/services/buildMetadata', () => ({ + __esModule: true, + default: jest.fn().mockImplementation(() => ({})), +})); + +import ActivityStream from '../activityStream'; +import { CommentParser } from 'shared/constants'; + +function createActivityStream() { + mockRegisterQueue.mockReturnValue({ + add: jest.fn(), + }); + + const db = { + models: { + Build: { + query: mockBuildQuery, + }, + }, + services: { + BuildService: { + enqueueResolveAndDeployBuild: mockEnqueueResolveAndDeployBuild, + }, + Deploy: { + hostForDeployableDeploy: jest.fn(), + hostForServiceDeploy: jest.fn(), + }, + }, + }; + + mockBuildQuery.mockReturnValue({ + findOne: mockBuildFindOne, + }); + + return new ActivityStream( + db as any, + {} as any, + {} as any, + { + registerQueue: mockRegisterQueue, + } as any + ); +} + +describe('ActivityStream comment overrides', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockBuildFindOne.mockResolvedValue({ + id: 999, + uuid: 'existing-build', + }); + mockBuildPatch.mockResolvedValue(undefined); + }); + + it('rejects an unavailable comment UUID before applying other overrides', async () => { + const service = createActivityStream(); + const build = { + id: 42, + uuid: 'current-build', + $query: jest.fn(() => ({ + patch: mockBuildPatch, + })), + }; + const commentBody = [ + CommentParser.HEADER, + 'url: existing-build', + 'ENV:FEATURE_ENABLED:true', + CommentParser.FOOTER, + ].join('\n'); + + await (service as any).applyCommentOverrides({ + build, + deploys: [], + pullRequest: { + deployOnUpdate: true, + }, + commentBody, + runUuid: 'run-uuid', + }); + + expect(mockBuildFindOne).toHaveBeenCalledWith({ uuid: 'existing-build' }); + expect(mockBuildPatch).not.toHaveBeenCalled(); + expect(mockEnqueueResolveAndDeployBuild).not.toHaveBeenCalled(); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'UUID: comment override rejected newUuid=existing-build error=UUID is not available' + ); + }); +}); diff --git a/src/server/services/activityStream.ts b/src/server/services/activityStream.ts index 794e25b6..42a991f5 100644 --- a/src/server/services/activityStream.ts +++ b/src/server/services/activityStream.ts @@ -203,6 +203,16 @@ export default class ActivityStream extends BaseService { const vanityUrl = CommentHelper.parseVanityUrl(commentBody); const envOverrides = CommentHelper.parseEnvironmentOverrides(commentBody); const redeployOnPush = CommentHelper.parseRedeployOnPushes(commentBody); + const requestedUuid = vanityUrl && vanityUrl !== build.uuid ? vanityUrl : null; + const override = new OverrideService(this.db, this.redis, this.redlock, this.queueManager); + + if (requestedUuid) { + const validation = await override.validateUuid(requestedUuid, build.id); + if (!validation.valid) { + getLogger().warn(`UUID: comment override rejected newUuid=${requestedUuid} error=${validation.error}`); + return; + } + } getLogger().debug(`Parsed environment overrides: ${JSON.stringify(envOverrides)}`); @@ -217,9 +227,8 @@ export default class ActivityStream extends BaseService { await Promise.all(serviceOverrides.map((override) => this.patchServiceOverride(build, deploys, override))); // handle build uuid updates here - if (vanityUrl && vanityUrl !== build.uuid) { - const override = new OverrideService(); - await override.updateBuildUuid(build, vanityUrl); + if (requestedUuid) { + await override.updateBuildUuid(build, requestedUuid); } // if pull request should be built and deployed again, add it to build queue diff --git a/src/server/services/override.ts b/src/server/services/override.ts index 96f58601..3f7229c3 100644 --- a/src/server/services/override.ts +++ b/src/server/services/override.ts @@ -20,23 +20,30 @@ import { Build } from 'server/models'; import * as k8s from 'server/lib/kubernetes'; import DeployService from './deploy'; -interface ValidationResult { +export interface ValidationResult { valid: boolean; error?: string; } -interface UpdateResult { +export interface UpdateResult { build: Build; deploysUpdated: number; } +export class BuildUuidValidationError extends Error { + constructor(message: string) { + super(message); + this.name = 'BuildUuidValidationError'; + } +} + export default class OverrideService extends BaseService { /** * Validate UUID format and uniqueness * @param uuid The UUID to validate * @returns ValidationResult with validation status and error details */ - async validateUuid(uuid: string): Promise { + async validateUuid(uuid: string, currentBuildId?: number): Promise { if (uuid.length < 3 || uuid.length > 50) { return { valid: false, error: 'UUID must be between 3 and 50 characters' }; } @@ -52,7 +59,7 @@ export default class OverrideService extends BaseService { try { const existingBuild = await this.db.models.Build.query().findOne({ uuid }); - if (existingBuild) { + if (existingBuild && existingBuild.id !== currentBuildId) { return { valid: false, error: 'UUID is not available' }; } } catch (error) { @@ -77,6 +84,11 @@ export default class OverrideService extends BaseService { getLogger().info(`Override: updating newUuid=${newUuid}`); try { + const validation = await this.validateUuid(newUuid, build.id); + if (!validation.valid) { + throw new BuildUuidValidationError(validation.error || 'Invalid UUID'); + } + return await this.db.models.Build.transact(async (trx) => { await build.$query(trx).patch({ uuid: newUuid, @@ -93,14 +105,28 @@ export default class OverrideService extends BaseService { // this will not work for database configured services const deployService = new DeployService(); const updateDeploys = deploys.map(async (deploy) => { - const newDeployUuid = `${deploy.deployable.name}-${newUuid}`; - return deploy.$query(trx).patch({ + const deployName = deploy.deployable?.name ?? deploy.service?.name; + if (!deployName) { + getLogger().warn(`Deploy: missing service name while updating build UUID deployId=${deploy.id}`); + return; + } + + const newDeployUuid = `${deployName}-${newUuid}`; + deploy.uuid = newDeployUuid; + const patchFields: Record = { uuid: newDeployUuid, internalHostname: newDeployUuid, - publicUrl: build.enableFullYaml + }; + + const publicUrl = + build.enableFullYaml && deploy.deployable ? deployService.hostForDeployableDeploy(deploy, deploy.deployable) - : deployService.hostForServiceDeploy(deploy, deploy.service), - }); + : deployService.hostForServiceDeploy(deploy, deploy.service); + if (publicUrl) { + patchFields.publicUrl = publicUrl; + } + + return deploy.$query(trx).patch(patchFields); }); await Promise.all(updateDeploys); diff --git a/src/shared/openApiSpec.ts b/src/shared/openApiSpec.ts index f2f167ce..3a0e6365 100644 --- a/src/shared/openApiSpec.ts +++ b/src/shared/openApiSpec.ts @@ -3007,6 +3007,19 @@ export const openApiSpecificationForV2Api: OAS3Options = { ], }, + UpdateBuildUUIDRequest: { + type: 'object', + properties: { + uuid: { + type: 'string', + example: 'curly-meadow-171613', + description: 'The new UUID to assign to the build.', + }, + }, + required: ['uuid'], + additionalProperties: false, + }, + BuildMetadataLink: { type: 'object', properties: { @@ -3214,6 +3227,22 @@ export const openApiSpecificationForV2Api: OAS3Options = { ], }, + /** + * @description The specific success response for the PATCH /builds/{uuid} endpoint. + */ + UpdateBuildUUIDSuccessResponse: { + allOf: [ + { $ref: '#/components/schemas/SuccessApiResponse' }, + { + type: 'object', + properties: { + data: { $ref: '#/components/schemas/Build' }, + }, + required: ['data'], + }, + ], + }, + /** * @description The specific success response for the GET /schema/validate endpoint. */