docs(19): add phase verification report

.planning/phases/19-classification-engine/19-VERIFICATION.md

@@ -0,0 +1,180 @@
+---
+phase: 19-classification-engine
+verified: 2026-07-16T08:35:00Z
+status: human_needed
+score: 6/6 must-haves verified
+overrides_applied: 0
+human_verification:
+  - test: "Run the manual curl auth-matrix (200/401/403/400/404) against a live npm run dev instance with real session cookies"
+    expected: "200 (admin/super-admin + verdict payload), 401 (no session), 403 (plain user role), 400 (malformed id), 404 (unknown campaign)"
+    why_human: "Requires a running server with real Postgres + Better Auth session state; not runnable in either executor's worktree or this verification environment. Static code review traces every branch to the correct status code (see Behavioral Spot-Checks / Manual Curl Auth-Matrix sections) — this is a regression-guard re-run, not an open correctness question."
+  - test: "Confirm blastRadius.clicked > 0 is the right trigger for escalating THREAT recommended actions to reset_password/isolate_endpoint/disable_forwarding_rule"
+    expected: "A user/product decision on whether this escalation trigger is correct before Phase 20 builds approval-gating on top of it"
+    why_human: "Product/security-policy judgment call, not resolvable by reading code. Confirmed (via CONTEXT.md/PATTERNS.md/RESEARCH.md cross-check) to be a genuinely open reasoned assumption, not a decision that conflicts with anything locked for Phase 19 — but it materially affects Phase 20."
+---
+
+# Phase 19: Classification Engine Verification Report
+
+**Phase Goal:** Every campaign gets a deterministic SPAM/UNWANTED/THREAT verdict, built from
+bounded structured evidence (never raw unbounded email), that correctly flags destructive-action
+recommendations for approval and doesn't cry wolf on routine KnowBe4 simulations.
+**Verified:** 2026-07-16
+**Status:** human_needed
+**Re-verification:** No — initial verification
+
+## Goal Achievement
+
+All 6 must-haves (CLASSIFY-01..06) are VERIFIED against the merged code — see below. Status is
+`human_needed` (not `passed`) solely because two non-blocking items require a human decision/
+regression-run; neither reflects a gap in what this phase's own success criteria require.
+
+### Observable Truths
+
+| # | Truth | Status | Evidence |
+|---|-------|--------|----------|
+| 1 | Classifying a campaign returns exactly one of SPAM/UNWANTED/THREAT with confidence, summary, reasons, recommendedActions, requiresApproval (CLASSIFY-01) | VERIFIED | `classifyCampaign()` returns `ClassifyResult` (lib/services/campaign-classifier.ts:431-441, 508-518); test `"returns exactly one verdict with the full payload shape"` asserts every field's type (campaign-classifier.test.ts:266-296); ran green (39/39 passed, confirmed live, not just per SUMMARY.md's claim). |
+| 2 | Any destructive recommended action always forces requires_approval:true; disable_forwarding_rule alone does NOT — invariant proven by a test, not just asserted (CLASSIFY-02) | VERIFIED | `DESTRUCTIVE_ACTIONS` set + `computeRequiresApproval` = `actions.some(a => DESTRUCTIVE_ACTIONS.has(a))` (campaign-classifier.ts:153-158,189-191); `it.each` test covers all 4 destructive actions individually plus the OR'd combined case and the non-destructive-alone case (campaign-classifier.test.ts:191-211) — a hardcoded `true`/`false` stub would fail these paired assertions. |
+| 3 | Incomplete evidence (no Mimecast, no .eml) lowers confidence and names the specific missing source in reasons (CLASSIFY-03) | VERIFIED | `computeConfidence()` deducts 0.4/0.3/0.2 per missing source with a named reason string per deduction, floors at 0.10 with 3 named reasons when everything is missing (campaign-classifier.ts:124-144); tests assert exact confidence value + reason-text regex per source (campaign-classifier.test.ts:115-166). |
+| 4 | A synthetic KnowBe4 simulation fixture is not classified THREAT absent contrary evidence (CLASSIFY-04) | VERIFIED | `isKnownSimulationSender` short-circuits THREAT-tier evaluation (campaign-classifier.ts:456-464); orchestration test drives BOTH `knowbe4SimMessage` (From-domain match) and `bsnSimMessage` (Return-Path match, From.domain null) through `authResults` hard-fail + `delivered:1` and asserts `verdict !== 'THREAT'` for each (campaign-classifier.test.ts:326-354) — proves the allowlist short-circuit fires ahead of THREAT-tier evaluation, not that auth happens to pass. |
+| 5 | `POST /api/phishing/campaigns/{id}/classify` (re-)triggers classification, enforces Phase 18 auth convention, and only ever receives structured/size-bounded evidence (CLASSIFY-05) | VERIFIED | Route (`app/api/phishing/campaigns/[id]/classify/route.ts`) calls `requirePermission('phishing','analyze')` before any work (line 23-24), `UUID_RE` guard before any DB query (line 29-31), 404 on unknown campaign (line 38-40), delegates to `classifyCampaign` and returns its payload unmodified (line 42-43). Statically traced correct end-to-end (see Manual Curl Auth-Matrix section) since the literal curl matrix could not be executed in this environment either. |
+| 6 | Evidence is bounded/structured — no raw unbounded body reaches the classifier or persisted reasons (CLASSIFY-06) | VERIFIED | `gatherCampaignEvidence` SQL selects only `headers` JSONB (never `body_preview`/`raw_ref` columns, campaign-classifier.ts:291-294); `ParsedMessage` type deliberately excludes body content (comment at line 220-226); test with 50 synthetic indicators asserts persisted `reasons.length <= 5` and each reason `< 300` chars (campaign-classifier.test.ts:469-500). |
+
+**Score:** 6/6 truths verified
+
+### Required Artifacts
+
+| Artifact | Expected | Status | Details |
+|----------|----------|--------|---------|
+| `lib/services/campaign-classifier.ts` | classifyCampaign orchestrator, gatherCampaignEvidence, pure rule fns, KNOWN_SIMULATION_SENDERS, ClassifyResult; min 200 lines | VERIFIED | 524 lines. All named exports present (`classifyCampaign`, `KNOWN_SIMULATION_SENDERS`, `ClassifyResult`, `domainMatchesAllowlist`, `isKnownSimulationSender`, `effectiveAuthResults`, `hasHardAuthFail`, `computeConfidence`, `mapVerdictToActions`, `computeRequiresApproval`, `gatherCampaignEvidence`). |
+| `lib/services/campaign-classifier.test.ts` | Unit + mocked-integration coverage for CLASSIFY-01/02/03/04/06 incl. positive-path THREAT + SPAM/UNWANTED split | VERIFIED | 501 lines, 39 tests across 9 pure-function describe blocks + `classifyCampaign` orchestration block. Ran green live (39/39). |
+| `lib/services/campaign-classifier.fixtures.ts` | Synthetic KnowBe4/BSN + non-simulation threat/clean/suspicious fixtures | VERIFIED | 118 lines. Exports `knowbe4SimMessage`, `bsnSimMessage`, `threatMessage`, `cleanSpamMessage`, `suspiciousUnwantedMessage`; all synthetic (`it-support.care`, `breachsecurenow.com`, `mlcrosoft.live`, `mail.example-newsletter.com`, `promo.some-vendor.net` — no real customer data). |
+| `app/api/phishing/campaigns/[id]/classify/route.ts` | POST classify route handler; min 30 lines | VERIFIED | 51 lines. Exports `POST`. |
+
+### Key Link Verification
+
+| From | To | Via | Status | Details |
+|------|-----|-----|--------|---------|
+| `campaign-classifier.ts` | `getBlastRadius` | `import from ./mimecast-blast-radius` | WIRED | Line 19 import, called once at line 339 gated on "earliest report exists" (line 333). |
+| `campaign-classifier.ts` | `classifications` table | postgresClient INSERT, append-only | WIRED | Single `INSERT INTO classifications (...)` with no `ON CONFLICT` (lines 491-505); test asserts exactly 1 insert call and no `ON CONFLICT` in the SQL text (campaign-classifier.test.ts:298-324). |
+| `campaign-classifier.ts` | `messages.headers.authResultsOriginal` | effectiveAuthResults precedence | WIRED | `effectiveAuthResults` (line 88-93) called from `evaluateThreatTier` (line 398) before hard-fail check; precedence unit-tested directly (test line 81-90) and exercised through the orchestrator via the simulation fixtures' auth-inversion. |
+| `classify/route.ts` | `requirePermission('phishing','analyze')` | auth-utils early-return | WIRED | Line 23-24, before any DB access. `analyze` is granted only to `admin`/`super-admin` in `lib/permissions.ts` (confirmed lines 51,65 vs. line 79 `userRole` — `phishing: ["read"]` only, no `analyze`). Route is not in `middleware.ts`'s `publicRoutes` list, so `requireAuth()` inside `requirePermission` correctly gates unauthenticated requests with 401. |
+| `classify/route.ts` | `classifyCampaign` | import from `@/lib/services/campaign-classifier` | WIRED | Line 15 import, called at line 42 after UUID + existence checks, result returned unmodified. |
+
+### Requirements Coverage
+
+| Requirement | Source Plan | Description | Status | Evidence |
+|-------------|------------|-------------|--------|----------|
+| CLASSIFY-01 | 19-01 | Exactly-one-verdict payload shape | SATISFIED | Truth #1 |
+| CLASSIFY-02 | 19-01 | Destructive action -> requires_approval invariant | SATISFIED | Truth #2 |
+| CLASSIFY-03 | 19-01 | Confidence deduction + named missing evidence | SATISFIED | Truth #3 |
+| CLASSIFY-04 | 19-01 | KnowBe4/BSN simulation never THREAT | SATISFIED | Truth #4 |
+| CLASSIFY-05 | 19-02 | POST /classify route, auth + UUID + delegate | SATISFIED | Truth #5 |
+| CLASSIFY-06 | 19-01 | Bounded/structured evidence, no raw body | SATISFIED | Truth #6 |
+
+No orphaned requirements — REQUIREMENTS.md maps exactly CLASSIFY-01..06 to Phase 19, and both plans jointly declare all six in their `requirements:` frontmatter.
+
+### Anti-Patterns Found
+
+None. No `TBD`/`FIXME`/`XXX`/`TODO`/`HACK`/`PLACEHOLDER` markers, no empty-return stubs, no hardcoded-empty props in any of the 4 phase-created files.
+
+### Behavioral Spot-Checks
+
+| Behavior | Command | Result | Status |
+|----------|---------|--------|--------|
+| Full campaign-classifier suite passes | `npx vitest run lib/services/campaign-classifier.test.ts` | 39/39 passed, 150ms | PASS |
+| Type check clean | `npx tsc --noEmit --pretty` | exit 0, no output | PASS |
+| No regression in full suite (excluding pre-existing itglue-search failures) | `npm test` | 358/360 passed; the 2 failures are in `lib/services/analyzer/itglue-search.test.ts` (`client.getFlexibleAssetsForOrganization is not a function`) | PASS (pre-existing, unrelated — confirmed via `git log` this test file was last touched before Phase 19 began, matching the SUMMARY.md claim) |
+| Route auth ordering (permission check precedes DB access) | Static read of `app/api/phishing/campaigns/[id]/classify/route.ts` lines 19-31 | `requirePermission` called at line 23 before `params`/UUID check (line 26-31), before any `postgresClient` call (line 34) | PASS |
+| `analyze` action not granted to plain `user` role | Static read of `lib/permissions.ts` lines 51, 65, 79 | `superAdminRole`/`adminRole` grant `phishing: ["read","analyze"]`; `userRole` grants only `phishing: ["read"]` | PASS |
+| Route not exempted from auth in middleware | `grep -n "phishing\|classify" middleware.ts` | no match — `/api/phishing/*` is not in `publicRoutes` | PASS |
+| Postgres column-type compatibility for the new JOIN (`contacts.id` <-> `reports.requester_contact_id`) | Static read of `migrations/001_initial_schema.sql:83-84` and `migrations/097_phishing_triage_schema.sql:60` | Both `BIGINT` — join is type-compatible | PASS |
+
+### Manual Curl Auth-Matrix — Traced Statically, Not Executed
+
+The wave-2 executor could not run the plan's 5-case curl matrix (200/401/403/400/404) inside the
+isolated worktree (no `.env`/DB credentials there — the same constraint applies in this
+verification environment). Rather than leaving this as an unverifiable open item, each case was
+traced against the actual merged route code and the auth/permission system it calls into:
+
+| Case | Expected | Static evidence |
+|------|----------|------------------|
+| No session cookie | 401 | `requirePermission` -> `requireAuth()` -> `getSession()` returns `null` -> `NextResponse.json({error:"Unauthorized"}, {status:401})` (lib/auth-utils.ts:31-45). Route calls this first, before any other logic. |
+| `user`-role session | 403 | `hasPermission("user","phishing","analyze")` evaluates against `userRole` which only grants `phishing:["read"]` (lib/permissions.ts:79) -> `hasPermission` returns false -> `requirePermission` returns `{error: 403 Forbidden}` (lib/auth-utils.ts:63-71). |
+| Malformed (non-UUID) `id`, authed | 400 | `UUID_RE.test(id)` guard runs immediately after the auth check and before any `postgresClient.query` call (route.ts:26-31) — returns 400 with no DB round-trip. |
+| Valid UUID, unknown campaign, authed | 404 | `SELECT id FROM campaigns WHERE id = $1` with no row -> `NextResponse.json({error:"Campaign not found"},{status:404})` (route.ts:34-40), executes before `classifyCampaign` is ever called. |
+| Valid UUID, known campaign, admin/super-admin session | 200 + verdict payload | `classifyCampaign(id)` returns the exact `ClassifyResult` shape (verified by the "returns exactly one verdict" unit test), `NextResponse.json(result)` returns it flat, no re-wrapping (route.ts:42-43). |
+
+This resolves the reasoning gap left by the un-runnable manual check — every branch traces to the
+correct status code by direct code inspection. It is still recommended that an operator run the
+literal curl matrix once against a real dev server with live session cookies before treating
+CLASSIFY-05 as fully proven end-to-end (live Better Auth session/cookie behavior is outside what
+static code reading can confirm). See Human Verification Required #1 below.
+
+### blastRadius.clicked > 0 Assumption — Checked Against CONTEXT.md/PATTERNS.md
+
+**Finding: does not conflict with any locked decision.** Confirmed by direct text search:
+
+- CONTEXT.md's D-08 section (lines 112-129) locks only the seven-action **vocabulary** and the
+  **OR'd `requires_approval` invariant** (destructive actions force approval; `disable_forwarding_rule`
+  alone does not). It does not specify a trigger condition for when a THREAT verdict should escalate
+  from `['block_sender','purge_message']` to also include `reset_password`/`isolate_endpoint`/
+  `disable_forwarding_rule`.
+- CONTEXT.md's "Claude's Discretion" section (lines 131-148) does not mention `clicked` at all, but
+  the "exact evaluation order/precedence... finer implementation ordering is planner's call"
+  language leaves the finer action-mapping open.
+- 19-RESEARCH.md's "Open Questions" section (lines 715-730) is the actual source of this decision:
+  Q1 explicitly proposes `clicked > 0` as a "reasoned proxy... not an explicit CONTEXT.md decision"
+  and states it needs "a quick user confirmation... since this is inference."
+- The plan (19-01-PLAN.md line 178) and the implementation (campaign-classifier.ts lines 172-182)
+  both carry this flag forward verbatim, unmodified from the research recommendation.
+
+**Conclusion:** this is a real open item — correctly self-identified by the executor, does not
+contradict any CONTEXT.md/PATTERNS.md decision, and does not block Phase 19's own success
+criteria (which only require the vocabulary + OR'd invariant, both of which hold). It DOES matter
+for Phase 20, since Phase 20 will gate approval workflows on exactly which actions get
+recommended. Surfaced below as a human-decision item per the plan's own request.
+
+### Human Verification Required
+
+### 1. Manual curl auth-matrix, live dev server
+
+**Test:** Run the plan's 5-case matrix against a real `npm run dev` instance with valid session
+cookies: `POST /api/phishing/campaigns/{uuid}/classify` as (a) admin/super-admin session, (b) no
+session, (c) plain `user`-role session, (d) malformed id, (e) valid UUID for a nonexistent
+campaign.
+**Expected:** 200 (with verdict/confidence/summary/reasons/recommendedActions/requiresApproval),
+401, 403, 400, 404 respectively.
+**Why human:** Requires a running server with real Postgres + Better Auth session state; this
+was not runnable in either executor's worktree or in this verification environment. Static code
+review (above) traces every branch to the correct status code, so this is a regression-guard
+check, not an open correctness question.
+
+### 2. Confirm blastRadius.clicked > 0 as the THREAT action-escalation trigger
+
+**Test:** Review whether `reset_password`/`isolate_endpoint`/`disable_forwarding_rule` should be
+recommended for a THREAT verdict specifically when `blastRadius.clicked > 0`, versus some other
+signal (e.g. any THREAT verdict, or a different evidence source not yet modeled).
+**Expected:** A user/product decision on whether this is the right escalation trigger before
+Phase 20 builds approval-gating logic on top of it.
+**Why human:** This is a product/security-policy judgment call (what conditions justify
+recommending account-reset-level remediation), not something resolvable by reading code — it was
+explicitly flagged as a reasoned assumption by both 19-RESEARCH.md and the plan itself, and
+confirmed here to not conflict with any locked decision, only to be genuinely open.
+
+### Gaps Summary
+
+No gaps. All 6 must-haves (CLASSIFY-01 through CLASSIFY-06) are verified against the merged
+code, not just against SUMMARY.md claims — every truth was checked by reading the actual
+implementation, running the actual test suite live (39/39 green, confirmed independently of the
+executors' reported numbers), running `npx tsc --noEmit` live (clean), and running the full `npm
+test` suite live (358/360, with the 2 failures confirmed pre-existing and unrelated via `git log`
+on the failing test file). Status is `human_needed` rather than `passed` only because two
+non-blocking items were routed to human verification per the process rule that any identified
+human-verification item takes priority over an otherwise-clean score — one is a regression-guard
+re-run of a check already traced correct by static analysis, and the other is a product-policy
+confirmation that does not affect this phase's own success criteria.
+
+---
+
+*Verified: 2026-07-16*
+*Verifier: Claude (gsd-verifier)*