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 index f6d680390..ba3c0bcd6 100644 --- a/apps/api/src/security-penetration-tests/finding-context.util.spec.ts +++ b/apps/api/src/security-penetration-tests/finding-context.util.spec.ts @@ -28,6 +28,18 @@ describe('normalizeTargetUrl', () => { ); }); + 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'); }); diff --git a/apps/api/src/security-penetration-tests/finding-context.util.ts b/apps/api/src/security-penetration-tests/finding-context.util.ts index c8f42737b..229c2511a 100644 --- a/apps/api/src/security-penetration-tests/finding-context.util.ts +++ b/apps/api/src/security-penetration-tests/finding-context.util.ts @@ -12,19 +12,26 @@ export interface FindingContextNote { /** * 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`). Non-URL input - * is returned trimmed — the create DTO already enforces a valid URL. + * (`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 = ''; - let normalized = url.toString(); - while (normalized.endsWith('/')) { - normalized = normalized.slice(0, -1); + while (url.pathname.length > 1 && url.pathname.endsWith('/')) { + url.pathname = url.pathname.slice(0, -1); } - return normalized; + 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; } 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 index 0a6068eb6..6e6c101a2 100644 --- a/apps/api/src/security-penetration-tests/report-appendix.util.spec.ts +++ b/apps/api/src/security-penetration-tests/report-appendix.util.spec.ts @@ -1,7 +1,8 @@ -import { PDFDocument } from 'pdf-lib'; +import { PDFDocument, StandardFonts } from 'pdf-lib'; import { appendContextNotesToMarkdown, appendContextNotesToPdf, + wrapText, type ReportContextNote, } from './report-appendix.util'; @@ -108,6 +109,28 @@ describe('appendContextNotesToPdf', () => { 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({ diff --git a/apps/api/src/security-penetration-tests/report-appendix.util.ts b/apps/api/src/security-penetration-tests/report-appendix.util.ts index cc51f9d72..940be9fc0 100644 --- a/apps/api/src/security-penetration-tests/report-appendix.util.ts +++ b/apps/api/src/security-penetration-tests/report-appendix.util.ts @@ -61,7 +61,38 @@ function sanitizePdfText(text: string): string { .replace(/[^\n\x20-\x7E¡-ÿ]/g, '?'); } -function wrapText(params: { +// 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; @@ -77,6 +108,18 @@ function wrapText(params: { 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, 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 e41f54b52..575bb50ad 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 @@ -277,6 +277,28 @@ describe('SecurityPenetrationTestsService', () => { ); }); + 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' }), { 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 2b0afd15f..a6afcb2b0 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 @@ -461,20 +461,20 @@ export class SecurityPenetrationTestsService { * 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 db.securityPenetrationTestFindingContext.findMany({ - where: { - organizationId, - targetUrl: normalizeTargetUrl(payload.targetUrl), - }, - orderBy: { createdAt: 'asc' }, - select: { issueTitle: true, context: true }, - }); + const findingContexts = await this.findContextNotesQuietly( + organizationId, + payload.targetUrl, + ); return buildAdditionalContext({ userProvidedContext: payload.additionalContext,