fix(phishing-recipient-seubert): scope campaign grouping Tier 3 to company, not reporting contact
Tier 3 is the only tier automatic (webhook-triggered) grouping ever reaches, since grouping runs before message parsing. It was scoped to reports.requester_contact_id, so the same campaign reported by different employees at the same company never consolidated into one campaign — each report's evidence/blast-radius view silently under-reported the campaign's true recipients. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
e9478101a3
commit
5f308f836f
2 changed files with 85 additions and 42 deletions
|
|
@ -83,7 +83,7 @@ function makeClient(rows: MockRows) {
|
||||||
query: vi.fn(async (sql: string, params?: unknown[]) => {
|
query: vi.fn(async (sql: string, params?: unknown[]) => {
|
||||||
clientCalls.push({ sql, params: params ?? [] });
|
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 };
|
return { rows: rows.ownReport ?? [], rowCount: rows.ownReport?.length ?? 0 };
|
||||||
}
|
}
|
||||||
if (sql.includes('FROM messages') && sql.includes('WHERE report_id = $1')) {
|
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')) {
|
if (sql.includes('message_id = ANY')) {
|
||||||
return { rows: rows.candidateIndicators ?? [], rowCount: rows.candidateIndicators?.length ?? 0 };
|
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 };
|
return { rows: rows.tier3 ?? [], rowCount: rows.tier3?.length ?? 0 };
|
||||||
}
|
}
|
||||||
if (sql.includes('SELECT campaign_key')) {
|
if (sql.includes('SELECT campaign_key')) {
|
||||||
|
|
@ -140,7 +140,6 @@ describe('groupReportIntoCampaign', () => {
|
||||||
|
|
||||||
const REPORT_ROW = {
|
const REPORT_ROW = {
|
||||||
title: 'Re: Invoice Alert',
|
title: 'Re: Invoice Alert',
|
||||||
requester_contact_id: 5,
|
|
||||||
company_id: 10,
|
company_id: 10,
|
||||||
created_at: '2026-07-15T10:00:00Z',
|
created_at: '2026-07-15T10:00:00Z',
|
||||||
};
|
};
|
||||||
|
|
@ -173,6 +172,36 @@ describe('groupReportIntoCampaign', () => {
|
||||||
expect(callsContaining('INSERT INTO campaigns')).toHaveLength(0);
|
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 () => {
|
it('creates exactly one new campaign when no tier matches anything', async () => {
|
||||||
stage({
|
stage({
|
||||||
ownReport: [REPORT_ROW],
|
ownReport: [REPORT_ROW],
|
||||||
|
|
@ -192,7 +221,7 @@ describe('groupReportIntoCampaign', () => {
|
||||||
const insertCalls = callsContaining('INSERT INTO campaigns');
|
const insertCalls = callsContaining('INSERT INTO campaigns');
|
||||||
expect(insertCalls).toHaveLength(1);
|
expect(insertCalls).toHaveLength(1);
|
||||||
expect(insertCalls[0].params).toEqual([
|
expect(insertCalls[0].params).toEqual([
|
||||||
'sender_subject_client:5:invoice alert:10',
|
'sender_subject_client:invoice alert:10',
|
||||||
'sender_subject_client',
|
'sender_subject_client',
|
||||||
]);
|
]);
|
||||||
expect(callsContaining('UPDATE campaigns')).toHaveLength(0);
|
expect(callsContaining('UPDATE campaigns')).toHaveLength(0);
|
||||||
|
|
@ -407,7 +436,7 @@ describe('groupReportIntoCampaign', () => {
|
||||||
ownMessage: [],
|
ownMessage: [],
|
||||||
tier3: [],
|
tier3: [],
|
||||||
ownCampaign: [
|
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: [],
|
ownMessage: [],
|
||||||
tier3: [],
|
tier3: [],
|
||||||
ownCampaign: [
|
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' }],
|
insertCampaign: [{ id: 'diverged-new-campaign' }],
|
||||||
});
|
});
|
||||||
|
|
@ -532,7 +561,7 @@ describe('groupReportIntoCampaign', () => {
|
||||||
ownMessage: [],
|
ownMessage: [],
|
||||||
tier3: [],
|
tier3: [],
|
||||||
ownCampaign: [
|
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' }],
|
insertCampaign: [{ id: 'diverged-new-campaign-h' }],
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -10,14 +10,32 @@
|
||||||
* logic between callers, mirroring `phishing-detector.ts`'s shared-core
|
* logic between callers, mirroring `phishing-detector.ts`'s shared-core
|
||||||
* architecture.
|
* architecture.
|
||||||
*
|
*
|
||||||
* D-07 limitation (load-bearing, stated explicitly): `parseAndStoreMessage`
|
* D-07 limitation (load-bearing, stated explicitly): `groupReportIntoCampaign`
|
||||||
* (the only writer of `messages`/`indicators` rows — Phase 16) is not wired
|
* always runs BEFORE `parseAndStoreMessage` on the automatic webhook path
|
||||||
* into the automatic webhook/cron path this phase. That means the automatic
|
* (see `webhook-service.ts`'s `triggerPhishingDetection()` — grouping happens
|
||||||
* path only ever has `reports`/`contacts` data available, so Tier 1
|
* first, parsing happens afterward inside `runGatedPhishingStages()`). That
|
||||||
* (Message-ID) and Tier 2 (attachment-hash/URL-domain) can only ever match
|
* means at grouping time the CURRENT report never has its own `messages`/
|
||||||
* for a report that has already been through an explicit `/analyze` call at
|
* `indicators` row yet, so Tier 1 (Message-ID) and Tier 2 (attachment-hash/
|
||||||
* least once. Until then, automatic grouping effectively only reaches
|
* URL-domain) — both of which require the report's OWN signal to search
|
||||||
* Tier 3 (sender + normalized subject + client + 24h window).
|
* 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';
|
import type { PoolClient } from 'pg';
|
||||||
|
|
@ -64,7 +82,6 @@ export interface GroupReportResult {
|
||||||
|
|
||||||
interface OwnReportRow {
|
interface OwnReportRow {
|
||||||
title: string | null;
|
title: string | null;
|
||||||
requester_contact_id: number | null;
|
|
||||||
company_id: number | null;
|
company_id: number | null;
|
||||||
created_at: string;
|
created_at: string;
|
||||||
campaign_id: string | null;
|
campaign_id: string | null;
|
||||||
|
|
@ -109,14 +126,15 @@ function computeTier2Key(
|
||||||
return `attachment_or_url:${keyParts.join(',')}:${normalizedSubject}:${senderValue}`;
|
return `attachment_or_url:${keyParts.join(',')}:${normalizedSubject}:${senderValue}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Builds a `sender_subject_client:...` key from sender + subject + company (Tier 3). */
|
/**
|
||||||
function computeTier3Key(
|
* Builds a `sender_subject_client:...` key from normalized subject + company
|
||||||
requesterContactId: number | null,
|
* (Tier 3) — deliberately company-wide, NOT scoped to a single reporting
|
||||||
normalizedSubject: string,
|
* contact, so the same campaign reported by different employees at the same
|
||||||
companyId: number | null
|
* company still consolidates into one campaign (see file-level bug-fix note).
|
||||||
): string | null {
|
*/
|
||||||
if (!requesterContactId || !normalizedSubject || !companyId) return null;
|
function computeTier3Key(normalizedSubject: string, companyId: number | null): string | null {
|
||||||
return `sender_subject_client:${requesterContactId}:${normalizedSubject}:${companyId}`;
|
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) => {
|
return await postgresClient.transaction(async (client) => {
|
||||||
const ownReportRes = await client.query<OwnReportRow>(
|
const ownReportRes = await client.query<OwnReportRow>(
|
||||||
`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
|
FROM reports
|
||||||
WHERE id = $1`,
|
WHERE id = $1`,
|
||||||
[reportId]
|
[reportId]
|
||||||
|
|
@ -311,24 +329,24 @@ export async function groupReportIntoCampaign(
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------
|
// ---------------------------------------------------------------------
|
||||||
// Tier 3: sender + normalizeSubject(title) + client + 24h window
|
// Tier 3: normalizeSubject(title) + company + 24h window (D-02, fixed
|
||||||
// (D-02). Joins reports.requester_contact_id -> contacts (Pitfall 5 —
|
// per file-level bug-fix note). Company-wide — deliberately NOT scoped
|
||||||
// NOT `contact_id`). Self-exclusion (`r.id != $3`) required for the
|
// to `reports.requester_contact_id` — so the same campaign reported by
|
||||||
// same reason as Tiers 1-2.
|
// 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 }>(
|
const tier3 = await client.query<{ campaign_id: string; title: string | null }>(
|
||||||
`SELECT r.campaign_id::text AS campaign_id, r.title
|
`SELECT r.campaign_id::text AS campaign_id, r.title
|
||||||
FROM reports r
|
FROM reports r
|
||||||
JOIN contacts c ON c.id = r.requester_contact_id
|
WHERE r.company_id = $1
|
||||||
WHERE r.requester_contact_id = $1
|
|
||||||
AND r.company_id = $2
|
|
||||||
AND r.campaign_id IS NOT NULL
|
AND r.campaign_id IS NOT NULL
|
||||||
AND r.id != $3
|
AND r.id != $2
|
||||||
AND r.created_at BETWEEN $4::timestamptz - INTERVAL '24 hours'
|
AND r.created_at BETWEEN $3::timestamptz - INTERVAL '24 hours'
|
||||||
AND $4::timestamptz + INTERVAL '24 hours'
|
AND $3::timestamptz + INTERVAL '24 hours'
|
||||||
ORDER BY r.created_at ASC`,
|
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);
|
const match = tier3.rows.find((r) => normalizeSubject(r.title) === normalizedSubject);
|
||||||
if (match) {
|
if (match) {
|
||||||
|
|
@ -349,11 +367,7 @@ export async function groupReportIntoCampaign(
|
||||||
normalizedSubject,
|
normalizedSubject,
|
||||||
ownSenderValue
|
ownSenderValue
|
||||||
);
|
);
|
||||||
const tier3Key = computeTier3Key(
|
const tier3Key = computeTier3Key(normalizedSubject, ownReport.company_id);
|
||||||
ownReport.requester_contact_id,
|
|
||||||
normalizedSubject,
|
|
||||||
ownReport.company_id
|
|
||||||
);
|
|
||||||
const currentKeys = [tier1Key, tier2Key, tier3Key, `report:${reportId}`].filter(
|
const currentKeys = [tier1Key, tier2Key, tier3Key, `report:${reportId}`].filter(
|
||||||
(k): k is string => k !== null
|
(k): k is string => k !== null
|
||||||
);
|
);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue