diff --git a/apps/api/src/security-penetration-tests/dto/create-penetration-test.dto.ts b/apps/api/src/security-penetration-tests/dto/create-penetration-test.dto.ts index c85be302ac..ff21278567 100644 --- a/apps/api/src/security-penetration-tests/dto/create-penetration-test.dto.ts +++ b/apps/api/src/security-penetration-tests/dto/create-penetration-test.dto.ts @@ -4,7 +4,9 @@ import { IsBoolean, IsEnum, IsOptional, + IsString, IsUrl, + MaxLength, } from 'class-validator'; import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; @@ -107,4 +109,15 @@ export class CreatePenetrationTestDto { @ArrayUnique() @IsEnum(pentestCheckValues, { each: true }) checks?: PentestCheck[]; + + @ApiPropertyOptional({ + description: + 'Free-text context shared with the testing agent, e.g. remediation notes or accepted-by-design explanations from a previous run. Saved per-finding context notes for the same target are appended automatically. Max 4000 characters.', + required: false, + maxLength: 4000, + }) + @IsOptional() + @IsString() + @MaxLength(4000) + additionalContext?: string; } diff --git a/apps/api/src/security-penetration-tests/dto/upsert-finding-context.dto.ts b/apps/api/src/security-penetration-tests/dto/upsert-finding-context.dto.ts new file mode 100644 index 0000000000..c6c4571e21 --- /dev/null +++ b/apps/api/src/security-penetration-tests/dto/upsert-finding-context.dto.ts @@ -0,0 +1,24 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { IsNotEmpty, IsString, MaxLength } from 'class-validator'; + +export class UpsertFindingContextDto { + @ApiProperty({ + description: 'Penetration test run ID the finding belongs to', + example: 'pentest-abc123', + }) + @IsString() + @IsNotEmpty() + runId!: string; + + @ApiProperty({ + description: + 'Context for the finding, e.g. an accepted-by-design rationale or remediation details. Shared with the testing agent on future scans of the same target. Max 2000 characters.', + example: + 'Read access to appConfiguration is accepted by design: the collection only holds non-secret bootstrap configuration and write access is restricted to privileged users.', + maxLength: 2000, + }) + @IsString() + @IsNotEmpty() + @MaxLength(2000) + context!: string; +} diff --git a/apps/api/src/security-penetration-tests/finding-context.util.spec.ts b/apps/api/src/security-penetration-tests/finding-context.util.spec.ts new file mode 100644 index 0000000000..e0dc8156fc --- /dev/null +++ b/apps/api/src/security-penetration-tests/finding-context.util.spec.ts @@ -0,0 +1,130 @@ +import { + buildAdditionalContext, + MAX_ADDITIONAL_CONTEXT_LENGTH, + normalizeTargetUrl, +} from './finding-context.util'; + +describe('normalizeTargetUrl', () => { + it('lowercases the host and strips trailing slashes', () => { + expect(normalizeTargetUrl('https://App.Example.com/')).toBe( + 'https://app.example.com', + ); + }); + + it('strips trailing slashes from paths but keeps the path itself', () => { + expect(normalizeTargetUrl('https://app.example.com/portal/')).toBe( + 'https://app.example.com/portal', + ); + }); + + it('drops URL fragments', () => { + expect(normalizeTargetUrl('https://app.example.com/#section')).toBe( + 'https://app.example.com', + ); + }); + + it('keeps query strings', () => { + expect(normalizeTargetUrl('https://app.example.com/?env=staging')).toBe( + 'https://app.example.com/?env=staging', + ); + }); + + it('preserves a trailing slash that belongs to a query value', () => { + expect( + normalizeTargetUrl('https://app.example.com/?next=/portal/'), + ).toBe('https://app.example.com/?next=/portal/'); + }); + + it('strips path trailing slashes while keeping the query intact', () => { + expect(normalizeTargetUrl('https://app.example.com/app/?v=2')).toBe( + 'https://app.example.com/app?v=2', + ); + }); + + it('returns non-URL input trimmed', () => { + expect(normalizeTargetUrl(' not a url ')).toBe('not a url'); + }); +}); + +describe('buildAdditionalContext', () => { + it('returns undefined when there is nothing to send', () => { + expect( + buildAdditionalContext({ findingContexts: [] }), + ).toBeUndefined(); + expect( + buildAdditionalContext({ + userProvidedContext: ' ', + findingContexts: [], + }), + ).toBeUndefined(); + }); + + it('returns only the user-provided context when there are no notes', () => { + expect( + buildAdditionalContext({ + userProvidedContext: ' Focus on auth flows. ', + findingContexts: [], + }), + ).toBe('Focus on auth flows.'); + }); + + it('formats stored notes as a numbered list with titles', () => { + const result = buildAdditionalContext({ + findingContexts: [ + { issueTitle: 'appConfiguration read access', context: 'By design.' }, + { issueTitle: 'Storage attachment access', context: 'Hardened.' }, + ], + }); + + expect(result).toContain('1. "appConfiguration read access": By design.'); + expect(result).toContain('2. "Storage attachment access": Hardened.'); + expect(result).toContain('Customer-provided context'); + }); + + it('places the user context before the stored notes', () => { + const result = buildAdditionalContext({ + userProvidedContext: 'Retest of the May findings.', + findingContexts: [{ issueTitle: 'Issue A', context: 'Fixed.' }], + }); + + expect(result).toBeDefined(); + const userIndex = (result as string).indexOf('Retest of the May findings.'); + const notesIndex = (result as string).indexOf('1. "Issue A": Fixed.'); + expect(userIndex).toBeGreaterThanOrEqual(0); + expect(notesIndex).toBeGreaterThan(userIndex); + }); + + it('does not add an omission marker when everything fits', () => { + const result = buildAdditionalContext({ + userProvidedContext: 'User intent.', + findingContexts: [{ issueTitle: 'Issue A', context: 'Fixed.' }], + }); + + expect(result).not.toContain('omitted for length'); + }); + + it('caps the composed briefing, keeping user context and marking omitted notes', () => { + const findingContexts = Array.from({ length: 30 }, (_, i) => ({ + issueTitle: `Finding ${i + 1}`, + context: 'x'.repeat(1900), + })); + + const result = buildAdditionalContext({ + userProvidedContext: 'User intent.', + findingContexts, + }); + + expect(result).toBeDefined(); + expect((result as string).length).toBeLessThanOrEqual( + MAX_ADDITIONAL_CONTEXT_LENGTH, + ); + expect(result).toContain('User intent.'); + expect(result).toContain('1. "Finding 1"'); + expect(result).toMatch(/\d+ more notes omitted for length/); + // Whole notes are dropped, never cut: every included note line ends + // with its full 1900-char body. + const includedBodies = (result as string).match(/x{1900}/g) ?? []; + expect(includedBodies.length).toBeGreaterThan(0); + expect(result).not.toMatch(/x{1901,}/); + }); +}); diff --git a/apps/api/src/security-penetration-tests/finding-context.util.ts b/apps/api/src/security-penetration-tests/finding-context.util.ts new file mode 100644 index 0000000000..c8c74b484c --- /dev/null +++ b/apps/api/src/security-penetration-tests/finding-context.util.ts @@ -0,0 +1,98 @@ +/** + * Pure helpers for pentest finding-context notes. Kept free of Nest/DB + * imports so both the storage service and the run-creation path can use + * them, and so they unit-test without mocks. + */ + +export interface FindingContextNote { + issueTitle: string; + context: string; +} + +/** + * Canonical form of a scan target so notes written on a run match future + * runs of the same target regardless of casing or a trailing slash + * (`https://App.example.com/` ≡ `https://app.example.com`). Only the + * path's trailing slashes are stripped — a `/` at the end of a query + * value belongs to that value and must survive. Non-URL input is + * returned trimmed — the create DTO already enforces a valid URL. + */ +export function normalizeTargetUrl(value: string): string { + const trimmed = value.trim(); + try { + const url = new URL(trimmed); + url.hash = ''; + while (url.pathname.length > 1 && url.pathname.endsWith('/')) { + url.pathname = url.pathname.slice(0, -1); + } + const normalized = url.toString(); + // URL always renders the bare root path with a trailing slash + // (`https://x.com/`); drop it for origin-only targets so keys stay + // in the `https://x.com` form. + return url.pathname === '/' && !url.search + ? normalized.replace(/\/$/, '') + : normalized; + } catch { + return trimmed; + } +} + +/** + * Budget for the composed briefing. The provider has no documented limit + * (verified against its OpenAPI spec and prompt-injection source), but an + * unbounded string composed from arbitrarily many notes shouldn't be sent + * to an external API or an agent prompt. When over budget, whole notes + * are dropped — never cut mid-sentence — and an explicit omission marker + * tells the agent the list is incomplete. + */ +export const MAX_ADDITIONAL_CONTEXT_LENGTH = 20_000; + +const NOTES_HEADER = + 'Customer-provided context on findings reported in previous scans of this target. ' + + 'Take it into account when validating and reporting findings (e.g. behavior that is ' + + 'accepted by design, or issues the customer has since remediated):'; + +/** + * Composes the `additionalContext` string sent to the pentest provider on + * run creation: the user's free-text context for this run (if any) followed + * by the stored per-finding notes from previous scans of the same target. + * Returns undefined when there is nothing to send. + */ +export function buildAdditionalContext(params: { + userProvidedContext?: string; + findingContexts: FindingContextNote[]; +}): string | undefined { + const userSection = params.userProvidedContext?.trim(); + + if (params.findingContexts.length === 0) { + return userSection || undefined; + } + + const noteLines = params.findingContexts.map( + (note, index) => + `${index + 1}. "${note.issueTitle.trim()}": ${note.context.trim()}`, + ); + + const compose = (includedCount: number): string => { + const omitted = noteLines.length - includedCount; + const lines = noteLines.slice(0, includedCount); + if (omitted > 0) { + lines.push( + `(${omitted} more note${omitted === 1 ? '' : 's'} omitted for length — see the finding context notes in Comp AI)`, + ); + } + const sections = userSection ? [userSection] : []; + sections.push([NOTES_HEADER, ...lines].join('\n')); + return sections.join('\n\n'); + }; + + let included = noteLines.length; + while ( + included > 0 && + compose(included).length > MAX_ADDITIONAL_CONTEXT_LENGTH + ) { + included -= 1; + } + + return compose(included); +} diff --git a/apps/api/src/security-penetration-tests/pentest-finding-contexts.controller.spec.ts b/apps/api/src/security-penetration-tests/pentest-finding-contexts.controller.spec.ts new file mode 100644 index 0000000000..d9c3664b75 --- /dev/null +++ b/apps/api/src/security-penetration-tests/pentest-finding-contexts.controller.spec.ts @@ -0,0 +1,109 @@ +// The controller's runtime import chain reaches `@db`, which instantiates +// a Prisma client at module load. These tests never touch the database. +jest.mock('@db', () => ({ db: {} })); + +jest.mock('../auth/hybrid-auth.guard', () => ({ + HybridAuthGuard: class { + canActivate() { + return true; + } + }, +})); + +// The real PermissionGuard transitively imports better-auth's ESM bundle, +// which Jest cannot parse. The mock must keep PERMISSIONS_KEY's real value +// ('required_permissions') because @RequirePermission writes its metadata +// under it at class-definition time. +jest.mock('../auth/permission.guard', () => ({ + PermissionGuard: class { + canActivate() { + return true; + } + }, + PERMISSIONS_KEY: 'required_permissions', +})); + +import { PERMISSIONS_KEY } from '../auth/permission.guard'; +import type { RequiredPermission } from '../auth/permission.guard'; +import { PentestFindingContextsController } from './pentest-finding-contexts.controller'; +import type { PentestFindingContextsService } from './pentest-finding-contexts.service'; + +describe('PentestFindingContextsController', () => { + const listForTargetMock = jest.fn(); + const upsertMock = jest.fn(); + const removeMock = jest.fn(); + + const serviceMock = { + listForTarget: listForTargetMock, + upsert: upsertMock, + remove: removeMock, + } as unknown as jest.Mocked; + + const controller = new PentestFindingContextsController(serviceMock); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('lists context notes for a target', async () => { + const expected = [{ id: 'ptfc_1', providerIssueId: 'issue_1' }]; + listForTargetMock.mockResolvedValueOnce(expected); + + const response = await controller.list( + 'org_123', + 'https://app.example.com', + ); + + expect(listForTargetMock).toHaveBeenCalledWith( + 'org_123', + 'https://app.example.com', + ); + expect(response).toEqual(expected); + }); + + it('upserts a context note for a finding', async () => { + const expected = { id: 'ptfc_1', providerIssueId: 'issue_1' }; + upsertMock.mockResolvedValueOnce(expected); + + const response = await controller.upsert('org_123', 'issue_1', { + runId: 'run_123', + context: 'Accepted by design.', + }); + + expect(upsertMock).toHaveBeenCalledWith({ + organizationId: 'org_123', + issueId: 'issue_1', + runId: 'run_123', + context: 'Accepted by design.', + }); + expect(response).toEqual(expected); + }); + + it('removes a context note for a finding', async () => { + removeMock.mockResolvedValueOnce({ deleted: true }); + + const response = await controller.remove('org_123', 'issue_1'); + + expect(removeMock).toHaveBeenCalledWith({ + organizationId: 'org_123', + issueId: 'issue_1', + }); + expect(response).toEqual({ deleted: true }); + }); + + it('requires pentest permissions on every endpoint', () => { + const requirements = (['list', 'upsert', 'remove'] as const).map( + (method) => + Reflect.getMetadata( + PERMISSIONS_KEY, + PentestFindingContextsController.prototype[method], + ) as RequiredPermission[] | undefined, + ); + + expect(requirements).toEqual([ + [{ resource: 'pentest', actions: ['read'] }], + [{ resource: 'pentest', actions: ['update'] }], + [{ resource: 'pentest', actions: ['update'] }], + ]); + }); +}); diff --git a/apps/api/src/security-penetration-tests/pentest-finding-contexts.controller.ts b/apps/api/src/security-penetration-tests/pentest-finding-contexts.controller.ts new file mode 100644 index 0000000000..da9395caf4 --- /dev/null +++ b/apps/api/src/security-penetration-tests/pentest-finding-contexts.controller.ts @@ -0,0 +1,102 @@ +import { + Body, + Controller, + Delete, + Get, + Param, + Put, + Query, + UseGuards, +} from '@nestjs/common'; +import { + ApiBody, + ApiExtension, + ApiHeader, + ApiOperation, + ApiQuery, + ApiResponse, + ApiSecurity, + ApiTags, +} from '@nestjs/swagger'; +import { OrganizationId } from '../auth/auth-context.decorator'; +import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; +import { PermissionGuard } from '../auth/permission.guard'; +import { RequirePermission } from '../auth/require-permission.decorator'; +import { UpsertFindingContextDto } from './dto/upsert-finding-context.dto'; +import { PentestFindingContextsService } from './pentest-finding-contexts.service'; + +@ApiTags('Security Penetration Tests') +@Controller({ path: 'pentest-finding-contexts', version: '1' }) +@ApiSecurity('apikey') +@ApiHeader({ + name: 'X-Organization-Id', + description: + 'Organization ID (required for session auth, optional for API key auth)', + required: false, +}) +@UseGuards(HybridAuthGuard, PermissionGuard) +export class PentestFindingContextsController { + constructor(private readonly service: PentestFindingContextsService) {} + + @Get() + @RequirePermission('pentest', 'read') + @ApiOperation({ + summary: 'List pentest finding context notes', + description: + 'Returns the customer-written context notes attached to pentest findings for a target URL. These notes are shared with the testing agent on future scans of the same target so retests are informed, not blind.', + }) + @ApiQuery({ + name: 'targetUrl', + required: true, + description: 'Target URL the notes are attached to', + }) + @ApiResponse({ status: 200, description: 'Context notes returned' }) + @ApiExtension('x-speakeasy-mcp', { name: 'list-pentest-finding-contexts' }) + async list( + @OrganizationId() organizationId: string, + @Query('targetUrl') targetUrl?: string, + ) { + return this.service.listForTarget(organizationId, targetUrl); + } + + @Put(':issueId') + @RequirePermission('pentest', 'update') + @ApiOperation({ + summary: 'Add context to a pentest finding', + description: + 'Saves a customer context note on a pentest finding, e.g. an accepted-by-design rationale or remediation details. Future scans of the same target pass the note to the testing agent so the issue is retested with that context.', + }) + @ApiBody({ type: UpsertFindingContextDto }) + @ApiResponse({ status: 200, description: 'Context note saved' }) + @ApiResponse({ status: 404, description: 'Run or finding not found' }) + @ApiExtension('x-speakeasy-mcp', { name: 'set-pentest-finding-context' }) + async upsert( + @OrganizationId() organizationId: string, + @Param('issueId') issueId: string, + @Body() body: UpsertFindingContextDto, + ) { + return this.service.upsert({ + organizationId, + issueId, + runId: body.runId, + context: body.context, + }); + } + + @Delete(':issueId') + @RequirePermission('pentest', 'update') + @ApiOperation({ + summary: 'Remove context from a pentest finding', + description: + 'Deletes the customer context note attached to a pentest finding so future scans of the target no longer receive it from the testing agent briefing.', + }) + @ApiResponse({ status: 200, description: 'Context note deleted' }) + @ApiResponse({ status: 404, description: 'No context found for finding' }) + @ApiExtension('x-speakeasy-mcp', { name: 'delete-pentest-finding-context' }) + async remove( + @OrganizationId() organizationId: string, + @Param('issueId') issueId: string, + ) { + return this.service.remove({ organizationId, issueId }); + } +} diff --git a/apps/api/src/security-penetration-tests/pentest-finding-contexts.service.spec.ts b/apps/api/src/security-penetration-tests/pentest-finding-contexts.service.spec.ts new file mode 100644 index 0000000000..dfb4037de6 --- /dev/null +++ b/apps/api/src/security-penetration-tests/pentest-finding-contexts.service.spec.ts @@ -0,0 +1,239 @@ +import { + BadRequestException, + HttpException, + HttpStatus, + NotFoundException, +} from '@nestjs/common'; +import { db } from '@db'; +import type { Issue } from '@maced/api-client'; +import { PentestFindingContextsService } from './pentest-finding-contexts.service'; +import type { SecurityPenetrationTestsService } from './security-penetration-tests.service'; + +jest.mock('@db', () => ({ + db: { + securityPenetrationTestFindingContext: { + findMany: jest.fn(), + upsert: jest.fn(), + deleteMany: jest.fn(), + }, + }, +})); + +type MockDb = { + securityPenetrationTestFindingContext: { + findMany: jest.Mock; + upsert: jest.Mock; + deleteMany: jest.Mock; + }; +}; + +const mockPenetrationTests: jest.Mocked< + Pick +> = { + getReport: jest.fn(), + getReportIssues: jest.fn(), +}; + +describe('PentestFindingContextsService', () => { + const mockedDb = db as unknown as MockDb; + let service: PentestFindingContextsService; + + beforeEach(() => { + jest.clearAllMocks(); + service = new PentestFindingContextsService( + mockPenetrationTests as unknown as SecurityPenetrationTestsService, + ); + mockPenetrationTests.getReport.mockResolvedValue({ + id: 'run_123', + targetUrl: 'https://App.example.com/', + status: 'completed', + createdAt: '2026-06-01T00:00:00.000Z', + updatedAt: '2026-06-01T00:00:00.000Z', + }); + const issue: Issue = { + id: 'issue_1', + runId: 'run_123', + findingId: null, + title: 'appConfiguration read access', + summary: null, + severity: 'medium', + status: 'open', + cve: null, + cweId: null, + cvssScore: null, + affectedEndpoint: null, + proofOfConcept: null, + impact: null, + remediation: null, + validationSteps: null, + evidence: null, + attackPath: null, + createdAt: '2026-06-01T00:00:00.000Z', + updatedAt: '2026-06-01T00:00:00.000Z', + }; + mockPenetrationTests.getReportIssues.mockResolvedValue([issue]); + }); + + describe('listForTarget', () => { + it('rejects a missing targetUrl', async () => { + await expect(service.listForTarget('org_123', undefined)).rejects.toThrow( + BadRequestException, + ); + await expect(service.listForTarget('org_123', ' ')).rejects.toThrow( + BadRequestException, + ); + expect( + mockedDb.securityPenetrationTestFindingContext.findMany, + ).not.toHaveBeenCalled(); + }); + + it('queries by org and normalized target URL', async () => { + mockedDb.securityPenetrationTestFindingContext.findMany.mockResolvedValue( + [], + ); + + await service.listForTarget('org_123', 'https://App.example.com/'); + + expect( + mockedDb.securityPenetrationTestFindingContext.findMany, + ).toHaveBeenCalledWith( + expect.objectContaining({ + where: { + organizationId: 'org_123', + targetUrl: 'https://app.example.com', + }, + }), + ); + }); + }); + + describe('upsert', () => { + const params = { + organizationId: 'org_123', + issueId: 'issue_1', + runId: 'run_123', + context: ' Accepted by design. ', + }; + + it('rejects whitespace-only context before touching the provider', async () => { + await expect( + service.upsert({ ...params, context: ' ' }), + ).rejects.toThrow(BadRequestException); + expect(mockPenetrationTests.getReport).not.toHaveBeenCalled(); + }); + + it('propagates ownership errors from getReport (cross-org run)', async () => { + mockPenetrationTests.getReport.mockRejectedValue( + new HttpException({ error: 'Report not found' }, HttpStatus.NOT_FOUND), + ); + + await expect(service.upsert(params)).rejects.toMatchObject({ + status: HttpStatus.NOT_FOUND, + }); + expect( + mockedDb.securityPenetrationTestFindingContext.upsert, + ).not.toHaveBeenCalled(); + }); + + it('rejects when the run has no target URL', async () => { + mockPenetrationTests.getReport.mockResolvedValue({ + id: 'run_123', + targetUrl: '', + status: 'completed', + createdAt: '', + updatedAt: '', + }); + + await expect(service.upsert(params)).rejects.toMatchObject({ + status: HttpStatus.BAD_GATEWAY, + }); + }); + + it('rejects a whitespace-only target URL instead of storing an empty target', async () => { + mockPenetrationTests.getReport.mockResolvedValue({ + id: 'run_123', + targetUrl: ' ', + status: 'completed', + createdAt: '', + updatedAt: '', + }); + + await expect(service.upsert(params)).rejects.toMatchObject({ + status: HttpStatus.BAD_GATEWAY, + }); + expect( + mockedDb.securityPenetrationTestFindingContext.upsert, + ).not.toHaveBeenCalled(); + }); + + it('rejects when the issue is not part of the run', async () => { + await expect( + service.upsert({ ...params, issueId: 'issue_unknown' }), + ).rejects.toThrow(NotFoundException); + expect( + mockedDb.securityPenetrationTestFindingContext.upsert, + ).not.toHaveBeenCalled(); + }); + + it('upserts the trimmed note keyed by org + issue with a normalized target and title snapshot', async () => { + mockedDb.securityPenetrationTestFindingContext.upsert.mockResolvedValue({ + id: 'ptfc_1', + }); + + await service.upsert(params); + + const expectedShared = { + providerRunId: 'run_123', + targetUrl: 'https://app.example.com', + issueTitle: 'appConfiguration read access', + context: 'Accepted by design.', + }; + expect( + mockedDb.securityPenetrationTestFindingContext.upsert, + ).toHaveBeenCalledWith({ + where: { + organizationId_providerIssueId: { + organizationId: 'org_123', + providerIssueId: 'issue_1', + }, + }, + create: { + organizationId: 'org_123', + providerIssueId: 'issue_1', + ...expectedShared, + }, + update: expectedShared, + }); + }); + }); + + describe('remove', () => { + it('throws NotFound when no note exists', async () => { + mockedDb.securityPenetrationTestFindingContext.deleteMany.mockResolvedValue( + { count: 0 }, + ); + + await expect( + service.remove({ organizationId: 'org_123', issueId: 'issue_1' }), + ).rejects.toThrow(NotFoundException); + }); + + it('deletes the note scoped to the organization', async () => { + mockedDb.securityPenetrationTestFindingContext.deleteMany.mockResolvedValue( + { count: 1 }, + ); + + const result = await service.remove({ + organizationId: 'org_123', + issueId: 'issue_1', + }); + + expect( + mockedDb.securityPenetrationTestFindingContext.deleteMany, + ).toHaveBeenCalledWith({ + where: { organizationId: 'org_123', providerIssueId: 'issue_1' }, + }); + expect(result).toEqual({ deleted: true }); + }); + }); +}); diff --git a/apps/api/src/security-penetration-tests/pentest-finding-contexts.service.ts b/apps/api/src/security-penetration-tests/pentest-finding-contexts.service.ts new file mode 100644 index 0000000000..8a73fee140 --- /dev/null +++ b/apps/api/src/security-penetration-tests/pentest-finding-contexts.service.ts @@ -0,0 +1,122 @@ +import { + BadRequestException, + HttpException, + HttpStatus, + Injectable, + NotFoundException, +} from '@nestjs/common'; +import { db } from '@db'; +import type { SecurityPenetrationTestFindingContext } from '@db'; +import { normalizeTargetUrl } from './finding-context.util'; +import { SecurityPenetrationTestsService } from './security-penetration-tests.service'; + +/** + * Storage + validation for customer context notes on pentest findings. + * + * Notes are keyed by the provider's issue id and grouped by normalized + * target URL. `SecurityPenetrationTestsService.createReport` aggregates + * the notes for a target into the provider's `additionalContext` field so + * retests run informed instead of blind. + */ +@Injectable() +export class PentestFindingContextsService { + constructor( + private readonly penetrationTests: SecurityPenetrationTestsService, + ) {} + + async listForTarget( + organizationId: string, + targetUrl: string | undefined, + ): Promise { + if (!targetUrl?.trim()) { + throw new BadRequestException( + 'targetUrl query parameter is required', + ); + } + + return db.securityPenetrationTestFindingContext.findMany({ + where: { organizationId, targetUrl: normalizeTargetUrl(targetUrl) }, + orderBy: { createdAt: 'asc' }, + }); + } + + async upsert(params: { + organizationId: string; + issueId: string; + runId: string; + context: string; + }): Promise { + const context = params.context.trim(); + if (!context) { + throw new BadRequestException('context must not be empty'); + } + + // getReport asserts run ownership (404 on other orgs' runs) and gives + // us the authoritative target URL — never trust a client-supplied one. + const run = await this.penetrationTests.getReport( + params.organizationId, + params.runId, + ); + // Validate the normalized form: a whitespace-only provider URL would + // otherwise pass the truthiness check and persist an empty target that + // no future scan lookup could ever match. + const targetUrl = normalizeTargetUrl(run.targetUrl); + if (!targetUrl) { + throw new HttpException( + { error: 'Run target URL unavailable from provider' }, + HttpStatus.BAD_GATEWAY, + ); + } + + const issues = await this.penetrationTests.getReportIssues( + params.organizationId, + params.runId, + ); + const issue = issues.find(({ id }) => id === params.issueId); + if (!issue) { + throw new NotFoundException( + 'Finding not found in this penetration test run', + ); + } + + const shared = { + providerRunId: params.runId, + targetUrl, + issueTitle: issue.title, + context, + }; + + return db.securityPenetrationTestFindingContext.upsert({ + where: { + organizationId_providerIssueId: { + organizationId: params.organizationId, + providerIssueId: params.issueId, + }, + }, + create: { + organizationId: params.organizationId, + providerIssueId: params.issueId, + ...shared, + }, + update: shared, + }); + } + + async remove(params: { + organizationId: string; + issueId: string; + }): Promise<{ deleted: true }> { + const result = await db.securityPenetrationTestFindingContext.deleteMany({ + where: { + organizationId: params.organizationId, + providerIssueId: params.issueId, + }, + }); + + if (result.count === 0) { + throw new NotFoundException('No context found for this finding'); + } + + return { deleted: true }; + } +} diff --git a/apps/api/src/security-penetration-tests/report-appendix.util.spec.ts b/apps/api/src/security-penetration-tests/report-appendix.util.spec.ts new file mode 100644 index 0000000000..6e6c101a2b --- /dev/null +++ b/apps/api/src/security-penetration-tests/report-appendix.util.spec.ts @@ -0,0 +1,142 @@ +import { PDFDocument, StandardFonts } from 'pdf-lib'; +import { + appendContextNotesToMarkdown, + appendContextNotesToPdf, + wrapText, + type ReportContextNote, +} from './report-appendix.util'; + +const notes: ReportContextNote[] = [ + { + issueTitle: 'appConfiguration read access', + context: 'Accepted by design — non-secret bootstrap config.', + updatedAt: new Date('2026-06-11T10:00:00.000Z'), + }, + { + issueTitle: 'Unverified email access', + context: 'Email verification is now enabled.', + updatedAt: new Date('2026-06-10T10:00:00.000Z'), + }, +]; + +describe('appendContextNotesToMarkdown', () => { + it('returns the original markdown unchanged when there are no notes', () => { + const markdown = '# Report\n\nBody.'; + expect(appendContextNotesToMarkdown({ markdown, notes: [] })).toBe( + markdown, + ); + }); + + it('appends a clearly attributed appendix after the original content', () => { + const markdown = '# Report\n\nBody.'; + const result = appendContextNotesToMarkdown({ markdown, notes }); + + expect(result.startsWith('# Report\n\nBody.')).toBe(true); + expect(result).toContain( + '## Appendix: Customer context & management responses', + ); + expect(result).toContain('not findings or conclusions of the testing team'); + expect(result).toContain('### appConfiguration read access'); + expect(result).toContain('last updated 2026-06-11'); + expect(result).toContain('Accepted by design — non-secret bootstrap config.'); + expect(result).toContain('### Unverified email access'); + }); +}); + +describe('appendContextNotesToPdf', () => { + async function buildBasePdf(pages = 1): Promise { + const doc = await PDFDocument.create(); + for (let i = 0; i < pages; i += 1) { + doc.addPage([595, 842]); + } + return Buffer.from(await doc.save()); + } + + it('returns the original bytes untouched when there are no notes', async () => { + const original = await buildBasePdf(); + const result = await appendContextNotesToPdf({ + pdfBytes: original, + notes: [], + }); + expect(result).toBe(original); + }); + + it('appends appendix pages after the original pages', async () => { + const original = await buildBasePdf(2); + const result = await appendContextNotesToPdf({ + pdfBytes: original, + notes, + }); + + const merged = await PDFDocument.load(result); + expect(merged.getPageCount()).toBeGreaterThanOrEqual(3); + }); + + it('paginates long notes across multiple appendix pages', async () => { + const original = await buildBasePdf(1); + const longNote: ReportContextNote = { + issueTitle: 'Long finding', + context: Array.from( + { length: 200 }, + (_, i) => `sentence ${i} with several words to force wrapping.`, + ).join(' '), + updatedAt: new Date('2026-06-11T10:00:00.000Z'), + }; + + const result = await appendContextNotesToPdf({ + pdfBytes: original, + notes: [longNote], + }); + + const merged = await PDFDocument.load(result); + expect(merged.getPageCount()).toBeGreaterThanOrEqual(3); + }); + + it('tolerates characters outside WinAnsi instead of throwing', async () => { + const original = await buildBasePdf(1); + const result = await appendContextNotesToPdf({ + pdfBytes: original, + notes: [ + { + issueTitle: 'Unicode “smart” title — with emoji 🚀 and עברית', + context: 'Curly ‘quotes’, ellipsis… and  nbsp.', + updatedAt: new Date('2026-06-11T10:00:00.000Z'), + }, + ], + }); + + const merged = await PDFDocument.load(result); + expect(merged.getPageCount()).toBe(2); + }); + + it('hard-wraps unbroken tokens wider than the line (long URLs/IDs)', async () => { + const doc = await PDFDocument.create(); + const font = await doc.embedFont(StandardFonts.Helvetica); + const maxWidth = 483; // appendix body width: 595 - 2 * 56 + const longUrl = `https://app.example.com/api/v1/resources/${'a'.repeat(300)}`; + + const lines = wrapText({ + text: `Remediated, see ${longUrl} for the change.`, + font, + fontSize: 10, + maxWidth, + }); + + expect(lines.length).toBeGreaterThan(1); + for (const line of lines) { + expect(font.widthOfTextAtSize(line, 10)).toBeLessThanOrEqual(maxWidth); + } + // Nothing got dropped while splitting. + expect(lines.join('')).toContain('aaaaaaaaaa'); + expect(lines.join(' ')).toContain('Remediated,'); + }); + + it('throws on unparseable provider bytes (caller falls back to original)', async () => { + await expect( + appendContextNotesToPdf({ + pdfBytes: Buffer.from('not a pdf at all'), + notes, + }), + ).rejects.toThrow(); + }); +}); diff --git a/apps/api/src/security-penetration-tests/report-appendix.util.ts b/apps/api/src/security-penetration-tests/report-appendix.util.ts new file mode 100644 index 0000000000..940be9fc0b --- /dev/null +++ b/apps/api/src/security-penetration-tests/report-appendix.util.ts @@ -0,0 +1,242 @@ +import { PDFDocument, PDFFont, rgb, StandardFonts } from 'pdf-lib'; + +/** + * Renders the customer's finding-context notes as a clearly attributed + * appendix on the pentest report artifacts. The original Maced report is + * never modified in place: markdown gets a section appended after a + * horizontal rule, the PDF gets extra pages appended after the original + * ones. Callers MUST treat any thrown error as "serve the original + * artifact unchanged" — the appendix is additive, never load-bearing. + */ +export interface ReportContextNote { + issueTitle: string; + context: string; + updatedAt: Date; +} + +const APPENDIX_TITLE = 'Appendix: Customer context & management responses'; +const APPENDIX_DISCLAIMER = + 'The notes below were provided by the customer in Comp AI after testing. ' + + 'They are customer statements, not findings or conclusions of the testing team.'; + +function formatDate(date: Date): string { + return date.toISOString().slice(0, 10); +} + +export function appendContextNotesToMarkdown(params: { + markdown: string; + notes: ReportContextNote[]; +}): string { + if (params.notes.length === 0) { + return params.markdown; + } + + const sections = params.notes.map( + (note) => + `### ${note.issueTitle.trim()}\n\n` + + `_Customer note, last updated ${formatDate(note.updatedAt)}:_\n\n` + + `${note.context.trim()}`, + ); + + return ( + `${params.markdown.trimEnd()}\n\n---\n\n` + + `## ${APPENDIX_TITLE}\n\n` + + `_${APPENDIX_DISCLAIMER}_\n\n` + + `${sections.join('\n\n')}\n` + ); +} + +// pdf-lib's standard Helvetica fonts only encode WinAnsi (CP-1252-like). +// Map common typographic characters to ASCII and replace anything else +// outside Latin-1 so drawText can never throw on customer input. +function sanitizePdfText(text: string): string { + return text + .replace(/[‘’]/g, "'") + .replace(/[“”]/g, '"') + .replace(/[–—]/g, '-') + .replace(/…/g, '...') + .replace(/ /g, ' ') + .replace(/\r\n?/g, '\n') + .replace(/[\t\v\f]/g, ' ') + .replace(/[^\n\x20-\x7E¡-ÿ]/g, '?'); +} + +// Character-level split for a single token wider than the line (long +// URLs, IDs). Without this the token would be emitted as one overflowing +// line and run off the page margin. +function splitLongWord(params: { + word: string; + font: PDFFont; + fontSize: number; + maxWidth: number; +}): string[] { + const parts: string[] = []; + let current = ''; + for (const char of params.word) { + const candidate = current + char; + if ( + params.font.widthOfTextAtSize(candidate, params.fontSize) > + params.maxWidth && + current + ) { + parts.push(current); + current = char; + } else { + current = candidate; + } + } + if (current) { + parts.push(current); + } + return parts; +} + +/** Exported for tests — every returned line fits within maxWidth. */ +export function wrapText(params: { + text: string; + font: PDFFont; + fontSize: number; + maxWidth: number; +}): string[] { + const lines: string[] = []; + + for (const paragraph of params.text.split('\n')) { + if (!paragraph.trim()) { + lines.push(''); + continue; + } + + let currentLine = ''; + for (const word of paragraph.split(' ')) { + const wordWidth = params.font.widthOfTextAtSize(word, params.fontSize); + if (wordWidth > params.maxWidth) { + if (currentLine) { + lines.push(currentLine); + currentLine = ''; + } + const parts = splitLongWord({ ...params, word }); + lines.push(...parts.slice(0, -1)); + currentLine = parts[parts.length - 1] ?? ''; + continue; + } + + const candidate = currentLine ? `${currentLine} ${word}` : word; + const candidateWidth = params.font.widthOfTextAtSize( + candidate, + params.fontSize, + ); + if (candidateWidth > params.maxWidth && currentLine) { + lines.push(currentLine); + currentLine = word; + } else { + currentLine = candidate; + } + } + if (currentLine) { + lines.push(currentLine); + } + } + + return lines; +} + +const PAGE_SIZE: [number, number] = [595, 842]; // A4, matches NdaPdfService +const MARGIN = 56; +const BODY_SIZE = 10; +const BODY_LEADING = 14; + +export async function appendContextNotesToPdf(params: { + pdfBytes: Buffer; + notes: ReportContextNote[]; +}): Promise { + if (params.notes.length === 0) { + return params.pdfBytes; + } + + const pdfDoc = await PDFDocument.load(params.pdfBytes); + const helvetica = await pdfDoc.embedFont(StandardFonts.Helvetica); + const helveticaBold = await pdfDoc.embedFont(StandardFonts.HelveticaBold); + const maxWidth = PAGE_SIZE[0] - MARGIN * 2; + + let page = pdfDoc.addPage(PAGE_SIZE); + let y = PAGE_SIZE[1] - MARGIN; + + const ensureSpace = (needed: number) => { + if (y - needed < MARGIN) { + page = pdfDoc.addPage(PAGE_SIZE); + y = PAGE_SIZE[1] - MARGIN; + } + }; + + const drawWrapped = (opts: { + text: string; + font: PDFFont; + size: number; + leading: number; + color?: ReturnType; + }) => { + const lines = wrapText({ + text: sanitizePdfText(opts.text), + font: opts.font, + fontSize: opts.size, + maxWidth, + }); + for (const line of lines) { + ensureSpace(opts.leading); + if (line) { + page.drawText(line, { + x: MARGIN, + y, + size: opts.size, + font: opts.font, + color: opts.color ?? rgb(0.1, 0.1, 0.1), + }); + } + y -= opts.leading; + } + }; + + drawWrapped({ + text: APPENDIX_TITLE, + font: helveticaBold, + size: 15, + leading: 20, + }); + y -= 4; + drawWrapped({ + text: APPENDIX_DISCLAIMER, + font: helvetica, + size: 9, + leading: 12, + color: rgb(0.4, 0.4, 0.4), + }); + y -= 12; + + for (const note of params.notes) { + // Keep at least the title and one body line together on a page. + ensureSpace(16 + BODY_LEADING * 2); + drawWrapped({ + text: note.issueTitle.trim(), + font: helveticaBold, + size: 11, + leading: 15, + }); + drawWrapped({ + text: `Customer note, last updated ${formatDate(note.updatedAt)}`, + font: helvetica, + size: 8, + leading: 11, + color: rgb(0.45, 0.45, 0.45), + }); + y -= 2; + drawWrapped({ + text: note.context.trim(), + font: helvetica, + size: BODY_SIZE, + leading: BODY_LEADING, + }); + y -= 14; + } + + return Buffer.from(await pdfDoc.save()); +} diff --git a/apps/api/src/security-penetration-tests/security-penetration-tests.billing.spec.ts b/apps/api/src/security-penetration-tests/security-penetration-tests.billing.spec.ts index 830150233d..83153e7ccf 100644 --- a/apps/api/src/security-penetration-tests/security-penetration-tests.billing.spec.ts +++ b/apps/api/src/security-penetration-tests/security-penetration-tests.billing.spec.ts @@ -34,6 +34,9 @@ jest.mock('@db', () => ({ updateMany: jest.fn(), findUnique: jest.fn(), }, + securityPenetrationTestFindingContext: { + findMany: jest.fn(), + }, secret: { upsert: jest.fn(), }, @@ -47,6 +50,9 @@ type MockDb = { updateMany: jest.Mock; findUnique: jest.Mock; }; + securityPenetrationTestFindingContext: { + findMany: jest.Mock; + }; secret: { upsert: jest.Mock; }; @@ -102,6 +108,10 @@ describe('SecurityPenetrationTestsService billing usage', () => { mockedDb.securityPenetrationTestRun.updateMany.mockResolvedValue({ count: 1, }); + // Default: no stored finding-context notes for the target. + mockedDb.securityPenetrationTestFindingContext.findMany.mockResolvedValue( + [], + ); mockedDb.$transaction.mockImplementation( (callback: (tx: MockDb) => Promise) => callback(mockedDb), ); diff --git a/apps/api/src/security-penetration-tests/security-penetration-tests.controller.spec.ts b/apps/api/src/security-penetration-tests/security-penetration-tests.controller.spec.ts index cad7e6fff5..96e5fd2255 100644 --- a/apps/api/src/security-penetration-tests/security-penetration-tests.controller.spec.ts +++ b/apps/api/src/security-penetration-tests/security-penetration-tests.controller.spec.ts @@ -1,3 +1,7 @@ +// The controller's runtime import chain reaches `@db`, which instantiates +// a Prisma client at module load. These tests never touch the database. +jest.mock('@db', () => ({ db: {} })); + jest.mock('../auth/hybrid-auth.guard', () => ({ HybridAuthGuard: class { canActivate() { @@ -6,6 +10,18 @@ jest.mock('../auth/hybrid-auth.guard', () => ({ }, })); +// The real PermissionGuard transitively imports better-auth's ESM bundle, +// which Jest cannot parse. PERMISSIONS_KEY keeps its real value so +// @RequirePermission metadata stays intact for any metadata assertions. +jest.mock('../auth/permission.guard', () => ({ + PermissionGuard: class { + canActivate() { + return true; + } + }, + PERMISSIONS_KEY: 'required_permissions', +})); + import { SecurityPenetrationTestsController } from './security-penetration-tests.controller'; import type { SecurityPenetrationTestsService } from './security-penetration-tests.service'; import type { Request as ExpressRequest } from 'express'; diff --git a/apps/api/src/security-penetration-tests/security-penetration-tests.module.ts b/apps/api/src/security-penetration-tests/security-penetration-tests.module.ts index 1f648b8f34..6140e3fc9b 100644 --- a/apps/api/src/security-penetration-tests/security-penetration-tests.module.ts +++ b/apps/api/src/security-penetration-tests/security-penetration-tests.module.ts @@ -3,13 +3,23 @@ import { AuthModule } from '../auth/auth.module'; import { BillingModule } from '../billing/billing.module'; import { PentestCreditsController } from './pentest-credits.controller'; import { PentestCreditsService } from './pentest-credits.service'; +import { PentestFindingContextsController } from './pentest-finding-contexts.controller'; +import { PentestFindingContextsService } from './pentest-finding-contexts.service'; import { SecurityPenetrationTestsController } from './security-penetration-tests.controller'; import { SecurityPenetrationTestsService } from './security-penetration-tests.service'; @Module({ imports: [AuthModule, BillingModule], - controllers: [SecurityPenetrationTestsController, PentestCreditsController], - providers: [SecurityPenetrationTestsService, PentestCreditsService], + controllers: [ + SecurityPenetrationTestsController, + PentestCreditsController, + PentestFindingContextsController, + ], + providers: [ + SecurityPenetrationTestsService, + PentestCreditsService, + PentestFindingContextsService, + ], exports: [PentestCreditsService], }) export class SecurityPenetrationTestsModule {} diff --git a/apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts b/apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts index b3228195b9..575bb50ad8 100644 --- a/apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts +++ b/apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts @@ -2,6 +2,7 @@ import { HttpException, HttpStatus } from '@nestjs/common'; import { db } from '@db'; import { validate } from 'class-validator'; import { createHash } from 'node:crypto'; +import { PDFDocument } from 'pdf-lib'; import type { BillingEntitlementsService } from '../billing/billing-entitlements.service'; import type { CredentialVaultService } from '../integration-platform/services/credential-vault.service'; import { CreatePenetrationTestDto } from './dto/create-penetration-test.dto'; @@ -45,6 +46,9 @@ jest.mock('@db', () => ({ findUnique: jest.fn(), findMany: jest.fn(), }, + securityPenetrationTestFindingContext: { + findMany: jest.fn(), + }, secret: { upsert: jest.fn(), findUnique: jest.fn(), @@ -65,6 +69,9 @@ type MockDb = { findUnique: jest.Mock; findMany: jest.Mock; }; + securityPenetrationTestFindingContext: { + findMany: jest.Mock; + }; secret: { upsert: jest.Mock; findUnique: jest.Mock; @@ -139,6 +146,10 @@ describe('SecurityPenetrationTestsService', () => { mockedDb.securityPenetrationTestRun.findMany.mockResolvedValue([ { providerRunId: 'run_123' }, ]); + // Default: no stored finding-context notes for the target. + mockedDb.securityPenetrationTestFindingContext.findMany.mockResolvedValue( + [], + ); mockedDb.secret.upsert.mockResolvedValue({}); mockedDb.secret.findUnique.mockResolvedValue({ id: 'sec_default', @@ -233,6 +244,108 @@ describe('SecurityPenetrationTestsService', () => { expect(mockedDb.securityPenetrationTestRun.upsert).toHaveBeenCalledTimes(1); }); + it('omits additionalContext when there is no context to send', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ id: 'run_456', status: 'provisioning' }), { + status: 200, + }), + ); + + await service.createReport('org_123', { + targetUrl: 'https://app.example.com', + }); + + const requestBody = await getRequestBody(); + expect(requestBody).not.toHaveProperty('additionalContext'); + }); + + it('passes user-provided additional context to the provider', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ id: 'run_456', status: 'provisioning' }), { + status: 200, + }), + ); + + await service.createReport('org_123', { + targetUrl: 'https://app.example.com', + additionalContext: 'We deployed fixes for the auth findings last week.', + }); + + const requestBody = await getRequestBody(); + expect(requestBody.additionalContext).toBe( + 'We deployed fixes for the auth findings last week.', + ); + }); + + it('creates the run even when the notes lookup fails (best-effort context)', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ id: 'run_456', status: 'provisioning' }), { + status: 200, + }), + ); + // e.g. transient outage, or the table missing mid-deploy before the + // migration has run — must never block pentest creation. + mockedDb.securityPenetrationTestFindingContext.findMany.mockRejectedValue( + new Error('relation does not exist'), + ); + + const result = await service.createReport('org_123', { + targetUrl: 'https://app.example.com', + additionalContext: 'User-typed context.', + }); + + expect(result.id).toBe('run_456'); + const requestBody = await getRequestBody(); + expect(requestBody.additionalContext).toBe('User-typed context.'); + }); + + it('appends stored finding-context notes for the normalized target to additionalContext', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ id: 'run_456', status: 'provisioning' }), { + status: 200, + }), + ); + mockedDb.securityPenetrationTestFindingContext.findMany.mockResolvedValue([ + { + issueTitle: 'appConfiguration read access', + context: + 'Accepted by design — collection holds non-secret bootstrap config.', + }, + { + issueTitle: 'Unverified email access', + context: 'Email verification is now enabled in the tested environment.', + }, + ]); + + await service.createReport('org_123', { + // Mixed-case host + trailing slash — the stored-notes lookup must + // normalize before matching rows keyed by canonical target URL. + targetUrl: 'https://App.example.com/', + additionalContext: 'Focus on the three previously reported findings.', + }); + + expect( + mockedDb.securityPenetrationTestFindingContext.findMany, + ).toHaveBeenCalledWith( + expect.objectContaining({ + where: { + organizationId: 'org_123', + targetUrl: 'https://app.example.com', + }, + }), + ); + + const requestBody = await getRequestBody(); + const additionalContext = requestBody.additionalContext as string; + expect(additionalContext).toContain( + 'Focus on the three previously reported findings.', + ); + expect(additionalContext).toContain('"appConfiguration read access"'); + expect(additionalContext).toContain( + 'Email verification is now enabled in the tested environment.', + ); + }); + it('accepts valid scan profile fields in the create DTO', async () => { const dto = Object.assign(new CreatePenetrationTestDto(), { targetUrl: 'https://app.example.com', @@ -696,6 +809,138 @@ describe('SecurityPenetrationTestsService', () => { expect(output.contentType).toBe('text/markdown; charset=utf-8'); }); + it('appends customer context notes to the markdown report', async () => { + fetchMock.mockResolvedValueOnce( + new Response( + JSON.stringify({ + id: 'run_notes', + targetUrl: 'https://app.example.com', + status: 'completed', + createdAt: '2026-06-01T00:00:00.000Z', + updatedAt: '2026-06-01T00:00:00.000Z', + }), + { status: 200 }, + ), + ); + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ markdown: '# Report\n\nBody.' }), { + status: 200, + }), + ); + mockedDb.securityPenetrationTestFindingContext.findMany.mockResolvedValue([ + { + issueTitle: 'appConfiguration read access', + context: 'Accepted by design.', + updatedAt: new Date('2026-06-11T00:00:00.000Z'), + }, + ]); + + const output = await service.getReportOutput('org_123', 'run_notes'); + const markdown = output.buffer.toString('utf-8'); + + expect(markdown.startsWith('# Report\n\nBody.')).toBe(true); + expect(markdown).toContain( + '## Appendix: Customer context & management responses', + ); + expect(markdown).toContain('Accepted by design.'); + }); + + it('serves the original report when the notes lookup fails', async () => { + fetchMock.mockResolvedValueOnce( + new Response( + JSON.stringify({ + id: 'run_notes_err', + targetUrl: 'https://app.example.com', + status: 'completed', + createdAt: '2026-06-01T00:00:00.000Z', + updatedAt: '2026-06-01T00:00:00.000Z', + }), + { status: 200 }, + ), + ); + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ markdown: '# Report' }), { status: 200 }), + ); + mockedDb.securityPenetrationTestFindingContext.findMany.mockRejectedValue( + new Error('db unavailable'), + ); + + const output = await service.getReportOutput('org_123', 'run_notes_err'); + + expect(output.buffer.toString('utf-8')).toBe('# Report'); + }); + + it('serves the original PDF bytes when the provider PDF cannot be parsed', async () => { + const bogusPdf = Buffer.from('definitely not a pdf'); + fetchMock.mockResolvedValueOnce( + new Response( + JSON.stringify({ + id: 'run_pdf_bogus', + targetUrl: 'https://app.example.com', + status: 'completed', + createdAt: '2026-06-01T00:00:00.000Z', + updatedAt: '2026-06-01T00:00:00.000Z', + }), + { status: 200 }, + ), + ); + fetchMock.mockResolvedValueOnce( + new Response(bogusPdf, { + status: 200, + headers: { 'Content-Type': 'application/pdf' }, + }), + ); + mockedDb.securityPenetrationTestFindingContext.findMany.mockResolvedValue([ + { + issueTitle: 'Some finding', + context: 'Some note.', + updatedAt: new Date('2026-06-11T00:00:00.000Z'), + }, + ]); + + const output = await service.getReportPdf('org_123', 'run_pdf_bogus'); + + expect(output.buffer).toEqual(bogusPdf); + expect(output.contentType).toBe('application/pdf'); + }); + + it('appends appendix pages to the PDF report when notes exist', async () => { + const baseDoc = await PDFDocument.create(); + baseDoc.addPage([595, 842]); + const basePdf = Buffer.from(await baseDoc.save()); + + fetchMock.mockResolvedValueOnce( + new Response( + JSON.stringify({ + id: 'run_pdf_notes', + targetUrl: 'https://app.example.com', + status: 'completed', + createdAt: '2026-06-01T00:00:00.000Z', + updatedAt: '2026-06-01T00:00:00.000Z', + }), + { status: 200 }, + ), + ); + fetchMock.mockResolvedValueOnce( + new Response(basePdf, { + status: 200, + headers: { 'Content-Type': 'application/pdf' }, + }), + ); + mockedDb.securityPenetrationTestFindingContext.findMany.mockResolvedValue([ + { + issueTitle: 'appConfiguration read access', + context: 'Accepted by design.', + updatedAt: new Date('2026-06-11T00:00:00.000Z'), + }, + ]); + + const output = await service.getReportPdf('org_123', 'run_pdf_notes'); + + const merged = await PDFDocument.load(output.buffer); + expect(merged.getPageCount()).toBeGreaterThanOrEqual(2); + }); + it('falls back to markdown content type when response omits content-type', async () => { const fixtureContent = 'raw report'; diff --git a/apps/api/src/security-penetration-tests/security-penetration-tests.service.ts b/apps/api/src/security-penetration-tests/security-penetration-tests.service.ts index 7a85337663..a6afcb2b04 100644 --- a/apps/api/src/security-penetration-tests/security-penetration-tests.service.ts +++ b/apps/api/src/security-penetration-tests/security-penetration-tests.service.ts @@ -33,6 +33,15 @@ import { type ScanDepth, } from './dto/create-penetration-test.dto'; import { BillingEntitlementsService } from '../billing/billing-entitlements.service'; +import { + buildAdditionalContext, + normalizeTargetUrl, +} from './finding-context.util'; +import { + appendContextNotesToMarkdown, + appendContextNotesToPdf, + type ReportContextNote, +} from './report-appendix.util'; import { PentestCreditsService } from './pentest-credits.service'; /** @@ -243,6 +252,12 @@ export class SecurityPenetrationTestsService { payload: CreatePenetrationTestDto, ): Promise { const resolvedWebhookUrl = this.resolveWebhookUrl(payload.webhookUrl); + // Resolved before the billing reservation so a DB failure here can't + // leave a debited allowance behind. + const additionalContext = await this.resolveAdditionalContext( + organizationId, + payload, + ); const billingUsageSourceId = `pending:${randomUUID()}`; let consumedSubscriptionAllowance = false; @@ -322,6 +337,7 @@ export class SecurityPenetrationTestsService { ? { evidenceLevel: payload.evidenceLevel } : {}), ...(payload.checks ? { checks: payload.checks } : {}), + ...(additionalContext ? { additionalContext } : {}), // Attribution metadata — Maced persists this verbatim and returns it on // list/get. Gives us a second source of truth for the org↔run mapping // (our `security_penetration_test_runs` table is the primary one) so @@ -439,6 +455,33 @@ export class SecurityPenetrationTestsService { }; } + /** + * Composes the free-text briefing sent to the testing agent: the + * caller's own `additionalContext` (if any) plus the per-finding + * context notes customers saved on findings from previous scans of the + * same target (see PentestFindingContextsService). This is what makes a + * re-run an informed retest instead of a blind one. + * + * The notes lookup is best-effort: a DB failure (transient outage, or + * the table missing mid-deploy before the migration runs) must never + * block creating a pentest — the run proceeds with whatever context + * the caller typed, and the miss is logged. + */ + private async resolveAdditionalContext( + organizationId: string, + payload: CreatePenetrationTestDto, + ): Promise { + const findingContexts = await this.findContextNotesQuietly( + organizationId, + payload.targetUrl, + ); + + return buildAdditionalContext({ + userProvidedContext: payload.additionalContext, + findingContexts, + }); + } + async getReport( organizationId: string, id: string, @@ -492,15 +535,24 @@ export class SecurityPenetrationTestsService { organizationId: string, id: string, ): Promise { - await this.getReport(organizationId, id); + const run = await this.getReport(organizationId, id); const report = await this.callMaced( () => this.macedClient.pentests.report(id), `fetching penetration test report ${id}`, ); + const notes = await this.findContextNotesQuietly( + organizationId, + run.targetUrl, + ); + const markdown = + notes.length > 0 + ? appendContextNotesToMarkdown({ markdown: report.markdown, notes }) + : report.markdown; + return { - buffer: Buffer.from(report.markdown, 'utf-8'), + buffer: Buffer.from(markdown, 'utf-8'), contentType: 'text/markdown; charset=utf-8', contentDisposition: null, }; @@ -510,20 +562,69 @@ export class SecurityPenetrationTestsService { organizationId: string, id: string, ): Promise { - await this.getReport(organizationId, id); + const run = await this.getReport(organizationId, id); const blob = await this.callMaced( () => this.macedClient.pentests.reportPdf(id), `fetching penetration test PDF ${id}`, ); + const original = Buffer.from(await blob.arrayBuffer()); + const notes = await this.findContextNotesQuietly( + organizationId, + run.targetUrl, + ); + + let buffer: Buffer = original; + if (notes.length > 0) { + try { + buffer = await appendContextNotesToPdf({ pdfBytes: original, notes }); + } catch (error) { + // The appendix is additive — a malformed/unparseable provider PDF + // must never break the download. Serve the original bytes. + this.logger.error( + `Unable to append context notes to PDF for run ${id}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + return { - buffer: Buffer.from(await blob.arrayBuffer()), + buffer, contentType: blob.type || 'application/pdf', contentDisposition: `attachment; filename="penetration-test-${id}.pdf"`, }; } + /** + * Context notes for a target, for the report appendix. Quiet: a notes + * lookup failure must never break a report download, so errors are + * logged and an empty list is returned (appendix simply omitted). + */ + private async findContextNotesQuietly( + organizationId: string, + targetUrl: string, + ): Promise { + if (!targetUrl) { + return []; + } + try { + return await db.securityPenetrationTestFindingContext.findMany({ + where: { organizationId, targetUrl: normalizeTargetUrl(targetUrl) }, + orderBy: { createdAt: 'asc' }, + select: { issueTitle: true, context: true, updatedAt: true }, + }); + } catch (error) { + this.logger.error( + `Unable to load finding context notes for report appendix: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return []; + } + } + async handleWebhook(params: { rawBody: Buffer | undefined; signatureHeader: string | undefined; diff --git a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/CreateRunPanel.test.tsx b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/CreateRunPanel.test.tsx index cb92aa75aa..c809784e7b 100644 --- a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/CreateRunPanel.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/CreateRunPanel.test.tsx @@ -16,6 +16,24 @@ vi.mock('sonner', () => ({ toast: { error: vi.fn() }, })); +// RunContextField fetches saved finding-context notes over SWR; tests must +// not hit the network. Override `notes` per test before rendering. +const findingContextsMock = vi.hoisted(() => ({ + notes: [] as Array<{ id: string; providerIssueId: string }>, +})); + +vi.mock('../hooks/use-pentest-finding-contexts', () => ({ + usePentestFindingContexts: () => ({ + contexts: findingContextsMock.notes, + contextByIssueId: new Map(), + isLoading: false, + error: undefined, + isSaving: false, + saveContext: vi.fn(), + removeContext: vi.fn(), + }), +})); + async function confirmAuthorization(user: ReturnType) { await user.click( screen.getByText('I own this target or have written authorization to test it.'), @@ -29,6 +47,65 @@ function checkInput(label: RegExp) { describe('CreateRunPanel', () => { beforeEach(() => { vi.clearAllMocks(); + findingContextsMock.notes = []; + }); + + it('includes trimmed additional context in the submit payload', async () => { + const user = userEvent.setup(); + const onSubmit = vi.fn(async () => ({ id: 'run_ctx' })); + + render(); + + await user.type(screen.getByLabelText(/target url/i), 'app.example.com'); + await user.type( + screen.getByLabelText(/context for the agent/i), + ' We remediated the auth findings last week. ', + ); + await confirmAuthorization(user); + await user.click(screen.getByRole('button', { name: /start scan/i })); + + await waitFor(() => { + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({ + additionalContext: 'We remediated the auth findings last week.', + }), + ); + }); + }); + + it('omits additionalContext when the context field is left empty', async () => { + const user = userEvent.setup(); + let submittedPayload: PentestCreateRequest | undefined; + const onSubmit = vi.fn(async (payload: PentestCreateRequest) => { + submittedPayload = payload; + return { id: 'run_no_ctx' }; + }); + + render(); + + await user.type(screen.getByLabelText(/target url/i), 'app.example.com'); + await confirmAuthorization(user); + await user.click(screen.getByRole('button', { name: /start scan/i })); + + await waitFor(() => { + expect(onSubmit).toHaveBeenCalled(); + }); + expect(submittedPayload).not.toHaveProperty('additionalContext'); + }); + + it('tells the user how many saved finding notes will be shared automatically', () => { + findingContextsMock.notes = [ + { id: 'ptfc_1', providerIssueId: 'issue_1' }, + { id: 'ptfc_2', providerIssueId: 'issue_2' }, + ]; + + render(); + + expect( + screen.getByText( + /2 saved finding context notes for this target will be shared with the agent automatically/i, + ), + ).toBeInTheDocument(); }); it('routes users without allowance to billing even when required fields are empty', async () => { diff --git a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/CreateRunPanel.tsx b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/CreateRunPanel.tsx index cbfbdbdd9a..21822c234a 100644 --- a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/CreateRunPanel.tsx +++ b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/CreateRunPanel.tsx @@ -22,6 +22,7 @@ import { toast } from 'sonner'; import { z } from 'zod'; import { AuthorizationConsentField } from './AuthorizationConsentField'; import { CreateRunTargetFields } from './CreateRunTargetFields'; +import { RunContextField } from './RunContextField'; import { RunExpectationSummary } from './RunExpectationSummary'; import { ScanAdvancedOptions } from './ScanAdvancedOptions'; import { ScanProfilePicker } from './ScanProfilePicker'; @@ -71,6 +72,10 @@ const createRunSchema = z.object({ .refine((value) => value === true, { message: 'Confirm you own or are authorized to test this target.', }), + additionalContext: z + .string() + .max(4000, 'Keep context under 4000 characters.') + .optional(), }); export type CreateRunForm = z.infer; @@ -98,11 +103,14 @@ export function CreateRunPanel({ evidenceLevel: standardDefaults.evidenceLevel, checks: standardDefaults.checks, authorized: false, + additionalContext: '', }, }); const evidenceLevel = form.watch('evidenceLevel'); const checks = form.watch('checks'); const authorized = form.watch('authorized'); + const targetUrlValue = form.watch('targetUrl'); + const normalizedTargetUrl = useMemo(() => normalizeUrl(targetUrlValue), [targetUrlValue]); const effectiveMode = useMemo( () => resolveEffectiveScanMode({ @@ -180,6 +188,9 @@ export function CreateRunPanel({ scanDepth: submitScanDepth, evidenceLevel: values.evidenceLevel, checks: values.checks, + ...(values.additionalContext?.trim() + ? { additionalContext: values.additionalContext.trim() } + : {}), }); router.push(`/${orgId}/security/penetration-tests/${encodeURIComponent(result.id)}`); } catch { @@ -235,6 +246,8 @@ export function CreateRunPanel({ + + ; + return ( + + ); } if (isLoading && !run) { diff --git a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.test.tsx b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.test.tsx new file mode 100644 index 0000000000..e6792dc26b --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.test.tsx @@ -0,0 +1,146 @@ +import type { + PentestFindingContext, + PentestIssue, +} from '@/lib/security/penetration-tests-client'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { FindingContextSection } from './FindingContextSection'; + +const permissionsMock = vi.hoisted(() => ({ + canUpdatePentest: false, +})); + +vi.mock('@/hooks/use-permissions', () => ({ + usePermissions: () => ({ + hasPermission: (resource: string, action: string) => + resource === 'pentest' && + (action === 'read' || + (action === 'update' && permissionsMock.canUpdatePentest)), + }), +})); + +const contextsMock = vi.hoisted(() => ({ + contextByIssueId: new Map(), + saveContext: vi.fn(), + removeContext: vi.fn(), +})); + +vi.mock('../hooks/use-pentest-finding-contexts', () => ({ + usePentestFindingContexts: () => ({ + contexts: [...contextsMock.contextByIssueId.values()], + contextByIssueId: contextsMock.contextByIssueId, + isLoading: false, + error: undefined, + isSaving: false, + saveContext: contextsMock.saveContext, + removeContext: contextsMock.removeContext, + }), +})); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + +const issue: PentestIssue = { + id: 'issue_1', + runId: 'run_1', + title: 'appConfiguration read access', + severity: 'medium', + status: 'open', + createdAt: '2026-06-01T00:00:00.000Z', + updatedAt: '2026-06-01T00:00:00.000Z', +}; + +const savedNote: PentestFindingContext = { + id: 'ptfc_1', + organizationId: 'org_1', + providerIssueId: 'issue_1', + providerRunId: 'run_1', + targetUrl: 'https://app.example.com', + issueTitle: 'appConfiguration read access', + context: 'Accepted by design — non-secret bootstrap config.', + createdAt: '2026-06-01T00:00:00.000Z', + updatedAt: '2026-06-01T00:00:00.000Z', +}; + +function renderSection() { + return render( + , + ); +} + +describe('FindingContextSection', () => { + beforeEach(() => { + vi.clearAllMocks(); + permissionsMock.canUpdatePentest = false; + contextsMock.contextByIssueId = new Map(); + }); + + it('renders nothing for read-only users when there is no saved note', () => { + renderSection(); + + expect(screen.queryByText(/retest context/i)).not.toBeInTheDocument(); + }); + + it('shows the saved note read-only to users without pentest:update', () => { + contextsMock.contextByIssueId = new Map([['issue_1', savedNote]]); + + renderSection(); + + expect(screen.getByText(/retest context/i)).toBeInTheDocument(); + expect( + screen.getByText('Accepted by design — non-secret bootstrap config.'), + ).toBeInTheDocument(); + expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: /save context/i }), + ).not.toBeInTheDocument(); + }); + + it('lets pentest:update users save context on a finding', async () => { + permissionsMock.canUpdatePentest = true; + contextsMock.saveContext.mockResolvedValue(undefined); + const user = userEvent.setup(); + + renderSection(); + + await user.type( + screen.getByRole('textbox'), + 'Accepted by design — reads are non-sensitive.', + ); + await user.click(screen.getByRole('button', { name: /save context/i })); + + await waitFor(() => { + expect(contextsMock.saveContext).toHaveBeenCalledWith({ + issueId: 'issue_1', + runId: 'run_1', + context: 'Accepted by design — reads are non-sensitive.', + }); + }); + }); + + it('lets pentest:update users remove a saved note', async () => { + permissionsMock.canUpdatePentest = true; + contextsMock.contextByIssueId = new Map([['issue_1', savedNote]]); + contextsMock.removeContext.mockResolvedValue(undefined); + const user = userEvent.setup(); + + renderSection(); + + expect( + screen.getByRole('button', { name: /update context/i }), + ).toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: /^remove$/i })); + + await waitFor(() => { + expect(contextsMock.removeContext).toHaveBeenCalledWith('issue_1'); + }); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.tsx b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.tsx new file mode 100644 index 0000000000..2256577760 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.tsx @@ -0,0 +1,141 @@ +'use client'; + +import { usePermissions } from '@/hooks/use-permissions'; +import type { PentestIssue } from '@/lib/security/penetration-tests-client'; +import { zodResolver } from '@hookform/resolvers/zod'; +import { Button, Textarea } from '@trycompai/design-system'; +import { useForm } from 'react-hook-form'; +import { toast } from 'sonner'; +import { z } from 'zod'; +import { usePentestFindingContexts } from '../hooks/use-pentest-finding-contexts'; + +const findingContextSchema = z.object({ + context: z + .string() + .trim() + .min(1, 'Context is required.') + .max(2000, 'Keep context under 2000 characters.'), +}); + +type FindingContextForm = z.infer; + +interface FindingContextSectionProps { + orgId: string; + issue: PentestIssue; + runId?: string | null; + targetUrl?: string | null; +} + +/** + * Customer context note on a single finding ("accepted by design + * because…", "fixed via…"). Saved notes are automatically shared with the + * testing agent on future scans of the same target, so retests validate + * against the customer's explanation instead of blindly re-flagging. + * Editing requires `pentest:update`; read-only roles see the saved note. + */ +export function FindingContextSection({ + orgId, + issue, + runId, + targetUrl, +}: FindingContextSectionProps) { + const { hasPermission } = usePermissions(); + const canEdit = hasPermission('pentest', 'update'); + const { contextByIssueId, isSaving, saveContext, removeContext } = + usePentestFindingContexts(orgId, targetUrl); + const existing = contextByIssueId.get(issue.id); + const resolvedRunId = issue.runId ?? runId ?? null; + + const form = useForm({ + resolver: zodResolver(findingContextSchema), + values: { context: existing?.context ?? '' }, + }); + + if (!targetUrl) return null; + if (!canEdit && !existing) return null; + + const handleSave = form.handleSubmit(async (values) => { + if (!resolvedRunId) return; + try { + await saveContext({ + issueId: issue.id, + runId: resolvedRunId, + context: values.context, + }); + toast.success('Context saved — future scans of this target will include it'); + } catch (error) { + toast.error(error instanceof Error ? error.message : 'Unable to save context'); + } + }); + + const handleRemove = async () => { + try { + await removeContext(issue.id); + form.reset({ context: '' }); + toast.success('Context removed'); + } catch (error) { + toast.error(error instanceof Error ? error.message : 'Unable to remove context'); + } + }; + + return ( +
+
+ Retest context +
+

+ Explain this finding — e.g. why it's accepted by design or how it was + remediated. Notes are shared with the testing agent on future scans of this + target, so retests take your context into account instead of re-flagging + blindly. +

+ + {canEdit ? ( +
+