diff --git a/.planning/phases/18-campaign-grouping-phishing-analysis-api/18-PATTERNS.md b/.planning/phases/18-campaign-grouping-phishing-analysis-api/18-PATTERNS.md new file mode 100644 index 0000000..5593d2a --- /dev/null +++ b/.planning/phases/18-campaign-grouping-phishing-analysis-api/18-PATTERNS.md @@ -0,0 +1,590 @@ +# Phase 18: Campaign Grouping & Phishing Analysis API — Pattern Map + +**Mapped:** 2026-07-15 +**Purpose:** For each file this phase creates/modifies, identify role, data flow, closest existing analog, and concrete code to replicate. Planner and executor should treat this as the primary "how do I write this" reference — RESEARCH.md remains the source for *why* (tiered-key semantics, D-01..D-08 rationale). + +--- + +## 1. `lib/services/campaign-grouping-service.ts` (NEW) + +**Role:** Pure-ish service function — the shared grouping core called from 3 call sites (webhook, cron sweep, `/analyze` route). Owns tiered-key computation + campaign find-or-create. + +**Data flow:** `reports` row (+ optionally `messages`/`indicators` rows if `parseAndStoreMessage` already ran) → tiered match queries against `campaigns`/`reports`/`contacts` → either `UPDATE campaigns` (bump `report_count`/`last_seen_at`) + `UPDATE reports SET campaign_id` on match, or `INSERT INTO campaigns` + link. All inside one `postgresClient.transaction()`. + +**Closest analog:** `lib/services/phishing-detector.ts` — same file shape (pure hashing/matching helper functions exported alongside one DB-touching orchestration function), same "shared core called from 3 sites" architecture, same `console.error` + rethrow error handling. + +### Exact structure to replicate (from `phishing-detector.ts`) + +```typescript +import { postgresClient } from './postgres-client'; + +// ============================================================================= +// Pure logic — tier-key helpers (mirrors matchesPhishingPatterns / computePhishingContentHash) +// ============================================================================= + +/** + * D-03: strip leading Re:/Fwd:/Fw: (repeated, case-insensitive), lowercase, trim. + */ +export function normalizeSubject(subject: string | null): string { + let s = (subject ?? '').trim(); + const prefixRe = /^(re|fwd|fw):\s*/i; + while (prefixRe.test(s)) { + s = s.replace(prefixRe, '').trim(); + } + return s.toLowerCase(); +} + +/** + * Pitfall 4: indicators.value for indicator_type='url' is a bare URL string, + * not a domain — extract at read time, guarded (malformed/relative URLs are + * possible in real-world phishing emails). + */ +export function extractUrlDomain(url: string): string | null { + try { + return new URL(url).hostname || null; + } catch { + return null; + } +} + +// ============================================================================= +// Orchestration — groupReportIntoCampaign (mirrors detectPhishingTicket) +// ============================================================================= + +export interface GroupReportResult { + campaignId: string; + groupMethod: 'message_id' | 'attachment_or_url' | 'sender_subject_client'; + created: boolean; +} + +export async function groupReportIntoCampaign( + reportId: string, + opts?: { skipIfAlreadyGrouped?: boolean } +): Promise { + try { + // D-08: automatic path (webhook/cron) passes skipIfAlreadyGrouped=true; + // /analyze route omits it (always re-run, may upgrade Tier-3 -> Tier-1). + if (opts?.skipIfAlreadyGrouped) { + const existing = await postgresClient.query<{ campaign_id: string | null }>( + `SELECT campaign_id::text AS campaign_id FROM reports WHERE id = $1`, + [reportId] + ); + if (existing.rows[0]?.campaign_id) { + return null; // already grouped, short-circuit (Pitfall/D-08) + } + } + + return await postgresClient.transaction(async (client) => { + // Tier 1 (Message-ID), Tier 2 (attachment-hash/URL-domain + subject + + // sender + 24h), Tier 3 (sender + normalizeSubject + client + 24h) — + // each gated on the previous tier finding nothing. See RESEARCH.md + // Pattern 2 for the exact query shape per tier and Pitfall 5 for the + // reports.requester_contact_id join column name. + // ... tier queries here, using client.query() (NOT postgresClient.query) + // so all reads/writes share the transaction ... + + // Find-or-create against `campaigns`: + // UPDATE campaigns SET report_count = report_count + 1, last_seen_at = NOW() + // WHERE id = $1 RETURNING id + // or + // INSERT INTO campaigns (campaign_key, group_method, first_seen_at, + // last_seen_at, report_count) VALUES ($1, $2, NOW(), NOW(), 1) + // RETURNING id::text AS id + // Then always: + // UPDATE reports SET campaign_id = $1, updated_at = NOW() WHERE id = $2 + + throw new Error('implement tier matching'); // placeholder — planner fills in + }); + } catch (error) { + console.error('[CAMPAIGN-GROUPING] Failed to group report into campaign', reportId, error); + throw error; + } +} +``` + +**D-07 comment requirement:** Document explicitly in this file (not silently built around) that the automatic webhook/cron path only ever reaches Tier 3 until a report has been through an explicit `/analyze` call at least once (no `messages`/`indicators` rows exist otherwise). Mirror the doc-comment style at the top of `phishing-eml-service.ts:1-22` (the "Hard invariant" / "live on-demand trigger" comment block) — same project convention of stating a load-bearing limitation directly above the export. + +**Schema facts this file must respect** (from `migrations/097_phishing_triage_schema.sql`): +- `campaigns` columns: `id UUID PK`, `campaign_key TEXT`, `group_method TEXT`, `first_seen_at TIMESTAMPTZ`, `last_seen_at TIMESTAMPTZ`, `report_count INTEGER DEFAULT 0`, `status TEXT DEFAULT 'open'`, `created_at`, `updated_at`. **No UNIQUE constraint on `campaign_key`** (only a plain index) — this is why the transaction wrapper matters (Pitfall 2). +- `reports.campaign_id` — nullable FK to `campaigns(id)`, `reports.requester_contact_id` is the sender-join column (NOT `contact_id` — Pitfall 5). +- `messages.report_id`, `messages.message_id` (nullable TEXT) — Tier 1 key source. +- `indicators.message_id`, `indicators.indicator_type` (`'attachment_hash' | 'url' | 'sender'`), `indicators.value`, `indicators.metadata` (added in migration 099) — Tier 2 key source. `value` for `'url'` rows is the **raw URL string**, not a domain (Pitfall 4) — extract with `extractUrlDomain()` at read time. +- `contacts.email_address` (VARCHAR) — join target for Tier 3's "sender"/recipient resolution, via `reports.requester_contact_id = contacts.id`. + +--- + +## 2. `lib/services/campaign-grouping-service.test.ts` (NEW) + +**Role:** Unit test for the pure helpers + mocked-DB orchestration test for `groupReportIntoCampaign`. + +**Closest analogs (two, for two different needs):** +- `lib/services/phishing-detector.test.ts` — for `normalizeSubject`/`extractUrlDomain` pure-function tests: **no mocking at all**, plain `describe`/`it`/`expect` against exported pure functions. +- `lib/services/phishing-eml-service.test.ts` — for `groupReportIntoCampaign`'s DB-touching behavior: mocks `postgresClient` BEFORE importing the module under test, using `vi.mock('./postgres-client', ...)` + a `queryMock` spy, then asserts against `queryMock.mock.calls`. + +### Exact mocking pattern to replicate + +```typescript +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Mock postgresClient BEFORE importing the module under test. +const queryMock = vi.fn(); +const transactionMock = vi.fn(); +vi.mock('./postgres-client', () => ({ + postgresClient: { + query: (...args: unknown[]) => queryMock(...args), + transaction: (...args: unknown[]) => transactionMock(...args), + }, +})); + +import { normalizeSubject, extractUrlDomain, groupReportIntoCampaign } from './campaign-grouping-service'; + +describe('normalizeSubject', () => { + it('strips a single Re: prefix, lowercases, trims', () => { + expect(normalizeSubject('Re: Your Invoice ')).toBe('your invoice'); + }); + it('strips repeated Re:/Fwd:/Fw: prefixes case-insensitively', () => { + expect(normalizeSubject('FW: Re: fwd: Urgent Payment')).toBe('urgent payment'); + }); + it('handles null', () => { + expect(normalizeSubject(null)).toBe(''); + }); +}); + +describe('extractUrlDomain', () => { + it('extracts hostname from a full URL', () => { + expect(extractUrlDomain('https://evil.example.com/path?x=1')).toBe('evil.example.com'); + }); + it('returns null for a malformed URL instead of throwing', () => { + expect(extractUrlDomain('not-a-url')).toBeNull(); + }); +}); + +describe('groupReportIntoCampaign', () => { + beforeEach(() => { + queryMock.mockReset(); + transactionMock.mockReset(); + }); + // ... transaction-mocked find-or-create tests (CAMP-01/CAMP-02) — see + // RESEARCH.md's Phase Requirements -> Test Map for the exact 3 required + // behaviors: tiered match order, subject normalization, report_count + // increment without creating a second campaign. +}); +``` + +**Fixtures:** reuse `lib/services/eml-parser.fixtures.ts` synthetic fixtures if a Message-ID-bearing test case is needed (no real customer `.eml` content — banned per milestone Out-of-Scope). No new fixture file required. + +**Run command:** `npx vitest run lib/services/campaign-grouping-service.test.ts` (config confirmed at `vitest.config.ts`: `include: ['lib/**/*.test.ts']` — this file's location under `lib/services/` is required for the test runner to discover it; a route-handler test would NOT be discovered). + +--- + +## 3. `app/api/phishing/tickets/[ticket_id]/analyze/route.ts` (NEW) + +**Role:** POST route — on-demand trigger for one ticket. Orchestrates `detectPhishingTicket` → `parseAndStoreMessage` → `groupReportIntoCampaign` (always re-run, no short-circuit per D-08) and returns the resulting campaign linkage. + +**Data flow:** `ticket_id` path param → look up ticket row (mirror `webhook-service.ts`'s "read the ticket row back from Postgres" pattern, since this route also doesn't have an Autotask entity payload handed to it) → `detectPhishingTicket(ticket)` → `parseAndStoreMessage({ reportId, ticketId })` → `groupReportIntoCampaign(reportId)` (no `skipIfAlreadyGrouped`) → JSON response. + +**Closest analogs:** +- `app/api/tickets/[id]/route.ts` and `app/api/workflow/executions/[id]/route.ts` — Next.js 16 dynamic route `params: Promise<>` convention (CONFIRMED, not legacy sync params). +- `app/api/rmm/executions/route.ts`'s `POST` handler — `requirePermission()` call-and-early-return + Zod body validation (not needed here — no body) + try/catch with client-vs-server error status split. + +### Exact route shape to replicate + +```typescript +import { NextRequest, NextResponse } from 'next/server'; +import { requirePermission } from '@/lib/auth-utils'; +import postgresClient from '@/lib/services/postgres-client'; +import { detectPhishingTicket, type DetectableTicket } from '@/lib/services/phishing-detector'; +import { parseAndStoreMessage } from '@/lib/services/phishing-eml-service'; +import { groupReportIntoCampaign } from '@/lib/services/campaign-grouping-service'; + +export async function POST( + request: NextRequest, + { params }: { params: Promise<{ ticket_id: string }> } +) { + const { error } = await requirePermission('phishing', 'analyze'); + if (error) return error; + + const { ticket_id } = await params; + const ticketId = Number(ticket_id); + if (!Number.isFinite(ticketId)) { + return NextResponse.json({ error: 'Invalid ticket_id' }, { status: 400 }); + } + + try { + const row = await postgresClient.query<{ + id: string; ticket_number: string | null; title: string | null; + description: string | null; company_id: number | null; + contact_id: number | null; created_by_contact_id: number | null; + }>( + `SELECT id, ticket_number, title, description, company_id, contact_id, created_by_contact_id + FROM tickets WHERE id = $1`, + [ticketId] + ); + const r = row.rows[0]; + if (!r) { + return NextResponse.json({ error: 'Ticket not found' }, { status: 404 }); + } + + const ticket: DetectableTicket = { + id: Number(r.id), ticket_number: r.ticket_number, title: r.title, + description: r.description, company_id: r.company_id, + contact_id: r.contact_id, created_by_contact_id: r.created_by_contact_id, + }; + + const detection = await detectPhishingTicket(ticket); + if (!detection.flagged || !detection.reportId) { + return NextResponse.json({ error: 'Ticket does not match known phishing patterns' }, { status: 400 }); + } + + await parseAndStoreMessage({ reportId: detection.reportId, ticketId }); + // D-08: /analyze always re-runs grouping unconditionally (no skipIfAlreadyGrouped). + const grouped = await groupReportIntoCampaign(detection.reportId); + + return NextResponse.json({ + reportId: detection.reportId, + campaignId: grouped?.campaignId ?? null, + groupMethod: grouped?.groupMethod ?? null, + created: grouped?.created ?? false, + }); + } catch (err) { + console.error('[PHISHING-ANALYZE] Failed to analyze ticket', ticketId, err); + return NextResponse.json( + { error: 'Failed to analyze ticket', message: err instanceof Error ? err.message : 'Unknown error' }, + { status: 500 } + ); + } +} +``` + +**Note on `parseAndStoreMessage`'s return:** it never throws for "expected" no-op cases (`{ stored: false, reason: 'no-eml-attachment' | 'no-attachment-content' | 'oversized-attachment' }`) — the route should NOT treat `stored: false` as an error; grouping still proceeds (it'll just stay at whatever tier is reachable from `reports`/`contacts` alone). Only an actual thrown error (Autotask failure, DB failure) should produce a 500. + +--- + +## 4. `app/api/phishing/campaigns/route.ts` (NEW) + +**Role:** GET route — paginated campaign list with per-campaign report counts. + +**Data flow:** query params (`limit`/`offset`/optional `status` filter) → `SELECT ... FROM campaigns` (paginated) → camelCase JSON response `{ items, total, limit, offset }`. + +**Closest analog:** `app/api/admin/device-link-conflicts/route.ts` for the overall shape (`requirePermission` → parse/clamp `limit`/`offset` → build optional `WHERE` filter with parameterized placeholders → `COUNT(*)` query → manual camelCase `.map()`). The simpler `app/api/workflow/executions/route.ts` list pattern is a weaker analog — RESEARCH.md flags it explicitly as **not** camelCase-transforming its output; do not copy that part. + +### Exact shape to replicate + +```typescript +import { NextRequest, NextResponse } from 'next/server'; +import { requirePermission } from '@/lib/auth-utils'; +import postgresClient from '@/lib/services/postgres-client'; + +interface CampaignRow { + id: string; + campaign_key: string | null; + group_method: string | null; + first_seen_at: string | null; + last_seen_at: string | null; + report_count: number; + status: string; + created_at: string; +} + +export async function GET(request: NextRequest) { + const { error } = await requirePermission('phishing', 'read'); + if (error) return error; + + const url = request.nextUrl; + const limit = Math.min(parseInt(url.searchParams.get('limit') ?? '50', 10) || 50, 200); + const offset = Math.max(parseInt(url.searchParams.get('offset') ?? '0', 10) || 0, 0); + const status = url.searchParams.get('status'); + + const params: unknown[] = [limit, offset]; + let statusFilter = ''; + if (status) { + params.push(status); + statusFilter = `WHERE status = $${params.length}`; + } + + const campaigns = await postgresClient.query( + `SELECT id::text, campaign_key, group_method, first_seen_at::text, last_seen_at::text, + report_count, status, created_at::text + FROM campaigns + ${statusFilter} + ORDER BY last_seen_at DESC NULLS LAST + LIMIT $1 OFFSET $2`, + params + ); + + const totalRes = await postgresClient.query<{ count: string }>( + `SELECT COUNT(*)::text AS count FROM campaigns ${statusFilter}`, + status ? [status] : [] + ); + const total = parseInt(totalRes.rows[0]?.count ?? '0', 10); + + const items = campaigns.rows.map((c) => ({ + id: c.id, + campaignKey: c.campaign_key, + groupMethod: c.group_method, + firstSeenAt: c.first_seen_at, + lastSeenAt: c.last_seen_at, + reportCount: c.report_count, + status: c.status, + createdAt: c.created_at, + })); + + return NextResponse.json({ items, total, limit, offset }); +} +``` + +**Security note (V5 Input Validation):** the `status` filter value is never string-interpolated — it always goes through a `$n` placeholder, exactly matching `device-link-conflicts/route.ts`'s `sourceFilter` pattern (the placeholder index is interpolated, never the value). + +--- + +## 5. `app/api/phishing/campaigns/[id]/route.ts` (NEW) + +**Role:** GET route — single campaign detail with nested linked reports, messages, indicators, classification history (classifications table is a Phase 19 stub — expect it to be empty this phase, but the query/response shape should already include it per CAMP-03's requirement). + +**Data flow:** `id` (UUID) path param → parent `campaigns` query → bulk-fetch linked `reports` (by `campaign_id`) → bulk-fetch `messages` (by `report_id IN (...)`) → bulk-fetch `indicators` (by `message_id IN (...)`) → bulk-fetch `classifications` (by `campaign_id`) → assemble nested camelCase JSON via `Map`-keyed joins in application code. + +**Closest analog:** `app/api/admin/device-link-conflicts/route.ts` — this is the **exact pattern to replicate** per RESEARCH.md Pattern 3 ("Bulk-Fetch Nested Detail"): one parent query, one-or-more bulk child queries keyed by an ID array (`= ANY($1::...[])`), a `Map` for O(1) lookup, then a final `.map()` building the camelCase shape. Combine with `app/api/tickets/[id]/route.ts` / `app/api/workflow/executions/[id]/route.ts` for the `params: Promise<{ id: string }>` dynamic-route convention and the "not found → 404" early return. + +### Exact shape to replicate + +```typescript +import { NextRequest, NextResponse } from 'next/server'; +import { requirePermission } from '@/lib/auth-utils'; +import postgresClient from '@/lib/services/postgres-client'; + +export async function GET( + request: NextRequest, + { params }: { params: Promise<{ id: string }> } +) { + const { error } = await requirePermission('phishing', 'read'); + if (error) return error; + + const { id } = await params; + // V5: validate UUID shape before querying — malformed UUID would otherwise + // surface as an unhandled Postgres error -> uncaught 500. + const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!UUID_RE.test(id)) { + return NextResponse.json({ error: 'Invalid campaign id' }, { status: 400 }); + } + + const campaignRes = await postgresClient.query( + `SELECT id::text, campaign_key, group_method, first_seen_at::text, + last_seen_at::text, report_count, status, created_at::text, updated_at::text + FROM campaigns WHERE id = $1`, + [id] + ); + const campaign = campaignRes.rows[0]; + if (!campaign) { + return NextResponse.json({ error: 'Campaign not found' }, { status: 404 }); + } + + // Bulk-fetch linked reports (+ join contacts for recipient email — Pitfall 3: + // campaigns has no recipients column, must be derived via this join). + const reportsRes = await postgresClient.query( + `SELECT r.id::text, r.ticket_id::text, r.ticket_number, r.company_name, + r.title, r.created_at::text, + c.email_address AS requester_email + FROM reports r + LEFT JOIN contacts c ON c.id = r.requester_contact_id + WHERE r.campaign_id = $1 + ORDER BY r.created_at ASC`, + [id] + ); + const reportIds = reportsRes.rows.map((r) => r.id); + + // Bulk-fetch messages keyed by report_id array. + const messagesRes = reportIds.length + ? await postgresClient.query( + `SELECT id::text, report_id::text, message_id, subject FROM messages WHERE report_id = ANY($1::uuid[])`, + [reportIds] + ) + : { rows: [] as any[] }; + const messageIds = messagesRes.rows.map((m) => m.id); + + // Bulk-fetch indicators keyed by message_id array. + const indicatorsRes = messageIds.length + ? await postgresClient.query( + `SELECT id::text, message_id::text, indicator_type, value, metadata FROM indicators WHERE message_id = ANY($1::uuid[])`, + [messageIds] + ) + : { rows: [] as any[] }; + + // Classifications (Phase 19 stub — likely empty this phase). + const classificationsRes = await postgresClient.query( + `SELECT id::text, verdict, confidence, summary, created_at::text FROM classifications WHERE campaign_id = $1 ORDER BY created_at DESC`, + [id] + ); + + return NextResponse.json({ + id: campaign.id, + campaignKey: campaign.campaign_key, + groupMethod: campaign.group_method, + firstSeenAt: campaign.first_seen_at, + lastSeenAt: campaign.last_seen_at, + reportCount: campaign.report_count, + status: campaign.status, + createdAt: campaign.created_at, + updatedAt: campaign.updated_at, + reports: reportsRes.rows.map((r) => ({ + id: r.id, ticketId: r.ticket_id, ticketNumber: r.ticket_number, + companyName: r.company_name, title: r.title, createdAt: r.created_at, + requesterEmail: r.requester_email, + })), + messages: messagesRes.rows.map((m) => ({ + id: m.id, reportId: m.report_id, messageId: m.message_id, subject: m.subject, + })), + indicators: indicatorsRes.rows.map((i) => ({ + id: i.id, messageId: i.message_id, indicatorType: i.indicator_type, + value: i.value, metadata: i.metadata, + })), + classifications: classificationsRes.rows.map((c) => ({ + id: c.id, verdict: c.verdict, confidence: c.confidence, + summary: c.summary, createdAt: c.created_at, + })), + }); +} +``` + +Note `messages` schema doesn't have a `subject` column directly (subject lives in `messages.headers` JSONB per `phishing-eml-service.ts:128-140`) — adjust the query to pull `headers->>'subject' AS subject` if a top-level subject field is wanted in the response; the excerpt above is illustrative of the join/assembly shape, confirm exact JSONB path during implementation. + +--- + +## 6. `lib/permissions.ts` (MODIFIED) + +**Role:** Add the `phishing` resource + role grants (D-05). + +**Closest analog:** the existing `rmm: ["read", "execute"]` resource — identical read/action-verb shape. + +### Exact diff shape + +```typescript +// In `statement` (mirrors the `rmm` entry): +export const statement = { + // ...existing entries... + rmm: ["read", "execute"], + phishing: ["read", "analyze", "approve", "remediate"], // D-05: full vocabulary now +} as const; + +// In `superAdminRole`: +export const superAdminRole = ac.newRole({ + // ...existing entries... + rmm: ["read", "execute"], + phishing: ["read", "analyze"], // approve/remediate ungranted until Phase 20 +}); + +// In `adminRole`: +export const adminRole = ac.newRole({ + // ...existing entries... + rmm: ["read", "execute"], + phishing: ["read", "analyze"], +}); + +// In `userRole`: +export const userRole = ac.newRole({ + // ...existing entries... + rmm: ["read"], + phishing: ["read"], // cannot trigger /analyze +}); +``` + +**Confirmed (Assumption A1 resolved):** `lib/auth-utils.ts:4` imports `{ hasPermission, type Permission }` directly from `./permissions` — i.e. `lib/permissions.ts` IS the file `requirePermission()` reads from. No second/parallel permission file exists. Add the `phishing` resource here, nowhere else. + +**Both `hasPermission()`'s `roles` map and `Permission['resource']` type are derived from `statement`** — no additional wiring needed beyond the 4 edits above; `keyof typeof statement` picks up `phishing` automatically for the `requirePermission('phishing', ...)` call sites' type-checking. + +--- + +## 7. `lib/services/webhook-service.ts` (MODIFIED) + +**Role:** Add one call to `groupReportIntoCampaign()` after the existing `detectPhishingTicket()` call inside `triggerPhishingDetection()`. + +**Exact call site:** `lib/services/webhook-service.ts:489` (inside `private async triggerPhishingDetection(payload)`, the last line of the method). + +### Exact diff + +```typescript +// lib/services/webhook-service.ts — add import at top (near line 17): +import { detectPhishingTicket, DetectableTicket } from './phishing-detector'; +import { groupReportIntoCampaign } from './campaign-grouping-service'; // ADD + +// ...inside triggerPhishingDetection(payload), replace the final line: +console.log(`[WEBHOOK] Triggering phishing detection for ticket ${payload.entityId}`); +const detection = await detectPhishingTicket(ticket); +// D-01/D-08: automatic path short-circuits if already grouped. +if (detection.flagged && detection.reportId) { + await groupReportIntoCampaign(detection.reportId, { skipIfAlreadyGrouped: true }); +} +``` + +This is a fire-and-forget path already (`triggerPhishingDetection(payload).catch(err => ...)` at line 118) — no additional try/catch needed inside the method itself; an unhandled rejection here is already caught by the caller's `.catch()`. + +--- + +## 8. `lib/services/phishing-sweep-service.ts` (MODIFIED) + +**Role:** Add one call to `groupReportIntoCampaign()` per ticket inside the existing sweep loop, after `detectPhishingTicket()`. + +**Exact call site:** `lib/services/phishing-sweep-service.ts:78-84` (inside the `for (const row of candidates.rows)` loop). + +### Exact diff + +```typescript +// lib/services/phishing-sweep-service.ts — add import at top (near line 16): +import { detectPhishingTicket, type DetectableTicket } from './phishing-detector'; +import { groupReportIntoCampaign } from './campaign-grouping-service'; // ADD + +// ...inside the sweep loop, replace: +try { + const detection = await detectPhishingTicket(ticket); + if (detection.skippedUnchanged) { + result.skippedUnchanged += 1; + } else if (detection.flagged) { + result.flagged += 1; + } + // ADD — D-01/D-08: grouping runs regardless of skippedUnchanged (a report + // could have been created by a previous sweep pass and still lack a + // campaign_id if grouping failed transiently that time), short-circuits + // internally if already grouped. + if (detection.flagged && detection.reportId) { + await groupReportIntoCampaign(detection.reportId, { skipIfAlreadyGrouped: true }); + } +} catch (err) { + result.errors += 1; + logger.warn( + `Phishing detection failed for ticket ${ticket.id}`, + { ticketId: ticket.id }, + err instanceof Error ? err : new Error(String(err)) + ); +} +``` + +Keep the grouping call inside the SAME try/catch as `detectPhishingTicket` — a grouping failure for one ticket should count against `result.errors` and not abort the sweep loop, exactly matching this file's existing per-ticket error isolation. + +--- + +## Cross-Cutting Notes + +### Import path convention +Two different import styles exist for `postgresClient` in this codebase — both work (default export + named export both point to the same singleton, confirmed at `lib/services/postgres-client.ts:419-420`): +- `import { postgresClient } from './postgres-client';` — used in `phishing-detector.ts`, `phishing-eml-service.ts`, `phishing-sweep-service.ts`, `webhook-service.ts` (relative, `lib/services/` internal files) +- `import postgresClient from '@/lib/services/postgres-client';` — used in all `app/api/*/route.ts` files read during this mapping (absolute alias, route handlers) + +**Follow this exact split**: `campaign-grouping-service.ts` uses the named/relative import (matches its sibling `lib/services/` files); the 3 new route files use the default/absolute-alias import (matches every other route file). + +### Auth convention (D-06) — 3x identical shape +Every one of the 3 new routes starts with: +```typescript +const { error } = await requirePermission('phishing', ''); +if (error) return error; +``` +No route in this phase needs `session` beyond this (the `/analyze` route doesn't yet need `session.user.id` for an audit trail — that's Phase 20 per RESEARCH.md's `rmm/executions` POST comparison, which DOES capture `session.user.id` for `triggeredByAuditId`). Confirm during planning whether to capture `session.user.id` now for forward-compatibility; not required by any locked decision in CONTEXT.md. + +### Test scope boundary +Per RESEARCH.md's Validation Architecture: `vitest.config.ts`'s `include` is `lib/**/*.test.ts` only — **no route-handler test file should be planned** for the 3 new `app/api/phishing/*/route.ts` files (would not be discovered by the test runner and has zero precedent in this repo, confirmed by `find app/api -iname "*.test.ts"` returning zero results). Only `lib/services/campaign-grouping-service.test.ts` is an automated-test task; the 3 routes are verified manually (curl/Postman) per the phase gate. + +### Files confirmed to need NO changes +- `lib/services/sync-scheduler.ts` — the `phishing-sweep` cron branch already calls `sweepPhishingTickets()`; that function lives in `phishing-sweep-service.ts` (modified above), not the scheduler itself. +- `middleware.ts` — `/api/phishing/*` is not in the `publicRoutes` array, so it already gets the default session-cookie check; `requirePermission()` inside each route handler does the fine-grained role/action check on top. + +--- + +*Pattern mapping: 2026-07-15*