From ee06769eb9e553b57c11e1e4f675cc0945bd33be Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Wed, 6 May 2026 19:18:37 -0700 Subject: [PATCH 1/2] fix: include comment env fields in build responses --- src/server/services/__tests__/build.test.ts | 83 +++++++++++++++++++++ src/server/services/build.ts | 8 +- src/shared/openApiSpec.test.ts | 12 +++ src/shared/openApiSpec.ts | 16 ++++ 4 files changed, 117 insertions(+), 2 deletions(-) diff --git a/src/server/services/__tests__/build.test.ts b/src/server/services/__tests__/build.test.ts index 0f95c2e..f67ba08 100644 --- a/src/server/services/__tests__/build.test.ts +++ b/src/server/services/__tests__/build.test.ts @@ -164,6 +164,89 @@ function createThenableQuery(result: any[] = []) { return query; } +describe('BuildService build response queries', () => { + function createQueueManager() { + return { + registerQueue: jest.fn(() => ({ + add: jest.fn(), + process: jest.fn(), + on: jest.fn(), + })), + }; + } + + test('selects comment env columns when listing builds', async () => { + const build = { + uuid: 'sample-build', + commentRuntimeEnv: { FEATURE_ENABLED: 'true' }, + commentInitEnv: { MIGRATION_ENABLED: 'true' }, + }; + const query: any = { + select: jest.fn(() => query), + where: jest.fn(() => query), + whereNotIn: jest.fn(() => query), + modify: jest.fn((callback: (builder: any) => void) => { + callback(query); + return query; + }), + withGraphFetched: jest.fn(() => query), + modifyGraph: jest.fn(() => query), + orderBy: jest.fn(() => query), + page: jest.fn().mockResolvedValue({ results: [build], total: 1 }), + }; + const buildService = new BuildService( + { + models: { + Build: { + query: jest.fn(() => query), + }, + }, + } as any, + {} as any, + {} as any, + createQueueManager() as any + ); + + const result = await buildService.getAllBuilds('', undefined, '', { page: 1, limit: 25 }); + + expect(result.data).toEqual([build]); + expect(query.select.mock.calls[0]).toEqual(expect.arrayContaining(['commentRuntimeEnv', 'commentInitEnv'])); + }); + + test('selects comment env columns when loading a build by UUID', async () => { + const build = { + uuid: 'sample-build', + commentRuntimeEnv: { FEATURE_ENABLED: 'true' }, + commentInitEnv: { MIGRATION_ENABLED: 'true' }, + }; + const query: any = { + findOne: jest.fn(() => query), + select: jest.fn(() => query), + withGraphFetched: jest.fn(() => query), + modifyGraph: jest.fn(() => query), + then: (resolve: (value: any) => void, reject: (reason: unknown) => void) => + Promise.resolve(build).then(resolve, reject), + }; + const buildService = new BuildService( + { + models: { + Build: { + query: jest.fn(() => query), + }, + }, + } as any, + {} as any, + {} as any, + createQueueManager() as any + ); + + await expect(buildService.getBuildByUUID('sample-build')).resolves.toBe(build); + + expect(query.findOne).toHaveBeenCalledWith({ uuid: 'sample-build' }); + expect(query.select.mock.calls[0]).toEqual(expect.arrayContaining(['commentRuntimeEnv', 'commentInitEnv'])); + }); +}); + describe('BuildService failure boundaries', () => { let buildService: BuildService; let recordDeployFailure: jest.Mock; diff --git a/src/server/services/build.ts b/src/server/services/build.ts index dc8c069..9de166d 100644 --- a/src/server/services/build.ts +++ b/src/server/services/build.ts @@ -254,7 +254,9 @@ export default class BuildService extends BaseService { 'updatedAt', 'isStatic', 'kind', - 'baseBuildId' + 'baseBuildId', + 'commentRuntimeEnv', + 'commentInitEnv' ) .where('kind', BuildKind.ENVIRONMENT) .whereNotIn('status', exclude) @@ -315,7 +317,9 @@ export default class BuildService extends BaseService { 'dependencyGraph', 'isStatic', 'kind', - 'baseBuildId' + 'baseBuildId', + 'commentRuntimeEnv', + 'commentInitEnv' ) .withGraphFetched('[baseBuild, pullRequest, deploys.[deployable, repository]]') .modifyGraph('pullRequest', (b) => { diff --git a/src/shared/openApiSpec.test.ts b/src/shared/openApiSpec.test.ts index e198127..8550242 100644 --- a/src/shared/openApiSpec.test.ts +++ b/src/shared/openApiSpec.test.ts @@ -35,6 +35,18 @@ describe('OpenAPI v2 agent session contract', () => { expect(schemas.UpdateBuildConfigSuccessResponse.allOf[1].properties.data).toEqual({ $ref: '#/components/schemas/Build', }); + expect(schemas.Build.properties.commentRuntimeEnv).toEqual( + expect.objectContaining({ + type: 'object', + additionalProperties: true, + }) + ); + expect(schemas.Build.properties.commentInitEnv).toEqual( + expect.objectContaining({ + type: 'object', + additionalProperties: true, + }) + ); expect(schemas.UpdateBuildServiceOverrideRequest).toBeUndefined(); expect(schemas.UpdateBuildEnvironmentOverridesRequest).toBeUndefined(); expect(schemas.UpdateBuildOptionsRequest).toBeUndefined(); diff --git a/src/shared/openApiSpec.ts b/src/shared/openApiSpec.ts index 753f97c..5687b0f 100644 --- a/src/shared/openApiSpec.ts +++ b/src/shared/openApiSpec.ts @@ -2974,6 +2974,22 @@ export const openApiSpecificationForV2Api: OAS3Options = { namespace: { type: 'string', example: 'env-white-poetry-596195' }, isStatic: { type: 'boolean', example: false }, baseBuildId: { type: 'integer', nullable: true }, + commentRuntimeEnv: { + type: 'object', + additionalProperties: true, + example: { + FEATURE_ENABLED: 'true', + }, + description: 'Runtime environment overrides parsed from build comments.', + }, + commentInitEnv: { + type: 'object', + additionalProperties: true, + example: { + MIGRATION_ENABLED: 'true', + }, + description: 'Init environment overrides parsed from build comments.', + }, createdAt: { type: 'string', format: 'date-time' }, updatedAt: { type: 'string', format: 'date-time' }, baseBuild: { From c900051dd192c2da89761c5ab6abf9e932255dc6 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Wed, 6 May 2026 19:30:20 -0700 Subject: [PATCH 2/2] fix: align build uuid validation --- src/app/api/v2/builds/[uuid]/route.ts | 10 ++++-- src/pages/api/v1/builds/[uuid]/index.ts | 21 +++++++++-- .../__tests__/buildUuidValidator.test.ts | 34 ++++++++++++++++++ .../lib/validation/buildUuidValidator.ts | 35 +++++++++++++++++++ .../services/__tests__/override.test.ts | 27 ++++++++++++++ src/server/services/override.ts | 14 +++----- src/shared/openApiSpec.test.ts | 7 ++++ src/shared/openApiSpec.ts | 6 +++- 8 files changed, 138 insertions(+), 16 deletions(-) create mode 100644 src/server/lib/validation/__tests__/buildUuidValidator.test.ts create mode 100644 src/server/lib/validation/buildUuidValidator.ts diff --git a/src/app/api/v2/builds/[uuid]/route.ts b/src/app/api/v2/builds/[uuid]/route.ts index 1cd7a0c..6febdbc 100644 --- a/src/app/api/v2/builds/[uuid]/route.ts +++ b/src/app/api/v2/builds/[uuid]/route.ts @@ -2,6 +2,7 @@ import { nanoid } from 'nanoid'; import { NextRequest } from 'next/server'; import { createApiHandler } from 'server/lib/createApiHandler'; import { errorResponse, successResponse } from 'server/lib/response'; +import { validateBuildUuidFormat } from 'server/lib/validation/buildUuidValidator'; import BuildService from 'server/services/build'; import OverrideService, { BuildUuidValidationError, type BuildConfigPatchInput } from 'server/services/override'; @@ -44,8 +45,13 @@ function validateBuildConfigPatch(body: unknown): BuildConfigPatchInput | Error const patch: BuildConfigPatchInput = {}; if (hasOwn(body, 'uuid')) { - if (typeof body.uuid !== 'string' || body.uuid.length === 0) { - return new Error('uuid must be a non-empty string'); + if (typeof body.uuid !== 'string') { + return new Error('uuid must be a string'); + } + + const uuidFormatError = validateBuildUuidFormat(body.uuid); + if (uuidFormatError) { + return new Error(uuidFormatError); } patch.uuid = body.uuid; } diff --git a/src/pages/api/v1/builds/[uuid]/index.ts b/src/pages/api/v1/builds/[uuid]/index.ts index 0416962..879ecd2 100644 --- a/src/pages/api/v1/builds/[uuid]/index.ts +++ b/src/pages/api/v1/builds/[uuid]/index.ts @@ -17,6 +17,7 @@ import { nanoid } from 'nanoid'; import { NextApiRequest, NextApiResponse } from 'next/types'; import { withLogContext, getLogger, LogStage } from 'server/lib/logger'; +import { validateBuildUuidFormat } from 'server/lib/validation/buildUuidValidator'; import BuildService from 'server/services/build'; import OverrideService, { BuildUuidValidationError } from 'server/services/override'; @@ -60,11 +61,22 @@ async function updateBuild(req: NextApiRequest, res: NextApiResponse, correlatio const { uuid } = req.query; const { uuid: newUuid } = req.body; - if (!newUuid || typeof newUuid !== 'string') { + if (newUuid == null) { getLogger().debug('Missing or invalid uuid in request body'); return res.status(400).json({ error: 'uuid is required' }); } + if (typeof newUuid !== 'string') { + getLogger().debug('Invalid uuid in request body'); + return res.status(400).json({ error: 'uuid must be a string' }); + } + + const uuidFormatError = validateBuildUuidFormat(newUuid); + if (uuidFormatError) { + getLogger().debug(`UUID validation failed: error=${uuidFormatError}`); + return res.status(400).json({ error: uuidFormatError }); + } + try { const override = new OverrideService(); @@ -211,7 +223,10 @@ async function updateBuild(req: NextApiRequest, res: NextApiResponse, correlatio * properties: * uuid: * type: string - * description: The new UUID (3-50 characters, alphanumeric + hyphens) + * minLength: 3 + * maxLength: 50 + * pattern: '^[a-z0-9-]+$' + * description: The new UUID (3-50 characters, lowercase letters, numbers, and hyphens) * example: my-custom-environment * required: * - uuid @@ -287,7 +302,7 @@ async function updateBuild(req: NextApiRequest, res: NextApiResponse, correlatio * same_uuid: * value: UUID must be different * invalid_format: - * value: UUID can only contain letters, numbers, and hyphens + * value: UUID can only contain lowercase letters, numbers, and hyphens * invalid_length: * value: UUID must be between 3 and 50 characters * invalid_boundaries: diff --git a/src/server/lib/validation/__tests__/buildUuidValidator.test.ts b/src/server/lib/validation/__tests__/buildUuidValidator.test.ts new file mode 100644 index 0000000..f1fe452 --- /dev/null +++ b/src/server/lib/validation/__tests__/buildUuidValidator.test.ts @@ -0,0 +1,34 @@ +/** + * 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 { validateBuildUuidFormat } from '../buildUuidValidator'; + +describe('validateBuildUuidFormat', () => { + test.each(['abc', 'abc-123', 'a1-b2-c3'])('accepts Kubernetes-safe UUID %s', (uuid) => { + expect(validateBuildUuidFormat(uuid)).toBeNull(); + }); + + test.each([ + ['ab', 'UUID must be between 3 and 50 characters'], + ['a'.repeat(51), 'UUID must be between 3 and 50 characters'], + ['ABC', 'UUID can only contain lowercase letters, numbers, and hyphens'], + ['abc_def', 'UUID can only contain lowercase letters, numbers, and hyphens'], + ['-abc', 'UUID cannot start or end with a hyphen'], + ['abc-', 'UUID cannot start or end with a hyphen'], + ])('rejects invalid UUID %s', (uuid, error) => { + expect(validateBuildUuidFormat(uuid)).toBe(error); + }); +}); diff --git a/src/server/lib/validation/buildUuidValidator.ts b/src/server/lib/validation/buildUuidValidator.ts new file mode 100644 index 0000000..da16333 --- /dev/null +++ b/src/server/lib/validation/buildUuidValidator.ts @@ -0,0 +1,35 @@ +/** + * 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. + */ + +export const BUILD_UUID_PATTERN = '^[a-z0-9-]+$'; +export const BUILD_UUID_MIN_LENGTH = 3; +export const BUILD_UUID_MAX_LENGTH = 50; + +export function validateBuildUuidFormat(uuid: string): string | null { + if (uuid.length < BUILD_UUID_MIN_LENGTH || uuid.length > BUILD_UUID_MAX_LENGTH) { + return 'UUID must be between 3 and 50 characters'; + } + + if (!new RegExp(BUILD_UUID_PATTERN).test(uuid)) { + return 'UUID can only contain lowercase letters, numbers, and hyphens'; + } + + if (uuid.startsWith('-') || uuid.endsWith('-')) { + return 'UUID cannot start or end with a hyphen'; + } + + return null; +} diff --git a/src/server/services/__tests__/override.test.ts b/src/server/services/__tests__/override.test.ts index 240104e..0949de7 100644 --- a/src/server/services/__tests__/override.test.ts +++ b/src/server/services/__tests__/override.test.ts @@ -730,6 +730,33 @@ describe('OverrideService.applyBuildOverrides', () => { }); }); +describe('OverrideService.validateUuid', () => { + it('rejects uppercase UUIDs before checking uniqueness', async () => { + const findOne = jest.fn(); + const query = jest.fn(() => ({ + findOne, + })); + const service = new OverrideService( + { + models: { + Build: { + query, + }, + }, + } as any, + {} as any, + {} as any, + {} as any + ); + + await expect(service.validateUuid('New-Build', 42)).resolves.toEqual({ + valid: false, + error: 'UUID can only contain lowercase letters, numbers, and hyphens', + }); + expect(query).not.toHaveBeenCalled(); + }); +}); + describe('OverrideService.applyBuildConfigPatch', () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/src/server/services/override.ts b/src/server/services/override.ts index bf469d2..7de6d84 100644 --- a/src/server/services/override.ts +++ b/src/server/services/override.ts @@ -16,6 +16,7 @@ import BaseService from './_service'; import { extractContextForQueue, getLogger, updateLogContext } from 'server/lib/logger'; +import { validateBuildUuidFormat } from 'server/lib/validation/buildUuidValidator'; import { Build, Deploy, PullRequest } from 'server/models'; import * as k8s from 'server/lib/kubernetes'; import DeployService from './deploy'; @@ -371,16 +372,9 @@ export default class OverrideService extends BaseService { * @returns ValidationResult with validation status and error details */ 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' }; - } - - if (!/^[a-zA-Z0-9-]+$/.test(uuid)) { - return { valid: false, error: 'UUID can only contain letters, numbers, and hyphens' }; - } - - if (uuid.startsWith('-') || uuid.endsWith('-')) { - return { valid: false, error: 'UUID cannot start or end with a hyphen' }; + const formatError = validateBuildUuidFormat(uuid); + if (formatError) { + return { valid: false, error: formatError }; } try { diff --git a/src/shared/openApiSpec.test.ts b/src/shared/openApiSpec.test.ts index 8550242..e528337 100644 --- a/src/shared/openApiSpec.test.ts +++ b/src/shared/openApiSpec.test.ts @@ -35,6 +35,13 @@ describe('OpenAPI v2 agent session contract', () => { expect(schemas.UpdateBuildConfigSuccessResponse.allOf[1].properties.data).toEqual({ $ref: '#/components/schemas/Build', }); + expect(schemas.UpdateBuildConfigPatchRequest.properties.uuid).toEqual( + expect.objectContaining({ + minLength: 3, + maxLength: 50, + pattern: '^[a-z0-9-]+$', + }) + ); expect(schemas.Build.properties.commentRuntimeEnv).toEqual( expect.objectContaining({ type: 'object', diff --git a/src/shared/openApiSpec.ts b/src/shared/openApiSpec.ts index 5687b0f..837cfd4 100644 --- a/src/shared/openApiSpec.ts +++ b/src/shared/openApiSpec.ts @@ -3028,8 +3028,12 @@ export const openApiSpecificationForV2Api: OAS3Options = { properties: { uuid: { type: 'string', + minLength: 3, + maxLength: 50, + pattern: '^[a-z0-9-]+$', example: 'curly-meadow-171613', - description: 'The new UUID to assign to the build.', + description: + 'The new UUID to assign to the build. Must use lowercase letters, numbers, and hyphens, and cannot start or end with a hyphen.', }, isStatic: { type: 'boolean',