From ade4bfacbf6041a8b6d26f2d96debf1ba2bd4793 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 12 Jun 2026 16:30:28 -0400 Subject: [PATCH 1/4] fix(cloud-security): run AWS integration checks on our server (scheduled false-fails) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AWS integration checks (S3, EC2, RDS, KMS, CloudTrail, IAM) false-failed on the scheduled and auto-run paths while the manual "Run" passed. Same code, same credentials — the only difference is where the check executes: - Manual runs on our server (ECS), inside our VPC, whose S3 endpoint allows our cross-account audit reads. - Scheduled/auto-run execute in the Trigger.dev runtime, whose VPC S3 endpoint policy blocks cross-account reads ("no VPC endpoint policy allows ..."). AWS allows/denies based on the VPC the request exits from, which lives in Trigger.dev's account and isn't ours to change. Fix (AWS only): when providerSlug === 'aws', the scheduled and auto-run Trigger tasks delegate execution to our server via a new service-token endpoint (POST /v1/integrations/internal/run-connection-checks/:connectionId) and persist the returned result with the existing shared logic. Trigger.dev still handles scheduling and retries; only where the AWS API calls run changed. GCP, Azure, dynamic and legacy integrations are untouched — they make plain HTTPS calls (no VPC endpoint) and keep executing in Trigger.dev exactly as before. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal-checks.controller.spec.ts | 66 ++++++++ .../controllers/internal-checks.controller.ts | 60 ++++++++ .../integration-platform.module.ts | 4 + .../connection-check-runner.service.spec.ts | 145 ++++++++++++++++++ .../connection-check-runner.service.ts | 144 +++++++++++++++++ apps/api/src/openapi/public-docs-quality.ts | 1 + .../run-checks-on-server.spec.ts | 79 ++++++++++ .../run-checks-on-server.ts | 57 +++++++ .../run-connection-checks.ts | 41 +++-- .../run-task-integration-checks.ts | 48 ++++-- 10 files changed, 613 insertions(+), 32 deletions(-) create mode 100644 apps/api/src/integration-platform/controllers/internal-checks.controller.spec.ts create mode 100644 apps/api/src/integration-platform/controllers/internal-checks.controller.ts create mode 100644 apps/api/src/integration-platform/services/connection-check-runner.service.spec.ts create mode 100644 apps/api/src/integration-platform/services/connection-check-runner.service.ts create mode 100644 apps/api/src/trigger/integration-platform/run-checks-on-server.spec.ts create mode 100644 apps/api/src/trigger/integration-platform/run-checks-on-server.ts diff --git a/apps/api/src/integration-platform/controllers/internal-checks.controller.spec.ts b/apps/api/src/integration-platform/controllers/internal-checks.controller.spec.ts new file mode 100644 index 0000000000..4a704dabf0 --- /dev/null +++ b/apps/api/src/integration-platform/controllers/internal-checks.controller.spec.ts @@ -0,0 +1,66 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { InternalChecksController } from './internal-checks.controller'; +import { HybridAuthGuard } from '../../auth/hybrid-auth.guard'; +import { PermissionGuard } from '../../auth/permission.guard'; +import { ServiceTokenOnlyGuard } from '../../auth/service-token-only.guard'; +import { ConnectionCheckRunnerService } from '../services/connection-check-runner.service'; + +jest.mock('@db', () => ({ db: {} })); +jest.mock('../../auth/auth.server', () => ({ + auth: { api: { getSession: jest.fn() } }, +})); +jest.mock('@trycompai/auth', () => ({ + statement: { integration: ['create', 'read', 'update', 'delete'] }, + BUILT_IN_ROLE_PERMISSIONS: {}, +})); + +describe('InternalChecksController', () => { + let controller: InternalChecksController; + const mockRunner = { runChecks: jest.fn() }; + const mockGuard = { canActivate: jest.fn().mockReturnValue(true) }; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + controllers: [InternalChecksController], + providers: [ + { provide: ConnectionCheckRunnerService, useValue: mockRunner }, + ], + }) + .overrideGuard(HybridAuthGuard) + .useValue(mockGuard) + .overrideGuard(ServiceTokenOnlyGuard) + .useValue(mockGuard) + .overrideGuard(PermissionGuard) + .useValue(mockGuard) + .compile(); + + controller = module.get(InternalChecksController); + jest.clearAllMocks(); + }); + + it('delegates to the runner with the connection, org and checkId', async () => { + const runResult = { results: [], totalFindings: 0, totalPassing: 0 }; + mockRunner.runChecks.mockResolvedValue(runResult); + + const result = await controller.runConnectionChecks('conn_1', 'org_1', { + checkId: 'aws-s3-public-access', + }); + + expect(mockRunner.runChecks).toHaveBeenCalledWith({ + connectionId: 'conn_1', + organizationId: 'org_1', + checkId: 'aws-s3-public-access', + }); + expect(result).toBe(runResult); + }); + + it('passes checkId undefined when omitted (run all)', async () => { + mockRunner.runChecks.mockResolvedValue({}); + await controller.runConnectionChecks('conn_1', 'org_1', {}); + expect(mockRunner.runChecks).toHaveBeenCalledWith({ + connectionId: 'conn_1', + organizationId: 'org_1', + checkId: undefined, + }); + }); +}); diff --git a/apps/api/src/integration-platform/controllers/internal-checks.controller.ts b/apps/api/src/integration-platform/controllers/internal-checks.controller.ts new file mode 100644 index 0000000000..8da42fa375 --- /dev/null +++ b/apps/api/src/integration-platform/controllers/internal-checks.controller.ts @@ -0,0 +1,60 @@ +import { Body, Controller, Param, Post, UseGuards } from '@nestjs/common'; +import { + ApiBody, + ApiOperation, + ApiPropertyOptional, + ApiTags, +} from '@nestjs/swagger'; +import { IsOptional, IsString } from 'class-validator'; +import { HybridAuthGuard } from '../../auth/hybrid-auth.guard'; +import { PermissionGuard } from '../../auth/permission.guard'; +import { ServiceTokenOnlyGuard } from '../../auth/service-token-only.guard'; +import { RequirePermission } from '../../auth/require-permission.decorator'; +import { OrganizationId } from '../../auth/auth-context.decorator'; +import { + ConnectionCheckRunnerService, + type RunAllChecksResult, +} from '../services/connection-check-runner.service'; + +// Internal payload. Service-token only — never called by the UI/customers. +class RunConnectionChecksOnServerDto { + @ApiPropertyOptional({ + description: + "Run a single check. Omit to run all of the connection's checks.", + }) + @IsOptional() + @IsString() + checkId?: string; +} + +/** + * Internal, service-token-only endpoint that runs a connection's checks ON OUR + * SERVER and returns the raw result (no persistence). Used exclusively by the + * AWS Trigger tasks so AWS S3 calls egress our VPC instead of Trigger.dev's + * (whose endpoint policy blocks our cross-account reads). All other providers + * keep executing inside Trigger.dev unchanged. + */ +@Controller({ path: 'integrations/internal', version: '1' }) +@ApiTags('Integrations') +export class InternalChecksController { + constructor(private readonly runner: ConnectionCheckRunnerService) {} + + @Post('run-connection-checks/:connectionId') + @UseGuards(HybridAuthGuard, ServiceTokenOnlyGuard, PermissionGuard) + @RequirePermission('integration', 'update') + @ApiOperation({ + summary: "Run a connection's checks on the API server (internal only)", + }) + @ApiBody({ type: RunConnectionChecksOnServerDto }) + async runConnectionChecks( + @Param('connectionId') connectionId: string, + @OrganizationId() organizationId: string, + @Body() body: RunConnectionChecksOnServerDto, + ): Promise { + return this.runner.runChecks({ + connectionId, + organizationId, + checkId: body.checkId, + }); + } +} diff --git a/apps/api/src/integration-platform/integration-platform.module.ts b/apps/api/src/integration-platform/integration-platform.module.ts index 4ed3260faa..87c029e79d 100644 --- a/apps/api/src/integration-platform/integration-platform.module.ts +++ b/apps/api/src/integration-platform/integration-platform.module.ts @@ -7,6 +7,7 @@ import { ConnectionsController } from './controllers/connections.controller'; import { AdminIntegrationsController } from './controllers/admin-integrations.controller'; import { DynamicIntegrationsController } from './controllers/dynamic-integrations.controller'; import { ChecksController } from './controllers/checks.controller'; +import { InternalChecksController } from './controllers/internal-checks.controller'; import { VariablesController } from './controllers/variables.controller'; import { TaskIntegrationsController } from './controllers/task-integrations.controller'; import { WebhookController } from './controllers/webhook.controller'; @@ -20,6 +21,7 @@ import { ConnectionAuthTeardownService } from './services/connection-auth-teardo import { OAuthTokenRevocationService } from './services/oauth-token-revocation.service'; import { DynamicManifestLoaderService } from './services/dynamic-manifest-loader.service'; import { TaskIntegrationChecksService } from './services/task-integration-checks.service'; +import { ConnectionCheckRunnerService } from './services/connection-check-runner.service'; import { ProviderRepository } from './repositories/provider.repository'; import { ConnectionRepository } from './repositories/connection.repository'; import { CredentialRepository } from './repositories/credential.repository'; @@ -42,6 +44,7 @@ import { GenericDeviceSyncService } from './services/generic-device-sync.service AdminIntegrationsController, DynamicIntegrationsController, ChecksController, + InternalChecksController, VariablesController, TaskIntegrationsController, WebhookController, @@ -58,6 +61,7 @@ import { GenericDeviceSyncService } from './services/generic-device-sync.service ConnectionAuthTeardownService, DynamicManifestLoaderService, TaskIntegrationChecksService, + ConnectionCheckRunnerService, IntegrationSyncLoggerService, GenericEmployeeSyncService, GenericDeviceSyncService, diff --git a/apps/api/src/integration-platform/services/connection-check-runner.service.spec.ts b/apps/api/src/integration-platform/services/connection-check-runner.service.spec.ts new file mode 100644 index 0000000000..1dc356288c --- /dev/null +++ b/apps/api/src/integration-platform/services/connection-check-runner.service.spec.ts @@ -0,0 +1,145 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { BadRequestException, NotFoundException } from '@nestjs/common'; +import { ConnectionCheckRunnerService } from './connection-check-runner.service'; +import { ConnectionRepository } from '../repositories/connection.repository'; +import { ProviderRepository } from '../repositories/provider.repository'; +import { CredentialVaultService } from './credential-vault.service'; +import { OAuthCredentialsService } from './oauth-credentials.service'; + +jest.mock('@db', () => ({ db: {} })); + +jest.mock('@trycompai/integration-platform', () => ({ + getManifest: jest.fn(), + runAllChecks: jest.fn(), +})); + +import { getManifest, runAllChecks } from '@trycompai/integration-platform'; + +const mockedGetManifest = getManifest as jest.Mock; +const mockedRunAllChecks = runAllChecks as jest.Mock; + +const AWS_MANIFEST = { + id: 'aws', + name: 'AWS', + auth: { type: 'custom' }, + checks: [{ id: 'aws-s3-public-access', name: 'S3 public access' }], +}; + +const RUN_RESULT = { + results: [{ checkId: 'aws-s3-public-access', status: 'success', result: {} }], + totalFindings: 0, + totalPassing: 3, +}; + +describe('ConnectionCheckRunnerService', () => { + let service: ConnectionCheckRunnerService; + + const mockConnectionRepository = { findById: jest.fn() }; + const mockProviderRepository = { findById: jest.fn() }; + const mockCredentialVaultService = { + getDecryptedCredentials: jest.fn(), + getValidAccessToken: jest.fn(), + refreshOAuthTokens: jest.fn(), + }; + const mockOAuthCredentialsService = { getCredentials: jest.fn() }; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + ConnectionCheckRunnerService, + { provide: ConnectionRepository, useValue: mockConnectionRepository }, + { provide: ProviderRepository, useValue: mockProviderRepository }, + { + provide: CredentialVaultService, + useValue: mockCredentialVaultService, + }, + { + provide: OAuthCredentialsService, + useValue: mockOAuthCredentialsService, + }, + ], + }).compile(); + + service = module.get(ConnectionCheckRunnerService); + jest.clearAllMocks(); + + mockConnectionRepository.findById.mockResolvedValue({ + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + status: 'active', + variables: {}, + }); + mockProviderRepository.findById.mockResolvedValue({ + id: 'prov_aws', + slug: 'aws', + }); + mockedGetManifest.mockReturnValue(AWS_MANIFEST); + mockCredentialVaultService.getDecryptedCredentials.mockResolvedValue({ + roleArn: 'arn:aws:iam::111111111111:role/x', + externalId: 'ext', + }); + mockedRunAllChecks.mockResolvedValue(RUN_RESULT); + }); + + it('runs the checks on the server and returns the raw result (no persistence)', async () => { + const result = await service.runChecks({ + connectionId: 'conn_1', + organizationId: 'org_1', + checkId: 'aws-s3-public-access', + }); + + expect(mockedRunAllChecks).toHaveBeenCalledWith( + expect.objectContaining({ + connectionId: 'conn_1', + organizationId: 'org_1', + checkId: 'aws-s3-public-access', + }), + ); + expect(result).toBe(RUN_RESULT); + }); + + it('runs ALL checks when no checkId is given (auto-run path)', async () => { + await service.runChecks({ + connectionId: 'conn_1', + organizationId: 'org_1', + }); + expect(mockedRunAllChecks).toHaveBeenCalledWith( + expect.objectContaining({ checkId: undefined }), + ); + }); + + it('throws NotFound for a connection in another org (no cross-tenant run)', async () => { + mockConnectionRepository.findById.mockResolvedValue({ + id: 'conn_1', + organizationId: 'org_OTHER', + providerId: 'prov_aws', + status: 'active', + }); + await expect( + service.runChecks({ connectionId: 'conn_1', organizationId: 'org_1' }), + ).rejects.toBeInstanceOf(NotFoundException); + expect(mockedRunAllChecks).not.toHaveBeenCalled(); + }); + + it('throws BadRequest for an inactive connection', async () => { + mockConnectionRepository.findById.mockResolvedValue({ + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + status: 'paused', + }); + await expect( + service.runChecks({ connectionId: 'conn_1', organizationId: 'org_1' }), + ).rejects.toBeInstanceOf(BadRequestException); + expect(mockedRunAllChecks).not.toHaveBeenCalled(); + }); + + it('throws BadRequest when credentials are missing', async () => { + mockCredentialVaultService.getDecryptedCredentials.mockResolvedValue(null); + await expect( + service.runChecks({ connectionId: 'conn_1', organizationId: 'org_1' }), + ).rejects.toBeInstanceOf(BadRequestException); + expect(mockedRunAllChecks).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/integration-platform/services/connection-check-runner.service.ts b/apps/api/src/integration-platform/services/connection-check-runner.service.ts new file mode 100644 index 0000000000..1e1b8f22f0 --- /dev/null +++ b/apps/api/src/integration-platform/services/connection-check-runner.service.ts @@ -0,0 +1,144 @@ +import { + BadRequestException, + Injectable, + Logger, + NotFoundException, +} from '@nestjs/common'; +import { getManifest, runAllChecks } from '@trycompai/integration-platform'; +import { ConnectionRepository } from '../repositories/connection.repository'; +import { ProviderRepository } from '../repositories/provider.repository'; +import { CredentialVaultService } from './credential-vault.service'; +import { OAuthCredentialsService } from './oauth-credentials.service'; +import { getStringValue } from '../utils/credential-utils'; + +export type RunAllChecksResult = Awaited>; + +/** + * Runs integration checks for a connection ON OUR SERVER (the API/ECS process) + * and returns the raw result WITHOUT persisting anything. + * + * Why this exists: AWS checks make S3 (and other) API calls that egress the + * runtime's network. In the Trigger.dev runtime those calls exit Trigger.dev's + * VPC, whose S3 endpoint policy blocks our cross-account audit reads + * ("no VPC endpoint policy allows ..."). Running them here egresses OUR VPC, + * whose endpoint allows the read — identical to the in-app manual "Run". + * + * Only the AWS Trigger tasks call this; GCP/Azure/dynamic/legacy integrations + * keep executing in Trigger.dev unchanged. Persistence + task status + emails + * stay in the caller, so AWS results are recorded exactly like every other + * provider's. + */ +@Injectable() +export class ConnectionCheckRunnerService { + private readonly logger = new Logger(ConnectionCheckRunnerService.name); + + constructor( + private readonly connectionRepository: ConnectionRepository, + private readonly providerRepository: ProviderRepository, + private readonly credentialVaultService: CredentialVaultService, + private readonly oauthCredentialsService: OAuthCredentialsService, + ) {} + + /** + * Run a connection's checks and return the raw `runAllChecks` result. + * Pass `checkId` to run a single check; omit it to run all of the + * connection's checks. Does NOT write to the database. + */ + async runChecks(params: { + connectionId: string; + organizationId: string; + checkId?: string; + }): Promise { + const { connectionId, organizationId, checkId } = params; + + const connection = await this.connectionRepository.findById(connectionId); + if (!connection || connection.organizationId !== organizationId) { + throw new NotFoundException('Connection not found'); + } + if (connection.status !== 'active') { + throw new BadRequestException( + `Connection is not active (status: ${connection.status})`, + ); + } + + const provider = await this.providerRepository.findById( + connection.providerId, + ); + if (!provider) { + throw new NotFoundException('Provider not found'); + } + + const manifest = getManifest(provider.slug); + if (!manifest) { + throw new NotFoundException(`Manifest for ${provider.slug} not found`); + } + if (!manifest.checks || manifest.checks.length === 0) { + throw new BadRequestException(`No checks defined for ${provider.slug}`); + } + + const credentials = + await this.credentialVaultService.getDecryptedCredentials(connectionId); + if (!credentials) { + throw new BadRequestException('No credentials found for connection'); + } + + const variables = + (connection.variables as Record< + string, + string | number | boolean | string[] | undefined + >) || {}; + + // Build the OAuth refresh callback for providers that support it. AWS is + // not oauth2, so this is a no-op for the AWS path that actually uses this. + let accessToken = getStringValue(credentials.access_token); + let onTokenRefresh: (() => Promise) | undefined; + if (manifest.auth.type === 'oauth2') { + const oauthConfig = manifest.auth.config; + if (oauthConfig.supportsRefreshToken !== false) { + const oauthCredentials = + await this.oauthCredentialsService.getCredentials( + provider.slug, + organizationId, + ); + if (oauthCredentials) { + const refreshConfig = { + tokenUrl: oauthConfig.tokenUrl, + refreshUrl: oauthConfig.refreshUrl, + clientId: oauthCredentials.clientId, + clientSecret: oauthCredentials.clientSecret, + clientAuthMethod: oauthConfig.clientAuthMethod, + scope: oauthCredentials.scopes.join(' '), + tokenParams: oauthConfig.tokenParams, + }; + const validAccessToken = + await this.credentialVaultService.getValidAccessToken( + connectionId, + refreshConfig, + ); + if (validAccessToken) accessToken = validAccessToken; + onTokenRefresh = () => + this.credentialVaultService.refreshOAuthTokens( + connectionId, + refreshConfig, + ); + } + } + } + + return runAllChecks({ + manifest, + accessToken, + credentials, + variables, + connectionId, + organizationId, + checkId, + onTokenRefresh, + logger: { + info: (msg, data) => this.logger.log(msg, data), + warn: (msg, data) => this.logger.warn(msg, data), + error: (msg, data) => this.logger.error(msg, data), + }, + }); + } +} diff --git a/apps/api/src/openapi/public-docs-quality.ts b/apps/api/src/openapi/public-docs-quality.ts index e133d52e3e..04e2d82be8 100644 --- a/apps/api/src/openapi/public-docs-quality.ts +++ b/apps/api/src/openapi/public-docs-quality.ts @@ -19,6 +19,7 @@ export const PUBLIC_DOCS_EXCLUDED_PREFIXES = [ '/v1/finding-template', '/v1/integrations/oauth', '/v1/integrations/oauth-apps', + '/v1/integrations/internal', '/v1/cloud-security/legacy', '/v1/cloud-security/remediation', '/v1/questionnaire/parse/upload/token', diff --git a/apps/api/src/trigger/integration-platform/run-checks-on-server.spec.ts b/apps/api/src/trigger/integration-platform/run-checks-on-server.spec.ts new file mode 100644 index 0000000000..22acf4559e --- /dev/null +++ b/apps/api/src/trigger/integration-platform/run-checks-on-server.spec.ts @@ -0,0 +1,79 @@ +import { runChecksOnServer } from './run-checks-on-server'; + +describe('runChecksOnServer', () => { + const ORIGINAL_TOKEN = process.env.SERVICE_TOKEN_TRIGGER; + const params = { + apiUrl: 'http://api', + connectionId: 'conn_1', + organizationId: 'org_1', + }; + + beforeEach(() => { + process.env.SERVICE_TOKEN_TRIGGER = 'svc-token'; + jest.restoreAllMocks(); + }); + + afterAll(() => { + if (ORIGINAL_TOKEN === undefined) delete process.env.SERVICE_TOKEN_TRIGGER; + else process.env.SERVICE_TOKEN_TRIGGER = ORIGINAL_TOKEN; + }); + + it('POSTs to the internal endpoint with service token + org header and returns the result', async () => { + const runResult = { results: [{}], totalFindings: 1, totalPassing: 2 }; + const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValue({ + ok: true, + json: async () => runResult, + } as unknown as Response); + + const result = await runChecksOnServer({ + ...params, + checkId: 'aws-s3-public-access', + }); + + expect(fetchMock).toHaveBeenCalledWith( + 'http://api/v1/integrations/internal/run-connection-checks/conn_1', + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({ + 'x-service-token': 'svc-token', + 'x-organization-id': 'org_1', + }), + body: JSON.stringify({ checkId: 'aws-s3-public-access' }), + }), + ); + expect(result).toEqual(runResult); + }); + + it('sends an empty body when no checkId is given (run all)', async () => { + const fetchMock = jest + .spyOn(global, 'fetch') + .mockResolvedValue({ + ok: true, + json: async () => ({}), + } as unknown as Response); + + await runChecksOnServer(params); + + expect(fetchMock).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ body: JSON.stringify({}) }), + ); + }); + + it('throws with the server message on a non-2xx response', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue({ + ok: false, + status: 500, + json: async () => ({ message: 'boom' }), + } as unknown as Response); + + await expect(runChecksOnServer(params)).rejects.toThrow('boom'); + }); + + it('throws when SERVICE_TOKEN_TRIGGER is not configured', async () => { + delete process.env.SERVICE_TOKEN_TRIGGER; + await expect(runChecksOnServer(params)).rejects.toThrow( + 'SERVICE_TOKEN_TRIGGER is not configured', + ); + }); +}); diff --git a/apps/api/src/trigger/integration-platform/run-checks-on-server.ts b/apps/api/src/trigger/integration-platform/run-checks-on-server.ts new file mode 100644 index 0000000000..695d8cc54d --- /dev/null +++ b/apps/api/src/trigger/integration-platform/run-checks-on-server.ts @@ -0,0 +1,57 @@ +import type { runAllChecks } from '@trycompai/integration-platform'; + +export type RunAllChecksResult = Awaited>; + +/** + * Run a connection's checks ON OUR SERVER (ECS) and return the raw result. + * + * Used by the AWS Trigger tasks only: AWS S3 calls made from the Trigger.dev + * runtime egress Trigger.dev's VPC, whose endpoint policy blocks our + * cross-account reads. Running them on our server egresses our own VPC (where + * the endpoint allows the read) — matching the in-app manual "Run". The caller + * still persists the returned result, so AWS runs are recorded exactly like + * every other provider's. + * + * Pass `checkId` to run a single check (scheduled path); omit it to run all of + * the connection's checks (auto-run-after-connect path). + * + * Throws on a transport failure (endpoint unreachable / non-2xx) so the caller's + * existing try/catch handles it (the task fails and the orchestrator retries). + * Per-check execution errors come back inside the result as usual. + */ +export async function runChecksOnServer(params: { + apiUrl: string; + connectionId: string; + organizationId: string; + checkId?: string; +}): Promise { + const { apiUrl, connectionId, organizationId, checkId } = params; + + const serviceToken = process.env.SERVICE_TOKEN_TRIGGER; + if (!serviceToken) { + throw new Error('SERVICE_TOKEN_TRIGGER is not configured'); + } + + const response = await fetch( + `${apiUrl}/v1/integrations/internal/run-connection-checks/${connectionId}`, + { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'x-service-token': serviceToken, + 'x-organization-id': organizationId, + }, + body: JSON.stringify(checkId ? { checkId } : {}), + }, + ); + + if (!response.ok) { + const errorData = await response.json().catch(() => ({})); + const message = + (errorData as { message?: string }).message || + `Server-side check run failed with status ${response.status}`; + throw new Error(message); + } + + return (await response.json()) as RunAllChecksResult; +} diff --git a/apps/api/src/trigger/integration-platform/run-connection-checks.ts b/apps/api/src/trigger/integration-platform/run-connection-checks.ts index e7515540ed..f03c221a6d 100644 --- a/apps/api/src/trigger/integration-platform/run-connection-checks.ts +++ b/apps/api/src/trigger/integration-platform/run-connection-checks.ts @@ -7,6 +7,7 @@ import { type IntegrationCredentialValues, } from './ensure-valid-credentials'; import { injectAwsResolvedSession } from './checks-aws-session'; +import { runChecksOnServer } from './run-checks-on-server'; /** * Trigger task that runs all checks for a connection. @@ -180,22 +181,30 @@ export const runConnectionChecks = task({ let totalPassing = 0; try { - // Run all checks - const result = await runAllChecks({ - manifest, - accessToken: getAccessToken(credentials), - credentials, - variables, - connectionId, - organizationId, - onTokenRefresh: - manifest.auth.type === 'oauth2' ? handleTokenRefresh : undefined, - logger: { - info: (msg, data) => logger.info(msg, data), - warn: (msg, data) => logger.warn(msg, data), - error: (msg, data) => logger.error(msg, data), - }, - }); + // AWS checks run ON OUR SERVER so their S3 calls egress our VPC (allowed) + // instead of Trigger.dev's (blocked). Every other provider keeps running + // here in the Trigger.dev runtime, unchanged. Same result shape either + // way, so the persistence below is shared. + const result = + providerSlug === 'aws' + ? await runChecksOnServer({ apiUrl, connectionId, organizationId }) + : await runAllChecks({ + manifest, + accessToken: getAccessToken(credentials), + credentials, + variables, + connectionId, + organizationId, + onTokenRefresh: + manifest.auth.type === 'oauth2' + ? handleTokenRefresh + : undefined, + logger: { + info: (msg, data) => logger.info(msg, data), + warn: (msg, data) => logger.warn(msg, data), + error: (msg, data) => logger.error(msg, data), + }, + }); totalFindings = result.totalFindings; totalPassing = result.totalPassing; diff --git a/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts b/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts index 4d68611326..d479bb56c7 100644 --- a/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts +++ b/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts @@ -11,6 +11,7 @@ import { type IntegrationCredentialValues, } from './ensure-valid-credentials'; import { injectAwsResolvedSession } from './checks-aws-session'; +import { runChecksOnServer } from './run-checks-on-server'; /** * Send email notifications for task status change @@ -339,22 +340,37 @@ export const runTaskIntegrationChecks = task({ // Run only the checks that apply to this task try { for (const checkId of effectiveCheckIds) { - const result = await runAllChecks({ - manifest, - accessToken: getAccessToken(credentials), - credentials, - variables, - connectionId, - organizationId, - checkId, // Run specific check - onTokenRefresh: - manifest.auth.type === 'oauth2' ? handleTokenRefresh : undefined, - logger: { - info: (msg, data) => logger.info(msg, data), - warn: (msg, data) => logger.warn(msg, data), - error: (msg, data) => logger.error(msg, data), - }, - }); + // AWS checks run ON OUR SERVER so their S3 calls egress our VPC (whose + // endpoint allows the read) instead of Trigger.dev's (which blocks it). + // Every other provider keeps executing here in the Trigger.dev runtime, + // unchanged. The result shape is identical either way, so all the + // persistence / status / email logic below is shared. + const result = + providerSlug === 'aws' + ? await runChecksOnServer({ + apiUrl, + connectionId, + organizationId, + checkId, + }) + : await runAllChecks({ + manifest, + accessToken: getAccessToken(credentials), + credentials, + variables, + connectionId, + organizationId, + checkId, // Run specific check + onTokenRefresh: + manifest.auth.type === 'oauth2' + ? handleTokenRefresh + : undefined, + logger: { + info: (msg, data) => logger.info(msg, data), + warn: (msg, data) => logger.warn(msg, data), + error: (msg, data) => logger.error(msg, data), + }, + }); const checkResult = result.results[0]; if (!checkResult) continue; From aa23647c29dd3a6fd6e2e4b963a0e11f5dc1ab32 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 12 Jun 2026 16:44:23 -0400 Subject: [PATCH 2/4] fix(cloud-security): address cubic review on AWS-on-server checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic (and an independent adversarial review) flagged three issues on the AWS server-run path: 1. The AWS branch still ran the Trigger-side credential/session preflight (requestValidCredentials + injectAwsResolvedSession) before delegating to the server. Those calls are unused on the server path (it decrypts creds and assumes the role itself), so a transient preflight failure would falsely fail an AWS run. Skip the preflight entirely for AWS and drop the now-dead injectAwsResolvedSession call (both Trigger tasks). 2. The internal fetch had no timeout, so a hung connection could block the task until maxDuration. Add a generous AbortController timeout (10m — well below the 15m maxDuration but high enough never to abort a legitimately long run) so a stalled socket surfaces as an error and the task retries. 3. (Review) The scheduled per-check loop was inside one outer try/catch; because runChecksOnServer throws on transport failure and several AWS checks share a task, a blip on one check aborted its siblings and skipped lastSyncAt/status. Catch the throw per-check, record that check as failed, and continue — matching runAllChecks' per-check resilience (hasExecutionErrors keeps integrationLastRunAt unwritten so the next tick retries). Non-AWS (GCP/Azure/dynamic/legacy) paths remain unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../run-checks-on-server.spec.ts | 33 ++- .../run-checks-on-server.ts | 63 +++-- .../run-connection-checks.ts | 104 ++++---- .../run-task-integration-checks.ts | 235 ++++++++++-------- 4 files changed, 256 insertions(+), 179 deletions(-) diff --git a/apps/api/src/trigger/integration-platform/run-checks-on-server.spec.ts b/apps/api/src/trigger/integration-platform/run-checks-on-server.spec.ts index 22acf4559e..ed59857003 100644 --- a/apps/api/src/trigger/integration-platform/run-checks-on-server.spec.ts +++ b/apps/api/src/trigger/integration-platform/run-checks-on-server.spec.ts @@ -34,6 +34,9 @@ describe('runChecksOnServer', () => { 'http://api/v1/integrations/internal/run-connection-checks/conn_1', expect.objectContaining({ method: 'POST', + // An abort signal is wired up so a hung connection times out and the + // task can retry instead of blocking until maxDuration. + signal: expect.any(AbortSignal), headers: expect.objectContaining({ 'x-service-token': 'svc-token', 'x-organization-id': 'org_1', @@ -44,13 +47,31 @@ describe('runChecksOnServer', () => { expect(result).toEqual(runResult); }); + it('throws a timeout error when the request is aborted (hung connection)', async () => { + jest.useFakeTimers(); + jest.spyOn(global, 'fetch').mockImplementation( + (_url, opts) => + new Promise((_resolve, reject) => { + (opts as RequestInit).signal?.addEventListener('abort', () => + reject(new Error('aborted')), + ); + }), + ); + + const promise = runChecksOnServer(params); + // Surface the rejection without an unhandled-rejection warning. + const assertion = expect(promise).rejects.toThrow('timed out'); + await jest.advanceTimersByTimeAsync(10 * 60 * 1000); + await assertion; + + jest.useRealTimers(); + }); + it('sends an empty body when no checkId is given (run all)', async () => { - const fetchMock = jest - .spyOn(global, 'fetch') - .mockResolvedValue({ - ok: true, - json: async () => ({}), - } as unknown as Response); + const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValue({ + ok: true, + json: async () => ({}), + } as unknown as Response); await runChecksOnServer(params); diff --git a/apps/api/src/trigger/integration-platform/run-checks-on-server.ts b/apps/api/src/trigger/integration-platform/run-checks-on-server.ts index 695d8cc54d..e0e136992c 100644 --- a/apps/api/src/trigger/integration-platform/run-checks-on-server.ts +++ b/apps/api/src/trigger/integration-platform/run-checks-on-server.ts @@ -2,6 +2,13 @@ import type { runAllChecks } from '@trycompai/integration-platform'; export type RunAllChecksResult = Awaited>; +// Generous backstop for a hung connection (no response). AWS checks legitimately +// take minutes across many buckets/regions, so this is deliberately well below +// the task's 15-minute maxDuration but high enough never to abort a real run — +// it only catches a stalled socket so the error surfaces and the task retries +// instead of blocking the whole 15 minutes. +const REQUEST_TIMEOUT_MS = 10 * 60 * 1000; + /** * Run a connection's checks ON OUR SERVER (ECS) and return the raw result. * @@ -32,26 +39,44 @@ export async function runChecksOnServer(params: { throw new Error('SERVICE_TOKEN_TRIGGER is not configured'); } - const response = await fetch( - `${apiUrl}/v1/integrations/internal/run-connection-checks/${connectionId}`, - { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'x-service-token': serviceToken, - 'x-organization-id': organizationId, - }, - body: JSON.stringify(checkId ? { checkId } : {}), - }, + const abortController = new AbortController(); + const timeoutId = setTimeout( + () => abortController.abort(), + REQUEST_TIMEOUT_MS, ); - if (!response.ok) { - const errorData = await response.json().catch(() => ({})); - const message = - (errorData as { message?: string }).message || - `Server-side check run failed with status ${response.status}`; - throw new Error(message); - } + try { + const response = await fetch( + `${apiUrl}/v1/integrations/internal/run-connection-checks/${connectionId}`, + { + method: 'POST', + signal: abortController.signal, + headers: { + 'Content-Type': 'application/json', + 'x-service-token': serviceToken, + 'x-organization-id': organizationId, + }, + body: JSON.stringify(checkId ? { checkId } : {}), + }, + ); + + if (!response.ok) { + const errorData = await response.json().catch(() => ({})); + const message = + (errorData as { message?: string }).message || + `Server-side check run failed with status ${response.status}`; + throw new Error(message); + } - return (await response.json()) as RunAllChecksResult; + return (await response.json()) as RunAllChecksResult; + } catch (error) { + if (abortController.signal.aborted) { + throw new Error( + `Server-side check run timed out after ${REQUEST_TIMEOUT_MS}ms`, + ); + } + throw error; + } finally { + clearTimeout(timeoutId); + } } diff --git a/apps/api/src/trigger/integration-platform/run-connection-checks.ts b/apps/api/src/trigger/integration-platform/run-connection-checks.ts index f03c221a6d..db590e209e 100644 --- a/apps/api/src/trigger/integration-platform/run-connection-checks.ts +++ b/apps/api/src/trigger/integration-platform/run-connection-checks.ts @@ -6,7 +6,6 @@ import { requestValidCredentials, type IntegrationCredentialValues, } from './ensure-valid-credentials'; -import { injectAwsResolvedSession } from './checks-aws-session'; import { runChecksOnServer } from './run-checks-on-server'; /** @@ -94,72 +93,69 @@ export const runConnectionChecks = task({ }; } - // Ensure we have valid credentials const apiUrl = process.env.BASE_URL || 'http://localhost:3333'; - let credentials: IntegrationCredentialValues; - logger.info('Ensuring valid credentials...'); - const credentialsResult = await requestValidCredentials({ - apiUrl, - connectionId, - organizationId, - }); - - if (!credentialsResult.success || !credentialsResult.credentials) { - const errorMessage = - credentialsResult.error || 'Failed to validate credentials'; - logger.error(errorMessage); - return { success: false, error: errorMessage }; - } - credentials = credentialsResult.credentials; - - const handleTokenRefresh = async (): Promise => { - logger.info('Force refreshing OAuth credentials after provider 401...'); - const refreshResult = await requestValidCredentials({ + // AWS checks run ON OUR SERVER (see below), which decrypts the credentials + // and assumes the cross-account role there. Skip the Trigger-side credential + // preflight for AWS — running it would add redundant failure points (a + // transient preflight error would falsely fail an AWS run that + // `runChecksOnServer` could have completed). + let credentials: IntegrationCredentialValues = {}; + let handleTokenRefresh: (() => Promise) | undefined; + + if (providerSlug !== 'aws') { + logger.info('Ensuring valid credentials...'); + const credentialsResult = await requestValidCredentials({ apiUrl, connectionId, organizationId, - forceRefresh: true, }); - if (!refreshResult.success || !refreshResult.credentials) { - logger.error(refreshResult.error || 'Forced token refresh failed'); - return null; + if (!credentialsResult.success || !credentialsResult.credentials) { + const errorMessage = + credentialsResult.error || 'Failed to validate credentials'; + logger.error(errorMessage); + return { success: false, error: errorMessage }; } + credentials = credentialsResult.credentials; + + handleTokenRefresh = async (): Promise => { + logger.info('Force refreshing OAuth credentials after provider 401...'); + const refreshResult = await requestValidCredentials({ + apiUrl, + connectionId, + organizationId, + forceRefresh: true, + }); + + if (!refreshResult.success || !refreshResult.credentials) { + logger.error(refreshResult.error || 'Forced token refresh failed'); + return null; + } - credentials = refreshResult.credentials; - return getAccessToken(credentials) ?? null; - }; + credentials = refreshResult.credentials; + return getAccessToken(credentials) ?? null; + }; - // Validate credentials based on auth type - if (manifest.auth.type === 'oauth2' && !getAccessToken(credentials)) { - logger.error( - `No OAuth access token found for connection: ${connectionId}`, - ); - return { success: false, error: 'No OAuth access token found' }; - } + // Validate credentials based on auth type + if (manifest.auth.type === 'oauth2' && !getAccessToken(credentials)) { + logger.error( + `No OAuth access token found for connection: ${connectionId}`, + ); + return { success: false, error: 'No OAuth access token found' }; + } - if ( - manifest.auth.type === 'custom' && - Object.keys(credentials).length === 0 - ) { - logger.error( - `No credentials found for custom integration: ${connectionId}`, - ); - return { success: false, error: 'No credentials found' }; + if ( + manifest.auth.type === 'custom' && + Object.keys(credentials).length === 0 + ) { + logger.error( + `No credentials found for custom integration: ${connectionId}`, + ); + return { success: false, error: 'No credentials found' }; + } } - // For AWS, resolve the cross-account session in ECS and inject the temp - // creds — the checks run in the Trigger.dev runtime, which cannot assume the - // role itself (no base creds / roleAssumer ARN there). - credentials = await injectAwsResolvedSession({ - credentials, - apiUrl, - connectionId, - organizationId, - providerSlug, - }); - const variables = (connection.variables as Record< string, diff --git a/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts b/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts index d479bb56c7..6cfdbb2be5 100644 --- a/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts +++ b/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts @@ -10,8 +10,10 @@ import { requestValidCredentials, type IntegrationCredentialValues, } from './ensure-valid-credentials'; -import { injectAwsResolvedSession } from './checks-aws-session'; -import { runChecksOnServer } from './run-checks-on-server'; +import { + runChecksOnServer, + type RunAllChecksResult, +} from './run-checks-on-server'; /** * Send email notifications for task status change @@ -219,91 +221,88 @@ export const runTaskIntegrationChecks = task({ return { success: false, error: 'Connection not found or inactive' }; } - // Ensure we have valid credentials (refresh OAuth tokens if needed) const apiUrl = process.env.BASE_URL || 'http://localhost:3333'; - let credentials: IntegrationCredentialValues; - - logger.info('Ensuring valid credentials (refreshing if needed)...'); - const credentialsResult = await requestValidCredentials({ - apiUrl, - connectionId, - organizationId, - }); - - if (!credentialsResult.success || !credentialsResult.credentials) { - const errorMessage = - credentialsResult.error || 'Failed to validate credentials'; - logger.error(errorMessage); - - // If unauthorized, mark connection as error - if (credentialsResult.status === 401) { - await db.integrationConnection.update({ - where: { id: connectionId }, - data: { - status: 'error', - errorMessage: - 'OAuth token expired. Please reconnect the integration.', - }, - }); - } - - return { success: false, error: errorMessage }; - } - credentials = credentialsResult.credentials; - logger.info('Credentials validated successfully'); - const handleTokenRefresh = async (): Promise => { - logger.info('Force refreshing OAuth credentials after provider 401...'); - const refreshResult = await requestValidCredentials({ + // AWS checks run ON OUR SERVER (see the loop below), which decrypts the + // credentials and assumes the cross-account role there. So the Trigger-side + // credential/session preflight is skipped for AWS — running it would add + // redundant failure points (a transient preflight error would falsely fail + // an AWS run that `runChecksOnServer` could have completed). + let credentials: IntegrationCredentialValues = {}; + let handleTokenRefresh: (() => Promise) | undefined; + + if (providerSlug !== 'aws') { + logger.info('Ensuring valid credentials (refreshing if needed)...'); + const credentialsResult = await requestValidCredentials({ apiUrl, connectionId, organizationId, - forceRefresh: true, }); - if (!refreshResult.success || !refreshResult.credentials) { - logger.error(refreshResult.error || 'Forced token refresh failed'); - return null; + if (!credentialsResult.success || !credentialsResult.credentials) { + const errorMessage = + credentialsResult.error || 'Failed to validate credentials'; + logger.error(errorMessage); + + // If unauthorized, mark connection as error + if (credentialsResult.status === 401) { + await db.integrationConnection.update({ + where: { id: connectionId }, + data: { + status: 'error', + errorMessage: + 'OAuth token expired. Please reconnect the integration.', + }, + }); + } + + return { success: false, error: errorMessage }; } + credentials = credentialsResult.credentials; + logger.info('Credentials validated successfully'); + + handleTokenRefresh = async (): Promise => { + logger.info('Force refreshing OAuth credentials after provider 401...'); + const refreshResult = await requestValidCredentials({ + apiUrl, + connectionId, + organizationId, + forceRefresh: true, + }); - credentials = refreshResult.credentials; - return getAccessToken(credentials) ?? null; - }; + if (!refreshResult.success || !refreshResult.credentials) { + logger.error(refreshResult.error || 'Forced token refresh failed'); + return null; + } - // Validate credentials based on auth type - if (manifest.auth.type === 'oauth2' && !getAccessToken(credentials)) { - logger.error( - `No OAuth access token found for connection: ${connectionId}`, - ); - return { - success: false, - error: 'No OAuth access token found. Please reconnect.', + credentials = refreshResult.credentials; + return getAccessToken(credentials) ?? null; }; - } - if ( - manifest.auth.type === 'custom' && - Object.keys(credentials).length === 0 - ) { - logger.error( - `No credentials found for custom integration: ${connectionId}`, - ); - return { - success: false, - error: 'No credentials found for custom integration', - }; - } + // Validate credentials based on auth type + if (manifest.auth.type === 'oauth2' && !getAccessToken(credentials)) { + logger.error( + `No OAuth access token found for connection: ${connectionId}`, + ); + return { + success: false, + error: 'No OAuth access token found. Please reconnect.', + }; + } - // For AWS, resolve the cross-account session in ECS and inject the temp - // creds — the checks run in the Trigger.dev runtime, which cannot assume the - // role itself (no base creds / roleAssumer ARN there). - credentials = await injectAwsResolvedSession({ - credentials, - apiUrl, - connectionId, - organizationId, - providerSlug, - }); + if ( + manifest.auth.type === 'custom' && + Object.keys(credentials).length === 0 + ) { + logger.error( + `No credentials found for custom integration: ${connectionId}`, + ); + return { + success: false, + error: 'No credentials found for custom integration', + }; + } + } const variables = (connection.variables as Record< @@ -345,32 +344,68 @@ export const runTaskIntegrationChecks = task({ // Every other provider keeps executing here in the Trigger.dev runtime, // unchanged. The result shape is identical either way, so all the // persistence / status / email logic below is shared. - const result = - providerSlug === 'aws' - ? await runChecksOnServer({ - apiUrl, - connectionId, - organizationId, - checkId, - }) - : await runAllChecks({ - manifest, - accessToken: getAccessToken(credentials), - credentials, - variables, - connectionId, - organizationId, - checkId, // Run specific check - onTokenRefresh: - manifest.auth.type === 'oauth2' - ? handleTokenRefresh - : undefined, - logger: { - info: (msg, data) => logger.info(msg, data), - warn: (msg, data) => logger.warn(msg, data), - error: (msg, data) => logger.error(msg, data), - }, - }); + let result: RunAllChecksResult; + try { + result = + providerSlug === 'aws' + ? await runChecksOnServer({ + apiUrl, + connectionId, + organizationId, + checkId, + }) + : await runAllChecks({ + manifest, + accessToken: getAccessToken(credentials), + credentials, + variables, + connectionId, + organizationId, + checkId, // Run specific check + onTokenRefresh: + manifest.auth.type === 'oauth2' + ? handleTokenRefresh + : undefined, + logger: { + info: (msg, data) => logger.info(msg, data), + warn: (msg, data) => logger.warn(msg, data), + error: (msg, data) => logger.error(msg, data), + }, + }); + } catch (error) { + // Only the AWS server-run path can throw here, and only on a transport + // blip (network/non-2xx) — per-check AWS execution errors come back + // inside the result, not thrown. Record THIS check as errored and keep + // going so one blip doesn't abort its sibling checks (multiple AWS + // checks share a task) or skip the lastSyncAt/status updates, mirroring + // runAllChecks' per-check resilience. hasExecutionErrors keeps + // integrationLastRunAt unwritten, so the next orchestrator tick retries. + const message = + error instanceof Error ? error.message : String(error); + const checkDef = manifest.checks?.find((c) => c.id === checkId); + hasFailedChecks = true; + hasExecutionErrors = true; + await db.integrationCheckRun.create({ + data: { + connectionId, + taskId, + checkId, + checkName: checkDef?.name ?? checkId, + status: 'failed', + startedAt: new Date(), + completedAt: new Date(), + durationMs: 0, + totalChecked: 0, + passedCount: 0, + failedCount: 0, + errorMessage: message, + }, + }); + logger.error( + `Server-run failed for check ${checkId} on task ${taskId}: ${message}`, + ); + continue; + } const checkResult = result.results[0]; if (!checkResult) continue; From 577bc057f0c564a9193188c523a8951f50b2bbf5 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 12 Jun 2026 16:50:11 -0400 Subject: [PATCH 3/4] fix(cloud-security): re-throw non-AWS errors in the per-check catch The per-check catch added for AWS server-run resilience wrapped both branches, so a (rare) runAllChecks throw on a non-AWS provider would be downgraded to a per-check failure instead of propagating. Re-throw when providerSlug !== 'aws' so non-AWS behavior is unchanged from before this PR; only AWS transport blips get the degrade-and-continue treatment. (cubic review) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../run-task-integration-checks.ts | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts b/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts index 6cfdbb2be5..ad7c5d0722 100644 --- a/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts +++ b/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts @@ -373,13 +373,21 @@ export const runTaskIntegrationChecks = task({ }, }); } catch (error) { - // Only the AWS server-run path can throw here, and only on a transport - // blip (network/non-2xx) — per-check AWS execution errors come back - // inside the result, not thrown. Record THIS check as errored and keep - // going so one blip doesn't abort its sibling checks (multiple AWS - // checks share a task) or skip the lastSyncAt/status updates, mirroring - // runAllChecks' per-check resilience. hasExecutionErrors keeps - // integrationLastRunAt unwritten, so the next orchestrator tick retries. + // Only the AWS server-run path is degraded here. Non-AWS providers run + // in-process via runAllChecks, which catches per-check failures and + // returns status:'error' rather than throwing — so a throw on the + // non-AWS branch is unexpected and must NOT be silently downgraded. + // Re-throw it to preserve the pre-change behavior (it propagates to the + // outer catch and fails the task). + if (providerSlug !== 'aws') throw error; + + // AWS server-run threw, and only on a transport blip (network/non-2xx) + // — per-check AWS execution errors come back inside the result, not + // thrown. Record THIS check as errored and keep going so one blip + // doesn't abort its sibling checks (multiple AWS checks share a task) + // or skip the lastSyncAt/status updates, mirroring runAllChecks' + // per-check resilience. hasExecutionErrors keeps integrationLastRunAt + // unwritten, so the next orchestrator tick retries. const message = error instanceof Error ? error.message : String(error); const checkDef = manifest.checks?.find((c) => c.id === checkId); From 91ca27b8ac6ad4c14ec469c7a4a35d6eb43758f6 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 12 Jun 2026 17:26:32 -0400 Subject: [PATCH 4/4] fix(cloud-security): exempt internal check-run endpoint from throttle + validate creds by auth type cubic Ultrareview findings: 1. The internal run-connection-checks endpoint went through the global ThrottlerGuard, so the 6 AM AWS fan-out could hit 429s and re-fail checks. Add @SkipThrottle() (matching the Trigger-called resolve-session endpoint). 2. ConnectionCheckRunnerService only checked for missing credentials; the in-app run paths also validate by auth type. Align it (oauth2 / api_key / basic / custom) so a server-run rejects malformed credentials up front with a clear error instead of executing the check on bad input. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../controllers/internal-checks.controller.ts | 5 +++ .../connection-check-runner.service.spec.ts | 9 +++++ .../connection-check-runner.service.ts | 35 +++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/apps/api/src/integration-platform/controllers/internal-checks.controller.ts b/apps/api/src/integration-platform/controllers/internal-checks.controller.ts index 8da42fa375..c2dc399a43 100644 --- a/apps/api/src/integration-platform/controllers/internal-checks.controller.ts +++ b/apps/api/src/integration-platform/controllers/internal-checks.controller.ts @@ -5,6 +5,7 @@ import { ApiPropertyOptional, ApiTags, } from '@nestjs/swagger'; +import { SkipThrottle } from '@nestjs/throttler'; import { IsOptional, IsString } from 'class-validator'; import { HybridAuthGuard } from '../../auth/hybrid-auth.guard'; import { PermissionGuard } from '../../auth/permission.guard'; @@ -40,6 +41,10 @@ export class InternalChecksController { constructor(private readonly runner: ConnectionCheckRunnerService) {} @Post('run-connection-checks/:connectionId') + // Called by the AWS Trigger tasks in bursts (the 6 AM schedule fans out across + // every AWS connection/check). Exempt from the global rate limiter so the burst + // doesn't hit 429s and re-fail the very checks this path exists to fix. + @SkipThrottle() @UseGuards(HybridAuthGuard, ServiceTokenOnlyGuard, PermissionGuard) @RequirePermission('integration', 'update') @ApiOperation({ diff --git a/apps/api/src/integration-platform/services/connection-check-runner.service.spec.ts b/apps/api/src/integration-platform/services/connection-check-runner.service.spec.ts index 1dc356288c..40749cc5ab 100644 --- a/apps/api/src/integration-platform/services/connection-check-runner.service.spec.ts +++ b/apps/api/src/integration-platform/services/connection-check-runner.service.spec.ts @@ -142,4 +142,13 @@ describe('ConnectionCheckRunnerService', () => { ).rejects.toBeInstanceOf(BadRequestException); expect(mockedRunAllChecks).not.toHaveBeenCalled(); }); + + it('validates by auth type — rejects empty custom credentials (matches in-app run)', async () => { + // AWS uses custom auth; empty creds must be rejected up front, not executed. + mockCredentialVaultService.getDecryptedCredentials.mockResolvedValue({}); + await expect( + service.runChecks({ connectionId: 'conn_1', organizationId: 'org_1' }), + ).rejects.toBeInstanceOf(BadRequestException); + expect(mockedRunAllChecks).not.toHaveBeenCalled(); + }); }); diff --git a/apps/api/src/integration-platform/services/connection-check-runner.service.ts b/apps/api/src/integration-platform/services/connection-check-runner.service.ts index 1e1b8f22f0..123cb4f7ab 100644 --- a/apps/api/src/integration-platform/services/connection-check-runner.service.ts +++ b/apps/api/src/integration-platform/services/connection-check-runner.service.ts @@ -82,6 +82,41 @@ export class ConnectionCheckRunnerService { throw new BadRequestException('No credentials found for connection'); } + // Validate credentials by auth type, matching the in-app run paths + // (checks.controller / task-integrations.controller) so a server-run rejects + // malformed credentials up front with a clear error instead of executing the + // check on bad input and producing an inconsistent outcome. + if (manifest.auth.type === 'oauth2' && !credentials.access_token) { + throw new BadRequestException( + 'No valid OAuth credentials found. Please reconnect.', + ); + } + if (manifest.auth.type === 'api_key') { + const apiKeyField = manifest.auth.config.name; + if (!credentials[apiKeyField] && !credentials.api_key) { + throw new BadRequestException( + 'API key not found. Please reconnect the integration.', + ); + } + } + if (manifest.auth.type === 'basic') { + const usernameField = manifest.auth.config.usernameField || 'username'; + const passwordField = manifest.auth.config.passwordField || 'password'; + if (!credentials[usernameField] || !credentials[passwordField]) { + throw new BadRequestException( + 'Username and password required. Please reconnect the integration.', + ); + } + } + if ( + manifest.auth.type === 'custom' && + Object.keys(credentials).length === 0 + ) { + throw new BadRequestException( + 'No valid credentials found for custom integration', + ); + } + const variables = (connection.variables as Record< string,