From 92ed5c0e5e2344af7c4a57a594e556f51f41ded7 Mon Sep 17 00:00:00 2001 From: lorentz Date: Wed, 15 Jul 2026 07:43:43 -0400 Subject: [PATCH] docs(15-02): add plan 02 summary Co-Authored-By: Claude Sonnet 5 --- .../15-02-SUMMARY.md | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 .planning/phases/15-data-model-detection-ticket-evidence/15-02-SUMMARY.md diff --git a/.planning/phases/15-data-model-detection-ticket-evidence/15-02-SUMMARY.md b/.planning/phases/15-data-model-detection-ticket-evidence/15-02-SUMMARY.md new file mode 100644 index 0000000..6c40394 --- /dev/null +++ b/.planning/phases/15-data-model-detection-ticket-evidence/15-02-SUMMARY.md @@ -0,0 +1,100 @@ +--- +phase: 15-data-model-detection-ticket-evidence +plan: 02 +subsystem: services +tags: [detection, phishing-triage, evidence, postgres, autotask, sha256] + +# Dependency graph +requires: + - phase: 15-01 + provides: "reports table (ticket_id UNIQUE FK, content_hash, matched_patterns JSONB, evidence JSONB)" +provides: + - "lib/services/phishing-detector.ts — KNOWN_PHISHING_PATTERNS, matchesPhishingPatterns, computePhishingContentHash, gatherTicketEvidence, detectPhishingTicket" + - "One shared detection entry point (detectPhishingTicket) for both the webhook path and cron sweep in Plan 03" +affects: [15-03-webhook-and-sweep, 16-message-parsing] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Case-insensitive substring matcher via .toLowerCase()+.includes() only (no RegExp/eval) — mirrors robotic-classifier.evaluateContains" + - "sha256 content-hash over only the fields that define reprocessing eligibility (title+description), excluding bump-prone fields — mirrors analyzer/preprocessor.computeContentHash" + - "Check-before-write idempotency: SELECT existing content_hash, skip evidence-gathering and write entirely when unchanged — mirrors analyzer/persistence.findExistingAnalysisByContentHash" + +key-files: + created: [lib/services/phishing-detector.ts, lib/services/phishing-detector.test.ts] + modified: [] + +key-decisions: + - "Split Task 1 pure-logic implementation from Task 2 evidence/orchestration into two separate commits (test -> feat -> feat) even though both live in the same file, so the TDD RED/GREEN gate sequence is unambiguous in git history" + - "Autotask client instantiated as a lazy module-level singleton with env-var config, mirroring ticket-reconciliation-service.ts's getClient() pattern, rather than introducing a shared factory (out of scope for this plan)" + - "Idempotency guard compares stored reports.content_hash to the freshly computed hash BEFORE gathering evidence, so an unchanged ticket never re-queries ticket_notes/time_entries/Autotask attachments" + +patterns-established: + - "Pattern: detector modules expose pure matching/hashing functions separately from the async DB/API orchestration function, so vitest can cover the pure logic without mocking postgresClient or AutotaskClient" + +requirements-completed: [DETECT-01, DETECT-02, EVID-01] + +# Metrics +duration: 13min +completed: 2026-07-15 +--- + +# Phase 15 Plan 02: Phishing Detector Summary + +**`lib/services/phishing-detector.ts` — a single deterministic detection core matching 8 locked DETECT-01 patterns, sha256 content-hashing for D-04 idempotent reprocessing, and EVID-01 evidence capture (company/notes/time-entries/attachment-metadata) upserted into the Plan 01 `reports` table via `ON CONFLICT (ticket_id)`.** + +## Performance + +- **Duration:** 13 min +- **Started:** 2026-07-15T11:41:00Z +- **Completed:** 2026-07-15T11:47:59Z +- **Tasks:** 2 completed +- **Files modified:** 2 + +## Accomplishments +- Implemented `KNOWN_PHISHING_PATTERNS` (the 8 locked DETECT-01 strings verbatim) and `matchesPhishingPatterns` — case-insensitive substring matching (`.toLowerCase()` + `.includes()` only, no `RegExp`/`eval`), returning both a `flagged` boolean and the exact subset of patterns present. +- Implemented `computePhishingContentHash` — sha256 over `{ title, description }` only, stable for identical input, changes on either field, and normalizes `null` description to `''`. +- Wrote and ran a 17-assertion vitest suite (`phishing-detector.test.ts`) covering all 8 patterns individually, the negative case, case-insensitivity, matched[] exactness, and hash stability/change/null-normalization — RED confirmed (module didn't exist) before GREEN implementation. +- Implemented `gatherTicketEvidence` — parameterized `$1` queries against `companies`, `ticket_notes`, and `time_entries`, plus Autotask attachment metadata (`fullPath`/`title`/`contentType` only, never base64 `data`), with the Autotask call wrapped in try/catch so an API failure degrades to an empty attachments array instead of throwing. +- Implemented `detectPhishingTicket` — the shared orchestration entry point: matches, hashes, checks the D-04 idempotency guard (SELECT existing `content_hash`, skip gathering/writing when unchanged), then upserts one `reports` row via `ON CONFLICT (ticket_id) DO UPDATE ... RETURNING id`, binding `requester_contact_id` from `ticket.contact_id` and `created_by_contact_id` from `ticket.created_by_contact_id`. + +## Task Commits + +Each task was committed atomically, with Task 1 following the full TDD RED/GREEN gate sequence: + +1. **Task 1 (RED): add failing tests for pattern matcher + content hash** - `0e7daf9` (test) +2. **Task 1 (GREEN): implement phishing pattern matcher + content hash** - `aabf532` (feat) +3. **Task 2: add evidence capture + detectPhishingTicket orchestration** - `15d0caa` (feat) + +**Plan metadata:** (this SUMMARY.md commit) + +_Note: Task 1 is TDD — test → feat. No REFACTOR commit was needed; the GREEN implementation was already clean._ + +## Files Created/Modified +- `lib/services/phishing-detector.ts` - Pure matcher/hash functions (`KNOWN_PHISHING_PATTERNS`, `matchesPhishingPatterns`, `computePhishingContentHash`) plus evidence capture and orchestration (`gatherTicketEvidence`, `detectPhishingTicket`) — the shared detection core for Plan 03's webhook and cron-sweep callers +- `lib/services/phishing-detector.test.ts` - 17 vitest assertions covering all 8 locked patterns individually, negative case, case-insensitivity, matched[] exactness, and content-hash stability/change/null-normalization + +## Decisions Made +- Split Task 1's pure-logic commit from Task 2's evidence/orchestration commit even though both extend the same file, so the RED (`test(...)`) → GREEN (`feat(...)`) gate sequence required by the TDD workflow is unambiguous in `git log`, and Task 2's orchestration work is its own reviewable `feat(...)` commit. +- Reused the `ticket-reconciliation-service.ts` lazy-singleton pattern for the AutotaskClient (env-var config, module-level cache) rather than introducing a new factory — no existing `getAutotaskClient()`/`isAutotaskConfigured()` factory was present to reuse, and adding one was out of scope for this plan. +- Idempotency check queries only `id, content_hash` from `reports` (not the full row) and returns immediately on a hash match, before any evidence-gathering queries run — this is what makes the D-04 guarantee ("reprocessing only when content hash changed") cheap for the common case of an unchanged ticket being re-scanned by the cron sweep. + +## Deviations from Plan + +None - plan executed exactly as written. The plan's own Task 1 instruction ("Do not add DB access in this task — pure functions only") was honored by writing only the pure matcher/hash functions in the first commit, then extending the same file with DB/API-touching code in Task 2's commit, exactly as the plan's two-task structure specifies. + +## Issues Encountered +None. + +## User Setup Required +None - no external service configuration required. The detector uses the existing `AUTOTASK_*` env vars already configured elsewhere in the codebase (no new credentials introduced). + +## Next Phase Readiness +- `detectPhishingTicket(ticket)` is ready to be called from both the Autotask webhook handler and a cron sweep in Plan 03 — same underlying logic, no duplicated matching/hashing/idempotency code between the two callers. +- The reports upsert path is fully wired against the Plan 01 schema (`ON CONFLICT (ticket_id)`, `content_hash`, `matched_patterns` JSONB, `evidence` JSONB) — verified via `npx tsc --noEmit --pretty` and the passing vitest suite; no live DB write was exercised in this plan (that happens when Plan 03 wires a real ticket through the detector against the dev Postgres instance). +- No blockers for Plan 03. + +--- +*Phase: 15-data-model-detection-ticket-evidence* +*Completed: 2026-07-15*