From 0f92e4e6b9add712e9a1513ac586f8d99e55ccc4 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 5 Jun 2026 10:57:23 -0400 Subject: [PATCH 1/2] fix(api): harden task automations for MCP/API clients Three related hardening fixes for the automation endpoints that customers hit through the MCP server. 1. create-version no longer 500s on a malformed body. The endpoint used an inline, untyped @Body() that the global ValidationPipe could not see, so a missing version/scriptKey reached Prisma and threw a non-null violation (raw 500). Add a validated, documented CreateVersionDto and map Prisma failures to clean responses: duplicate version -> 409, missing -> 404. 2. Bake a finite default request timeout (x-speakeasy-timeout = 120s) into the generated SDK and MCP server. Without it the generated request functions resolve their timeout to -1 ("no timeout"), so a hung upstream call leaves the MCP fetch dangling forever and the client marks the whole connection unhealthy. 120s covers our slowest endpoints while staying under the ALB's 300s idle timeout. 3. Hide the two automation-authoring tools (create-automation, create-version) from the MCP surface. A working automation needs its evidence script generated, and that step lives only in the enterprise-api browser chat, so over MCP these POSTs only ever create an empty shell plus a version row pointing at a script that was never generated. Disable them via the generation overlay until script generation is a first-class endpoint; the HTTP endpoints stay live for the web UI. openapi.json is edited surgically (root key only) to avoid clobbering the open uploads PR (#3042); the full create-version body appears on the next regen. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/openapi-docs.spec.ts | 9 ++ apps/api/src/openapi/public-docs-metadata.ts | 31 +++++++ .../automations/automations.controller.ts | 5 +- .../automations/automations.service.spec.ts | 83 +++++++++++++++++++ .../tasks/automations/automations.service.ts | 63 +++++++++----- .../dto/create-version.dto.spec.ts | 49 +++++++++++ .../automations/dto/create-version.dto.ts | 35 ++++++++ .../.speakeasy/mcp-uploads-overlay.yaml | 19 +++++ packages/docs/openapi.json | 1 + 9 files changed, 272 insertions(+), 23 deletions(-) create mode 100644 apps/api/src/tasks/automations/automations.service.spec.ts create mode 100644 apps/api/src/tasks/automations/dto/create-version.dto.spec.ts create mode 100644 apps/api/src/tasks/automations/dto/create-version.dto.ts diff --git a/apps/api/src/openapi-docs.spec.ts b/apps/api/src/openapi-docs.spec.ts index 74f95f62d4..d9d42d89ae 100644 --- a/apps/api/src/openapi-docs.spec.ts +++ b/apps/api/src/openapi-docs.spec.ts @@ -113,6 +113,7 @@ import { AppModule } from './app.module'; import { applyPublicOpenApiMetadata, PUBLIC_OPENAPI_DESCRIPTION, + PUBLIC_OPENAPI_TIMEOUT_MS, PUBLIC_OPENAPI_TITLE, PUBLIC_SERVER_URL, } from './openapi/public-docs-metadata'; @@ -156,6 +157,14 @@ describe('OpenAPI document', () => { ]); }); + it('bakes a finite default request timeout into the generated SDK + MCP server', () => { + // Without x-speakeasy-timeout the generated request funcs use -1 ("no + // timeout") and a hung upstream wedges the MCP connection forever. + expect( + (document as { 'x-speakeasy-timeout'?: number })['x-speakeasy-timeout'], + ).toBe(PUBLIC_OPENAPI_TIMEOUT_MS); + }); + it('keeps the public spec complete, SEO-ready, and free of private surfaces', () => { const issues = collectPublicOpenApiIssues(document); diff --git a/apps/api/src/openapi/public-docs-metadata.ts b/apps/api/src/openapi/public-docs-metadata.ts index d2c8b9ad91..b3454be8d8 100644 --- a/apps/api/src/openapi/public-docs-metadata.ts +++ b/apps/api/src/openapi/public-docs-metadata.ts @@ -26,6 +26,31 @@ export const PUBLIC_OPENAPI_DESCRIPTION = export const PUBLIC_SERVER_URL = 'https://api.trycomp.ai'; +/** + * Default request timeout (ms) baked into the generated SDK + MCP server via the + * `x-speakeasy-timeout` document-root extension. + * + * Speakeasy-generated request functions resolve their timeout to + * `operationTimeoutMs || clientTimeoutMs || -1`, and `-1` means "no timeout". + * Without this, a slow/hung upstream call leaves the MCP server's fetch dangling + * forever; the MCP client eventually gives up and marks the whole connection + * unhealthy (customer-reported wedging). A finite timeout makes the SDK abort + * the request and return a clean error instead, keeping the connection alive. + * + * 120s comfortably covers our slowest endpoints while staying under the ALB's + * 300s idle timeout (comp-private infra/modules/loadbalancer.ts). + */ +export const PUBLIC_OPENAPI_TIMEOUT_MS = 120_000; + +/** + * OpenAPIObject (from @nestjs/swagger) has no index signature for `x-*` + * extensions at the document root, so we widen it locally instead of reaching + * for `as any`. + */ +type OpenApiDocumentWithExtensions = OpenAPIObject & { + 'x-speakeasy-timeout'?: number; +}; + function getVisibilityForOperation( operation: OpenApiOperation, metadata?: PublicOperationMetadata, @@ -284,6 +309,12 @@ export function applyPublicOpenApiMetadata(document: OpenAPIObject): void { }, ]; + // Bake a finite default request timeout into the generated SDK + MCP server + // so a hung upstream call can never wedge the MCP connection. See + // PUBLIC_OPENAPI_TIMEOUT_MS for the full rationale. + (document as OpenApiDocumentWithExtensions)['x-speakeasy-timeout'] = + PUBLIC_OPENAPI_TIMEOUT_MS; + const paths = document.paths as Record< string, Record diff --git a/apps/api/src/tasks/automations/automations.controller.ts b/apps/api/src/tasks/automations/automations.controller.ts index 05ce95b6cc..c07c3f0434 100644 --- a/apps/api/src/tasks/automations/automations.controller.ts +++ b/apps/api/src/tasks/automations/automations.controller.ts @@ -10,6 +10,7 @@ import { UseGuards, } from '@nestjs/common'; import { + ApiBody, ApiOperation, ApiParam, ApiResponse, @@ -22,6 +23,7 @@ import { PermissionGuard } from '../../auth/permission.guard'; import { RequirePermission } from '../../auth/require-permission.decorator'; import { TasksService } from '../tasks.service'; import { AutomationsService } from './automations.service'; +import { CreateVersionDto } from './dto/create-version.dto'; import { UpdateAutomationDto } from './dto/update-automation.dto'; import { AUTOMATION_OPERATIONS } from './schemas/automation-operations'; import { CREATE_AUTOMATION_RESPONSES } from './schemas/create-automation.responses'; @@ -255,11 +257,12 @@ export class AutomationsController { }) @ApiParam({ name: 'taskId', description: 'Task ID' }) @ApiParam({ name: 'automationId', description: 'Automation ID' }) + @ApiBody({ type: CreateVersionDto }) async createVersion( @OrganizationId() organizationId: string, @Param('taskId') taskId: string, @Param('automationId') automationId: string, - @Body() body: { version: number; scriptKey: string; changelog?: string }, + @Body() body: CreateVersionDto, ) { await this.tasksService.verifyTaskAccess(organizationId, taskId); return this.automationsService.createVersion(automationId, body); diff --git a/apps/api/src/tasks/automations/automations.service.spec.ts b/apps/api/src/tasks/automations/automations.service.spec.ts new file mode 100644 index 0000000000..f7a2746ae2 --- /dev/null +++ b/apps/api/src/tasks/automations/automations.service.spec.ts @@ -0,0 +1,83 @@ +import { ConflictException, NotFoundException } from '@nestjs/common'; + +// Mock the DB layer before importing the service. We also provide a stand-in +// Prisma.PrismaClientKnownRequestError so the service's `instanceof` checks and +// error-code branches can be exercised without a real database. +jest.mock('@db', () => { + class PrismaClientKnownRequestError extends Error { + code: string; + constructor(message: string, { code }: { code: string }) { + super(message); + this.code = code; + this.name = 'PrismaClientKnownRequestError'; + } + } + + return { + db: { + $transaction: jest.fn(), + evidenceAutomationVersion: { create: jest.fn() }, + evidenceAutomation: { update: jest.fn() }, + }, + Prisma: { PrismaClientKnownRequestError }, + }; +}); + +import { db, Prisma } from '@db'; +import { AutomationsService } from './automations.service'; + +const prismaError = (code: string) => + new Prisma.PrismaClientKnownRequestError(code, { + code, + clientVersion: '5.0.0', + }); + +describe('AutomationsService.createVersion — error mapping', () => { + let service: AutomationsService; + const input = { version: 1, scriptKey: 'org_1/tsk_1/aut_1.v1.js' }; + + beforeEach(() => { + jest.clearAllMocks(); + service = new AutomationsService(); + }); + + it('records the version and returns it on success', async () => { + const created = { id: 'eav_1', version: 1, scriptKey: input.scriptKey }; + (db.$transaction as jest.Mock).mockResolvedValue([created, { id: 'aut_1' }]); + + const result = await service.createVersion('aut_1', input); + + expect(result).toEqual({ success: true, version: created }); + }); + + it('maps a duplicate version (P2002) to a 409 ConflictException', async () => { + (db.$transaction as jest.Mock).mockRejectedValue(prismaError('P2002')); + + await expect(service.createVersion('aut_1', input)).rejects.toBeInstanceOf( + ConflictException, + ); + }); + + it('maps a missing automation (P2003 FK violation) to a 404 NotFoundException', async () => { + (db.$transaction as jest.Mock).mockRejectedValue(prismaError('P2003')); + + await expect( + service.createVersion('missing', input), + ).rejects.toBeInstanceOf(NotFoundException); + }); + + it('maps a missing automation (P2025 record not found) to a 404 NotFoundException', async () => { + (db.$transaction as jest.Mock).mockRejectedValue(prismaError('P2025')); + + await expect( + service.createVersion('missing', input), + ).rejects.toBeInstanceOf(NotFoundException); + }); + + it('rethrows unexpected errors untouched (no masking real 500s)', async () => { + const boom = new Error('db exploded'); + (db.$transaction as jest.Mock).mockRejectedValue(boom); + + await expect(service.createVersion('aut_1', input)).rejects.toBe(boom); + }); +}); diff --git a/apps/api/src/tasks/automations/automations.service.ts b/apps/api/src/tasks/automations/automations.service.ts index 86a6fd8ee9..1e8bf00254 100644 --- a/apps/api/src/tasks/automations/automations.service.ts +++ b/apps/api/src/tasks/automations/automations.service.ts @@ -1,5 +1,10 @@ -import { Injectable, NotFoundException } from '@nestjs/common'; -import { db } from '@db'; +import { + ConflictException, + Injectable, + NotFoundException, +} from '@nestjs/common'; +import { db, Prisma } from '@db'; +import { CreateVersionDto } from './dto/create-version.dto'; import { UpdateAutomationDto } from './dto/update-automation.dto'; @Injectable() @@ -135,26 +140,40 @@ export class AutomationsService { }; } - async createVersion( - automationId: string, - data: { version: number; scriptKey: string; changelog?: string }, - ) { - const [version] = await db.$transaction([ - db.evidenceAutomationVersion.create({ - data: { - evidenceAutomationId: automationId, - version: data.version, - scriptKey: data.scriptKey, - changelog: data.changelog, - }, - }), - // Enable automation on publish if not already enabled - db.evidenceAutomation.update({ - where: { id: automationId }, - data: { isEnabled: true }, - }), - ]); - return { success: true, version }; + async createVersion(automationId: string, data: CreateVersionDto) { + try { + const [version] = await db.$transaction([ + db.evidenceAutomationVersion.create({ + data: { + evidenceAutomationId: automationId, + version: data.version, + scriptKey: data.scriptKey, + changelog: data.changelog, + }, + }), + // Enable automation on publish if not already enabled + db.evidenceAutomation.update({ + where: { id: automationId }, + data: { isEnabled: true }, + }), + ]); + return { success: true, version }; + } catch (err) { + if (err instanceof Prisma.PrismaClientKnownRequestError) { + // Duplicate (evidenceAutomationId, version) — version already published. + if (err.code === 'P2002') { + throw new ConflictException( + `Version ${data.version} already exists for this automation`, + ); + } + // Automation row missing — FK on create (P2003) or update target gone + // (P2025). Surface a clean 404 instead of a raw 500. + if (err.code === 'P2003' || err.code === 'P2025') { + throw new NotFoundException(`Automation ${automationId} not found`); + } + } + throw err; + } } async findRunsByAutomationId(automationId: string) { diff --git a/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts b/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts new file mode 100644 index 0000000000..84957afb01 --- /dev/null +++ b/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts @@ -0,0 +1,49 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { CreateVersionDto } from './create-version.dto'; + +/** + * The original endpoint accepted an inline, untyped `@Body()` — invisible to the + * ValidationPipe — so a missing `version`/`scriptKey` slipped through and blew up + * with a Prisma non-null violation (500). These tests prove the DTO now rejects + * those payloads at the validation layer (400) before they reach the service. + */ +describe('CreateVersionDto', () => { + async function validatePayload(payload: Record) { + return validate(plainToInstance(CreateVersionDto, payload)); + } + + it('accepts a valid payload', async () => { + const errors = await validatePayload({ + version: 1, + scriptKey: 'org_1/tsk_1/aut_1.v1.js', + changelog: 'initial publish', + }); + expect(errors).toHaveLength(0); + }); + + it('rejects a missing version (previously a 500)', async () => { + const errors = await validatePayload({ scriptKey: 'k' }); + expect(errors.some((e) => e.property === 'version')).toBe(true); + }); + + it('rejects a missing scriptKey (previously a 500)', async () => { + const errors = await validatePayload({ version: 1 }); + expect(errors.some((e) => e.property === 'scriptKey')).toBe(true); + }); + + it('rejects a version below 1', async () => { + const errors = await validatePayload({ version: 0, scriptKey: 'k' }); + expect(errors.some((e) => e.property === 'version')).toBe(true); + }); + + it('rejects an empty scriptKey', async () => { + const errors = await validatePayload({ version: 1, scriptKey: '' }); + expect(errors.some((e) => e.property === 'scriptKey')).toBe(true); + }); + + it('treats changelog as optional', async () => { + const errors = await validatePayload({ version: 2, scriptKey: 'k' }); + expect(errors.some((e) => e.property === 'changelog')).toBe(false); + }); +}); diff --git a/apps/api/src/tasks/automations/dto/create-version.dto.ts b/apps/api/src/tasks/automations/dto/create-version.dto.ts new file mode 100644 index 0000000000..a44e74cc6d --- /dev/null +++ b/apps/api/src/tasks/automations/dto/create-version.dto.ts @@ -0,0 +1,35 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { IsInt, IsNotEmpty, IsOptional, IsString, Min } from 'class-validator'; + +/** + * Records that an automation script has been generated + published to S3. + * `version` and `scriptKey` are REQUIRED — the row references an already-stored + * script. The web UI's publish flow supplies both from the enterprise publish + * step; calling this without them used to 500 (Prisma non-null violation). + */ +export class CreateVersionDto { + @ApiProperty({ + description: 'Version number for this published script', + example: 1, + }) + @IsInt() + @Min(1) + version!: number; + + @ApiProperty({ + description: + 'S3 key of the already-generated & published automation script (returned by the publish step).', + example: 'org_abc123/tsk_abc123/aut_abc123.v1.js', + }) + @IsString() + @IsNotEmpty() + scriptKey!: string; + + @ApiProperty({ + description: 'Optional changelog describing this version', + required: false, + }) + @IsOptional() + @IsString() + changelog?: string; +} diff --git a/apps/mcp-server/.speakeasy/mcp-uploads-overlay.yaml b/apps/mcp-server/.speakeasy/mcp-uploads-overlay.yaml index 8c6e95b18e..f0b0d393df 100644 --- a/apps/mcp-server/.speakeasy/mcp-uploads-overlay.yaml +++ b/apps/mcp-server/.speakeasy/mcp-uploads-overlay.yaml @@ -79,3 +79,22 @@ actions: update: x-speakeasy-mcp: disabled: true + + # 7-8. Disable the automation-authoring tools for MCP. Creating a working + # automation requires generating its evidence-collection SCRIPT, and that + # step lives ONLY in the enterprise-api browser chat (an AI tool writes the + # .draft.js to S3, then publish copies it to .vN.js). Neither the public API + # nor MCP can produce that script. So over MCP these two POSTs only ever + # create an empty automation shell + a version row pointing at a script that + # was never generated — a dead end that confuses agents. Hide them until the + # script-generation step is exposed as a first-class (async) endpoint. + # The HTTP endpoints stay live for the web UI, which drives the full flow. + # Read/list/update automation tools remain available over MCP. + - target: "$.paths['/v1/tasks/{taskId}/automations'].post" + update: + x-speakeasy-mcp: + disabled: true + - target: "$.paths['/v1/tasks/{taskId}/automations/{automationId}/versions'].post" + update: + x-speakeasy-mcp: + disabled: true diff --git a/packages/docs/openapi.json b/packages/docs/openapi.json index 99f0678f1c..307557782f 100644 --- a/packages/docs/openapi.json +++ b/packages/docs/openapi.json @@ -1,5 +1,6 @@ { "openapi": "3.0.0", + "x-speakeasy-timeout": 120000, "paths": { "/v1/organization": { "get": { From 4bf5454e34eca1bbfe5bdc9f231c62932b612c66 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 5 Jun 2026 11:24:55 -0400 Subject: [PATCH 2/2] fix(api): reject whitespace-only scriptKey in CreateVersionDto @IsNotEmpty() treats a whitespace-only string (" ", "\t\n") as valid, so a blank scriptKey could pass validation and persist a version row whose script can never be fetched at runtime. Trim the value first (type-guarded so non-strings still hit @IsString) so a blank collapses to "" and @IsNotEmpty rejects it; legitimate keys are normalized. Issue identified by cubic on PR #3044. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../automations/dto/create-version.dto.spec.ts | 17 +++++++++++++++++ .../tasks/automations/dto/create-version.dto.ts | 5 +++++ 2 files changed, 22 insertions(+) diff --git a/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts b/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts index 84957afb01..5396fff702 100644 --- a/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts +++ b/apps/api/src/tasks/automations/dto/create-version.dto.spec.ts @@ -42,6 +42,23 @@ describe('CreateVersionDto', () => { expect(errors.some((e) => e.property === 'scriptKey')).toBe(true); }); + it('rejects a whitespace-only scriptKey (would otherwise persist a blank key)', async () => { + for (const scriptKey of [' ', '\t\n', '   ']) { + const errors = await validatePayload({ version: 1, scriptKey }); + expect(errors.some((e) => e.property === 'scriptKey')).toBe(true); + } + }); + + it('trims surrounding whitespace from a valid scriptKey', async () => { + const dto = plainToInstance(CreateVersionDto, { + version: 1, + scriptKey: ' org_1/tsk_1/aut_1.v1.js ', + }); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + expect(dto.scriptKey).toBe('org_1/tsk_1/aut_1.v1.js'); + }); + it('treats changelog as optional', async () => { const errors = await validatePayload({ version: 2, scriptKey: 'k' }); expect(errors.some((e) => e.property === 'changelog')).toBe(false); diff --git a/apps/api/src/tasks/automations/dto/create-version.dto.ts b/apps/api/src/tasks/automations/dto/create-version.dto.ts index a44e74cc6d..8bc521f111 100644 --- a/apps/api/src/tasks/automations/dto/create-version.dto.ts +++ b/apps/api/src/tasks/automations/dto/create-version.dto.ts @@ -1,4 +1,5 @@ import { ApiProperty } from '@nestjs/swagger'; +import { Transform } from 'class-transformer'; import { IsInt, IsNotEmpty, IsOptional, IsString, Min } from 'class-validator'; /** @@ -22,6 +23,10 @@ export class CreateVersionDto { example: 'org_abc123/tsk_abc123/aut_abc123.v1.js', }) @IsString() + // Trim first so a whitespace-only key collapses to '' and @IsNotEmpty rejects + // it — otherwise a blank key would persist and the automation would later + // fail to fetch a script at that key. Non-strings pass through for @IsString. + @Transform(({ value }) => (typeof value === 'string' ? value.trim() : value)) @IsNotEmpty() scriptKey!: string;