diff --git a/.planning/phases/18-campaign-grouping-phishing-analysis-api/18-RESEARCH.md b/.planning/phases/18-campaign-grouping-phishing-analysis-api/18-RESEARCH.md
new file mode 100644
index 0000000..5e9f36a
--- /dev/null
+++ b/.planning/phases/18-campaign-grouping-phishing-analysis-api/18-RESEARCH.md
@@ -0,0 +1,719 @@
+# Phase 18: Campaign Grouping & Phishing Analysis API - Research
+
+**Researched:** 2026-07-15
+**Domain:** Postgres-backed dedupe/grouping logic + Next.js 16 App Router API routes on an existing internal PSA codebase (Pulse)
+**Confidence:** HIGH (all findings below are `[VERIFIED: codebase]` — read directly from the files that exist in this repo today, not from training data or external docs)
+
+
+## User Constraints (from CONTEXT.md)
+
+### Locked Decisions
+
+**Grouping Trigger Point**
+- **D-01:** Campaign grouping runs automatically as part of the same
+ detection path Phase 15's `phishing-detector.ts` already uses (webhook
+ fire-and-forget `ticket.created` handler + the scheduled cron sweep) — the
+ moment a `reports` row is created or updated, grouping runs against it so a
+ campaign forms/accumulates without waiting for anyone to call the API. The
+ new `POST /api/phishing/tickets/{ticket_id}/analyze` endpoint (DETECT-03)
+ calls the SAME shared grouping function on-demand for one ticket — not a
+ separate/duplicate implementation. Rationale: success criterion #2
+ ("accumulates... as new duplicate reports arrive over time") implies
+ grouping must happen automatically, not only when explicitly requested.
+
+**Grouping Parameters**
+- **D-02:** Time window for the fallback tiers (attachment-hash/URL-domain +
+ subject + sender, and sender + normalized-subject + client) is **24
+ hours**. Rationale: catches same-day mass-phishing blasts (the realistic
+ case — one attacker campaign, multiple people reporting the same day)
+ without over-grouping unrelated reports that happen to share sender+subject
+ weeks apart.
+- **D-03:** Subject normalization for the fallback tiers: strip leading
+ `Re:`/`Fwd:`/`Fw:` prefixes (case-insensitive, repeated occurrences),
+ lowercase, trim whitespace. Standard email-threading normalization, nothing
+ more elaborate.
+
+**Campaign Merge Behavior**
+- **D-04:** Campaigns are never merged into each other. Each new report
+ attaches to at most one existing campaign (the single best/first match by
+ the tiered key) or creates a new one. If a report's tiered key
+ theoretically matches two distinct existing open campaigns, this is
+ treated as a rare edge case — do NOT implement multi-campaign-merge logic
+ (transactional reassignment of reports/messages/indicators/classifications
+ across campaigns) in this phase. Attach to the first/best match found and
+ move on; revisit only if this proves to be a real, recurring problem in
+ practice.
+
+**Permission Model**
+- **D-05:** Add a new `phishing` resource to `lib/permissions.ts`'s
+ `statement` with the FULL action set from the milestone spec now —
+ `phishing: ["read", "analyze", "approve", "remediate"]` — but this phase
+ only GRANTS `read` and `analyze` in role definitions:
+ - `superAdminRole` / `adminRole`: `phishing: ["read", "analyze"]`
+ - `userRole`: `phishing: ["read"]` (can browse campaigns, cannot trigger
+ on-demand analysis)
+ `approve`/`remediate` are declared in the statement (so the vocabulary
+ exists) but ungranted to any role until Phase 20 wires them up. Rationale:
+ establishes the full permission vocabulary once so Phase 20 only adds role
+ grants, not new statement keys — avoids touching the same statement block
+ twice across phases.
+
+**Route Auth Granularity**
+- **D-06:** Every `/api/phishing/*` route uses the fine-grained
+ `requirePermission()` check from day one, not plain `requireAuth()`:
+ - `GET /api/phishing/campaigns` and `GET /api/phishing/campaigns/{id}` →
+ `requirePermission('phishing', 'read')`
+ - `POST /api/phishing/tickets/{ticket_id}/analyze` →
+ `requirePermission('phishing', 'analyze')`
+ This establishes the exact auth convention (per-route, per-action
+ permission check) that Phase 19-21's endpoints must copy. Since `userRole`
+ gets `read` granted per D-05, this doesn't restrict any current user from
+ browsing — it wires the fine-grained check through now rather than
+ retrofitting it later.
+
+### Claude's Discretion
+
+- **Exact SQL/query shape for `GET /api/phishing/campaigns/{id}`'s nested
+ response** (linked reports, messages, indicators, classification history)
+ — single JOIN-heavy query vs. multiple queries assembled in application
+ code. Follow whatever the existing `entity-sync.ts` / DetailModal-backing
+ API routes in this codebase already do for similarly-shaped nested detail
+ responses.
+- **Exact shape of the tiered-key matching queries** (how "attachment-hash/
+ URL-domain" is queried against the `indicators` table's `indicator_type`
+ values `'attachment_hash'`/`'url'`/`'sender'` from Phase 16, how URL-domain
+ is extracted from a full URL string) — implementation detail, resolve via
+ research against the actual `indicators` schema and data shapes Phase 16
+ produces.
+- **Response body shape for `POST /analyze`** — should return the resulting
+ campaign linkage (campaign ID, grouping method used, whether a new
+ campaign was created vs. an existing one was matched) — exact field names
+ are planner's call, following the project's camelCase API response
+ convention.
+- **Where the shared grouping function lives** (new file e.g.
+ `lib/services/campaign-grouping-service.ts` vs. extending
+ `phishing-detector.ts`) — planner's call, following whatever composition
+ pattern is cleanest given `phishing-detector.ts`'s current size and
+ responsibilities.
+- **Whether `campaigns.status` transitions in this phase** (e.g., does
+ grouping ever set/change `status` beyond the migration's default `'open'`)
+ — not mentioned in success criteria; likely out of scope for this phase
+ (status transitions are Phase 19/20's classification/remediation concern),
+ but confirm during planning.
+
+### Deferred Ideas (OUT OF SCOPE)
+
+- **Multi-campaign merge logic** — deferred per D-04. If a report's tiered
+ key ever legitimately matches two distinct existing campaigns in practice
+ (not just in theory), building proper merge/reassignment logic is future
+ work, not blocking this phase.
+- **`campaigns.status` transition logic** (open → closed/resolved etc.) —
+ out of scope for this phase; belongs to Phase 19 (classification) or
+ Phase 20 (remediation).
+- **approve/remediate permission grants** — the `phishing` resource's
+ `approve`/`remediate` actions are declared in the statement (D-05) but not
+ granted to any role until Phase 20.
+
+None — discussion otherwise stayed within phase scope.
+
+
+
+## Phase Requirements
+
+| ID | Description | Research Support |
+|----|-------------|------------------|
+| CAMP-01 | Reports are grouped into a campaign using original Message-ID first, then attachment-hash/URL-domain + subject + sender + time-window, then sender + normalized subject + client + time-window as fallback keys | See Architecture Patterns (Pattern 1/2), Common Pitfalls 1/2/4 — tier key sources confirmed against actual `indicators`/`messages`/`reports`/`contacts` columns; 24h window and subject normalization specified per D-02/D-03 with a code example |
+| CAMP-02 | A campaign can accumulate many linked ticket reports and recipients over time as duplicates are detected | See Common Pitfalls 3 (no `recipients` column — must be derived via join), Architecture Patterns Pattern 2 (transaction-wrapped find-or-create, `report_count`/`last_seen_at` update) |
+| CAMP-03 | An operator can list campaigns and view a single campaign's full detail (linked reports, messages, indicators, classification history) via API | See Architecture Patterns Pattern 3 (bulk-fetch nested detail, exact analog `app/api/admin/device-link-conflicts/route.ts`) |
+| DETECT-03 | An operator can trigger analysis of one specific ticket by ID on demand (`POST /api/phishing/tickets/{ticket_id}/analyze`) | See Architecture Patterns Pattern 1 and 4 (shared function call chain: detect → parseAndStoreMessage → group; Next.js 16 `params: Promise<>` convention confirmed) |
+| ACCESS-01 | All `/api/phishing/*` endpoints enforce existing Pulse auth conventions (`requireAuth`/`requirePermission`), with approve/remediate requiring elevated permission beyond plain read access | See Code Examples (`requirePermission` call shape, `lib/permissions.ts` diff), Security Domain (V4 Access Control row) |
+
+
+## Summary
+
+This phase adds no new libraries, no new infra, and no UI. Everything needed already
+exists in the repo: `postgresClient` singleton, `requirePermission()`, the
+`reports`/`messages`/`indicators`/`campaigns` tables (migrations 097/099), and two
+already-wired trigger points (`webhook-service.ts`'s `triggerPhishingDetection()` and
+`sync-scheduler.ts`'s `phishing-sweep` cron branch → `phishing-sweep-service.ts`). The
+work is: (1) a new pure grouping function that computes a tiered campaign key and
+finds-or-creates a `campaigns` row, called from the two existing trigger points plus a
+new on-demand route; (2) three new `/api/phishing/*` routes gated by a new `phishing`
+permission resource.
+
+**The single most important finding requiring a planning decision:** `parseAndStoreMessage()`
+(the function that creates `messages`/`indicators` rows — the only source of Message-ID
+and attachment-hash/URL data) is **not called anywhere in the automatic webhook/cron
+path today**. It has zero callers outside its own test file. Per Phase 16's own docstring,
+it was deliberately left uncalled, waiting for "the live on-demand trigger
+(`POST /api/phishing/tickets/{id}/analyze`)" to arrive in this phase. This means: **as
+currently wired, the automatic grouping path (webhook `ticket.created` + cron sweep) will
+only ever have `reports` + `contacts` table data available — never `messages`/`indicators` —
+so Tier 1 (Message-ID) and Tier 2 (attachment-hash/URL-domain) can only ever match for a
+ticket that someone has already run through `POST /analyze` at least once.** This is
+flagged in Open Questions below with a recommendation; it materially affects whether
+success criterion #1's tiered matching is fully live automatically or only via manual
+trigger for tiers 1-2.
+
+**Primary recommendation:** Build one new pure service (e.g.
+`lib/services/campaign-grouping-service.ts`) exporting a single
+`groupReportIntoCampaign(reportId)` function that: looks up the report row, looks up any
+linked `messages`/`indicators` rows (if `parseAndStoreMessage` has already run for this
+report), tries Tier 1 → Tier 2 → Tier 3 matching queries in order inside a
+`postgresClient.transaction()`, and either links `reports.campaign_id` to an existing
+campaign (bumping `report_count`/`last_seen_at`) or inserts a new `campaigns` row. Call
+this same function from `webhook-service.ts` (after `detectPhishingTicket`),
+`phishing-sweep-service.ts` (after `detectPhishingTicket`, per-ticket), and the new
+`POST /api/phishing/tickets/{ticket_id}/analyze` route (after `detectPhishingTicket` +
+`parseAndStoreMessage`).
+
+## Architectural Responsibility Map
+
+| Capability | Primary Tier | Secondary Tier | Rationale |
+|------------|-------------|----------------|-----------|
+| Campaign key computation (tiered match logic) | API/Backend (`lib/services/`) | — | Pure business logic, no UI, testable in isolation like `phishing-detector.ts` |
+| Campaign find-or-create + accumulation | Database/Storage (Postgres via `postgresClient`) | API/Backend | `campaigns`/`reports.campaign_id` are the durable state; service layer owns the transaction |
+| On-demand analyze trigger | API/Backend (`app/api/phishing/tickets/[ticket_id]/analyze/route.ts`) | — | New route, orchestrates existing services, no new infra |
+| Campaign list/detail read | API/Backend (`app/api/phishing/campaigns/*`) | Database/Storage | Read-only JSON assembly over existing tables; no caching layer needed at this scale |
+| Auth/permission gate | API/Backend (`requirePermission()` in each route) | Browser/Client (none — no UI this phase) | `middleware.ts` only checks session-cookie presence; role/permission check is always in the route handler per this codebase's convention |
+| Automatic grouping trigger | API/Backend (webhook handler + cron sweep, both already server-side) | — | Extends Phase 15's existing dual-path wiring; no new trigger mechanism |
+
+## Standard Stack
+
+No new packages required. This phase is 100% additive Postgres queries + Next.js route
+handlers using infrastructure already present in `package.json`.
+
+### Core (existing, reused)
+| Library | Version (installed) | Purpose | Why Standard |
+|---------|---------|---------|--------------|
+| `pg` (via `postgresClient` singleton) | 8.11.0 `[VERIFIED: package.json]` | All DB access | Project convention — no ORM |
+| Next.js App Router route handlers | 16.1.1 `[VERIFIED: package.json]` | `/api/phishing/*` | Only backend pattern used in this repo |
+| Node built-in `crypto` | n/a (built-in) | Content hashing, if a composite-key hash is used | Already the pattern in `phishing-detector.ts`'s `computePhishingContentHash` |
+| Node built-in `URL` | n/a (built-in) | Domain extraction from indicator `value` (URL string) for Tier 2 | Precedent: `lib/services/personal-channels.ts:35` already does `new URL(input)` |
+
+### Alternatives Considered
+| Instead of | Could Use | Tradeoff |
+|------------|-----------|----------|
+| Pure-JS tiered key computation + targeted parameterized queries | One giant SQL `JOIN`/`OR` query encoding all three tiers | JS approach matches this codebase's existing style (small pure functions + explicit queries, see `computePhishingContentHash`); a mega-query would be harder to test and reason about, and this phase's data volumes (single-tenant MSP phishing reports) don't need query-level optimization |
+
+**Installation:** None — no `npm install` needed for this phase.
+
+## Package Legitimacy Audit
+
+Not applicable — this phase introduces zero new dependencies. Table omitted per the
+package-legitimacy-gate's own scope (only required "whenever this phase installs
+external packages").
+
+## Architecture Patterns
+
+### System Architecture Diagram
+
+```
+ ┌─────────────────────────────┐
+ │ Autotask webhook │
+ │ ticket.created │
+ └──────────────┬────────────────┘
+ │ fire-and-forget
+ ▼
+ webhook-service.ts: triggerPhishingDetection(payload)
+ │
+ ▼
+ detectPhishingTicket(ticket) ──► upserts `reports` row
+ │
+ ▼
+ groupReportIntoCampaign(reportId) [NEW, this phase]
+ │
+ ┌──────────────┼───────────────────────┐
+ ▼ ▼ ▼
+ Tier 1: Message-ID Tier 2: attach-hash/ Tier 3: sender+
+ (via messages/ URL-domain+subject+ subject+client+
+ indicators, if sender+24h window 24h window
+ parseAndStoreMessage (indicators, (reports +
+ already ran) 24h window) contacts join)
+ │ │ │
+ └──────────────┴───────────┬───────────┘
+ ▼
+ find-or-create in `campaigns`
+ (postgresClient.transaction)
+ │
+ ▼
+ UPDATE reports SET campaign_id = ...
+
+ ┌─────────────────────────────┐
+ │ sync-scheduler.ts cron │
+ │ sync_type='phishing-sweep' │──► sweepPhishingTickets() ──► same detectPhishingTicket
+ └─────────────────────────────┘ + groupReportIntoCampaign
+ loop, per ticket
+
+ ┌─────────────────────────────────────────────┐
+ │ POST /api/phishing/tickets/{id}/analyze │ requirePermission('phishing','analyze')
+ └──────────────────┬────────────────────────────┘
+ ▼
+ detectPhishingTicket(ticket)
+ ▼
+ parseAndStoreMessage({reportId, ticketId}) [existing, Phase 16 — currently uncalled elsewhere]
+ ▼
+ groupReportIntoCampaign(reportId) [NEW — same shared function as above]
+ ▼
+ returns { campaignId, groupMethod, created: boolean }
+
+ ┌─────────────────────────────────────────────┐
+ │ GET /api/phishing/campaigns │ requirePermission('phishing','read')
+ │ GET /api/phishing/campaigns/{id} │ requirePermission('phishing','read')
+ └──────────────────┬────────────────────────────┘
+ ▼
+ SELECT campaigns [+ bulk-fetch linked reports/messages/
+ indicators/classifications, assembled in application code
+ — see Pattern: Bulk-Fetch Nested Detail below]
+```
+
+### Recommended Project Structure
+```
+lib/services/
+├── phishing-detector.ts # existing — unchanged
+├── phishing-eml-service.ts # existing — unchanged (gains a caller: the new /analyze route)
+├── phishing-sweep-service.ts # existing — gains one new call per ticket (groupReportIntoCampaign)
+├── campaign-grouping-service.ts # NEW — groupReportIntoCampaign(reportId), tier key helpers, normalizeSubject()
+app/api/phishing/
+├── tickets/[ticket_id]/analyze/route.ts # NEW — POST, requirePermission('phishing','analyze')
+├── campaigns/route.ts # NEW — GET (list), requirePermission('phishing','read')
+├── campaigns/[id]/route.ts # NEW — GET (detail), requirePermission('phishing','read')
+lib/permissions.ts # MODIFIED — add `phishing` resource + role grants (D-05)
+lib/services/webhook-service.ts # MODIFIED — triggerPhishingDetection() gains one call
+lib/services/sync-scheduler.ts # unchanged (phishing-sweep branch already calls sweepPhishingTickets() — no scheduler.ts edit needed, only phishing-sweep-service.ts)
+```
+
+### Pattern 1: Shared pure detector/grouping function, called from 3 sites
+**What:** One function (`groupReportIntoCampaign`) with zero duplicated logic, called
+from the webhook handler, the cron sweep, and the on-demand API route.
+**When to use:** Exactly this phase's D-01 requirement.
+**Example (existing precedent to follow exactly — `phishing-detector.ts`'s
+`detectPhishingTicket` is called identically from all 3 sites already):**
+```typescript
+// lib/services/webhook-service.ts:488-489 (existing, unmodified pattern to extend)
+console.log(`[WEBHOOK] Triggering phishing detection for ticket ${payload.entityId}`);
+await detectPhishingTicket(ticket);
+// ADD: await groupReportIntoCampaign(result.reportId) when result.flagged
+```
+```typescript
+// lib/services/phishing-sweep-service.ts:78-84 (existing loop to extend)
+const detection = await detectPhishingTicket(ticket);
+if (detection.skippedUnchanged) {
+ result.skippedUnchanged += 1;
+} else if (detection.flagged) {
+ result.flagged += 1;
+ // ADD: await groupReportIntoCampaign(detection.reportId)
+}
+```
+
+### Pattern 2: Tiered find-or-create inside a transaction
+**What:** Because `campaigns.campaign_key` has **no UNIQUE constraint** (migration 097
+only creates a non-unique `idx_campaigns_campaign_key` index — `[VERIFIED: migrations/097_phishing_triage_schema.sql]`), a naive `SELECT` then `INSERT`-if-not-found
+race is possible under concurrent webhook+cron execution. Wrap the whole find-or-create
+in `postgresClient.transaction()` (existing method, `lib/services/postgres-client.ts:96-111`).
+**When to use:** Every call to `groupReportIntoCampaign`.
+**Example:**
+```typescript
+// Source: lib/services/postgres-client.ts:96-111 (existing transaction() method)
+await postgresClient.transaction(async (client) => {
+ // Tier 1: Message-ID (only reachable if messages row exists — see Open Questions)
+ const tier1 = await client.query(
+ `SELECT r.campaign_id::text AS campaign_id
+ FROM messages m
+ JOIN reports r ON r.id = m.report_id
+ WHERE m.message_id = $1 AND m.message_id IS NOT NULL AND r.campaign_id IS NOT NULL
+ ORDER BY r.created_at ASC LIMIT 1`,
+ [messageId]
+ );
+ // ... Tier 2 / Tier 3 queries follow the same shape, each gated on the
+ // previous tier finding nothing ...
+ // Then either UPDATE campaigns SET report_count = report_count + 1,
+ // last_seen_at = NOW() WHERE id = $1, or INSERT INTO campaigns (...) RETURNING id.
+});
+```
+
+### Pattern 3: Bulk-Fetch Nested Detail (the exact analog to follow for GET /campaigns/{id})
+**What:** This codebase's established pattern for assembling a parent + nested-children
+JSON response is NOT a single mega-JOIN — it's (1) one query for the parent rows, (2) one
+bulk query for all related child rows keyed by an array of parent-derived IDs, (3) a
+`Map` for O(1) lookup, (4) a final `.map()` that manually builds the camelCase response
+shape.
+**When to use:** `GET /api/phishing/campaigns` (list, with per-campaign counts) and
+`GET /api/phishing/campaigns/{id}` (detail, with linked reports/messages/indicators/classifications).
+**Example — read directly from the codebase (exact pattern to replicate):**
+```typescript
+// Source: app/api/admin/device-link-conflicts/route.ts:37-133 (existing, full file read)
+export async function GET(request: NextRequest) {
+ const { error } = await requirePermission('admin', 'access');
+ if (error) return error;
+ // 1. Parent query with LEFT JOINs for 1:1 relations
+ const reviews = await postgresClient.query(`SELECT ... FROM device_link_review r JOIN ... `, params);
+ // 2. Bulk-fetch all related child rows keyed by an ID array (ANY($1::bigint[]))
+ const ciDetails = new Map();
+ const ciRes = await postgresClient.query(`SELECT ... WHERE ci.id = ANY($1::bigint[])`, [Array.from(allCiIds)]);
+ for (const ci of ciRes.rows) ciDetails.set(ci.id, ci);
+ // 3. Manual camelCase assembly in a final .map()
+ const items = reviews.rows.map((r) => ({ id: r.id, detectedAt: r.detected_at, xref: { ... }, candidates: [...] }));
+ return NextResponse.json({ items, total, limit, offset });
+}
+```
+For the simpler paginated list (`GET /api/phishing/campaigns`), the shorter
+`workflow/executions/route.ts` pattern (`{ data, total, limit, offset }`, dynamic
+WHERE-clause building from query params) is also a valid, simpler precedent — but note
+it does **not** camelCase-transform its output (it spreads raw snake_case rows). Follow
+`device-link-conflicts`'s manual-camelCase version instead, since CLAUDE.md's convention
+is explicit ("API responses are camelCase — handlers transform manually").
+
+### Pattern 4: Next.js 16 dynamic route param convention (CONFIRMED)
+**What:** `params` is a `Promise` that must be `await`ed — this is the actual, current
+convention in this repo, confirmed by reading 3 existing dynamic routes.
+**Example:**
+```typescript
+// Source: app/api/tickets/[id]/route.ts:18-23 and app/api/workflow/executions/[id]/route.ts:4-9 (both existing, both this exact shape)
+export async function GET(
+ request: NextRequest,
+ { params }: { params: Promise<{ id: string }> }
+) {
+ const { id } = await params;
+ // ...
+}
+```
+For `POST /api/phishing/tickets/{ticket_id}/analyze`, the folder path is
+`app/api/phishing/tickets/[ticket_id]/analyze/route.ts` and the param type is
+`{ params: Promise<{ ticket_id: string }> }` — use `Number(ticket_id)` before passing to
+`DetectableTicket.id` (matches `tickets/[id]/route.ts:27`'s `parseInt(id)` pattern; `tickets.id`
+is `BIGINT`).
+
+### Anti-Patterns to Avoid
+- **Duplicating detection/grouping logic in the route handler:** the route must call the
+ shared service function, not reimplement matching inline (this is the entire point of
+ D-01 and mirrors how `detectPhishingTicket` is already the single source of truth).
+- **Multi-campaign merge machinery:** explicitly out of scope per D-04. Attach to the
+ first/best match found; do not build transactional reassignment across campaigns.
+- **SQL-encoded fuzzy tier logic:** don't try to encode "strip Re:/Fwd:, 24h window,
+ domain-from-URL" all inside one WHERE clause — compute keys in JS (matching
+ `computePhishingContentHash`'s style), then run targeted parameterized queries.
+
+## Don't Hand-Roll
+
+| Problem | Don't Build | Use Instead | Why |
+|---------|-------------|-------------|-----|
+| URL domain extraction from indicator `value` | Regex-based hostname parser | Node built-in `new URL(value).hostname` | Already the precedent in `lib/services/personal-channels.ts:35`; regex-based URL parsing is a classic source of bugs (userinfo, ports, IDN, trailing slashes) |
+| Permission/role definitions | A separate phishing-specific auth module | `lib/permissions.ts`'s existing `statement`/`superAdminRole`/`adminRole`/`userRole` + `requirePermission()` from `lib/auth-utils.ts` | This IS the project's only auth mechanism (Better Auth 1.4's `createAccessControl`); adding a parallel one would violate CLAUDE.md's "Better Auth is final" |
+| Transaction/locking primitives | Custom advisory-lock or optimistic-concurrency code | `postgresClient.transaction()` (existing, `postgres-client.ts:96-111`) | Already wraps BEGIN/COMMIT/ROLLBACK correctly; Postgres row-level locking inside the callback (if needed) is a plain `SELECT ... FOR UPDATE` inside that same transaction |
+
+**Key insight:** every piece of infrastructure this phase needs (hashing helper style,
+transaction wrapper, permission system, route+auth pattern, nested-detail JSON assembly)
+already has an exact precedent somewhere in this repo. There is no case in this phase
+where reaching for an external library is justified.
+
+## Common Pitfalls
+
+### Pitfall 1: Automatic path has no Message-ID/indicator data to key on (see Open Questions)
+**What goes wrong:** Tier 1/2 grouping silently never fires for tickets that arrive via
+webhook/cron and are never explicitly `/analyze`d, even though CAMP-01 lists Message-ID
+as the "primary" tier.
+**Why it happens:** `parseAndStoreMessage()` (the only writer of `messages`/`indicators`)
+has zero callers today outside its own test — confirmed via
+`grep -rn "parseAndStoreMessage" --include=*.ts` returning only the definition file and
+`phishing-eml-service.test.ts`. Phase 16's own plan doc (`16-03-PLAN.md:59`) states the
+"live on-demand trigger... arrives in Phase 18" as the intended first caller.
+**How to avoid:** Decide explicitly during planning (see Open Questions) whether this
+phase also wires `parseAndStoreMessage` into the webhook/cron path, or whether Tier 1/2
+are accepted as "only reachable after an explicit `/analyze` call" for v1.
+**Warning signs:** If a test/manual check creates two tickets with the same `.eml`
+Message-ID via webhook only (no `/analyze` call) and they land in two different
+campaigns instead of one, this is the expected (if surprising) behavior under the
+"accept as-is" option.
+
+### Pitfall 2: `campaigns.campaign_key` has no UNIQUE constraint
+**What goes wrong:** A naive check-then-insert (`SELECT ... campaign_key = $1` then
+`INSERT INTO campaigns`) run concurrently by webhook + cron sweep for two reports that
+should be the same campaign can create two campaign rows for one campaign.
+**Why it happens:** Migration 097 only creates `idx_campaigns_campaign_key` (a plain
+non-unique index), not a unique constraint — `[VERIFIED: migrations/097_phishing_triage_schema.sql:36]`.
+**How to avoid:** Wrap find-or-create in `postgresClient.transaction()`. Given this
+project's own precedent for "rare edge case, document don't over-engineer" (per
+`17-mimecast-blast-radius-lookup/17-CONTEXT.md` D-05, cited in this phase's own
+CONTEXT.md), a transaction without row-level locking is likely sufficient — true webhook/cron
+concurrency on the *same* ticket is rare. Flag as a known limitation rather than adding a
+`SELECT ... FOR UPDATE` advisory-lock scheme, unless the planner judges the race
+realistic enough to matter (e.g. bulk backfill sweep + webhook firing together).
+
+### Pitfall 3: `campaigns` table has no `recipients` column
+**What goes wrong:** Assuming there's a `recipients` array/JSONB column to append to on
+each new report (CAMP-02 says "accumulate... recipients").
+**Why it happens:** Migration 097's `campaigns` table columns are exactly:
+`id, campaign_key, group_method, first_seen_at, last_seen_at, report_count, status,
+created_at, updated_at` — no recipients field. `[VERIFIED: migrations/097_phishing_triage_schema.sql:24-34]`
+**How to avoid:** "Recipients" for a campaign must be computed at read time (in
+`GET /api/phishing/campaigns/{id}`) by joining all linked `reports` rows (via
+`reports.campaign_id`) to their `requester_contact_id` → `contacts.email_address`/`first_name`/`last_name`
+(contacts table confirmed at `migrations/001_initial_schema.sql:83-103`, has
+`email_address`). Do not add a new denormalized column for this in this phase unless the
+planner has a specific performance reason to.
+
+### Pitfall 4: `indicators.value` for `indicator_type='url'` is a bare URL string, not a domain
+**What goes wrong:** Querying `indicators.value = $domain` directly for Tier 2 matching
+returns nothing, because the stored value is the full URL (e.g.
+`https://evil.example.com/path?x=1`), not `evil.example.com`.
+**Why it happens:** `phishing-eml-service.ts:186-193` inserts
+`[messageId, 'url', url, ...]` where `url` is the raw string from
+`normalized.urls` (`eml-parser.ts`'s `extractUrls`) — no domain extraction happens at
+write time. `[VERIFIED: lib/services/phishing-eml-service.ts:184-193]`
+**How to avoid:** Extract the domain in the grouping service at *read* time
+(`new URL(indicatorRow.value).hostname`), not by querying `indicators.value` directly for
+an exact domain match. Guard `new URL()` in a try/catch — malformed/relative URLs are
+possible in real-world phishing emails.
+
+### Pitfall 5: `DetectableTicket.contact_id` vs `reports.requester_contact_id` naming mismatch
+**What goes wrong:** Assuming a `sender`/`requester` field exists on the `reports` row
+under an obvious name.
+**Why it happens:** The TS interface field is `contact_id` (optional) but the DB column
+and the actual reports insert map it to `requester_contact_id` —
+`[VERIFIED: lib/services/phishing-detector.ts:77, 234]`.
+**How to avoid:** When joining `reports` → `contacts` for Tier 3's "sender", join on
+`reports.requester_contact_id = contacts.id`, not a column literally named `sender` or
+`contact_id`.
+
+## Code Examples
+
+### Subject normalization (D-03) — style matches `computePhishingContentHash`
+```typescript
+// New helper, following the existing pure-function style in phishing-detector.ts
+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();
+}
+```
+
+### requirePermission call-and-early-return (exact syntax to copy 3x)
+```typescript
+// Source: app/api/admin/rmm/settings/route.ts:30-31 and app/api/rmm/executions/route.ts:84
+const { error } = await requirePermission('phishing', 'read');
+if (error) return error;
+// or, for the analyze route (needs session.user.id for future audit trail in Phase 20):
+const { session, error } = await requirePermission('phishing', 'analyze');
+if (error) return error;
+```
+
+### `lib/permissions.ts` diff shape (D-05) — closest analog is `rmm: ["read", "execute"]`
+```typescript
+// statement (add one line, mirrors existing rmm entry at lib/permissions.ts:30)
+phishing: ["read", "analyze", "approve", "remediate"],
+
+// superAdminRole / adminRole (add one line each, at lib/permissions.ts:46 and :59)
+phishing: ["read", "analyze"],
+
+// userRole (add one line, at lib/permissions.ts:72)
+phishing: ["read"],
+```
+Note: `lib/permissions.ts` (the file read in full for this research) is a **different,
+older/parallel permission module** from `lib/auth-utils.ts`'s `requirePermission()` — both
+exist in the repo (`[VERIFIED: lib/permissions.ts` full read`]`). `lib/auth-utils.ts`
+imports `hasPermission`/`Permission`/`statement` presumably from this same file (confirm
+the exact import path during planning — `lib/auth-utils.ts:51-74`'s `requirePermission`
+signature matches `Permission["resource"]`/`statement` shapes 1:1 with `lib/permissions.ts`,
+so they are the same system, not two competing ones).
+
+## State of the Art
+
+Not applicable — no external library/framework version drift to track. All patterns
+above are read directly from this repo's current `master` branch as of 2026-07-15.
+
+## Assumptions Log
+
+| # | Claim | Section | Risk if Wrong |
+|---|-------|---------|---------------|
+| A1 | `lib/auth-utils.ts`'s `hasPermission`/`Permission`/`statement` imports resolve to `lib/permissions.ts` (not a second, hidden permissions file) | Code Examples | Low — trivially confirmed by opening `lib/auth-utils.ts`'s import block during planning; if wrong, the `phishing` resource must be added to whichever file is actually imported |
+| A2 | No true webhook+cron concurrency race on the same ticket is realistic enough to require `SELECT ... FOR UPDATE` locking (Pitfall 2) | Common Pitfalls | Low-medium — if wrong, two campaigns could be created for one real campaign in rare timing windows; recoverable manually since D-04 already accepts "rare edge case" duplication as out of scope for merge-logic |
+
+**Everything else in this research is `[VERIFIED: codebase]`** — read directly from
+migration files, service files, route files, and config, not from training-data
+assumptions about how this codebase "probably" works.
+
+## Open Questions
+
+1. **Should this phase also wire `parseAndStoreMessage()` into the automatic
+ webhook/cron path, or leave it reachable only via the on-demand `/analyze` endpoint?**
+ - What we know: `parseAndStoreMessage` has zero callers today outside its test file.
+ Phase 16's own plan doc explicitly earmarked "the live on-demand trigger" (this
+ phase's `/analyze` route) as its first real caller. CONTEXT.md's D-01 says grouping
+ runs automatically on webhook/cron using whatever the `reports` row has — it does
+ NOT explicitly say eml-parsing also becomes automatic.
+ - What's unclear: Whether the project intends Tier 1/Message-ID grouping to be a
+ realistic automatic behavior (requiring eml parsing on every ticket.created event —
+ an extra Autotask attachment-content fetch + optional B2 upload per ticket, a
+ meaningfully bigger automatic-path cost) or an "only when someone has looked at this
+ ticket" behavior.
+ - Recommendation: Default to **NOT** wiring `parseAndStoreMessage` into the automatic
+ path in this phase (matches Phase 16's explicit intent and keeps the automatic path
+ cheap — no extra Autotask/B2 calls per ticket.created event). Document this
+ explicitly as a known limitation: automatic grouping effectively only achieves Tier 3
+ (sender+subject+client+time-window) until an operator calls `/analyze` for a given
+ ticket, after which Tier 1/2 become available for that ticket's future duplicates.
+ Surface this tradeoff to the user/planner explicitly rather than silently building
+ around it — it changes what "automatically grouped" means in practice for success
+ criterion #1.
+
+2. **Does `groupReportIntoCampaign` re-run for a report already linked to a campaign?**
+ - What we know: `detectPhishingTicket`'s idempotency guard (content-hash unchanged)
+ still refreshes the `evidence` JSONB on every call — so it's called repeatedly over
+ a ticket's lifetime (every webhook re-fire, every cron sweep pass within the 7-day
+ window).
+ - What's unclear: Whether `groupReportIntoCampaign` should short-circuit immediately
+ if `reports.campaign_id IS NOT NULL` (cheap, avoids re-running tier queries for
+ already-grouped reports on every sweep), or should still re-run to catch a late-
+ arriving `.eml` upgrading a Tier-3-only report to a Tier-1 match.
+ - Recommendation: Short-circuit if already linked (`campaign_id IS NOT NULL`) for the
+ automatic webhook/cron path (cheap, avoids redundant work every 7-day sweep pass),
+ but let the `/analyze` route always re-run grouping unconditionally (it's an explicit
+ operator request and may follow a fresh `/analyze` call that just populated
+ `messages`/`indicators` for the first time, deserving a chance to upgrade to Tier 1).
+
+3. **Does `lib/auth-utils.ts` import `statement`/`hasPermission` from `lib/permissions.ts`
+ directly?** (See Assumption A1.) Confirm the import path when adding the `phishing`
+ resource so it lands in the file `requirePermission()` actually reads from.
+
+## Environment Availability
+
+Skipped — this phase has no new external tool/service/runtime dependencies. All
+infrastructure (Postgres, the existing Autotask client, B2 client) is already used by
+Phase 15/16 code this phase calls into, and none of it is newly required by this phase
+specifically.
+
+## Validation Architecture
+
+### Test Framework
+| Property | Value |
+|----------|-------|
+| Framework | vitest 4.1.5 `[VERIFIED: package.json]` |
+| Config file | `vitest.config.ts` — `include: ['lib/**/*.test.ts']` only `[VERIFIED: vitest.config.ts]` |
+| Quick run command | `npm test -- campaign-grouping-service` (or `npx vitest run lib/services/campaign-grouping-service.test.ts`) |
+| Full suite command | `npm test` |
+
+**Critical confirmed finding:** `vitest.config.ts`'s `include` glob is
+`lib/**/*.test.ts` — it does **not** include `app/**`. There is no precedent anywhere in
+this repo for testing a `route.ts` handler directly (`find app/api -iname "*.test.ts"`
+returns zero results), and the current config would not even discover such a test file if
+one were added. **Test the service layer only** (`campaign-grouping-service.ts`'s pure
+functions and DB-touching functions, mocking `postgresClient` the same way
+`phishing-eml-service.test.ts` mocks it) — do not plan a task to add route-handler tests;
+it would be inconsistent with every other API route in this codebase and outside the
+current test runner's scope.
+
+### Phase Requirements → Test Map
+| Req ID | Behavior | Test Type | Automated Command | File Exists? |
+|--------|----------|-----------|-------------------|-------------|
+| CAMP-01 | Tiered key computation (Message-ID > attachment-hash/URL-domain+subject+sender+24h > sender+normalized-subject+client+24h) | unit | `npx vitest run lib/services/campaign-grouping-service.test.ts` | ❌ Wave 0 |
+| CAMP-01 | `normalizeSubject()` strips Re:/Fwd:/Fw: (repeated, case-insensitive), lowercases, trims | unit | same file | ❌ Wave 0 |
+| CAMP-02 | Second report with matching key increments `report_count`, updates `last_seen_at`, links `campaign_id`, never creates a second campaign | unit (mocked postgresClient) | same file | ❌ Wave 0 |
+| DETECT-03 | `POST /api/phishing/tickets/{id}/analyze` orchestrates detect→parse→group and returns campaign linkage | manual / integration — no route-handler test precedent exists in this repo (see Validation Architecture note above) | manual curl/Postman against a dev server, or a service-layer test of the orchestration function if the route delegates to one | n/a |
+| CAMP-03 | `GET /api/phishing/campaigns` and `/{id}` return expected nested shape | manual / integration (same reasoning as above) | manual verification | n/a |
+| ACCESS-01 | Every `/api/phishing/*` route rejects unauthenticated/unauthorized requests with 401/403 | manual — no existing precedent for testing `requirePermission()` wiring via automated route tests in this repo | manual curl without/with session cookie, or code-review checklist during `/gsd:verify-work` | n/a |
+
+### Sampling Rate
+- **Per task commit:** `npx vitest run lib/services/campaign-grouping-service.test.ts`
+- **Per wave merge:** `npm test` (full suite — note pre-existing unrelated failures in
+ `lib/services/analyzer/itglue-search.test.ts` per Phase 16's `deferred-items.md`; do not
+ attempt to fix those as part of this phase)
+- **Phase gate:** Full suite green (modulo the known pre-existing failure) +
+ `npx tsc --noEmit --pretty` clean + manual curl verification of all 3 new routes'
+ auth behavior (since no automated route-handler test precedent exists to rely on)
+
+### Wave 0 Gaps
+- [ ] `lib/services/campaign-grouping-service.test.ts` — new file, covers CAMP-01/CAMP-02
+ pure-function and mocked-DB behavior (mirror `phishing-detector.test.ts` and
+ `phishing-eml-service.test.ts`'s `vi.mock('./postgres-client')` approach)
+- [ ] No new fixtures needed — reuse `eml-parser.fixtures.ts` synthetic fixtures if
+ Message-ID matching needs a fixture value (per Out-of-Scope: "Real customer `.eml`
+ fixtures in tests" is banned; synthetic only)
+- [ ] Framework install: none — vitest already configured and used extensively
+
+## Security Domain
+
+### Applicable ASVS Categories
+
+| ASVS Category | Applies | Standard Control |
+|---------------|---------|-----------------|
+| V2 Authentication | No (unchanged) | Better Auth 1.4 session cookie — no change this phase |
+| V3 Session Management | No (unchanged) | Better Auth 1.4 — no change this phase |
+| V4 Access Control | **Yes** | `requirePermission('phishing', 'read'|'analyze')` per route (D-06) — new `phishing` resource in `lib/permissions.ts`'s `statement`/role definitions (D-05) |
+| V5 Input Validation | Yes | `ticket_id`/`id` path params: validate as numeric/UUID before querying (mirror `tickets/[id]/route.ts`'s `parseInt(id)`); campaign `id` is a UUID (`campaigns.id UUID PRIMARY KEY`) — validate format before querying to avoid a malformed-UUID Postgres error leaking as a 500 |
+| V6 Cryptography | No | No crypto operations beyond the existing `computePhishingContentHash`-style hashing (not security-sensitive — it's a dedupe key, not an auth/integrity control) |
+
+### Known Threat Patterns for this stack
+
+| Pattern | STRIDE | Standard Mitigation |
+|---------|--------|---------------------|
+| Broken access control — a `user`-role account calling `POST /analyze` (which only `read` is granted to per D-05) | Elevation of Privilege | `requirePermission('phishing', 'analyze')` returns 403 via `hasPermission()`'s role-statement lookup — already the exact mechanism used for `rmm.execute` today (`app/api/rmm/executions/route.ts:84`) |
+| IDOR via `ticket_id`/campaign `id` path param (guessing another company's ticket ID) | Information Disclosure | This codebase's existing routes (e.g. `tickets/[id]/route.ts`) do not currently scope by company/tenant for authenticated users — this is a single-tenant internal MSP tool (Wulf Consulting staff only), so this is consistent with the existing security model, not a new gap introduced by this phase. No additional scoping logic needed beyond matching existing route conventions. |
+| SQL injection via unvalidated query params in the list route's dynamic WHERE-clause building | Tampering | Follow `device-link-conflicts/route.ts`'s pattern exactly: parameterized `$n` placeholders built alongside a `params: unknown[]` array — never string-interpolate a user-supplied value into SQL text (the `sourceFilter` string in that file only ever contains the placeholder index, never the value itself) |
+| Never-fetch invariant regression (EVID-04/SC#3) — a future contributor "helpfully" adding a URL-preview feature that fetches an indicator's URL | Tampering / SSRF | Not this phase's job to re-verify (Phase 16 already covers it), but note for the planner: this phase's grouping logic only ever reads `indicators.value` as a string for domain-extraction (`new URL(value).hostname`) — never a network fetch. Keep it that way. |
+
+## Sources
+
+### Primary (HIGH confidence — direct codebase reads, this session)
+- `migrations/097_phishing_triage_schema.sql` — full read, all 7 table definitions
+- `migrations/099_indicators_metadata.sql` — full read, `indicators.metadata` addition
+- `migrations/098_phishing_sweep_schedule.sql` — full read, cron schedule seed
+- `migrations/001_initial_schema.sql` — `contacts` table columns (lines 83-103)
+- `lib/services/phishing-detector.ts` — full read (250 lines)
+- `lib/services/phishing-eml-service.ts` — full read (225 lines)
+- `lib/services/phishing-sweep-service.ts` — full read (102 lines)
+- `lib/services/webhook-service.ts` — `triggerPhishingDetection()` and
+ `triggerWorkflowEngine()`, lines 395-491
+- `lib/services/sync-scheduler.ts` — `phishing-sweep` sync_type branch (lines 25, 303-309, 472-477)
+- `lib/services/eml-parser.ts` — `NormalizedMessage` interface, `extractUrls`,
+ `messageId`/`domain` fields (lines 93-272)
+- `lib/permissions.ts` — full read (110 lines), `statement`/roles/`hasPermission`
+- `lib/auth-utils.ts` — `requireAuth`/`requirePermission`/`requireAdmin`/`requireSuperAdmin`
+ (lines 31-124)
+- `lib/services/postgres-client.ts` — `transaction()`, `upsert()`, `bulkUpsert()` signatures
+ (lines 96-238)
+- `app/api/admin/rmm/settings/route.ts` — full read, `requirePermission` call shape
+- `app/api/tickets/[id]/route.ts` — full read, dynamic-route `params: Promise<>` convention
+- `app/api/rmm/executions/route.ts` — full read, list+POST pattern with `requirePermission`
+- `app/api/workflow/executions/route.ts` and `[id]/route.ts` — full reads, simpler
+ list/detail pattern (no camelCase transform — noted as the weaker analog)
+- `app/api/admin/device-link-conflicts/route.ts` — full read, bulk-fetch + Map +
+ manual-camelCase nested-detail pattern (the recommended analog)
+- `middleware.ts` — full read of `publicRoutes` array, confirms `/api/phishing/*` is not
+ public and gets the default session-cookie check
+- `vitest.config.ts` — full read, confirms `include: ['lib/**/*.test.ts']` only
+- `lib/services/phishing-detector.test.ts` — read, confirms mocking-free pure-function
+ test style for exported helpers
+- `.planning/phases/16-eml-mime-evidence-parser/16-03-PLAN.md` and `16-03-SUMMARY.md` —
+ confirm `parseAndStoreMessage` was deliberately left uncalled pending this phase
+- `.planning/phases/18-campaign-grouping-phishing-analysis-api/18-CONTEXT.md` — full read
+- `.planning/REQUIREMENTS.md` — full read
+- `.planning/STATE.md` — full read
+- `.planning/config.json` — full read (`nyquist_validation: true`, no
+ `security_enforcement` key present → treated as enabled per instructions)
+
+### Secondary (MEDIUM confidence)
+None — no external sources were needed for this phase; every fact is derived from
+reading this repository directly.
+
+### Tertiary (LOW confidence)
+None.
+
+## Metadata
+
+**Confidence breakdown:**
+- Standard stack: HIGH — no new packages, all existing infra confirmed by direct file reads
+- Architecture: HIGH — patterns pulled from 3+ existing analogous route files, transaction
+ method read from source
+- Pitfalls: HIGH — all 5 pitfalls are concrete schema/code facts (missing constraint,
+ missing column, raw-URL-not-domain, field-name mismatch, uncalled function), not
+ speculative
+
+**Research date:** 2026-07-15
+**Valid until:** Stable — this research is tied to the current state of `master` in this
+repo, not to any external library version. Re-verify only if `migrations/097`/`099`,
+`phishing-detector.ts`, `phishing-eml-service.ts`, `lib/permissions.ts`, or
+`lib/auth-utils.ts` change before this phase is planned/executed.