fix(21): join real url indicators + cast NUMERIC confidence in triage-note plan

.planning/phases/21-autotask-triage-note/21-02-PLAN.md

@@ -17,15 +17,17 @@ must_haves:
     - "For a classified campaign the endpoint posts an internal (non-portal) Autotask TicketNote to every ticket linked to the campaign"
     - "The API response always includes the generated sanitized note text plus a per-ticket posted/error status list"
     - "A single ticket's Autotask write failure is captured per-ticket and does not abort the whole call, and no unsanitized/partial write is attempted as a fallback"
+    - "When the campaign's linked messages have extracted url indicators (Phase 16), those indicator URLs are gathered via the reports→messages→indicators join and flow (sanitized) into the posted note text"
+    - "confidence, read from the NUMERIC classifications.confidence column, is coerced to a JS number (not a pg-string) before being placed on TriageNoteEvidence"
   artifacts:
     - path: "lib/services/triage-note-service.ts"
-      provides: "generateAndPostTriageNote(campaignId) orchestrator: evidence gather + note post loop + partial-failure result"
+      provides: "generateAndPostTriageNote(campaignId) orchestrator: evidence gather + indicator-URL join + note post loop + partial-failure result"
       contains: "export async function generateAndPostTriageNote"
     - path: "app/api/phishing/campaigns/[id]/triage-note/route.ts"
       provides: "POST route: permission gate + UUID guard + campaign-exists check + service delegation"
       exports: ["POST"]
     - path: "lib/services/triage-note-service.test.ts"
-      provides: "Vitest coverage of per-ticket write loop + partial-failure + note text return"
+      provides: "Vitest coverage of per-ticket write loop + partial-failure + indicator-URL flow + note text return"
   key_links:
     - from: "app/api/phishing/campaigns/[id]/triage-note/route.ts"
       to: "lib/services/triage-note-service.ts"
@@ -39,6 +41,10 @@ must_haves:
       to: "lib/services/triage-note-format.ts"
       via: "formatTriageNote(evidence)"
       pattern: "formatTriageNote"
+    - from: "lib/services/triage-note-service.ts"
+      to: "indicators (via messages/reports)"
+      via: "JOIN messages ON report_id JOIN indicators ON message_id WHERE indicator_type = 'url'"
+      pattern: "indicator_type = 'url'"
     - from: "app/api/phishing/campaigns/[id]/triage-note/route.ts"
       to: "lib/auth-utils.ts"
       via: "requirePermission('phishing', 'analyze')"
@@ -46,7 +52,7 @@ must_haves:
 ---
 
 <objective>
-Build the triage-note service and its on-demand API route. The service gathers a campaign's current evidence (linked reports/tickets, most-recent classification, current remediation state, fresh blast radius), renders the sanitized note via Plan 01's formatter, and posts one internal Autotask TicketNote per linked ticket — capturing each write's success/failure independently. The route is a thin, access-controlled controller matching the existing `classify` route shape.
+Build the triage-note service and its on-demand API route. The service gathers a campaign's current evidence (linked reports/tickets, extracted url indicators, most-recent classification, current remediation state, fresh blast radius), renders the sanitized note via Plan 01's formatter, and posts one internal Autotask TicketNote per linked ticket — capturing each write's success/failure independently. The route is a thin, access-controlled controller matching the existing `classify` route shape.
 
 Purpose: Satisfies NOTE-01 — Pulse already has a safe Autotask note-write path (`createEntity('TicketNotes', ...)`), so this phase posts the note to every ticket linked to the campaign (D-01) via a new operator-triggered endpoint (D-02), always posting fresh state (D-03), including the full picture of classification + blast radius + current remediation state (D-04), and reporting per-ticket write outcomes so a failed write never aborts the call or gets silently swallowed (D-05/D-06).
 
@@ -88,10 +94,18 @@ Output: `lib/services/triage-note-service.ts`, its test, and `app/api/phishing/c
 <!-- SCHEMA (read-only) migrations/097_phishing_triage_schema.sql -->
 <!--   reports(id, ticket_id BIGINT NOT NULL, ticket_number, company_id, company_name, requester_contact_id, -->
 <!--           title, description, evidence JSONB, campaign_id, created_at)  UNIQUE(ticket_id) -->
-<!--   classifications(campaign_id, verdict, confidence, summary, reasons JSONB, recommended_actions JSONB, requires_approval, created_at) -->
+<!--   messages(id, report_id UUID REFERENCES reports(id), message_id, headers JSONB, urls JSONB, attachments JSONB, body_preview) -->
+<!--   indicators(id, message_id UUID REFERENCES messages(id), indicator_type TEXT, value TEXT)  -- Phase 16 extracted URLs/senders/hashes -->
+<!--   classifications(campaign_id, verdict, confidence NUMERIC, summary, reasons JSONB, recommended_actions JSONB, requires_approval, created_at) -->
+<!--     ^ confidence is NUMERIC -> node-pg returns it as a JS STRING (no setTypeParser override in postgres-client.ts). -->
+<!--       app/api/phishing/campaigns/[id]/route.ts types the same column `confidence: string | null` — MUST cast to number. -->
 <!--   remediation_actions(campaign_id, action_type, status, params JSONB, approved_by, approved_at, created_at) -->
 <!--   campaigns(id, status, report_count) -->
 
+<!-- INDICATOR-URL JOIN precedent: app/api/phishing/campaigns/[id]/route.ts already walks -->
+<!--   reports -> messages (report_id) -> indicators (message_id), then filters by indicator_type. -->
+<!--   Copy that join shape to gather url indicators scoped to a campaign. -->
+
 <!-- ROUTE TWIN (copy structure verbatim): app/api/phishing/campaigns/[id]/classify/route.ts -->
 <!--   default import postgresClient; requirePermission('phishing','analyze'); UUID_RE guard; -->
 <!--   SELECT id FROM campaigns WHERE id=$1 -> 404; delegate; NextResponse.json(result); -->
@@ -102,10 +116,11 @@ Output: `lib/services/triage-note-service.ts`, its test, and `app/api/phishing/c
 <tasks>
 
 <task type="auto" tdd="true">
-  <name>Task 1: triage-note-service (evidence gather + per-ticket post loop)</name>
+  <name>Task 1: triage-note-service (evidence gather + indicator-URL join + per-ticket post loop)</name>
   <files>lib/services/triage-note-service.ts, lib/services/triage-note-service.test.ts</files>
   <read_first>
-    - lib/services/triage-note-format.ts (TriageNoteEvidence contract + formatTriageNote — from Plan 01)
+    - lib/services/triage-note-format.ts (TriageNoteEvidence contract + formatTriageNote — from Plan 01; note confidence is typed `number | null`, urls is `string[]`)
+    - app/api/phishing/campaigns/[id]/route.ts (the EXACT reports→messages→indicators join + indicator_type filter to copy for the url-indicator gather; also confirms classifications.confidence is read as `string | null` from the NUMERIC column — the reason a cast is required here)
     - lib/services/campaign-classifier.ts (evidence-gathering query shapes: reports WHERE campaign_id, getBlastRadius call inputs, classification/reasons handling)
     - lib/services/remediation-service.ts (most-recent-row ORDER BY created_at DESC LIMIT 1 idiom; remediation_actions column reads)
     - lib/services/workflow-engine.ts (runAiTroubleshooting ~lines 560-613: the createEntity('TicketNotes', {...}) write shape to copy — noteType:1 Internal, publish:1)
@@ -119,17 +134,21 @@ Output: `lib/services/triage-note-service.ts`, its test, and `app/api/phishing/c
     - result.tickets has one entry per linked report; all posted:true when every write succeeds
     - When getAutotaskClient().createEntity rejects for exactly one of the 3 tickets, that ticket's entry is posted:false with a non-empty error string, the other two are posted:true, and the promise still resolves (the whole call is NOT aborted) — D-05
     - result.noteText is always present and non-empty regardless of any write outcome — D-06 (text never withheld)
-    - noteText is produced by formatTriageNote(evidence) built from the most-recent classification row, current remediation_actions rows, and a getBlastRadius() result — so an evidence URL with a token query param is absent from noteText (sanitization flows through)
+    - noteText is produced by formatTriageNote(evidence) built from the most-recent classification row, current remediation_actions rows, extracted url indicators, and a getBlastRadius() result
+    - When the campaign's linked reports have messages with `indicator_type = 'url'` indicator rows, those indicator values populate evidence.urls and appear (sanitized) in noteText; a url indicator carrying a token query param (e.g. 'http://evil.example/p?token=leak') is present as its sanitized form and the substring 'token=leak' is absent from noteText
+    - When there are no url indicators for the campaign, evidence.urls is [] and the call still succeeds (no url section content required)
+    - confidence placed on the TriageNoteEvidence object is a JS number (or null), never a pg string — a classification row with confidence 0.92 yields evidence.confidence === 0.92 (typeof 'number'), not '0.92'
     - A campaign with zero linked reports resolves { noteText, tickets: [] } (still returns the note text; no writes attempted) rather than throwing
     - A campaign with no classification row still resolves (note renders 'not yet classified' via formatter) rather than throwing
   </behavior>
   <action>
     Create `lib/services/triage-note-service.ts` exporting `interface TriageNotePostResult { ticketId: string; posted: boolean; error?: string }`, `interface TriageNoteResult { noteText: string; tickets: TriageNotePostResult[] }`, and `async function generateAndPostTriageNote(campaignId: string): Promise<TriageNoteResult>`.
-    Read side: import the default `postgresClient` from `./postgres-client`. Query all linked reports with `SELECT id::text, ticket_id::text AS ticket_id, ticket_number, title, company_name, requester_contact_id, evidence, created_at FROM reports WHERE campaign_id = $1 ORDER BY created_at ASC`. Query the most-recent classification with `SELECT verdict, confidence, summary, reasons, recommended_actions, requires_approval, created_at FROM classifications WHERE campaign_id = $1 ORDER BY created_at DESC LIMIT 1` (nullable — handle absence). Query current remediation state with `SELECT action_type, status, approved_by, approved_at::text AS approved_at FROM remediation_actions WHERE campaign_id = $1 ORDER BY created_at ASC`. Parse JSONB `reasons`/`recommended_actions` into `string[]` defensively (already-array vs JSON string). Derive candidate `urls` from report `evidence` if present.
+    Read side: import the default `postgresClient` from `./postgres-client`. Query all linked reports with `SELECT id::text, ticket_id::text AS ticket_id, ticket_number, title, company_name, requester_contact_id, evidence, created_at FROM reports WHERE campaign_id = $1 ORDER BY created_at ASC`. Query the most-recent classification with `SELECT verdict, confidence::float8 AS confidence, summary, reasons, recommended_actions, requires_approval, created_at FROM classifications WHERE campaign_id = $1 ORDER BY created_at DESC LIMIT 1` (nullable — handle absence). The `confidence::float8` cast makes node-pg return a real JS number instead of the string it returns for a bare NUMERIC column; still defensively wrap with `row.confidence == null ? null : Number(row.confidence)` when assigning to the evidence object so the `TriageNoteEvidence.confidence: number | null` contract holds at runtime. Query current remediation state with `SELECT action_type, status, approved_by, approved_at::text AS approved_at FROM remediation_actions WHERE campaign_id = $1 ORDER BY created_at ASC`. Parse JSONB `reasons`/`recommended_actions` into `string[]` defensively (already-array vs JSON string).
+    URL indicators: gather the campaign's extracted url indicators by copying the reports→messages→indicators join used in `app/api/phishing/campaigns/[id]/route.ts`, filtered to url indicators: `SELECT i.value FROM indicators i JOIN messages m ON m.id = i.message_id JOIN reports r ON r.id = m.report_id WHERE r.campaign_id = $1 AND i.indicator_type = 'url'`. Map the rows to a `string[]` for `evidence.urls` (dedupe if you like; not required). This is the real source of indicator URLs — `reports.evidence` (an EvidencePayload of company_name/notes/time_entries/attachments per phishing-detector.ts) has NO url field, so do NOT try to derive urls from it. If the campaign has no messages/indicators yet (Phase 16 not run for these reports), the query returns zero rows and `evidence.urls` is `[]` — that is expected and fine.
     Blast radius: call `getBlastRadius({ sender, recipient, subject, dateWindow: { start, end } })` fresh (D-04 current truth) using best-available sender/recipient/subject derived from the primary report's evidence/title and a ±24h window around the primary report `created_at` — copy the input-shaping approach from `campaign-classifier.ts`. `getBlastRadius` never throws (returns `{ status: 'unavailable' }` when Mimecast is not configured), so no guard needed beyond passing what you have.
-    Build a `TriageNoteEvidence` object (import the interface + `formatTriageNote` from `./triage-note-format`) mapping: campaignId, reportCount = reports.length, companyName/subject from the primary report, verdict/confidence/summary/reasons/recommendedActions/requiresApproval from the classification (nulls when absent), blastRadius, remediationActions (map action_type→actionType, approved_by→approvedBy, approved_at→approvedAt), urls. Call `formatTriageNote(evidence)` to get `noteText`.
+    Build a `TriageNoteEvidence` object (import the interface + `formatTriageNote` from `./triage-note-format`) mapping: campaignId, reportCount = reports.length, companyName/subject from the primary report, verdict/confidence (coerced to number|null as above)/summary/reasons/recommendedActions/requiresApproval from the classification (nulls when absent), blastRadius, remediationActions (map action_type→actionType, approved_by→approvedBy, approved_at→approvedAt), urls (the joined indicator values). Call `formatTriageNote(evidence)` to get `noteText` (the formatter routes urls through sanitizeUrl and the whole string through sanitizeNoteText).
     Write side: `const client = getAutotaskClient();` then loop the reports; for each, in its OWN try/catch, `await client.createEntity('TicketNotes', { ticketID: Number(report.ticket_id), title: 'Phishing Triage Summary', description: noteText, noteType: 1, publish: 1 })` and push `{ ticketId: report.ticket_id, posted: true }`; on catch, `console.error('[PHISHING-TRIAGE-NOTE] Failed to post note to ticket', report.ticket_id, err)` and push `{ ticketId, posted: false, error: err instanceof Error ? err.message : 'Unknown error' }`. The per-ticket try/catch MUST be inside the loop (not around it) so one failure never aborts remaining writes (D-05). Return `{ noteText, tickets }`.
-    Write `lib/services/triage-note-service.test.ts` following `remediation-service.test.ts`: `vi.mock('./postgres-client', ...)` with a `queryMock` returning seeded reports/classification/remediation rows per SQL, `vi.mock('./autotask-factory', ...)` exposing a `createEntityMock`, and `vi.mock('./mimecast-blast-radius', ...)` returning a fixed BlastRadiusResult. Cover every <behavior> bullet, especially the one-ticket-fails-others-succeed case and the note-text-always-returned case.
+    Write `lib/services/triage-note-service.test.ts` following `remediation-service.test.ts`: `vi.mock('./postgres-client', ...)` with a `queryMock` returning seeded reports/classification/remediation/indicator rows per SQL (match the indicator query by its `indicator_type = 'url'` / `FROM indicators` substring and return url-indicator rows), `vi.mock('./autotask-factory', ...)` exposing a `createEntityMock`, and `vi.mock('./mimecast-blast-radius', ...)` returning a fixed BlastRadiusResult. Cover every <behavior> bullet, especially: (a) the one-ticket-fails-others-succeed case, (b) the note-text-always-returned case, (c) the indicator-URL-with-token-query-param case asserting the sanitized url is in noteText and 'token=leak' is not, and (d) the confidence-is-a-number case asserting `typeof evidenceConfidence === 'number'` (assert via a fixture where confidence is returned as the string '0.92' from the mocked query — proving the cast/coercion runs even if the mock hands back a string like real pg would).
   </action>
   <verify>
     <automated>npx vitest run lib/services/triage-note-service.test.ts</automated>
@@ -139,11 +158,15 @@ Output: `lib/services/triage-note-service.ts`, its test, and `app/api/phishing/c
     - Source assertion: `grep -q "export async function generateAndPostTriageNote" lib/services/triage-note-service.ts`
     - Source assertion: `grep -q "createEntity('TicketNotes'" lib/services/triage-note-service.ts` and `grep -q "noteType: 1" lib/services/triage-note-service.ts` and `grep -q "publish: 1" lib/services/triage-note-service.ts`
     - Source assertion: `grep -q "formatTriageNote" lib/services/triage-note-service.ts` (uses Plan 01 formatter)
+    - Source assertion: `grep -q "indicator_type = 'url'" lib/services/triage-note-service.ts` (real indicator-URL join present, not derived from reports.evidence)
+    - Source assertion: `grep -Eq "confidence::float8|Number\(.*confidence" lib/services/triage-note-service.ts` (NUMERIC confidence cast/coerced to a JS number)
     - Behavior assertion: a test mocks createEntity to reject on the 2nd of 3 tickets and asserts tickets[1].posted===false, tickets[0].posted===true, tickets[2].posted===true, and the returned promise resolves (call not aborted)
     - Behavior assertion: a test asserts result.noteText is a non-empty string even when a write fails
+    - Behavior assertion: a test seeds a url indicator row 'http://evil.example/p?token=leak' and asserts the sanitized url ('http://evil.example/p') is a substring of result.noteText while 'token=leak' is NOT — proving indicator URLs flow into the posted note when present
+    - Behavior assertion: a test seeds a classification whose confidence comes back as the string '0.92' from the mocked query and asserts the value placed on TriageNoteEvidence is `typeof 'number'` and === 0.92
     - `npx tsc --noEmit --pretty` reports no new errors
   </acceptance_criteria>
-  <done>Service gathers current campaign evidence, renders the sanitized note, posts one internal TicketNote per linked ticket with independent per-ticket error capture, and returns note text + per-ticket status; all tests pass; type-check clean.</done>
+  <done>Service gathers current campaign evidence (including real url indicators via the reports→messages→indicators join and a numeric-coerced confidence), renders the sanitized note, posts one internal TicketNote per linked ticket with independent per-ticket error capture, and returns note text + per-ticket status; all tests pass; type-check clean.</done>
 </task>
 
 <task type="auto">
@@ -182,7 +205,7 @@ Output: `lib/services/triage-note-service.ts`, its test, and `app/api/phishing/c
 | Boundary | Description |
 |----------|-------------|
 | client → API route | Untrusted caller hits POST /api/phishing/campaigns/{id}/triage-note with an arbitrary campaign id |
-| campaign evidence → Autotask note | Attacker-controlled email content flows through the formatter into notes written to Autotask tickets |
+| campaign evidence → Autotask note | Attacker-controlled email content (incl. extracted url indicators) flows through the formatter into notes written to Autotask tickets |
 | Autotask API → API response | External write outcomes (incl. error messages) are surfaced back to the caller |
 
 ## STRIDE Threat Register
@@ -190,23 +213,25 @@ Output: `lib/services/triage-note-service.ts`, its test, and `app/api/phishing/c
 | Threat ID | Category | Component | Disposition | Mitigation Plan |
 |-----------|----------|-----------|-------------|-----------------|
 | T-21-04 | Elevation of Privilege | route permission tier | mitigate | Route calls `requirePermission('phishing', 'analyze')` and early-returns 401/403 before any DB read or Autotask write — same convention as every other /api/phishing/* route (ACCESS-01 carry-forward). Note generation is informational (not a destructive state change), so 'analyze' (not 'approve') is the correct tier per CONTEXT.md discretion. |
-| T-21-05 | Tampering / Injection | UUID guard + parameterized queries | mitigate | `UUID_RE` rejects malformed ids with 400 before querying; all reads use `$1` parameterized queries (no string interpolation), preventing SQL injection via the id or campaign data. |
-| T-21-06 | Information Disclosure | note content written to Autotask | mitigate | Note text is produced solely by Plan 01's `formatTriageNote`, which routes all URLs/free-text through `sanitizeNoteText`/`sanitizeUrl` (query strings + tokens stripped) — no raw malicious URL or secret is written. Notes are `noteType:1`/`publish:1` (internal Autotask users, never customer portal per AUTOTASK_API_GUIDE.md). |
+| T-21-05 | Tampering / Injection | UUID guard + parameterized queries | mitigate | `UUID_RE` rejects malformed ids with 400 before querying; all reads (including the reports→messages→indicators url join) use `$1` parameterized queries (no string interpolation), preventing SQL injection via the id or campaign data. |
+| T-21-06 | Information Disclosure | note content written to Autotask | mitigate | Note text is produced solely by Plan 01's `formatTriageNote`, which routes all URLs (including the joined url indicators) through `sanitizeUrl` and the whole string through `sanitizeNoteText` (query strings + tokens stripped) — no raw malicious URL or secret is written. Notes are `noteType:1`/`publish:1` (internal Autotask users, never customer portal per AUTOTASK_API_GUIDE.md). |
 | T-21-07 | Information Disclosure | partial-failure response detail | accept | Per-ticket `error` returns only `err.message` (Autotask API message), never a stack trace or internal path; the whole-request catch likewise returns only `err.message`. Operator-facing internal tool; bounded error text is acceptable and necessary for the "paste failed note manually" workflow (D-06). |
 | T-21-08 | Denial of Service / Repudiation | no note-history persistence | accept | No audit/history row is written (CONTEXT.md defers this); Autotask itself is the record of truth for posted notes. Repeated calls posting duplicate notes are expected/acceptable (D-03). |
 | T-21-SC | Tampering | npm/pip/cargo installs | accept | No new package installs in this plan; RESEARCH disabled for project and no install tasks present. |
 </threat_model>
 
 <verification>
-- `npx vitest run lib/services/triage-note-service.test.ts` — passes (per-ticket loop + partial-failure + note-text-always-returned)
+- `npx vitest run lib/services/triage-note-service.test.ts` — passes (per-ticket loop + partial-failure + indicator-URL flow + numeric-confidence + note-text-always-returned)
 - `npx tsc --noEmit --pretty` — no new type errors
 - `grep -q "requirePermission('phishing', 'analyze')" app/api/phishing/campaigns/[id]/triage-note/route.ts` — auth gate present
 - `grep -q "createEntity('TicketNotes'" lib/services/triage-note-service.ts` — safe write path used
+- `grep -q "indicator_type = 'url'" lib/services/triage-note-service.ts` — real indicator-URL join present
 </verification>
 
 <success_criteria>
 - POST /api/phishing/campaigns/{id}/triage-note is auth-gated (phishing:analyze), UUID-guarded, and 404s unknown campaigns (NOTE-01 + ACCESS-01 carry-forward)
-- For a classified campaign, an internal triage note summarizing classification + evidence + blast radius + current remediation state is posted to every linked ticket (D-01, D-04)
+- For a classified campaign, an internal triage note summarizing classification + evidence (incl. extracted url indicators) + blast radius + current remediation state is posted to every linked ticket (D-01, D-04)
+- Extracted url indicators are gathered via the reports→messages→indicators join and flow (sanitized) into the note when present; confidence is coerced from the NUMERIC column to a JS number on TriageNoteEvidence
 - The response always returns the sanitized note text plus a per-ticket posted/error status list (D-06)
 - A single ticket's write failure is captured per-ticket without aborting the call and without any unsanitized/partial fallback write (D-05)
 - All tests pass; type-check clean