diff --git a/lib/services/campaign-grouping-service.test.ts b/lib/services/campaign-grouping-service.test.ts index dcb868c..5e007d5 100644 --- a/lib/services/campaign-grouping-service.test.ts +++ b/lib/services/campaign-grouping-service.test.ts @@ -83,7 +83,7 @@ function makeClient(rows: MockRows) { query: vi.fn(async (sql: string, params?: unknown[]) => { clientCalls.push({ sql, params: params ?? [] }); - if (sql.includes('requester_contact_id, company_id, created_at')) { + if (sql.includes('SELECT title, company_id, created_at')) { return { rows: rows.ownReport ?? [], rowCount: rows.ownReport?.length ?? 0 }; } if (sql.includes('FROM messages') && sql.includes('WHERE report_id = $1')) { @@ -101,7 +101,7 @@ function makeClient(rows: MockRows) { if (sql.includes('message_id = ANY')) { return { rows: rows.candidateIndicators ?? [], rowCount: rows.candidateIndicators?.length ?? 0 }; } - if (sql.includes('BETWEEN $4::timestamptz')) { + if (sql.includes('BETWEEN $3::timestamptz') && sql.includes('FROM reports r')) { return { rows: rows.tier3 ?? [], rowCount: rows.tier3?.length ?? 0 }; } if (sql.includes('SELECT campaign_key')) { @@ -140,7 +140,6 @@ describe('groupReportIntoCampaign', () => { const REPORT_ROW = { title: 'Re: Invoice Alert', - requester_contact_id: 5, company_id: 10, created_at: '2026-07-15T10:00:00Z', }; @@ -173,6 +172,36 @@ describe('groupReportIntoCampaign', () => { expect(callsContaining('INSERT INTO campaigns')).toHaveLength(0); }); + it('bug fix (phishing-recipient-seubert): Tier 3 matches a sibling report from a DIFFERENT reporting contact at the same company — company-wide, not contact-scoped', async () => { + // Regression coverage for the fix: Tier 3 previously required + // r.requester_contact_id = $1, which meant two different employees at + // the same company reporting the identical campaign (same normalized + // subject, same company, within 24h) could never be consolidated into + // one campaign. The query itself must not filter or join on any + // contact/requester column, and must scope only by company_id. + stage({ + ownReport: [REPORT_ROW], // this report's own reporter is irrelevant to the match now + ownMessage: [], + tier3: [{ campaign_id: 'shared-campaign', title: 'Invoice Alert' }], + }); + + const result = await groupReportIntoCampaign('report-different-reporter'); + + expect(result).toEqual({ + campaignId: 'shared-campaign', + groupMethod: 'sender_subject_client', + created: false, + }); + + const tier3Calls = clientCalls.filter( + (c) => c.sql.includes('FROM reports r') && c.sql.includes('r.company_id = $1') + ); + expect(tier3Calls).toHaveLength(1); + expect(tier3Calls[0].sql).not.toContain('requester_contact_id'); + expect(tier3Calls[0].sql).not.toContain('JOIN contacts'); + expect(tier3Calls[0].params).toEqual([10, 'report-different-reporter', REPORT_ROW.created_at]); + }); + it('creates exactly one new campaign when no tier matches anything', async () => { stage({ ownReport: [REPORT_ROW], @@ -192,7 +221,7 @@ describe('groupReportIntoCampaign', () => { const insertCalls = callsContaining('INSERT INTO campaigns'); expect(insertCalls).toHaveLength(1); expect(insertCalls[0].params).toEqual([ - 'sender_subject_client:5:invoice alert:10', + 'sender_subject_client:invoice alert:10', 'sender_subject_client', ]); expect(callsContaining('UPDATE campaigns')).toHaveLength(0); @@ -407,7 +436,7 @@ describe('groupReportIntoCampaign', () => { ownMessage: [], tier3: [], ownCampaign: [ - { campaign_key: 'sender_subject_client:5:invoice alert:10', group_method: 'sender_subject_client' }, + { campaign_key: 'sender_subject_client:invoice alert:10', group_method: 'sender_subject_client' }, ], }); @@ -450,7 +479,7 @@ describe('groupReportIntoCampaign', () => { ownMessage: [], tier3: [], ownCampaign: [ - { campaign_key: 'sender_subject_client:5:old subject:10', group_method: 'sender_subject_client' }, + { campaign_key: 'sender_subject_client:old subject:10', group_method: 'sender_subject_client' }, ], insertCampaign: [{ id: 'diverged-new-campaign' }], }); @@ -532,7 +561,7 @@ describe('groupReportIntoCampaign', () => { ownMessage: [], tier3: [], ownCampaign: [ - { campaign_key: 'sender_subject_client:5:old subject:10', group_method: 'sender_subject_client' }, + { campaign_key: 'sender_subject_client:old subject:10', group_method: 'sender_subject_client' }, ], insertCampaign: [{ id: 'diverged-new-campaign-h' }], }); diff --git a/lib/services/campaign-grouping-service.ts b/lib/services/campaign-grouping-service.ts index e11a59d..7b0e205 100644 --- a/lib/services/campaign-grouping-service.ts +++ b/lib/services/campaign-grouping-service.ts @@ -10,14 +10,32 @@ * logic between callers, mirroring `phishing-detector.ts`'s shared-core * architecture. * - * D-07 limitation (load-bearing, stated explicitly): `parseAndStoreMessage` - * (the only writer of `messages`/`indicators` rows — Phase 16) is not wired - * into the automatic webhook/cron path this phase. That means the automatic - * path only ever has `reports`/`contacts` data available, so Tier 1 - * (Message-ID) and Tier 2 (attachment-hash/URL-domain) can only ever match - * for a report that has already been through an explicit `/analyze` call at - * least once. Until then, automatic grouping effectively only reaches - * Tier 3 (sender + normalized subject + client + 24h window). + * D-07 limitation (load-bearing, stated explicitly): `groupReportIntoCampaign` + * always runs BEFORE `parseAndStoreMessage` on the automatic webhook path + * (see `webhook-service.ts`'s `triggerPhishingDetection()` — grouping happens + * first, parsing happens afterward inside `runGatedPhishingStages()`). That + * means at grouping time the CURRENT report never has its own `messages`/ + * `indicators` row yet, so Tier 1 (Message-ID) and Tier 2 (attachment-hash/ + * URL-domain) — both of which require the report's OWN signal to search + * for candidates — can never match on the automatic path's one-and-only + * grouping call (subsequent webhook events short-circuit via + * `skipIfAlreadyGrouped`). Automatic grouping therefore always resolves via + * Tier 3 (normalized subject + company + 24h window). + * + * Bug fix (debug session phishing-recipient-seubert): Tier 3 previously + * scoped its match to `reports.requester_contact_id` — i.e. it only ever + * merged reports filed by the SAME reporting employee. Since Tier 3 is the + * only tier automatic grouping can ever reach (see D-07 above), that meant + * the same phishing campaign sent to and reported by MULTIPLE different + * employees at the same company could never be consolidated into one + * campaign — each recipient's report silently became its own single-report + * campaign, so any single ticket's evidence/blast-radius view under-reported + * the campaign's true recipient list. Tier 3 now scopes to company + subject + * only (no contact/requester restriction), matching its `sender_subject_ + * client` name's original intent of grouping the same external campaign + * across a company, independent of who reported it. (`sender` isn't + * literally available yet at this point — see D-07 — so "client" scoping is + * company-wide, deliberately wider than a single reporter.) */ import type { PoolClient } from 'pg'; @@ -64,7 +82,6 @@ export interface GroupReportResult { interface OwnReportRow { title: string | null; - requester_contact_id: number | null; company_id: number | null; created_at: string; campaign_id: string | null; @@ -109,14 +126,15 @@ function computeTier2Key( return `attachment_or_url:${keyParts.join(',')}:${normalizedSubject}:${senderValue}`; } -/** Builds a `sender_subject_client:...` key from sender + subject + company (Tier 3). */ -function computeTier3Key( - requesterContactId: number | null, - normalizedSubject: string, - companyId: number | null -): string | null { - if (!requesterContactId || !normalizedSubject || !companyId) return null; - return `sender_subject_client:${requesterContactId}:${normalizedSubject}:${companyId}`; +/** + * Builds a `sender_subject_client:...` key from normalized subject + company + * (Tier 3) — deliberately company-wide, NOT scoped to a single reporting + * contact, so the same campaign reported by different employees at the same + * company still consolidates into one campaign (see file-level bug-fix note). + */ +function computeTier3Key(normalizedSubject: string, companyId: number | null): string | null { + if (!normalizedSubject || !companyId) return null; + return `sender_subject_client:${normalizedSubject}:${companyId}`; } /** @@ -174,7 +192,7 @@ export async function groupReportIntoCampaign( return await postgresClient.transaction(async (client) => { const ownReportRes = await client.query( - `SELECT title, requester_contact_id, company_id, created_at, campaign_id::text AS campaign_id + `SELECT title, company_id, created_at, campaign_id::text AS campaign_id FROM reports WHERE id = $1`, [reportId] @@ -311,24 +329,24 @@ export async function groupReportIntoCampaign( } // --------------------------------------------------------------------- - // Tier 3: sender + normalizeSubject(title) + client + 24h window - // (D-02). Joins reports.requester_contact_id -> contacts (Pitfall 5 — - // NOT `contact_id`). Self-exclusion (`r.id != $3`) required for the - // same reason as Tiers 1-2. + // Tier 3: normalizeSubject(title) + company + 24h window (D-02, fixed + // per file-level bug-fix note). Company-wide — deliberately NOT scoped + // to `reports.requester_contact_id` — so the same campaign reported by + // different employees at the same company still consolidates into one + // campaign. Self-exclusion (`r.id != $2`) required for the same reason + // as Tiers 1-2. // --------------------------------------------------------------------- - if (!matchCampaignId && ownReport.requester_contact_id && ownReport.company_id && normalizedSubject) { + if (!matchCampaignId && ownReport.company_id && normalizedSubject) { const tier3 = await client.query<{ campaign_id: string; title: string | null }>( `SELECT r.campaign_id::text AS campaign_id, r.title FROM reports r - JOIN contacts c ON c.id = r.requester_contact_id - WHERE r.requester_contact_id = $1 - AND r.company_id = $2 + WHERE r.company_id = $1 AND r.campaign_id IS NOT NULL - AND r.id != $3 - AND r.created_at BETWEEN $4::timestamptz - INTERVAL '24 hours' - AND $4::timestamptz + INTERVAL '24 hours' + AND r.id != $2 + AND r.created_at BETWEEN $3::timestamptz - INTERVAL '24 hours' + AND $3::timestamptz + INTERVAL '24 hours' ORDER BY r.created_at ASC`, - [ownReport.requester_contact_id, ownReport.company_id, reportId, ownReport.created_at] + [ownReport.company_id, reportId, ownReport.created_at] ); const match = tier3.rows.find((r) => normalizeSubject(r.title) === normalizedSubject); if (match) { @@ -349,11 +367,7 @@ export async function groupReportIntoCampaign( normalizedSubject, ownSenderValue ); - const tier3Key = computeTier3Key( - ownReport.requester_contact_id, - normalizedSubject, - ownReport.company_id - ); + const tier3Key = computeTier3Key(normalizedSubject, ownReport.company_id); const currentKeys = [tier1Key, tier2Key, tier3Key, `report:${reportId}`].filter( (k): k is string => k !== null );