test(15): persist verification report
This commit is contained in:
parent
502e0b95a7
commit
c33b6615c9
1 changed files with 160 additions and 0 deletions
|
|
@ -0,0 +1,160 @@
|
|||
---
|
||||
phase: 15-data-model-detection-ticket-evidence
|
||||
verified: 2026-07-15T12:20:40Z
|
||||
status: passed
|
||||
score: 4/4 must-haves verified
|
||||
overrides_applied: 0
|
||||
---
|
||||
|
||||
# Phase 15: Data Model, Detection & Ticket Evidence Verification Report
|
||||
|
||||
**Phase Goal:** The durable phishing-triage schema exists in Postgres, and Pulse can scan
|
||||
Autotask/Pulse tickets for known phishing/spam-report patterns idempotently, capturing base
|
||||
ticket-level evidence for each candidate.
|
||||
**Verified:** 2026-07-15T12:20:40Z
|
||||
**Status:** passed
|
||||
**Re-verification:** No — initial verification
|
||||
|
||||
## Goal Achievement
|
||||
|
||||
### Observable Truths (ROADMAP Success Criteria)
|
||||
|
||||
| # | Truth | Status | Evidence |
|
||||
|---|-------|--------|----------|
|
||||
| 1 | Migration `097_*.sql` creates `campaigns`, `reports`, `messages`, `indicators`, `classifications`, `remediation_actions`, `audit_events` tables with `IF NOT EXISTS`, ready for later phases | VERIFIED | `migrations/097_phishing_triage_schema.sql` contains exactly 7 `CREATE TABLE IF NOT EXISTS` statements, no DROP/TRUNCATE. **Confirmed applied to live dev Postgres** — `docker exec pulse-postgres psql` returned `7` for `information_schema.tables` count on all 7 names. |
|
||||
| 2 | Running the ticket scanner flags candidates matching the 8 known patterns and persists a `reports` row per candidate | VERIFIED | `matchesPhishingPatterns` in `lib/services/phishing-detector.ts:42-51` implements case-insensitive substring match over `KNOWN_PHISHING_PATTERNS` (exactly 8 entries, verbatim match to DETECT-01 list). `detectPhishingTicket` (lines 176-249) upserts one `reports` row via `INSERT ... ON CONFLICT (ticket_id) DO UPDATE`. 17/17 unit tests pass (`npx vitest run lib/services/phishing-detector.test.ts`), covering all 8 patterns individually plus negative/case-insensitive cases. |
|
||||
| 3 | Re-scanning unchanged tickets does not reprocess/duplicate `reports` rows; a ticket whose data changed IS reprocessed | VERIFIED | `content_hash` (sha256 over title+description only, `computePhishingContentHash` lines 58-65) is compared against the stored value before any write (lines 187-206). Unchanged hash → skip full rewrite (only evidence refreshed, see below). Changed hash → full upsert via `ON CONFLICT (ticket_id)` (DB-enforced by `uq_reports_ticket_id` — confirmed present via `pg_constraint` query against live DB: `uq_reports_ticket_id\|u`). Hash tests (stability, title-change, description-change, null-normalization) pass. |
|
||||
| 4 | Each flagged ticket's evidence includes ticket ID/number, company, requester/reporter, title, description, notes, time entries, attachment metadata (EVID-01) | VERIFIED | `gatherTicketEvidence` (lines 117-163) queries `companies.company_name`, `ticket_notes`, `time_entries`, and Autotask attachment metadata (fullPath/title/contentType, no base64 `data`). `reports` row also carries `ticket_id`, `ticket_number`, `requester_contact_id` (from `ticket.contact_id`), `created_by_contact_id`, `title`, `description` directly. Confirmed live DB `reports` table has all these columns. |
|
||||
|
||||
**Score:** 4/4 truths verified
|
||||
|
||||
### Required Artifacts
|
||||
|
||||
| Artifact | Expected | Status | Details |
|
||||
|----------|----------|--------|---------|
|
||||
| `migrations/097_phishing_triage_schema.sql` | 7-table schema, `reports` fully designed | VERIFIED | Exists, 7 `CREATE TABLE IF NOT EXISTS`, `reports` has `ticket_id BIGINT NOT NULL REFERENCES tickets(id)`, `content_hash TEXT NOT NULL`, `matched_patterns JSONB`, `evidence JSONB`, `CONSTRAINT uq_reports_ticket_id UNIQUE (ticket_id)`. Applied to live dev DB (verified via docker exec). |
|
||||
| `migrations/098_phishing_sweep_schedule.sql` | phishing-sweep `sync_schedules` seed row | VERIFIED | Exists, `INSERT ... ON CONFLICT (id) DO NOTHING`. Applied to live dev DB — `SELECT * FROM sync_schedules WHERE id='phishing-sweep'` returns `phishing-sweep\|phishing-sweep\|f\|0 5 * * *`. |
|
||||
| `lib/services/phishing-detector.ts` | `KNOWN_PHISHING_PATTERNS`, `matchesPhishingPatterns`, `computePhishingContentHash`, `gatherTicketEvidence`, `detectPhishingTicket` | VERIFIED | All 5 exports present (249 lines), wired, tested. |
|
||||
| `lib/services/phishing-detector.test.ts` | Unit tests for matcher + hash, all 8 patterns + negative + stability | VERIFIED | 17 tests, all passing, one assertion per locked pattern by name plus negative/case-insensitive/hash cases. |
|
||||
| `lib/services/phishing-sweep-service.ts` | `sweepPhishingTickets()` bounded reconciliation loop | VERIFIED | 102 lines. Bounded `LIMIT $1` (500), `is_deleted = false` filter, 7-day window, per-row try/catch (no rethrow), calls shared `detectPhishingTicket` only — no duplicated logic. |
|
||||
| `lib/services/webhook-service.ts` (modified) | Fire-and-forget phishing trigger on ticket CREATE | VERIFIED (post-fix) | `triggerPhishingDetection` reads the ticket back from Postgres (not `payload.entity`) after `handleCreateOrUpdate()` has upserted it in the same synchronous flow. See Fix Verification below. |
|
||||
| `lib/services/sync-scheduler.ts` (modified) | `phishing-sweep` sync_type, default schedule, dispatch branch | VERIFIED | Union includes `'phishing-sweep'` (line 25), `defaultSchedules` entry with `is_enabled: false` (lines 302-308), dispatch branch dynamically imports and calls `sweepPhishingTickets`, logs scanned/flagged/skipped/errors summary (lines 472-477). |
|
||||
|
||||
### Key Link Verification
|
||||
|
||||
| From | To | Via | Status | Details |
|
||||
|------|-----|-----|--------|---------|
|
||||
| `reports.ticket_id` | `tickets.id` | foreign key | VERIFIED | `reports_ticket_id_fkey` present in live DB `pg_constraint`. |
|
||||
| `reports.content_hash` | phishing-detector idempotency | unique key on ticket_id + stored hash | VERIFIED | `uq_reports_ticket_id` unique constraint present in live DB; hash comparison gate in `detectPhishingTicket`. |
|
||||
| webhook `ticket.created` handler | `detectPhishingTicket` | fire-and-forget `.catch()` | VERIFIED | `webhook-service.ts:118-120`, not awaited in the request path. |
|
||||
| `sync-scheduler.ts` dispatch | `sweepPhishingTickets` | dynamic `await import()` on `sync_type === 'phishing-sweep'` | VERIFIED | `sync-scheduler.ts:472-477`. |
|
||||
|
||||
### Fix Verification (Critical Bug + Follow-up Warnings)
|
||||
|
||||
The phase 15 code review (`15-REVIEW.md`) found one critical bug (webhook detection never fired
|
||||
against real data) and two warnings, fixed in commits `ecc34b4`, `c875081`, `7c63c5f`. A
|
||||
subsequent re-review (`15-REVIEW.md`, re-review section) found one more warning fixed in
|
||||
`9c4584d`. All four were independently re-verified against the current code (not just SUMMARY
|
||||
claims):
|
||||
|
||||
| Fix | Commit | Verified? | Evidence |
|
||||
|-----|--------|-----------|----------|
|
||||
| CR-01: webhook detection reads real ticket data, not unpopulated `payload.entity` | ecc34b4 | VERIFIED | `triggerPhishingDetection` (webhook-service.ts:457-490) queries `SELECT ... FROM tickets WHERE id = $1` using `payload.entityId`, not `payload.entity`. Guard: if row not found, `console.warn` + early `return` (no throw, no crash). Ordering confirmed: `handleCreateOrUpdate()` is `await`ed at line 95, `triggerPhishingDetection` fires at line 118 — strictly after the ticket upsert on the happy path. |
|
||||
| WR-01: shared `getAutotaskClient` factory replaces private duplicate | c875081 | VERIFIED | `phishing-detector.ts:17` imports `getAutotaskClient` from `./autotask-factory`; no private `_autotaskClient`/duplicate singleton remains in the file (grep confirms only the import + one call site). `autotask-factory.ts:16-20` throws a clear config-validation error when Autotask env vars are missing; that error is caught by the existing try/catch in `gatherTicketEvidence` (lines 145-155), degrading to `attachments: []`. |
|
||||
| WR-02: evidence snapshot refreshed even when content_hash unchanged | 7c63c5f | VERIFIED | `phishing-detector.ts:192-206` — on hash match, still calls `gatherTicketEvidence` and issues a targeted `UPDATE reports SET evidence = $1::jsonb, updated_at = NOW() WHERE ticket_id = $2`, leaving `title`/`description`/`matched_patterns`/`content_hash` untouched. Confirmed the webhook path only fires on CREATE (line 114), so status/assignee churn on an already-detected ticket doesn't reach this branch via webhook; the sweep is the repeated caller and is itself idempotent/hash-gated. |
|
||||
| WR-01 (new, post-re-review): soft-deleted notes/time entries excluded from evidence | 9c4584d | VERIFIED | `gatherTicketEvidence` (phishing-detector.ts:126-142) now filters both the `ticket_notes` and `time_entries` queries with `AND is_deleted = false`. Confirmed both columns actually exist on the live DB (`information_schema.columns` query for both tables returned `is_deleted`). |
|
||||
|
||||
No regressions introduced by any of the four fixes — `npx tsc --noEmit --pretty` is clean and
|
||||
`npx vitest run lib/services/phishing-detector.test.ts` passes 17/17 after all fixes are applied
|
||||
(current HEAD).
|
||||
|
||||
### Database Verification (Live Dev Postgres)
|
||||
|
||||
Both migrations were confirmed **applied to the running `pulse-postgres` container**, not just
|
||||
present as files:
|
||||
|
||||
```
|
||||
docker exec pulse-postgres psql -U pulse_user -d pulse_autotask -c \
|
||||
"SELECT count(*) FROM information_schema.tables WHERE table_name IN
|
||||
('campaigns','reports','messages','indicators','classifications','remediation_actions','audit_events');"
|
||||
→ 7
|
||||
|
||||
docker exec pulse-postgres psql -U pulse_user -d pulse_autotask -c "\d reports"
|
||||
→ all expected columns present (ticket_id, content_hash, matched_patterns, evidence, etc.)
|
||||
|
||||
docker exec pulse-postgres psql -U pulse_user -d pulse_autotask -c \
|
||||
"SELECT id, sync_type, is_enabled, cron_expression FROM sync_schedules WHERE id='phishing-sweep';"
|
||||
→ phishing-sweep | phishing-sweep | f | 0 5 * * *
|
||||
|
||||
pg_constraint on reports → uq_reports_ticket_id (unique), reports_ticket_id_fkey,
|
||||
reports_campaign_id_fkey all present
|
||||
```
|
||||
|
||||
### Requirements Coverage
|
||||
|
||||
| Requirement | Source Plan | Description | Status | Evidence |
|
||||
|-------------|------------|-------------|--------|----------|
|
||||
| DETECT-01 | 15-01, 15-02, 15-03 | Scan recent tickets, flag candidates matching known patterns | SATISFIED | `matchesPhishingPatterns` + webhook/sweep wiring, all 8 patterns tested |
|
||||
| DETECT-02 | 15-01, 15-02, 15-03 | Re-scanning doesn't reprocess unchanged ticket; reprocesses on change | SATISFIED | content-hash gate + `uq_reports_ticket_id` upsert, tested |
|
||||
| EVID-01 | 15-01, 15-02 | Extract ticket ID/number, company, requester/reporter, title, description, notes, time entries, attachment metadata | SATISFIED | `gatherTicketEvidence` + `reports` schema columns, confirmed in live DB |
|
||||
|
||||
No orphaned requirements — REQUIREMENTS.md's Traceability table maps only DETECT-01, DETECT-02,
|
||||
EVID-01 to Phase 15, and all three appear in the plan frontmatters and are satisfied above.
|
||||
|
||||
### Anti-Patterns Found
|
||||
|
||||
None blocking. No `TBD`/`FIXME`/`XXX`/`TODO`/`HACK`/`PLACEHOLDER` markers in any file modified by
|
||||
this phase. No empty stub implementations. No hardcoded empty evidence.
|
||||
|
||||
Two informational items, both explicitly accepted as backlog in `15-REVIEW.md`'s "Post-Review Fix
|
||||
Note" (not blocking phase 15's goal, which is about idempotent detection + evidence capture, not
|
||||
100% webhook delivery guarantees):
|
||||
|
||||
- If Autotask's entity-fetch fails during `handleCreateOrUpdate` (rate limit/transient outage),
|
||||
a genuinely new ticket's row never lands in Postgres before `triggerPhishingDetection` reads it
|
||||
back — detection is silently skipped with only a `console.warn`, and the only backstop is the
|
||||
`phishing-sweep` cron, which ships `is_enabled: false` by default (matches the existing
|
||||
`tickets-reconcile`/`pax8-daily` disabled-by-default convention in this codebase).
|
||||
- The webhook path only triggers detection on ticket CREATE (D-01 explicit decision in
|
||||
`15-CONTEXT.md`), not UPDATE — a ticket retitled into a matching pattern after creation is only
|
||||
caught by the (disabled-by-default) sweep.
|
||||
|
||||
These are real operational caveats worth an admin enabling `phishing-sweep` in production, but
|
||||
they do not block Phase 15's stated goal — the schema exists, detection is idempotent on the
|
||||
paths that do fire, and evidence capture is complete for every ticket that is detected.
|
||||
|
||||
### Behavioral Spot-Checks
|
||||
|
||||
| Behavior | Command | Result | Status |
|
||||
|----------|---------|--------|--------|
|
||||
| Phishing detector pure-function + orchestration test suite | `npx vitest run lib/services/phishing-detector.test.ts` | 17/17 passed | PASS |
|
||||
| Type check across modified files | `npx tsc --noEmit --pretty` | 0 errors | PASS |
|
||||
| All 7 tables present in live dev Postgres | `docker exec pulse-postgres psql ... information_schema.tables` | 7 | PASS |
|
||||
| `reports` unique constraint present in live dev Postgres | `docker exec pulse-postgres psql ... pg_constraint` | `uq_reports_ticket_id\|u` | PASS |
|
||||
| `phishing-sweep` schedule row present in live dev Postgres | `docker exec pulse-postgres psql ... sync_schedules` | row found, `is_enabled=f` | PASS |
|
||||
| Commits referenced in review/fix reports actually exist | `git show --stat <hash>` for ecc34b4, c875081, 7c63c5f, 9c4584d | all 4 found with matching diffs | PASS |
|
||||
|
||||
### Probe Execution
|
||||
|
||||
No `scripts/*/tests/probe-*.sh` files declared or found for this phase; no probe-based
|
||||
verification was specified in the PLAN/SUMMARY files. Step 7c: SKIPPED (no probes declared).
|
||||
|
||||
### Human Verification Required
|
||||
|
||||
None. All must-haves are verifiable via code inspection, unit tests, type-check, and direct
|
||||
querying of the live dev database — no visual/UX/real-time behavior requiring human judgment in
|
||||
this phase (no UI, per `**UI hint**: no` in ROADMAP.md).
|
||||
|
||||
### Gaps Summary
|
||||
|
||||
No gaps. All 4 ROADMAP success criteria are verified against the actual codebase and the live
|
||||
dev database (not just SUMMARY claims). The critical bug found in code review (webhook detection
|
||||
reading an unpopulated field) and all three follow-up warnings were independently re-verified as
|
||||
correctly fixed, with no regressions. Two known operational caveats (sweep disabled by default;
|
||||
CREATE-only webhook trigger) are explicitly accepted as intentional/backlog per the phase's own
|
||||
review trail and do not block the phase goal.
|
||||
|
||||
---
|
||||
|
||||
_Verified: 2026-07-15T12:20:40Z_
|
||||
_Verifier: Claude (gsd-verifier)_
|
||||
Loading…
Add table
Add a link
Reference in a new issue