From ec7c2114bf71068fd162ae8414dafd60f53ba35f Mon Sep 17 00:00:00 2001 From: lorentz Date: Thu, 16 Jul 2026 12:19:06 -0400 Subject: [PATCH] docs(phase-21): complete phase execution --- .planning/REQUIREMENTS.md | 86 +++++++++++----- .planning/ROADMAP.md | 2 +- .planning/STATE.md | 21 ++-- .../21-VERIFICATION.md | 97 +++++++++++++++++++ 4 files changed, 169 insertions(+), 37 deletions(-) create mode 100644 .planning/phases/21-autotask-triage-note/21-VERIFICATION.md diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 55d5834..dcce7d9 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -16,7 +16,7 @@ destructive remediation gated behind explicit human approval. "reported message destinations", "Microsoft directly") - [x] **DETECT-02**: Re-scanning does not reprocess a ticket already ingested unless its source ticket data has changed since last processed (idempotent) -- [ ] **DETECT-03**: An operator can trigger analysis of one specific ticket by ID +- [x] **DETECT-03**: An operator can trigger analysis of one specific ticket by ID on demand (`POST /api/phishing/tickets/{ticket_id}/analyze`) instead of waiting for the scheduled scan @@ -39,12 +39,12 @@ destructive remediation gated behind explicit human approval. ### Campaign Grouping -- [ ] **CAMP-01**: Reports are grouped into a campaign using original Message-ID +- [x] **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 -- [ ] **CAMP-02**: A campaign can accumulate many linked ticket reports and +- [x] **CAMP-02**: A campaign can accumulate many linked ticket reports and recipients over time as duplicates are detected -- [ ] **CAMP-03**: An operator can list campaigns and view a single campaign's +- [x] **CAMP-03**: An operator can list campaigns and view a single campaign's full detail (linked reports, messages, indicators, classification history) via API (`GET /api/phishing/campaigns`, `GET /api/phishing/campaigns/{id}`) @@ -78,26 +78,26 @@ destructive remediation gated behind explicit human approval. ### Remediation & Approval Safety -- [ ] **REMED-01**: Recommended remediation actions are recorded as `proposed` +- [x] **REMED-01**: Recommended remediation actions are recorded as `proposed` but never executed automatically in this milestone -- [ ] **REMED-02**: An authorized operator can approve a campaign's remediation +- [x] **REMED-02**: An authorized operator can approve a campaign's remediation via API (`POST /api/phishing/campaigns/{id}/approve`), recording approver, timestamp, and the exact approved action parameters -- [ ] **REMED-03**: `POST /api/phishing/campaigns/{id}/remediate` proceeds only +- [x] **REMED-03**: `POST /api/phishing/campaigns/{id}/remediate` proceeds only for approved actions against a configured, non-destructive-by-default provider path; otherwise it returns `not_implemented`/an explicit failure — it never silently succeeds without taking or logging an action -- [ ] **REMED-04**: Re-running remediation against an already-completed action +- [x] **REMED-04**: Re-running remediation against an already-completed action does not duplicate the destructive effect (idempotent) -- [ ] **REMED-05**: An operator can mark a campaign as a false positive via API +- [x] **REMED-05**: An operator can mark a campaign as a false positive via API (`POST /api/phishing/campaigns/{id}/mark-false-positive`) -- [ ] **REMED-06**: Every state-changing action (classify, approve, remediate, +- [x] **REMED-06**: Every state-changing action (classify, approve, remediate, mark-false-positive) is recorded as an audit event with actor, event type, and payload ### Autotask Integration -- [ ] **NOTE-01**: If Pulse already has a safe Autotask note-writing method, the +- [x] **NOTE-01**: If Pulse already has a safe Autotask note-writing method, the system can post an internal triage note summarizing classification, evidence, blast radius, and recommended actions (sanitized — no raw secrets/tokens/full malicious URL query strings); otherwise the note text is returned via API @@ -105,10 +105,38 @@ destructive remediation gated behind explicit human approval. ### Access Control -- [ ] **ACCESS-01**: All `/api/phishing/*` endpoints enforce existing Pulse auth +- [x] **ACCESS-01**: All `/api/phishing/*` endpoints enforce existing Pulse auth conventions (`requireAuth`/`requirePermission`), with approve/remediate requiring elevated permission beyond plain read access +### Approval UI (LiveLink) + +- [ ] **REVIEW-01**: A stable, ticket-ID-addressable Pulse route (e.g. + `/phishing/tickets/{ticketId}`) resolves the ticket to its campaign and + renders that campaign's review page, suitable as an Autotask LiveLink target + (LiveLink supplies the ticket ID as dynamic content, not the internal + campaign UUID), authenticated via the existing Better Auth session only — + no separate token or query-param auth scheme +- [ ] **REVIEW-02**: The page displays the campaign's timeline — linked + reports, classification history, and audit events (classify/approve/ + remediate/mark-false-positive) — in chronological order +- [ ] **REVIEW-03**: The page displays the gathered evidence — parsed EML + headers/URLs/attachments, sanitized body preview, and Mimecast blast-radius + data (including an explicit `unavailable` state when Mimecast isn't + configured) — never rendering a raw/unsanitized body or unredacted secrets +- [ ] **REVIEW-04**: The page displays the current classification (SPAM/ + UNWANTED/THREAT), confidence, reasons, and recommended remediation + action(s) +- [ ] **REVIEW-05**: An operator can approve, remediate, or mark a campaign as + a false positive directly from the page, calling the existing + `/api/phishing/campaigns/{id}` approve/remediate/mark-false-positive + endpoints and reflecting the resulting state (e.g. a remediated campaign + shows as remediated, not re-offered for approval) +- [ ] **REVIEW-06**: An operator without the elevated permission approve/ + remediate already require sees those actions disabled or hidden rather than + a failed request; the page enforces no separate or relaxed permission model + from the underlying APIs + ## v2 Requirements Deferred to future release. Tracked but not in current roadmap. @@ -151,14 +179,14 @@ Populated during roadmap creation. |-------------|-------|--------| | DETECT-01 | Phase 15 | Complete | | DETECT-02 | Phase 15 | Complete | -| DETECT-03 | Phase 18 | Pending | +| DETECT-03 | Phase 18 | Complete | | EVID-01 | Phase 15 | Complete | | EVID-02 | Phase 16 | Complete | | EVID-03 | Phase 16 | Complete | | EVID-04 | Phase 16 | Complete | -| CAMP-01 | Phase 18 | Pending | -| CAMP-02 | Phase 18 | Pending | -| CAMP-03 | Phase 18 | Pending | +| CAMP-01 | Phase 18 | Complete | +| CAMP-02 | Phase 18 | Complete | +| CAMP-03 | Phase 18 | Complete | | BLAST-01 | Phase 17 | Complete | | BLAST-02 | Phase 17 | Complete | | CLASSIFY-01 | Phase 19 | Pending | @@ -167,18 +195,24 @@ Populated during roadmap creation. | CLASSIFY-04 | Phase 19 | Pending | | CLASSIFY-05 | Phase 19 | Pending | | CLASSIFY-06 | Phase 19 | Pending | -| REMED-01 | Phase 20 | Pending | -| REMED-02 | Phase 20 | Pending | -| REMED-03 | Phase 20 | Pending | -| REMED-04 | Phase 20 | Pending | -| REMED-05 | Phase 20 | Pending | -| REMED-06 | Phase 20 | Pending | -| NOTE-01 | Phase 21 | Pending | -| ACCESS-01 | Phase 18 | Pending | +| REMED-01 | Phase 20 | Complete | +| REMED-02 | Phase 20 | Complete | +| REMED-03 | Phase 20 | Complete | +| REMED-04 | Phase 20 | Complete | +| REMED-05 | Phase 20 | Complete | +| REMED-06 | Phase 20 | Complete | +| NOTE-01 | Phase 21 | Complete | +| ACCESS-01 | Phase 18 | Complete | +| REVIEW-01 | Phase 22 | Pending | +| REVIEW-02 | Phase 22 | Pending | +| REVIEW-03 | Phase 22 | Pending | +| REVIEW-04 | Phase 22 | Pending | +| REVIEW-05 | Phase 22 | Pending | +| REVIEW-06 | Phase 22 | Pending | **Coverage:** -- v1 requirements: 26 total -- Mapped to phases: 26 (Phases 15-21) +- v1 requirements: 32 total +- Mapped to phases: 32 (Phases 15-22) - Unmapped: 0 ✓ --- diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index b7e6f5c..5a497cc 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -502,7 +502,7 @@ Phases execute in numeric order. v1.0 (Phases 1-9.1) shipped 2026-07-10. v2.0 (P | 18. Campaign Grouping & Phishing Analysis API | v3.0 | 5/5 | Complete | 2026-07-16 | | 19. Classification Engine | v3.0 | 2/2 | Complete | 2026-07-16 | | 20. Remediation, Approval & Audit Safety | v3.0 | 2/2 | Complete | 2026-07-16 | -| 21. Autotask Triage Note | v3.0 | 2/2 | Complete | 2026-07-16 | +| 21. Autotask Triage Note | v3.0 | 2/2 | Complete | 2026-07-16 | | 22. Approval UI (LiveLink) | v3.0 | 0/TBD | Not started | - | ### Phase 22: Approval UI (LiveLink) diff --git a/.planning/STATE.md b/.planning/STATE.md index 9f59ee4..6f4fca4 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -2,15 +2,15 @@ gsd_state_version: 1.0 milestone: v3.0 milestone_name: Phishing Triage Automation -status: executing -stopped_at: Phase 21 context gathered -last_updated: "2026-07-16T16:03:23.367Z" +status: ready_to_plan +stopped_at: Phase 21 complete (2/2) — ready to discuss Phase 22 +last_updated: 2026-07-16T16:19:01.306Z last_activity: 2026-07-16 -- Phase 21 execution started progress: total_phases: 8 completed_phases: 6 total_plans: 18 - completed_plans: 16 + completed_plans: 18 percent: 75 --- @@ -21,14 +21,14 @@ progress: See: .planning/PROJECT.md (updated 2026-07-14) **Core value:** A manager/security operator can see every phishing/spam report ticket automatically triaged, deduplicated into campaigns, and classified — with any destructive remediation gated behind explicit human approval. -**Current focus:** Phase 21 — autotask-triage-note +**Current focus:** Phase 22 — approval ui livelink addressable campaign review and approve ## Current Position -Phase: 21 (autotask-triage-note) — EXECUTING -Plan: 1 of 2 -Status: Executing Phase 21 -Last activity: 2026-07-16 -- Phase 21 execution started +Phase: 22 +Plan: Not started +Status: Ready to plan +Last activity: 2026-07-16 Progress: [░░░░░░░░░░] 0% @@ -36,7 +36,7 @@ Progress: [░░░░░░░░░░] 0% **Velocity:** -- Total plans completed: 58 (v1.0: 42, v2.0: 20 across phases 10-14 — see per-phase table) +- Total plans completed: 60 (v1.0: 42, v2.0: 20 across phases 10-14 — see per-phase table) - Average duration: — - Total execution time: 0.0 hours (v3.0) @@ -49,6 +49,7 @@ Progress: [░░░░░░░░░░] 0% | 15-21 (v3.0) | TBD | - | - | | 15 | 3 | - | - | | 18 | 5 | - | - | +| 21 | 2 | - | - | **Recent Trend:** diff --git a/.planning/phases/21-autotask-triage-note/21-VERIFICATION.md b/.planning/phases/21-autotask-triage-note/21-VERIFICATION.md new file mode 100644 index 0000000..fb50f72 --- /dev/null +++ b/.planning/phases/21-autotask-triage-note/21-VERIFICATION.md @@ -0,0 +1,97 @@ +--- +phase: 21-autotask-triage-note +verified: 2026-07-16T16:30:00Z +status: passed +score: 8/8 must-haves verified +overrides_applied: 0 +--- + +# Phase 21: Autotask Triage Note Verification Report + +**Phase Goal:** Once a campaign is classified, a human-readable, sanitized internal triage note either gets posted to the Autotask ticket (if a safe write path already exists) or is returned via API for manual use — never a raw/unsanitized dump, never a silent no-op. +**Verified:** 2026-07-16T16:30:00Z +**Status:** passed +**Re-verification:** No — initial verification + +## Goal Achievement + +### Observable Truths + +| # | Truth | Status | Evidence | +|---|-------|--------|----------| +| 1 | If Pulse has a safe existing Autotask note-writing method, triggering note generation posts an internal triage note summarizing classification, evidence, blast radius, and recommended actions to the originating ticket | ✓ VERIFIED | `workflow-engine.ts:583-589` is the pre-existing safe write path (`createEntity('TicketNotes', {..., noteType:1, publish:1})`), confirmed by direct inspection. `triage-note-service.ts:176-182` reuses the identical shape, looping over every `reports` row linked to the campaign (`campaign_id = $1`) and posting one `TicketNotes` write per ticket (D-01). `formatTriageNote` (`triage-note-format.ts:80-133`) renders verdict, confidence, summary, reasons, blast radius (both branches), recommended actions, and current remediation state as labeled prose sections. | +| 2 | The posted (or returned) note text is sanitized — no raw secrets/tokens/full malicious URL query strings appear in it | ✓ VERIFIED | `triage-note-sanitize.ts` — `sanitizeUrl` strips query+fragment unconditionally (origin+pathname only, `sanitizeUrl.ts:44-54`); `sanitizeNoteText` redacts Bearer/Authorization sequences and credential-style query params (`token\|secret\|password\|api[_-]?key\|key\|credential`, case-insensitive). `formatTriageNote` routes every URL through `sanitizeUrl` (line 115) and the entire assembled string through `sanitizeNoteText` (line 132) before returning — defense-in-depth so no field can bypass redaction. 25/25 Vitest tests pass, including explicit assertions that `'token=leak'`/`'access_token=SECRET'`/`'password=p'` substrings are absent from output while bare emails and 64-char hex hashes are preserved (evidence, not secrets, per CONTEXT.md). | +| 3 | If no safe note-writing path exists, the same note content is returned via the API response instead of attempting any Autotask write, and no partial/unsanitized write is ever attempted as a fallback | ✓ VERIFIED | Pulse DOES have a safe write path (confirmed truth #1), so the "otherwise" branch of NOTE-01 applies only to the D-05 per-ticket-failure case (per 21-CONTEXT.md's explicit reading of NOTE-01). `triage-note-service.ts:172-192` wraps each ticket's write in its OWN try/catch INSIDE the loop — one failure is captured as `{ posted:false, error }` without aborting remaining writes, and `noteText` (already fully sanitized) is unconditionally returned in the result regardless of write outcome (D-06). No retry-with-unsanitized-text or partial-write fallback path exists anywhere in the service. Verified by test "always returns non-empty noteText even when a write fails (D-06)" and "captures a single ticket write failure without aborting the remaining writes (D-05)" — both pass. | + +**Score:** 3/3 roadmap truths verified (plus 5 additional PLAN-level must-haves below, all verified) + +### Required Artifacts + +| Artifact | Expected | Status | Details | +|----------|----------|--------|---------| +| `lib/services/triage-note-sanitize.ts` | `sanitizeUrl` + `sanitizeNoteText` pure functions | ✓ VERIFIED | Both exported, pure (no I/O), `REDACTED_MARKER` constant present; module-level SECURITY-CRITICAL comment mirrors `itglue-redact.ts` convention | +| `lib/services/triage-note-format.ts` | `TriageNoteEvidence` interface + `formatTriageNote(evidence)` | ✓ VERIFIED | Interface exported with all required fields (campaignId, reportCount, verdict, confidence, blastRadius: BlastRadiusResult, remediationActions, urls, etc.); formatter imports and routes through sanitizer | +| `lib/services/triage-note-sanitize.test.ts` | Vitest coverage of URL-query stripping + secret redaction | ✓ VERIFIED | 8 test cases, all pass | +| `lib/services/triage-note-format.test.ts` | Vitest coverage of note sections + no-raw-secret invariant | ✓ VERIFIED | 9 test cases, all pass | +| `lib/services/triage-note-service.ts` | `generateAndPostTriageNote(campaignId)` orchestrator | ✓ VERIFIED | Exported async function; gathers reports/classification/remediation/indicator-URL evidence, calls formatter, posts per-ticket TicketNotes with independent try/catch | +| `lib/services/triage-note-service.test.ts` | Vitest coverage of per-ticket write loop + partial-failure + indicator-URL flow | ✓ VERIFIED | 8 test cases (all-succeed, one-fails-isolation, note-text-always-returned, sanitized-URL-flow, zero-indicators, string-confidence-coercion, zero-reports, no-classification), all pass | +| `app/api/phishing/campaigns/[id]/triage-note/route.ts` | POST route: permission gate + UUID guard + campaign-exists check + service delegation | ✓ VERIFIED | Exports `POST`; calls `requirePermission('phishing','analyze')`, UUID_RE guard (400), campaign-exists check (404), delegates to `generateAndPostTriageNote`, returns result verbatim, 500 catch on whole-request failure | + +### Key Link Verification + +| From | To | Via | Status | Details | +|------|-----|-----|--------|---------| +| `triage-note-format.ts` | `triage-note-sanitize.ts` | `import { sanitizeUrl, sanitizeNoteText }` | ✓ WIRED | Line 17; both functions called (lines 115, 132) | +| `triage-note-format.ts` | `mimecast-blast-radius.ts` | `import type { BlastRadiusResult }` | ✓ WIRED | Line 18; both discriminated-union branches rendered (`formatBlastRadiusSection`) | +| `route.ts` | `triage-note-service.ts` | `generateAndPostTriageNote(id)` | ✓ WIRED | Called at line 49, result returned verbatim via `NextResponse.json(result)` | +| `triage-note-service.ts` | Autotask TicketNotes | `getAutotaskClient().createEntity('TicketNotes', ...)` | ✓ WIRED | Line 176; `noteType: 1`, `publish: 1` matching the confirmed safe-write precedent in `workflow-engine.ts:583-589` | +| `triage-note-service.ts` | `triage-note-format.ts` | `formatTriageNote(evidence)` | ✓ WIRED | Line 168 | +| `triage-note-service.ts` | indicators (via messages/reports) | `JOIN messages ... JOIN reports ... WHERE indicator_type = 'url'` | ✓ WIRED | Lines 116-122; matches schema in `migrations/097_phishing_triage_schema.sql` exactly (indicators.message_id → messages.id, messages.report_id → reports.id, reports.campaign_id) | +| `route.ts` | `lib/auth-utils.ts` | `requirePermission('phishing', 'analyze')` | ✓ WIRED | Line 27; `phishing:analyze` confirmed granted to user/admin/super-admin in `lib/permissions.ts:33,50,64` (read-only role at line 78 lacks it, correctly gated) | + +### Data-Flow Trace (Level 4) + +| Artifact | Data Variable | Source | Produces Real Data | Status | +|----------|---------------|--------|---------------------|--------| +| `triage-note-service.ts` evidence object | `urls` | Real SQL join `indicators → messages → reports` filtered `indicator_type='url'`, NOT derived from `reports.evidence` (which has no url field, confirmed against schema and service comment) | Yes | ✓ FLOWING | +| `triage-note-service.ts` evidence object | `confidence` | `classifications.confidence::float8` SQL cast + defensive `Number()` runtime coercion — test explicitly seeds a string `'0.92'` from the mock and asserts `typeof === 'number'` | Yes | ✓ FLOWING | +| `triage-note-service.ts` evidence object | `remediationActions` | `SELECT ... FROM remediation_actions WHERE campaign_id = $1` — live current state, not frozen at classify-time | Yes | ✓ FLOWING | +| `triage-note-service.ts` evidence object | `blastRadius` | Fresh `getBlastRadius()` call per invocation (not read from a persisted/stale column) | Yes | ✓ FLOWING | + +### Behavioral Spot-Checks + +| Behavior | Command | Result | Status | +|----------|---------|--------|--------| +| Sanitizer + formatter + service unit test suites | `npx vitest run lib/services/triage-note-sanitize.test.ts lib/services/triage-note-format.test.ts lib/services/triage-note-service.test.ts` | 3 files, 25/25 tests passed | ✓ PASS | +| Type-check clean (whole repo, not just phase files) | `npx tsc --noEmit --pretty` | No output (0 errors) | ✓ PASS | +| Route reachable and requires auth (not a public bypass route) | `grep phishing middleware.ts` | No match — `/api/phishing/*` is NOT in the public-routes allowlist, so middleware's session-cookie check applies before `requirePermission` runs | ✓ PASS | +| Safe-write-path premise (NOTE-01's "if a safe path exists") independently confirmed | `grep -n "createEntity('TicketNotes'" workflow-engine.ts` | Found at line 583, `noteType:1`/`publish:1`, matching what the new service reuses | ✓ PASS | + +### Requirements Coverage + +| Requirement | Source Plan | Description | Status | Evidence | +|-------------|-------------|--------------|--------|----------| +| NOTE-01 | 21-01-PLAN.md, 21-02-PLAN.md | Post a sanitized internal triage note (classification, evidence, blast radius, recommended actions) to the originating ticket via the existing safe write path; if no such path existed, return note text via API instead | ✓ SATISFIED | Full pipeline exists and is tested end-to-end: sanitizer (25 tests incl. this file) → formatter → service (evidence gather + per-ticket post loop) → route (auth-gated, UUID-guarded, 404-on-missing-campaign). Note: `.planning/REQUIREMENTS.md` line 100 still shows `[ ]` (unchecked checkbox) for NOTE-01 despite the phase being functionally complete — this is a documentation-sync gap, not an implementation gap (see Anti-Patterns below). | + +No orphaned requirements found — REQUIREMENTS.md line 204 maps NOTE-01 to "Phase 21" and it is the only requirement ID declared across both plan frontmatters for this phase. + +### Anti-Patterns Found + +| File | Line | Pattern | Severity | Impact | +|------|------|---------|----------|--------| +| `.planning/REQUIREMENTS.md` | 100 | NOTE-01 checkbox still `[ ]` (unchecked) despite full implementation + passing tests | ℹ️ Info | Cosmetic/tracking-doc gap only — does not affect goal achievement. No `TBD`/`FIXME`/`XXX`/`TODO`/`HACK`/`PLACEHOLDER` markers found in any of the 7 phase-created source/test files. | + +No blockers. No warnings. Scanned all 7 files created by this phase for debt markers, stub patterns, empty implementations, and hardcoded-empty data — none found. + +### Human Verification Required + +None. This phase is API/service-only (no new UI per CONTEXT.md's "UI hint: no"), all logic is pure-function or DB/Autotask-integration testable via mocks, and all behaviors are covered by automated Vitest assertions plus direct source/schema cross-referencing. No visual, real-time, or subjective-UX behavior to verify. + +### Gaps Summary + +No gaps. All three roadmap success criteria and all PLAN-frontmatter must-haves (truths, artifacts, key links) are verified against the actual codebase — not just SUMMARY.md claims. Both plans' commits (`cc93707`, `226f300`, `b4e05eb`, `17ba0e8`, `34a0269`, `2d410f8`, `e3a9cb5`) exist in git history. The one non-blocking observation is the unchecked NOTE-01 checkbox in REQUIREMENTS.md, which is a tracking-document sync issue, not a code gap — the implementation, tests, and wiring all satisfy the requirement's text. + +--- + +*Verified: 2026-07-16T16:30:00Z* +*Verifier: Claude (gsd-verifier)*