fix(18-05): add own-campaign revalidation guard before create-new (CR-03)
Re-analyzing a single-report campaign (no sibling report exists yet) fell through to "create new campaign" because every tier query self-excludes the report's own row, so matchCampaignId stayed null and the report's still-valid campaign was abandoned in favor of a duplicate campaigns row sharing the same campaign_key. Add a guard that runs only when no sibling matched and the report already has campaign_id: recompute the report's current tier keys once, look up its own campaign row, and reuse it (created:false) when the stored campaign_key still matches. Falls through to create-new unchanged when the campaign row is gone or the signal has genuinely diverged. D-08 sibling upgrades are untouched (guard only runs when no sibling matched). - Tests A/B reproduce the exact CR-03 duplicate-campaign scenario and failed against the pre-fix code (confirmed during RED) - Tests C/D guard the create-new no-regression and signal-diverged fall-through paths
This commit is contained in:
parent
24aa571245
commit
633b48c3b2
2 changed files with 162 additions and 13 deletions
|
|
@ -70,6 +70,8 @@ interface MockRows {
|
|||
tier2Candidates?: unknown[];
|
||||
candidateIndicators?: unknown[];
|
||||
tier3?: unknown[];
|
||||
/** CR-03: rows for the own-campaign revalidation SELECT (campaigns WHERE id = ownReport.campaign_id). */
|
||||
ownCampaign?: unknown[];
|
||||
insertCampaign?: unknown[];
|
||||
}
|
||||
|
||||
|
|
@ -102,6 +104,9 @@ function makeClient(rows: MockRows) {
|
|||
if (sql.includes('BETWEEN $4::timestamptz')) {
|
||||
return { rows: rows.tier3 ?? [], rowCount: rows.tier3?.length ?? 0 };
|
||||
}
|
||||
if (sql.includes('SELECT campaign_key')) {
|
||||
return { rows: rows.ownCampaign ?? [], rowCount: rows.ownCampaign?.length ?? 0 };
|
||||
}
|
||||
if (sql.includes('UPDATE campaigns')) {
|
||||
return { rows: [], rowCount: 1 };
|
||||
}
|
||||
|
|
@ -360,4 +365,103 @@ describe('groupReportIntoCampaign', () => {
|
|||
expect(callsContaining('INSERT INTO campaigns')).toHaveLength(0);
|
||||
expect(callsContaining('UPDATE reports SET campaign_id')).toHaveLength(0);
|
||||
});
|
||||
|
||||
// ===========================================================================
|
||||
// CR-03 (gap closure, 18-05 Task 1): own-campaign revalidation guard.
|
||||
//
|
||||
// Root cause: every tier query above self-excludes the report's OWN row
|
||||
// (`r.id != $x`). For a single-report campaign (no sibling report exists
|
||||
// yet), all three tiers return zero rows on re-analyze, matchCampaignId
|
||||
// stays null, and — pre-fix — control fell straight into "create new
|
||||
// campaign", producing a SECOND campaigns row with the identical
|
||||
// campaign_key. Tests A/B below reproduce that exact scenario and FAILED
|
||||
// against the pre-fix code (confirmed during RED).
|
||||
// ===========================================================================
|
||||
|
||||
it('Test A (CR-03 core): single-report re-analyze via Tier-1 stored key reuses the report\'s own existing campaign with zero mutating queries', async () => {
|
||||
stage({
|
||||
ownReport: [{ ...REPORT_ROW, campaign_id: 'campaign-own' }],
|
||||
ownMessage: [{ id: 'own-msg-id', message_id: 'lone-message-id' }],
|
||||
tier1: [], // no sibling shares this message_id — single-report campaign
|
||||
ownIndicators: [],
|
||||
tier2Candidates: [],
|
||||
tier3: [],
|
||||
ownCampaign: [{ campaign_key: 'message_id:lone-message-id', group_method: 'message_id' }],
|
||||
});
|
||||
|
||||
const result = await groupReportIntoCampaign('report-own-a');
|
||||
|
||||
expect(result).toEqual({
|
||||
campaignId: 'campaign-own',
|
||||
groupMethod: 'message_id',
|
||||
created: false,
|
||||
});
|
||||
expect(callsContaining('INSERT INTO campaigns')).toHaveLength(0);
|
||||
expect(callsContaining('UPDATE campaigns')).toHaveLength(0);
|
||||
expect(callsContaining('UPDATE reports SET campaign_id')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('Test B (CR-03 Tier-3-only variant): single-report re-analyze via Tier-3 stored key reuses the report\'s own existing campaign with zero mutating queries', async () => {
|
||||
stage({
|
||||
ownReport: [{ ...REPORT_ROW, campaign_id: 'campaign-own' }],
|
||||
ownMessage: [],
|
||||
tier3: [],
|
||||
ownCampaign: [
|
||||
{ campaign_key: 'sender_subject_client:5:invoice alert:10', group_method: 'sender_subject_client' },
|
||||
],
|
||||
});
|
||||
|
||||
const result = await groupReportIntoCampaign('report-own-b');
|
||||
|
||||
expect(result).toEqual({
|
||||
campaignId: 'campaign-own',
|
||||
groupMethod: 'sender_subject_client',
|
||||
created: false,
|
||||
});
|
||||
expect(callsContaining('INSERT INTO campaigns')).toHaveLength(0);
|
||||
expect(callsContaining('UPDATE campaigns')).toHaveLength(0);
|
||||
expect(callsContaining('UPDATE reports SET campaign_id')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('Test C (no regression): campaign_id=null still creates exactly one campaign and never runs the own-campaign guard', async () => {
|
||||
stage({
|
||||
ownReport: [REPORT_ROW], // no campaign_id field => null/undefined, never grouped
|
||||
ownMessage: [],
|
||||
tier3: [],
|
||||
insertCampaign: [{ id: 'brand-new-campaign' }],
|
||||
});
|
||||
|
||||
const result = await groupReportIntoCampaign('report-own-c');
|
||||
|
||||
expect(result).toEqual({
|
||||
campaignId: 'brand-new-campaign',
|
||||
groupMethod: 'sender_subject_client',
|
||||
created: true,
|
||||
});
|
||||
expect(callsContaining('INSERT INTO campaigns')).toHaveLength(1);
|
||||
expect(callsContaining('UPDATE campaigns')).toHaveLength(0);
|
||||
// The own-campaign guard must not fire when there is no own campaign_id.
|
||||
expect(callsContaining('SELECT campaign_key')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('Test D (signal genuinely diverged): stored own-campaign key no longer matches current signal, falls through to create-new', async () => {
|
||||
stage({
|
||||
ownReport: [{ ...REPORT_ROW, campaign_id: 'campaign-own' }],
|
||||
ownMessage: [],
|
||||
tier3: [],
|
||||
ownCampaign: [
|
||||
{ campaign_key: 'sender_subject_client:5:old subject:10', group_method: 'sender_subject_client' },
|
||||
],
|
||||
insertCampaign: [{ id: 'diverged-new-campaign' }],
|
||||
});
|
||||
|
||||
const result = await groupReportIntoCampaign('report-own-d');
|
||||
|
||||
expect(result).toEqual({
|
||||
campaignId: 'diverged-new-campaign',
|
||||
groupMethod: 'sender_subject_client',
|
||||
created: true,
|
||||
});
|
||||
expect(callsContaining('INSERT INTO campaigns')).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -318,6 +318,27 @@ export async function groupReportIntoCampaign(
|
|||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// Own-signal tier keys, computed once here and reused by both the
|
||||
// own-campaign revalidation guard (CR-03, below) and the create-new
|
||||
// fall-through further down — avoids computing them twice.
|
||||
// ---------------------------------------------------------------------
|
||||
const tier1Key = computeTier1Key(ownMessage?.message_id ?? null);
|
||||
const tier2Key = computeTier2Key(
|
||||
ownAttachmentHashes,
|
||||
ownUrlDomains,
|
||||
normalizedSubject,
|
||||
ownSenderValue
|
||||
);
|
||||
const tier3Key = computeTier3Key(
|
||||
ownReport.requester_contact_id,
|
||||
normalizedSubject,
|
||||
ownReport.company_id
|
||||
);
|
||||
const currentKeys = [tier1Key, tier2Key, tier3Key, `report:${reportId}`].filter(
|
||||
(k): k is string => k !== null
|
||||
);
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// Find-or-create against `campaigns` (CAMP-02).
|
||||
// ---------------------------------------------------------------------
|
||||
|
|
@ -345,23 +366,47 @@ export async function groupReportIntoCampaign(
|
|||
return { campaignId: matchCampaignId, groupMethod: matchGroupMethod, created: false };
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// CR-03: no sibling matched on any tier (matchCampaignId is null).
|
||||
// Every tier query above self-excludes the report's own row
|
||||
// (`r.id != $x`), so a single-report campaign (no sibling report
|
||||
// exists yet — the common case for any brand-new phishing lure before
|
||||
// a second person reports it) always reaches here with matchCampaignId
|
||||
// still null. Pre-fix, control fell straight into "create new
|
||||
// campaign" below, abandoning the report's still-valid campaign and
|
||||
// producing a SECOND campaigns row with the identical campaign_key on
|
||||
// every repeat /analyze call. This guard sits OUTSIDE the sibling-
|
||||
// match branch above precisely because it must only run when NO
|
||||
// sibling matched — it never suppresses a legitimate D-08 sibling
|
||||
// upgrade, which returns from the branch above instead.
|
||||
// ---------------------------------------------------------------------
|
||||
if (!matchCampaignId && ownReport.campaign_id) {
|
||||
const ownCampaignRes = await client.query<{ campaign_key: string; group_method: string }>(
|
||||
`SELECT campaign_key, group_method
|
||||
FROM campaigns
|
||||
WHERE id = $1`,
|
||||
[ownReport.campaign_id]
|
||||
);
|
||||
const ownCampaign = ownCampaignRes.rows[0];
|
||||
if (ownCampaign && currentKeys.includes(ownCampaign.campaign_key)) {
|
||||
// The report's own campaign still satisfies its freshly-recomputed
|
||||
// current tier key — it's still validly linked, reuse it as-is.
|
||||
return {
|
||||
campaignId: ownReport.campaign_id,
|
||||
groupMethod: ownCampaign.group_method as GroupReportResult['groupMethod'],
|
||||
created: false,
|
||||
};
|
||||
}
|
||||
// Own campaign row is gone, or the report's signal has genuinely
|
||||
// diverged from its stored key — fall through to create-new below.
|
||||
// (CR-02: decrementing this now-abandoned origin campaign's
|
||||
// report_count is handled in Task 2 of the 18-05 gap-closure plan.)
|
||||
}
|
||||
|
||||
// No match on any tier — create a new campaign. Prefer the strongest
|
||||
// available key (Tier 1 > Tier 2 > Tier 3) so future duplicates of THIS
|
||||
// report can match it; do NOT set/transition campaigns.status (leave
|
||||
// migration default 'open' — Phase 19/20's concern).
|
||||
const tier1Key = computeTier1Key(ownMessage?.message_id ?? null);
|
||||
const tier2Key = computeTier2Key(
|
||||
ownAttachmentHashes,
|
||||
ownUrlDomains,
|
||||
normalizedSubject,
|
||||
ownSenderValue
|
||||
);
|
||||
const tier3Key = computeTier3Key(
|
||||
ownReport.requester_contact_id,
|
||||
normalizedSubject,
|
||||
ownReport.company_id
|
||||
);
|
||||
|
||||
let newCampaignKey: string;
|
||||
let newGroupMethod: GroupReportResult['groupMethod'];
|
||||
if (tier1Key) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue